From 4969f486f26e614bb8fb69ed12ad1c14c4cfcd04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Mon, 23 Oct 2017 17:31:05 -0300 Subject: [PATCH] Support `Gitaly::User`'s gl_username field We also unify conversions from and to `Gitaly::User`s in the `Gitlab::Git::User` class. --- Gemfile | 2 +- Gemfile.lock | 4 ++-- lib/gitlab/git/user.rb | 7 ++++-- lib/gitlab/gitaly_client/operation_service.rb | 10 ++++----- lib/gitlab/gitaly_client/util.rb | 10 --------- spec/lib/gitlab/git/user_spec.rb | 22 +++++++++++++++++-- .../gitaly_client/operation_service_spec.rb | 2 +- spec/lib/gitlab/gitaly_client/util_spec.rb | 14 ------------ 8 files changed, 34 insertions(+), 37 deletions(-) diff --git a/Gemfile b/Gemfile index f64cdca31c0..5adba4d58df 100644 --- a/Gemfile +++ b/Gemfile @@ -398,7 +398,7 @@ group :ed25519 do end # Gitaly GRPC client -gem 'gitaly-proto', '~> 0.45.0', require: 'gitaly' +gem 'gitaly-proto', '~> 0.48.0', require: 'gitaly' gem 'toml-rb', '~> 0.3.15', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 601d5ca16e2..53efb1c76c2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -273,7 +273,7 @@ GEM po_to_json (>= 1.0.0) rails (>= 3.2.0) gherkin-ruby (0.3.2) - gitaly-proto (0.45.0) + gitaly-proto (0.48.0) google-protobuf (~> 3.1) grpc (~> 1.0) github-linguist (4.7.6) @@ -1030,7 +1030,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.2.0) - gitaly-proto (~> 0.45.0) + gitaly-proto (~> 0.48.0) github-linguist (~> 4.7.0) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-markup (~> 1.6.2) diff --git a/lib/gitlab/git/user.rb b/lib/gitlab/git/user.rb index da74719ae87..e6b61417de1 100644 --- a/lib/gitlab/git/user.rb +++ b/lib/gitlab/git/user.rb @@ -7,9 +7,8 @@ module Gitlab new(gitlab_user.username, gitlab_user.name, gitlab_user.email, Gitlab::GlId.gl_id(gitlab_user)) end - # TODO support the username field in Gitaly https://gitlab.com/gitlab-org/gitaly/issues/628 def self.from_gitaly(gitaly_user) - new('', gitaly_user.name, gitaly_user.email, gitaly_user.gl_id) + new(gitaly_user.gl_username, gitaly_user.name, gitaly_user.email, gitaly_user.gl_id) end def initialize(username, name, email, gl_id) @@ -22,6 +21,10 @@ module Gitlab def ==(other) [username, name, email, gl_id] == [other.username, other.name, other.email, other.gl_id] end + + def to_gitaly + Gitaly::User.new(gl_username: username, gl_id: gl_id, name: name, email: email) + end end end end diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index 91f34011f6e..adaf255f24b 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -10,7 +10,7 @@ module Gitlab request = Gitaly::UserDeleteTagRequest.new( repository: @gitaly_repo, tag_name: GitalyClient.encode(tag_name), - user: Util.gitaly_user(user) + user: Gitlab::Git::User.from_gitlab(user).to_gitaly ) response = GitalyClient.call(@repository.storage, :operation_service, :user_delete_tag, request) @@ -23,7 +23,7 @@ module Gitlab def add_tag(tag_name, user, target, message) request = Gitaly::UserCreateTagRequest.new( repository: @gitaly_repo, - user: Util.gitaly_user(user), + user: Gitlab::Git::User.from_gitlab(user).to_gitaly, tag_name: GitalyClient.encode(tag_name), target_revision: GitalyClient.encode(target), message: GitalyClient.encode(message.to_s) @@ -45,7 +45,7 @@ module Gitlab request = Gitaly::UserCreateBranchRequest.new( repository: @gitaly_repo, branch_name: GitalyClient.encode(branch_name), - user: Util.gitaly_user(user), + user: Gitlab::Git::User.from_gitlab(user).to_gitaly, start_point: GitalyClient.encode(start_point) ) response = GitalyClient.call(@repository.storage, :operation_service, @@ -65,7 +65,7 @@ module Gitlab request = Gitaly::UserDeleteBranchRequest.new( repository: @gitaly_repo, branch_name: GitalyClient.encode(branch_name), - user: Util.gitaly_user(user) + user: Gitlab::Git::User.from_gitlab(user).to_gitaly ) response = GitalyClient.call(@repository.storage, :operation_service, :user_delete_branch, request) @@ -87,7 +87,7 @@ module Gitlab request_enum.push( Gitaly::UserMergeBranchRequest.new( repository: @gitaly_repo, - user: Util.gitaly_user(user), + user: Gitlab::Git::User.from_gitlab(user).to_gitaly, commit_id: source_sha, branch: GitalyClient.encode(target_branch), message: GitalyClient.encode(message) diff --git a/lib/gitlab/gitaly_client/util.rb b/lib/gitlab/gitaly_client/util.rb index a1222a7e718..b1a033280b4 100644 --- a/lib/gitlab/gitaly_client/util.rb +++ b/lib/gitlab/gitaly_client/util.rb @@ -18,16 +18,6 @@ module Gitlab ) end - def gitaly_user(gitlab_user) - return unless gitlab_user - - Gitaly::User.new( - gl_id: Gitlab::GlId.gl_id(gitlab_user), - name: GitalyClient.encode(gitlab_user.name), - email: GitalyClient.encode(gitlab_user.email) - ) - end - def gitlab_tag_from_gitaly_tag(repository, gitaly_tag) if gitaly_tag.target_commit.present? commit = Gitlab::Git::Commit.decorate(repository, gitaly_tag.target_commit) diff --git a/spec/lib/gitlab/git/user_spec.rb b/spec/lib/gitlab/git/user_spec.rb index 31d5f59a562..eb8db819045 100644 --- a/spec/lib/gitlab/git/user_spec.rb +++ b/spec/lib/gitlab/git/user_spec.rb @@ -5,14 +5,20 @@ describe Gitlab::Git::User do let(:name) { 'Jane Doe' } let(:email) { 'janedoe@example.com' } let(:gl_id) { 'user-123' } + let(:user) do + described_class.new(username, name, email, gl_id) + end subject { described_class.new(username, name, email, gl_id) } describe '.from_gitaly' do - let(:gitaly_user) { Gitaly::User.new(name: name, email: email, gl_id: gl_id) } + let(:gitaly_user) do + Gitaly::User.new(gl_username: username, name: name, email: email, gl_id: gl_id) + end + subject { described_class.from_gitaly(gitaly_user) } - it { expect(subject).to eq(described_class.new('', name, email, gl_id)) } + it { expect(subject).to eq(user) } end describe '.from_gitlab' do @@ -35,4 +41,16 @@ describe Gitlab::Git::User do it { expect(subject).not_to eq_other(username, name, email + 'x', gl_id) } it { expect(subject).not_to eq_other(username, name, email, gl_id + 'x') } end + + describe '#to_gitaly' do + subject { user.to_gitaly } + + it 'creates a Gitaly::User with the correct data' do + expect(subject).to be_a(Gitaly::User) + expect(subject.gl_username).to eq(username) + expect(subject.name).to eq(name) + expect(subject.email).to eq(email) + expect(subject.gl_id).to eq(gl_id) + end + end end diff --git a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb index 7bd6a7fa842..e144e28b5d8 100644 --- a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb @@ -5,7 +5,7 @@ describe Gitlab::GitalyClient::OperationService do let(:repository) { project.repository.raw } let(:client) { described_class.new(repository) } let(:user) { create(:user) } - let(:gitaly_user) { Gitlab::GitalyClient::Util.gitaly_user(user) } + let(:gitaly_user) { Gitlab::Git::User.from_gitlab(user).to_gitaly } describe '#user_create_branch' do let(:branch_name) { 'new' } diff --git a/spec/lib/gitlab/gitaly_client/util_spec.rb b/spec/lib/gitlab/gitaly_client/util_spec.rb index c0c29552494..d1e0136f8c1 100644 --- a/spec/lib/gitlab/gitaly_client/util_spec.rb +++ b/spec/lib/gitlab/gitaly_client/util_spec.rb @@ -26,18 +26,4 @@ describe Gitlab::GitalyClient::Util do expect(subject.git_alternate_object_directories).to eq(git_alternate_object_directory) end end - - describe '.gitaly_user' do - let(:user) { create(:user) } - let(:gl_id) { Gitlab::GlId.gl_id(user) } - - subject { described_class.gitaly_user(user) } - - it 'creates a Gitaly::User from a GitLab user' do - expect(subject).to be_a(Gitaly::User) - expect(subject.name).to eq(user.name) - expect(subject.email).to eq(user.email) - expect(subject.gl_id).to eq(gl_id) - end - end end