diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-12-20 17:22:11 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-12-20 17:22:11 +0300 |
commit | 0c872e02b2c822e3397515ec324051ff540f0cd5 (patch) | |
tree | ce2fb6ce7030e4dad0f4118d21ab6453e5938cdd /spec/services/protected_branches | |
parent | f7e05a6853b12f02911494c4b3fe53d9540d74fc (diff) |
Add latest changes from gitlab-org/gitlab@15-7-stable-eev15.7.0-rc42
Diffstat (limited to 'spec/services/protected_branches')
5 files changed, 284 insertions, 201 deletions
diff --git a/spec/services/protected_branches/api_service_spec.rb b/spec/services/protected_branches/api_service_spec.rb index 94484f5a7b9..c98e253267b 100644 --- a/spec/services/protected_branches/api_service_spec.rb +++ b/spec/services/protected_branches/api_service_spec.rb @@ -3,32 +3,55 @@ require 'spec_helper' RSpec.describe ProtectedBranches::ApiService do - let_it_be(:project) { create(:project) } - let_it_be(:user) { create(:user, maintainer_projects: [project]) } - - it 'creates a protected branch with prefilled defaults' do - expect(::ProtectedBranches::CreateService).to receive(:new).with( - project, user, hash_including( - push_access_levels_attributes: [{ access_level: Gitlab::Access::MAINTAINER }], - merge_access_levels_attributes: [{ access_level: Gitlab::Access::MAINTAINER }] - ) - ).and_call_original - - expect(described_class.new(project, user, { name: 'new name' }).create).to be_valid + shared_examples 'execute with entity' do + it 'creates a protected branch with prefilled defaults' do + expect(::ProtectedBranches::CreateService).to receive(:new).with( + entity, user, hash_including( + push_access_levels_attributes: [{ access_level: Gitlab::Access::MAINTAINER }], + merge_access_levels_attributes: [{ access_level: Gitlab::Access::MAINTAINER }] + ) + ).and_call_original + + expect(described_class.new(entity, user, { name: 'new name' }).create).to be_valid + end + + it 'updates a protected branch without prefilled defaults' do + expect(::ProtectedBranches::UpdateService).to receive(:new).with( + entity, user, hash_including( + push_access_levels_attributes: [], + merge_access_levels_attributes: [] + ) + ).and_call_original + + expect do + expect(described_class.new(entity, user, { name: 'new name' }).update(protected_branch)).to be_valid + end.not_to change { protected_branch.reload.allow_force_push } + end + end + + context 'with entity project' do + let_it_be_with_reload(:entity) { create(:project) } + let_it_be_with_reload(:protected_branch) { create(:protected_branch, project: entity, allow_force_push: true) } + let(:user) { entity.first_owner } + + it_behaves_like 'execute with entity' end - it 'updates a protected branch without prefilled defaults' do - protected_branch = create(:protected_branch, project: project, allow_force_push: true) + context 'with entity group' do + let_it_be_with_reload(:entity) { create(:group) } + let_it_be_with_reload(:user) { create(:user) } + let_it_be_with_reload(:protected_branch) do + create(:protected_branch, group: entity, project: nil, allow_force_push: true) + end - expect(::ProtectedBranches::UpdateService).to receive(:new).with( - project, user, hash_including( - push_access_levels_attributes: [], - merge_access_levels_attributes: [] - ) - ).and_call_original + before do + allow(Ability).to receive(:allowed?).with(user, :update_protected_branch, protected_branch).and_return(true) + allow(Ability) + .to receive(:allowed?) + .with(user, :create_protected_branch, instance_of(ProtectedBranch)) + .and_return(true) + end - expect do - expect(described_class.new(project, user, { name: 'new name' }).update(protected_branch)).to be_valid - end.not_to change { protected_branch.reload.allow_force_push } + it_behaves_like 'execute with entity' end end diff --git a/spec/services/protected_branches/cache_service_spec.rb b/spec/services/protected_branches/cache_service_spec.rb index d7a3258160b..ea434922661 100644 --- a/spec/services/protected_branches/cache_service_spec.rb +++ b/spec/services/protected_branches/cache_service_spec.rb @@ -4,122 +4,150 @@ require 'spec_helper' RSpec.describe ProtectedBranches::CacheService, :clean_gitlab_redis_cache do - subject(:service) { described_class.new(project, user) } + shared_examples 'execute with entity' do + subject(:service) { described_class.new(entity, user) } - let_it_be(:project) { create(:project) } - let_it_be(:user) { project.first_owner } + let(:immediate_expiration) { 0 } - let(:immediate_expiration) { 0 } - - describe '#fetch' do - it 'caches the value' do - expect(service.fetch('main') { true }).to eq(true) - expect(service.fetch('not-found') { false }).to eq(false) - - # Uses cached values - expect(service.fetch('main') { false }).to eq(true) - expect(service.fetch('not-found') { true }).to eq(false) - end - - it 'sets expiry on the key' do - stub_const("#{described_class.name}::CACHE_EXPIRE_IN", immediate_expiration) - - expect(service.fetch('main') { true }).to eq(true) - expect(service.fetch('not-found') { false }).to eq(false) - - expect(service.fetch('main') { false }).to eq(false) - expect(service.fetch('not-found') { true }).to eq(true) - end - - it 'does not set an expiry on the key after the hash is already created' do - expect(service.fetch('main') { true }).to eq(true) + describe '#fetch' do + it 'caches the value' do + expect(service.fetch('main') { true }).to eq(true) + expect(service.fetch('not-found') { false }).to eq(false) - stub_const("#{described_class.name}::CACHE_EXPIRE_IN", immediate_expiration) + # Uses cached values + expect(service.fetch('main') { false }).to eq(true) + expect(service.fetch('not-found') { true }).to eq(false) + end - expect(service.fetch('not-found') { false }).to eq(false) + it 'sets expiry on the key' do + stub_const("#{described_class.name}::CACHE_EXPIRE_IN", immediate_expiration) - expect(service.fetch('main') { false }).to eq(true) - expect(service.fetch('not-found') { true }).to eq(false) - end + expect(service.fetch('main') { true }).to eq(true) + expect(service.fetch('not-found') { false }).to eq(false) - context 'when CACHE_LIMIT is exceeded' do - before do - stub_const("#{described_class.name}::CACHE_LIMIT", 2) + expect(service.fetch('main') { false }).to eq(false) + expect(service.fetch('not-found') { true }).to eq(true) end - it 'recreates cache' do + it 'does not set an expiry on the key after the hash is already created' do expect(service.fetch('main') { true }).to eq(true) + + stub_const("#{described_class.name}::CACHE_EXPIRE_IN", immediate_expiration) + expect(service.fetch('not-found') { false }).to eq(false) - # Uses cached values expect(service.fetch('main') { false }).to eq(true) expect(service.fetch('not-found') { true }).to eq(false) + end - # Overflow - expect(service.fetch('new-branch') { true }).to eq(true) + context 'when CACHE_LIMIT is exceeded' do + before do + stub_const("#{described_class.name}::CACHE_LIMIT", 2) + end - # Refreshes values - expect(service.fetch('main') { false }).to eq(false) - expect(service.fetch('not-found') { true }).to eq(true) - end - end + it 'recreates cache' do + expect(service.fetch('main') { true }).to eq(true) + expect(service.fetch('not-found') { false }).to eq(false) + + # Uses cached values + expect(service.fetch('main') { false }).to eq(true) + expect(service.fetch('not-found') { true }).to eq(false) - context 'when dry_run is on' do - it 'does not use cached value' do - expect(service.fetch('main', dry_run: true) { true }).to eq(true) - expect(service.fetch('main', dry_run: true) { false }).to eq(false) + # Overflow + expect(service.fetch('new-branch') { true }).to eq(true) + + # Refreshes values + expect(service.fetch('main') { false }).to eq(false) + expect(service.fetch('not-found') { true }).to eq(true) + end end - context 'when cache mismatch' do - it 'logs an error' do + context 'when dry_run is on' do + it 'does not use cached value' do expect(service.fetch('main', dry_run: true) { true }).to eq(true) + expect(service.fetch('main', dry_run: true) { false }).to eq(false) + end + + context 'when cache mismatch' do + it 'logs an error' do + expect(service.fetch('main', dry_run: true) { true }).to eq(true) + + expect(Gitlab::AppLogger).to receive(:error).with( + { + 'class' => described_class.name, + 'message' => /Cache mismatch/, + 'record_class' => entity.class.name, + 'record_id' => entity.id, + 'record_path' => entity.full_path + } + ) + + expect(service.fetch('main', dry_run: true) { false }).to eq(false) + end + end - expect(Gitlab::AppLogger).to receive(:error).with( - { - 'class' => described_class.name, - 'message' => /Cache mismatch/, - 'project_id' => project.id, - 'project_path' => project.full_path - } - ) + context 'when cache matches' do + it 'does not log an error' do + expect(service.fetch('main', dry_run: true) { true }).to eq(true) - expect(service.fetch('main', dry_run: true) { false }).to eq(false) + expect(Gitlab::AppLogger).not_to receive(:error) + + expect(service.fetch('main', dry_run: true) { true }).to eq(true) + end end end + end - context 'when cache matches' do - it 'does not log an error' do - expect(service.fetch('main', dry_run: true) { true }).to eq(true) + describe '#refresh' do + it 'clears cached values' do + expect(service.fetch('main') { true }).to eq(true) + expect(service.fetch('not-found') { false }).to eq(false) - expect(Gitlab::AppLogger).not_to receive(:error) + service.refresh - expect(service.fetch('main', dry_run: true) { true }).to eq(true) + # Recreates cache + expect(service.fetch('main') { false }).to eq(false) + expect(service.fetch('not-found') { true }).to eq(true) + end + end + + describe 'metrics' do + it 'records hit ratio metrics' do + expect_next_instance_of(Gitlab::Cache::Metrics) do |metrics| + expect(metrics).to receive(:increment_cache_miss).once + expect(metrics).to receive(:increment_cache_hit).exactly(4).times end + + 5.times { service.fetch('main') { true } } end end end - describe '#refresh' do - it 'clears cached values' do - expect(service.fetch('main') { true }).to eq(true) - expect(service.fetch('not-found') { false }).to eq(false) + context 'with entity project' do + let_it_be_with_reload(:entity) { create(:project) } + let(:user) { entity.first_owner } - service.refresh + it_behaves_like 'execute with entity' + end + + context 'with entity group' do + let_it_be_with_reload(:entity) { create(:group) } + let_it_be_with_reload(:user) { create(:user) } - # Recreates cache - expect(service.fetch('main') { false }).to eq(false) - expect(service.fetch('not-found') { true }).to eq(true) + before do + entity.add_owner(user) end - end - describe 'metrics' do - it 'records hit ratio metrics' do - expect_next_instance_of(Gitlab::Cache::Metrics) do |metrics| - expect(metrics).to receive(:increment_cache_miss).once - expect(metrics).to receive(:increment_cache_hit).exactly(4).times + context 'when feature flag enabled' do + it_behaves_like 'execute with entity' + end + + context 'when feature flag disabled' do + before do + stub_feature_flags(group_protected_branches: false) end - 5.times { service.fetch('main') { true } } + it_behaves_like 'execute with entity' end end end diff --git a/spec/services/protected_branches/create_service_spec.rb b/spec/services/protected_branches/create_service_spec.rb index b42524e761c..9c8fe769ed8 100644 --- a/spec/services/protected_branches/create_service_spec.rb +++ b/spec/services/protected_branches/create_service_spec.rb @@ -3,70 +3,75 @@ require 'spec_helper' RSpec.describe ProtectedBranches::CreateService do - let_it_be_with_reload(:project) { create(:project) } - - let(:user) { project.first_owner } - let(:params) do - { - name: name, - merge_access_levels_attributes: [{ access_level: Gitlab::Access::MAINTAINER }], - push_access_levels_attributes: [{ access_level: Gitlab::Access::MAINTAINER }] - } - end - - subject(:service) { described_class.new(project, user, params) } - - describe '#execute' do - let(:name) { 'master' } - - it 'creates a new protected branch' do - expect { service.execute }.to change(ProtectedBranch, :count).by(1) - expect(project.protected_branches.last.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) - expect(project.protected_branches.last.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) + shared_examples 'execute with entity' do + let(:params) do + { + name: name, + merge_access_levels_attributes: [{ access_level: Gitlab::Access::MAINTAINER }], + push_access_levels_attributes: [{ access_level: Gitlab::Access::MAINTAINER }] + } end - it 'refreshes the cache' do - expect_next_instance_of(ProtectedBranches::CacheService) do |cache_service| - expect(cache_service).to receive(:refresh) - end + subject(:service) { described_class.new(entity, user, params) } - service.execute - end - - context 'when protecting a branch with a name that contains HTML tags' do - let(:name) { 'foo<b>bar<\b>' } + describe '#execute' do + let(:name) { 'master' } it 'creates a new protected branch' do expect { service.execute }.to change(ProtectedBranch, :count).by(1) - expect(project.protected_branches.last.name).to eq(name) + expect(entity.protected_branches.last.push_access_levels.map(&:access_level)).to match_array([Gitlab::Access::MAINTAINER]) + expect(entity.protected_branches.last.merge_access_levels.map(&:access_level)).to match_array([Gitlab::Access::MAINTAINER]) end - end - context 'when user does not have permission' do - let(:user) { create(:user) } + it 'refreshes the cache' do + expect_next_instance_of(ProtectedBranches::CacheService) do |cache_service| + expect(cache_service).to receive(:refresh) + end - before do - project.add_developer(user) + service.execute end - it 'creates a new protected branch if we skip authorization step' do - expect { service.execute(skip_authorization: true) }.to change(ProtectedBranch, :count).by(1) + context 'when protecting a branch with a name that contains HTML tags' do + let(:name) { 'foo<b>bar<\b>' } + + it 'creates a new protected branch' do + expect { service.execute }.to change(ProtectedBranch, :count).by(1) + expect(entity.protected_branches.last.name).to eq(name) + end end - it 'raises Gitlab::Access:AccessDeniedError' do - expect { service.execute }.to raise_error(Gitlab::Access::AccessDeniedError) + context 'when a policy restricts rule creation' do + it "prevents creation of the protected branch rule" do + disallow(:create_protected_branch, an_instance_of(ProtectedBranch)) + + expect do + service.execute + end.to raise_error(Gitlab::Access::AccessDeniedError) + end + + it 'creates a new protected branch if we skip authorization step' do + expect { service.execute(skip_authorization: true) }.to change(ProtectedBranch, :count).by(1) + end end end + end - context 'when a policy restricts rule creation' do - it "prevents creation of the protected branch rule" do - disallow(:create_protected_branch, an_instance_of(ProtectedBranch)) + context 'with entity project' do + let_it_be_with_reload(:entity) { create(:project) } + let(:user) { entity.first_owner } - expect do - service.execute - end.to raise_error(Gitlab::Access::AccessDeniedError) - end + it_behaves_like 'execute with entity' + end + + context 'with entity group' do + let_it_be_with_reload(:entity) { create(:group) } + let_it_be_with_reload(:user) { create(:user) } + + before do + allow(Ability).to receive(:allowed?).with(user, :create_protected_branch, instance_of(ProtectedBranch)).and_return(true) end + + it_behaves_like 'execute with entity' end def disallow(ability, protected_branch) diff --git a/spec/services/protected_branches/destroy_service_spec.rb b/spec/services/protected_branches/destroy_service_spec.rb index 123deeea005..421d4aae5bb 100644 --- a/spec/services/protected_branches/destroy_service_spec.rb +++ b/spec/services/protected_branches/destroy_service_spec.rb @@ -3,37 +3,54 @@ require 'spec_helper' RSpec.describe ProtectedBranches::DestroyService do - let_it_be_with_reload(:project) { create(:project) } + shared_examples 'execute with entity' do + subject(:service) { described_class.new(entity, user) } - let!(:protected_branch) { create(:protected_branch, project: project) } - let(:user) { project.first_owner } + describe '#execute' do + it 'destroys a protected branch' do + service.execute(protected_branch) - subject(:service) { described_class.new(project, user) } - - describe '#execute' do - it 'destroys a protected branch' do - service.execute(protected_branch) + expect(protected_branch).to be_destroyed + end - expect(protected_branch).to be_destroyed - end + it 'refreshes the cache' do + expect_next_instance_of(ProtectedBranches::CacheService) do |cache_service| + expect(cache_service).to receive(:refresh) + end - it 'refreshes the cache' do - expect_next_instance_of(ProtectedBranches::CacheService) do |cache_service| - expect(cache_service).to receive(:refresh) + service.execute(protected_branch) end - service.execute(protected_branch) + context 'when a policy restricts rule deletion' do + it "prevents deletion of the protected branch rule" do + disallow(:destroy_protected_branch, protected_branch) + + expect do + service.execute(protected_branch) + end.to raise_error(Gitlab::Access::AccessDeniedError) + end + end end + end - context 'when a policy restricts rule deletion' do - it "prevents deletion of the protected branch rule" do - disallow(:destroy_protected_branch, protected_branch) + context 'with entity project' do + let_it_be_with_reload(:entity) { create(:project) } + let!(:protected_branch) { create(:protected_branch, project: entity) } + let(:user) { entity.first_owner } - expect do - service.execute(protected_branch) - end.to raise_error(Gitlab::Access::AccessDeniedError) - end + it_behaves_like 'execute with entity' + end + + context 'with entity group' do + let_it_be_with_reload(:entity) { create(:group) } + let_it_be_with_reload(:user) { create(:user) } + let!(:protected_branch) { create(:protected_branch, group: entity, project: nil) } + + before do + allow(Ability).to receive(:allowed?).with(user, :destroy_protected_branch, protected_branch).and_return(true) end + + it_behaves_like 'execute with entity' end def disallow(ability, protected_branch) diff --git a/spec/services/protected_branches/update_service_spec.rb b/spec/services/protected_branches/update_service_spec.rb index 2ff6c3c489a..c70cc032a6a 100644 --- a/spec/services/protected_branches/update_service_spec.rb +++ b/spec/services/protected_branches/update_service_spec.rb @@ -3,54 +3,64 @@ require 'spec_helper' RSpec.describe ProtectedBranches::UpdateService do - let_it_be_with_reload(:project) { create(:project) } + shared_examples 'execute with entity' do + let(:params) { { name: new_name } } - let!(:protected_branch) { create(:protected_branch, project: project) } - let(:user) { project.first_owner } - let(:params) { { name: new_name } } + subject(:service) { described_class.new(entity, user, params) } - subject(:service) { described_class.new(project, user, params) } + describe '#execute' do + let(:new_name) { 'new protected branch name' } + let(:result) { service.execute(protected_branch) } - describe '#execute' do - let(:new_name) { 'new protected branch name' } - let(:result) { service.execute(protected_branch) } + it 'updates a protected branch' do + expect(result.reload.name).to eq(params[:name]) + end - it 'updates a protected branch' do - expect(result.reload.name).to eq(params[:name]) - end + it 'refreshes the cache' do + expect_next_instance_of(ProtectedBranches::CacheService) do |cache_service| + expect(cache_service).to receive(:refresh) + end - it 'refreshes the cache' do - expect_next_instance_of(ProtectedBranches::CacheService) do |cache_service| - expect(cache_service).to receive(:refresh) + result end - result - end - - context 'when updating name of a protected branch to one that contains HTML tags' do - let(:new_name) { 'foo<b>bar<\b>' } - let(:result) { service.execute(protected_branch) } + context 'when updating name of a protected branch to one that contains HTML tags' do + let(:new_name) { 'foo<b>bar<\b>' } + let(:result) { service.execute(protected_branch) } - it 'updates a protected branch' do - expect(result.reload.name).to eq(new_name) + it 'updates a protected branch' do + expect(result.reload.name).to eq(new_name) + end end - end - context 'without admin_project permissions' do - let(:user) { create(:user) } + context 'when a policy restricts rule update' do + it "prevents update of the protected branch rule" do + disallow(:update_protected_branch, protected_branch) - it "raises error" do - expect { service.execute(protected_branch) }.to raise_error(Gitlab::Access::AccessDeniedError) + expect { service.execute(protected_branch) }.to raise_error(Gitlab::Access::AccessDeniedError) + end end end + end - context 'when a policy restricts rule update' do - it "prevents update of the protected branch rule" do - disallow(:update_protected_branch, protected_branch) + context 'with entity project' do + let_it_be_with_reload(:entity) { create(:project) } + let!(:protected_branch) { create(:protected_branch, project: entity) } + let(:user) { entity.first_owner } - expect { service.execute(protected_branch) }.to raise_error(Gitlab::Access::AccessDeniedError) - end + it_behaves_like 'execute with entity' + end + + context 'with entity group' do + let_it_be_with_reload(:entity) { create(:group) } + let_it_be_with_reload(:user) { create(:user) } + let!(:protected_branch) { create(:protected_branch, group: entity, project: nil) } + + before do + allow(Ability).to receive(:allowed?).with(user, :update_protected_branch, protected_branch).and_return(true) end + + it_behaves_like 'execute with entity' end def disallow(ability, protected_branch) |