From c3c69a607aac0698b51db2e9cc9cba0c585d291a Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Wed, 16 Mar 2016 22:25:44 -0500 Subject: [PATCH] Separate enabling of caching and setting the cache store --- lib/active_model/serializer/caching.rb | 71 +++++++- .../cached_serializer.rb | 4 +- test/cache_test.rb | 13 ++ .../caching_configuration_test_isolated.rb | 168 ++++++++++++++++++ 4 files changed, 248 insertions(+), 8 deletions(-) create mode 100644 test/serializers/caching_configuration_test_isolated.rb diff --git a/lib/active_model/serializer/caching.rb b/lib/active_model/serializer/caching.rb index 4d015acb..8a51ec77 100644 --- a/lib/active_model/serializer/caching.rb +++ b/lib/active_model/serializer/caching.rb @@ -5,7 +5,7 @@ module ActiveModel 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 @@ module ActiveModel # 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 @@ module ActiveModel # @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 diff --git a/lib/active_model_serializers/cached_serializer.rb b/lib/active_model_serializers/cached_serializer.rb index 8bc85600..11a6259d 100644 --- a/lib/active_model_serializers/cached_serializer.rb +++ b/lib/active_model_serializers/cached_serializer.rb @@ -18,11 +18,11 @@ module ActiveModelSerializers 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 diff --git a/test/cache_test.rb b/test/cache_test.rb index 72b486e3..46d3a973 100644 --- a/test/cache_test.rb +++ b/test/cache_test.rb @@ -34,6 +34,19 @@ module ActiveModelSerializers @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) diff --git a/test/serializers/caching_configuration_test_isolated.rb b/test/serializers/caching_configuration_test_isolated.rb new file mode 100644 index 00000000..1b6d5541 --- /dev/null +++ b/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