Support `Gitaly::User`'s gl_username fieldgitaly-gl-username

We also unify conversions from and to `Gitaly::User`s in the
`Gitlab::Git::User` class.

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