Separate enabling of caching and setting the cache store

Author: bf4

lib/active_model/serializer/caching.rb

@@ -5,7 +5,7 @@ module Caching
 
       included do
         with_options instance_writer: false, instance_reader: false do |serializer|
-          serializer.class_attribute :_cache         # @api private : the cache object
+          serializer.class_attribute :_cache         # @api private : the cache store
           serializer.class_attribute :_fragmented    # @api private : @see ::fragmented
           serializer.class_attribute :_cache_key     # @api private : when present, is first item in cache_key
           serializer.class_attribute :_cache_only    # @api private : when fragment caching, whitelists cached_attributes. Cannot combine with except
@@ -71,6 +71,7 @@ def fragmented(serializer)
         #   when Rails.configuration.action_controller.perform_caching
         #
         # @params options [Hash] with valid keys:
+        #   cache_store    : @see ::_cache
         #   key            : @see ::_cache_key
         #   only           : @see ::_cache_only
         #   except         : @see ::_cache_except
@@ -88,16 +89,74 @@ def fragmented(serializer)
         # @todo require less code comments. See
         # https://github.com/rails-api/active_model_serializers/pull/1249#issuecomment-146567837
         def cache(options = {})
-          self._cache = ActiveModelSerializers.config.cache_store if ActiveModelSerializers.config.perform_caching
-          serializer = self
-          ActiveSupport.on_load(:action_controller) do
-            serializer._cache = ActiveModelSerializers.config.cache_store if ActiveModelSerializers.config.perform_caching
-          end
+          self._cache =
+            options.delete(:cache_store) ||
+            ActiveModelSerializers.config.cache_store ||
+            ActiveSupport::Cache.lookup_store(:null_store)
           self._cache_key = options.delete(:key)
           self._cache_only = options.delete(:only)
           self._cache_except = options.delete(:except)
           self._cache_options = options.empty? ? nil : options
         end
+
+        # @return [true, false]
+        # We're using class variables here because it is a class attribute
+        # that is globally true for the `ActiveModel::Serializer` class; i.e. neither per subclass nor inherited.
+        #
+        # We're not using a class_attribute because of the special behavior in
+        # `perform_caching` setting itself to `ActiveModelSerializers.config.perform_caching`
+        # when first called if it wasn't first set.
+        #
+        # This is to allow us to have a global config that can be set any time before
+        # `perform_caching` is called.
+        #
+        # One downside of this, is that subsequent setting of the global config will not change
+        # `ActiveModel::Serializer.perform_caching`, but that should be an edge case that
+        # is easily handled.
+        #
+        # If you, reading this, can figure out how to have ActiveModel::Serializer always delegate
+        # `perform_caching` and `perform_caching=` to the global config, that would make a nice PR.
+        # rubocop:disable Style/ClassVars
+        def perform_caching
+          return @@perform_caching if defined?(@@perform_caching)
+          self.perform_caching = ActiveModelSerializers.config.perform_caching
+        end
+        alias perform_caching? perform_caching
+
+        # @param [true, false]
+        def perform_caching=(perform_caching)
+          @@perform_caching = perform_caching
+        end
+        # rubocop:enable Style/ClassVars
+
+        # The canonical method for getting the cache store for the serializer.
+        #
+        # @return [nil] when _cache is not set (i.e. when `cache` has not been called)
+        # @return [._cache] when _cache is not the NullStore
+        # @return [ActiveModelSerializers.config.cache_store] when _cache is the NullStore.
+        #   This is so we can use `cache` being called to mean the serializer should be cached
+        #   even if ActiveModelSerializers.config.cache_store has not yet been set.
+        #   That means that when _cache is the NullStore and ActiveModelSerializers.config.cache_store
+        #   is configured, `cache_store` becomes `ActiveModelSerializers.config.cache_store`.
+        # @return [nil] when _cache is the NullStore and ActiveModelSerializers.config.cache_store is nil.
+        def cache_store
+          return nil if _cache.nil?
+          return _cache if _cache.class != ActiveSupport::Cache::NullStore
+          if ActiveModelSerializers.config.cache_store
+            self._cache = ActiveModelSerializers.config.cache_store
+          else
+            nil
+          end
+        end
+
+        def cache_enabled?
+          perform_caching? && cache_store && !_cache_only && !_cache_except
+        end
+
+        def fragment_cache_enabled?
+          perform_caching? && cache_store &&
+            (_cache_only && !_cache_except || !_cache_only && _cache_except)
+        end
       end
     end
   end

