Merge branch 'pb-render-cached-helper' into 'master'

Implement render_cached helper

See merge request gitlab-org/gitlab!63986

lib/api/helpers/caching.rb

@@ -8,19 +8,12 @@
 module API
   module Helpers
     module Caching
-      # @return [ActiveSupport::Duration]
-      DEFAULT_EXPIRY = 1.day
-
+      include Gitlab::Cache::Helpers
       # @return [Hash]
       DEFAULT_CACHE_OPTIONS = {
         race_condition_ttl: 5.seconds
       }.freeze
 
-      # @return [ActiveSupport::Cache::Store]
-      def cache
-        Rails.cache
-      end
-
       # This is functionally equivalent to the standard `#present` used in
       # Grape endpoints, but the JSON for the object, or for each object of
       # a collection, will be cached.
@@ -45,7 +38,7 @@ module API
       # @param expires_in [ActiveSupport::Duration, Integer] an expiry time for the cache entry
       # @param presenter_args [Hash] keyword arguments to be passed to the entity
       # @return [Gitlab::Json::PrecompiledJson]
-      def present_cached(obj_or_collection, with:, cache_context: -> (_) { current_user&.cache_key }, expires_in: DEFAULT_EXPIRY, **presenter_args)
+      def present_cached(obj_or_collection, with:, cache_context: -> (_) { current_user&.cache_key }, expires_in: Gitlab::Cache::Helpers::DEFAULT_EXPIRY, **presenter_args)
         json =
           if obj_or_collection.is_a?(Enumerable)
             cached_collection(
@@ -120,77 +113,6 @@ module API
       def apply_default_cache_options(opts = {})
         DEFAULT_CACHE_OPTIONS.merge(opts)
       end
-
-      # Optionally uses a `Proc` to add context to a cache key
-      #
-      # @param object [Object] must respond to #cache_key
-      # @param context [Proc] a proc that will be called with the object as an argument, and which should return a
-      #                       string or array of strings to be combined into the cache key
-      # @return [String]
-      def contextual_cache_key(object, context)
-        return object.cache_key if context.nil?
-
-        [object.cache_key, context.call(object)].flatten.join(":")
-      end
-
-      # Used for fetching or rendering a single object
-      #
-      # @param object [Object] the object to render
-      # @param presenter [Grape::Entity]
-      # @param presenter_args [Hash] keyword arguments to be passed to the entity
-      # @param context [Proc]
-      # @param expires_in [ActiveSupport::Duration, Integer] an expiry time for the cache entry
-      # @return [String]
-      def cached_object(object, presenter:, presenter_args:, context:, expires_in:)
-        cache.fetch(contextual_cache_key(object, context), expires_in: expires_in) do
-          Gitlab::Json.dump(presenter.represent(object, **presenter_args).as_json)
-        end
-      end
-
-      # Used for fetching or rendering multiple objects
-      #
-      # @param objects [Enumerable<Object>] the objects to render
-      # @param presenter [Grape::Entity]
-      # @param presenter_args [Hash] keyword arguments to be passed to the entity
-      # @param context [Proc]
-      # @param expires_in [ActiveSupport::Duration, Integer] an expiry time for the cache entry
-      # @return [Array<String>]
-      def cached_collection(collection, presenter:, presenter_args:, context:, expires_in:)
-        json = fetch_multi(collection, context: context, expires_in: expires_in) do |obj|
-          Gitlab::Json.dump(presenter.represent(obj, **presenter_args).as_json)
-        end
-
-        json.values
-      end
-
-      # An adapted version of ActiveSupport::Cache::Store#fetch_multi.
-      #
-      # The original method only provides the missing key to the block,
-      # not the missing object, so we have to create a map of cache keys
-      # to the objects to allow us to pass the object to the missing value
-      # block.
-      #
-      # The result is that this is functionally identical to `#fetch`.
-      def fetch_multi(*objs, context:, **kwargs)
-        objs.flatten!
-        map = multi_key_map(objs, context: context)
-
-        # TODO: `contextual_cache_key` should be constructed based on the guideline https://docs.gitlab.com/ee/development/redis.html#multi-key-commands.
-        Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do
-          cache.fetch_multi(*map.keys, **kwargs) do |key|
-            yield map[key]
-          end
-        end
-      end
-
-      # @param objects [Enumerable<Object>] objects which _must_ respond to `#cache_key`
-      # @param context [Proc] a proc that can be called to help generate each cache key
-      # @return [Hash]
-      def multi_key_map(objects, context:)
-        objects.index_by do |object|
-          contextual_cache_key(object, context)
-        end
-      end
     end
   end
 end

lib/gitlab/cache/helpers.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Cache
+    module Helpers
+      # @return [ActiveSupport::Duration]
+      DEFAULT_EXPIRY = 1.day
+
+      # @return [ActiveSupport::Cache::Store]
+      def cache
+        Rails.cache
+      end
+
+      def render_cached(obj_or_collection, with:, cache_context: -> (_) { current_user&.cache_key }, expires_in: Gitlab::Cache::Helpers::DEFAULT_EXPIRY, **presenter_args)
+        json =
+          if obj_or_collection.is_a?(Enumerable)
+            cached_collection(
+              obj_or_collection,
+              presenter: with,
+              presenter_args: presenter_args,
+              context: cache_context,
+              expires_in: expires_in
+            )
+          else
+            cached_object(
+              obj_or_collection,
+              presenter: with,
+              presenter_args: presenter_args,
+              context: cache_context,
+              expires_in: expires_in
+            )
+          end
+
+        render Gitlab::Json::PrecompiledJson.new(json)
+      end
+
+      private
+
+      # Optionally uses a `Proc` to add context to a cache key
+      #
+      # @param object [Object] must respond to #cache_key
+      # @param context [Proc] a proc that will be called with the object as an argument, and which should return a
+      #                       string or array of strings to be combined into the cache key
+      # @return [String]
+      def contextual_cache_key(presenter, object, context)
+        return object.cache_key if context.nil?
+
+        [presenter.class.name, object.cache_key, context.call(object)].flatten.join(":")
+      end
+
+      # Used for fetching or rendering a single object
+      #
+      # @param object [Object] the object to render
+      # @param presenter [Grape::Entity]
+      # @param presenter_args [Hash] keyword arguments to be passed to the entity
+      # @param context [Proc]
+      # @param expires_in [ActiveSupport::Duration, Integer] an expiry time for the cache entry
+      # @return [String]
+      def cached_object(object, presenter:, presenter_args:, context:, expires_in:)
+        cache.fetch(contextual_cache_key(presenter, object, context), expires_in: expires_in) do
+          Gitlab::Json.dump(presenter.represent(object, **presenter_args).as_json)
+        end
+      end
+
+      # Used for fetching or rendering multiple objects
+      #
+      # @param objects [Enumerable<Object>] the objects to render
+      # @param presenter [Grape::Entity]
+      # @param presenter_args [Hash] keyword arguments to be passed to the entity
+      # @param context [Proc]
+      # @param expires_in [ActiveSupport::Duration, Integer] an expiry time for the cache entry
+      # @return [Array<String>]
+      def cached_collection(collection, presenter:, presenter_args:, context:, expires_in:)
+        json = fetch_multi(presenter, collection, context: context, expires_in: expires_in) do |obj|
+          Gitlab::Json.dump(presenter.represent(obj, **presenter_args).as_json)
+        end
+
+        json.values
+      end
+
+      # An adapted version of ActiveSupport::Cache::Store#fetch_multi.
+      #
+      # The original method only provides the missing key to the block,
+      # not the missing object, so we have to create a map of cache keys
+      # to the objects to allow us to pass the object to the missing value
+      # block.
+      #
+      # The result is that this is functionally identical to `#fetch`.
+      def fetch_multi(presenter, *objs, context:, **kwargs)
+        objs.flatten!
+        map = multi_key_map(presenter, objs, context: context)
+
+        # TODO: `contextual_cache_key` should be constructed based on the guideline https://docs.gitlab.com/ee/development/redis.html#multi-key-commands.
+        Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do
+          cache.fetch_multi(*map.keys, **kwargs) do |key|
+            yield map[key]
+          end
+        end
+      end
+
+      # @param objects [Enumerable<Object>] objects which _must_ respond to `#cache_key`
+      # @param context [Proc] a proc that can be called to help generate each cache key
+      # @return [Hash]
+      def multi_key_map(presenter, objects, context:)
+        objects.index_by do |object|
+          contextual_cache_key(presenter, object, context)
+        end
+      end
+    end
+  end
+end

lib/gitlab/json.rb

@@ -228,6 +228,14 @@ module Gitlab
 
         raise UnsupportedFormatError
       end
+
+      def render_in(_view_context)
+        to_s
+      end
+
+      def format
+        :json
+      end
     end
 
     class LimitedEncoder

spec/lib/api/helpers/caching_spec.rb

@@ -44,108 +44,16 @@ RSpec.describe API::Helpers::Caching, :use_clean_rails_redis_caching do
       }
     end
 
