diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fe928295..1a46a83f4 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index fe51fb8fc..d78b873fb 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -48,9 +48,12 @@ def self.serializer_for(resource, options = {}) # @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) diff --git a/lib/active_model/serializer/attributes.rb b/lib/active_model/serializer/attributes.rb index 11d39c4b2..d1968d77e 100644 --- a/lib/active_model/serializer/attributes.rb +++ b/lib/active_model/serializer/attributes.rb @@ -68,7 +68,7 @@ def _attributes # @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| diff --git a/lib/active_model_serializers.rb b/lib/active_model_serializers.rb index 7e036c6bd..d68aba424 100644 --- a/lib/active_model_serializers.rb +++ b/lib/active_model_serializers.rb @@ -5,6 +5,8 @@ module ActiveModelSerializers extend ActiveSupport::Autoload autoload :Model + autoload :CachedSerializer + autoload :FragmentCache autoload :Callbacks autoload :Deserialization autoload :Logging diff --git a/lib/active_model_serializers/adapter.rb b/lib/active_model_serializers/adapter.rb index 335de9db9..643b141cf 100644 --- a/lib/active_model_serializers/adapter.rb +++ b/lib/active_model_serializers/adapter.rb @@ -3,8 +3,6 @@ module Adapter 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) diff --git a/lib/active_model_serializers/adapter/attributes.rb b/lib/active_model_serializers/adapter/attributes.rb index c89417e83..c062127c3 100644 --- a/lib/active_model_serializers/adapter/attributes.rb +++ b/lib/active_model_serializers/adapter/attributes.rb @@ -17,10 +17,6 @@ def serializable_hash(options = nil) 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) diff --git a/lib/active_model_serializers/adapter/base.rb b/lib/active_model_serializers/adapter/base.rb index 9b31cffcf..18002af4a 100644 --- a/lib/active_model_serializers/adapter/base.rb +++ b/lib/active_model_serializers/adapter/base.rb @@ -23,8 +23,8 @@ def as_json(options = nil) hash end - def fragment_cache(*_args) - fail NotImplementedError, 'This is an abstract method. Should be implemented at the concrete adapter.' + def fragment_cache(cached_hash, non_cached_hash) + non_cached_hash.merge cached_hash end def cache_check(serializer) diff --git a/lib/active_model_serializers/adapter/cached_serializer.rb b/lib/active_model_serializers/adapter/cached_serializer.rb deleted file mode 100644 index 6f0b04789..000000000 --- a/lib/active_model_serializers/adapter/cached_serializer.rb +++ /dev/null @@ -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 diff --git a/lib/active_model_serializers/adapter/fragment_cache.rb b/lib/active_model_serializers/adapter/fragment_cache.rb deleted file mode 100644 index f1c46e6e5..000000000 --- a/lib/active_model_serializers/adapter/fragment_cache.rb +++ /dev/null @@ -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 User::Admin - # 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 diff --git a/lib/active_model_serializers/adapter/json.rb b/lib/active_model_serializers/adapter/json.rb index 9652a04f0..3cc364365 100644 --- a/lib/active_model_serializers/adapter/json.rb +++ b/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 diff --git a/lib/active_model_serializers/adapter/json/fragment_cache.rb b/lib/active_model_serializers/adapter/json/fragment_cache.rb deleted file mode 100644 index d042063ad..000000000 --- a/lib/active_model_serializers/adapter/json/fragment_cache.rb +++ /dev/null @@ -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 diff --git a/lib/active_model_serializers/adapter/json_api.rb b/lib/active_model_serializers/adapter/json_api.rb index 956fa3d67..1fbfabe0d 100644 --- a/lib/active_model_serializers/adapter/json_api.rb +++ b/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 @@ def failure_document 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 diff --git a/lib/active_model_serializers/adapter/json_api/fragment_cache.rb b/lib/active_model_serializers/adapter/json_api/fragment_cache.rb deleted file mode 100644 index d29534402..000000000 --- a/lib/active_model_serializers/adapter/json_api/fragment_cache.rb +++ /dev/null @@ -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 diff --git a/lib/active_model_serializers/cached_serializer.rb b/lib/active_model_serializers/cached_serializer.rb new file mode 100644 index 000000000..8bc85600b --- /dev/null +++ b/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 diff --git a/lib/active_model_serializers/fragment_cache.rb b/lib/active_model_serializers/fragment_cache.rb new file mode 100644 index 000000000..425f147b0 --- /dev/null +++ b/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 User::Admin + # 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 diff --git a/test/serializers/cached_serializer_test.rb b/test/active_model_serializers/cached_serializer_test.rb similarity index 100% rename from test/serializers/cached_serializer_test.rb rename to test/active_model_serializers/cached_serializer_test.rb diff --git a/test/active_model_serializers/fragment_cache_test.rb b/test/active_model_serializers/fragment_cache_test.rb new file mode 100644 index 000000000..b44b90524 --- /dev/null +++ b/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 diff --git a/test/adapter/fragment_cache_test.rb b/test/adapter/fragment_cache_test.rb deleted file mode 100644 index a240b56e3..000000000 --- a/test/adapter/fragment_cache_test.rb +++ /dev/null @@ -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 diff --git a/test/serializers/cache_test.rb b/test/cache_test.rb similarity index 99% rename from test/serializers/cache_test.rb rename to test/cache_test.rb index 7c76d2704..1d1c9fc2a 100644 --- a/test/serializers/cache_test.rb +++ b/test/cache_test.rb @@ -150,7 +150,7 @@ def test_object_cache_keys 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' }