Commit c0b41064 authored by Ahmad Sherif's avatar Ahmad Sherif

Migrate Repository#find_commits_by_message to Gitaly

Closes gitaly#443
parent b12107a0
......@@ -391,7 +391,7 @@ gem 'vmstat', '~> 2.3.0'
gem 'sys-filesystem', '~> 1.1.6'
# Gitaly GRPC client
gem 'gitaly', '~> 0.24.0'
gem 'gitaly', '~> 0.26.0'
gem 'toml-rb', '~> 0.3.15', require: false
......
......@@ -269,7 +269,7 @@ GEM
po_to_json (>= 1.0.0)
rails (>= 3.2.0)
gherkin-ruby (0.3.2)
gitaly (0.24.0)
gitaly (0.26.0)
google-protobuf (~> 3.1)
grpc (~> 1.0)
github-linguist (4.7.6)
......@@ -975,7 +975,7 @@ DEPENDENCIES
gettext (~> 3.2.2)
gettext_i18n_rails (~> 1.8.0)
gettext_i18n_rails_js (~> 1.2.0)
gitaly (~> 0.24.0)
gitaly (~> 0.26.0)
github-linguist (~> 4.7.0)
gitlab-flowdock-git-hook (~> 1.0.1)
gitlab-markup (~> 1.5.1)
......
......@@ -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
......@@ -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)
......
......@@ -234,12 +234,15 @@ describe Repository, models: true do
end
describe '#find_commits_by_message' do
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).to include(
'5937ac0a7beb003549fc5fd26fc247adbce4a52e',
'6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9',
'cfe32cf61b73a0d5e9f13e774abde7ff789b1660'
)
expect(commit_ids).not_to include('913c66a37b4a45b9769037c55c2d238bd0942d2e')
end
......@@ -248,6 +251,15 @@ describe Repository, models: true do
expect(commit_ids).to include('5937ac0a7beb003549fc5fd26fc247adbce4a52e')
end
end
context 'when Gitaly commits_by_message feature is enabled' do
it_behaves_like 'finding commits by message'
end
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
it 'should raise a storage error' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment