diff --git a/lib/gitlab/git/blob.rb b/lib/gitlab/git/blob.rb index f421bf69e8f..81e46028752 100644 --- a/lib/gitlab/git/blob.rb +++ b/lib/gitlab/git/blob.rb @@ -34,7 +34,7 @@ module Gitlab def raw(repository, sha) Gitlab::GitalyClient.migrate(:git_blob_raw) do |is_enabled| if is_enabled - Gitlab::GitalyClient::BlobService.new(repository).get_blob(oid: sha, limit: MAX_DATA_DISPLAY_SIZE) + repository.gitaly_blob_client.get_blob(oid: sha, limit: MAX_DATA_DISPLAY_SIZE) else rugged_raw(repository, sha, limit: MAX_DATA_DISPLAY_SIZE) end @@ -70,11 +70,19 @@ module Gitlab # Returns array of Gitlab::Git::Blob # Does not guarantee blob data will be set def batch_lfs_pointers(repository, blob_ids) - blob_ids.lazy - .select { |sha| possible_lfs_blob?(repository, sha) } - .map { |sha| rugged_raw(repository, sha, limit: LFS_POINTER_MAX_SIZE) } - .select(&:lfs_pointer?) - .force + return [] if blob_ids.empty? + + repository.gitaly_migrate(:batch_lfs_pointers) do |is_enabled| + if is_enabled + repository.gitaly_blob_client.batch_lfs_pointers(blob_ids) + else + blob_ids.lazy + .select { |sha| possible_lfs_blob?(repository, sha) } + .map { |sha| rugged_raw(repository, sha, limit: LFS_POINTER_MAX_SIZE) } + .select(&:lfs_pointer?) + .force + end + end end def binary?(data) @@ -258,7 +266,7 @@ module Gitlab Gitlab::GitalyClient.migrate(:git_blob_load_all_data) do |is_enabled| @data = begin if is_enabled - Gitlab::GitalyClient::BlobService.new(repository).get_blob(oid: id, limit: -1).data + repository.gitaly_blob_client.get_blob(oid: id, limit: -1).data else repository.lookup(id).content end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 9864e4251f1..d7c712e75c5 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -1331,6 +1331,10 @@ module Gitlab @gitaly_remote_client ||= Gitlab::GitalyClient::RemoteService.new(self) end + def gitaly_blob_client + @gitaly_blob_client ||= Gitlab::GitalyClient::BlobService.new(self) + end + def gitaly_conflicts_client(our_commit_oid, their_commit_oid) Gitlab::GitalyClient::ConflictsService.new(self, our_commit_oid, their_commit_oid) end diff --git a/lib/gitlab/gitaly_client/blob_service.rb b/lib/gitlab/gitaly_client/blob_service.rb index a250eb75bd4..ee36684197b 100644 --- a/lib/gitlab/gitaly_client/blob_service.rb +++ b/lib/gitlab/gitaly_client/blob_service.rb @@ -32,6 +32,26 @@ module Gitlab binary: Gitlab::Git::Blob.binary?(data) ) end + + def batch_lfs_pointers(blob_ids) + request = Gitaly::GetLFSPointersRequest.new( + repository: @gitaly_repo, + blob_ids: blob_ids + ) + + response = GitalyClient.call(@gitaly_repo.storage_name, :blob_service, :get_lfs_pointers, request) + + response.flat_map do |message| + message.lfs_pointers.map do |lfs_pointer| + Gitlab::Git::Blob.new( + id: lfs_pointer.oid, + size: lfs_pointer.size, + data: lfs_pointer.data, + binary: Gitlab::Git::Blob.binary?(lfs_pointer.data) + ) + end + end + end end end end diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb index 8706c89c147..168207552ff 100644 --- a/spec/lib/gitlab/git/blob_spec.rb +++ b/spec/lib/gitlab/git/blob_spec.rb @@ -260,29 +260,42 @@ describe Gitlab::Git::Blob, seed_helper: true do ) end - it 'returns a list of Gitlab::Git::Blob' do - blobs = described_class.batch_lfs_pointers(repository, [lfs_blob.id]) + shared_examples 'fetching batch of LFS pointers' do + it 'returns a list of Gitlab::Git::Blob' do + blobs = described_class.batch_lfs_pointers(repository, [lfs_blob.id]) - expect(blobs.count).to eq(1) - expect(blobs).to all( be_a(Gitlab::Git::Blob) ) + expect(blobs.count).to eq(1) + expect(blobs).to all( be_a(Gitlab::Git::Blob) ) + end + + it 'silently ignores tree objects' do + blobs = described_class.batch_lfs_pointers(repository, [tree_object.oid]) + + expect(blobs).to eq([]) + end + + it 'silently ignores non lfs objects' do + blobs = described_class.batch_lfs_pointers(repository, [non_lfs_blob.id]) + + expect(blobs).to eq([]) + end + + it 'avoids loading large blobs into memory' do + # This line could call `lookup` on `repository`, so do here before mocking. + non_lfs_blob_id = non_lfs_blob.id + + expect(repository).not_to receive(:lookup) + + described_class.batch_lfs_pointers(repository, [non_lfs_blob_id]) + end end - it 'silently ignores tree objects' do - blobs = described_class.batch_lfs_pointers(repository, [tree_object.oid]) - - expect(blobs).to eq([]) + context 'when Gitaly batch_lfs_pointers is enabled' do + it_behaves_like 'fetching batch of LFS pointers' end - it 'silently ignores non lfs objects' do - blobs = described_class.batch_lfs_pointers(repository, [non_lfs_blob.id]) - - expect(blobs).to eq([]) - end - - it 'avoids loading large blobs into memory' do - expect(repository).not_to receive(:lookup) - - described_class.batch_lfs_pointers(repository, [non_lfs_blob.id]) + context 'when Gitaly batch_lfs_pointers is disabled', :disable_gitaly do + it_behaves_like 'fetching batch of LFS pointers' end end