Merge branch 'gitlab-git-user' into 'master'

Rename Gitlab::Git::Committer to User

See merge request gitlab-org/gitlab-ce!14254

app/models/repository.rb

@@ -172,7 +172,7 @@ class Repository
   end
 
   def add_branch(user, branch_name, ref)
-    branch = raw_repository.add_branch(branch_name, committer: user, target: ref)
+    branch = raw_repository.add_branch(branch_name, user: user, target: ref)
 
     after_create_branch
 
@@ -182,7 +182,7 @@ class Repository
   end
 
   def add_tag(user, tag_name, target, message = nil)
-    raw_repository.add_tag(tag_name, committer: user, target: target, message: message)
+    raw_repository.add_tag(tag_name, user: user, target: target, message: message)
   rescue Gitlab::Git::Repository::InvalidRef
     false
   end
@@ -190,7 +190,7 @@ class Repository
   def rm_branch(user, branch_name)
     before_remove_branch
 
-    raw_repository.rm_branch(branch_name, committer: user)
+    raw_repository.rm_branch(branch_name, user: user)
 
     after_remove_branch
     true
@@ -199,7 +199,7 @@ class Repository
   def rm_tag(user, tag_name)
     before_remove_tag
 
-    raw_repository.rm_tag(tag_name, committer: user)
+    raw_repository.rm_tag(tag_name, user: user)
 
     after_remove_tag
     true

lib/gitlab/git/operation_service.rb

 module Gitlab
   module Git
     class OperationService
-      attr_reader :committer, :repository
+      attr_reader :user, :repository
 
-      def initialize(committer, new_repository)
-        committer = Gitlab::Git::Committer.from_user(committer) if committer.is_a?(User)
-        @committer = committer
+      def initialize(user, new_repository)
+        if user
+          user = Gitlab::Git::User.from_gitlab(user) unless user.respond_to?(:gl_id)
+          @user = user
+        end
 
         # Refactoring aid
         unless new_repository.is_a?(Gitlab::Git::Repository)
@@ -128,7 +130,7 @@ module Gitlab
 
       def with_hooks(ref, newrev, oldrev)
         Gitlab::Git::HooksService.new.execute(
-          committer,
+          user,
           repository,
           oldrev,
           newrev,

lib/gitlab/git/repository.rb

@@ -610,43 +610,43 @@ module Gitlab
         # TODO: implement this method
       end
 
-      def add_branch(branch_name, committer:, target:)
+      def add_branch(branch_name, user:, target:)
         target_object = Ref.dereference_object(lookup(target))
         raise InvalidRef.new("target not found: #{target}") unless target_object
 
-        OperationService.new(committer, self).add_branch(branch_name, target_object.oid)
+        OperationService.new(user, self).add_branch(branch_name, target_object.oid)
         find_branch(branch_name)
       rescue Rugged::ReferenceError => ex
         raise InvalidRef, ex
       end
 
-      def add_tag(tag_name, committer:, target:, message: nil)
+      def add_tag(tag_name, user:, target:, message: nil)
         target_object = Ref.dereference_object(lookup(target))
         raise InvalidRef.new("target not found: #{target}") unless target_object
 
-        committer = Committer.from_user(committer) if committer.is_a?(User)
+        user = Gitlab::Git::User.from_gitlab(user) unless user.respond_to?(:gl_id)
 
         options = nil # Use nil, not the empty hash. Rugged cares about this.
         if message
           options = {
             message: message,
-            tagger: Gitlab::Git.committer_hash(email: committer.email, name: committer.name)
+            tagger: Gitlab::Git.committer_hash(email: user.email, name: user.name)
           }
         end
 
-        OperationService.new(committer, self).add_tag(tag_name, target_object.oid, options)
+        OperationService.new(user, self).add_tag(tag_name, target_object.oid, options)
 
         find_tag(tag_name)
       rescue Rugged::ReferenceError => ex
         raise InvalidRef, ex
       end
 
-      def rm_branch(branch_name, committer:)
-        OperationService.new(committer, self).rm_branch(find_branch(branch_name))
+      def rm_branch(branch_name, user:)
+        OperationService.new(user, self).rm_branch(find_branch(branch_name))
       end
 
-      def rm_tag(tag_name, committer:)
-        OperationService.new(committer, self).rm_tag(find_tag(tag_name))
+      def rm_tag(tag_name, user:)
+        OperationService.new(user, self).rm_tag(find_tag(tag_name))
       end
 
       def find_tag(name)

lib/gitlab/git/committer.rb → lib/gitlab/git/user.rb

 module Gitlab
   module Git
-    class Committer
+    class User
       attr_reader :name, :email, :gl_id
 
-      def self.from_user(user)
-        new(user.name, user.email, Gitlab::GlId.gl_id(user))
+      def self.from_gitlab(gitlab_user)
+        new(gitlab_user.name, gitlab_user.email, Gitlab::GlId.gl_id(gitlab_user))
+      end
+
+      def self.from_gitaly(gitaly_user)
+        new(gitaly_user.name, gitaly_user.email, gitaly_user.gl_id)
       end
 
       def initialize(name, email, gl_id)

spec/lib/gitlab/git/hooks_service_spec.rb

 require 'spec_helper'
 
 describe Gitlab::Git::HooksService, seed_helper: true do
-  let(:committer) { Gitlab::Git::Committer.new('Jane Doe', 'janedoe@example.com', 'user-456') }
+  let(:user) { Gitlab::Git::User.new('Jane Doe', 'janedoe@example.com', 'user-456') }
   let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, 'project-123') }
   let(:service) { described_class.new }
 
