From 35a7c81034dabb9b4b02c13291ece954412abb9a Mon Sep 17 00:00:00 2001
From: Benjamin Fleischer <github@benjaminfleischer.com>
Date: Mon, 6 Jun 2016 21:10:16 -0500
Subject: [PATCH 1/5] Fix up caching, especially fragment_cache

---
 lib/active_model/serializer.rb                |  4 +-
 lib/active_model/serializer/caching.rb        | 89 +++++++------------
 .../adapter/json_api.rb                       |  2 +-
 .../json_api/resource_identifier_test.rb      |  2 +-
 test/cache_test.rb                            | 40 +++++----
 5 files changed, 62 insertions(+), 75 deletions(-)

diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb
index 570073c7..f94ba556 100644
--- a/lib/active_model/serializer.rb
+++ b/lib/active_model/serializer.rb
@@ -167,7 +167,7 @@ module ActiveModel
       adapter_options ||= {}
       options[:include_directive] ||= ActiveModel::Serializer.include_directive_from_options(adapter_options)
       cached_attributes = adapter_options[:cached_attributes] ||= {}
-      resource = cached_attributes(options[:fields], cached_attributes, adapter_instance)
+      resource = fetch_attributes(options[:fields], cached_attributes, adapter_instance)
       relationships = resource_relationships(adapter_options, options, adapter_instance)
       resource.merge(relationships)
     end
@@ -195,6 +195,8 @@ module ActiveModel
       if respond_to?(attr)
         send(attr)
       elsif self.class._fragmented
+        # Attribute method wasn't available on this (fragment cached) serializer,
+        # so read it from the original serializer it was based on.
         self.class._fragmented.read_attribute_for_serialization(attr)
       else
         object.read_attribute_for_serialization(attr)
diff --git a/lib/active_model/serializer/caching.rb b/lib/active_model/serializer/caching.rb
index 5adf4ee3..0897ea01 100644
--- a/lib/active_model/serializer/caching.rb
+++ b/lib/active_model/serializer/caching.rb
@@ -1,5 +1,3 @@
-# TODO(BF): refactor file to be smaller
-# rubocop:disable Metrics/ModuleLength
 module ActiveModel
   class Serializer
     UndefinedCacheKey = Class.new(StandardError)
@@ -9,10 +7,10 @@ module ActiveModel
       included do
         with_options instance_writer: false, instance_reader: false do |serializer|
           serializer.class_attribute :_cache         # @api private : the cache store
-          serializer.class_attribute :_fragmented    # @api private : @see ::fragmented
+          serializer.class_attribute :_fragmented    # @api private : Used ONLY by FragmentedSerializer to reference original serializer
           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_only    # @api private : when fragment caching, whitelists fetch_attributes. Cannot combine with except
+          serializer.class_attribute :_cache_except  # @api private : when fragment caching, blacklists fetch_attributes. Cannot combine with only
           serializer.class_attribute :_cache_options # @api private : used by CachedSerializer, passed to _cache.fetch
           #  _cache_options include:
           #    expires_in
@@ -79,13 +77,6 @@ module ActiveModel
           _attributes - cached_attributes
         end
 
-        # @api private
-        # Used by FragmentCache on the CachedSerializer
-        #  to call attribute methods on the fragmented cached serializer.
-        def fragmented(serializer)
-          self._fragmented = serializer
-        end
-
         # Enables a serializer to be automatically cached
         #
         # Sets +::_cache+ object to <tt>ActionController::Base.cache_store</tt>
@@ -208,46 +199,44 @@ module ActiveModel
         end
       end
 
-      def cached_attributes(fields, cached_attributes, adapter_instance)
+      ### INSTANCE METHODS
+      def fetch_attributes(fields, cached_attributes, adapter_instance)
         if self.class.cache_enabled?
           key = cache_key(adapter_instance)
           cached_attributes.fetch(key) do
-            cache_check(adapter_instance) do
+            self.class.cache_store.fetch(key, self.class._cache_options) do
               attributes(fields)
             end
           end
+        elsif self.class.fragment_cache_enabled?
+          fetch_fragment_cache(adapter_instance)
         else
-          cache_check(adapter_instance) do
-            attributes(fields)
-          end
+          attributes(fields)
         end
       end
 
-      def cache_check(adapter_instance)
-        if self.class.cache_enabled?
-          self.class.cache_store.fetch(cache_key(adapter_instance), self.class._cache_options) do
+      def fetch(adapter_instance, cache_options = self.class._cache_options)
+        if self.class.cache_store
+          self.class.cache_store.fetch(cache_key(adapter_instance), cache_options) do
             yield
           end
-        elsif self.class.fragment_cache_enabled?
-          fetch_fragment_cache(adapter_instance)
         else
           yield
         end
       end
 
-      # 1. Create a CachedSerializer and NonCachedSerializer from the serializer class
+      # 1. Create a CachedSerializer from the serializer class
       # 2. Serialize the above two with the given adapter
       # 3. Pass their serializations to the adapter +::fragment_cache+
       #
       # It will split the serializer into two, one that will be cached and one that will not
       #
       # Given a resource name
-      # 1. Dynamically creates a CachedSerializer and NonCachedSerializer
+      # 1. Dynamically creates a CachedSerializer
       #   for a given class 'name'
       # 2. Call
       #       CachedSerializer.cache(serializer._cache_options)
-      #       CachedSerializer.fragmented(serializer)
-      #       NonCachedSerializer.cache(serializer._cache_options)
+      #       CachedSerializer._fragmented = serializer
       # 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