lib/active_model_serializers/cached_serializer.rb

@@ -18,11 +18,11 @@ def cache_check(adapter_instance)
     end
 
     def cached?
-      @klass._cache && !@klass._cache_only && !@klass._cache_except
+      @klass.cache_enabled?
     end
 
     def fragment_cached?
-      @klass._cache && (@klass._cache_only && !@klass._cache_except || !@klass._cache_only && @klass._cache_except)
+      @klass.fragment_cache_enabled?
     end
 
     def cache_key

test/cache_test.rb

@@ -34,6 +34,19 @@ def setup
       @blog_serializer     = BlogSerializer.new(@blog)
     end
 
+    def test_explicit_cache_store
+      default_store = Class.new(ActiveModel::Serializer) do
+        cache
+      end
+      explicit_store = Class.new(ActiveModel::Serializer) do
+        cache cache_store: ActiveSupport::Cache::FileStore
+      end
+
+      assert ActiveSupport::Cache::MemoryStore, ActiveModelSerializers.config.cache_store
+      assert ActiveSupport::Cache::MemoryStore, default_store.cache_store
+      assert ActiveSupport::Cache::FileStore, explicit_store.cache_store
+    end
+
     def test_inherited_cache_configuration
       inherited_serializer = Class.new(PostSerializer)
 

test/serializers/caching_configuration_test_isolated.rb