-    context "single object" do
+    context 'single object' do
       let_it_be(:presentable) { create(:todo, project: project) }
 
-      it { is_expected.to be_a(Gitlab::Json::PrecompiledJson) }
-
-      it "uses the presenter" do
-        expect(presenter).to receive(:represent).with(presentable, project: project)
-
-        subject
-      end
-
-      it "is valid JSON" do
-        parsed = Gitlab::Json.parse(subject.to_s)
-
-        expect(parsed).to be_a(Hash)
-        expect(parsed["id"]).to eq(presentable.id)
-      end
-
-      it "fetches from the cache" do
-        expect(instance.cache).to receive(:fetch).with("#{presentable.cache_key}:#{user.cache_key}", expires_in: described_class::DEFAULT_EXPIRY).once
-
-        subject
-      end
-
-      context "when a cache context is supplied" do
-        before do
-          kwargs[:cache_context] = -> (todo) { todo.project.cache_key }
-        end
-
-        it "uses the context to augment the cache key" do
-          expect(instance.cache).to receive(:fetch).with("#{presentable.cache_key}:#{project.cache_key}", expires_in: described_class::DEFAULT_EXPIRY).once
-
-          subject
-        end
-      end
-
-      context "when expires_in is supplied" do
-        it "sets the expiry when accessing the cache" do
-          kwargs[:expires_in] = 7.days
-
-          expect(instance.cache).to receive(:fetch).with("#{presentable.cache_key}:#{user.cache_key}", expires_in: 7.days).once
-
-          subject
-        end
-      end
+      it_behaves_like 'object cache helper'
     end
 
