diff --git a/app/models/repository.rb b/app/models/repository.rb index 063dc74021d5ce5f04a2b85bef9998980fb7aff5..b6581486983e7323711f14d3b917cb47c8d010af 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 1c82599c5793d3f92db2ba4d4ee502a0e3f3ab58..04b28cfaed8d7daed6a0109bb121f3c5bc886e6d 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 757fc35a78fdf54f0a6c58498c6f371dbf94b48e..f4270a0992884d46b53adb8191049fb947545471 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 9bd4bd464f7940253aeb7fd48014fdae3ae22cb6..6779bd2818a715eee785764a32cfd2f462df2b27 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 d00d78cee7ee5f724d362b0ff639178983c0b399..c59b3f8c70c80cf01a71b438ab1a14b86a426bcf 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 bf127843d55519539e9414ff4a6c12bfe40ffe01..6d0a0f2629d24ae9aa862674eaf2b9bd08f7ca2c 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 8b27ad51789bc54d2f64a160bb09a0fb2d4942c0..79d592731e990c8c6afa6ab1bd3f71ba617804fc 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 d28912e1301cdac9f5af947a53bbebb083396495..0550dec15a6a52655d17804b2d9accd753957893 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 c17fdb8d1f11779c1624714a0550b1cdf240977a..f3a766ed9fd1c8c5379710667be1064a59c7bb75 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 0000000000000000000000000000000000000000..2f61be184ce27da09eb72011e48be6c8e1a0de90 --- /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