Commit 52eeb56b authored by Yorick Peterse's avatar Yorick Peterse

Refactor code for protecting default branches

This refactors some of the logic used for protecting default branches,
in particular Project#after_create_default_branch. The logic for this
method is moved into a separate service class. Ideally we'd get rid of
Project#after_create_default_branch entirely, but unfortunately
Project#after_import depends on it. This means it has to stick around
until we also refactor Project#after_import.

For branch protection levels we introduce
Gitlab::Access::BranchProtection, which provides a small wrapper around
Integer based branch protection levels. Using this class removes the
need for having to constantly refer to Gitlab::Access::PROTECTION_*
constants.
parent 3e9c9f97
......@@ -1602,24 +1602,7 @@ class Project < ActiveRecord::Base
# rubocop: disable CodeReuse/ServiceClass
def after_create_default_branch
return unless default_branch
# Ensure HEAD points to the default branch in case it is not master
change_head(default_branch)
if Gitlab::CurrentSettings.default_branch_protection != Gitlab::Access::PROTECTION_NONE && !ProtectedBranch.protected?(self, default_branch)
params = {
name: default_branch,
push_access_levels_attributes: [{
access_level: Gitlab::CurrentSettings.default_branch_protection == Gitlab::Access::PROTECTION_DEV_CAN_PUSH ? Gitlab::Access::DEVELOPER : Gitlab::Access::MAINTAINER
}],
merge_access_levels_attributes: [{
access_level: Gitlab::CurrentSettings.default_branch_protection == Gitlab::Access::PROTECTION_DEV_CAN_MERGE ? Gitlab::Access::DEVELOPER : Gitlab::Access::MAINTAINER
}]
}
ProtectedBranches::CreateService.new(self, creator, params).execute(skip_authorization: true)
end
Projects::ProtectDefaultBranchService.new(self).execute
end
# rubocop: enable CodeReuse/ServiceClass
......
# frozen_string_literal: true
module Projects
# Service class that can be used to execute actions necessary after creating a
# default branch.
class ProtectDefaultBranchService
attr_reader :project, :default_branch_protection
# @param [Project] project
def initialize(project)
@project = project
@default_branch_protection = Gitlab::Access::BranchProtection
.new(Gitlab::CurrentSettings.default_branch_protection)
end
def execute
protect_default_branch if default_branch
end
def protect_default_branch
# Ensure HEAD points to the default branch in case it is not master
project.change_head(default_branch)
create_protected_branch if protect_branch?
end
def create_protected_branch
params = {
name: default_branch,
push_access_levels_attributes: [{ access_level: push_access_level }],
merge_access_levels_attributes: [{ access_level: merge_access_level }]
}
# The creator of the project is always allowed to create protected
# branches, so we skip the authorization check in this service class.
ProtectedBranches::CreateService
.new(project, project.creator, params)
.execute(skip_authorization: true)
end
def protect_branch?
default_branch_protection.any? &&
!ProtectedBranch.protected?(project, default_branch)
end
def default_branch
project.default_branch
end
def push_access_level
if default_branch_protection.developer_can_push?
Gitlab::Access::DEVELOPER
else
Gitlab::Access::MAINTAINER
end
end
def merge_access_level
if default_branch_protection.developer_can_merge?
Gitlab::Access::DEVELOPER
else
Gitlab::Access::MAINTAINER
end
end
end
end
# frozen_string_literal: true
module Gitlab
module Access
# A wrapper around Integer based branch protection levels.
#
# This wrapper can be used to work with branch protection levels without
# having to directly refer to the constants. For example, instead of this:
#
# if access_level == Gitlab::Access::PROTECTION_DEV_CAN_PUSH
# ...
# end
#
# You can write this instead:
#
# protection = BranchProtection.new(access_level)
#
# if protection.developer_can_push?
# ...
# end
class BranchProtection
attr_reader :level
# @param [Integer] level The branch protection level as an Integer.
def initialize(level)
@level = level
end
def any?
level != PROTECTION_NONE
end
def developer_can_push?
level == PROTECTION_DEV_CAN_PUSH
end
def developer_can_merge?
level == PROTECTION_DEV_CAN_MERGE
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::Access::BranchProtection do
describe '#any?' do
using RSpec::Parameterized::TableSyntax
where(:level, :result) do
Gitlab::Access::PROTECTION_NONE | false
Gitlab::Access::PROTECTION_DEV_CAN_PUSH | true
Gitlab::Access::PROTECTION_DEV_CAN_MERGE | true
Gitlab::Access::PROTECTION_FULL | true
end
with_them do
it { expect(described_class.new(level).any?).to eq(result) }
end
end
describe '#developer_can_push?' do
using RSpec::Parameterized::TableSyntax
where(:level, :result) do
Gitlab::Access::PROTECTION_NONE | false
Gitlab::Access::PROTECTION_DEV_CAN_PUSH | true
Gitlab::Access::PROTECTION_DEV_CAN_MERGE | false
Gitlab::Access::PROTECTION_FULL | false
end
with_them do
it do
expect(described_class.new(level).developer_can_push?).to eq(result)
end
end
end
describe '#developer_can_merge?' do
using RSpec::Parameterized::TableSyntax
where(:level, :result) do
Gitlab::Access::PROTECTION_NONE | false
Gitlab::Access::PROTECTION_DEV_CAN_PUSH | false
Gitlab::Access::PROTECTION_DEV_CAN_MERGE | true
Gitlab::Access::PROTECTION_FULL | false
end
with_them do
it do
expect(described_class.new(level).developer_can_merge?).to eq(result)
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Projects::ProtectDefaultBranchService do
let(:service) { described_class.new(project) }
let(:project) { instance_spy(Project) }
describe '#execute' do
before do
allow(service)
.to receive(:protect_default_branch)
end
context 'without a default branch' do
it 'does nothing' do
allow(service)
.to receive(:default_branch)
.and_return(nil)
service.execute
expect(service)
.not_to have_received(:protect_default_branch)
end
end
context 'with a default branch' do
it 'protects the default branch' do
allow(service)
.to receive(:default_branch)
.and_return('master')
service.execute
expect(service)
.to have_received(:protect_default_branch)
end
end
end
describe '#protect_default_branch' do
before do
allow(service)
.to receive(:default_branch)
.and_return('master')
allow(project)
.to receive(:change_head)
.with('master')
allow(service)
.to receive(:create_protected_branch)
end
context 'when branch protection is needed' do
before do
allow(service)
.to receive(:protect_branch?)
.and_return(true)
allow(service)
.to receive(:create_protected_branch)
end
it 'changes the HEAD of the project' do
service.protect_default_branch
expect(project)
.to have_received(:change_head)
end
it 'protects the default branch' do
service.protect_default_branch
expect(service)
.to have_received(:create_protected_branch)
end
end
context 'when branch protection is not needed' do
before do
allow(service)
.to receive(:protect_branch?)
.and_return(false)
end
it 'changes the HEAD of the project' do
service.protect_default_branch
expect(project)
.to have_received(:change_head)
end
it 'does not protect the default branch' do
service.protect_default_branch
expect(service)
.not_to have_received(:create_protected_branch)
end
end
end
describe '#create_protected_branch' do
it 'creates the protected branch' do
creator = instance_spy(User)
create_service = instance_spy(ProtectedBranches::CreateService)
access_level = Gitlab::Access::DEVELOPER
params = {
name: 'master',
push_access_levels_attributes: [{ access_level: access_level }],
merge_access_levels_attributes: [{ access_level: access_level }]
}
allow(project)
.to receive(:creator)
.and_return(creator)
allow(ProtectedBranches::CreateService)
.to receive(:new)
.with(project, creator, params)
.and_return(create_service)
allow(service)
.to receive(:push_access_level)
.and_return(access_level)
allow(service)
.to receive(:merge_access_level)
.and_return(access_level)
allow(service)
.to receive(:default_branch)
.and_return('master')
allow(create_service)
.to receive(:execute)
.with(skip_authorization: true)
service.create_protected_branch
expect(create_service)
.to have_received(:execute)
end
end
describe '#protect_branch?' do
context 'when default branch protection is disabled' do
it 'returns false' do
allow(Gitlab::CurrentSettings)
.to receive(:default_branch_protection)
.and_return(Gitlab::Access::PROTECTION_NONE)
expect(service.protect_branch?).to eq(false)
end
end
context 'when default branch protection is enabled' do
before do
allow(Gitlab::CurrentSettings)
.to receive(:default_branch_protection)
.and_return(Gitlab::Access::PROTECTION_DEV_CAN_MERGE)
allow(service)
.to receive(:default_branch)
.and_return('master')
end
it 'returns false if the branch is already protected' do
allow(ProtectedBranch)
.to receive(:protected?)
.with(project, 'master')
.and_return(true)
expect(service.protect_branch?).to eq(false)
end
it 'returns true if the branch is not yet protected' do
allow(ProtectedBranch)
.to receive(:protected?)
.with(project, 'master')
.and_return(false)
expect(service.protect_branch?).to eq(true)
end
end
end
describe '#default_branch' do
it 'returns the default branch of the project' do
allow(project)
.to receive(:default_branch)
.and_return('master')
expect(service.default_branch).to eq('master')
end
end
describe '#push_access_level' do
context 'when developers can push' do
it 'returns the DEVELOPER access level' do
allow(Gitlab::CurrentSettings)
.to receive(:default_branch_protection)
.and_return(Gitlab::Access::PROTECTION_DEV_CAN_PUSH)
expect(service.push_access_level).to eq(Gitlab::Access::DEVELOPER)
end
end
context 'when developers can not push' do
it 'returns the MAINTAINER access level' do
allow(Gitlab::CurrentSettings)
.to receive(:default_branch_protection)
.and_return(Gitlab::Access::PROTECTION_DEV_CAN_MERGE)
expect(service.push_access_level).to eq(Gitlab::Access::MAINTAINER)
end
end
end
describe '#merge_access_level' do
context 'when developers can merge' do
it 'returns the DEVELOPER access level' do
allow(Gitlab::CurrentSettings)
.to receive(:default_branch_protection)
.and_return(Gitlab::Access::PROTECTION_DEV_CAN_MERGE)
expect(service.merge_access_level).to eq(Gitlab::Access::DEVELOPER)
end
end
context 'when developers can not merge' do
it 'returns the MAINTAINER access level' do
allow(Gitlab::CurrentSettings)
.to receive(:default_branch_protection)
.and_return(Gitlab::Access::PROTECTION_DEV_CAN_PUSH)
expect(service.merge_access_level).to eq(Gitlab::Access::MAINTAINER)
end
end
end
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