Merge pull request #1471 from bf4/move_serializer_caching_from_adapter

[Cleanup] Serializer caching is its own concern

CHANGELOG.md

@@ -34,6 +34,7 @@ Fixes:
 - [#1488](https://github.com/rails-api/active_model_serializers/pull/1488) Require ActiveSupport's string inflections (@nate00)
 
 Misc:
+- [#1471](https://github.com/rails-api/active_model_serializers/pull/1471) [Cleanup] Serializer caching is its own concern. (@bf4)
 - [#1482](https://github.com/rails-api/active_model_serializers/pull/1482) Document JSON API implementation defs and progress in class. (@bf4)
 - [#1551](https://github.com/rails-api/active_model_serializers/pull/1551) Added codebeat badge (@korzonek)
 - [#1527](https://github.com/rails-api/active_model_serializers/pull/1527) Refactor fragment cache class. (@groyoh)

lib/active_model/serializer.rb

@@ -48,9 +48,12 @@ module ActiveModel
     # @see ActiveModelSerializers::Adapter.lookup
     # Deprecated
     def self.adapter
-      warn 'Calling adapter method in Serializer, please use the ActiveModelSerializers::Adapter.configured_adapter'
       ActiveModelSerializers::Adapter.lookup(config.adapter)
     end
+    class << self
+      extend ActiveModelSerializers::Deprecate
+      deprecate :adapter, 'ActiveModelSerializers::Adapter.configured_adapter'
+    end
 
     # @api private
     def self.serializer_lookup_chain_for(klass)

lib/active_model/serializer/attributes.rb

@@ -68,7 +68,7 @@ module ActiveModel
         # @api private
         # maps attribute value to explict key name
         # @see Serializer::attribute
-        # @see Adapter::FragmentCache#fragment_serializer
+        # @see FragmentCache#fragment_serializer
         def _attributes_keys
           _attributes_data
             .each_with_object({}) do |(key, attr), hash|

lib/active_model_serializers.rb

@@ -5,6 +5,8 @@ require 'active_support/core_ext/string/inflections'
 module ActiveModelSerializers
   extend ActiveSupport::Autoload
   autoload :Model
+  autoload :CachedSerializer
+  autoload :FragmentCache
   autoload :Callbacks
   autoload :Deserialization
   autoload :Logging

lib/active_model_serializers/adapter.rb

@@ -3,8 +3,6 @@ module ActiveModelSerializers
     UnknownAdapterError = Class.new(ArgumentError)
     ADAPTER_MAP = {} # rubocop:disable Style/MutableConstant
     private_constant :ADAPTER_MAP if defined?(private_constant)
-    require 'active_model_serializers/adapter/fragment_cache'
-    require 'active_model_serializers/adapter/cached_serializer'
 
     class << self # All methods are class functions
       def new(*args)

lib/active_model_serializers/adapter/attributes.rb

@@ -17,10 +17,6 @@ module ActiveModelSerializers
         end
       end
 
-      def fragment_cache(cached_hash, non_cached_hash)
-        Json::FragmentCache.new.fragment_cache(cached_hash, non_cached_hash)
-      end
-
       private
 
       def serializable_hash_for_collection(options)

lib/active_model_serializers/adapter/base.rb

@@ -23,8 +23,8 @@ module ActiveModelSerializers
         hash
       end
 
-      def fragment_cache(*_args)
+      def fragment_cache(cached_hash, non_cached_hash)
-        fail NotImplementedError, 'This is an abstract method. Should be implemented at the concrete adapter.'
+        non_cached_hash.merge cached_hash
       end
 
       def cache_check(serializer)

lib/active_model_serializers/adapter/cached_serializer.rb

@@ -1,77 +0,0 @@
-module ActiveModelSerializers
-  module Adapter
-    class CachedSerializer
-      def initialize(serializer)
-        @cached_serializer = serializer
-        @klass             = @cached_serializer.class
-      end
-
-      def cache_check(adapter_instance)
-        if cached?
-          @klass._cache.fetch(cache_key, @klass._cache_options) do
-            yield
-          end
-        elsif fragment_cached?
-          FragmentCache.new(adapter_instance, @cached_serializer, adapter_instance.instance_options).fetch
-        else
-          yield
-        end
-      end
-
-      def cached?
-        @klass._cache && !@klass._cache_only && !@klass._cache_except
-      end
-
-      def fragment_cached?
-        @klass._cache && (@klass._cache_only && !@klass._cache_except || !@klass._cache_only && @klass._cache_except)
-      end
-
-      def cache_key
-        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]
-        @cache_key = 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
-
-      # find all cache_key for the collection_serializer
-      # @param collection_serializer
-      # @param include_tree
-      # @return [Array] all cache_key of collection_serializer
-      def self.object_cache_keys(serializers, include_tree)
-        cache_keys = []
-
-        serializers.each do |serializer|
-          cache_keys << object_cache_key(serializer)
-
-          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)
-              end
-            else
-              cache_keys << object_cache_key(association.serializer)
-            end
-          end
-        end
-
-        cache_keys.compact.uniq
-      end
-
-      # @return [String, nil] the cache_key of the serializer or nil
-      def self.object_cache_key(serializer)
-        return unless serializer.present? && serializer.object.present?
-
-        cached_serializer = new(serializer)
-        cached_serializer.cached? ? cached_serializer.cache_key : nil
-      end
-    end
-  end
-end

lib/active_model_serializers/adapter/fragment_cache.rb

@@ -1,118 +0,0 @@
-module ActiveModelSerializers
-  module Adapter
-    class FragmentCache
-      attr_reader :serializer
-
-      def initialize(adapter, serializer, options)
-        @instance_options = options
-        @adapter    = adapter
-        @serializer = serializer
-      end
-
-      # 1. Create a CachedSerializer and NonCachedSerializer from the serializer class
-      # 2. Serialize the above two with the given adapter
-      # 3. Pass their serializations to the adapter +::fragment_cache+
-      def fetch
-        object = serializer.object
-
-        # It will split the serializer into two, one that will be cached and one that will not
-        serializers = fragment_serializer(object.class.name)
-
-        # Get serializable hash from both
-        cached_hash     = serialize(object, serializers[:cached])
-        non_cached_hash = serialize(object, serializers[:non_cached])
-
-        # Merge both results
-        adapter.fragment_cache(cached_hash, non_cached_hash)
-      end
-
-      protected
-
-      attr_reader :instance_options, :adapter
-
-      private
-
-      def serialize(object, serializer_class)
-        ActiveModel::SerializableResource.new(
-          object,
-          serializer: serializer_class,
-          adapter: adapter.class
-        ).serializable_hash
-      end
-
-      # Given a hash of its cached and non-cached serializers
-      # 1. Determine cached attributes from serializer class options
-      # 2. Add cached attributes to cached Serializer
-      # 3. Add non-cached attributes to non-cached Serializer
-      def cache_attributes(serializers)
-        klass                 = serializer.class
-        attributes            = klass._attributes
-        cache_only            = klass._cache_only
-        cached_attributes     = cache_only ? cache_only : attributes - klass._cache_except
-        non_cached_attributes = attributes - cached_attributes
-        attributes_keys       = klass._attributes_keys
-
-        add_attributes_to_serializer(serializers[:cached], cached_attributes, attributes_keys)
-        add_attributes_to_serializer(serializers[:non_cached], non_cached_attributes, attributes_keys)
-      end
-
-      def add_attributes_to_serializer(serializer, attributes, attributes_keys)
-        attributes.each do |attribute|
-          options = attributes_keys[attribute] || {}
-          serializer.attribute(attribute, options)
-        end
-      end
-
-      # Given a resource name
-      # 1. Dyanmically creates a CachedSerializer and NonCachedSerializer
-      #   for a given class 'name'
-      # 2. Call
-      #       CachedSerializer.cache(serializer._cache_options)
-      #       CachedSerializer.fragmented(serializer)
-      #       NontCachedSerializer.cache(serializer._cache_options)
-      # 3. Build a hash keyed to the +cached+ and +non_cached+ serializers
-      # 4. Call +cached_attributes+ on the serializer class and the above hash
-      # 5. Return the hash
-      #
-      # @example
-      #   When +name+ is <tt>User::Admin</tt>
-      #   creates the Serializer classes (if they don't exist).
-      #     User_AdminCachedSerializer
-      #     User_AdminNOnCachedSerializer
-      #
-      def fragment_serializer(name)
-        klass      = serializer.class
-        cached     = "#{to_valid_const_name(name)}CachedSerializer"
-        non_cached = "#{to_valid_const_name(name)}NonCachedSerializer"
-
-        cached_serializer     = get_or_create_serializer(cached)
-        non_cached_serializer = get_or_create_serializer(non_cached)
-
-        klass._cache_options ||= {}
-        cache_key = klass._cache_key
-        klass._cache_options[:key] = cache_key if cache_key
-        cached_serializer.cache(klass._cache_options)
-
-        type = klass._type
-        cached_serializer.type(type)
-        non_cached_serializer.type(type)
-
-        non_cached_serializer.fragmented(serializer)
-        cached_serializer.fragmented(serializer)
-
-        serializers = { cached: cached_serializer, non_cached: non_cached_serializer }
-        cache_attributes(serializers)
-        serializers
-      end
-
-      def get_or_create_serializer(name)
-        return Object.const_get(name) if Object.const_defined?(name)
-        Object.const_set(name, Class.new(ActiveModel::Serializer))
-      end
-
-      def to_valid_const_name(name)
-        name.gsub('::', '_')
-      end
-    end
-  end
-end

lib/active_model_serializers/adapter/json.rb

@@ -1,19 +1,10 @@
 module ActiveModelSerializers
   module Adapter
     class Json < Base
-      extend ActiveSupport::Autoload
-      autoload :FragmentCache
-
       def serializable_hash(options = nil)
         options ||= {}
         { root => Attributes.new(serializer, instance_options).serializable_hash(options) }
       end
-
-      private
-
-      def fragment_cache(cached_hash, non_cached_hash)
-        ActiveModelSerializers::Adapter::Json::FragmentCache.new.fragment_cache(cached_hash, non_cached_hash)
-      end
     end
   end
 end

lib/active_model_serializers/adapter/json/fragment_cache.rb

@@ -1,11 +0,0 @@
-module ActiveModelSerializers
-  module Adapter
-    class Json
-      class FragmentCache
-        def fragment_cache(cached_hash, non_cached_hash)
-          non_cached_hash.merge cached_hash
-        end
-      end
-    end
-  end
-end

lib/active_model_serializers/adapter/json_api.rb

@@ -22,7 +22,6 @@ module ActiveModelSerializers
   module Adapter
     class JsonApi < Base
       extend ActiveSupport::Autoload
-      autoload :FragmentCache
       autoload :Jsonapi
       autoload :ResourceIdentifier
       autoload :Relationship
@@ -169,7 +168,14 @@ module ActiveModelSerializers
 
       def fragment_cache(cached_hash, non_cached_hash)
         root = false if instance_options.include?(:include)
-        FragmentCache.new.fragment_cache(root, cached_hash, non_cached_hash)
+        core_cached       = cached_hash.first
+        core_non_cached   = non_cached_hash.first
+        no_root_cache     = cached_hash.delete_if { |key, _value| key == core_cached[0] }
+        no_root_non_cache = non_cached_hash.delete_if { |key, _value| key == core_non_cached[0] }
+        cached_resource   = (core_cached[1]) ? core_cached[1].deep_merge(core_non_cached[1]) : core_non_cached[1]
+        hash = root ? { root => cached_resource } : cached_resource
+
+        hash.deep_merge no_root_non_cache.deep_merge no_root_cache
       end
 
       protected

lib/active_model_serializers/adapter/json_api/fragment_cache.rb

@@ -1,18 +0,0 @@
-module ActiveModelSerializers
-  module Adapter
-    class JsonApi
-      class FragmentCache
-        def fragment_cache(root, cached_hash, non_cached_hash)
-          core_cached       = cached_hash.first
-          core_non_cached   = non_cached_hash.first
-          no_root_cache     = cached_hash.delete_if { |key, _value| key == core_cached[0] }
-          no_root_non_cache = non_cached_hash.delete_if { |key, _value| key == core_non_cached[0] }
-          cached_resource   = (core_cached[1]) ? core_cached[1].deep_merge(core_non_cached[1]) : core_non_cached[1]
-          hash = root ? { root => cached_resource } : cached_resource
-
-          hash.deep_merge no_root_non_cache.deep_merge no_root_cache
-        end
-      end
-    end
-  end
-end

lib/active_model_serializers/cached_serializer.rb

@@ -0,0 +1,75 @@
+module ActiveModelSerializers
+  class CachedSerializer
+    def initialize(serializer)
+      @cached_serializer = serializer
+      @klass             = @cached_serializer.class
+    end
+
+    def cache_check(adapter_instance)
+      if cached?
+        @klass._cache.fetch(cache_key, @klass._cache_options) do
+          yield
+        end
+      elsif fragment_cached?
+        FragmentCache.new(adapter_instance, @cached_serializer, adapter_instance.instance_options).fetch
+      else
+        yield
+      end
+    end
+
+    def cached?
+      @klass._cache && !@klass._cache_only && !@klass._cache_except
+    end
+
+    def fragment_cached?
+      @klass._cache && (@klass._cache_only && !@klass._cache_except || !@klass._cache_only && @klass._cache_except)
+    end
+
+    def cache_key
+      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]
+      @cache_key = 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
+
+    # find all cache_key for the collection_serializer
+    # @param collection_serializer
+    # @param include_tree
+    # @return [Array] all cache_key of collection_serializer
+    def self.object_cache_keys(serializers, include_tree)
+      cache_keys = []
+
+      serializers.each do |serializer|
+        cache_keys << object_cache_key(serializer)
+
+        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)
+            end
+          else
+            cache_keys << object_cache_key(association.serializer)
+          end
+        end
+      end
+
+      cache_keys.compact.uniq
+    end
+
+    # @return [String, nil] the cache_key of the serializer or nil
+    def self.object_cache_key(serializer)
+      return unless serializer.present? && serializer.object.present?
+
+      cached_serializer = new(serializer)
+      cached_serializer.cached? ? cached_serializer.cache_key : nil
+    end
+  end
+end

lib/active_model_serializers/fragment_cache.rb

@@ -0,0 +1,116 @@
+module ActiveModelSerializers
+  class FragmentCache
+    attr_reader :serializer
+
+    def initialize(adapter, serializer, options)
+      @instance_options = options
+      @adapter    = adapter
+      @serializer = serializer
+    end
+
+    # 1. Create a CachedSerializer and NonCachedSerializer from the serializer class
+    # 2. Serialize the above two with the given adapter
+    # 3. Pass their serializations to the adapter +::fragment_cache+
+    def fetch
+      object = serializer.object
+
+      # It will split the serializer into two, one that will be cached and one that will not
+      serializers = fragment_serializer(object.class.name)
+
+      # Get serializable hash from both
+      cached_hash     = serialize(object, serializers[:cached])
+      non_cached_hash = serialize(object, serializers[:non_cached])
+
+      # Merge both results
+      adapter.fragment_cache(cached_hash, non_cached_hash)
+    end
+
+    protected
+
+    attr_reader :instance_options, :adapter
+
+    private
+
+    def serialize(object, serializer_class)
+      ActiveModel::SerializableResource.new(
+        object,
+        serializer: serializer_class,
+        adapter: adapter.class
+      ).serializable_hash
+    end
+
+    # Given a hash of its cached and non-cached serializers
+    # 1. Determine cached attributes from serializer class options
+    # 2. Add cached attributes to cached Serializer
+    # 3. Add non-cached attributes to non-cached Serializer
+    def cache_attributes(serializers)
+      klass                 = serializer.class
+      attributes            = klass._attributes
+      cache_only            = klass._cache_only
+      cached_attributes     = cache_only ? cache_only : attributes - klass._cache_except
+      non_cached_attributes = attributes - cached_attributes
+      attributes_keys       = klass._attributes_keys
+
+      add_attributes_to_serializer(serializers[:cached], cached_attributes, attributes_keys)
+      add_attributes_to_serializer(serializers[:non_cached], non_cached_attributes, attributes_keys)
+    end
+
+    def add_attributes_to_serializer(serializer, attributes, attributes_keys)
+      attributes.each do |attribute|
+        options = attributes_keys[attribute] || {}
+        serializer.attribute(attribute, options)
+      end
+    end
+
+    # Given a resource name
+    # 1. Dynamically creates a CachedSerializer and NonCachedSerializer
+    #   for a given class 'name'
+    # 2. Call
+    #       CachedSerializer.cache(serializer._cache_options)
+    #       CachedSerializer.fragmented(serializer)
+    #       NonCachedSerializer.cache(serializer._cache_options)
+    # 3. Build a hash keyed to the +cached+ and +non_cached+ serializers
+    # 4. Call +cached_attributes+ on the serializer class and the above hash
+    # 5. Return the hash
+    #
+    # @example
+    #   When +name+ is <tt>User::Admin</tt>
+    #   creates the Serializer classes (if they don't exist).
+    #     User_AdminCachedSerializer
+    #     User_AdminNonCachedSerializer
+    #
+    def fragment_serializer(name)
+      klass      = serializer.class
+      cached     = "#{to_valid_const_name(name)}CachedSerializer"
+      non_cached = "#{to_valid_const_name(name)}NonCachedSerializer"
+
+      cached_serializer     = get_or_create_serializer(cached)
+      non_cached_serializer = get_or_create_serializer(non_cached)
+
+      klass._cache_options ||= {}
+      cache_key = klass._cache_key
+      klass._cache_options[:key] = cache_key if cache_key
+      cached_serializer.cache(klass._cache_options)
+
+      type = klass._type
+      cached_serializer.type(type)
+      non_cached_serializer.type(type)
+
+      non_cached_serializer.fragmented(serializer)
+      cached_serializer.fragmented(serializer)
+
+      serializers = { cached: cached_serializer, non_cached: non_cached_serializer }
+      cache_attributes(serializers)
+      serializers
+    end
+
+    def get_or_create_serializer(name)
+      return Object.const_get(name) if Object.const_defined?(name)
+      Object.const_set(name, Class.new(ActiveModel::Serializer))
+    end
+
+    def to_valid_const_name(name)
+      name.gsub('::', '_')
+    end
+  end
+end

test/active_model_serializers/fragment_cache_test.rb

@@ -0,0 +1,34 @@
+require 'test_helper'
+module ActiveModelSerializers
+  class FragmentCacheTest < ActiveSupport::TestCase
+    def setup
+      super
+      @spam            = Spam::UnrelatedLink.new(id: 'spam-id-1')
+      @author          = Author.new(name: 'Joao M. D. Moura')
+      @role            = Role.new(name: 'Great Author', description: nil)
+      @role.author     = [@author]
+      @role_serializer = RoleSerializer.new(@role)
+      @spam_serializer = Spam::UnrelatedLinkSerializer.new(@spam)
+      adapter          = ActiveModelSerializers::Adapter.configured_adapter
+      @role_hash       = FragmentCache.new(adapter.new(@role_serializer), @role_serializer, {})
+      @spam_hash       = FragmentCache.new(adapter.new(@spam_serializer), @spam_serializer, {})
+    end
+
+    def test_fragment_fetch_with_virtual_attributes
+      expected_result = {
+        id: @role.id,
+        description: @role.description,
+        slug: "#{@role.name}-#{@role.id}",
+        name: @role.name
+      }
+      assert_equal(@role_hash.fetch, expected_result)
+    end
+
+    def test_fragment_fetch_with_namespaced_object
+      expected_result = {
+        id: @spam.id
+      }
+      assert_equal(@spam_hash.fetch, expected_result)
+    end
+  end
+end

test/adapter/fragment_cache_test.rb

@@ -1,48 +0,0 @@
-require 'test_helper'
-module ActiveModelSerializers
-  module Adapter
-    class FragmentCacheTest < ActiveSupport::TestCase
-      TypedRoleSerializer = Class.new(ActiveModel::Serializer) do
-        type 'my-roles'
-        cache only: [:name], skip_digest: true
-        attributes :id, :name, :description
-
-        belongs_to :author
-      end
-
-      def setup
-        super
-        @spam            = Spam::UnrelatedLink.new(id: 'spam-id-1')
-        @author          = Author.new(name: 'Joao M. D. Moura')
-        @role            = Role.new(name: 'Great Author', description: nil)
-        @role.author     = [@author]
-        @role_serializer = RoleSerializer.new(@role)
-        @spam_serializer = Spam::UnrelatedLinkSerializer.new(@spam)
-        @role_hash       = FragmentCache.new(::ActiveModelSerializers::Adapter.configured_adapter.new(@role_serializer), @role_serializer, {})
-        @spam_hash       = FragmentCache.new(::ActiveModelSerializers::Adapter.configured_adapter.new(@spam_serializer), @spam_serializer, {})
-      end
-
-      def test_fragment_fetch_with_virtual_attributes
-        expected_result = {
-          id: @role.id,
-          description: @role.description,
-          slug: "#{@role.name}-#{@role.id}",
-          name: @role.name
-        }
-        assert_equal(@role_hash.fetch, expected_result)
-      end
-
-      def test_fragment_fetch_with_namespaced_object
-        expected_result = {
-          id: @spam.id
-        }
-        assert_equal(@spam_hash.fetch, expected_result)
-      end
-
-      def test_fragment_fetch_with_type_override
-        serialization = serializable(Role.new(name: 'Another Author'), serializer: TypedRoleSerializer, adapter: :json_api).serializable_hash
-        assert_equal(TypedRoleSerializer._type, serialization.fetch(:data).fetch(:type))
-      end
-    end
-  end
-end

test/cache_test.rb

@@ -89,7 +89,7 @@ module ActiveModelSerializers
       assert_equal(nil, ActionController::Base.cache_store.fetch(@post.cache_key))
       assert_equal(nil, ActionController::Base.cache_store.fetch(@comment.cache_key))
 
-      Timecop.freeze(Time.now) do
+      Timecop.freeze(Time.current) do
         render_object_with_cache(@post)
 
         assert_equal(@post_serializer.attributes, ActionController::Base.cache_store.fetch(@post.cache_key))
@@ -101,7 +101,7 @@ module ActiveModelSerializers
       # Clean the Cache
       ActionController::Base.cache_store.clear
 
-      Timecop.freeze(Time.now) do
+      Timecop.freeze(Time.current) do
         # Generate a new Cache of Post object and each objects related to it.
         render_object_with_cache(@post)
 
@@ -150,7 +150,7 @@ module ActiveModelSerializers
       serializer = ActiveModel::Serializer::CollectionSerializer.new([@comment, @comment])
       include_tree = ActiveModel::Serializer::IncludeTree.from_include_args('*')
 
-      actual = Adapter::CachedSerializer.object_cache_keys(serializer, include_tree)
+      actual = CachedSerializer.object_cache_keys(serializer, include_tree)
 
       assert_equal actual.size, 3
       assert actual.any? { |key| key == 'comment/1' }
@@ -161,7 +161,7 @@ module ActiveModelSerializers
     def test_cached_attributes
       serializer = ActiveModel::Serializer::CollectionSerializer.new([@comment, @comment])
 
-      Timecop.freeze(Time.now) do
+      Timecop.freeze(Time.current) do
         render_object_with_cache(@comment)
 
         attributes = Adapter::Attributes.new(serializer)