diff --git a/lib/active_model/serializer/caching.rb b/lib/active_model/serializer/caching.rb index 85a395d3..d1db9a3d 100644 --- a/lib/active_model/serializer/caching.rb +++ b/lib/active_model/serializer/caching.rb @@ -7,7 +7,7 @@ module ActiveModel 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 :_cache_key # @api private : when present, is first item in cache_key + serializer.class_attribute :_cache_key # @api private : when present, is first item in cache_key. Ignored if the serializable object defines #cache_key. serializer.class_attribute :_cache_only # @api private : when fragment caching, whitelists cached_attributes. Cannot combine with except serializer.class_attribute :_cache_except # @api private : when fragment caching, blacklists cached_attributes. Cannot combine with only serializer.class_attribute :_cache_options # @api private : used by CachedSerializer, passed to _cache.fetch @@ -58,6 +58,10 @@ module ActiveModel ''.freeze end + def _skip_digest? + _cache_options && _cache_options[:skip_digest] + end + # @api private # Used by FragmentCache on the CachedSerializer # to call attribute methods on the fragmented cached serializer. @@ -142,6 +146,18 @@ module ActiveModel (_cache_only && !_cache_except || !_cache_only && _cache_except) end end + + # Use object's cache_key if available, else derive a key from the object + # Pass the `key` option to the `cache` declaration or override this method to customize the cache key + def cache_key + if object.respond_to?(:cache_key) + object.cache_key + else + object_time_safe = object.updated_at + object_time_safe = object_time_safe.strftime('%Y%m%d%H%M%S%9N') if object_time_safe.respond_to?(:strftime) + "#{self.class._cache_key}/#{object.id}-#{object_time_safe}" + end + end end end end diff --git a/lib/active_model_serializers/adapter/base.rb b/lib/active_model_serializers/adapter/base.rb index cc6092d6..9fa62edc 100644 --- a/lib/active_model_serializers/adapter/base.rb +++ b/lib/active_model_serializers/adapter/base.rb @@ -8,6 +8,10 @@ module ActiveModelSerializers ActiveModelSerializers::Adapter.register(subclass) end + def self.name + to_s.demodulize + end + attr_reader :serializer, :instance_options def initialize(serializer, options = {}) @@ -15,6 +19,10 @@ module ActiveModelSerializers @instance_options = options end + def name + self.class.name + end + def serializable_hash(_options = nil) fail NotImplementedError, 'This is an abstract method. Should be implemented at the concrete adapter.' end diff --git a/lib/active_model_serializers/cached_serializer.rb b/lib/active_model_serializers/cached_serializer.rb index 11a6259d..117ff0d8 100644 --- a/lib/active_model_serializers/cached_serializer.rb +++ b/lib/active_model_serializers/cached_serializer.rb @@ -1,13 +1,16 @@ module ActiveModelSerializers class CachedSerializer + UndefinedCacheKey = Class.new(StandardError) + def initialize(serializer) @cached_serializer = serializer - @klass = @cached_serializer.class + return unless cached? && !@cached_serializer.object.respond_to?(:cache_key) && @klass._cache_key.blank? + fail(UndefinedCacheKey, "#{@cached_serializer.object} must define #cache_key, or the cache_key option must be passed into cache on #{@cached_serializer}") end def cache_check(adapter_instance) if cached? - @klass._cache.fetch(cache_key, @klass._cache_options) do + @klass._cache.fetch(cache_key(adapter_instance), @klass._cache_options) do yield end elsif fragment_cached? @@ -25,21 +28,16 @@ module ActiveModelSerializers @klass.fragment_cache_enabled? end - def cache_key + def cache_key(adapter_instance) 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] + parts << @cached_serializer.cache_key + parts << adapter_instance.name.underscore + parts << @klass._cache_digest unless @klass._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 diff --git a/test/cache_test.rb b/test/cache_test.rb index d4c9fb72..f30d0284 100644 --- a/test/cache_test.rb +++ b/test/cache_test.rb @@ -4,6 +4,21 @@ require 'tempfile' module ActiveModelSerializers class CacheTest < ActiveSupport::TestCase + UncachedAuthor = Class.new(Author) do + # To confirm cache_key is set using updated_at and cache_key option passed to cache + undef_method :cache_key + end + + Article = Class.new(::Model) do + # To confirm error is raised when cache_key is not set and cache_key option not passed to cache + undef_method :cache_key + end + + ArticleSerializer = Class.new(ActiveModel::Serializer) do + cache only: [:place], skip_digest: true + attributes :title + end + InheritedRoleSerializer = Class.new(RoleSerializer) do cache key: 'inherited_role', only: [:name, :special_attribute] attribute :special_attribute @@ -81,15 +96,26 @@ module ActiveModelSerializers assert_equal(nil, @comment_serializer.class._cache_key) end - def test_cache_key_interpolation_with_updated_at - render_object_with_cache(@author) - assert_equal(nil, cache_store.fetch(@author.cache_key)) - assert_equal(@author_serializer.attributes.to_json, cache_store.fetch("#{@author_serializer.class._cache_key}/#{@author_serializer.object.id}-#{@author_serializer.object.updated_at.strftime("%Y%m%d%H%M%S%9N")}").to_json) + + def test_error_is_raised_if_cache_key_is_not_defined_on_object_or_passed_as_cache_option + article = Article.new(title: 'Must Read') + assert_raises ActiveModel::Serializer::Adapter::CachedSerializer::UndefinedCacheKey do + render_object_with_cache(article) + end + end + + def test_cache_key_interpolation_with_updated_at_when_cache_key_is_not_defined_on_object + uncached_author = UncachedAuthor.new(name: 'Joao M. D. Moura') + uncached_author_serializer = AuthorSerializer.new(uncached_author) + + render_object_with_cache(uncached_author) + key = cache_key_with_adapter("#{uncached_author_serializer.class._cache_key}/#{uncached_author_serializer.object.id}-#{uncached_author_serializer.object.updated_at.strftime("%Y%m%d%H%M%S%9N")}") + assert_equal(uncached_author_serializer.attributes.to_json, ActionController::Base.cache_store.fetch(key).to_json) end def test_default_cache_key_fallback render_object_with_cache(@comment) - assert_equal(@comment_serializer.attributes.to_json, cache_store.fetch(@comment.cache_key).to_json) + assert_equal(@comment_serializer.attributes.to_json, ActionController::Base.cache_store.fetch(cache_key_with_adapter(@comment.cache_key)).to_json) end def test_cache_options_definition @@ -111,8 +137,8 @@ module ActiveModelSerializers Timecop.freeze(Time.current) do render_object_with_cache(@post) - assert_equal(@post_serializer.attributes, cache_store.fetch(@post.cache_key)) - assert_equal(@comment_serializer.attributes, cache_store.fetch(@comment.cache_key)) + assert_equal(@post_serializer.attributes, ActionController::Base.cache_store.fetch(cache_key_with_adapter(@post.cache_key))) + assert_equal(@comment_serializer.attributes, ActionController::Base.cache_store.fetch(cache_key_with_adapter(@comment.cache_key))) end end @@ -122,8 +148,9 @@ module ActiveModelSerializers render_object_with_cache(@post) # Check if it cached the objects separately - assert_equal(@post_serializer.attributes, cached_serialization(@post_serializer)) - assert_equal(@comment_serializer.attributes, cached_serialization(@comment_serializer)) + assert_equal(@post_serializer.attributes, ActionController::Base.cache_store.fetch(cache_key_with_adapter(@post.cache_key))) + assert_equal(@comment_serializer.attributes, ActionController::Base.cache_store.fetch(cache_key_with_adapter(@comment.cache_key))) + # Simulating update on comments relationship with Post new_comment = Comment.new(id: 2567, body: 'ZOMG A NEW COMMENT') @@ -134,8 +161,8 @@ module ActiveModelSerializers render_object_with_cache(@post) # Check if the the new comment was cached - assert_equal(new_comment_serializer.attributes, cached_serialization(new_comment_serializer)) - assert_equal(@post_serializer.attributes, cached_serialization(@post_serializer)) + assert_equal(new_comment_serializer.attributes, ActionController::Base.cache_store.fetch(cache_key_with_adapter(new_comment.cache_key))) + assert_equal(@post_serializer.attributes, ActionController::Base.cache_store.fetch(cache_key_with_adapter(@post.cache_key))) end end @@ -150,7 +177,7 @@ module ActiveModelSerializers hash = render_object_with_cache(@location) assert_equal(hash, expected_result) - assert_equal({ place: 'Nowhere' }, cache_store.fetch(@location.cache_key)) + assert_equal({ place: 'Nowhere' }, ActionController::Base.cache_store.fetch(cache_key_with_adapter(@location.cache_key))) end def test_fragment_cache_with_inheritance @@ -161,9 +188,14 @@ module ActiveModelSerializers refute_includes(base.keys, :special_attribute) end + def test_uses_adapter_in_cache_key + render_object_with_cache(@post) + assert_equal(@post_serializer.attributes, ActionController::Base.cache_store.fetch("#{@post.cache_key}/#{adapter.class.to_s.demodulize.underscore}")) + end + def test_uses_file_digest_in_cache_key render_object_with_cache(@blog) - assert_equal(@blog_serializer.attributes, cache_store.fetch(@blog.cache_key_with_digest)) + assert_equal(@blog_serializer.attributes, ActionController::Base.cache_store.fetch("#{cache_key_with_adapter(@blog.cache_key)}/#{@blog.class::FILE_DIGEST}")) end def test_cache_digest_definition @@ -254,7 +286,16 @@ module ActiveModelSerializers private def render_object_with_cache(obj, options = {}) - serializable(obj, options).serializable_hash + @serializable_resource = serializable(obj, options).serializable_hash + @serializable_resource.serializable_hash + end + + def adapter + @serializable_resource.adapter + end + + def cache_key_with_adapter(key) + "#{key}/#{adapter.name.underscore}" end def cache_store diff --git a/test/fixtures/poro.rb b/test/fixtures/poro.rb index b1351364..c2bddf8e 100644 --- a/test/fixtures/poro.rb +++ b/test/fixtures/poro.rb @@ -55,8 +55,8 @@ Bio = Class.new(Model) Blog = Class.new(Model) do FILE_DIGEST = Digest::MD5.hexdigest(File.open(__FILE__).read) - def cache_key_with_digest - "#{cache_key}/#{FILE_DIGEST}" + def digest + FILE_DIGEST end end Role = Class.new(Model)