@@ -256,40 +245,47 @@ module ActiveModel
       #   When +name+ is <tt>User::Admin</tt>
       #   creates the Serializer classes (if they don't exist).
       #     CachedUser_AdminSerializer
-      #     NonCachedUser_AdminSerializer
       #
       # 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 fetch_fragment_cache(adapter_instance)
-        serializer_class_name = self.class.name.gsub('::'.freeze, '_'.freeze)
         self.class._cache_options ||= {}
         self.class._cache_options[:key] = self.class._cache_key if self.class._cache_key
 
-        cached_serializer = _get_or_create_fragment_cached_serializer(serializer_class_name)
+        cached_serializer = _get_or_create_fragment_cached_serializer
         cached_hash = ActiveModelSerializers::SerializableResource.new(
           object,
           serializer: cached_serializer,
           adapter: adapter_instance.class
         ).serializable_hash
 
-        non_cached_serializer = _get_or_create_fragment_non_cached_serializer(serializer_class_name)
-        non_cached_hash = ActiveModelSerializers::SerializableResource.new(
-          object,
-          serializer: non_cached_serializer,
-          adapter: adapter_instance.class
-        ).serializable_hash
+        fields = self.class.non_cached_attributes
+        non_cached_hash = attributes(fields, true)
 
         # Merge both results
         adapter_instance.fragment_cache(cached_hash, non_cached_hash)
       end
 
-      def _get_or_create_fragment_cached_serializer(serializer_class_name)
-        cached_serializer = _get_or_create_fragment_serializer "Cached#{serializer_class_name}"
+      def _get_or_create_fragment_cached_serializer
+        serializer_class_name = self.class.name.gsub('::'.freeze, '_'.freeze)
+        cached_serializer_name = "Cached#{serializer_class_name}"
+        if Object.const_defined?(cached_serializer_name)
+          cached_serializer = Object.const_get(cached_serializer_name)
+          # HACK: Test concern in production code :(
+          # But it's better than running all the cached fragment serializer
+          # code multiple times.
+          if Rails.env == 'test'.freeze
+            Object.send(:remove_const, cached_serializer_name)
+            return _get_or_create_fragment_cached_serializer
+          end
+          return cached_serializer
+        end
+        cached_serializer = Object.const_set(cached_serializer_name, Class.new(ActiveModel::Serializer))
         cached_serializer.cache(self.class._cache_options)
         cached_serializer.type(self.class._type)
-        cached_serializer.fragmented(self)
+        cached_serializer._fragmented = self
         self.class.cached_attributes.each do |attribute|
           options = self.class._attributes_keys[attribute] || {}
           cached_serializer.attribute(attribute, options)
@@ -297,22 +293,6 @@ module ActiveModel
         cached_serializer
       end
 
-      def _get_or_create_fragment_non_cached_serializer(serializer_class_name)
-        non_cached_serializer = _get_or_create_fragment_serializer "NonCached#{serializer_class_name}"
-        non_cached_serializer.type(self.class._type)
-        non_cached_serializer.fragmented(self)
-        self.class.non_cached_attributes.each do |attribute|
-          options = self.class._attributes_keys[attribute] || {}
-          non_cached_serializer.attribute(attribute, options)
-        end
-        non_cached_serializer
-      end
-
-      def _get_or_create_fragment_serializer(name)
-        return Object.const_get(name) if Object.const_defined?(name)
-        Object.const_set(name, Class.new(ActiveModel::Serializer))
-      end
-
       def cache_key(adapter_instance)
         return @cache_key if defined?(@cache_key)
 
@@ -339,4 +319,3 @@ module ActiveModel
     end
   end
 end
-# rubocop:enable Metrics/ModuleLength
diff --git a/lib/active_model_serializers/adapter/json_api.rb b/lib/active_model_serializers/adapter/json_api.rb
index 5293078c..6232086b 100644
--- a/lib/active_model_serializers/adapter/json_api.rb
+++ b/lib/active_model_serializers/adapter/json_api.rb
@@ -294,7 +294,7 @@ module ActiveModelSerializers
 
       # {http://jsonapi.org/format/#document-resource-objects Document Resource Objects}
       def resource_object_for(serializer)
-        resource_object = serializer.cache_check(self) do
+        resource_object = serializer.fetch(self) do
           resource_object = ResourceIdentifier.new(serializer, instance_options).as_json
 
           requested_fields = fieldset && fieldset.fields_for(resource_object[:type])
diff --git a/test/adapter/json_api/resource_identifier_test.rb b/test/adapter/json_api/resource_identifier_test.rb
index a6f312c4..7624d92b 100644
--- a/test/adapter/json_api/resource_identifier_test.rb
+++ b/test/adapter/json_api/resource_identifier_test.rb
@@ -42,7 +42,7 @@ module ActiveModelSerializers
         end
 
         def test_id_defined_on_fragmented
-          FragmentedSerializer.fragmented(WithDefinedIdSerializer.new(@model))
+          FragmentedSerializer._fragmented = WithDefinedIdSerializer.new(@model)
           test_id(FragmentedSerializer, 'special_id')
         end
 
diff --git a/test/cache_test.rb b/test/cache_test.rb
index 9dba8d70..e804eb5b 100644
--- a/test/cache_test.rb
+++ b/test/cache_test.rb
@@ -204,7 +204,7 @@ module ActiveModelSerializers
 
     # Based on original failing test by @kevintyll
     # rubocop:disable Metrics/AbcSize
-    def test_a_serializer_rendered_by_two_adapter_returns_differently_cached_attributes
+    def test_a_serializer_rendered_by_two_adapter_returns_differently_fetch_attributes
       Object.const_set(:Alert, Class.new(ActiveModelSerializers::Model) do
         attr_accessor :id, :status, :resource, :started_at, :ended_at, :updated_at, :created_at
       end)
