diff options
author | Robert Speicher <robert@gitlab.com> | 2017-08-08 00:33:45 +0300 |
---|---|---|
committer | Robert Speicher <robert@gitlab.com> | 2017-08-08 00:33:45 +0300 |
commit | 475f41acd4a8e0666736d40feb4d8945cf2c306a (patch) | |
tree | 73288e5750c8a212ad5394a2fb0f7090237a6289 | |
parent | 6085ce1352eee7b3e18b014f0f68719cae780da8 (diff) | |
parent | c0b41064ff1eb4c5465d3c2a9efa0a22b60c3f4c (diff) |
Merge branch 'feature/migrate-find-commits-by-message-to-gitaly' into 'master'
Migrate Repository#find_commits_by_message to Gitaly
Closes gitaly#443
See merge request !13268
-rw-r--r-- | app/models/repository.rb | 39 | ||||
-rw-r--r-- | lib/gitlab/gitaly_client/commit_service.rb | 14 | ||||
-rw-r--r-- | spec/models/repository_spec.rb | 30 |
3 files changed, 63 insertions, 20 deletions
diff --git a/app/models/repository.rb b/app/models/repository.rb index f86a0869b01..3b5d0e00c70 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -130,17 +130,13 @@ class Repository return [] end - ref ||= root_ref - - args = %W( - log #{ref} --pretty=%H --skip #{offset} - --max-count #{limit} --grep=#{query} --regexp-ignore-case - ) - args = args.concat(%W(-- #{path})) if path.present? - - git_log_results = run_git(args).first.lines - - git_log_results.map { |c| commit(c.chomp) }.compact + raw_repository.gitaly_migrate(:commits_by_message) do |is_enabled| + if is_enabled + find_commits_by_message_by_gitaly(query, ref, path, limit, offset) + else + find_commits_by_message_by_shelling_out(query, ref, path, limit, offset) + end + end end def find_branch(name, fresh_repo: true) @@ -1184,4 +1180,25 @@ class Repository def circuit_breaker @circuit_breaker ||= Gitlab::Git::Storage::CircuitBreaker.for_storage(project.repository_storage) end + + def find_commits_by_message_by_shelling_out(query, ref, path, limit, offset) + ref ||= root_ref + + args = %W( + log #{ref} --pretty=%H --skip #{offset} + --max-count #{limit} --grep=#{query} --regexp-ignore-case + ) + args = args.concat(%W(-- #{path})) if path.present? + + git_log_results = run_git(args).first.lines + + git_log_results.map { |c| commit(c.chomp) }.compact + end + + def find_commits_by_message_by_gitaly(query, ref, path, limit, offset) + raw_repository + .gitaly_commit_client + .commits_by_message(query, revision: ref, path: path, limit: limit, offset: offset) + .map { |c| commit(c) } + end end diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index ac6817e6d0e..3f577ac8530 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -135,6 +135,20 @@ module Gitlab consume_commits_response(response) end + def commits_by_message(query, revision: '', path: '', limit: 1000, offset: 0) + request = Gitaly::CommitsByMessageRequest.new( + repository: @gitaly_repo, + query: query, + revision: revision.to_s.force_encoding(Encoding::ASCII_8BIT), + path: path.to_s.force_encoding(Encoding::ASCII_8BIT), + limit: limit.to_i, + offset: offset.to_i + ) + + response = GitalyClient.call(@repository.storage, :commit_service, :commits_by_message, request) + consume_commits_response(response) + end + def languages(ref = nil) request = Gitaly::CommitLanguagesRequest.new(repository: @gitaly_repo, revision: ref || '') response = GitalyClient.call(@repository.storage, :commit_service, :commit_languages, request) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 4ddda5b638c..cfa77648338 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -234,19 +234,31 @@ describe Repository, models: true do end describe '#find_commits_by_message' do - it 'returns commits with messages containing a given string' do - commit_ids = repository.find_commits_by_message('submodule').map(&:id) + shared_examples 'finding commits by message' do + it 'returns commits with messages containing a given string' do + commit_ids = repository.find_commits_by_message('submodule').map(&:id) - expect(commit_ids).to include('5937ac0a7beb003549fc5fd26fc247adbce4a52e') - expect(commit_ids).to include('6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') - expect(commit_ids).to include('cfe32cf61b73a0d5e9f13e774abde7ff789b1660') - expect(commit_ids).not_to include('913c66a37b4a45b9769037c55c2d238bd0942d2e') + expect(commit_ids).to include( + '5937ac0a7beb003549fc5fd26fc247adbce4a52e', + '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9', + 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' + ) + expect(commit_ids).not_to include('913c66a37b4a45b9769037c55c2d238bd0942d2e') + end + + it 'is case insensitive' do + commit_ids = repository.find_commits_by_message('SUBMODULE').map(&:id) + + expect(commit_ids).to include('5937ac0a7beb003549fc5fd26fc247adbce4a52e') + end end - it 'is case insensitive' do - commit_ids = repository.find_commits_by_message('SUBMODULE').map(&:id) + context 'when Gitaly commits_by_message feature is enabled' do + it_behaves_like 'finding commits by message' + end - expect(commit_ids).to include('5937ac0a7beb003549fc5fd26fc247adbce4a52e') + context 'when Gitaly commits_by_message feature is disabled', skip_gitaly_mock: true do + it_behaves_like 'finding commits by message' end describe 'when storage is broken', broken_storage: true do |