diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb
index b3331b99a6b4720961412a403a7b64aa24ccb587..ac42b3febc670627d23b0178e70a03157a3e0065 100644
--- a/app/models/protected_branch.rb
+++ b/app/models/protected_branch.rb
@@ -37,36 +37,11 @@ def self.protected?(project, ref_name)
return true if project.empty_repo? && project.default_branch_protected?
return false if ref_name.blank?
- dry_run = Feature.disabled?(:rely_on_protected_branches_cache, project)
-
- new_cache_result = new_cache(project, ref_name, dry_run: dry_run)
-
- return new_cache_result unless new_cache_result.nil?
-
- deprecated_cache(project, ref_name)
- end
-
- def self.new_cache(project, ref_name, dry_run: true)
- ProtectedBranches::CacheService.new(project).fetch(ref_name, dry_run: dry_run) do # rubocop: disable CodeReuse/ServiceClass
- self.matching(ref_name, protected_refs: protected_refs(project)).present?
- end
- end
-
- # Deprecated: https://gitlab.com/gitlab-org/gitlab/-/issues/370608
- # ----------------------------------------------------------------
- CACHE_EXPIRE_IN = 1.hour
-
- def self.deprecated_cache(project, ref_name)
- Rails.cache.fetch(protected_ref_cache_key(project, ref_name), expires_in: CACHE_EXPIRE_IN) do
+ ProtectedBranches::CacheService.new(project).fetch(ref_name) do # rubocop: disable CodeReuse/ServiceClass
self.matching(ref_name, protected_refs: protected_refs(project)).present?
end
end
- def self.protected_ref_cache_key(project, ref_name)
- "protected_ref-#{project.cache_key}-#{Digest::SHA1.hexdigest(ref_name)}"
- end
- # End of deprecation --------------------------------------------
-
def self.allow_force_push?(project, ref_name)
if Feature.enabled?(:group_protected_branches)
protected_branches = project.all_protected_branches.matching(ref_name)
diff --git a/config/feature_flags/development/rely_on_protected_branches_cache.yml b/config/feature_flags/development/rely_on_protected_branches_cache.yml
deleted file mode 100644
index 5154d4cee083fee16840806e60551102070025e1..0000000000000000000000000000000000000000
--- a/config/feature_flags/development/rely_on_protected_branches_cache.yml
+++ /dev/null
@@ -1,8 +0,0 @@
----
-name: rely_on_protected_branches_cache
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/92937
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/370608
-milestone: '15.4'
-type: development
-group: group::source code
-default_enabled: false
diff --git a/spec/models/protected_branch_spec.rb b/spec/models/protected_branch_spec.rb
index 71e22f848cced9a9dc99efa9435c251304d0e83d..c99c92e6c190622b18b3b49dffd29aa2ad9cedd3 100644
--- a/spec/models/protected_branch_spec.rb
+++ b/spec/models/protected_branch_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe ProtectedBranch do
+RSpec.describe ProtectedBranch, feature_category: :source_code_management do
subject { build_stubbed(:protected_branch) }
describe 'Associations' do
@@ -214,85 +214,52 @@
let_it_be(:project) { create(:project, :repository) }
let_it_be(:protected_branch) { create(:protected_branch, project: project, name: "“jawnâ€") }
- let(:rely_on_new_cache) { true }
-
- shared_examples_for 'hash based cache implementation' do
- it 'calls only hash based cache implementation' do
- expect_next_instance_of(ProtectedBranches::CacheService) do |instance|
- expect(instance).to receive(:fetch).with('missing-branch', anything).and_call_original
- end
-
- expect(Rails.cache).not_to receive(:fetch)
-
- described_class.protected?(project, 'missing-branch')
- end
- end
-
before do
- stub_feature_flags(rely_on_protected_branches_cache: rely_on_new_cache)
allow(described_class).to receive(:matching).and_call_original
# the original call works and warms the cache
described_class.protected?(project, protected_branch.name)
end
- context 'Dry-run: true (rely_on_protected_branches_cache is off, new hash-based is used)' do
- let(:rely_on_new_cache) { false }
+ it 'correctly invalidates a cache' do
+ expect(described_class).to receive(:matching).with(protected_branch.name, protected_refs: anything).exactly(3).times.and_call_original
- it 'recalculates a fresh value every time in order to check the cache is not returning stale data' do
- expect(described_class).to receive(:matching).with(protected_branch.name, protected_refs: anything).twice
+ create_params = { name: 'bar', merge_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }] }
+ branch = ProtectedBranches::CreateService.new(project, project.owner, create_params).execute
+ expect(described_class.protected?(project, protected_branch.name)).to eq(true)
- 2.times { described_class.protected?(project, protected_branch.name) }
- end
+ ProtectedBranches::UpdateService.new(project, project.owner, name: 'ber').execute(branch)
+ expect(described_class.protected?(project, protected_branch.name)).to eq(true)
- it_behaves_like 'hash based cache implementation'
+ ProtectedBranches::DestroyService.new(project, project.owner).execute(branch)
+ expect(described_class.protected?(project, protected_branch.name)).to eq(true)
end
- context 'Dry-run: false (rely_on_protected_branches_cache is enabled, new hash-based cache is used)' do
- let(:rely_on_new_cache) { true }
-
- it 'correctly invalidates a cache' do
- expect(described_class).to receive(:matching).with(protected_branch.name, protected_refs: anything).exactly(3).times.and_call_original
-
- create_params = { name: 'bar', merge_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }] }
- branch = ProtectedBranches::CreateService.new(project, project.owner, create_params).execute
- expect(described_class.protected?(project, protected_branch.name)).to eq(true)
+ context 'when project is updated' do
+ it 'does not invalidate a cache' do
+ expect(described_class).not_to receive(:matching).with(protected_branch.name, protected_refs: anything)
- ProtectedBranches::UpdateService.new(project, project.owner, name: 'ber').execute(branch)
- expect(described_class.protected?(project, protected_branch.name)).to eq(true)
+ project.touch
- ProtectedBranches::DestroyService.new(project, project.owner).execute(branch)
- expect(described_class.protected?(project, protected_branch.name)).to eq(true)
- end
-
- it_behaves_like 'hash based cache implementation'
-
- context 'when project is updated' do
- it 'does not invalidate a cache' do
- expect(described_class).not_to receive(:matching).with(protected_branch.name, protected_refs: anything)
-
- project.touch
-
- described_class.protected?(project, protected_branch.name)
- end
+ described_class.protected?(project, protected_branch.name)
end
+ end
- context 'when other project protected branch is updated' do
- it 'does not invalidate the current project cache' do
- expect(described_class).not_to receive(:matching).with(protected_branch.name, protected_refs: anything)
+ context 'when other project protected branch is updated' do
+ it 'does not invalidate the current project cache' do
+ expect(described_class).not_to receive(:matching).with(protected_branch.name, protected_refs: anything)
- another_project = create(:project)
- ProtectedBranches::CreateService.new(another_project, another_project.owner, name: 'bar').execute
+ another_project = create(:project)
+ ProtectedBranches::CreateService.new(another_project, another_project.owner, name: 'bar').execute
- described_class.protected?(project, protected_branch.name)
- end
+ described_class.protected?(project, protected_branch.name)
end
+ end
- it 'correctly uses the cached version' do
- expect(described_class).not_to receive(:matching)
+ it 'correctly uses the cached version' do
+ expect(described_class).not_to receive(:matching)
- expect(described_class.protected?(project, protected_branch.name)).to eq(true)
- end
+ expect(described_class.protected?(project, protected_branch.name)).to eq(true)
end
end
end