From b82f415f09dd67da010a8c08397a13499e70efeb Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 22 Nov 2016 18:14:41 +0800 Subject: [PATCH] Move all branch creation to raw_ensure_branch, and keep it only called in update_branch_with_hooks. --- app/models/project.rb | 11 ---- app/models/repository.rb | 85 ++++++++++++++++++++------- app/services/create_branch_service.rb | 8 +-- app/services/files/base_service.rb | 13 +--- app/services/files/update_service.rb | 1 + 5 files changed, 69 insertions(+), 49 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 5346e18b051..f8a54324341 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -998,17 +998,6 @@ class Project < ActiveRecord::Base Gitlab::UploadsTransfer.new.rename_project(path_was, path, namespace.path) end - def fetch_ref(source_project, branch_name, ref) - repository.fetch_ref( - source_project.repository.path_to_repo, - "refs/heads/#{ref}", - "refs/heads/#{branch_name}" - ) - - repository.after_create_branch - repository.find_branch(branch_name) - end - # Expires various caches before a project is renamed. def expire_caches_before_rename(old_path) repo = Repository.new(old_path, self) diff --git a/app/models/repository.rb b/app/models/repository.rb index a029993db7f..9162f494a60 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -784,13 +784,16 @@ class Repository @tags ||= raw_repository.tags end + # rubocop:disable Metrics/ParameterLists def commit_dir( user, path, message, branch, - author_email: nil, author_name: nil, source_branch: nil) + author_email: nil, author_name: nil, + source_branch: nil, source_project: project) update_branch_with_hooks( user, branch, - source_branch: source_branch) do |ref| + source_branch: source_branch, + source_project: source_project) do |ref| options = { commit: { branch: ref, @@ -804,15 +807,18 @@ class Repository raw_repository.mkdir(path, options) end end + # rubocop:enable Metrics/ParameterLists # rubocop:disable Metrics/ParameterLists def commit_file( user, path, content, message, branch, update, - author_email: nil, author_name: nil, source_branch: nil) + author_email: nil, author_name: nil, + source_branch: nil, source_project: project) update_branch_with_hooks( user, branch, - source_branch: source_branch) do |ref| + source_branch: source_branch, + source_project: source_project) do |ref| options = { commit: { branch: ref, @@ -837,11 +843,13 @@ class Repository def update_file( user, path, content, branch:, previous_path:, message:, - author_email: nil, author_name: nil, source_branch: nil) + author_email: nil, author_name: nil, + source_branch: nil, source_project: project) update_branch_with_hooks( user, branch, - source_branch: source_branch) do |ref| + source_branch: source_branch, + source_project: source_project) do |ref| options = { commit: { branch: ref, @@ -867,13 +875,16 @@ class Repository end # rubocop:enable Metrics/ParameterLists + # rubocop:disable Metrics/ParameterLists def remove_file( user, path, message, branch, - author_email: nil, author_name: nil, source_branch: nil) + author_email: nil, author_name: nil, + source_branch: nil, source_project: project) update_branch_with_hooks( user, branch, - source_branch: source_branch) do |ref| + source_branch: source_branch, + source_project: source_project) do |ref| options = { commit: { branch: ref, @@ -890,14 +901,18 @@ class Repository Gitlab::Git::Blob.remove(raw_repository, options) end end + # rubocop:enable Metrics/ParameterLists + # rubocop:disable Metrics/ParameterLists def multi_action( user:, branch:, message:, actions:, - author_email: nil, author_name: nil, source_branch: nil) + author_email: nil, author_name: nil, + source_branch: nil, source_project: project) update_branch_with_hooks( user, branch, - source_branch: source_branch) do |ref| + source_branch: source_branch, + source_project: source_project) do |ref| index = rugged.index branch_commit = find_branch(ref) @@ -942,6 +957,7 @@ class Repository Rugged::Commit.create(rugged, options) end end + # rubocop:enable Metrics/ParameterLists def get_committer_and_author(user, email: nil, name: nil) committer = user_to_committer(user) @@ -991,8 +1007,6 @@ class Repository end def revert(user, commit, base_branch, revert_tree_id = nil) - source_sha = raw_ensure_branch(base_branch, source_commit: commit). - first.dereferenced_target.sha revert_tree_id ||= check_revert_content(commit, base_branch) return false unless revert_tree_id @@ -1000,8 +1014,11 @@ class Repository update_branch_with_hooks( user, base_branch, - source_branch: revert_tree_id) do + source_commit: commit) do + + source_sha = find_branch(base_branch).dereferenced_target.sha committer = user_to_committer(user) + Rugged::Commit.create(rugged, message: commit.revert_message, author: committer, @@ -1012,8 +1029,6 @@ class Repository end def cherry_pick(user, commit, base_branch, cherry_pick_tree_id = nil) - source_sha = raw_ensure_branch(base_branch, source_commit: commit). - first.dereferenced_target.sha cherry_pick_tree_id ||= check_cherry_pick_content(commit, base_branch) return false unless cherry_pick_tree_id @@ -1021,8 +1036,11 @@ class Repository update_branch_with_hooks( user, base_branch, - source_branch: cherry_pick_tree_id) do + source_commit: commit) do + + source_sha = find_branch(base_branch).dereferenced_target.sha committer = user_to_committer(user) + Rugged::Commit.create(rugged, message: commit.message, author: { @@ -1130,12 +1148,19 @@ class Repository # Whenever `source_branch` is passed, if `branch` doesn't exist, it would # be created from `source_branch`. - def update_branch_with_hooks(current_user, branch, source_branch: nil) + def update_branch_with_hooks( + current_user, branch, + source_branch: nil, source_commit: nil, source_project: project) update_autocrlf_option - ref = Gitlab::Git::BRANCH_REF_PREFIX + branch target_branch, new_branch_added = - raw_ensure_branch(branch, source_branch: source_branch) + raw_ensure_branch( + branch, + source_branch: source_branch, + source_project: source_project + ) + + ref = Gitlab::Git::BRANCH_REF_PREFIX + branch was_empty = empty? # Make commit @@ -1244,11 +1269,31 @@ class Repository Gitlab::Metrics.add_event(event, { path: path_with_namespace }.merge(tags)) end - def raw_ensure_branch(branch_name, source_commit: nil, source_branch: nil) + def raw_ensure_branch( + branch_name, source_commit: nil, source_branch: nil, source_project: nil) old_branch = find_branch(branch_name) + if source_commit && source_branch + raise ArgumentError, + 'Should only pass either :source_branch or :source_commit, not both' + end + if old_branch [old_branch, false] + elsif project != source_project + unless source_branch + raise ArgumentError, + 'Should also pass :source_branch if' + + ' :source_project is different from current project' + end + + fetch_ref( + source_project.repository.path_to_repo, + "#{Gitlab::Git::BRANCH_REF_PREFIX}#{source_branch}", + "#{Gitlab::Git::BRANCH_REF_PREFIX}#{branch}" + ) + + [find_branch(branch_name), true] elsif source_commit || source_branch oldrev = Gitlab::Git::BLANK_SHA ref = Gitlab::Git::BRANCH_REF_PREFIX + branch_name diff --git a/app/services/create_branch_service.rb b/app/services/create_branch_service.rb index ecb5994fab8..b2bc3626c0f 100644 --- a/app/services/create_branch_service.rb +++ b/app/services/create_branch_service.rb @@ -1,16 +1,12 @@ require_relative 'base_service' class CreateBranchService < BaseService - def execute(branch_name, ref, source_project: @project) + def execute(branch_name, ref) failure = validate_new_branch(branch_name) return failure if failure - new_branch = if source_project != @project - @project.fetch_ref(source_project, branch_name, ref) - else - repository.add_branch(current_user, branch_name, ref) - end + new_branch = repository.add_branch(current_user, branch_name, ref) if new_branch success(new_branch) diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb index 8689c83d40e..6779bd2818a 100644 --- a/app/services/files/base_service.rb +++ b/app/services/files/base_service.rb @@ -23,7 +23,7 @@ module Files validate # Create new branch if it different from source_branch - ensure_target_branch if different_branch? + validate_target_branch if different_branch? result = commit if result @@ -71,17 +71,6 @@ module Files end end - def ensure_target_branch - validate_target_branch - - if @source_project != project - # Make sure we have the source_branch in target project, - # and use source_branch as target_branch directly to avoid - # unnecessary source branch in target project. - @project.fetch_ref(@source_project, @target_branch, @source_branch) - end - end - def validate_target_branch result = ValidateNewBranchService.new(project, current_user). execute(@target_branch) diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb index f3a766ed9fd..14e5af4d8c6 100644 --- a/app/services/files/update_service.rb +++ b/app/services/files/update_service.rb @@ -11,6 +11,7 @@ module Files message: @commit_message, author_email: @author_email, author_name: @author_name, + source_project: @source_project, source_branch: @source_branch) end