summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/models/repository.rb4
-rw-r--r--changelogs/unreleased/search-scope-blobs-fix.yml5
-rw-r--r--lib/gitlab/file_finder.rb9
-rw-r--r--lib/gitlab/git/repository.rb4
-rw-r--r--lib/gitlab/gitaly_client/repository_service.rb12
-rw-r--r--lib/gitlab/project_search_results.rb22
-rw-r--r--spec/lib/gitlab/project_search_results_spec.rb33
7 files changed, 67 insertions, 22 deletions
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
View Lorry Controller Status