summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJohn Cai <jcai@gitlab.com>2019-01-02 15:39:47 -0800
committerJohn Cai <jcai@gitlab.com>2019-02-05 23:45:06 -0800
commitc3e66ad0933ecefbfa7298387c8fb77dc6e6abdc (patch)
tree50191db60ddab4701d92231a321f374709f3f85a
parentd0187de202d94fa445a28c347b7a54dbf09a22a8 (diff)
downloadgitlab-ce-support-chunking-in-client.tar.gz
Modifying gitaly search files client to add chunking supportsupport-chunking-in-client
updates gitaly proto to 1.7.0, modifies the search files gitaly client call to use the new chunked_response flag in the rpc request, and stitch the responses together. maintains backwards compatibility with older gitaly servers.
-rw-r--r--Gemfile2
-rw-r--r--Gemfile.lock4
-rw-r--r--changelogs/unreleased/support-chunking-in-client.yml5
-rw-r--r--lib/gitlab/gitaly_client/repository_service.rb33
-rw-r--r--spec/lib/gitlab/git/repository_spec.rb77
5 files changed, 102 insertions, 19 deletions
diff --git a/Gemfile b/Gemfile
index a2527b17b70..98189534849 100644
--- a/Gemfile
+++ b/Gemfile
@@ -422,7 +422,7 @@ group :ed25519 do
end
# Gitaly GRPC client
-gem 'gitaly-proto', '~> 1.5.0', require: 'gitaly'
+gem 'gitaly-proto', '~> 1.7.0', require: 'gitaly'
gem 'grpc', '~> 1.15.0'
gem 'google-protobuf', '~> 3.6'
diff --git a/Gemfile.lock b/Gemfile.lock
index f661da41507..98d00e612d6 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -278,7 +278,7 @@ GEM
gettext_i18n_rails (>= 0.7.1)
po_to_json (>= 1.0.0)
rails (>= 3.2.0)
- gitaly-proto (1.5.0)
+ gitaly-proto (1.7.0)
grpc (~> 1.0)
github-markup (1.7.0)
gitlab-default_value_for (3.1.1)
@@ -1020,7 +1020,7 @@ DEPENDENCIES
gettext (~> 3.2.2)
gettext_i18n_rails (~> 1.8.0)
gettext_i18n_rails_js (~> 1.3)
- gitaly-proto (~> 1.5.0)
+ gitaly-proto (~> 1.7.0)
github-markup (~> 1.7.0)
gitlab-default_value_for (~> 3.1.1)
gitlab-markup (~> 1.6.5)
diff --git a/changelogs/unreleased/support-chunking-in-client.yml b/changelogs/unreleased/support-chunking-in-client.yml
new file mode 100644
index 00000000000..e50648ea4b2
--- /dev/null
+++ b/changelogs/unreleased/support-chunking-in-client.yml
@@ -0,0 +1,5 @@
+---
+title: Fix code search when text is larger than max gRPC message size
+merge_request: 24111
+author:
+type: changed
diff --git a/lib/gitlab/gitaly_client/repository_service.rb b/lib/gitlab/gitaly_client/repository_service.rb
index 8a1abfbf874..a7e20d9429e 100644
--- a/lib/gitlab/gitaly_client/repository_service.rb
+++ b/lib/gitlab/gitaly_client/repository_service.rb
@@ -324,13 +324,40 @@ 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)
- request = Gitaly::SearchFilesByContentRequest.new(repository: @gitaly_repo, ref: ref, query: query)
- GitalyClient.call(@storage, :repository_service, :search_files_by_content, request).flat_map(&:matches)
+ def search_files_by_content(ref, query, chunked_response: true)
+ request = Gitaly::SearchFilesByContentRequest.new(repository: @gitaly_repo, ref: ref, query: query, chunked_response: chunked_response)
+ response = GitalyClient.call(@storage, :repository_service, :search_files_by_content, request)
+
+ search_results_from_response(response)
end
private
+ def search_results_from_response(gitaly_response)
+ matches = []
+ current_match = +""
+
+ gitaly_response.each do |message|
+ next if message.nil?
+
+ # Old client will ignore :chunked_response flag
+ # and return messages with `matches` key.
+ # This code path will be removed post 12.0 release
+ if message.matches.any?
+ matches += message.matches
+ else
+ current_match << message.match_data
+
+ if message.end_of_match
+ matches << current_match
+ current_match = +""
+ end
+ end
+ end
+
+ matches
+ end
+
def gitaly_fetch_stream_to_file(save_path, rpc_name, request_class, timeout)
request = request_class.new(repository: @gitaly_repo)
response = GitalyClient.call(
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index a19e3e84f83..e668858549b 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -20,6 +20,8 @@ describe Gitlab::Git::Repository, :seed_helper do
end
let(:mutable_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') }
+ let(:mutable_repository_path) { File.join(TestEnv.repos_path, mutable_repository.relative_path) }
+ let(:mutable_repository_rugged) { Rugged::Repository.new(mutable_repository_path) }
let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') }
let(:repository_path) { File.join(TestEnv.repos_path, repository.relative_path) }
let(:repository_rugged) { Rugged::Repository.new(repository_path) }
@@ -497,6 +499,48 @@ describe Gitlab::Git::Repository, :seed_helper do
end
end
+ describe '#search_files_by_content' do
+ let(:repository) { mutable_repository }
+ let(:repository_rugged) { mutable_repository_rugged }
+
+ before do
+ repository.create_branch('search-files-by-content-branch', 'master')
+ new_commit_edit_new_file_on_branch(repository_rugged, 'encoding/CHANGELOG', 'search-files-by-content-branch', 'committing something', 'search-files-by-content change')
+ new_commit_edit_new_file_on_branch(repository_rugged, 'anotherfile', 'search-files-by-content-branch', 'committing something', 'search-files-by-content change')
+ end
+
+ after do
+ ensure_seeds
+ end
+
+ shared_examples 'search files by content' do
+ it 'should have 2 items' do
+ expect(search_results.size).to eq(2)
+ end
+
+ it 'should have the correct matching line' do
+ expect(search_results).to contain_exactly("search-files-by-content-branch:encoding/CHANGELOG\u00001\u0000search-files-by-content change\n",
+ "search-files-by-content-branch:anotherfile\u00001\u0000search-files-by-content change\n")
+ end
+ end
+
+ it_should_behave_like 'search files by content' do
+ let(:search_results) do
+ repository.search_files_by_content('search-files-by-content', 'search-files-by-content-branch')
+ end
+ end
+
+ it_should_behave_like 'search files by content' do
+ let(:search_results) do
+ repository.gitaly_repository_client.search_files_by_content(
+ 'search-files-by-content-branch',
+ 'search-files-by-content',
+ chunked_response: false
+ )
+ end
+ end
+ end
+
describe '#find_remote_root_ref' do
it 'gets the remote root ref from GitalyClient' do
expect_any_instance_of(Gitlab::GitalyClient::RemoteService)
@@ -544,7 +588,7 @@ describe Gitlab::Git::Repository, :seed_helper do
# Add new commits so that there's a renamed file in the commit history
@commit_with_old_name_id = new_commit_edit_old_file(repository_rugged).oid
@rename_commit_id = new_commit_move_file(repository_rugged).oid
- @commit_with_new_name_id = new_commit_edit_new_file(repository_rugged).oid
+ @commit_with_new_name_id = new_commit_edit_new_file(repository_rugged, "encoding/CHANGELOG", "Edit encoding/CHANGELOG", "I'm a new changelog with different text").oid
end
after do
@@ -1964,7 +2008,7 @@ describe Gitlab::Git::Repository, :seed_helper do
end
# Build the options hash that's passed to Rugged::Commit#create
- def commit_options(repo, index, message)
+ def commit_options(repo, index, target, ref, message)
options = {}
options[:tree] = index.write_tree(repo)
options[:author] = {
@@ -1978,8 +2022,8 @@ describe Gitlab::Git::Repository, :seed_helper do
time: Time.gm(2014, "mar", 3, 20, 15, 1)
}
options[:message] ||= message
- options[:parents] = repo.empty? ? [] : [repo.head.target].compact
- options[:update_ref] = "HEAD"
+ options[:parents] = repo.empty? ? [] : [target].compact
+ options[:update_ref] = ref
options
end
@@ -1995,6 +2039,8 @@ describe Gitlab::Git::Repository, :seed_helper do
options = commit_options(
repo,
index,
+ repo.head.target,
+ "HEAD",
"Edit CHANGELOG in its original location"
)
@@ -2003,19 +2049,24 @@ describe Gitlab::Git::Repository, :seed_helper do
end
# Writes a new commit to the repo and returns a Rugged::Commit. Replaces the
- # contents of encoding/CHANGELOG with new text.
- def new_commit_edit_new_file(repo)
- oid = repo.write("I'm a new changelog with different text", :blob)
+ # contents of the specified file_path with new text.
+ def new_commit_edit_new_file(repo, file_path, commit_message, text, branch = repo.head)
+ oid = repo.write(text, :blob)
index = repo.index
- index.read_tree(repo.head.target.tree)
- index.add(path: "encoding/CHANGELOG", oid: oid, mode: 0100644)
-
- options = commit_options(repo, index, "Edit encoding/CHANGELOG")
-
+ index.read_tree(branch.target.tree)
+ index.add(path: file_path, oid: oid, mode: 0100644)
+ options = commit_options(repo, index, branch.target, branch.canonical_name, commit_message)
sha = Rugged::Commit.create(repo, options)
repo.lookup(sha)
end
+ # Writes a new commit to the repo and returns a Rugged::Commit. Replaces the
+ # contents of encoding/CHANGELOG with new text.
+ def new_commit_edit_new_file_on_branch(repo, file_path, branch_name, commit_message, text)
+ branch = repo.branches[branch_name]
+ new_commit_edit_new_file(repo, file_path, commit_message, text, branch)
+ end
+
# Writes a new commit to the repo and returns a Rugged::Commit. Moves the
# CHANGELOG file to the encoding/ directory.
def new_commit_move_file(repo)
@@ -2027,7 +2078,7 @@ describe Gitlab::Git::Repository, :seed_helper do
index.add(path: "encoding/CHANGELOG", oid: oid, mode: 0100644)
index.remove("CHANGELOG")
- options = commit_options(repo, index, "Move CHANGELOG to encoding/")
+ options = commit_options(repo, index, repo.head.target, "HEAD", "Move CHANGELOG to encoding/")
sha = Rugged::Commit.create(repo, options)
repo.lookup(sha)