-    context "for a collection of objects" do
+    context 'collection of objects' do
       let_it_be(:presentable) { Array.new(5).map { create(:todo, project: project) } }
 
-      it { is_expected.to be_an(Gitlab::Json::PrecompiledJson) }
-
-      it "uses the presenter" do
-        presentable.each do |todo|
-          expect(presenter).to receive(:represent).with(todo, project: project)
-        end
-
-        subject
-      end
-
-      it "is valid JSON" do
-        parsed = Gitlab::Json.parse(subject.to_s)
-
-        expect(parsed).to be_an(Array)
-
-        presentable.each_with_index do |todo, i|
-          expect(parsed[i]["id"]).to eq(todo.id)
-        end
-      end
-
-      it "fetches from the cache" do
-        keys = presentable.map { |todo| "#{todo.cache_key}:#{user.cache_key}" }
-
-        expect(instance.cache).to receive(:fetch_multi).with(*keys, expires_in: described_class::DEFAULT_EXPIRY).once.and_call_original
-
-        subject
-      end
-
-      context "when a cache context is supplied" do
-        before do
-          kwargs[:cache_context] = -> (todo) { todo.project.cache_key }
-        end
-
-        it "uses the context to augment the cache key" do
-          keys = presentable.map { |todo| "#{todo.cache_key}:#{project.cache_key}" }
-
-          expect(instance.cache).to receive(:fetch_multi).with(*keys, expires_in: described_class::DEFAULT_EXPIRY).once.and_call_original
-
-          subject
-        end
-      end
-
-      context "expires_in is supplied" do
-        it "sets the expiry when accessing the cache" do
-          keys = presentable.map { |todo| "#{todo.cache_key}:#{user.cache_key}" }
-          kwargs[:expires_in] = 7.days
-
-          expect(instance.cache).to receive(:fetch_multi).with(*keys, expires_in: 7.days).once.and_call_original
-
-          subject
-        end
-      end
+      it_behaves_like 'collection cache helper'
     end
   end
 

