Commit 1b7f8c19 authored by Jan Provaznik's avatar Jan Provaznik

Merge branch 'brodock/branches-services-refactor' into 'master'

Refactor Branch related services to move to `branches` folder

See merge request gitlab-org/gitlab!21012
parents e8fd4888 9b26edbe
......@@ -46,7 +46,7 @@ class Projects::BranchesController < Projects::ApplicationController
def diverging_commit_counts
respond_to do |format|
format.json do
service = Branches::DivergingCommitCountsService.new(repository)
service = ::Branches::DivergingCommitCountsService.new(repository)
branches = BranchesFinder.new(repository, params.permit(names: [])).execute
Gitlab::GitalyClient.allow_n_plus_1_calls do
......@@ -63,7 +63,7 @@ class Projects::BranchesController < Projects::ApplicationController
redirect_to_autodeploy = project.empty_repo? && project.deployment_platform.present?
result = CreateBranchService.new(project, current_user)
result = ::Branches::CreateService.new(project, current_user)
.execute(branch_name, ref)
success = (result[:status] == :success)
......@@ -102,7 +102,7 @@ class Projects::BranchesController < Projects::ApplicationController
def destroy
@branch_name = Addressable::URI.unescape(params[:id])
result = DeleteBranchService.new(project, current_user).execute(@branch_name)
result = ::Branches::DeleteService.new(project, current_user).execute(@branch_name)
respond_to do |format|
format.html do
......@@ -118,7 +118,7 @@ class Projects::BranchesController < Projects::ApplicationController
end
def destroy_all_merged
DeleteMergedBranchesService.new(@project, current_user).async_execute
::Branches::DeleteMergedService.new(@project, current_user).async_execute
redirect_to project_branches_path(@project),
notice: _('Merged branches are being deleted. This can take some time depending on the number of branches. Please refresh the page to see changes.')
......
# frozen_string_literal: true
module Branches
class CreateService < BaseService
def execute(branch_name, ref, create_master_if_empty: true)
create_master_branch if create_master_if_empty && project.empty_repo?
result = ::Branches::ValidateNewService.new(project).execute(branch_name)
return result if result[:status] == :error
new_branch = repository.add_branch(current_user, branch_name, ref)
if new_branch
success(new_branch)
else
error("Invalid reference name: #{branch_name}")
end
rescue Gitlab::Git::PreReceiveError => ex
error(ex.message)
end
def success(branch)
super().merge(branch: branch)
end
private
def create_master_branch
project.repository.create_file(
current_user,
'/README.md',
'',
message: 'Add README.md',
branch_name: 'master'
)
end
end
end
# frozen_string_literal: true
module Branches
class DeleteMergedService < BaseService
def async_execute
DeleteMergedBranchesWorker.perform_async(project.id, current_user.id)
end
def execute
raise Gitlab::Access::AccessDeniedError unless can?(current_user, :push_code, project)
branches = project.repository.merged_branch_names
# Prevent deletion of branches relevant to open merge requests
branches -= merge_request_branch_names
# Prevent deletion of protected branches
branches = branches.reject { |branch| ProtectedBranch.protected?(project, branch) }
branches.each do |branch|
::Branches::DeleteService.new(project, current_user).execute(branch)
end
end
private
# rubocop: disable CodeReuse/ActiveRecord
def merge_request_branch_names
# reorder(nil) is necessary for SELECT DISTINCT because default scope adds an ORDER BY
source_names = project.origin_merge_requests.opened.reorder(nil).distinct.pluck(:source_branch)
target_names = project.merge_requests.opened.reorder(nil).distinct.pluck(:target_branch)
(source_names + target_names).uniq
end
# rubocop: enable CodeReuse/ActiveRecord
end
end
# frozen_string_literal: true
module Branches
class DeleteService < BaseService
def execute(branch_name)
repository = project.repository
branch = repository.find_branch(branch_name)
unless current_user.can?(:push_code, project)
return ServiceResponse.error(
message: 'You dont have push access to repo',
http_status: 405)
end
unless branch
return ServiceResponse.error(
message: 'No such branch',
http_status: 404)
end
if repository.rm_branch(current_user, branch_name)
ServiceResponse.success(message: 'Branch was deleted')
else
ServiceResponse.error(
message: 'Failed to remove branch',
http_status: 400)
end
rescue Gitlab::Git::PreReceiveError => ex
ServiceResponse.error(message: ex.message, http_status: 400)
end
end
end
# frozen_string_literal: true
module Branches
class ValidateNewService < BaseService
def initialize(project)
@project = project
end
def execute(branch_name, force: false)
return error('Branch name is invalid') unless valid_name?(branch_name)
if branch_exist?(branch_name) && !force
return error('Branch already exists')
end
success
rescue Gitlab::Git::PreReceiveError => ex
error(ex.message)
end
private
def valid_name?(branch_name)
Gitlab::GitRefValidator.validate(branch_name)
end
def branch_exist?(branch_name)
project.repository.branch_exists?(branch_name)
end
end
end
......@@ -32,7 +32,7 @@ module Commits
end
def prepare_branch!
branch_result = CreateBranchService.new(project, current_user)
branch_result = ::Branches::CreateService.new(project, current_user)
.execute(@branch_name, @start_branch)
if branch_result[:status] != :success
......
......@@ -101,7 +101,7 @@ module Commits
end
def validate_new_branch_name!
result = ValidateNewBranchService.new(project, current_user).execute(@branch_name, force: force?)
result = ::Branches::ValidateNewService.new(project).execute(@branch_name, force: force?)
if result[:status] == :error
raise_error("Something went wrong when we tried to create '#{@branch_name}' for you: #{result[:message]}")
......
# frozen_string_literal: true
class CreateBranchService < BaseService
def execute(branch_name, ref, create_master_if_empty: true)
create_master_branch if create_master_if_empty && project.empty_repo?
result = ValidateNewBranchService.new(project, current_user)
.execute(branch_name)
return result if result[:status] == :error
new_branch = repository.add_branch(current_user, branch_name, ref)
if new_branch
success(new_branch)
else
error("Invalid reference name: #{branch_name}")
end
rescue Gitlab::Git::PreReceiveError => ex
error(ex.message)
end
def success(branch)
super().merge(branch: branch)
end
private
def create_master_branch
project.repository.create_file(
current_user,
'/README.md',
'',
message: 'Add README.md',
branch_name: 'master'
)
end
end
# frozen_string_literal: true
class DeleteBranchService < BaseService
def execute(branch_name)
repository = project.repository
branch = repository.find_branch(branch_name)
unless current_user.can?(:push_code, project)
return ServiceResponse.error(
message: 'You dont have push access to repo',
http_status: 405)
end
unless branch
return ServiceResponse.error(
message: 'No such branch',
http_status: 404)
end
if repository.rm_branch(current_user, branch_name)
ServiceResponse.success(message: 'Branch was deleted')
else
ServiceResponse.error(
message: 'Failed to remove branch',
http_status: 400)
end
rescue Gitlab::Git::PreReceiveError => ex
ServiceResponse.error(message: ex.message, http_status: 400)
end
end
# frozen_string_literal: true
class DeleteMergedBranchesService < BaseService
def async_execute
DeleteMergedBranchesWorker.perform_async(project.id, current_user.id)
end
def execute
raise Gitlab::Access::AccessDeniedError unless can?(current_user, :push_code, project)
branches = project.repository.merged_branch_names
# Prevent deletion of branches relevant to open merge requests
branches -= merge_request_branch_names
# Prevent deletion of protected branches
branches = branches.reject { |branch| ProtectedBranch.protected?(project, branch) }
branches.each do |branch|
DeleteBranchService.new(project, current_user).execute(branch)
end
end
private
# rubocop: disable CodeReuse/ActiveRecord
def merge_request_branch_names
# reorder(nil) is necessary for SELECT DISTINCT because default scope adds an ORDER BY
source_names = project.origin_merge_requests.opened.reorder(nil).distinct.pluck(:source_branch)
target_names = project.merge_requests.opened.reorder(nil).distinct.pluck(:target_branch)
(source_names + target_names).uniq
end
# rubocop: enable CodeReuse/ActiveRecord
end
......@@ -19,7 +19,7 @@ module MergeRequests
return error('Not allowed to create merge request') unless can_create_merge_request?
return error('Invalid issue iid') unless @issue_iid.present? && issue.present?
result = CreateBranchService.new(target_project, current_user).execute(branch_name, ref)
result = ::Branches::CreateService.new(target_project, current_user).execute(branch_name, ref)
return result if result[:status] == :error
new_merge_request = create(merge_request)
......
......@@ -99,7 +99,7 @@ module MergeRequests
log_info("Post merge finished on JID #{merge_jid} with state #{state}")
if delete_source_branch?
DeleteBranchService.new(@merge_request.source_project, branch_deletion_user)
::Branches::DeleteService.new(@merge_request.source_project, branch_deletion_user)
.execute(merge_request.source_branch)
end
end
......
# frozen_string_literal: true
require_relative 'base_service'
class ValidateNewBranchService < BaseService
def execute(branch_name, force: false)
valid_branch = Gitlab::GitRefValidator.validate(branch_name)
unless valid_branch
return error('Branch name is invalid')
end
if project.repository.branch_exists?(branch_name) && !force
return error('Branch already exists')
end
success
rescue Gitlab::Git::PreReceiveError => ex
error(ex.message)
end
end
......@@ -15,7 +15,7 @@ class DeleteMergedBranchesWorker
user = User.find(user_id)
begin
DeleteMergedBranchesService.new(project, user).execute
::Branches::DeleteMergedService.new(project, user).execute
rescue Gitlab::Access::AccessDeniedError
return
end
......
......@@ -51,7 +51,7 @@ module Projects
local_branch = local_branches[name]
if local_branch.nil?
result = CreateBranchService.new(project, current_user).execute(name, upstream_branch.dereferenced_target.sha, create_master_if_empty: false)
result = ::Branches::CreateService.new(project, current_user).execute(name, upstream_branch.dereferenced_target.sha, create_master_if_empty: false)
if result[:status] == :error
errors << result[:message]
end
......
......@@ -93,7 +93,7 @@ module VulnerabilityFeedback
branch_name = merge_request.source_branch
merge_request&.destroy &&
DeleteBranchService.new(project, current_user).execute(branch_name)
::Branches::DeleteService.new(project, current_user).execute(branch_name)
end
end
end
......@@ -2,7 +2,7 @@
require 'spec_helper'
describe DeleteBranchService do
describe Branches::DeleteService do
let(:project) { create(:project, :repository) }
let(:repository) { project.repository }
let(:user) { create(:user) }
......
......@@ -269,7 +269,7 @@ describe Projects::UpdateMirrorService do
repository = project.repository
allow(project).to receive(:fetch_mirror) { create_file(repository) }
expect(CreateBranchService).not_to receive(:create_master_branch)
expect(::Branches::CreateService).not_to receive(:create_master_branch)
service.execute
......
......@@ -137,7 +137,7 @@ module API
post ':id/repository/branches' do
authorize_push_project
result = CreateBranchService.new(user_project, current_user)
result = ::Branches::CreateService.new(user_project, current_user)
.execute(params[:branch], params[:ref])
if result[:status] == :success
......@@ -162,7 +162,7 @@ module API
commit = user_project.repository.commit(branch.dereferenced_target)
destroy_conditionally!(commit, last_updated: commit.authored_date) do
result = DeleteBranchService.new(user_project, current_user)
result = ::Branches::DeleteService.new(user_project, current_user)
.execute(params[:branch])
if result.error?
......@@ -173,7 +173,7 @@ module API
desc 'Delete all merged branches'
delete ':id/repository/merged_branches' do
DeleteMergedBranchesService.new(user_project, current_user).async_execute
::Branches::DeleteMergedService.new(user_project, current_user).async_execute
accepted!
end
......
......@@ -178,7 +178,7 @@ describe Projects::BranchesController do
it 'redirects to newly created branch' do
result = { status: :success, branch: double(name: branch) }
expect_any_instance_of(CreateBranchService).to receive(:execute).and_return(result)
expect_any_instance_of(::Branches::CreateService).to receive(:execute).and_return(result)
expect(SystemNoteService).to receive(:new_issue_branch).and_return(true)
post :create,
......@@ -200,7 +200,7 @@ describe Projects::BranchesController do
it 'redirects to autodeploy setup page' do
result = { status: :success, branch: double(name: branch) }
expect_any_instance_of(CreateBranchService).to receive(:execute).and_return(result)
expect_any_instance_of(::Branches::CreateService).to receive(:execute).and_return(result)
expect(SystemNoteService).to receive(:new_issue_branch).and_return(true)
post :create,
......@@ -221,7 +221,7 @@ describe Projects::BranchesController do
create(:cluster, :provided_by_gcp, projects: [project])
expect_any_instance_of(CreateBranchService).to receive(:execute).and_return(result)
expect_any_instance_of(::Branches::CreateService).to receive(:execute).and_return(result)
expect(SystemNoteService).to receive(:new_issue_branch).and_return(true)
post :create,
......@@ -459,7 +459,7 @@ describe Projects::BranchesController do
end
it 'starts worker to delete merged branches' do
expect_any_instance_of(DeleteMergedBranchesService).to receive(:async_execute)
expect_any_instance_of(::Branches::DeleteMergedService).to receive(:async_execute)
destroy_all_merged
end
......
......@@ -9,7 +9,7 @@ describe 'Merge request > User sees deleted target branch', :js do
before do
project.add_maintainer(user)
DeleteBranchService.new(project, user).execute('feature')
::Branches::DeleteService.new(project, user).execute('feature')
sign_in(user)
visit project_merge_request_path(project, merge_request)
end
......
......@@ -178,10 +178,11 @@ describe 'Merge request > User selects branches for new MR', :js do
end
context 'with special characters in branch names' do
let(:create_branch_service) { ::Branches::CreateService.new(project, user) }
it 'escapes quotes in branch names' do
special_branch_name = '"with-quotes"'
CreateBranchService.new(project, user)
.execute(special_branch_name, 'add-pdf-file')
create_branch_service.execute(special_branch_name, 'add-pdf-file')
visit project_new_merge_request_path(project)
select_source_branch(special_branch_name)
......@@ -192,8 +193,7 @@ describe 'Merge request > User selects branches for new MR', :js do
it 'does not escape unicode in branch names' do
special_branch_name = 'ʕ•ᴥ•ʔ'
CreateBranchService.new(project, user)
.execute(special_branch_name, 'add-pdf-file')
create_branch_service.execute(special_branch_name, 'add-pdf-file')
visit project_new_merge_request_path(project)
select_source_branch(special_branch_name)
......
......@@ -131,7 +131,7 @@ describe API::Branches do
end
new_branch_name = 'protected-branch'
CreateBranchService.new(project, current_user).execute(new_branch_name, 'master')
::Branches::CreateService.new(project, current_user).execute(new_branch_name, 'master')
create(:protected_branch, name: new_branch_name, project: project)
expect do
......
......@@ -2,9 +2,9 @@
require 'spec_helper'
describe CreateBranchService do
describe Branches::CreateService do
let(:user) { create(:user) }
let(:service) { described_class.new(project, user) }
subject(:service) { described_class.new(project, user) }
describe '#execute' do
context 'when repository is empty' do
......@@ -30,7 +30,7 @@ describe CreateBranchService do
allow(project.repository).to receive(:add_branch).and_return(false)
end
it 'retruns an error with the branch name' do
it 'returns an error with the branch name' do
result = service.execute('my-feature', 'master')
expect(result[:status]).to eq(:error)
......
......@@ -2,7 +2,7 @@
require 'spec_helper'
describe DeleteMergedBranchesService do
describe Branches::DeleteMergedService do
include ProjectForksHelper
subject(:service) { described_class.new(project, project.owner) }
......
......@@ -2,11 +2,11 @@
require 'spec_helper'
describe DeleteBranchService do
describe Branches::DeleteService do
let(:project) { create(:project, :repository) }
let(:repository) { project.repository }
let(:user) { create(:user) }
let(:service) { described_class.new(project, user) }
subject(:service) { described_class.new(project, user) }
shared_examples 'a deleted branch' do |branch_name|
it 'removes the branch' do
......
# frozen_string_literal: true
require 'spec_helper'
describe Branches::ValidateNewService do
let(:project) { create(:project, :repository) }
subject(:service) { described_class.new(project) }
describe '#execute' do
context 'validation' do
it 'returns error with an invalid branch name' do
result = service.execute('refs/heads/invalid_branch')
expect(result[:status]).to eq(:error)
expect(result[:message]).to eq('Branch name is invalid')
end
it 'returns success with a valid branch name' do
result = service.execute('valid_branch_name')
expect(result[:status]).to eq(:success)
end
end
context 'branch exist' do
it 'returns error when branch exists' do
result = service.execute('master')
expect(result[:status]).to eq(:error)
expect(result[:message]).to eq('Branch already exists')
end
it 'returns success when branch name is available' do
result = service.execute('valid_branch_name')
expect(result[:status]).to eq(:success)
end
end
end
end
......@@ -211,7 +211,8 @@ describe MergeRequests::MergeService do
end
it 'does not delete the source branch' do
expect(DeleteBranchService).not_to receive(:new)
expect(::Branches::DeleteService).not_to receive(:new)
service.execute(merge_request)
end
end
......@@ -226,7 +227,7 @@ describe MergeRequests::MergeService do
end
it 'does not delete the source branch' do
expect(DeleteBranchService).not_to receive(:new)
expect(::Branches::DeleteService).not_to receive(:new)
service.execute(merge_request)
end
end
......@@ -238,7 +239,7 @@ describe MergeRequests::MergeService do
end
it 'removes the source branch using the author user' do
expect(DeleteBranchService).to receive(:new)
expect(::Branches::DeleteService).to receive(:new)
.with(merge_request.source_project, merge_request.author)
.and_call_original
service.execute(merge_request)
......@@ -248,7 +249,7 @@ describe MergeRequests::MergeService do
let(:service) { described_class.new(project, user, merge_params.merge('should_remove_source_branch' => false)) }
it 'does not delete the source branch' do
expect(DeleteBranchService).not_to receive(:new)
expect(::Branches::DeleteService).not_to receive(:new)
service.execute(merge_request)
end
end
......@@ -260,7 +261,7 @@ describe MergeRequests::MergeService do
end
it 'removes the source branch using the current user' do
expect(DeleteBranchService).to receive(:new)
expect(::Branches::DeleteService).to receive(:new)
.with(merge_request.source_project, user)
.and_call_original
service.execute(merge_request)
......
......@@ -61,7 +61,7 @@ describe MergeRequests::MergeToRefService do
end
it 'does not delete the source branch' do
expect(DeleteBranchService).not_to receive(:new)
expect(::Branches::DeleteService).not_to receive(:new)
process_merge_to_ref
end
......
......@@ -113,7 +113,7 @@ describe MergeRequests::RefreshService do
context 'when source branch ref does not exists' do
before do
DeleteBranchService.new(@project, @user).execute(@merge_request.source_branch)
::Branches::DeleteService.new(@project, @user).execute(@merge_request.source_branch)
end
it 'closes MRs without source branch ref' do
......
......@@ -50,7 +50,7 @@ module PositionTracerHelpers
end
def create_branch(new_name, branch_name)
CreateBranchService.new(project, current_user).execute(new_name, branch_name)
::Branches::CreateService.new(project, current_user).execute(new_name, branch_name)
end
def create_file(branch_name, file_name, content)
......
......@@ -8,11 +8,14 @@ RSpec.shared_examples 'a creatable merge request' do
page.within '.dropdown-menu-user' do
click_link user2.name
end
expect(find('input[name="merge_request[assignee_ids][]"]', visible: false).value).to match(user2.id.to_s)
page.within '.js-assignee-search' do
expect(page).to have_content user2.name
end
click_link 'Assign to me'
expect(find('input[name="merge_request[assignee_ids][]"]', visible: false).value).to match(user.id.to_s)
page.within '.js-assignee-search' do
expect(page).to have_content user.name
......@@ -22,6 +25,7 @@ RSpec.shared_examples 'a creatable merge request' do
page.within '.issue-milestone' do
click_link milestone.title
end
expect(find('input[name="merge_request[milestone_id]"]', visible: false).value).to match(milestone.id.to_s)
page.within '.js-milestone-select' do
expect(page).to have_content milestone.title
......@@ -32,6 +36,7 @@ RSpec.shared_examples 'a creatable merge request' do
click_link label.title
click_link label2.title
end
page.within '.js-label-select' do
expect(page).to have_content label.title
end
......@@ -58,8 +63,9 @@ RSpec.shared_examples 'a creatable merge request' do
it 'updates the branches when selecting a new target project', :js do
target_project_member = target_project.owner
CreateBranchService.new(target_project, target_project_member)
::Branches::CreateService.new(target_project, target_project_member)
.execute('a-brand-new-branch-to-test', 'master')
visit project_new_merge_request_path(source_project)
first('.js-target-project').click
......
......@@ -8,8 +8,8 @@ describe DeleteMergedBranchesWorker do
let(:project) { create(:project, :repository) }
describe "#perform" do
it "calls DeleteMergedBranchesService" do
expect_any_instance_of(DeleteMergedBranchesService).to receive(:execute).and_return(true)
it "delegates to Branches::DeleteMergedService" do
expect_any_instance_of(::Branches::DeleteMergedService).to receive(:execute).and_return(true)
worker.perform(project.id, project.owner.id)
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment