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 User::Admin - # 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