spec/lib/gitlab/cache/helpers_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Cache::Helpers, :use_clean_rails_redis_caching do
+  subject(:instance) { Class.new.include(described_class).new }
+
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:user) { create(:user) }
+
+  let(:presenter) { MergeRequestSerializer.new(current_user: user, project: project) }
+
+  before do
+    # We have to stub #render as it's a Rails controller method unavailable in
+    # the module by itself
+    allow(instance).to receive(:render) { |data| data }
+    allow(instance).to receive(:current_user) { user }
+  end
+
+  describe "#render_cached" do
+    subject do
+      instance.render_cached(presentable, **kwargs)
+    end
+
+    let(:kwargs) do
+      {
+        with: presenter,
+        project: project
+      }
+    end
+
+    context 'single object' do
+      let_it_be(:presentable) { create(:merge_request, source_project: project, source_branch: 'wip') }
+
+      it_behaves_like 'object cache helper'
+    end
+
+    context 'collection of objects' do
+      let_it_be(:presentable) do
+        [
+          create(:merge_request, source_project: project, source_branch: 'fix'),
+          create(:merge_request, source_project: project, source_branch: 'master')
+        ]
+      end
+
+      it_behaves_like 'collection cache helper'
+    end
+  end
+end

spec/support/shared_examples/lib/cache_helpers_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples_for 'object cache helper' do
+  it { is_expected.to be_a(Gitlab::Json::PrecompiledJson) }
+
+  it "uses the presenter" do
+    expect(presenter).to receive(:represent).with(presentable, project: project)
+
+    subject
+  end
+
+  it "is valid JSON" do
+    parsed = Gitlab::Json.parse(subject.to_s)
+
+    expect(parsed).to be_a(Hash)
+    expect(parsed["id"]).to eq(presentable.id)
+  end
+
+  it "fetches from the cache" do
+    expect(instance.cache).to receive(:fetch).with("#{presenter.class.name}:#{presentable.cache_key}:#{user.cache_key}", expires_in: described_class::DEFAULT_EXPIRY).once
+
+    subject
+  end
+
+  context "when a cache context is supplied" do
+    before do
+      kwargs[:cache_context] = -> (item) { item.project.cache_key }
+    end
+
+    it "uses the context to augment the cache key" do
+      expect(instance.cache).to receive(:fetch).with("#{presenter.class.name}:#{presentable.cache_key}:#{project.cache_key}", expires_in: described_class::DEFAULT_EXPIRY).once
+
+      subject
+    end
+  end
+
+  context "when expires_in is supplied" do
+    it "sets the expiry when accessing the cache" do
+      kwargs[:expires_in] = 7.days
+
+      expect(instance.cache).to receive(:fetch).with("#{presenter.class.name}:#{presentable.cache_key}:#{user.cache_key}", expires_in: 7.days).once
+
+      subject
+    end
+  end
+end
+
+RSpec.shared_examples_for 'collection cache helper' do
+  it { is_expected.to be_an(Gitlab::Json::PrecompiledJson) }
+
+  it "uses the presenter" do
+    presentable.each do |item|
+      expect(presenter).to receive(:represent).with(item, project: project)
+    end
+
+    subject
+  end
+
+  it "is valid JSON" do
+    parsed = Gitlab::Json.parse(subject.to_s)
+
+    expect(parsed).to be_an(Array)
+
+    presentable.each_with_index do |item, i|
+      expect(parsed[i]["id"]).to eq(item.id)
+    end
+  end
+
+  it "fetches from the cache" do
+    keys = presentable.map { |item| "#{presenter.class.name}:#{item.cache_key}:#{user.cache_key}" }
+
+    expect(instance.cache).to receive(:fetch_multi).with(*keys, expires_in: described_class::DEFAULT_EXPIRY).once.and_call_original
+
+    subject
+  end
+
+  context "when a cache context is supplied" do
+    before do
+      kwargs[:cache_context] = -> (item) { item.project.cache_key }
+    end
+
+    it "uses the context to augment the cache key" do
+      keys = presentable.map { |item| "#{presenter.class.name}:#{item.cache_key}:#{project.cache_key}" }
+
+      expect(instance.cache).to receive(:fetch_multi).with(*keys, expires_in: described_class::DEFAULT_EXPIRY).once.and_call_original
+
+      subject
+    end
+  end
+
+  context "expires_in is supplied" do
+    it "sets the expiry when accessing the cache" do
+      keys = presentable.map { |item| "#{presenter.class.name}:#{item.cache_key}:#{user.cache_key}" }
+      kwargs[:expires_in] = 7.days
+
+      expect(instance.cache).to receive(:fetch_multi).with(*keys, expires_in: 7.days).once.and_call_original
+
+      subject
+    end
+  end
+end