Merge pull request #1478 from bf4/caching_fix

[FIX] Serializers can now be defined *before* Rails initializes and cache store will be correctly set

Author: Yohan Robert

CHANGELOG.md

@@ -24,6 +24,8 @@ Features:
 - [#1340](https://github.com/rails-api/active_model_serializers/pull/1340) Add support for resource-level meta. (@beauby)
 
 Fixes:
+- [#1478](https://github.com/rails-api/active_model_serializers/pull/1478) Cache store will now be correctly set when serializers are
+  loaded *before* Rails initializes. (@bf4)
 - [#1570](https://github.com/rails-api/active_model_serializers/pull/1570) Fixed pagination issue with last page size. (@bmorrall)
 - [#1516](https://github.com/rails-api/active_model_serializers/pull/1516) No longer return a nil href when only
   adding meta to a relationship link. (@groyoh)

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,12 +89,58 @@ 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
+          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
+
+        # Value is from ActiveModelSerializers.config.perform_caching. Is used to
+        # globally enable or disable all serializer caching, just like
+        # Rails.configuration.action_controller.perform_caching, which is its
+        # default value in a Rails application.
+        # @return [true, false]
+        # Memoizes value of config first time it is called with a non-nil value.
+        # rubocop:disable Style/ClassVars
+        def perform_caching
+          return @@perform_caching if defined?(@@perform_caching) && !@@perform_caching.nil?
+          @@perform_caching = ActiveModelSerializers.config.perform_caching
+        end
+        alias perform_caching? perform_caching
+        # 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