When caching, return the object's cache_key up front if it's defined.…

… This will prevent objects PORO objects that don't have updated_at defined, from throwing an error. Not as big a deal now that PORO objects can inherit ActiveModelSerializers::Model, but still necessary if it's not inherited for whatever reason.

Add the Adapter type to the cache key.  This prevents incorrect results when the same object is serialized with different adapters.

lib/active_model/serializer/adapter/base.rb

@@ -7,13 +7,21 @@ def self.inherited(subclass)
           ActiveModel::Serializer::Adapter.register(subclass)
         end
 
+        def self.name
+          to_s.demodulize
+        end
+
         attr_reader :serializer, :instance_options
 
         def initialize(serializer, options = {})
-          @serializer = serializer
+          @serializer       = serializer
           @instance_options = options
         end
 
+        def name
+          self.class.name
+        end
+
         def serializable_hash(_options = nil)
           fail NotImplementedError, 'This is an abstract method. Should be implemented at the concrete adapter.'
         end

lib/active_model/serializer/adapter/cached_serializer.rb

@@ -2,14 +2,18 @@ module ActiveModel
   class Serializer
     module Adapter
       class CachedSerializer
+        UndefinedCacheKey = Class.new(StandardError)
+
         def initialize(serializer)
           @cached_serializer = serializer
           @klass             = @cached_serializer.class
+          return unless cached? && !@cached_serializer.object.respond_to?(:cache_key) && @klass._cache_key.blank?
+          fail(UndefinedCacheKey, "#{@cached_serializer.object} must define #cache_key, or the cache_key option must be passed into cache on #{@cached_serializer}")
         end
 
         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,18 +31,13 @@ def fragment_cached?
           @klass._cache_only && !@klass._cache_except || !@klass._cache_only && @klass._cache_except
         end
 
-        def cache_key
+        def cache_key(adapter_instance)
           parts = []
-          parts << object_cache_key
-          parts << @klass._cache_digest unless @klass._cache_options && @klass._cache_options[:skip_digest]
+          parts << @cached_serializer.cache_key
+          parts << adapter_instance.name.underscore
+          parts << @klass._cache_digest unless @klass._skip_digest?
           parts.join('/')
         end
-
-        def object_cache_key
-          object_time_safe = @cached_serializer.object.updated_at
-          object_time_safe = object_time_safe.strftime('%Y%m%d%H%M%S%9N') if object_time_safe.respond_to?(:strftime)
-          (@klass._cache_key) ? "#{@klass._cache_key}/#{@cached_serializer.object.id}-#{object_time_safe}" : @cached_serializer.object.cache_key
-        end
       end
     end
   end

lib/active_model/serializer/adapter/json.rb

@@ -3,7 +3,7 @@ class Serializer
     module Adapter
       class Json < Base
         extend ActiveSupport::Autoload
-        autoload :FragmentCache
+        autoload :FragmentCache, File.expand_path(File.dirname(__FILE__) + '/json/fragment_cache')
 
         def serializable_hash(options = nil)
           options ||= {}

lib/active_model/serializer/adapter/json_api.rb

@@ -3,9 +3,9 @@ class Serializer
     module Adapter
       class JsonApi < Base
         extend ActiveSupport::Autoload
-        autoload :PaginationLinks
-        autoload :FragmentCache
-        autoload :Link
+        autoload :PaginationLinks, File.expand_path(File.dirname(__FILE__) + '/json_api/pagination_links')
+        autoload :FragmentCache, File.expand_path(File.dirname(__FILE__) + '/json_api/fragment_cache')
+        autoload :Link, File.expand_path(File.dirname(__FILE__) + '/json_api/link')
 
         # TODO: if we like this abstraction and other API objects to it,
         # then extract to its own file and require it.

lib/active_model/serializer/caching.rb

@@ -7,7 +7,7 @@ module Caching
         with_options instance_writer: false, instance_reader: false do |serializer|
           serializer.class_attribute :_cache         # @api private : the cache object
           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_key     # @api private : when present, is first item in cache_key.  Ignored if the serializable object defines #cache_key.
           serializer.class_attribute :_cache_only    # @api private : when fragment caching, whitelists cached_attributes. Cannot combine with except
           serializer.class_attribute :_cache_except  # @api private : when fragment caching, blacklists cached_attributes. Cannot combine with only
           serializer.class_attribute :_cache_options # @api private : used by CachedSerializer, passed to _cache.fetch
@@ -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.
@@ -95,6 +99,18 @@ def cache(options = {})
           self._cache_options = (options.empty?) ? nil : options
         end
       end
+
+      # Use object's cache_key if available, else derive a key from the object
+      # Pass the `key` option to the `cache` declaration or override this method to customize the cache key
+      def cache_key
+        if object.respond_to?(:cache_key)
+          object.cache_key
+        else
+          object_time_safe = object.updated_at
+          object_time_safe = object_time_safe.strftime('%Y%m%d%H%M%S%9N') if object_time_safe.respond_to?(:strftime)
+          "#{self.class._cache_key}/#{object.id}-#{object_time_safe}"
+        end
+      end
     end
   end
 end

