diff --git a/CHANGELOG.md b/CHANGELOG.md index 03daa367..8340262f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ Breaking changes: Features: +- [#1642](https://github.com/rails-api/active_model_serializers/pull/1642) Prefer object.cache_key over the generated + cache key. (@bf4 via #1346 by @kevintyll) - [#1637](https://github.com/rails-api/active_model_serializers/pull/1637) Make references to 'ActionController::Base.cache_store' explicit in order to avoid issues when application controllers inherit from 'ActionController::API'. (@ncuesta) - [#1633](https://github.com/rails-api/active_model_serializers/pull/1633) Yield 'serializer' to serializer association blocks. (@bf4) diff --git a/lib/active_model/serializer/caching.rb b/lib/active_model/serializer/caching.rb index d1db9a3d..0d2160af 100644 --- a/lib/active_model/serializer/caching.rb +++ b/lib/active_model/serializer/caching.rb @@ -58,10 +58,6 @@ 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. @@ -146,18 +142,6 @@ 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 9fa62edc..cc6092d6 100644 --- a/lib/active_model_serializers/adapter/base.rb +++ b/lib/active_model_serializers/adapter/base.rb @@ -8,10 +8,6 @@ module ActiveModelSerializers ActiveModelSerializers::Adapter.register(subclass) end - def self.name - to_s.demodulize - end - attr_reader :serializer, :instance_options def initialize(serializer, options = {}) @@ -19,10 +15,6 @@ 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 117ff0d8..9eea3967 100644 --- a/lib/active_model_serializers/cached_serializer.rb +++ b/lib/active_model_serializers/cached_serializer.rb @@ -4,13 +4,12 @@ module ActiveModelSerializers def initialize(serializer) @cached_serializer = serializer - 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}") + @klass = @cached_serializer.class end def cache_check(adapter_instance) if cached? - @klass._cache.fetch(cache_key(adapter_instance), @klass._cache_options) do + @klass._cache.fetch(cache_key, @klass._cache_options) do yield end elsif fragment_cached? @@ -28,16 +27,29 @@ module ActiveModelSerializers @klass.fragment_cache_enabled? end - def cache_key(adapter_instance) + def cache_key return @cache_key if defined?(@cache_key) parts = [] - parts << @cached_serializer.cache_key - parts << adapter_instance.name.underscore - parts << @klass._cache_digest unless @klass._skip_digest? + parts << object_cache_key + parts << @klass._cache_digest unless @klass._cache_options && @klass._cache_options[:skip_digest] @cache_key = parts.join('/') 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 object_cache_key + if @cached_serializer.object.respond_to?(:cache_key) + @cached_serializer.object.cache_key + elsif (cache_key = (@klass._cache_key || @klass._cache_options[: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) + "#{cache_key}/#{@cached_serializer.object.id}-#{object_time_safe}" + else + fail UndefinedCacheKey, "#{@cached_serializer.object.class} must define #cache_key, or the 'key:' option must be passed into '#{@klass}.cache'" + end + 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 f30d0284..d5049656 100644 --- a/test/cache_test.rb +++ b/test/cache_test.rb @@ -96,26 +96,27 @@ module ActiveModelSerializers assert_equal(nil, @comment_serializer.class._cache_key) end - - 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) + key = "#{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, 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, ActionController::Base.cache_store.fetch(cache_key_with_adapter(@comment.cache_key)).to_json) + key = @comment.cache_key + assert_equal(@comment_serializer.attributes.to_json, cache_store.fetch(key).to_json) + end + + def test_error_is_raised_if_cache_key_is_not_defined_on_object_or_passed_as_cache_option + article = Article.new(title: 'Must Read') + e = assert_raises ActiveModelSerializers::CachedSerializer::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) end def test_cache_options_definition @@ -137,8 +138,10 @@ module ActiveModelSerializers Timecop.freeze(Time.current) do render_object_with_cache(@post) - 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))) + key = @post.cache_key + assert_equal(@post_serializer.attributes, cache_store.fetch(key)) + key = @comment.cache_key + assert_equal(@comment_serializer.attributes, cache_store.fetch(key)) end end @@ -148,9 +151,10 @@ module ActiveModelSerializers render_object_with_cache(@post) # Check if it cached the objects separately - 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))) - + key = @post.cache_key + assert_equal(@post_serializer.attributes, cache_store.fetch(key)) + key = @comment.cache_key + assert_equal(@comment_serializer.attributes, cache_store.fetch(key)) # Simulating update on comments relationship with Post new_comment = Comment.new(id: 2567, body: 'ZOMG A NEW COMMENT') @@ -161,8 +165,10 @@ module ActiveModelSerializers render_object_with_cache(@post) # Check if the the new comment was cached - 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))) + key = new_comment.cache_key + assert_equal(new_comment_serializer.attributes, cache_store.fetch(key)) + key = @post.cache_key + assert_equal(@post_serializer.attributes, cache_store.fetch(key)) end end @@ -177,7 +183,7 @@ module ActiveModelSerializers hash = render_object_with_cache(@location) assert_equal(hash, expected_result) - assert_equal({ place: 'Nowhere' }, ActionController::Base.cache_store.fetch(cache_key_with_adapter(@location.cache_key))) + assert_equal({ place: 'Nowhere' }, cache_store.fetch(@location.cache_key)) end def test_fragment_cache_with_inheritance @@ -188,18 +194,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, ActionController::Base.cache_store.fetch("#{cache_key_with_adapter(@blog.cache_key)}/#{@blog.class::FILE_DIGEST}")) + key = "#{@blog.cache_key}/#{::Model::FILE_DIGEST}" + assert_equal(@blog_serializer.attributes, cache_store.fetch(key)) end def test_cache_digest_definition - assert_equal(FILE_DIGEST, @post_serializer.class._cache_digest) + assert_equal(::Model::FILE_DIGEST, @post_serializer.class._cache_digest) end def test_object_cache_keys @@ -211,7 +213,7 @@ module ActiveModelSerializers assert_equal actual.size, 3 assert actual.any? { |key| key == 'comment/1' } assert actual.any? { |key| key =~ %r{post/post-\d+} } - assert actual.any? { |key| key =~ %r{writer/author-\d+} } + assert actual.any? { |key| key =~ %r{author/author-\d+} } end def test_cached_attributes @@ -228,7 +230,7 @@ module ActiveModelSerializers assert_equal cached_attributes[@comment.post.cache_key], Post.new(id: 'post', title: 'New Post', body: 'Body').attributes writer = @comment.post.blog.writer - writer_cache_key = "writer/#{writer.id}-#{writer.updated_at.strftime("%Y%m%d%H%M%S%9N")}" + writer_cache_key = writer.cache_key assert_equal cached_attributes[writer_cache_key], Author.new(id: 'author', name: 'Joao M. D. Moura').attributes end @@ -286,16 +288,7 @@ module ActiveModelSerializers private def render_object_with_cache(obj, options = {}) - @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}" + serializable(obj, options).serializable_hash end def cache_store diff --git a/test/fixtures/poro.rb b/test/fixtures/poro.rb index c2bddf8e..9689e615 100644 --- a/test/fixtures/poro.rb +++ b/test/fixtures/poro.rb @@ -1,6 +1,8 @@ verbose = $VERBOSE $VERBOSE = nil class Model < ActiveModelSerializers::Model + FILE_DIGEST = Digest::MD5.hexdigest(File.open(__FILE__).read) + ### Helper methods, not required to be serializable # Convenience when not adding @attributes readers and writers @@ -52,13 +54,7 @@ Post = Class.new(Model) Like = Class.new(Model) Author = Class.new(Model) Bio = Class.new(Model) -Blog = Class.new(Model) do - FILE_DIGEST = Digest::MD5.hexdigest(File.open(__FILE__).read) - - def digest - FILE_DIGEST - end -end +Blog = Class.new(Model) Role = Class.new(Model) User = Class.new(Model) Location = Class.new(Model)