Add `source_branch` option for various git operations

If `source_branch` option is passed, and target branch cannot be found,
`Repository#update_branch_with_hooks` would try to create a new branch
from `source_branch`.

This way, we could make changes in the new branch while only firing
the hooks once for the changes. Previously, we can only create a new
branch first then make changes to the new branch, firing hooks twice.
This behaviour is bad for CI.

Fixes #7237

10 files changed, 145 insertions, 53 deletions

diff --git a/app/models/repository.rb b/app/models/repository.rb
index 063dc74021d..b6581486983 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -786,8 +786,12 @@ class Repository
     @root_ref ||= cache.fetch(:root_ref) { raw_repository.root_ref }
   end
 
-  def commit_dir(user, path, message, branch, author_email: nil, author_name: nil)
-    update_branch_with_hooks(user, branch) do |ref|
+  def commit_dir(user, path, message, branch,
+    author_email: nil, author_name: nil, source_branch: nil)
+    update_branch_with_hooks(
+      user,
+      branch,
+      source_branch: source_branch) do |ref|
       options = {
         commit: {
           branch: ref,
@@ -802,8 +806,12 @@ class Repository
     end
   end
 
-  def commit_file(user, path, content, message, branch, update, author_email: nil, author_name: nil)
-    update_branch_with_hooks(user, branch) do |ref|
+  def commit_file(user, path, content, message, branch, update,
+    author_email: nil, author_name: nil, source_branch: nil)
+    update_branch_with_hooks(
+      user,
+      branch,
+      source_branch: source_branch) do |ref|
       options = {
         commit: {
           branch: ref,
@@ -823,8 +831,13 @@ class Repository
     end
   end
 
-  def update_file(user, path, content, branch:, previous_path:, message:, author_email: nil, author_name: nil)
-    update_branch_with_hooks(user, branch) do |ref|
+  def update_file(user, path, content,
+    branch:, previous_path:, message:,
+    author_email: nil, author_name: nil, source_branch: nil)
+    update_branch_with_hooks(
+      user,
+      branch,
+      source_branch: source_branch) do |ref|
       options = {
         commit: {
           branch: ref,
@@ -849,8 +862,12 @@ class Repository
     end
   end
 
-  def remove_file(user, path, message, branch, author_email: nil, author_name: nil)
-    update_branch_with_hooks(user, branch) do |ref|
+  def remove_file(user, path, message, branch,
+    author_email: nil, author_name: nil, source_branch: nil)
+    update_branch_with_hooks(
+      user,
+      branch,
+      source_branch: source_branch) do |ref|
       options = {
         commit: {
           branch: ref,
@@ -868,17 +885,18 @@ class Repository
     end
   end
 
-  def multi_action(user:, branch:, message:, actions:, author_email: nil, author_name: nil)
-    update_branch_with_hooks(user, branch) do |ref|
+  def multi_action(user:, branch:, message:, actions:,
+    author_email: nil, author_name: nil, source_branch: nil)
+    update_branch_with_hooks(
+      user,
+      branch,
+      source_branch: source_branch) do |ref|
       index = rugged.index
       parents = []
-      branch = find_branch(ref)
 
-      if branch
-        last_commit = branch.dereferenced_target
-        index.read_tree(last_commit.raw_commit.tree)
-        parents = [last_commit.sha]
-      end
+      last_commit = find_branch(ref).dereferenced_target
+      index.read_tree(last_commit.raw_commit.tree)
+      parents = [last_commit.sha]
 
       actions.each do |action|
         case action[:action]
@@ -967,7 +985,10 @@ class Repository
 
     return false unless revert_tree_id
 
-    update_branch_with_hooks(user, base_branch) do
+    update_branch_with_hooks(
+      user,
+      base_branch,
+      source_branch: revert_tree_id) do
       committer = user_to_committer(user)
       source_sha = Rugged::Commit.create(rugged,
         message: commit.revert_message,
@@ -984,7 +1005,10 @@ class Repository
 
     return false unless cherry_pick_tree_id
 
-    update_branch_with_hooks(user, base_branch) do
+    update_branch_with_hooks(
+      user,
+      base_branch,
+      source_branch: cherry_pick_tree_id) do
       committer = user_to_committer(user)
       source_sha = Rugged::Commit.create(rugged,
         message: commit.message,
@@ -1082,11 +1106,11 @@ class Repository
     fetch_ref(path_to_repo, ref, ref_path)
   end
 
-  def update_branch_with_hooks(current_user, branch)
+  def update_branch_with_hooks(current_user, branch, source_branch: nil)
     update_autocrlf_option
 
     ref = Gitlab::Git::BRANCH_REF_PREFIX + branch
-    target_branch = find_branch(branch)
+    target_branch, new_branch_added = raw_ensure_branch(branch, source_branch)
     was_empty = empty?
 
     # Make commit
@@ -1096,7 +1120,7 @@ class Repository
       raise CommitError.new('Failed to create commit')
     end
 
-    if rugged.lookup(newrev).parent_ids.empty? || target_branch.nil?
+    if rugged.lookup(newrev).parent_ids.empty?
       oldrev = Gitlab::Git::BLANK_SHA
     else
       oldrev = rugged.merge_base(newrev, target_branch.dereferenced_target.sha)
@@ -1105,11 +1129,9 @@ class Repository
     GitHooksService.new.execute(current_user, path_to_repo, oldrev, newrev, ref) do
       update_ref!(ref, newrev, oldrev)
 
-      if was_empty || !target_branch
-        # If repo was empty expire cache
-        after_create if was_empty
-        after_create_branch
-      end
+      # If repo was empty expire cache
+      after_create if was_empty
+      after_create_branch if was_empty || new_branch_added
     end
 
     newrev
@@ -1169,4 +1191,28 @@ class Repository
   def repository_event(event, tags = {})
     Gitlab::Metrics.add_event(event, { path: path_with_namespace }.merge(tags))
   end
+
+  def raw_ensure_branch(branch_name, source_branch)
+    old_branch = find_branch(branch_name)
+
+    if old_branch
+      [old_branch, false]
+    elsif source_branch
+      oldrev = Gitlab::Git::BLANK_SHA
+      ref    = Gitlab::Git::BRANCH_REF_PREFIX + branch_name
+      target = commit(source_branch).try(:id)
+
+      unless target
+        raise CommitError.new(
+          "Cannot find branch #{branch_name} nor #{source_branch}")
+      end
+
+      update_ref!(ref, target, oldrev)
+
+      [find_branch(branch_name), true]
+    else
+      raise CommitError.new(
+        "Cannot find branch #{branch_name} and source_branch is not set")
+    end
+  end
 end
diff --git a/app/services/commits/change_service.rb b/app/services/commits/change_service.rb
index 1c82599c579..04b28cfaed8 100644
--- a/app/services/commits/change_service.rb
+++ b/app/services/commits/change_service.rb
@@ -29,7 +29,7 @@ module Commits
       tree_id = repository.public_send("check_#{action}_content", @commit, @target_branch)
 
       if tree_id
-        create_target_branch(into) if @create_merge_request
+        validate_target_branch(into) if @create_merge_request
 
         repository.public_send(action, current_user, @commit, into, tree_id)
         success
@@ -50,12 +50,12 @@ module Commits
       true
     end
 
-    def create_target_branch(new_branch)
+    def validate_target_branch(new_branch)
       # Temporary branch exists and contains the change commit
-      return success if repository.find_branch(new_branch)
+      return if repository.find_branch(new_branch)
 
-      result = CreateBranchService.new(@project, current_user)
-                                  .execute(new_branch, @target_branch, source_project: @source_project)
+      result = ValidateNewBranchService.new(@project, current_user).
+        execute(new_branch)
 
       if result[:status] == :error
         raise ChangeError, "There was an error creating the source branch: #{result[:message]}"
diff --git a/app/services/create_branch_service.rb b/app/services/create_branch_service.rb
index 757fc35a78f..f4270a09928 100644
--- a/app/services/create_branch_service.rb
+++ b/app/services/create_branch_service.rb
@@ -2,18 +2,9 @@ require_relative 'base_service'
 
 class CreateBranchService < BaseService
   def execute(branch_name, ref, source_project: @project)
-    valid_branch = Gitlab::GitRefValidator.validate(branch_name)
+    failure = validate_new_branch(branch_name)
 
-    unless valid_branch
-      return error('Branch name is invalid')
-    end
-
-    repository = project.repository
-    existing_branch = repository.find_branch(branch_name)
-
-    if existing_branch
-      return error('Branch already exists')
-    end
+    return failure if failure
 
     new_branch = if source_project != @project
                    repository.fetch_ref(
@@ -41,4 +32,13 @@ class CreateBranchService < BaseService
   def success(branch)
     super().merge(branch: branch)
   end
+
+  private
+
+  def validate_new_branch(branch_name)
+    result = ValidateNewBranchService.new(project, current_user).
+      execute(branch_name)
+
+    error(result[:message]) if result[:status] == :error
+  end
 end
diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb
index 9bd4bd464f7..6779bd2818a 100644
--- a/app/services/files/base_service.rb
+++ b/app/services/files/base_service.rb
@@ -23,9 +23,7 @@ module Files
       validate
 
       # Create new branch if it different from source_branch
-      if different_branch?
-        create_target_branch
-      end
+      validate_target_branch if different_branch?
 
       result = commit
       if result
@@ -73,10 +71,11 @@ module Files
       end
     end
 
-    def create_target_branch
-      result = CreateBranchService.new(project, current_user).execute(@target_branch, @source_branch, source_project: @source_project)
+    def validate_target_branch
+      result = ValidateNewBranchService.new(project, current_user).
+        execute(@target_branch)
 
-      unless result[:status] == :success
+      if result[:status] == :error
         raise_error("Something went wrong when we tried to create #{@target_branch} for you: #{result[:message]}")
       end
     end
diff --git a/app/services/files/create_dir_service.rb b/app/services/files/create_dir_service.rb
index d00d78cee7e..c59b3f8c70c 100644
--- a/app/services/files/create_dir_service.rb
+++ b/app/services/files/create_dir_service.rb
@@ -3,7 +3,14 @@ require_relative "base_service"
 module Files
   class CreateDirService < Files::BaseService
     def commit
-      repository.commit_dir(current_user, @file_path, @commit_message, @target_branch, author_email: @author_email, author_name: @author_name)
+      repository.commit_dir(
+        current_user,
+        @file_path,
+        @commit_message,
+        @target_branch,
+        author_email: @author_email,
+        author_name: @author_name,
+        source_branch: @source_branch)
     end
 
     def validate
diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb
index bf127843d55..6d0a0f2629d 100644
--- a/app/services/files/create_service.rb
+++ b/app/services/files/create_service.rb
@@ -3,7 +3,16 @@ require_relative "base_service"
 module Files
   class CreateService < Files::BaseService
     def commit
-      repository.commit_file(current_user, @file_path, @file_content, @commit_message, @target_branch, false, author_email: @author_email, author_name: @author_name)
+      repository.commit_file(
+        current_user,
+        @file_path,
+        @file_content,
+        @commit_message,
+        @target_branch,
+        false,
+        author_email: @author_email,
+        author_name: @author_name,
+        source_branch: @source_branch)
     end
 
     def validate
diff --git a/app/services/files/delete_service.rb b/app/services/files/delete_service.rb
index 8b27ad51789..79d592731e9 100644
--- a/app/services/files/delete_service.rb
+++ b/app/services/files/delete_service.rb
@@ -3,7 +3,14 @@ require_relative "base_service"
 module Files
   class DeleteService < Files::BaseService
     def commit
-      repository.remove_file(current_user, @file_path, @commit_message, @target_branch, author_email: @author_email, author_name: @author_name)
+      repository.remove_file(
+        current_user,
+        @file_path,
+        @commit_message,
+        @target_branch,
+        author_email: @author_email,
+        author_name: @author_name,
+        source_branch: @source_branch)
     end
   end
 end
diff --git a/app/services/files/multi_service.rb b/app/services/files/multi_service.rb
index d28912e1301..0550dec15a6 100644
--- a/app/services/files/multi_service.rb
+++ b/app/services/files/multi_service.rb
@@ -11,7 +11,8 @@ module Files
         message: @commit_message,
         actions: params[:actions],
         author_email: @author_email,
-        author_name: @author_name
+        author_name: @author_name,
+        source_branch: @source_branch
       )
     end
 
diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb
index c17fdb8d1f1..f3a766ed9fd 100644
--- a/app/services/files/update_service.rb
+++ b/app/services/files/update_service.rb
@@ -10,7 +10,8 @@ module Files
                              previous_path: @previous_path,
                              message: @commit_message,
                              author_email: @author_email,
-                             author_name: @author_name)
+                             author_name: @author_name,
+                             source_branch: @source_branch)
     end
 
     private
diff --git a/app/services/validate_new_branch_service.rb b/app/services/validate_new_branch_service.rb
new file mode 100644
index 00000000000..2f61be184ce
--- /dev/null
+++ b/app/services/validate_new_branch_service.rb
@@ -0,0 +1,22 @@
+require_relative 'base_service'
+
+class ValidateNewBranchService < BaseService
+  def execute(branch_name)
+    valid_branch = Gitlab::GitRefValidator.validate(branch_name)
+
+    unless valid_branch
+      return error('Branch name is invalid')
+    end
+
+    repository = project.repository
+    existing_branch = repository.find_branch(branch_name)
+
+    if existing_branch
+      return error('Branch already exists')
+    end
+
+    success
+  rescue GitHooksService::PreReceiveError => ex
+    error(ex.message)
+  end
+end