@@ -225,7 +225,7 @@ module ActiveModelSerializers
         created_at: Time.new(2016, 3, 31, 21, 37, 35, 0)
       )
 
-      expected_cached_attributes = {
+      expected_fetch_attributes = {
         id: 1,
         status: 'fail',
         resource: 'resource-1',
@@ -250,7 +250,7 @@ module ActiveModelSerializers
       # 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 expected_fetch_attributes, alert.attributes
       assert_equal alert.attributes, attributes_serialization
       attributes_cache_key = serializable_alert.adapter.serializer.cache_key(serializable_alert.adapter)
       assert_equal attributes_serialization, cache_store.fetch(attributes_cache_key)
@@ -296,23 +296,28 @@ module ActiveModelSerializers
       assert actual.any? { |key| key =~ %r{author/author-\d+} }
     end
 
-    def test_cached_attributes
-      serializer = ActiveModel::Serializer::CollectionSerializer.new([@comment, @comment])
+    def test_fetch_attributes_from_cache
+      serializers = ActiveModel::Serializer::CollectionSerializer.new([@comment, @comment])
 
       Timecop.freeze(Time.current) do
         render_object_with_cache(@comment)
 
-        attributes = Adapter::Attributes.new(serializer)
-        include_directive = ActiveModelSerializers.default_include_directive
-        cached_attributes = ActiveModel::Serializer.cache_read_multi(serializer, attributes, include_directive)
+        options = {}
+        adapter_options = {}
+        adapter_instance = ActiveModelSerializers::Adapter::Attributes.new(serializers, adapter_options)
+        serializers.serializable_hash(adapter_options, options, adapter_instance)
+        cached_attributes = adapter_options.fetch(:cached_attributes)
 
-        assert_equal cached_attributes["#{@comment.cache_key}/#{attributes.cache_key}"], Comment.new(id: 1, body: 'ZOMG A COMMENT').attributes
-        assert_equal cached_attributes["#{@comment.post.cache_key}/#{attributes.cache_key}"], Post.new(id: 'post', title: 'New Post', body: 'Body').attributes
+        include_directive = ActiveModelSerializers.default_include_directive
+        manual_cached_attributes = ActiveModel::Serializer.cache_read_multi(serializers, adapter_instance, include_directive)
+        assert_equal manual_cached_attributes, cached_attributes
+
+        assert_equal cached_attributes["#{@comment.cache_key}/#{adapter_instance.cache_key}"], Comment.new(id: 1, body: 'ZOMG A COMMENT').attributes
+        assert_equal cached_attributes["#{@comment.post.cache_key}/#{adapter_instance.cache_key}"], 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}/#{attributes.cache_key}"], Author.new(id: 'author', name: 'Joao M. D. Moura').attributes
+        assert_equal cached_attributes["#{writer_cache_key}/#{adapter_instance.cache_key}"], Author.new(id: 'author', name: 'Joao M. D. Moura').attributes
       end
     end
 
@@ -442,6 +447,9 @@ module ActiveModelSerializers
         name: @role.name
       }
       assert_equal(@role_hash, expected_result)
+    ensure
+      fragmented_serializer = @role_serializer
+      Object.send(:remove_const, fragmented_serializer._get_or_create_fragment_cached_serializer.name)
     end
 
     def test_fragment_fetch_with_namespaced_object
@@ -452,6 +460,9 @@ module ActiveModelSerializers
         id: @spam.id
       }
       assert_equal(@spam_hash, expected_result)
+    ensure
+      fragmented_serializer = @spam_serializer
+      Object.send(:remove_const, fragmented_serializer._get_or_create_fragment_cached_serializer.name)
     end
 
     private
@@ -476,10 +487,5 @@ module ActiveModelSerializers
     def adapter
       @serializable_resource.adapter
     end
-
-    def cached_serialization(serializer)
-      cache_key = serializer.cache_key(adapter)
-      cache_store.fetch(cache_key)
-    end
   end
 end

From 253205bb49d5d1c2ec3599c150437cf4fe37ee64 Mon Sep 17 00:00:00 2001
From: Benjamin Fleischer <github@benjaminfleischer.com>
Date: Tue, 7 Jun 2016 01:50:03 -0500
Subject: [PATCH 2/5] Improve Coverage

---
 lib/active_model/serializer/adapter/base.rb          | 2 ++
 lib/active_model/serializer/belongs_to_reflection.rb | 3 ---
 lib/active_model/serializer/has_many_reflection.rb   | 3 ---
 lib/active_model/serializer/has_one_reflection.rb    | 3 ---
 lib/active_model_serializers/adapter.rb              | 2 ++
 lib/active_model_serializers/deserialization.rb      | 2 ++
 lib/active_model_serializers/model.rb                | 2 ++
 lib/active_model_serializers/railtie.rb              | 2 ++
 8 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/lib/active_model/serializer/adapter/base.rb b/lib/active_model/serializer/adapter/base.rb
index f27328f7..013a9705 100644
--- a/lib/active_model/serializer/adapter/base.rb
+++ b/lib/active_model/serializer/adapter/base.rb
@@ -7,9 +7,11 @@ module ActiveModel
           deprecate :inherited, 'ActiveModelSerializers::Adapter::Base.'
         end
 
+        # :nocov:
         def initialize(serializer, options = {})
           super(ActiveModelSerializers::Adapter::Base.new(serializer, options))
         end
+        # :nocov:
       end
     end
   end