@@ -0,0 +1,168 @@
+# Execute this test in isolation
+require 'support/isolated_unit'
+
+class CachingConfigurationTest < ActiveSupport::TestCase
+  include ActiveSupport::Testing::Isolation
+
+  setup do
+    require 'rails'
+    # AMS needs to be required before Rails.application is initialized for
+    # Railtie's to fire in Rails.application.initialize!
+    # (and make_basic_app initializes the app)
+    require 'active_model_serializers'
+    # Create serializers before Rails.application.initialize!
+    # To ensure we're testing that the cache settings depend on
+    # the Railtie firing, not on the ActionController being loaded.
+    create_serializers
+  end
+
+  def create_serializers
+    @cached_serializer = Class.new(ActiveModel::Serializer) do
+      cache skip_digest: true
+      attributes :id, :name, :title
+    end
+    @fragment_cached_serializer = Class.new(ActiveModel::Serializer) do
+      cache only: :id
+      attributes :id, :name, :title
+    end
+    @non_cached_serializer = Class.new(ActiveModel::Serializer) do
+      attributes :id, :name, :title
+    end
+  end
+
+  class PerformCachingTrue < CachingConfigurationTest
+    setup do
+      # Let's make that Rails app and initialize it!
+      make_basic_app do |app|
+        app.config.action_controller.perform_caching = true
+        app.config.action_controller.cache_store = ActiveSupport::Cache.lookup_store(:memory_store)
+      end
+    end
+
+    test 'it sets perform_caching to true on AMS.config and serializers' do
+      assert Rails.configuration.action_controller.perform_caching
+      assert ActiveModelSerializers.config.perform_caching
+      assert ActiveModel::Serializer.perform_caching?
+      assert @cached_serializer.perform_caching?
+      assert @non_cached_serializer.perform_caching?
+      assert @fragment_cached_serializer.perform_caching?
+    end
+
+    test 'it sets the AMS.config.cache_store to the controller cache_store' do
+      assert_equal controller_cache_store, ActiveSupport::Cache::MemoryStore
+      assert_equal controller_cache_store, ActiveModelSerializers.config.cache_store.class
+    end
+
+    test 'it sets the cached serializer cache_store to the ActionController::Base.cache_store' do
+      assert_equal ActiveSupport::Cache::NullStore, @cached_serializer._cache.class
+      assert_equal controller_cache_store, @cached_serializer.cache_store.class
+      assert_equal ActiveSupport::Cache::MemoryStore, @cached_serializer._cache.class
+    end
+
+    test 'the cached serializer has cache_enabled?' do
+      assert @cached_serializer.cache_enabled?
+    end
+
+    test 'the cached serializer does not have fragment_cache_enabled?' do
+      refute @cached_serializer.fragment_cache_enabled?
+    end
+
+    test 'the non-cached serializer cache_store is nil' do
+      assert_equal nil, @non_cached_serializer._cache
+      assert_equal nil, @non_cached_serializer.cache_store
+      assert_equal nil, @non_cached_serializer._cache
+    end
+
+    test 'the non-cached serializer does not have cache_enabled?' do
+      refute @non_cached_serializer.cache_enabled?
+    end
+
+    test 'the non-cached serializer does not have fragment_cache_enabled?' do
+      refute @non_cached_serializer.fragment_cache_enabled?
+    end
+
+    test 'it sets the fragment cached serializer cache_store to the ActionController::Base.cache_store' do
+      assert_equal ActiveSupport::Cache::NullStore, @fragment_cached_serializer._cache.class
+      assert_equal controller_cache_store, @fragment_cached_serializer.cache_store.class
+      assert_equal ActiveSupport::Cache::MemoryStore, @fragment_cached_serializer._cache.class
+    end
+
+    test 'the fragment cached serializer does not have cache_enabled?' do
+      refute @fragment_cached_serializer.cache_enabled?
+    end
+
+    test 'the fragment cached serializer has fragment_cache_enabled?' do
+      assert @fragment_cached_serializer.fragment_cache_enabled?
+    end
+  end
+
+  class PerformCachingFalse < CachingConfigurationTest
+    setup do
+      # Let's make that Rails app and initialize it!
+      make_basic_app do |app|
+        app.config.action_controller.perform_caching = false
+        app.config.action_controller.cache_store = ActiveSupport::Cache.lookup_store(:memory_store)
+      end
+    end
+
+    test 'it sets perform_caching to false on AMS.config and serializers' do
+      refute Rails.configuration.action_controller.perform_caching
+      refute ActiveModelSerializers.config.perform_caching
+      refute ActiveModel::Serializer.perform_caching?
+      refute @cached_serializer.perform_caching?
+      refute @non_cached_serializer.perform_caching?
+      refute @fragment_cached_serializer.perform_caching?
+    end
+
+    test 'it sets the AMS.config.cache_store to the controller cache_store' do
+      assert_equal controller_cache_store, ActiveSupport::Cache::MemoryStore
+      assert_equal controller_cache_store, ActiveModelSerializers.config.cache_store.class
+    end
+
+    test 'it sets the cached serializer cache_store to the ActionController::Base.cache_store' do
+      assert_equal ActiveSupport::Cache::NullStore, @cached_serializer._cache.class
+      assert_equal controller_cache_store, @cached_serializer.cache_store.class
+      assert_equal ActiveSupport::Cache::MemoryStore, @cached_serializer._cache.class
+    end
+
+    test 'the cached serializer does not have cache_enabled?' do
+      refute @cached_serializer.cache_enabled?
+    end
+
+    test 'the cached serializer does not have fragment_cache_enabled?' do
+      refute @cached_serializer.fragment_cache_enabled?
+    end
+
+    test 'the non-cached serializer cache_store is nil' do
+      assert_equal nil, @non_cached_serializer._cache
+      assert_equal nil, @non_cached_serializer.cache_store
+      assert_equal nil, @non_cached_serializer._cache
+    end
+
+    test 'the non-cached serializer does not have cache_enabled?' do
+      refute @non_cached_serializer.cache_enabled?
+    end
+
+    test 'the non-cached serializer does not have fragment_cache_enabled?' do
+      refute @non_cached_serializer.fragment_cache_enabled?
+    end
+
+    test 'it sets the fragment cached serializer cache_store to the ActionController::Base.cache_store' do
+      assert_equal ActiveSupport::Cache::NullStore, @fragment_cached_serializer._cache.class
+      assert_equal controller_cache_store, @fragment_cached_serializer.cache_store.class
+      assert_equal ActiveSupport::Cache::MemoryStore, @fragment_cached_serializer._cache.class
+    end
+
+    test 'the fragment cached serializer does not have cache_enabled?' do
+      refute @fragment_cached_serializer.cache_enabled?
+    end
+
+    test 'the fragment cached serializer does not have fragment_cache_enabled?' do
+      refute @fragment_cached_serializer.fragment_cache_enabled?
+    end
+  end
+
+  def controller_cache_store
+    ActionController::Base.cache_store.class
+  end
+end