@@ -18,7 +18,7 @@ describe Gitlab::Git::HooksService, seed_helper: true do
         hook = double(trigger: [true, nil])
         expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook)
 
-        service.execute(committer, repository, @blankrev, @newrev, @ref) { }
+        service.execute(user, repository, @blankrev, @newrev, @ref) { }
       end
     end
 
@@ -28,7 +28,7 @@ describe Gitlab::Git::HooksService, seed_helper: true do
         expect(service).not_to receive(:run_hook).with('post-receive')
 
         expect do
-          service.execute(committer, repository, @blankrev, @newrev, @ref)
+          service.execute(user, repository, @blankrev, @newrev, @ref)
         end.to raise_error(Gitlab::Git::HooksService::PreReceiveError)
       end
     end
@@ -40,7 +40,7 @@ describe Gitlab::Git::HooksService, seed_helper: true do
         expect(service).not_to receive(:run_hook).with('post-receive')
 
         expect do
-          service.execute(committer, repository, @blankrev, @newrev, @ref)
+          service.execute(user, repository, @blankrev, @newrev, @ref)
         end.to raise_error(Gitlab::Git::HooksService::PreReceiveError)
       end
     end

spec/lib/gitlab/git/committer_spec.rb → spec/lib/gitlab/git/user_spec.rb

 require 'spec_helper'
 
-describe Gitlab::Git::Committer do
+describe Gitlab::Git::User do
   let(:name) { 'Jane Doe' }
   let(:email) { 'janedoe@example.com' }
   let(:gl_id) { 'user-123' }

spec/models/repository_spec.rb

@@ -8,7 +8,7 @@ describe Repository, models: true do
   let(:repository) { project.repository }
   let(:broken_repository) { create(:project, :broken_storage).repository }
   let(:user) { create(:user) }
-  let(:committer) { Gitlab::Git::Committer.from_user(user) }
+  let(:git_user) { Gitlab::Git::User.from_gitlab(user) }
 
   let(:commit_options) do
     author = repository.user_to_committer(user)
@@ -886,7 +886,7 @@ describe Repository, models: true do
     context 'when pre hooks were successful' do
       it 'runs without errors' do
         expect_any_instance_of(Gitlab::Git::HooksService).to receive(:execute)
-          .with(committer, repository.raw_repository, old_rev, blank_sha, 'refs/heads/feature')
+          .with(git_user, repository.raw_repository, old_rev, blank_sha, 'refs/heads/feature')
 
         expect { repository.rm_branch(user, 'feature') }.not_to raise_error
       end