diff --git a/lib/active_model/serializer/belongs_to_reflection.rb b/lib/active_model/serializer/belongs_to_reflection.rb
index 8cc5a201..a014b7a5 100644
--- a/lib/active_model/serializer/belongs_to_reflection.rb
+++ b/lib/active_model/serializer/belongs_to_reflection.rb
@@ -2,9 +2,6 @@ module ActiveModel
   class Serializer
     # @api private
     class BelongsToReflection < SingularReflection
-      def macro
-        :belongs_to
-      end
     end
   end
 end
diff --git a/lib/active_model/serializer/has_many_reflection.rb b/lib/active_model/serializer/has_many_reflection.rb
index 08be4417..60ccc481 100644
--- a/lib/active_model/serializer/has_many_reflection.rb
+++ b/lib/active_model/serializer/has_many_reflection.rb
@@ -2,9 +2,6 @@ module ActiveModel
   class Serializer
     # @api private
     class HasManyReflection < CollectionReflection
-      def macro
-        :has_many
-      end
     end
   end
 end
diff --git a/lib/active_model/serializer/has_one_reflection.rb b/lib/active_model/serializer/has_one_reflection.rb
index 5a915f7f..bf41b1d1 100644
--- a/lib/active_model/serializer/has_one_reflection.rb
+++ b/lib/active_model/serializer/has_one_reflection.rb
@@ -2,9 +2,6 @@ module ActiveModel
   class Serializer
     # @api private
     class HasOneReflection < SingularReflection
-      def macro
-        :has_one
-      end
     end
   end
 end
diff --git a/lib/active_model_serializers/adapter.rb b/lib/active_model_serializers/adapter.rb
index 93e4c044..98caab44 100644
--- a/lib/active_model_serializers/adapter.rb
+++ b/lib/active_model_serializers/adapter.rb
@@ -5,11 +5,13 @@ module ActiveModelSerializers
     private_constant :ADAPTER_MAP if defined?(private_constant)
 
     class << self # All methods are class functions
+      # :nocov:
       def new(*args)
         fail ArgumentError, 'Adapters inherit from Adapter::Base.' \
           "Adapter.new called with args: '#{args.inspect}', from" \
           "'caller[0]'."
       end
+      # :nocov:
 
       def configured_adapter
         lookup(ActiveModelSerializers.config.adapter)
diff --git a/lib/active_model_serializers/deserialization.rb b/lib/active_model_serializers/deserialization.rb
index 6b6d417b..878dd98d 100644
--- a/lib/active_model_serializers/deserialization.rb
+++ b/lib/active_model_serializers/deserialization.rb
@@ -6,8 +6,10 @@ module ActiveModelSerializers
       Adapter::JsonApi::Deserialization.parse(*args)
     end
 
+    # :nocov:
     def jsonapi_parse!(*args)
       Adapter::JsonApi::Deserialization.parse!(*args)
     end
+    # :nocov:
   end
 end
diff --git a/lib/active_model_serializers/model.rb b/lib/active_model_serializers/model.rb
index 62971392..53cd6587 100644
--- a/lib/active_model_serializers/model.rb
+++ b/lib/active_model_serializers/model.rb
@@ -38,6 +38,7 @@ module ActiveModelSerializers
     end
 
     # The following methods are needed to be minimally implemented for ActiveModel::Errors
+    # :nocov:
     def self.human_attribute_name(attr, _options = {})
       attr
     end
@@ -45,5 +46,6 @@ module ActiveModelSerializers
     def self.lookup_ancestors
       [self]
     end
+    # :nocov:
   end
 end
diff --git a/lib/active_model_serializers/railtie.rb b/lib/active_model_serializers/railtie.rb
index 971393fe..c7d6c0d1 100644
--- a/lib/active_model_serializers/railtie.rb
+++ b/lib/active_model_serializers/railtie.rb
@@ -32,11 +32,13 @@ module ActiveModelSerializers
       end
     end
 
+    # :nocov:
     generators do |app|
       Rails::Generators.configure!(app.config.generators)
       Rails::Generators.hidden_namespaces.uniq!
       require 'generators/rails/resource_override'
     end
+    # :nocov:
 
     if Rails.env.test?
       ActionController::TestCase.send(:include, ActiveModelSerializers::Test::Schema)

From b8924157d729d32c9db436650b499ebcc523ea83 Mon Sep 17 00:00:00 2001
From: Benjamin Fleischer <github@benjaminfleischer.com>
Date: Tue, 7 Jun 2016 03:42:03 -0500
Subject: [PATCH 3/5] Remove remaining fragmented cache class

---
 lib/active_model/serializer.rb                |   4 -
 lib/active_model/serializer/caching.rb        | 106 ++++++------------
 .../json_api/resource_identifier_test.rb      |   9 +-
 test/cache_test.rb                            |  15 +--
 4 files changed, 50 insertions(+), 84 deletions(-)

diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb
index f94ba556..5c312665 100644
--- a/lib/active_model/serializer.rb
+++ b/lib/active_model/serializer.rb
@@ -194,10 +194,6 @@ module ActiveModel
     def read_attribute_for_serialization(attr)
       if respond_to?(attr)
         send(attr)
-      elsif self.class._fragmented
-        # Attribute method wasn't available on this (fragment cached) serializer,
-        # so read it from the original serializer it was based on.
-        self.class._fragmented.read_attribute_for_serialization(attr)
       else
         object.read_attribute_for_serialization(attr)
       end
