Merge pull request #1644 from bf4/kevintyll-master-cache_serializers_by_adapter

[FIX] Include adapter in cache key so that serialization is per adapter

Author: bf4

CHANGELOG.md

@@ -3,6 +3,8 @@
 Breaking changes:
 
 Features:
+- [#1644](https://github.com/rails-api/active_model_serializers/pull/1644) Include adapter name in cache key so
+  that the same serializer can be cached per adapter. (@bf4 via #1346 by @kevintyll)
 - [#1642](https://github.com/rails-api/active_model_serializers/pull/1642) Prefer object.cache_key over the generated
   cache key. (@bf4 via #1346 by @kevintyll)
 - [#1637](https://github.com/rails-api/active_model_serializers/pull/1637) Make references to 'ActionController::Base.cache_store' explicit

lib/active_model/serializer/caching.rb

@@ -58,6 +58,10 @@ def digest_caller_file(caller_line)
           ''.freeze
         end
 
+        def _skip_digest?
+          _cache_options && _cache_options[:skip_digest]
+        end
+
         # @api private
         # Used by FragmentCache on the CachedSerializer
         #  to call attribute methods on the fragmented cached serializer.

lib/active_model_serializers/adapter/attributes.rb

@@ -30,7 +30,7 @@ def serializable_hash_for_collection(options)
       def cache_read_multi
         return {} if ActiveModelSerializers.config.cache_store.blank?
 
-        keys = CachedSerializer.object_cache_keys(serializer, @include_tree)
+        keys = CachedSerializer.object_cache_keys(serializer, self, @include_tree)
 
         return {} if keys.blank?
 
@@ -49,7 +49,7 @@ def cache_attributes
       def cached_attributes(cached_serializer)
         return yield unless cached_serializer.cached?
 
-        @cached_attributes.fetch(cached_serializer.cache_key) { yield }
+        @cached_attributes.fetch(cached_serializer.cache_key(self)) { yield }
       end
 
       def serializable_hash_for_single_resource(options)

lib/active_model_serializers/adapter/base.rb

@@ -15,6 +15,10 @@ def initialize(serializer, options = {})
         @instance_options = options
       end
 
+      def cached_name
+        @cached_name ||= self.class.name.demodulize.underscore
+      end
+
       def serializable_hash(_options = nil)
         fail NotImplementedError, 'This is an abstract method. Should be implemented at the concrete adapter.'
       end

lib/active_model_serializers/cached_serializer.rb

@@ -9,7 +9,7 @@ def initialize(serializer)
 
     def cache_check(adapter_instance)
       if cached?
-        @klass._cache.fetch(cache_key, @klass._cache_options) do
+        @klass._cache.fetch(cache_key(adapter_instance), @klass._cache_options) do
           yield
         end
       elsif fragment_cached?
@@ -27,12 +27,13 @@ def fragment_cached?
       @klass.fragment_cache_enabled?
     end
 
-    def cache_key
+    def cache_key(adapter_instance)
       return @cache_key if defined?(@cache_key)
 
       parts = []
       parts << object_cache_key
-      parts << @klass._cache_digest unless @klass._cache_options && @klass._cache_options[:skip_digest]
+      parts << adapter_instance.cached_name
+      parts << @klass._cache_digest unless @klass._skip_digest?
       @cache_key = parts.join('/')
     end
 
@@ -54,19 +55,19 @@ def object_cache_key
     # @param collection_serializer
     # @param include_tree
     # @return [Array] all cache_key of collection_serializer
-    def self.object_cache_keys(serializers, include_tree)
+    def self.object_cache_keys(serializers, adapter_instance, include_tree)
       cache_keys = []
 
       serializers.each do |serializer|
-        cache_keys << object_cache_key(serializer)
+        cache_keys << object_cache_key(serializer, adapter_instance)
 
         serializer.associations(include_tree).each do |association|
           if association.serializer.respond_to?(:each)
             association.serializer.each do |sub_serializer|
-              cache_keys << object_cache_key(sub_serializer)
+              cache_keys << object_cache_key(sub_serializer, adapter_instance)
             end
           else
-            cache_keys << object_cache_key(association.serializer)
+            cache_keys << object_cache_key(association.serializer, adapter_instance)
           end
         end
       end
@@ -75,11 +76,11 @@ def self.object_cache_keys(serializers, include_tree)
     end
 
     # @return [String, nil] the cache_key of the serializer or nil
-    def self.object_cache_key(serializer)
+    def self.object_cache_key(serializer, adapter_instance)
       return unless serializer.present? && serializer.object.present?
 
       cached_serializer = new(serializer)
-      cached_serializer.cached? ? cached_serializer.cache_key : nil
+      cached_serializer.cached? ? cached_serializer.cache_key(adapter_instance) : nil
     end
   end
 end

test/cache_test.rb

@@ -102,12 +102,13 @@ def test_cache_key_interpolation_with_updated_at_when_cache_key_is_not_defined_o
 
       render_object_with_cache(uncached_author)
       key = "#{uncached_author_serializer.class._cache_key}/#{uncached_author_serializer.object.id}-#{uncached_author_serializer.object.updated_at.strftime("%Y%m%d%H%M%S%9N")}"
+      key = "#{key}/#{adapter.cached_name}"
       assert_equal(uncached_author_serializer.attributes.to_json, cache_store.fetch(key).to_json)
     end
 
     def test_default_cache_key_fallback
       render_object_with_cache(@comment)
-      key = @comment.cache_key
+      key = "#{@comment.cache_key}/#{adapter.cached_name}"
       assert_equal(@comment_serializer.attributes.to_json, cache_store.fetch(key).to_json)
     end
 
@@ -138,9 +139,9 @@ def test_associations_separately_cache
       Timecop.freeze(Time.current) do
         render_object_with_cache(@post)
 
-        key = @post.cache_key
+        key = "#{@post.cache_key}/#{adapter.cached_name}"
         assert_equal(@post_serializer.attributes, cache_store.fetch(key))
-        key = @comment.cache_key
+        key = "#{@comment.cache_key}/#{adapter.cached_name}"
         assert_equal(@comment_serializer.attributes, cache_store.fetch(key))
       end
     end
@@ -151,9 +152,9 @@ def test_associations_cache_when_updated
         render_object_with_cache(@post)
 
         # Check if it cached the objects separately
-        key = @post.cache_key
+        key = "#{@post.cache_key}/#{adapter.cached_name}"
         assert_equal(@post_serializer.attributes, cache_store.fetch(key))
-        key = @comment.cache_key
+        key = "#{@comment.cache_key}/#{adapter.cached_name}"
         assert_equal(@comment_serializer.attributes, cache_store.fetch(key))
 
         # Simulating update on comments relationship with Post
@@ -165,9 +166,9 @@ def test_associations_cache_when_updated
         render_object_with_cache(@post)
 
         # Check if the the new comment was cached
-        key = new_comment.cache_key
+        key = "#{new_comment.cache_key}/#{adapter.cached_name}"
         assert_equal(new_comment_serializer.attributes, cache_store.fetch(key))
-        key = @post.cache_key
+        key = "#{@post.cache_key}/#{adapter.cached_name}"
         assert_equal(@post_serializer.attributes, cache_store.fetch(key))
       end
     end
@@ -183,7 +184,8 @@ def test_fragment_fetch_with_virtual_associations
       hash = render_object_with_cache(@location)
 
       assert_equal(hash, expected_result)
-      assert_equal({ place: 'Nowhere' }, cache_store.fetch(@location.cache_key))
+      key = "#{@location.cache_key}/#{adapter.cached_name}"
+      assert_equal({ place: 'Nowhere' }, cache_store.fetch(key))
     end
 
     def test_fragment_cache_with_inheritance
@@ -194,9 +196,87 @@ def test_fragment_cache_with_inheritance
       refute_includes(base.keys, :special_attribute)
     end
 
+    def test_uses_adapter_in_cache_key
+      render_object_with_cache(@post)
+      key = "#{@post.cache_key}/#{adapter.class.to_s.demodulize.underscore}"
+      assert_equal(@post_serializer.attributes, cache_store.fetch(key))
+    end
+
+    # Based on original failing test by @kevintyll
+    # rubocop:disable Metrics/AbcSize
+    def test_a_serializer_rendered_by_two_adapter_returns_differently_cached_attributes
+      Object.const_set(:Alert, Class.new(ActiveModelSerializers::Model) do
+        attr_accessor :id, :status, :resource, :started_at, :ended_at, :updated_at, :created_at
+      end)
+      Object.const_set(:UncachedAlertSerializer, Class.new(ActiveModel::Serializer) do
+        attributes :id, :status, :resource, :started_at, :ended_at, :updated_at, :created_at
+      end)
+      Object.const_set(:AlertSerializer, Class.new(UncachedAlertSerializer) do
+        cache
+      end)
+
+      alert = Alert.new(
+        id: 1,
+        status: 'fail',
+        resource: 'resource-1',
+        started_at: Time.new(2016, 3, 31, 21, 36, 35, 0),
+        ended_at: nil,
+        updated_at: Time.new(2016, 3, 31, 21, 27, 35, 0),
+        created_at: Time.new(2016, 3, 31, 21, 37, 35, 0)
+      )
+
+      expected_cached_attributes = {
+        id: 1,
+        status: 'fail',
+        resource: 'resource-1',
+        started_at: alert.started_at,
+        ended_at: nil,
+        updated_at: alert.updated_at,
+        created_at: alert.created_at
+      }
+      expected_cached_jsonapi_attributes = {
+        id: '1',
+        type: 'alerts',
+        attributes: {
+          status: 'fail',
+          resource: 'resource-1',
+          started_at: alert.started_at,
+          ended_at: nil,
+          updated_at: alert.updated_at,
+          created_at: alert.created_at
+        }
+      }
+
+      # Assert attributes are serialized correctly
+      serializable_alert = serializable(alert, serializer: AlertSerializer, adapter: :attributes)
+      attributes_serialization = serializable_alert.as_json
+      assert_equal expected_cached_attributes, alert.attributes
+      assert_equal alert.attributes, attributes_serialization
+      attributes_cache_key = CachedSerializer.new(serializable_alert.adapter.serializer).cache_key(serializable_alert.adapter)
+      assert_equal attributes_serialization, cache_store.fetch(attributes_cache_key)
+
+      serializable_alert = serializable(alert, serializer: AlertSerializer, adapter: :json_api)
+      jsonapi_cache_key = CachedSerializer.new(serializable_alert.adapter.serializer).cache_key(serializable_alert.adapter)
+      # Assert cache keys differ
+      refute_equal attributes_cache_key, jsonapi_cache_key
+      # Assert (cached) serializations differ
+      jsonapi_serialization = serializable_alert.as_json
+      assert_equal alert.status, jsonapi_serialization.fetch(:data).fetch(:attributes).fetch(:status)
+      serializable_alert = serializable(alert, serializer: UncachedAlertSerializer, adapter: :json_api)
+      assert_equal serializable_alert.as_json, jsonapi_serialization
+
+      cached_serialization = cache_store.fetch(jsonapi_cache_key)
+      assert_equal expected_cached_jsonapi_attributes, cached_serialization
+    ensure
+      Object.send(:remove_const, :Alert)
+      Object.send(:remove_const, :AlertSerializer)
+      Object.send(:remove_const, :UncachedAlertSerializer)
+    end
+    # rubocop:enable Metrics/AbcSize
+
     def test_uses_file_digest_in_cache_key
       render_object_with_cache(@blog)
-      key = "#{@blog.cache_key}/#{::Model::FILE_DIGEST}"
+      key = "#{@blog.cache_key}/#{adapter.cached_name}/#{::Model::FILE_DIGEST}"
       assert_equal(@blog_serializer.attributes, cache_store.fetch(key))
     end
 
@@ -205,13 +285,13 @@ def test_cache_digest_definition
     end
 
     def test_object_cache_keys
-      serializer = ActiveModel::Serializer::CollectionSerializer.new([@comment, @comment])
+      serializable = ActiveModelSerializers::SerializableResource.new([@comment, @comment])
       include_tree = ActiveModel::Serializer::IncludeTree.from_include_args('*')
 
-      actual = CachedSerializer.object_cache_keys(serializer, include_tree)
+      actual = CachedSerializer.object_cache_keys(serializable.adapter.serializer, serializable.adapter, include_tree)
 
       assert_equal actual.size, 3
-      assert actual.any? { |key| key == 'comment/1' }
+      assert actual.any? { |key| key == "comment/1/#{serializable.adapter.cached_name}" }
       assert actual.any? { |key| key =~ %r{post/post-\d+} }
       assert actual.any? { |key| key =~ %r{author/author-\d+} }
     end
@@ -226,13 +306,13 @@ def test_cached_attributes
         attributes.send(:cache_attributes)
         cached_attributes = attributes.instance_variable_get(:@cached_attributes)
 
-        assert_equal cached_attributes[@comment.cache_key], Comment.new(id: 1, body: 'ZOMG A COMMENT').attributes
-        assert_equal cached_attributes[@comment.post.cache_key], Post.new(id: 'post', title: 'New Post', body: 'Body').attributes
+        assert_equal cached_attributes["#{@comment.cache_key}/#{attributes.cached_name}"], Comment.new(id: 1, body: 'ZOMG A COMMENT').attributes
+        assert_equal cached_attributes["#{@comment.post.cache_key}/#{attributes.cached_name}"], Post.new(id: 'post', title: 'New Post', body: 'Body').attributes
 
         writer = @comment.post.blog.writer
         writer_cache_key = writer.cache_key
 
-        assert_equal cached_attributes[writer_cache_key], Author.new(id: 'author', name: 'Joao M. D. Moura').attributes
+        assert_equal cached_attributes["#{writer_cache_key}/#{attributes.cached_name}"], Author.new(id: 'author', name: 'Joao M. D. Moura').attributes
       end
     end
 
@@ -287,16 +367,21 @@ def test_warn_on_serializer_not_defined_in_file
 
     private
 
+    def cache_store
+      ActiveModelSerializers.config.cache_store
+    end
+
     def render_object_with_cache(obj, options = {})
-      serializable(obj, options).serializable_hash
+      @serializable_resource = serializable(obj, options)
+      @serializable_resource.serializable_hash
     end
 
-    def cache_store
-      ActiveModelSerializers.config.cache_store
+    def adapter
+      @serializable_resource.adapter
     end
 
     def cached_serialization(serializer)
-      cache_key = CachedSerializer.new(serializer).cache_key
+      cache_key = CachedSerializer.new(serializer).cache_key(adapter)
       cache_store.fetch(cache_key)
     end
   end