@@ -932,20 +932,20 @@ describe Repository, models: true do
         service = Gitlab::Git::HooksService.new
         expect(Gitlab::Git::HooksService).to receive(:new).and_return(service)
         expect(service).to receive(:execute)
-          .with(committer, target_repository.raw_repository, old_rev, new_rev, updating_ref)
+          .with(git_user, target_repository.raw_repository, old_rev, new_rev, updating_ref)
           .and_yield(service).and_return(true)
       end
 
       it 'runs without errors' do
         expect do
-          Gitlab::Git::OperationService.new(committer, repository.raw_repository).with_branch('feature') do
+          Gitlab::Git::OperationService.new(git_user, repository.raw_repository).with_branch('feature') do
             new_rev
           end
         end.not_to raise_error
       end
 
       it 'ensures the autocrlf Git option is set to :input' do
-        service = Gitlab::Git::OperationService.new(committer, repository.raw_repository)
+        service = Gitlab::Git::OperationService.new(git_user, repository.raw_repository)
 
         expect(service).to receive(:update_autocrlf_option)
 
@@ -956,7 +956,7 @@ describe Repository, models: true do
         it 'updates the head' do
           expect(repository.find_branch('feature').dereferenced_target.id).to eq(old_rev)
 
-          Gitlab::Git::OperationService.new(committer, repository.raw_repository).with_branch('feature') do
+          Gitlab::Git::OperationService.new(git_user, repository.raw_repository).with_branch('feature') do
             new_rev
           end
 
@@ -974,7 +974,7 @@ describe Repository, models: true do
           expect(target_project.repository.raw_repository).to receive(:fetch_ref)
             .and_call_original
 
-          Gitlab::Git::OperationService.new(committer, target_repository.raw_repository)
+          Gitlab::Git::OperationService.new(git_user, target_repository.raw_repository)
             .with_branch(
               'master',
               start_repository: project.repository.raw_repository,
@@ -990,7 +990,7 @@ describe Repository, models: true do
         it 'does not fetch_ref and just pass the commit' do
           expect(target_repository).not_to receive(:fetch_ref)
 
-          Gitlab::Git::OperationService.new(committer, target_repository.raw_repository)
+          Gitlab::Git::OperationService.new(git_user, target_repository.raw_repository)
             .with_branch('feature', start_repository: project.repository.raw_repository) { new_rev }
         end
       end
@@ -1009,7 +1009,7 @@ describe Repository, models: true do
         end
 
         expect do
-          Gitlab::Git::OperationService.new(committer, target_project.repository.raw_repository)
+          Gitlab::Git::OperationService.new(git_user, target_project.repository.raw_repository)
             .with_branch('feature',
                          start_repository: project.repository.raw_repository,
                          &:itself)
@@ -1031,7 +1031,7 @@ describe Repository, models: true do
         repository.add_branch(user, branch, old_rev)
 
         expect do
-          Gitlab::Git::OperationService.new(committer, repository.raw_repository).with_branch(branch) do
+          Gitlab::Git::OperationService.new(git_user, repository.raw_repository).with_branch(branch) do
             new_rev
           end
         end.not_to raise_error
@@ -1049,7 +1049,7 @@ describe Repository, models: true do
         # Updating 'master' to new_rev would lose the commits on 'master' that
         # are not contained in new_rev. This should not be allowed.
         expect do
-          Gitlab::Git::OperationService.new(committer, repository.raw_repository).with_branch(branch) do
+          Gitlab::Git::OperationService.new(git_user, repository.raw_repository).with_branch(branch) do
             new_rev
           end
         end.to raise_error(Gitlab::Git::CommitError)
@@ -1061,7 +1061,7 @@ describe Repository, models: true do
         allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
 
         expect do
-          Gitlab::Git::OperationService.new(committer, repository.raw_repository).with_branch('feature') do
+          Gitlab::Git::OperationService.new(git_user, repository.raw_repository).with_branch('feature') do
             new_rev
           end
         end.to raise_error(Gitlab::Git::HooksService::PreReceiveError)