diff --git a/lib/active_model/serializer/caching.rb b/lib/active_model/serializer/caching.rb
index 0897ea01..e7bd6e73 100644
--- a/lib/active_model/serializer/caching.rb
+++ b/lib/active_model/serializer/caching.rb
@@ -7,7 +7,6 @@ module ActiveModel
       included do
         with_options instance_writer: false, instance_reader: false do |serializer|
           serializer.class_attribute :_cache         # @api private : the cache store
-          serializer.class_attribute :_fragmented    # @api private : Used ONLY by FragmentedSerializer to reference original serializer
           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 fetch_attributes. Cannot combine with except
           serializer.class_attribute :_cache_except  # @api private : when fragment caching, blacklists fetch_attributes. Cannot combine with only
@@ -69,12 +68,15 @@ module ActiveModel
           _cache_options && _cache_options[:skip_digest]
         end
 
-        def cached_attributes
-          _cache_only ? _cache_only : _attributes - _cache_except
-        end
-
-        def non_cached_attributes
-          _attributes - cached_attributes
+        def fragmented_attributes
+          cached = _cache_only ? _cache_only : _attributes - _cache_except
+          cached = cached.map! {|field| _attributes_keys.fetch(field, field) }
+          non_cached = _attributes - cached
+          non_cached = non_cached.map! {|field| _attributes_keys.fetch(field, field) }
+          {
+            cached: cached,
+            non_cached: non_cached
+          }
         end
 
         # Enables a serializer to be automatically cached
@@ -205,13 +207,13 @@ module ActiveModel
           key = cache_key(adapter_instance)
           cached_attributes.fetch(key) do
             self.class.cache_store.fetch(key, self.class._cache_options) do
-              attributes(fields)
+              attributes(fields, true)
             end
           end
         elsif self.class.fragment_cache_enabled?
-          fetch_fragment_cache(adapter_instance)
+          fetch_attributes_fragment(adapter_instance)
         else
-          attributes(fields)
+          attributes(fields, true)
         end
       end
 
@@ -225,74 +227,40 @@ module ActiveModel
         end
       end
 
-      # 1. Create a CachedSerializer from the serializer class
-      # 2. Serialize the above two with the given adapter
-      # 3. Pass their serializations to the adapter +::fragment_cache+
-      #
-      # It will split the serializer into two, one that will be cached and one that will not
-      #
-      # Given a resource name
-      # 1. Dynamically creates a CachedSerializer
-      #   for a given class 'name'
-      # 2. Call
-      #       CachedSerializer.cache(serializer._cache_options)
-      #       CachedSerializer._fragmented = serializer
-      # 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).
-      #     CachedUser_AdminSerializer
-      #
-      # 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 fetch_fragment_cache(adapter_instance)
+      # 1. Determine cached fields from serializer class options
+      # 2. Get non_cached_fields and fetch cache_fields
+      # 3. Merge the two hashes using adapter_instance#fragment_cache
+      def fetch_attributes_fragment(adapter_instance)
         self.class._cache_options ||= {}
         self.class._cache_options[:key] = self.class._cache_key if self.class._cache_key
+        options = { include_directive: ActiveModel::Serializer.include_directive_from_options({})}
+        fields = self.class.fragmented_attributes
 
-        cached_serializer = _get_or_create_fragment_cached_serializer
-        cached_hash = ActiveModelSerializers::SerializableResource.new(
-          object,
-          serializer: cached_serializer,
-          adapter: adapter_instance.class
-        ).serializable_hash
+        non_cached_fields = fields[:non_cached].dup
+        non_cached_hash = attributes(non_cached_fields, true)
+        (non_cached_fields - non_cached_hash.keys).each do |missing_field|
+          # TODO: use _attributes_keys?
+          # gets any other associations, etc.
+          non_cached_hash[missing_field] = read_attribute_for_serialization(missing_field)
+        end
 
-        fields = self.class.non_cached_attributes
-        non_cached_hash = attributes(fields, true)
+        cached_fields = fields[:cached].dup
+        key = cache_key(adapter_instance)
+        cached_hash =
+          self.class.cache_store.fetch(key, self.class._cache_options) do
+            hash = attributes(cached_fields, true)
+            (cached_fields - hash.keys).each do |missing_field|
+              # TODO: use _attributes_keys?
+              # gets any other associations, etc.
+              hash[missing_field] = read_attribute_for_serialization(missing_field)
+            end
+            hash
+          end
 
         # Merge both results
         adapter_instance.fragment_cache(cached_hash, non_cached_hash)
       end
 
-      def _get_or_create_fragment_cached_serializer
-        serializer_class_name = self.class.name.gsub('::'.freeze, '_'.freeze)
-        cached_serializer_name = "Cached#{serializer_class_name}"
-        if Object.const_defined?(cached_serializer_name)
-          cached_serializer = Object.const_get(cached_serializer_name)
-          # HACK: Test concern in production code :(
-          # But it's better than running all the cached fragment serializer
-          # code multiple times.
-          if Rails.env == 'test'.freeze
-            Object.send(:remove_const, cached_serializer_name)
-            return _get_or_create_fragment_cached_serializer
-          end
-          return cached_serializer
-        end
-        cached_serializer = Object.const_set(cached_serializer_name, Class.new(ActiveModel::Serializer))
-        cached_serializer.cache(self.class._cache_options)
-        cached_serializer.type(self.class._type)
-        cached_serializer._fragmented = self
-        self.class.cached_attributes.each do |attribute|
-          options = self.class._attributes_keys[attribute] || {}
-          cached_serializer.attribute(attribute, options)
-        end
-        cached_serializer
-      end
-
       def cache_key(adapter_instance)
         return @cache_key if defined?(@cache_key)
 
