Merge branch '3062-improve-project-cache' into 'master'

Change project cache key to depend on ID instead of full path

Closes #42191

See merge request gitlab-org/gitlab-ce!23135

5 files changed, 42 insertions, 16 deletions

diff --git a/app/models/repository.rb b/app/models/repository.rb
index 6e179f61a7b..fff6d4be275 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -1049,11 +1049,19 @@ class Repository
   end
 
   def cache
-    @cache ||= Gitlab::RepositoryCache.new(self)
+    @cache ||= if is_wiki
+                 Gitlab::RepositoryCache.new(self, extra_namespace: 'wiki')
+               else
+                 Gitlab::RepositoryCache.new(self)
+               end
   end
 
   def request_store_cache
-    @request_store_cache ||= Gitlab::RepositoryCache.new(self, backend: Gitlab::SafeRequestStore)
+    @request_store_cache ||= if is_wiki
+                               Gitlab::RepositoryCache.new(self, extra_namespace: 'wiki', backend: Gitlab::SafeRequestStore)
+                             else
+                               Gitlab::RepositoryCache.new(self, backend: Gitlab::SafeRequestStore)
+                             end
   end
 
   def tags_sorted_by_committed_date
diff --git a/lib/gitlab/cache/ci/project_pipeline_status.rb b/lib/gitlab/cache/ci/project_pipeline_status.rb
index dfbb83f7bb9..78b0eaac8cd 100644
--- a/lib/gitlab/cache/ci/project_pipeline_status.rb
+++ b/lib/gitlab/cache/ci/project_pipeline_status.rb
@@ -42,7 +42,7 @@ module Gitlab
         end
 
         def self.cache_key_for_project(project)
-          "#{Gitlab::Redis::Cache::CACHE_NAMESPACE}:projects/#{project.id}/pipeline_status/#{project.commit&.sha}"
+          "#{Gitlab::Redis::Cache::CACHE_NAMESPACE}:project:#{project.id}:pipeline_status:#{project.commit&.sha}"
         end
 
         def self.update_for_pipeline(pipeline)
diff --git a/lib/gitlab/repository_cache.rb b/lib/gitlab/repository_cache.rb
index 56007574b1b..6b0808f5304 100644
--- a/lib/gitlab/repository_cache.rb
+++ b/lib/gitlab/repository_cache.rb
@@ -7,13 +7,13 @@ module Gitlab
 
     def initialize(repository, extra_namespace: nil, backend: Rails.cache)
       @repository = repository
-      @namespace = "#{repository.full_path}:#{repository.project.id}"
+      @namespace = "project:#{repository.project.id}"
       @namespace = "#{@namespace}:#{extra_namespace}" if extra_namespace
       @backend = backend
     end
 
     def cache_key(type)
-      "#{type}:#{namespace}"
+      "#{namespace}:#{type}"
     end
 
     def expire(key)
diff --git a/spec/lib/gitlab/repository_cache_spec.rb b/spec/lib/gitlab/repository_cache_spec.rb
index 741ee12633f..1b9a8b4ab0d 100644
--- a/spec/lib/gitlab/repository_cache_spec.rb
+++ b/spec/lib/gitlab/repository_cache_spec.rb
@@ -4,14 +4,14 @@ describe Gitlab::RepositoryCache do
   let(:backend) { double('backend').as_null_object }
   let(:project) { create(:project) }
   let(:repository) { project.repository }
-  let(:namespace) { "#{repository.full_path}:#{project.id}" }
+  let(:namespace) { "project:#{project.id}" }
   let(:cache) { described_class.new(repository, backend: backend) }
 
   describe '#cache_key' do
     subject { cache.cache_key(:foo) }
 
     it 'includes the namespace' do
-      expect(subject).to eq "foo:#{namespace}"
+      expect(subject).to eq "#{namespace}:foo"
     end
 
     context 'with a given namespace' do
@@ -22,7 +22,7 @@ describe Gitlab::RepositoryCache do
       end
 
       it 'includes the full namespace' do
-        expect(subject).to eq "foo:#{namespace}:#{extra_namespace}"
+        expect(subject).to eq "#{namespace}:#{extra_namespace}:foo"
       end
     end
   end
@@ -30,21 +30,21 @@ describe Gitlab::RepositoryCache do
   describe '#expire' do
     it 'expires the given key from the cache' do
       cache.expire(:foo)
-      expect(backend).to have_received(:delete).with("foo:#{namespace}")
+      expect(backend).to have_received(:delete).with("#{namespace}:foo")
     end
   end
 
   describe '#fetch' do
     it 'fetches the given key from the cache' do
       cache.fetch(:bar)
-      expect(backend).to have_received(:fetch).with("bar:#{namespace}")
+      expect(backend).to have_received(:fetch).with("#{namespace}:bar")
     end
 
     it 'accepts a block' do
       p = -> {}
 
       cache.fetch(:baz, &p)
-      expect(backend).to have_received(:fetch).with("baz:#{namespace}", &p)
+      expect(backend).to have_received(:fetch).with("#{namespace}:baz", &p)
     end
   end
 
@@ -67,7 +67,7 @@ describe Gitlab::RepositoryCache do
         end
 
         it 'caches the value' do
-          expect(backend).to receive(:write).with("#{key}:#{namespace}", true)
+          expect(backend).to receive(:write).with("#{namespace}:#{key}", true)
 
           cache.fetch_without_caching_false(key) { true }
         end
@@ -83,7 +83,7 @@ describe Gitlab::RepositoryCache do
         end
 
         it 'does not cache the value' do
-          expect(backend).not_to receive(:write).with("#{key}:#{namespace}", true)
+          expect(backend).not_to receive(:write).with("#{namespace}:#{key}", true)
 
           cache.fetch_without_caching_false(key, &p)
         end
@@ -92,7 +92,7 @@ describe Gitlab::RepositoryCache do
 
     context 'when the cached value is truthy' do
       before do
-        backend.write("#{key}:#{namespace}", true)
+        backend.write("#{namespace}:#{key}", true)
       end
 
       it 'returns the cached value' do
@@ -116,7 +116,7 @@ describe Gitlab::RepositoryCache do
 
     context 'when the cached value is falsey' do
       before do
-        backend.write("#{key}:#{namespace}", false)
+        backend.write("#{namespace}:#{key}", false)
       end
 
       it 'returns the result of the block' do
@@ -126,7 +126,7 @@ describe Gitlab::RepositoryCache do
       end
 
       it 'writes the truthy value to the cache' do
-        expect(backend).to receive(:write).with("#{key}:#{namespace}", 'block result')
+        expect(backend).to receive(:write).with("#{namespace}:#{key}", 'block result')
 
         cache.fetch_without_caching_false(key) { 'block result' }
       end
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 2502fcac531..187283b284b 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -2301,4 +2301,22 @@ describe Repository do
       repository.merge_base('master', 'fix')
     end
   end
+
+  describe '#cache' do
+    subject(:cache) { repository.send(:cache) }
+
+    it 'returns a RepositoryCache' do
+      expect(subject).to be_kind_of Gitlab::RepositoryCache
+    end
+
+    it 'when is_wiki it includes wiki as part of key' do
+      allow(repository).to receive(:is_wiki) { true }
+
+      expect(subject.namespace).to include('wiki')
+    end
+
+    it 'when is_wiki is false extra_namespace is nil' do
+      expect(subject.namespace).not_to include('wiki')
+    end
+  end
 end