Decouple GitOperationService from User

3 files changed, 28 insertions, 16 deletions

diff --git a/app/models/repository.rb b/app/models/repository.rb
index c1e4fcf94a4..3c5074e5157 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -164,7 +164,8 @@ class Repository
 
     return false unless newrev
 
-    GitOperationService.new(user, self).add_branch(branch_name, newrev)
+    committer = Gitlab::Git::Committer.from_user(user)
+    GitOperationService.new(committer, self).add_branch(branch_name, newrev)
 
     after_create_branch
     find_branch(branch_name)
@@ -176,7 +177,8 @@ class Repository
 
     return false unless newrev
 
-    GitOperationService.new(user, self).add_tag(tag_name, newrev, options)
+    committer = Gitlab::Git::Committer.from_user(user)
+    GitOperationService.new(committer, self).add_tag(tag_name, newrev, options)
 
     find_tag(tag_name)
   end
@@ -185,7 +187,8 @@ class Repository
     before_remove_branch
     branch = find_branch(branch_name)
 
-    GitOperationService.new(user, self).rm_branch(branch)
+    committer = Gitlab::Git::Committer.from_user(user)
+    GitOperationService.new(committer, self).rm_branch(branch)
 
     after_remove_branch
     true
@@ -195,7 +198,8 @@ class Repository
     before_remove_tag
     tag = find_tag(tag_name)
 
-    GitOperationService.new(user, self).rm_tag(tag)
+    committer = Gitlab::Git::Committer.from_user(user)
+    GitOperationService.new(committer, self).rm_tag(tag)
 
     after_remove_tag
     true
@@ -763,7 +767,8 @@ class Repository
     author_email: nil, author_name: nil,
     start_branch_name: nil, start_project: project)
 
-    GitOperationService.new(user, self).with_branch(
+    committer = Gitlab::Git::Committer.from_user(user)
+    GitOperationService.new(committer, self).with_branch(
       branch_name,
       start_branch_name: start_branch_name,
       start_project: start_project) do |start_commit|
@@ -819,7 +824,8 @@ class Repository
   end
 
   def merge(user, source, merge_request, options = {})
-    GitOperationService.new(user, self).with_branch(
+    committer = Gitlab::Git::Committer.from_user(user)
+    GitOperationService.new(committer, self).with_branch(
       merge_request.target_branch) do |start_commit|
       our_commit = start_commit.sha
       their_commit = source
@@ -846,7 +852,8 @@ class Repository
   def revert(
     user, commit, branch_name,
     start_branch_name: nil, start_project: project)
-    GitOperationService.new(user, self).with_branch(
+    committer = Gitlab::Git::Committer.from_user(user)
+    GitOperationService.new(committer, self).with_branch(
       branch_name,
       start_branch_name: start_branch_name,
       start_project: start_project) do |start_commit|
@@ -869,7 +876,8 @@ class Repository
   def cherry_pick(
     user, commit, branch_name,
     start_branch_name: nil, start_project: project)
-    GitOperationService.new(user, self).with_branch(
+    committer = Gitlab::Git::Committer.from_user(user)
+    GitOperationService.new(committer, self).with_branch(
       branch_name,
       start_branch_name: start_branch_name,
       start_project: start_project) do |start_commit|
@@ -894,7 +902,8 @@ class Repository
   end
 
   def resolve_conflicts(user, branch_name, params)
-    GitOperationService.new(user, self).with_branch(branch_name) do
+    committer = Gitlab::Git::Committer.from_user(user)
+    GitOperationService.new(committer, self).with_branch(branch_name) do
       committer = user_to_committer(user)
 
       create_commit(params.merge(author: committer, committer: committer))
diff --git a/app/services/git_hooks_service.rb b/app/services/git_hooks_service.rb
index eab65d09299..e85007c26e0 100644
--- a/app/services/git_hooks_service.rb
+++ b/app/services/git_hooks_service.rb
@@ -3,9 +3,9 @@ class GitHooksService
 
   attr_accessor :oldrev, :newrev, :ref
 
-  def execute(user, project, oldrev, newrev, ref)
+  def execute(committer, project, oldrev, newrev, ref)
     @project    = project
-    @user       = Gitlab::GlId.gl_id(user)
+    @gl_id       = committer.gl_id
     @oldrev     = oldrev
     @newrev     = newrev
     @ref        = ref
@@ -27,6 +27,6 @@ class GitHooksService
 
   def run_hook(name)
     hook = Gitlab::Git::Hook.new(name, @project)
-    hook.trigger(@user, oldrev, newrev, ref)
+    hook.trigger(@gl_id, oldrev, newrev, ref)
   end
 end
diff --git a/app/services/git_operation_service.rb b/app/services/git_operation_service.rb
index 545ca0742e4..f7fce3d8a5d 100644
--- a/app/services/git_operation_service.rb
+++ b/app/services/git_operation_service.rb
@@ -1,8 +1,11 @@
 class GitOperationService
-  attr_reader :user, :repository
+  attr_reader :committer, :repository
 
-  def initialize(new_user, new_repository)
-    @user = new_user
+  def initialize(committer, new_repository)
+    if committer && !committer.is_a?(Gitlab::Git::Committer)
+      raise "expected Gitlab::Git::Committer, got #{committer.inspect}"
+    end
+    @committer = committer
     @repository = new_repository
   end
 
@@ -119,7 +122,7 @@ class GitOperationService
 
   def with_hooks(ref, newrev, oldrev)
     GitHooksService.new.execute(
-      user,
+      committer,
       repository.project,
       oldrev,
       newrev,