diff --git a/test/adapter/json_api/resource_identifier_test.rb b/test/adapter/json_api/resource_identifier_test.rb
index 7624d92b..fa91ff72 100644
--- a/test/adapter/json_api/resource_identifier_test.rb
+++ b/test/adapter/json_api/resource_identifier_test.rb
@@ -14,7 +14,13 @@ module ActiveModelSerializers
           end
         end
 
-        class FragmentedSerializer < ActiveModel::Serializer; end
+        class FragmentedSerializer < ActiveModel::Serializer
+          cache only: :id
+
+          def id
+            'special_id'
+          end
+        end
 
         setup do
           @model = Author.new(id: 1, name: 'Steve K.')
@@ -42,7 +48,6 @@ module ActiveModelSerializers
         end
 
         def test_id_defined_on_fragmented
-          FragmentedSerializer._fragmented = WithDefinedIdSerializer.new(@model)
           test_id(FragmentedSerializer, 'special_id')
         end
 
diff --git a/test/cache_test.rb b/test/cache_test.rb
index e804eb5b..5df93432 100644
--- a/test/cache_test.rb
+++ b/test/cache_test.rb
@@ -117,7 +117,7 @@ module ActiveModelSerializers
       e = assert_raises ActiveModel::Serializer::UndefinedCacheKey do
         render_object_with_cache(article)
       end
-      assert_match(/ActiveModelSerializers::CacheTest::Article must define #cache_key, or the 'key:' option must be passed into 'CachedActiveModelSerializers_CacheTest_ArticleSerializer.cache'/, e.message)
+      assert_match(/ActiveModelSerializers::CacheTest::Article must define #cache_key, or the 'key:' option must be passed into 'ActiveModelSerializers::CacheTest::ArticleSerializer.cache'/, e.message)
     end
 
     def test_cache_options_definition
@@ -438,7 +438,8 @@ module ActiveModelSerializers
       @role            = Role.new(name: 'Great Author', description: nil)
       @role.author     = [@author]
       @role_serializer = RoleSerializer.new(@role)
-      @role_hash       = @role_serializer.fetch_fragment_cache(ActiveModelSerializers::Adapter.configured_adapter.new(@role_serializer))
+      adapter_instance = ActiveModelSerializers::Adapter.configured_adapter.new(@role_serializer)
+      @role_hash       = @role_serializer.fetch_attributes_fragment(adapter_instance)
 
       expected_result = {
         id: @role.id,
@@ -446,23 +447,19 @@ module ActiveModelSerializers
         slug: "#{@role.name}-#{@role.id}",
         name: @role.name
       }
+
       assert_equal(@role_hash, expected_result)
-    ensure
-      fragmented_serializer = @role_serializer
-      Object.send(:remove_const, fragmented_serializer._get_or_create_fragment_cached_serializer.name)
     end
 
     def test_fragment_fetch_with_namespaced_object
       @spam            = Spam::UnrelatedLink.new(id: 'spam-id-1')
       @spam_serializer = Spam::UnrelatedLinkSerializer.new(@spam)
-      @spam_hash       = @spam_serializer.fetch_fragment_cache(ActiveModelSerializers::Adapter.configured_adapter.new(@spam_serializer))
+      adapter_instance = ActiveModelSerializers::Adapter.configured_adapter.new(@spam_serializer)
+      @spam_hash       = @spam_serializer.fetch_attributes_fragment(adapter_instance)
       expected_result = {
         id: @spam.id
       }
       assert_equal(@spam_hash, expected_result)
-    ensure
-      fragmented_serializer = @spam_serializer
-      Object.send(:remove_const, fragmented_serializer._get_or_create_fragment_cached_serializer.name)
     end
 
     private

From 5375e009e28ebf2d6c66891fed9fcb2386180c9f Mon Sep 17 00:00:00 2001
From: Benjamin Fleischer <github@benjaminfleischer.com>
Date: Tue, 7 Jun 2016 03:57:24 -0500
Subject: [PATCH 4/5] Test caching with fragmented key

- on association, fix up assocation logic
- on attribute
---
 lib/active_model/serializer/caching.rb        | 20 ++-----
 .../explicit_serializer_test.rb               |  2 +-
 test/cache_test.rb                            | 55 ++++++++++++++-----
 test/fixtures/poro.rb                         | 11 ++--
 4 files changed, 53 insertions(+), 35 deletions(-)

diff --git a/lib/active_model/serializer/caching.rb b/lib/active_model/serializer/caching.rb
index e7bd6e73..a1692275 100644
--- a/lib/active_model/serializer/caching.rb
+++ b/lib/active_model/serializer/caching.rb
@@ -70,9 +70,9 @@ module ActiveModel
 
         def fragmented_attributes
           cached = _cache_only ? _cache_only : _attributes - _cache_except
-          cached = cached.map! {|field| _attributes_keys.fetch(field, field) }
+          cached = cached.map! { |field| _attributes_keys.fetch(field, field) }
           non_cached = _attributes - cached
-          non_cached = non_cached.map! {|field| _attributes_keys.fetch(field, field) }
+          non_cached = non_cached.map! { |field| _attributes_keys.fetch(field, field) }
           {
             cached: cached,
             non_cached: non_cached
@@ -233,28 +233,20 @@ module ActiveModel
       def fetch_attributes_fragment(adapter_instance)
         self.class._cache_options ||= {}
         self.class._cache_options[:key] = self.class._cache_key if self.class._cache_key
-        options = { include_directive: ActiveModel::Serializer.include_directive_from_options({})}
         fields = self.class.fragmented_attributes
 
         non_cached_fields = fields[:non_cached].dup
         non_cached_hash = attributes(non_cached_fields, true)
-        (non_cached_fields - non_cached_hash.keys).each do |missing_field|
-          # TODO: use _attributes_keys?
-          # gets any other associations, etc.
-          non_cached_hash[missing_field] = read_attribute_for_serialization(missing_field)
-        end
+        include_directive = JSONAPI::IncludeDirective.new(non_cached_fields - non_cached_hash.keys)
+        non_cached_hash.merge! resource_relationships({}, { include_directive: include_directive }, adapter_instance)
 
         cached_fields = fields[:cached].dup
         key = cache_key(adapter_instance)
         cached_hash =
           self.class.cache_store.fetch(key, self.class._cache_options) do
             hash = attributes(cached_fields, true)
-            (cached_fields - hash.keys).each do |missing_field|
-              # TODO: use _attributes_keys?
-              # gets any other associations, etc.
-              hash[missing_field] = read_attribute_for_serialization(missing_field)
-            end
-            hash
+            include_directive = JSONAPI::IncludeDirective.new(cached_fields - hash.keys)
+            hash.merge! resource_relationships({}, { include_directive: include_directive }, adapter_instance)
           end
 
         # Merge both results
diff --git a/test/action_controller/explicit_serializer_test.rb b/test/action_controller/explicit_serializer_test.rb
index 8886d07d..a23b6f6b 100644
--- a/test/action_controller/explicit_serializer_test.rb
+++ b/test/action_controller/explicit_serializer_test.rb
@@ -123,7 +123,7 @@ module ActionController
               id: 42,
               lat: '-23.550520',
               lng: '-46.633309',
-              place: 'Nowhere' # is a virtual attribute on LocationSerializer
+              address: 'Nowhere' # is a virtual attribute on LocationSerializer
             }
           ]
         }
