From f4149bcddca9c0e7aac078b3e7c198f5624ea107 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Fri, 14 Aug 2015 16:23:40 +0200
Subject: [PATCH] Refactor how repository makes commit with pre/post receive
 hooks

---
 app/models/repository.rb             | 119 ++++++++++++++++++++-------
 app/services/commit_service.rb       |  56 -------------
 app/services/files/base_service.rb   |   2 +-
 app/services/files/create_service.rb |   4 +-
 app/services/files/delete_service.rb |   4 +-
 app/services/files/update_service.rb |   4 +-
 6 files changed, 94 insertions(+), 95 deletions(-)

diff --git a/app/models/repository.rb b/app/models/repository.rb
index 32b3541907f..99b6cd3cad1 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -1,4 +1,9 @@
+require 'securerandom'
+
 class Repository
+  class PreReceiveError < StandardError; end
+  class CommitError < StandardError; end
+
   include Gitlab::ShellAdapter
 
   attr_accessor :raw_repository, :path_with_namespace, :project
@@ -364,43 +369,47 @@ class Repository
     @root_ref ||= raw_repository.root_ref
   end
 
-  def commit_file(user, path, content, message, ref)
-    path[0] = '' if path[0] == '/'
+  def commit_file(user, path, content, message, branch)
+    commit_with_hooks(user, branch) do |ref|
+      path[0] = '' if path[0] == '/'
 
-    committer = user_to_comitter(user)
-    options = {}
-    options[:committer] = committer
-    options[:author] = committer
-    options[:commit] = {
-      message: message,
-      branch: ref,
-    }
+      committer = user_to_comitter(user)
+      options = {}
+      options[:committer] = committer
+      options[:author] = committer
+      options[:commit] = {
+        message: message,
+        branch: ref,
+      }
 
-    options[:file] = {
-      content: content,
-      path: path
-    }
+      options[:file] = {
+        content: content,
+        path: path
+      }
 
-    Gitlab::Git::Blob.commit(raw_repository, options)
+      Gitlab::Git::Blob.commit(raw_repository, options)
+    end
   end
 
-  def remove_file(user, path, message, ref)
-    path[0] = '' if path[0] == '/'
+  def remove_file(user, path, message, branch)
+    commit_with_hooks(user, branch) do |branch|
+      path[0] = '' if path[0] == '/'
 
-    committer = user_to_comitter(user)
-    options = {}
-    options[:committer] = committer
-    options[:author] = committer
-    options[:commit] = {
-      message: message,
-      branch: ref
-    }
+      committer = user_to_comitter(user)
+      options = {}
+      options[:committer] = committer
+      options[:author] = committer
+      options[:commit] = {
+        message: message,
+        branch: ref
+      }
 
-    options[:file] = {
-      path: path
-    }
+      options[:file] = {
+        path: path
+      }
 
-    Gitlab::Git::Blob.remove(raw_repository, options)
+      Gitlab::Git::Blob.remove(raw_repository, options)
+    end
   end
 
   def user_to_comitter(user)
@@ -479,6 +488,58 @@ class Repository
     Gitlab::Popen.popen(args, path_to_repo)
   end
 
+  def commit_with_hooks(current_user, branch)
+    oldrev = Gitlab::Git::BLANK_SHA
+    ref = Gitlab::Git::BRANCH_REF_PREFIX + branch
+    gl_id = Gitlab::ShellEnv.gl_id(current_user)
+
+    # Create temporary ref
+    random_string = SecureRandom.hex
+    tmp_ref = "refs/tmp/#{random_string}/head"
+
+    unless empty?
+      oldrev = find_branch(branch).target
+      rugged.references.create(tmp_ref, oldrev)
+    end
+
+    # Make commit in tmp ref
+    newrev = yield(tmp_ref)
+
+    unless newrev
+      raise CommitError.new('Failed to create commit')
+    end
+
+    # Run GitLab pre-receive hook
+    pre_receive_hook = Gitlab::Git::Hook.new('pre-receive', path_to_repo)
+    status = pre_receive_hook.trigger(gl_id, oldrev, newrev, ref)
+
+    if status
+      if empty?
+        # Create branch
+        rugged.references.create(ref, newrev)
+      else
+        # Update head
+        current_head = find_branch(branch).target
+
+        # Make sure target branch was not changed during pre-receive hook
+        if current_head == oldrev
+          rugged.references.update(ref, newrev)
+        else
+          raise CommitError.new('Commit was rejected because branch received new push')
+        end
+      end
+
+      # Run GitLab post receive hook
+      post_receive_hook = Gitlab::Git::Hook.new('post-receive', path_to_repo)
+      status = post_receive_hook.trigger(gl_id, oldrev, newrev, ref)
+    else
+      # Remove tmp ref and return error to user
+      rugged.references.delete(tmp_ref)
+
+      raise PreReceiveError.new('Commit was rejected by pre-reveive hook')
+    end
+  end
+
   private
 
   def cache
