diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index 5767f06b0ce..269a048cf5d 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -222,14 +222,25 @@ module Gitlab end def find_commit(revision) - request = Gitaly::FindCommitRequest.new( - repository: @gitaly_repo, - revision: encode_binary(revision) - ) + if RequestStore.active? + # We don't use RequeStstore.fetch(key) { ... } directly because `revision` + # can be a branch name, so we can't use it as a key as it could point + # to another commit later on (happens a lot in tests). + key = { + storage: @gitaly_repo.storage_name, + relative_path: @gitaly_repo.relative_path, + commit_id: revision + } + return RequestStore[key] if RequestStore.exist?(key) - response = GitalyClient.call(@repository.storage, :commit_service, :find_commit, request, timeout: GitalyClient.medium_timeout) + commit = call_find_commit(revision) + return unless commit - response.commit + key[:commit_id] = commit.id + RequestStore[key] = commit + else + call_find_commit(revision) + end end def patch(revision) @@ -346,6 +357,17 @@ module Gitlab def encode_repeated(a) Google::Protobuf::RepeatedField.new(:bytes, a.map { |s| encode_binary(s) } ) end + + def call_find_commit(revision) + request = Gitaly::FindCommitRequest.new( + repository: @gitaly_repo, + revision: encode_binary(revision) + ) + + response = GitalyClient.call(@repository.storage, :commit_service, :find_commit, request, timeout: GitalyClient.medium_timeout) + + response.commit + end end end end diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index 3722a91c050..001c4d3e10a 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -166,6 +166,32 @@ describe Gitlab::GitalyClient::CommitService do described_class.new(repository).find_commit(revision) end + + describe 'caching', :request_store do + let(:commit_dbl) { double(id: 'f01b' * 10) } + + context 'when passed revision is a branch name' do + it 'calls Gitaly' do + expect_any_instance_of(Gitaly::CommitService::Stub).to receive(:find_commit).twice.and_return(double(commit: commit_dbl)) + + commit = nil + 2.times { commit = described_class.new(repository).find_commit('master') } + + expect(commit).to eq(commit_dbl) + end + end + + context 'when passed revision is a commit ID' do + it 'returns a cached commit' do + expect_any_instance_of(Gitaly::CommitService::Stub).to receive(:find_commit).once.and_return(double(commit: commit_dbl)) + + commit = nil + 2.times { commit = described_class.new(repository).find_commit('f01b' * 10) } + + expect(commit).to eq(commit_dbl) + end + end + end end describe '#patch' do