diff --git a/test/cache_test.rb b/test/cache_test.rb
index 5df93432..57312a91 100644
--- a/test/cache_test.rb
+++ b/test/cache_test.rb
@@ -127,7 +127,7 @@ module ActiveModelSerializers
     end
 
     def test_fragment_cache_definition
-      assert_equal([:name], @role_serializer.class._cache_only)
+      assert_equal([:name, :slug], @role_serializer.class._cache_only)
       assert_equal([:content], @bio_serializer.class._cache_except)
     end
 
@@ -178,14 +178,14 @@ module ActiveModelSerializers
         id: @location.id,
         lat: @location.lat,
         lng: @location.lng,
-        place: 'Nowhere'
+        address: 'Nowhere'
       }
 
       hash = render_object_with_cache(@location)
 
       assert_equal(hash, expected_result)
       key = "#{@location.cache_key}/#{adapter.cache_key}"
-      assert_equal({ place: 'Nowhere' }, cache_store.fetch(key))
+      assert_equal({ address: 'Nowhere' }, cache_store.fetch(key))
     end
 
     def test_fragment_cache_with_inheritance
@@ -434,21 +434,46 @@ module ActiveModelSerializers
     end
 
     def test_fragment_fetch_with_virtual_attributes
-      @author          = Author.new(name: 'Joao M. D. Moura')
-      @role            = Role.new(name: 'Great Author', description: nil)
-      @role.author     = [@author]
-      @role_serializer = RoleSerializer.new(@role)
-      adapter_instance = ActiveModelSerializers::Adapter.configured_adapter.new(@role_serializer)
-      @role_hash       = @role_serializer.fetch_attributes_fragment(adapter_instance)
-
+      author          = Author.new(name: 'Joao M. D. Moura')
+      role            = Role.new(name: 'Great Author', description: nil)
+      role.author     = [author]
+      role_serializer = RoleSerializer.new(role)
+      adapter_instance = ActiveModelSerializers::Adapter.configured_adapter.new(role_serializer)
       expected_result = {
-        id: @role.id,
-        description: @role.description,
-        slug: "#{@role.name}-#{@role.id}",
-        name: @role.name
+        id: role.id,
+        description: role.description,
+        slug: "#{role.name}-#{role.id}",
+        name: role.name
       }
+      cache_store.clear
 
-      assert_equal(@role_hash, expected_result)
+      role_hash = role_serializer.fetch_attributes_fragment(adapter_instance)
+      assert_equal(role_hash, expected_result)
+
+      role.attributes[:id] = 'this has been updated'
+      role.name = 'this was cached'
+
+      role_hash = role_serializer.fetch_attributes_fragment(adapter_instance)
+      assert_equal(expected_result.merge(id: role.id), role_hash)
+    end
+
+    def test_fragment_fetch_with_except
+      adapter_instance = ActiveModelSerializers::Adapter.configured_adapter.new(@bio_serializer)
+      expected_result = {
+        id: @bio.id,
+        rating: nil,
+        content: @bio.content
+      }
+      cache_store.clear
+
+      bio_hash = @bio_serializer.fetch_attributes_fragment(adapter_instance)
+      assert_equal(expected_result, bio_hash)
+
+      @bio.content = 'this has been updated'
+      @bio.rating = 'this was cached'
+
+      bio_hash = @bio_serializer.fetch_attributes_fragment(adapter_instance)
+      assert_equal(expected_result.merge(content: @bio.content), bio_hash)
     end
 
     def test_fragment_fetch_with_namespaced_object
diff --git a/test/fixtures/poro.rb b/test/fixtures/poro.rb
index 71be0108..20c961ee 100644
--- a/test/fixtures/poro.rb
+++ b/test/fixtures/poro.rb
@@ -136,10 +136,11 @@ AuthorSerializer = Class.new(ActiveModel::Serializer) do
 end
 
 RoleSerializer = Class.new(ActiveModel::Serializer) do
