Merge branch '387978_remove_cache_key_duplication' into 'master'

Avoid duplicated keys in ProtectedBranches cache

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/148172



Merged-by: Vijay Hawoldar <vhawoldar@gitlab.com>
Approved-by: Panos Kanellidis <pkanellidis@gitlab.com>
Approved-by: Vijay Hawoldar <vhawoldar@gitlab.com>
Reviewed-by: Vasilii Iakliushin <viakliushin@gitlab.com>
Reviewed-by: Panos Kanellidis <pkanellidis@gitlab.com>
Co-authored-by: Vasilii Iakliushin <viakliushin@gitlab.com>

app/models/protected_branch/cache_key.rb

+# frozen_string_literal: true
+
+class ProtectedBranch::CacheKey # rubocop:disable Style/ClassAndModuleChildren -- Same problem as in push_access_level.rb
+  include Gitlab::Utils::StrongMemoize
+
+  CACHE_ROOT_KEY = 'cache:gitlab:protected_branch'
+
+  def initialize(entity)
+    @entity = entity
+  end
+
+  def to_s
+    need_to_scope? ? scoped_key : unscoped_key
+  end
+
+  private
+
+  attr_reader :entity
+
+  def need_to_scope?
+    Feature.enabled?(:group_protected_branches, group) ||
+      Feature.enabled?(:allow_protected_branches_for_group, group)
+  end
+
+  def scoped_key
+    [CACHE_ROOT_KEY, entity_scope, entity.id].join(':')
+  end
+
+  def unscoped_key
+    [CACHE_ROOT_KEY, entity.id].join(':')
+  end
+
+  def group
+    return entity if entity.is_a?(Group)
+    return entity.group if entity.is_a?(Project)
+
+    nil
+  end
+  strong_memoize_attr :group
+
+  def entity_scope
+    case entity
+    when Group
+      'group'
+    when Project
+      'project'
+    else
+      entity.class.name.downcase
+    end
+  end
+end

app/services/protected_branches/cache_service.rb

@@ -4,7 +4,6 @@ module ProtectedBranches
   class CacheService < ProtectedBranches::BaseService
     include Gitlab::Utils::StrongMemoize
 
-    CACHE_ROOT_KEY = 'cache:gitlab:protected_branch'
     TTL_UNSET = -1
     CACHE_EXPIRE_IN = 1.day
     CACHE_LIMIT = 1000
@@ -84,25 +83,10 @@ def check_and_log_discrepancy(cached_value, real_value, ref_name)
 
     def redis_key(entity = project_or_group)
       strong_memoize_with(:redis_key, entity) do
-        scope_redis_key?(entity) ? scoped_redis_key(entity) : unscoped_redis_key(entity)
+        ProtectedBranch::CacheKey.new(entity).to_s
       end
     end
 
-    def scope_redis_key?(entity)
-      group = entity.is_a?(Group) ? entity : entity.group
-
-      Feature.enabled?(:group_protected_branches, group) ||
-        Feature.enabled?(:allow_protected_branches_for_group, group)
-    end
-
-    def scoped_redis_key(entity)
-      [CACHE_ROOT_KEY, entity.class.name, entity.id].join(':')
-    end
-
-    def unscoped_redis_key(entity)
-      [CACHE_ROOT_KEY, entity.id].join(':')
-    end
-
     def metrics
       @metrics ||= Gitlab::Cache::Metrics.new(cache_metadata)
     end

spec/models/protected_branch/cache_key_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ProtectedBranch::CacheKey, feature_category: :source_code_management do
+  subject(:cache_key) { described_class.new(entity) }
+
+  let_it_be(:project) { create(:project) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:user) { create(:user) }
+
+  describe '#to_s' do
+    subject { cache_key.to_s }
+
+    shared_examples 'group feature flags are disabled' do
+      context 'when feature flags are disabled' do
+        before do
+          stub_feature_flags(group_protected_branches: false)
+          stub_feature_flags(allow_protected_branches_for_group: false)
+        end
+
+        it 'returns an unscoped key' do
+          is_expected.to eq "cache:gitlab:protected_branch:#{entity.id}"
+        end
+      end
+    end
+
+    context 'with entity project' do
+      let(:entity) { project }
+
+      it 'returns a scoped key' do
+        is_expected.to eq "cache:gitlab:protected_branch:project:#{project.id}"
+      end
+
+      context 'when a project presenter is provided' do
+        let(:entity) { ProjectPresenter.new(project) }
+
+        it 'returns the same key as a project' do
+          is_expected.to eq "cache:gitlab:protected_branch:project:#{project.id}"
+        end
+      end
+
+      it_behaves_like 'group feature flags are disabled'
+    end
+
+    context 'with entity group' do
+      let(:entity) { group }
+
+      it 'returns a scoped key' do
+        is_expected.to eq "cache:gitlab:protected_branch:group:#{group.id}"
+      end
+
+      it_behaves_like 'group feature flags are disabled'
+    end
+
+    context 'with an unsupported entity' do
+      let(:entity) { user }
+
+      it 'returns a scoped key' do
+        is_expected.to eq "cache:gitlab:protected_branch:user:#{user.id}"
+      end
+
+      it_behaves_like 'group feature flags are disabled'
+    end
+  end
+end