diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-08-18 11:17:02 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-08-18 11:17:02 +0300 |
commit | b39512ed755239198a9c294b6a45e65c05900235 (patch) | |
tree | d234a3efade1de67c46b9e5a38ce813627726aa7 /spec/services/protected_branches | |
parent | d31474cf3b17ece37939d20082b07f6657cc79a9 (diff) |
Add latest changes from gitlab-org/gitlab@15-3-stable-eev15.3.0-rc42
Diffstat (limited to 'spec/services/protected_branches')
4 files changed, 172 insertions, 30 deletions
diff --git a/spec/services/protected_branches/cache_service_spec.rb b/spec/services/protected_branches/cache_service_spec.rb new file mode 100644 index 00000000000..4fa7553c23d --- /dev/null +++ b/spec/services/protected_branches/cache_service_spec.rb @@ -0,0 +1,113 @@ +# frozen_string_literal: true +# rubocop:disable Style/RedundantFetchBlock +# +require 'spec_helper' + +RSpec.describe ProtectedBranches::CacheService, :clean_gitlab_redis_cache do + subject(:service) { described_class.new(project, user) } + + let_it_be(:project) { create(:project) } + let_it_be(:user) { project.first_owner } + + 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) + + stub_const("#{described_class.name}::CACHE_EXPIRE_IN", immediate_expiration) + + expect(service.fetch('not-found') { false }).to eq(false) + + expect(service.fetch('main') { false }).to eq(true) + expect(service.fetch('not-found') { true }).to eq(false) + end + + context 'when CACHE_LIMIT is exceeded' do + before do + stub_const("#{described_class.name}::CACHE_LIMIT", 2) + 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) + + # 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 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/, + 'project_id' => project.id, + 'project_path' => project.full_path + ) + + expect(service.fetch('main', dry_run: true) { false }).to eq(false) + 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) + + expect(Gitlab::AppLogger).not_to receive(:error) + + expect(service.fetch('main', dry_run: true) { true }).to eq(true) + end + 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) + + service.refresh + + # Recreates cache + expect(service.fetch('main') { false }).to eq(false) + expect(service.fetch('not-found') { true }).to eq(true) + end + end +end +# rubocop:enable Style/RedundantFetchBlock diff --git a/spec/services/protected_branches/create_service_spec.rb b/spec/services/protected_branches/create_service_spec.rb index 3ac42d41377..b42524e761c 100644 --- a/spec/services/protected_branches/create_service_spec.rb +++ b/spec/services/protected_branches/create_service_spec.rb @@ -3,7 +3,8 @@ require 'spec_helper' RSpec.describe ProtectedBranches::CreateService do - let(:project) { create(:project) } + let_it_be_with_reload(:project) { create(:project) } + let(:user) { project.first_owner } let(:params) do { @@ -13,22 +14,28 @@ RSpec.describe ProtectedBranches::CreateService do } end + subject(:service) { described_class.new(project, user, params) } + describe '#execute' do let(:name) { 'master' } - subject(:service) { described_class.new(project, user, params) } - 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]) end + it 'refreshes the cache' do + expect_next_instance_of(ProtectedBranches::CacheService) do |cache_service| + expect(cache_service).to receive(:refresh) + end + + service.execute + end + context 'when protecting a branch with a name that contains HTML tags' do let(:name) { 'foo<b>bar<\b>' } - subject(:service) { described_class.new(project, user, params) } - 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) @@ -52,16 +59,18 @@ RSpec.describe ProtectedBranches::CreateService do end context 'when a policy restricts rule creation' do - before do - policy = instance_double(ProtectedBranchPolicy, allowed?: false) - expect(ProtectedBranchPolicy).to receive(:new).and_return(policy) - end - 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 end end + + def disallow(ability, protected_branch) + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(user, ability, protected_branch).and_return(false) + end end diff --git a/spec/services/protected_branches/destroy_service_spec.rb b/spec/services/protected_branches/destroy_service_spec.rb index 4e55c72f312..9fa07820148 100644 --- a/spec/services/protected_branches/destroy_service_spec.rb +++ b/spec/services/protected_branches/destroy_service_spec.rb @@ -3,30 +3,41 @@ require 'spec_helper' RSpec.describe ProtectedBranches::DestroyService do - let(:protected_branch) { create(:protected_branch) } - let(:project) { protected_branch.project } + let_it_be_with_reload(:project) { create(:project) } + + let(:protected_branch) { create(:protected_branch, project: project) } let(:user) { project.first_owner } - describe '#execute' do - subject(:service) { described_class.new(project, user) } + 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 - context 'when a policy restricts rule deletion' do - before do - policy = instance_double(ProtectedBranchPolicy, allowed?: false) - expect(ProtectedBranchPolicy).to receive(:new).and_return(policy) + it 'refreshes the cache' do + expect_next_instance_of(ProtectedBranches::CacheService) do |cache_service| + expect(cache_service).to receive(:refresh) end + service.execute(protected_branch) + end + + 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 + + def disallow(ability, protected_branch) + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(user, ability, protected_branch).and_return(false) + end end diff --git a/spec/services/protected_branches/update_service_spec.rb b/spec/services/protected_branches/update_service_spec.rb index 4405af35c37..c4fe4d78070 100644 --- a/spec/services/protected_branches/update_service_spec.rb +++ b/spec/services/protected_branches/update_service_spec.rb @@ -3,27 +3,34 @@ require 'spec_helper' RSpec.describe ProtectedBranches::UpdateService do - let(:protected_branch) { create(:protected_branch) } - let(:project) { protected_branch.project } + let_it_be_with_reload(:project) { create(:project) } + + let(:protected_branch) { create(:protected_branch, project: project) } let(:user) { project.first_owner } let(:params) { { name: new_name } } + subject(:service) { described_class.new(project, user, params) } + describe '#execute' do let(:new_name) { 'new protected branch name' } let(:result) { service.execute(protected_branch) } - subject(:service) { described_class.new(project, user, params) } - 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 + + 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) } - subject(:service) { described_class.new(project, user, params) } - it 'updates a protected branch' do expect(result.reload.name).to eq(new_name) end @@ -37,15 +44,17 @@ RSpec.describe ProtectedBranches::UpdateService do end end - context 'when a policy restricts rule creation' do - before do - policy = instance_double(ProtectedBranchPolicy, allowed?: false) - expect(ProtectedBranchPolicy).to receive(:new).and_return(policy) - end + context 'when a policy restricts rule update' do + it "prevents update of the protected branch rule" do + disallow(:update_protected_branch, protected_branch) - it "prevents creation of the protected branch rule" do expect { service.execute(protected_branch) }.to raise_error(Gitlab::Access::AccessDeniedError) end end end + + def disallow(ability, protected_branch) + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(user, ability, protected_branch).and_return(false) + end end |