-  cache only: [:name], skip_digest: true
-  attributes :id, :name, :description, :slug
+  cache only: [:name, :slug], skip_digest: true
+  attributes :id, :name, :description
+  attribute :friendly_id, key: :slug
 
-  def slug
+  def friendly_id
     "#{object.name}-#{object.id}"
   end
 
@@ -153,10 +154,10 @@ LikeSerializer = Class.new(ActiveModel::Serializer) do
 end
 
 LocationSerializer = Class.new(ActiveModel::Serializer) do
-  cache only: [:place], skip_digest: true
+  cache only: [:address], skip_digest: true
   attributes :id, :lat, :lng
 
-  belongs_to :place
+  belongs_to :place, key: :address
 
   def place
     'Nowhere'

From b599360ae3607902f969566d675f9ac76c2ed0fa Mon Sep 17 00:00:00 2001
From: Benjamin Fleischer <github@benjaminfleischer.com>
Date: Tue, 7 Jun 2016 20:28:02 -0500
Subject: [PATCH 5/5] Provide convenience serializer_class for all the
 self.class calls

per groyoh
https://github.com/rails-api/active_model_serializers/pull/1781#discussion_r66021340
---
 lib/active_model/serializer/caching.rb | 32 +++++++++++++++-----------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/lib/active_model/serializer/caching.rb b/lib/active_model/serializer/caching.rb
index a1692275..dfef8f02 100644
--- a/lib/active_model/serializer/caching.rb
+++ b/lib/active_model/serializer/caching.rb
@@ -18,7 +18,7 @@ module ActiveModel
           #    race_condition_ttl
           #  Passed to ::_cache as
           #    serializer.cache_store.fetch(cache_key, @klass._cache_options)
-          #  Passed as second argument to serializer.cache_store.fetch(cache_key, self.class._cache_options)
+          #  Passed as second argument to serializer.cache_store.fetch(cache_key, serializer_class._cache_options)
           serializer.class_attribute :_cache_digest_file_path # @api private : Derived at inheritance
         end
       end
@@ -203,23 +203,23 @@ module ActiveModel
 
       ### INSTANCE METHODS
       def fetch_attributes(fields, cached_attributes, adapter_instance)
-        if self.class.cache_enabled?
+        if serializer_class.cache_enabled?
           key = cache_key(adapter_instance)
           cached_attributes.fetch(key) do
-            self.class.cache_store.fetch(key, self.class._cache_options) do
+            serializer_class.cache_store.fetch(key, serializer_class._cache_options) do
               attributes(fields, true)
             end
           end
-        elsif self.class.fragment_cache_enabled?
+        elsif serializer_class.fragment_cache_enabled?
           fetch_attributes_fragment(adapter_instance)
         else
           attributes(fields, true)
         end
       end
 
-      def fetch(adapter_instance, cache_options = self.class._cache_options)
-        if self.class.cache_store
-          self.class.cache_store.fetch(cache_key(adapter_instance), cache_options) do
+      def fetch(adapter_instance, cache_options = serializer_class._cache_options)
+        if serializer_class.cache_store
+          serializer_class.cache_store.fetch(cache_key(adapter_instance), cache_options) do
             yield
           end
         else
@@ -231,9 +231,9 @@ module ActiveModel
       # 2. Get non_cached_fields and fetch cache_fields
       # 3. Merge the two hashes using adapter_instance#fragment_cache
       def fetch_attributes_fragment(adapter_instance)
-        self.class._cache_options ||= {}
-        self.class._cache_options[:key] = self.class._cache_key if self.class._cache_key
-        fields = self.class.fragmented_attributes
+        serializer_class._cache_options ||= {}
+        serializer_class._cache_options[:key] = serializer_class._cache_key if serializer_class._cache_key
+        fields = serializer_class.fragmented_attributes
 
         non_cached_fields = fields[:non_cached].dup
         non_cached_hash = attributes(non_cached_fields, true)
@@ -243,7 +243,7 @@ module ActiveModel
         cached_fields = fields[:cached].dup
         key = cache_key(adapter_instance)
         cached_hash =
-          self.class.cache_store.fetch(key, self.class._cache_options) do
+          serializer_class.cache_store.fetch(key, serializer_class._cache_options) do
             hash = attributes(cached_fields, true)
             include_directive = JSONAPI::IncludeDirective.new(cached_fields - hash.keys)
             hash.merge! resource_relationships({}, { include_directive: include_directive }, adapter_instance)
@@ -259,7 +259,7 @@ module ActiveModel
         parts = []
         parts << object_cache_key
         parts << adapter_instance.cache_key
-        parts << self.class._cache_digest unless self.class._skip_digest?
+        parts << serializer_class._cache_digest unless serializer_class._skip_digest?
         @cache_key = parts.join('/')
       end
 
@@ -268,14 +268,18 @@ module ActiveModel
       def object_cache_key
         if object.respond_to?(:cache_key)
           object.cache_key
-        elsif (serializer_cache_key = (self.class._cache_key || self.class._cache_options[:key]))
+        elsif (serializer_cache_key = (serializer_class._cache_key || serializer_class._cache_options[:key]))
           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)
           "#{serializer_cache_key}/#{object.id}-#{object_time_safe}"
         else
-          fail UndefinedCacheKey, "#{object.class} must define #cache_key, or the 'key:' option must be passed into '#{self.class}.cache'"
+          fail UndefinedCacheKey, "#{object.class} must define #cache_key, or the 'key:' option must be passed into '#{serializer_class}.cache'"
         end
       end
+
+      def serializer_class
+        @serializer_class ||= self.class
+      end
     end
   end
 end