From cbe206985482bd128d578e7f025f8b9e4f6b0ad5 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: [PATCH] Incorporate CommitService.GetTreeEntries Gitaly call --- GITALY_SERVER_VERSION | 2 +- Gemfile | 2 +- Gemfile.lock | 8 +-- lib/gitlab/git/tree.rb | 59 ++++++++++++-------- lib/gitlab/gitaly_client/commit.rb | 25 +++++++++ spec/lib/gitlab/git/tree_spec.rb | 21 ++++++- spec/lib/gitlab/gitaly_client/commit_spec.rb | 30 +++++++--- 7 files changed, 109 insertions(+), 38 deletions(-) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index c5523bd09b187d..2157409059873c 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.17.0 +0.22.0 diff --git a/Gemfile b/Gemfile index 4bc89628756a4f..a9d68ccdefb8f5 100644 --- a/Gemfile +++ b/Gemfile @@ -398,7 +398,7 @@ gem 'sys-filesystem', '~> 1.1.6' gem 'net-ntp' # Gitaly GRPC client -gem 'gitaly', '~> 0.14.0' +gem 'gitaly', '~> 0.15.0' gem 'toml-rb', '~> 0.3.15', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 9f8962b87afd4f..8f24c353e30ff3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -300,7 +300,7 @@ GEM po_to_json (>= 1.0.0) rails (>= 3.2.0) gherkin-ruby (0.3.2) - gitaly (0.14.0) + gitaly (0.15.0) google-protobuf (~> 3.1) grpc (~> 1.0) github-linguist (4.7.6) @@ -355,7 +355,7 @@ GEM multi_json (~> 1.10) retriable (~> 1.4) signet (~> 0.6) - google-protobuf (3.2.0.2) + google-protobuf (3.3.0) googleauth (0.5.1) faraday (~> 0.9) jwt (~> 1.4) @@ -377,7 +377,7 @@ GEM grape-entity (0.6.0) activesupport multi_json (>= 1.3.2) - grpc (1.4.0) + grpc (1.4.1) google-protobuf (~> 3.1) googleauth (~> 0.5.1) gssapi (1.2.0) @@ -1009,7 +1009,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.2.0) - gitaly (~> 0.14.0) + gitaly (~> 0.15.0) github-linguist (~> 4.7.0) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-license (~> 1.0) diff --git a/lib/gitlab/git/tree.rb b/lib/gitlab/git/tree.rb index 8122ff0e81fc8d..8e959c57c7cbb1 100644 --- a/lib/gitlab/git/tree.rb +++ b/lib/gitlab/git/tree.rb @@ -17,30 +17,13 @@ class << self 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 @@ def find_id_by_path(repository, root_id, path) 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.rb b/lib/gitlab/gitaly_client/commit.rb index aafc05206648ae..eddbbce1697084 100644 --- a/lib/gitlab/gitaly_client/commit.rb +++ b/lib/gitlab/gitaly_client/commit.rb @@ -56,6 +56,31 @@ def tree_entry(ref, path, limit = nil) 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 4b76a43e6b5068..161b4751cbf82b 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,22 @@ 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_spec.rb b/spec/lib/gitlab/gitaly_client/commit_spec.rb index dff5b25c7129bb..4e8e494589183d 100644 --- a/spec/lib/gitlab/gitaly_client/commit_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_spec.rb @@ -3,9 +3,13 @@ describe Gitlab::GitalyClient::Commit do let(:diff_stub) { double('Gitaly::Diff::Stub') } 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 @@ -18,7 +22,7 @@ expect_any_instance_of(Gitaly::Diff::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 @@ -33,12 +37,12 @@ expect_any_instance_of(Gitaly::Diff::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 @@ -48,7 +52,7 @@ 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 @@ -63,7 +67,7 @@ expect_any_instance_of(Gitaly::Diff::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 @@ -78,8 +82,20 @@ expect_any_instance_of(Gitaly::Diff::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 + + 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 -- GitLab