Merge pull request #1480 from bf4/fix_cache_store

[FIX] The cache store needs to be the actually store, not e.g. :memory_store

CHANGELOG.md

@@ -27,6 +27,8 @@ Features:
 - [#1340](https://github.com/rails-api/active_model_serializers/pull/1340) Add support for resource-level meta. (@beauby)
 
 Fixes:
+- [#1480](https://github.com/rails-api/active_model_serializers/pull/1480) Fix setting of cache_store from Rails configuration. (@bf4)
+  Fix uninentional mutating of value in memory cache store. (@groyoh)
 - [#1622](https://github.com/rails-api/active_model_serializers/pull/1622) Fragment cache changed from per-record to per-serializer.
   Now, two serializers that use the same model may be separately cached. (@lserman)
 - [#1478](https://github.com/rails-api/active_model_serializers/pull/1478) Cache store will now be correctly set when serializers are

lib/active_model_serializers/adapter/attributes.rb

@@ -55,7 +55,7 @@ def cached_attributes(cached_serializer)
       def serializable_hash_for_single_resource(options)
         resource = resource_object_for(options)
         relationships = resource_relationships(options)
-        resource.merge!(relationships)
+        resource.merge(relationships)
       end
 
       def resource_relationships(options)

lib/active_model_serializers/railtie.rb

@@ -25,8 +25,11 @@ class Railtie < Rails::Railtie
     # and also before eager_loading (if enabled).
     initializer 'active_model_serializers.set_configs', :after => 'action_controller.set_configs' do
       ActiveModelSerializers.logger = Rails.configuration.action_controller.logger
-      ActiveModelSerializers.config.cache_store     = Rails.configuration.action_controller.cache_store
       ActiveModelSerializers.config.perform_caching = Rails.configuration.action_controller.perform_caching
+      # We want this hook to run after the config has been set, even if ActionController has already loaded.
+      ActiveSupport.on_load(:action_controller) do
+        ActiveModelSerializers.config.cache_store = cache_store
+      end
     end
 
     generators do |app|

test/cache_test.rb

@@ -9,8 +9,8 @@ class CacheTest < ActiveSupport::TestCase
       attribute :special_attribute
     end
 
-    def setup
-      ActionController::Base.cache_store.clear
+    setup do
+      cache_store.clear
       @comment        = Comment.new(id: 1, body: 'ZOMG A COMMENT')
       @post           = Post.new(title: 'New Post', body: 'Body')
       @bio            = Bio.new(id: 1, content: 'AMS Contributor')
@@ -70,9 +70,9 @@ def test_override_cache_configuration
     end
 
     def test_cache_definition
-      assert_equal(ActionController::Base.cache_store, @post_serializer.class._cache)
-      assert_equal(ActionController::Base.cache_store, @author_serializer.class._cache)
-      assert_equal(ActionController::Base.cache_store, @comment_serializer.class._cache)
+      assert_equal(cache_store, @post_serializer.class._cache)
+      assert_equal(cache_store, @author_serializer.class._cache)
+      assert_equal(cache_store, @comment_serializer.class._cache)
     end
 
     def test_cache_key_definition
@@ -83,13 +83,13 @@ def test_cache_key_definition
 
     def test_cache_key_interpolation_with_updated_at
       render_object_with_cache(@author)
-      assert_equal(nil, ActionController::Base.cache_store.fetch(@author.cache_key))
-      assert_equal(@author_serializer.attributes.to_json, ActionController::Base.cache_store.fetch("#{@author_serializer.class._cache_key}/#{@author_serializer.object.id}-#{@author_serializer.object.updated_at.strftime("%Y%m%d%H%M%S%9N")}").to_json)
+      assert_equal(nil, cache_store.fetch(@author.cache_key))
+      assert_equal(@author_serializer.attributes.to_json, cache_store.fetch("#{@author_serializer.class._cache_key}/#{@author_serializer.object.id}-#{@author_serializer.object.updated_at.strftime("%Y%m%d%H%M%S%9N")}").to_json)
     end
 
     def test_default_cache_key_fallback
       render_object_with_cache(@comment)
-      assert_equal(@comment_serializer.attributes.to_json, ActionController::Base.cache_store.fetch(@comment.cache_key).to_json)
+      assert_equal(@comment_serializer.attributes.to_json, cache_store.fetch(@comment.cache_key).to_json)
     end
 
     def test_cache_options_definition
@@ -104,41 +104,38 @@ def test_fragment_cache_definition
     end
 
     def test_associations_separately_cache
-      ActionController::Base.cache_store.clear
-      assert_equal(nil, ActionController::Base.cache_store.fetch(@post.cache_key))
-      assert_equal(nil, ActionController::Base.cache_store.fetch(@comment.cache_key))
+      cache_store.clear
+      assert_equal(nil, cache_store.fetch(@post.cache_key))
+      assert_equal(nil, cache_store.fetch(@comment.cache_key))
 
       Timecop.freeze(Time.current) do
         render_object_with_cache(@post)
 
-        assert_equal(@post_serializer.attributes, ActionController::Base.cache_store.fetch(@post.cache_key))
-        assert_equal(@comment_serializer.attributes, ActionController::Base.cache_store.fetch(@comment.cache_key))
+        assert_equal(@post_serializer.attributes, cache_store.fetch(@post.cache_key))
+        assert_equal(@comment_serializer.attributes, cache_store.fetch(@comment.cache_key))
       end
     end
 
     def test_associations_cache_when_updated
-      # Clean the Cache
-      ActionController::Base.cache_store.clear
-
       Timecop.freeze(Time.current) do
         # Generate a new Cache of Post object and each objects related to it.
         render_object_with_cache(@post)
 
         # Check if it cached the objects separately
-        assert_equal(@post_serializer.attributes, ActionController::Base.cache_store.fetch(@post.cache_key))
-        assert_equal(@comment_serializer.attributes, ActionController::Base.cache_store.fetch(@comment.cache_key))
+        assert_equal(@post_serializer.attributes,    cached_serialization(@post_serializer))
+        assert_equal(@comment_serializer.attributes, cached_serialization(@comment_serializer))
 
         # Simulating update on comments relationship with Post
-        new_comment            = Comment.new(id: 2, body: 'ZOMG A NEW COMMENT')
+        new_comment            = Comment.new(id: 2567, body: 'ZOMG A NEW COMMENT')
         new_comment_serializer = CommentSerializer.new(new_comment)
         @post.comments         = [new_comment]
 
         # Ask for the serialized object
         render_object_with_cache(@post)
 
         # Check if the the new comment was cached
-        assert_equal(new_comment_serializer.attributes, ActionController::Base.cache_store.fetch(new_comment.cache_key))
-        assert_equal(@post_serializer.attributes, ActionController::Base.cache_store.fetch(@post.cache_key))
+        assert_equal(new_comment_serializer.attributes, cached_serialization(new_comment_serializer))
+        assert_equal(@post_serializer.attributes, cached_serialization(@post_serializer))
       end
     end
 
@@ -153,7 +150,7 @@ def test_fragment_fetch_with_virtual_associations
       hash = render_object_with_cache(@location)
 
       assert_equal(hash, expected_result)
-      assert_equal({ place: 'Nowhere' }, ActionController::Base.cache_store.fetch(@location.cache_key))
+      assert_equal({ place: 'Nowhere' }, cache_store.fetch(@location.cache_key))
     end
 
     def test_fragment_cache_with_inheritance
@@ -166,11 +163,11 @@ def test_fragment_cache_with_inheritance
 
     def test_uses_file_digest_in_cache_key
       render_object_with_cache(@blog)
-      assert_equal(@blog_serializer.attributes, ActionController::Base.cache_store.fetch(@blog.cache_key_with_digest))
+      assert_equal(@blog_serializer.attributes, cache_store.fetch(@blog.cache_key_with_digest))
     end
 
     def test_cache_digest_definition
-      assert_equal(::Model::FILE_DIGEST, @post_serializer.class._cache_digest)
+      assert_equal(FILE_DIGEST, @post_serializer.class._cache_digest)
     end
 
     def test_object_cache_keys
@@ -257,7 +254,16 @@ def test_warn_on_serializer_not_defined_in_file
     private
 
     def render_object_with_cache(obj, options = {})
-      SerializableResource.new(obj, options).serializable_hash
+      serializable(obj, options).serializable_hash
+    end
+
+    def cache_store
+      ActiveModelSerializers.config.cache_store
+    end
+
+    def cached_serialization(serializer)
+      cache_key = CachedSerializer.new(serializer).cache_key
+      cache_store.fetch(cache_key)
     end
   end
 end

test/fixtures/poro.rb

@@ -1,8 +1,6 @@
 verbose = $VERBOSE
 $VERBOSE = nil
 class Model < ActiveModelSerializers::Model
-  FILE_DIGEST = Digest::MD5.hexdigest(File.open(__FILE__).read)
-
   ### Helper methods, not required to be serializable
 
   # Convenience when not adding @attributes readers and writers
@@ -21,10 +19,6 @@ def method_missing(meth, *args)
   def respond_to_missing?(method_name, _include_private = false)
     attributes.key?(method_name.to_s.tr('=', '').to_sym) || super
   end
-
-  def cache_key_with_digest
-    "#{cache_key}/#{FILE_DIGEST}"
-  end
 end
 
 # see
@@ -58,7 +52,13 @@ class ProfilePreviewSerializer < ActiveModel::Serializer
 Like     = Class.new(Model)
 Author   = Class.new(Model)
 Bio      = Class.new(Model)
-Blog     = Class.new(Model)
+Blog     = Class.new(Model) do
+  FILE_DIGEST = Digest::MD5.hexdigest(File.open(__FILE__).read)
+
+  def cache_key_with_digest
+    "#{cache_key}/#{FILE_DIGEST}"
+  end
+end
 Role     = Class.new(Model)
 User     = Class.new(Model)
 Location = Class.new(Model)

test/serializers/caching_configuration_test_isolated.rb

@@ -37,6 +37,7 @@ class PerformCachingTrue < CachingConfigurationTest
         app.config.action_controller.perform_caching = true
         app.config.action_controller.cache_store = ActiveSupport::Cache.lookup_store(:memory_store)
       end
+      controller_cache_store # Force ActiveSupport.on_load(:action_controller) to run
     end
 
     test 'it sets perform_caching to true on AMS.config and serializers' do
@@ -103,6 +104,7 @@ class PerformCachingFalse < CachingConfigurationTest
         app.config.action_controller.perform_caching = false
         app.config.action_controller.cache_store = ActiveSupport::Cache.lookup_store(:memory_store)
       end
+      controller_cache_store # Force ActiveSupport.on_load(:action_controller) to run
     end
 
     test 'it sets perform_caching to false on AMS.config and serializers' do

test/support/rails_app.rb

@@ -5,7 +5,7 @@ module ActiveModelSerializers
       config.secret_key_base = 'abc123'
       config.active_support.test_order = :random
       config.action_controller.perform_caching = true
-      ActionController::Base.cache_store = :memory_store
+      config.action_controller.cache_store = :memory_store
     end
 
     app.routes.default_url_options = { host: 'example.com' }