test/fixtures/poro.rb

@@ -16,15 +16,15 @@ def method_missing(meth, *args)
     end
   end
 
+  def digest
+    FILE_DIGEST
+  end
+
   # required for ActiveModel::AttributeAssignment#_assign_attribute
   # in Rails 5
   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
 
 class Profile < Model

test/serializers/cache_test.rb

@@ -6,6 +6,21 @@ class Serializer
     class CacheTest < Minitest::Test
       include ActiveSupport::Testing::Stream
 
+      UncachedAuthor = Class.new(Author) do
+        # To confirm cache_key is set using updated_at and cache_key option passed to cache
+        undef_method :cache_key
+      end
+
+      Article = Class.new(::Model) do
+        # To confirm error is raised when cache_key is not set and cache_key option not passed to cache
+        undef_method :cache_key
+      end
+
+      ArticleSerializer = Class.new(ActiveModel::Serializer) do
+        cache only: [:place], skip_digest: true
+        attributes :title
+      end
+
       def setup
         ActionController::Base.cache_store.clear
         @comment        = Comment.new(id: 1, body: 'ZOMG A COMMENT')
@@ -65,15 +80,25 @@ def test_cache_key_definition
         assert_equal(nil, @comment_serializer.class._cache_key)
       end
 
-      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)
+      def test_error_is_raised_if_cache_key_is_not_defined_on_object_or_passed_as_cache_option
+        article = Article.new(title: 'Must Read')
+        assert_raises ActiveModel::Serializer::Adapter::CachedSerializer::UndefinedCacheKey do
+          render_object_with_cache(article)
+        end
+      end
+
+      def test_cache_key_interpolation_with_updated_at_when_cache_key_is_not_defined_on_object
+        uncached_author            = UncachedAuthor.new(name: 'Joao M. D. Moura')
+        uncached_author_serializer = AuthorSerializer.new(uncached_author)
+
+        render_object_with_cache(uncached_author)
+        key = cache_key_with_adapter("#{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")}")
+        assert_equal(uncached_author_serializer.attributes.to_json, ActionController::Base.cache_store.fetch(key).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, ActionController::Base.cache_store.fetch(cache_key_with_adapter(@comment.cache_key)).to_json)
       end
 
       def test_cache_options_definition
@@ -95,8 +120,8 @@ def test_associations_separately_cache
         Timecop.freeze(Time.now) 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, ActionController::Base.cache_store.fetch(cache_key_with_adapter(@post.cache_key)))
+          assert_equal(@comment_serializer.attributes, ActionController::Base.cache_store.fetch(cache_key_with_adapter(@comment.cache_key)))
         end
       end
 
@@ -109,8 +134,8 @@ def test_associations_cache_when_updated
           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, ActionController::Base.cache_store.fetch(cache_key_with_adapter(@post.cache_key)))
+          assert_equal(@comment_serializer.attributes, ActionController::Base.cache_store.fetch(cache_key_with_adapter(@comment.cache_key)))
 
           # Simulating update on comments relationship with Post
           new_comment            = Comment.new(id: 2, body: 'ZOMG A NEW COMMENT')
@@ -121,8 +146,8 @@ def test_associations_cache_when_updated
           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, ActionController::Base.cache_store.fetch(cache_key_with_adapter(new_comment.cache_key)))
+          assert_equal(@post_serializer.attributes, ActionController::Base.cache_store.fetch(cache_key_with_adapter(@post.cache_key)))
         end
       end
 
@@ -137,12 +162,17 @@ 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' }, ActionController::Base.cache_store.fetch(cache_key_with_adapter(@location.cache_key)))
+      end
+
+      def test_uses_adapter_in_cache_key
+        render_object_with_cache(@post)
+        assert_equal(@post_serializer.attributes, ActionController::Base.cache_store.fetch("#{@post.cache_key}/#{adapter.class.to_s.demodulize.underscore}"))
       end
 
       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, ActionController::Base.cache_store.fetch("#{cache_key_with_adapter(@blog.cache_key)}/#{@blog.class::FILE_DIGEST}"))
       end
 
       def test_cache_digest_definition
@@ -201,7 +231,16 @@ def test_warn_on_serializer_not_defined_in_file
       private
 
       def render_object_with_cache(obj)
-        ActiveModel::SerializableResource.new(obj).serializable_hash
+        @serializable_resource = ActiveModel::SerializableResource.new(obj)
+        @serializable_resource.serializable_hash
+      end
+
+      def adapter
+        @serializable_resource.adapter
+      end
+
+      def cache_key_with_adapter(key)
+        "#{key}/#{adapter.name.underscore}"
       end
     end
   end