From e4ac948c706d628a750aae7ad5f2fb7753e1f979 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Thu, 6 Apr 2017 16:54:15 +0200 Subject: [PATCH 1/5] Send more Gitaly::Repository fields --- app/models/repository.rb | 3 +++ lib/gitlab/git/repository.rb | 8 ++++++++ lib/gitlab/gitaly_client/commit.rb | 17 +++++++---------- lib/gitlab/gitaly_client/notifications.rb | 3 ++- lib/gitlab/gitaly_client/ref.rb | 3 ++- lib/gitlab/gitaly_client/util.rb | 20 +++++++++++++++----- lib/gitlab/workhorse.rb | 10 ++-------- spec/lib/gitlab/gitaly_client/commit_spec.rb | 2 +- 8 files changed, 40 insertions(+), 26 deletions(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index 1293cb1d486..4be783ac44f 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -1156,6 +1156,9 @@ class Repository @project.repository_storage_path end + delegate :gitaly_repository, to: :raw_repository + delegate :gitaly_channel, to: :raw_repository + def initialize_raw_repository Gitlab::Git::Repository.new(project.repository_storage, path_with_namespace + '.git') end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 9e338282e96..c0572fb50c1 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -968,6 +968,14 @@ module Gitlab @attributes.attributes(path) end + def gitaly_repository + Gitlab::GitalyClient::Util.repository(@repository_storage, @relative_path) + end + + def gitaly_channel + Gitlab::GitalyClient::Util.channel(@repository_storage) + end + private # Get the content of a blob for a given commit. If the blob is a commit diff --git a/lib/gitlab/gitaly_client/commit.rb b/lib/gitlab/gitaly_client/commit.rb index f15faebe27e..0c38a47178d 100644 --- a/lib/gitlab/gitaly_client/commit.rb +++ b/lib/gitlab/gitaly_client/commit.rb @@ -7,14 +7,13 @@ module Gitlab class << self def diff_from_parent(commit, options = {}) - project = commit.project - channel = GitalyClient.get_channel(project.repository_storage) - stub = Gitaly::Diff::Stub.new(nil, nil, channel_override: channel) - repo = Gitaly::Repository.new(path: project.repository.path_to_repo) + repository = commit.project.repository + gitaly_repo = repository.gitaly_repository + stub = Gitaly::Diff::Stub.new(nil, nil, channel_override: repository.gitaly_channel) parent = commit.parents[0] parent_id = parent ? parent.id : EMPTY_TREE_ID request = Gitaly::CommitDiffRequest.new( - repository: repo, + repository: gitaly_repo, left_commit_id: parent_id, right_commit_id: commit.id ) @@ -23,12 +22,10 @@ module Gitlab end def is_ancestor(repository, ancestor_id, child_id) - project = Project.find_by_path(repository.path) - channel = GitalyClient.get_channel(project.repository_storage) - stub = Gitaly::Commit::Stub.new(nil, nil, channel_override: channel) - repo = Gitaly::Repository.new(path: repository.path_to_repo) + gitaly_repo = repository.gitaly_repository + stub = Gitaly::Commit::Stub.new(nil, nil, channel_override: repository.gitaly_channel) request = Gitaly::CommitIsAncestorRequest.new( - repository: repo, + repository: gitaly_repo, ancestor_id: ancestor_id, child_id: child_id ) diff --git a/lib/gitlab/gitaly_client/notifications.rb b/lib/gitlab/gitaly_client/notifications.rb index f0d93ded91b..f9355e7b439 100644 --- a/lib/gitlab/gitaly_client/notifications.rb +++ b/lib/gitlab/gitaly_client/notifications.rb @@ -4,7 +4,8 @@ module Gitlab attr_accessor :stub def initialize(repository_storage, relative_path) - @channel, @repository = Util.process_path(repository_storage, relative_path) + @channel = Util.channel(repository_storage) + @repository = Util.repository(repository_storage, relative_path) @stub = Gitaly::Notifications::Stub.new(nil, nil, channel_override: @channel) end diff --git a/lib/gitlab/gitaly_client/ref.rb b/lib/gitlab/gitaly_client/ref.rb index fcdf452d567..3b1eca21d11 100644 --- a/lib/gitlab/gitaly_client/ref.rb +++ b/lib/gitlab/gitaly_client/ref.rb @@ -4,7 +4,8 @@ module Gitlab attr_accessor :stub def initialize(repository_storage, relative_path) - @channel, @repository = Util.process_path(repository_storage, relative_path) + @channel = Util.channel(repository_storage) + @repository = Util.repository(repository_storage, relative_path) @stub = Gitaly::Ref::Stub.new(nil, nil, channel_override: @channel) end diff --git a/lib/gitlab/gitaly_client/util.rb b/lib/gitlab/gitaly_client/util.rb index d272c25d1f9..f5b40e7def7 100644 --- a/lib/gitlab/gitaly_client/util.rb +++ b/lib/gitlab/gitaly_client/util.rb @@ -1,12 +1,22 @@ module Gitlab module GitalyClient module Util - def self.process_path(repository_storage, relative_path) - channel = GitalyClient.get_channel(repository_storage) - storage_path = Gitlab.config.repositories.storages[repository_storage]['path'] - repository = Gitaly::Repository.new(path: File.join(storage_path, relative_path)) + class << self + def self.process_path(repository_storage, relative_path) + [channel(repository_storage), repository(repository_storage, relative_path)] + end - [channel, repository] + def repository(repository_storage, relative_path) + Gitaly::Repository.new( + path: File.join(Gitlab.config.repositories.storages[repository_storage]['path'], relative_path), + storage_name: repository_storage, + relative_path: relative_path, + ) + end + + def channel(repository_storage) + GitalyClient.get_channel(repository_storage) + end end end end diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb index a8a7bf9bc12..e6e40f6945d 100644 --- a/lib/gitlab/workhorse.rb +++ b/lib/gitlab/workhorse.rb @@ -24,14 +24,8 @@ module Gitlab } if Gitlab.config.gitaly.enabled - storage = repository.project.repository_storage - address = Gitlab::GitalyClient.get_address(storage) - # TODO: use GitalyClient code to assemble the Repository message - params[:Repository] = Gitaly::Repository.new( - path: repo_path, - storage_name: storage, - relative_path: Gitlab::RepoPath.strip_storage_path(repo_path), - ).to_h + address = Gitlab::GitalyClient.get_address(repository.project.repository_storage) + params[:Repository] = repository.gitaly_repository.to_h feature_enabled = case action.to_s when 'git_receive_pack' diff --git a/spec/lib/gitlab/gitaly_client/commit_spec.rb b/spec/lib/gitlab/gitaly_client/commit_spec.rb index 4684b1d1ac0..58f11ff8906 100644 --- a/spec/lib/gitlab/gitaly_client/commit_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_spec.rb @@ -4,7 +4,7 @@ describe Gitlab::GitalyClient::Commit do describe '.diff_from_parent' do let(:diff_stub) { double('Gitaly::Diff::Stub') } let(:project) { create(:project, :repository) } - let(:repository_message) { Gitaly::Repository.new(path: project.repository.path) } + let(:repository_message) { project.repository.gitaly_repository } let(:commit) { project.commit('913c66a37b4a45b9769037c55c2d238bd0942d2e') } before do From 7ecc7f6a56bf2f6a736435d3b43f3dffa5a534f9 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Thu, 6 Apr 2017 17:05:11 +0200 Subject: [PATCH 2/5] Remove unused code --- lib/gitlab/gitaly_client/util.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/gitlab/gitaly_client/util.rb b/lib/gitlab/gitaly_client/util.rb index f5b40e7def7..b9d209dd0fe 100644 --- a/lib/gitlab/gitaly_client/util.rb +++ b/lib/gitlab/gitaly_client/util.rb @@ -2,10 +2,6 @@ module Gitlab module GitalyClient module Util class << self - def self.process_path(repository_storage, relative_path) - [channel(repository_storage), repository(repository_storage, relative_path)] - end - def repository(repository_storage, relative_path) Gitaly::Repository.new( path: File.join(Gitlab.config.repositories.storages[repository_storage]['path'], relative_path), From 6a07a2430b24fe8040124b103029b0129996c3e5 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Fri, 7 Apr 2017 11:17:25 +0200 Subject: [PATCH 3/5] Combine delegations in one line --- app/models/repository.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index 4be783ac44f..f4c51cdfdf4 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -1156,8 +1156,7 @@ class Repository @project.repository_storage_path end - delegate :gitaly_repository, to: :raw_repository - delegate :gitaly_channel, to: :raw_repository + delegate :gitaly_channel, :gitaly_repository, to: :raw_repository def initialize_raw_repository Gitlab::Git::Repository.new(project.repository_storage, path_with_namespace + '.git') From 6474c67d200dff074bfbb34c946e9da01d18004d Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Fri, 7 Apr 2017 11:17:47 +0200 Subject: [PATCH 4/5] Remove spaces --- lib/gitlab/gitaly_client/commit.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/gitlab/gitaly_client/commit.rb b/lib/gitlab/gitaly_client/commit.rb index 0c38a47178d..b7f39f3ef0b 100644 --- a/lib/gitlab/gitaly_client/commit.rb +++ b/lib/gitlab/gitaly_client/commit.rb @@ -9,10 +9,10 @@ module Gitlab def diff_from_parent(commit, options = {}) repository = commit.project.repository gitaly_repo = repository.gitaly_repository - stub = Gitaly::Diff::Stub.new(nil, nil, channel_override: repository.gitaly_channel) - parent = commit.parents[0] + stub = Gitaly::Diff::Stub.new(nil, nil, channel_override: repository.gitaly_channel) + parent = commit.parents[0] parent_id = parent ? parent.id : EMPTY_TREE_ID - request = Gitaly::CommitDiffRequest.new( + request = Gitaly::CommitDiffRequest.new( repository: gitaly_repo, left_commit_id: parent_id, right_commit_id: commit.id From 20d415a676fc4ee7e6f48e56e1abbb8d78abf75f Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Fri, 7 Apr 2017 11:19:20 +0200 Subject: [PATCH 5/5] Use Gitlab::Git::Repository#gitaly_... more --- lib/api/internal.rb | 2 +- lib/gitlab/git/repository.rb | 4 ++-- lib/gitlab/gitaly_client/notifications.rb | 10 +++++----- lib/gitlab/gitaly_client/ref.rb | 14 +++++++------- lib/gitlab/gitaly_client/util.rb | 4 ---- .../lib/gitlab/gitaly_client/notifications_spec.rb | 2 +- spec/lib/gitlab/gitaly_client/ref_spec.rb | 2 +- 7 files changed, 17 insertions(+), 21 deletions(-) diff --git a/lib/api/internal.rb b/lib/api/internal.rb index 56c597dffcb..70d0d57204d 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -142,7 +142,7 @@ module API project = Project.find_by_full_path(relative_path.sub(/\.(git|wiki)\z/, '')) begin - Gitlab::GitalyClient::Notifications.new(project.repository_storage, relative_path).post_receive + Gitlab::GitalyClient::Notifications.new(project.repository).post_receive rescue GRPC::Unavailable => e render_api_error(e, 500) end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index c0572fb50c1..fc473b2c21e 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -973,7 +973,7 @@ module Gitlab end def gitaly_channel - Gitlab::GitalyClient::Util.channel(@repository_storage) + Gitlab::GitalyClient.get_channel(@repository_storage) end private @@ -1255,7 +1255,7 @@ module Gitlab end def gitaly_ref_client - @gitaly_ref_client ||= Gitlab::GitalyClient::Ref.new(@repository_storage, @relative_path) + @gitaly_ref_client ||= Gitlab::GitalyClient::Ref.new(self) end end end diff --git a/lib/gitlab/gitaly_client/notifications.rb b/lib/gitlab/gitaly_client/notifications.rb index f9355e7b439..a94a54883db 100644 --- a/lib/gitlab/gitaly_client/notifications.rb +++ b/lib/gitlab/gitaly_client/notifications.rb @@ -3,14 +3,14 @@ module Gitlab class Notifications attr_accessor :stub - def initialize(repository_storage, relative_path) - @channel = Util.channel(repository_storage) - @repository = Util.repository(repository_storage, relative_path) - @stub = Gitaly::Notifications::Stub.new(nil, nil, channel_override: @channel) + # 'repository' is a Gitlab::Git::Repository + def initialize(repository) + @gitaly_repo = repository.gitaly_repository + @stub = Gitaly::Notifications::Stub.new(nil, nil, channel_override: repository.gitaly_channel) end def post_receive - request = Gitaly::PostReceiveRequest.new(repository: @repository) + request = Gitaly::PostReceiveRequest.new(repository: @gitaly_repo) @stub.post_receive(request) end end diff --git a/lib/gitlab/gitaly_client/ref.rb b/lib/gitlab/gitaly_client/ref.rb index 3b1eca21d11..d3c0743db4e 100644 --- a/lib/gitlab/gitaly_client/ref.rb +++ b/lib/gitlab/gitaly_client/ref.rb @@ -3,24 +3,24 @@ module Gitlab class Ref attr_accessor :stub - def initialize(repository_storage, relative_path) - @channel = Util.channel(repository_storage) - @repository = Util.repository(repository_storage, relative_path) - @stub = Gitaly::Ref::Stub.new(nil, nil, channel_override: @channel) + # 'repository' is a Gitlab::Git::Repository + def initialize(repository) + @gitaly_repo = repository.gitaly_repository + @stub = Gitaly::Ref::Stub.new(nil, nil, channel_override: repository.gitaly_channel) end def default_branch_name - request = Gitaly::FindDefaultBranchNameRequest.new(repository: @repository) + request = Gitaly::FindDefaultBranchNameRequest.new(repository: @gitaly_repo) stub.find_default_branch_name(request).name.gsub(/^refs\/heads\//, '') end def branch_names - request = Gitaly::FindAllBranchNamesRequest.new(repository: @repository) + request = Gitaly::FindAllBranchNamesRequest.new(repository: @gitaly_repo) consume_refs_response(stub.find_all_branch_names(request), prefix: 'refs/heads/') end def tag_names - request = Gitaly::FindAllTagNamesRequest.new(repository: @repository) + request = Gitaly::FindAllTagNamesRequest.new(repository: @gitaly_repo) consume_refs_response(stub.find_all_tag_names(request), prefix: 'refs/tags/') end diff --git a/lib/gitlab/gitaly_client/util.rb b/lib/gitlab/gitaly_client/util.rb index b9d209dd0fe..4acd297f5cb 100644 --- a/lib/gitlab/gitaly_client/util.rb +++ b/lib/gitlab/gitaly_client/util.rb @@ -9,10 +9,6 @@ module Gitlab relative_path: relative_path, ) end - - def channel(repository_storage) - GitalyClient.get_channel(repository_storage) - end end end end diff --git a/spec/lib/gitlab/gitaly_client/notifications_spec.rb b/spec/lib/gitlab/gitaly_client/notifications_spec.rb index 39c2048fef8..b87dacb175b 100644 --- a/spec/lib/gitlab/gitaly_client/notifications_spec.rb +++ b/spec/lib/gitlab/gitaly_client/notifications_spec.rb @@ -4,7 +4,7 @@ describe Gitlab::GitalyClient::Notifications do describe '#post_receive' do let(:project) { create(:empty_project) } let(:repo_path) { project.repository.path_to_repo } - subject { described_class.new(project.repository_storage, project.full_path + '.git') } + subject { described_class.new(project.repository) } it 'sends a post_receive message' do expect_any_instance_of(Gitaly::Notifications::Stub). diff --git a/spec/lib/gitlab/gitaly_client/ref_spec.rb b/spec/lib/gitlab/gitaly_client/ref_spec.rb index 79c9ca993e4..5405eafd281 100644 --- a/spec/lib/gitlab/gitaly_client/ref_spec.rb +++ b/spec/lib/gitlab/gitaly_client/ref_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::GitalyClient::Ref do let(:project) { create(:empty_project) } let(:repo_path) { project.repository.path_to_repo } - let(:client) { Gitlab::GitalyClient::Ref.new(project.repository_storage, project.full_path + '.git') } + let(:client) { Gitlab::GitalyClient::Ref.new(project.repository) } before do allow(Gitlab.config.gitaly).to receive(:enabled).and_return(true)