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 85a395d3..0d2160af 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 diff --git a/lib/active_model_serializers/cached_serializer.rb b/lib/active_model_serializers/cached_serializer.rb index 11a6259d..9eea3967 100644 --- a/lib/active_model_serializers/cached_serializer.rb +++ b/lib/active_model_serializers/cached_serializer.rb @@ -1,5 +1,7 @@ module ActiveModelSerializers class CachedSerializer + UndefinedCacheKey = Class.new(StandardError) + def initialize(serializer) @cached_serializer = serializer @klass = @cached_serializer.class @@ -34,10 +36,18 @@ module ActiveModelSerializers @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 - 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 + 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 diff --git a/test/cache_test.rb b/test/cache_test.rb index d4c9fb72..d5049656 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,27 @@ 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_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 = "#{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, cache_store.fetch(@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 @@ -111,8 +138,10 @@ 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)) + 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 @@ -122,8 +151,10 @@ 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)) + 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') @@ -134,8 +165,10 @@ 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)) + 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 @@ -163,11 +196,12 @@ module ActiveModelSerializers 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)) + 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 @@ -179,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 @@ -196,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 diff --git a/test/fixtures/poro.rb b/test/fixtures/poro.rb index b1351364..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 cache_key_with_digest - "#{cache_key}/#{FILE_DIGEST}" - end -end +Blog = Class.new(Model) Role = Class.new(Model) User = Class.new(Model) Location = Class.new(Model)