Merge branch '217947-extract-process-memory-caching-mechanism-into-a-concern' into 'master'

Extract process memory caching mechanism into a concern

Closes #217947

See merge request gitlab-org/gitlab!32538

app/models/ci/instance_variable.rb

@@ -3,6 +3,7 @@
 module Ci
   class InstanceVariable < ApplicationRecord
     extend Gitlab::Ci::Model
+    extend Gitlab::ProcessMemoryCache::Helper
     include Ci::NewHasVariable
     include Ci::Maskable
 
@@ -13,7 +14,8 @@ module Ci
     }
 
     scope :unprotected, -> { where(protected: false) }
-    after_commit { self.class.touch_redis_cache_timestamp }
+
+    after_commit { self.class.invalidate_memory_cache(:ci_instance_variable_data) }
 
     class << self
       def all_cached
@@ -24,10 +26,6 @@ module Ci
         cached_data[:unprotected]
       end
 
-      def touch_redis_cache_timestamp(time = Time.current.to_f)
-        shared_backend.write(:ci_instance_variable_changed_at, time)
-      end
-
       private
 
       def cached_data
@@ -37,40 +35,6 @@ module Ci
           { all: all_records, unprotected: all_records.reject(&:protected?) }
         end
       end
-
-      def fetch_memory_cache(key, &payload)
-        cache = process_backend.read(key)
-
-        if cache && !stale_cache?(cache)
-          cache[:data]
-        else
-          store_cache(key, &payload)
-        end
-      end
-
-      def stale_cache?(cache_info)
-        shared_timestamp = shared_backend.read(:ci_instance_variable_changed_at)
-        return true unless shared_timestamp
-
-        shared_timestamp.to_f > cache_info[:cached_at].to_f
-      end
-
-      def store_cache(key)
-        data = yield
-        time = Time.current.to_f
-
-        process_backend.write(key, data: data, cached_at: time)
-        touch_redis_cache_timestamp(time)
-        data
-      end
-
-      def shared_backend
-        Rails.cache
-      end
-
-      def process_backend
-        Gitlab::ProcessMemoryCache.cache_backend
-      end
     end
   end
 end

lib/gitlab/process_memory_cache/helper.rb

+# frozen_string_literal: true
+
+module Gitlab
+  class ProcessMemoryCache
+    module Helper
+      def fetch_memory_cache(key, &payload)
+        cache = cache_backend.read(key)
+
+        if cache && !stale_cache?(key, cache)
+          cache[:data]
+        else
+          store_cache(key, &payload)
+        end
+      end
+
+      def invalidate_memory_cache(key)
+        touch_cache_timestamp(key)
+      end
+
+      private
+
+      def touch_cache_timestamp(key, time = Time.current.to_f)
+        shared_backend.write(key, time)
+      end
+
+      def stale_cache?(key, cache_info)
+        shared_timestamp = shared_backend.read(key)
+        return true unless shared_timestamp
+
+        shared_timestamp.to_f > cache_info[:cached_at].to_f
+      end
+
+      def store_cache(key)
+        data = yield
+        time = Time.current.to_f
+
+        cache_backend.write(key, data: data, cached_at: time)
+        touch_cache_timestamp(key, time)
+        data
+      end
+
+      def shared_backend
+        Rails.cache
+      end
+
+      def cache_backend
+        ::Gitlab::ProcessMemoryCache.cache_backend
+      end
+    end
+  end
+end

spec/lib/gitlab/process_memory_cache/helper_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::ProcessMemoryCache::Helper, :use_clean_rails_memory_store_caching do
+  let(:minimal_test_class) do
+    Class.new do
+      include Gitlab::ProcessMemoryCache::Helper
+
+      def cached_content
+        fetch_memory_cache(:cached_content_instance_key) { expensive_computation }
+      end
+
+      def clear_cached_content
+        invalidate_memory_cache(:cached_content_instance_key)
+      end
+    end
+  end
+
+  before do
+    stub_const("MinimalTestClass", minimal_test_class)
+  end
+
+  subject { MinimalTestClass.new }
+
+  describe '.fetch_memory_cache' do
+    it 'memoizes the result' do
+      is_expected.to receive(:expensive_computation).once.and_return(1)
+
+      2.times do
+        expect(subject.cached_content).to eq(1)
+      end
+    end
+
+    it 'resets the cache when the shared key is missing', :aggregate_failures do
+      expect(Rails.cache).to receive(:read).with(:cached_content_instance_key).twice.and_return(nil)
+      is_expected.to receive(:expensive_computation).thrice.and_return(1, 2, 3)
+
+      3.times do |index|
+        expect(subject.cached_content).to eq(index + 1)
+      end
+    end
+  end
+
+  describe '.invalidate_memory_cache' do
+    it 'invalidates the cache' do
+      is_expected.to receive(:expensive_computation).twice.and_return(1, 2)
+
+      expect { subject.clear_cached_content }.to change { subject.cached_content }
+    end
+  end
+end

spec/models/ci/instance_variable_spec.rb

@@ -39,7 +39,7 @@ describe Ci::InstanceVariable do
     it { expect(described_class.all_cached).to contain_exactly(protected_variable, unprotected_variable) }
 
     it 'memoizes the result' do
-      expect(described_class).to receive(:store_cache).with(:ci_instance_variable_data).once.and_call_original
+      expect(described_class).to receive(:unscoped).once.and_call_original
 
       2.times do
         expect(described_class.all_cached).to contain_exactly(protected_variable, unprotected_variable)
@@ -65,15 +65,6 @@ describe Ci::InstanceVariable do
 
       expect(described_class.all_cached).to contain_exactly(protected_variable, unprotected_variable, variable)
     end
-
-    it 'resets the cache when the shared key is missing' do
-      expect(Rails.cache).to receive(:read).with(:ci_instance_variable_changed_at).twice.and_return(nil)
-      expect(described_class).to receive(:store_cache).with(:ci_instance_variable_data).thrice.and_call_original
-
-      3.times do
-        expect(described_class.all_cached).to contain_exactly(protected_variable, unprotected_variable)
-      end
-    end
   end
 
   describe '.unprotected_cached', :use_clean_rails_memory_store_caching do
@@ -83,7 +74,7 @@ describe Ci::InstanceVariable do
     it { expect(described_class.unprotected_cached).to contain_exactly(unprotected_variable) }
 
     it 'memoizes the result' do
-      expect(described_class).to receive(:store_cache).with(:ci_instance_variable_data).once.and_call_original
+      expect(described_class).to receive(:unscoped).once.and_call_original
 
       2.times do
         expect(described_class.unprotected_cached).to contain_exactly(unprotected_variable)