From cfaf1cca4403b826af2286b1ab0a69ad01c58738 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Sun, 16 Feb 2020 21:08:53 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- app/models/repository.rb | 4 +-- .../unreleased/search-scope-blobs-fix.yml | 5 +++ lib/gitlab/file_finder.rb | 9 ++--- lib/gitlab/git/repository.rb | 4 +-- .../gitaly_client/repository_service.rb | 12 +++++-- lib/gitlab/project_search_results.rb | 22 ++++++++----- .../lib/gitlab/project_search_results_spec.rb | 33 +++++++++++++++++-- 7 files changed, 67 insertions(+), 22 deletions(-) create mode 100644 changelogs/unreleased/search-scope-blobs-fix.yml diff --git a/app/models/repository.rb b/app/models/repository.rb index 37aceeae5f8..cddffa9bb1d 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -993,10 +993,10 @@ class Repository raw_repository.ls_files(actual_ref) end - def search_files_by_content(query, ref) + def search_files_by_content(query, ref, options = {}) return [] if empty? || query.blank? - raw_repository.search_files_by_content(query, ref) + raw_repository.search_files_by_content(query, ref, options) end def search_files_by_name(query, ref) diff --git a/changelogs/unreleased/search-scope-blobs-fix.yml b/changelogs/unreleased/search-scope-blobs-fix.yml new file mode 100644 index 00000000000..71b63c40714 --- /dev/null +++ b/changelogs/unreleased/search-scope-blobs-fix.yml @@ -0,0 +1,5 @@ +--- +title: Fix blobs search API degradation +merge_request: 24607 +author: +type: fixed diff --git a/lib/gitlab/file_finder.rb b/lib/gitlab/file_finder.rb index a71baadfdb3..d438b0415fa 100644 --- a/lib/gitlab/file_finder.rb +++ b/lib/gitlab/file_finder.rb @@ -13,14 +13,15 @@ module Gitlab @ref = ref end - def find(query) + def find(query, content_match_cutoff: nil) query = Gitlab::Search::Query.new(query, encode_binary: true) do filter :filename, matcher: ->(filter, blob) { blob.binary_path =~ /#{filter[:regex_value]}$/i } filter :path, matcher: ->(filter, blob) { blob.binary_path =~ /#{filter[:regex_value]}/i } filter :extension, matcher: ->(filter, blob) { blob.binary_path =~ /\.#{filter[:regex_value]}$/i } end - files = find_by_path(query.term) + find_by_content(query.term) + content_match_cutoff = nil if query.filters.any? + files = find_by_path(query.term) + find_by_content(query.term, { limit: content_match_cutoff }) files = query.filter_results(files) if query.filters.any? @@ -29,8 +30,8 @@ module Gitlab private - def find_by_content(query) - repository.search_files_by_content(query, ref).map do |result| + def find_by_content(query, options) + repository.search_files_by_content(query, ref, options).map do |result| Gitlab::Search::FoundBlob.new(content_match: result, project: project, ref: ref, repository: repository) end end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 840a70e3ae5..6bfe744a5cd 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -956,13 +956,13 @@ module Gitlab gitaly_ref_client.tag_names_contains_sha(sha) end - def search_files_by_content(query, ref) + def search_files_by_content(query, ref, options = {}) return [] if empty? || query.blank? safe_query = Regexp.escape(query) ref ||= root_ref - gitaly_repository_client.search_files_by_content(ref, safe_query) + gitaly_repository_client.search_files_by_content(ref, safe_query, options) end def can_be_merged?(source_sha, target_branch) diff --git a/lib/gitlab/gitaly_client/repository_service.rb b/lib/gitlab/gitaly_client/repository_service.rb index d0e5e0db830..597ae4651ea 100644 --- a/lib/gitlab/gitaly_client/repository_service.rb +++ b/lib/gitlab/gitaly_client/repository_service.rb @@ -332,11 +332,11 @@ module Gitlab GitalyClient.call(@storage, :repository_service, :search_files_by_name, request, timeout: GitalyClient.fast_timeout).flat_map(&:files) end - def search_files_by_content(ref, query) + def search_files_by_content(ref, query, options = {}) request = Gitaly::SearchFilesByContentRequest.new(repository: @gitaly_repo, ref: ref, query: query) response = GitalyClient.call(@storage, :repository_service, :search_files_by_content, request, timeout: GitalyClient.default_timeout) - search_results_from_response(response) + search_results_from_response(response, options) end def disconnect_alternates @@ -361,18 +361,24 @@ module Gitlab private - def search_results_from_response(gitaly_response) + def search_results_from_response(gitaly_response, options = {}) + limit = options[:limit] + matches = [] + matches_count = 0 current_match = +"" gitaly_response.each do |message| next if message.nil? + break if limit && matches_count >= limit + current_match << message.match_data if message.end_of_match matches << current_match current_match = +"" + matches_count += 1 end end diff --git a/lib/gitlab/project_search_results.rb b/lib/gitlab/project_search_results.rb index 60a2efdd849..eb7ca80dd60 100644 --- a/lib/gitlab/project_search_results.rb +++ b/lib/gitlab/project_search_results.rb @@ -2,7 +2,7 @@ module Gitlab class ProjectSearchResults < SearchResults - attr_reader :project, :repository_ref + attr_reader :project, :repository_ref, :per_page def initialize(current_user, project, query, repository_ref = nil, per_page: 20) @current_user = current_user @@ -17,7 +17,7 @@ module Gitlab when 'notes' notes.page(page).per(per_page) when 'blobs' - paginated_blobs(blobs, page) + paginated_blobs(blobs(page), page) when 'wiki_blobs' paginated_blobs(wiki_blobs, page) when 'commits' @@ -32,7 +32,7 @@ module Gitlab def formatted_count(scope) case scope when 'blobs' - blobs_count.to_s + formatted_limited_count(limited_blobs_count) when 'notes' formatted_limited_count(limited_notes_count) when 'wiki_blobs' @@ -48,8 +48,8 @@ module Gitlab super.where(id: @project.team.members) # rubocop:disable CodeReuse/ActiveRecord end - def blobs_count - @blobs_count ||= blobs.count + def limited_blobs_count + @limited_blobs_count ||= blobs.count end # rubocop: disable CodeReuse/ActiveRecord @@ -81,7 +81,7 @@ module Gitlab counts = %i(limited_milestones_count limited_notes_count limited_merge_requests_count limited_issues_count - blobs_count wiki_blobs_count) + limited_blobs_count wiki_blobs_count) counts.all? { |count_method| public_send(count_method).zero? } # rubocop:disable GitlabSecurity/PublicSend end @@ -95,10 +95,16 @@ module Gitlab results end - def blobs + def limit_up_to_page(page) + current_page = page&.to_i || 1 + offset = per_page * (current_page - 1) + count_limit + offset + end + + def blobs(page = 1) return [] unless Ability.allowed?(@current_user, :download_code, @project) - @blobs ||= Gitlab::FileFinder.new(project, repository_project_ref).find(query) + @blobs ||= Gitlab::FileFinder.new(project, repository_project_ref).find(query, content_match_cutoff: limit_up_to_page(page)) end def wiki_blobs diff --git a/spec/lib/gitlab/project_search_results_spec.rb b/spec/lib/gitlab/project_search_results_spec.rb index ae4c14e4deb..d206d31eb96 100644 --- a/spec/lib/gitlab/project_search_results_spec.rb +++ b/spec/lib/gitlab/project_search_results_spec.rb @@ -31,7 +31,7 @@ describe Gitlab::ProjectSearchResults do let(:results) { described_class.new(user, project, query) } where(:scope, :count_method, :expected) do - 'blobs' | :blobs_count | '1234' + 'blobs' | :limited_blobs_count | max_limited_count 'notes' | :limited_notes_count | max_limited_count 'wiki_blobs' | :wiki_blobs_count | '1234' 'commits' | :commits_count | '1234' @@ -141,9 +141,9 @@ describe Gitlab::ProjectSearchResults do describe 'blob search' do let(:project) { create(:project, :public, :repository) } + let(:blob_type) { 'blobs' } it_behaves_like 'general blob search', 'repository', 'blobs' do - let(:blob_type) { 'blobs' } let(:disabled_project) { create(:project, :public, :repository, :repository_disabled) } let(:private_project) { create(:project, :public, :repository, :repository_private) } let(:expected_file_by_path) { 'files/images/wm.svg' } @@ -151,9 +151,36 @@ describe Gitlab::ProjectSearchResults do end it_behaves_like 'blob search repository ref', 'project' do - let(:blob_type) { 'blobs' } let(:entity) { project } end + + context 'pagination' do + let(:per_page) { 20 } + let(:count_limit) { described_class::COUNT_LIMIT } + let(:file_finder) { instance_double('Gitlab::FileFinder') } + let(:results) { described_class.new(user, project, query, per_page: per_page) } + let(:repository_ref) { 'master' } + + before do + allow(file_finder).to receive(:find).and_return([]) + expect(Gitlab::FileFinder).to receive(:new).with(project, repository_ref).and_return(file_finder) + end + + it 'limits search results based on the first page' do + expect(file_finder).to receive(:find).with(query, content_match_cutoff: count_limit) + results.objects(blob_type, 1) + end + + it 'limits search results based on the second page' do + expect(file_finder).to receive(:find).with(query, content_match_cutoff: count_limit + per_page) + results.objects(blob_type, 2) + end + + it 'limits search results based on the third page' do + expect(file_finder).to receive(:find).with(query, content_match_cutoff: count_limit + per_page * 2) + results.objects(blob_type, 3) + end + end end describe 'wiki search' do