diff --git a/app/services/commit_service.rb b/app/services/commit_service.rb
index 5000e73b473..c77da061a9c 100644
--- a/app/services/commit_service.rb
+++ b/app/services/commit_service.rb
@@ -1,61 +1,5 @@
-require 'securerandom'
 
 class CommitService
-  class PreReceiveError < StandardError; end
-  class CommitError < StandardError; end
-
   def self.transaction(project, current_user, branch)
-    repository = project.repository
-    path_to_repo = repository.path_to_repo
-    empty_repo = repository.empty?
-    oldrev = Gitlab::Git::BLANK_SHA
-    ref = Gitlab::Git::BRANCH_REF_PREFIX + branch
-    gl_id = Gitlab::ShellEnv.gl_id(current_user)
-
-    # Create temporary ref
-    random_string = SecureRandom.hex
-    tmp_ref = "refs/tmp/#{random_string}/head"
-
-    unless empty_repo
-      oldrev = repository.find_branch(branch).target
-      repository.rugged.references.create(tmp_ref, oldrev)
-    end
-
-    # Make commit in tmp ref
-    newrev = yield(tmp_ref)
-
-    unless newrev
-      raise CommitError.new('Failed to create commit')
-    end
-
-    # Run GitLab pre-receive hook
-    pre_receive_hook = Gitlab::Git::Hook.new('pre-receive', path_to_repo)
-    status = pre_receive_hook.trigger(gl_id, oldrev, newrev, ref)
-
-    if status
-      if empty_repo
-        # Create branch
-        repository.rugged.references.create(ref, newrev)
-      else
-        # Update head
-        current_head = repository.find_branch(branch).target
-
-        # Make sure target branch was not changed during pre-receive hook
-        if current_head == oldrev
-          repository.rugged.references.update(ref, newrev)
-        else
-          raise CommitError.new('Commit was rejected because branch received new push')
-        end
-      end
-
-      # Run GitLab post receive hook
-      post_receive_hook = Gitlab::Git::Hook.new('post-receive', path_to_repo)
-      status = post_receive_hook.trigger(gl_id, oldrev, newrev, ref)
-    else
-      # Remove tmp ref and return error to user
-      repository.rugged.references.delete(tmp_ref)
-
-      raise PreReceiveError.new('Commit was rejected by pre-reveive hook')
-    end
   end
 end
diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb
index 507e21f5818..7aecee217d8 100644
--- a/app/services/files/base_service.rb
+++ b/app/services/files/base_service.rb
@@ -26,7 +26,7 @@ module Files
       else
         error("Something went wrong. Your changes were not committed")
       end
-    rescue CommitService::CommitError, CommitService::PreReceiveError, ValidationError => ex
+    rescue Repository::CommitError, Repository::PreReceiveError, ValidationError => ex
       error(ex.message)
     end
 
diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb
index 3e00864f00d..91d715b2d63 100644
--- a/app/services/files/create_service.rb
+++ b/app/services/files/create_service.rb
@@ -3,9 +3,7 @@ require_relative "base_service"
 module Files
   class CreateService < Files::BaseService
     def commit
-      CommitService.transaction(project, current_user, @target_branch)  do |tmp_ref|
-        repository.commit_file(current_user, @file_path, @file_content, @commit_message, tmp_ref)
-      end
+      repository.commit_file(current_user, @file_path, @file_content, @commit_message, @target_branch)
     end
 
     def validate
diff --git a/app/services/files/delete_service.rb b/app/services/files/delete_service.rb
index d61ca31cf9d..27c881c3430 100644
--- a/app/services/files/delete_service.rb
+++ b/app/services/files/delete_service.rb
@@ -3,9 +3,7 @@ require_relative "base_service"
 module Files
   class DeleteService < Files::BaseService
     def commit
-      CommitService.transaction(project, current_user, @target_branch)  do |tmp_ref|
-        repository.remove_file(current_user, @file_path, @commit_message, tmp_ref)
-      end
+      repository.remove_file(current_user, @file_path, @commit_message, @target_branch)
     end
   end
 end
diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb
index 69212b36996..a20903c6f02 100644
--- a/app/services/files/update_service.rb
+++ b/app/services/files/update_service.rb
@@ -3,9 +3,7 @@ require_relative "base_service"
 module Files
   class UpdateService < Files::BaseService
     def commit
-      CommitService.transaction(project, current_user, @target_branch)  do |tmp_ref|
-        repository.commit_file(current_user, @file_path, @file_content, @commit_message, tmp_ref)
-      end
+      repository.commit_file(current_user, @file_path, @file_content, @commit_message, @target_branch)
     end
   end
 end