From 9eb5cdd73faea1f6f6722fd11615405bfe04848d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Tue, 18 Jul 2017 01:02:56 -0400 Subject: Incorporate CommitService.GetTreeEntries Gitaly call --- GITALY_SERVER_VERSION | 2 +- lib/gitlab/git/tree.rb | 59 +++++++++++++--------- lib/gitlab/gitaly_client/commit_service.rb | 25 +++++++++ spec/lib/gitlab/git/tree_spec.rb | 23 ++++++++- .../gitlab/gitaly_client/commit_service_spec.rb | 32 +++++++++--- 5 files changed, 108 insertions(+), 33 deletions(-) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 21574090598..ca222b7cf39 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.22.0 +0.23.0 diff --git a/lib/gitlab/git/tree.rb b/lib/gitlab/git/tree.rb index 8122ff0e81f..8e959c57c7c 100644 --- a/lib/gitlab/git/tree.rb +++ b/lib/gitlab/git/tree.rb @@ -17,30 +17,13 @@ module Gitlab def where(repository, sha, path = nil) path = nil if path == '' || path == '/' - commit = repository.lookup(sha) - root_tree = commit.tree - - tree = if path - id = find_id_by_path(repository, root_tree.oid, path) - if id - repository.lookup(id) - else - [] - end - else - root_tree - end - - tree.map do |entry| - new( - id: entry[:oid], - root_id: root_tree.oid, - name: entry[:name], - type: entry[:type], - mode: entry[:filemode].to_s(8), - path: path ? File.join(path, entry[:name]) : entry[:name], - commit_id: sha - ) + Gitlab::GitalyClient.migrate(:tree_entries) do |is_enabled| + if is_enabled + client = Gitlab::GitalyClient::CommitService.new(repository) + client.tree_entries(repository, sha, path) + else + tree_entries_from_rugged(repository, sha, path) + end end end @@ -74,6 +57,34 @@ module Gitlab entry[:oid] end end + + def tree_entries_from_rugged(repository, sha, path) + commit = repository.lookup(sha) + root_tree = commit.tree + + tree = if path + id = find_id_by_path(repository, root_tree.oid, path) + if id + repository.lookup(id) + else + [] + end + else + root_tree + end + + tree.map do |entry| + new( + id: entry[:oid], + root_id: root_tree.oid, + name: entry[:name], + type: entry[:type], + mode: entry[:filemode].to_s(8), + path: path ? File.join(path, entry[:name]) : entry[:name], + commit_id: sha + ) + end + end end def initialize(options) diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index b749955cddc..6d5673ad2bc 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -60,6 +60,31 @@ module Gitlab entry end + def tree_entries(repository, revision, path) + request = Gitaly::GetTreeEntriesRequest.new( + repository: @gitaly_repo, + revision: revision, + path: path.presence || '.' + ) + + response = GitalyClient.call(@repository.storage, :commit_service, :get_tree_entries, request) + + response.flat_map do |message| + message.entries.map do |gitaly_tree_entry| + entry_path = gitaly_tree_entry.path.dup + Gitlab::Git::Tree.new( + id: gitaly_tree_entry.oid, + root_id: gitaly_tree_entry.root_oid, + type: gitaly_tree_entry.type.downcase, + mode: gitaly_tree_entry.mode.to_s(8), + name: File.basename(entry_path), + path: entry_path, + commit_id: gitaly_tree_entry.commit_oid + ) + end + end + end + def commit_count(ref) request = Gitaly::CountCommitsRequest.new( repository: @gitaly_repo, diff --git a/spec/lib/gitlab/git/tree_spec.rb b/spec/lib/gitlab/git/tree_spec.rb index 4b76a43e6b5..98ddd3c3664 100644 --- a/spec/lib/gitlab/git/tree_spec.rb +++ b/spec/lib/gitlab/git/tree_spec.rb @@ -1,8 +1,9 @@ require "spec_helper" describe Gitlab::Git::Tree, seed_helper: true do + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + context :repo do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } let(:tree) { Gitlab::Git::Tree.where(repository, SeedRepo::Commit::ID) } it { expect(tree).to be_kind_of Array } @@ -74,4 +75,24 @@ describe Gitlab::Git::Tree, seed_helper: true do it { expect(submodule.name).to eq('gitlab-shell') } end end + + describe '#where' do + context 'with gitaly disabled' do + before do + allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_return(false) + end + + it 'calls #tree_entries_from_rugged' do + expect(described_class).to receive(:tree_entries_from_rugged) + + described_class.where(repository, SeedRepo::Commit::ID, '/') + end + end + + it 'gets the tree entries from GitalyClient' do + expect_any_instance_of(Gitlab::GitalyClient::CommitService).to receive(:tree_entries) + + described_class.where(repository, SeedRepo::Commit::ID, '/') + 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 b3b4a1e2218..0868c793a33 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -2,9 +2,13 @@ require 'spec_helper' describe Gitlab::GitalyClient::CommitService do let(:project) { create(:project, :repository) } + let(:storage_name) { project.repository_storage } + let(:relative_path) { project.path_with_namespace + '.git' } let(:repository) { project.repository } let(:repository_message) { repository.gitaly_repository } - let(:commit) { project.commit('913c66a37b4a45b9769037c55c2d238bd0942d2e') } + let(:revision) { '913c66a37b4a45b9769037c55c2d238bd0942d2e' } + let(:commit) { project.commit(revision) } + let(:client) { described_class.new(repository) } describe '#diff_from_parent' do context 'when a commit has a parent' do @@ -20,7 +24,7 @@ describe Gitlab::GitalyClient::CommitService do expect_any_instance_of(Gitaly::DiffService::Stub).to receive(:commit_diff).with(request, kind_of(Hash)) - described_class.new(repository).diff_from_parent(commit) + client.diff_from_parent(commit) end end @@ -38,12 +42,12 @@ describe Gitlab::GitalyClient::CommitService do expect_any_instance_of(Gitaly::DiffService::Stub).to receive(:commit_diff).with(request, kind_of(Hash)) - described_class.new(repository).diff_from_parent(initial_commit) + client.diff_from_parent(initial_commit) end end it 'returns a Gitlab::Git::DiffCollection' do - ret = described_class.new(repository).diff_from_parent(commit) + ret = client.diff_from_parent(commit) expect(ret).to be_kind_of(Gitlab::Git::DiffCollection) end @@ -53,7 +57,7 @@ describe Gitlab::GitalyClient::CommitService do expect(Gitlab::Git::DiffCollection).to receive(:new).with(kind_of(Enumerable), options) - described_class.new(repository).diff_from_parent(commit, options) + client.diff_from_parent(commit, options) end end @@ -68,7 +72,7 @@ describe Gitlab::GitalyClient::CommitService do expect_any_instance_of(Gitaly::DiffService::Stub).to receive(:commit_delta).with(request, kind_of(Hash)).and_return([]) - described_class.new(repository).commit_deltas(commit) + client.commit_deltas(commit) end end @@ -83,7 +87,7 @@ describe Gitlab::GitalyClient::CommitService do expect_any_instance_of(Gitaly::DiffService::Stub).to receive(:commit_delta).with(request, kind_of(Hash)).and_return([]) - described_class.new(repository).commit_deltas(initial_commit) + client.commit_deltas(initial_commit) end end end @@ -91,6 +95,7 @@ describe Gitlab::GitalyClient::CommitService do describe '#between' do let(:from) { 'master' } let(:to) { '4b825dc642cb6eb9a060e54bf8d69288fbee4904' } + it 'sends an RPC request' do request = Gitaly::CommitsBetweenRequest.new( repository: repository_message, from: from, to: to @@ -102,4 +107,17 @@ describe Gitlab::GitalyClient::CommitService do described_class.new(repository).between(from, to) end end + + describe '#tree_entries' do + let(:path) { '/' } + + it 'sends a get_tree_entries message' do + expect_any_instance_of(Gitaly::CommitService::Stub) + .to receive(:get_tree_entries) + .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) + .and_return([]) + + client.tree_entries(repository, revision, path) + end + end end -- cgit v1.2.1