From 59e50e33b3203bfe450f82d2ee2cc83b87f9e5fb Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Thu, 21 Dec 2017 15:05:35 +0100 Subject: [PATCH 1/2] Reroute batch blobs to single blob RPC Given the priorities shifted for the Gitaly team, this endpoint does not get a dedicated endpoint yet. To make it work in a cloud native environment the request needs to go to Gitaly, not rugged. This is achieved by rerouting to the generic TreeEntry endpoint. --- lib/gitlab/git/blob.rb | 31 +++++++++++++++++++++++++------ spec/lib/gitlab/git/blob_spec.rb | 15 ++++----------- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/lib/gitlab/git/blob.rb b/lib/gitlab/git/blob.rb index 228d97a87ab..bd91125d3b6 100644 --- a/lib/gitlab/git/blob.rb +++ b/lib/gitlab/git/blob.rb @@ -50,10 +50,19 @@ module Gitlab # to the caller to limit the number of blobs and blob_size_limit. # # Gitaly migration issue: https://gitlab.com/gitlab-org/gitaly/issues/798 - def batch(repository, blob_references, blob_size_limit: nil) - blob_size_limit ||= MAX_DATA_DISPLAY_SIZE - blob_references.map do |sha, path| - find_by_rugged(repository, sha, path, limit: blob_size_limit) + def batch(repository, blob_references, blob_size_limit: MAX_DATA_DISPLAY_SIZE) + Gitlab::GitalyClient.migrate(:list_blobs_by_sha_path) do |is_enabled| + if is_enabled + Gitlab::GitalyClient.allow_n_plus_1_calls do + blob_references.map do |sha, path| + find_by_gitaly(repository, sha, path, limit: blob_size_limit) + end + end + else + blob_references.map do |sha, path| + find_by_rugged(repository, sha, path, limit: blob_size_limit) + end + end end end @@ -122,13 +131,23 @@ module Gitlab ) end - def find_by_gitaly(repository, sha, path) + def find_by_gitaly(repository, sha, path, limit: MAX_DATA_DISPLAY_SIZE) path = path.sub(/\A\/*/, '') path = '/' if path.empty? name = File.basename(path) - entry = Gitlab::GitalyClient::CommitService.new(repository).tree_entry(sha, path, MAX_DATA_DISPLAY_SIZE) + + # Gitaly will think that setting the limit to 0 means unlimited, while + # the client might only need the metadata and thus set the limit to 0. + # In this method we'll than set the limit to 1, but clear the byte of data + # that we got back so fot the outside world it looks like the limit was + # actually 0. + req_limit = limit == 0 ? 1 : limit + + entry = Gitlab::GitalyClient::CommitService.new(repository).tree_entry(sha, path, req_limit) return unless entry + entry.data = "" if limit == 0 + case entry.type when :COMMIT new( diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb index c04a9688503..7f5946b1658 100644 --- a/spec/lib/gitlab/git/blob_spec.rb +++ b/spec/lib/gitlab/git/blob_spec.rb @@ -202,16 +202,6 @@ describe Gitlab::Git::Blob, seed_helper: true do context 'limiting' do subject { described_class.batch(repository, blob_references, blob_size_limit: blob_size_limit) } - context 'default' do - let(:blob_size_limit) { nil } - - it 'limits to MAX_DATA_DISPLAY_SIZE' do - stub_const('Gitlab::Git::Blob::MAX_DATA_DISPLAY_SIZE', 100) - - expect(subject.first.data.size).to eq(100) - end - end - context 'positive' do let(:blob_size_limit) { 10 } @@ -221,7 +211,10 @@ describe Gitlab::Git::Blob, seed_helper: true do context 'zero' do let(:blob_size_limit) { 0 } - it { expect(subject.first.data).to eq('') } + it 'only loads the metadata' do + expect(subject.first.size).not_to be(0) + expect(subject.first.data).to eq('') + end end context 'negative' do From 6b15784ce7d893ed509c00d9f51a4702787799ed Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Fri, 5 Jan 2018 09:41:05 +0000 Subject: [PATCH 2/2] Fix typos in a code comment --- lib/gitlab/git/blob.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/git/blob.rb b/lib/gitlab/git/blob.rb index bd91125d3b6..a1755143abe 100644 --- a/lib/gitlab/git/blob.rb +++ b/lib/gitlab/git/blob.rb @@ -138,8 +138,8 @@ module Gitlab # Gitaly will think that setting the limit to 0 means unlimited, while # the client might only need the metadata and thus set the limit to 0. - # In this method we'll than set the limit to 1, but clear the byte of data - # that we got back so fot the outside world it looks like the limit was + # In this method we'll then set the limit to 1, but clear the byte of data + # that we got back so for the outside world it looks like the limit was # actually 0. req_limit = limit == 0 ? 1 : limit