From 9e4b5c208b00aad76f12067a47be323dfa0cafc6 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Thu, 31 Mar 2016 13:18:20 -0500 Subject: [PATCH] Include adapter in cache key --- lib/active_model/serializer/caching.rb | 12 --- .../adapter/attributes.rb | 4 +- lib/active_model_serializers/adapter/base.rb | 8 +- .../cached_serializer.rb | 27 +++--- test/cache_test.rb | 89 +++++++------------ test/fixtures/poro.rb | 4 - 6 files changed, 51 insertions(+), 93 deletions(-) diff --git a/lib/active_model/serializer/caching.rb b/lib/active_model/serializer/caching.rb index d1db9a3db..8577fdcae 100644 --- a/lib/active_model/serializer/caching.rb +++ b/lib/active_model/serializer/caching.rb @@ -146,18 +146,6 @@ def fragment_cache_enabled? (_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/attributes.rb b/lib/active_model_serializers/adapter/attributes.rb index 8281392b2..e2437c331 100644 --- a/lib/active_model_serializers/adapter/attributes.rb +++ b/lib/active_model_serializers/adapter/attributes.rb @@ -30,7 +30,7 @@ def serializable_hash_for_collection(options) def cache_read_multi return {} if ActiveModelSerializers.config.cache_store.blank? - keys = CachedSerializer.object_cache_keys(serializer, @include_tree) + keys = CachedSerializer.object_cache_keys(serializer, self, @include_tree) return {} if keys.blank? @@ -49,7 +49,7 @@ def cache_attributes def cached_attributes(cached_serializer) return yield unless cached_serializer.cached? - @cached_attributes.fetch(cached_serializer.cache_key) { yield } + @cached_attributes.fetch(cached_serializer.cache_key(self)) { yield } end def serializable_hash_for_single_resource(options) diff --git a/lib/active_model_serializers/adapter/base.rb b/lib/active_model_serializers/adapter/base.rb index 9fa62edce..71eb59abf 100644 --- a/lib/active_model_serializers/adapter/base.rb +++ b/lib/active_model_serializers/adapter/base.rb @@ -8,10 +8,6 @@ def self.inherited(subclass) ActiveModelSerializers::Adapter.register(subclass) end - def self.name - to_s.demodulize - end - attr_reader :serializer, :instance_options def initialize(serializer, options = {}) @@ -19,8 +15,8 @@ def initialize(serializer, options = {}) @instance_options = options end - def name - self.class.name + def cached_name + @cached_name ||= self.class.name.demodulize.underscore end def serializable_hash(_options = nil) diff --git a/lib/active_model_serializers/cached_serializer.rb b/lib/active_model_serializers/cached_serializer.rb index 117ff0d8f..7478201da 100644 --- a/lib/active_model_serializers/cached_serializer.rb +++ b/lib/active_model_serializers/cached_serializer.rb @@ -1,11 +1,8 @@ module ActiveModelSerializers class CachedSerializer - UndefinedCacheKey = Class.new(StandardError) - 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) @@ -32,29 +29,35 @@ def cache_key(adapter_instance) return @cache_key if defined?(@cache_key) parts = [] - parts << @cached_serializer.cache_key - parts << adapter_instance.name.underscore + parts << object_cache_key + parts << adapter_instance.cached_name if adapter_instance 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 # @return [Array] all cache_key of collection_serializer - def self.object_cache_keys(serializers, include_tree) + def self.object_cache_keys(serializers, adapter_instance, include_tree) cache_keys = [] serializers.each do |serializer| - cache_keys << object_cache_key(serializer) + cache_keys << object_cache_key(serializer, adapter_instance) 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) + cache_keys << object_cache_key(sub_serializer, adapter_instance) end else - cache_keys << object_cache_key(association.serializer) + cache_keys << object_cache_key(association.serializer, adapter_instance) end end end @@ -63,11 +66,11 @@ def self.object_cache_keys(serializers, include_tree) end # @return [String, nil] the cache_key of the serializer or nil - def self.object_cache_key(serializer) + def self.object_cache_key(serializer, adapter_instance) return unless serializer.present? && serializer.object.present? cached_serializer = new(serializer) - cached_serializer.cached? ? cached_serializer.cache_key : nil + cached_serializer.cached? ? cached_serializer.cache_key(adapter_instance) : nil end end end diff --git a/test/cache_test.rb b/test/cache_test.rb index f30d02845..f9e58bed2 100644 --- a/test/cache_test.rb +++ b/test/cache_test.rb @@ -4,21 +4,6 @@ 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 @@ -96,26 +81,18 @@ def test_cache_key_definition 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) + def test_cache_key_interpolation_with_updated_at + render_object_with_cache(@author) + assert_equal(nil, cache_store.fetch(@author.cache_key)) + key = "#{@author_serializer.class._cache_key}/#{@author_serializer.object.id}-#{@author_serializer.object.updated_at.strftime("%Y%m%d%H%M%S%9N")}" + key = "#{key}/#{adapter.cached_name}" + assert_equal(@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}/#{adapter.cached_name}" + assert_equal(@comment_serializer.attributes.to_json, cache_store.fetch(key).to_json) end def test_cache_options_definition @@ -137,8 +114,8 @@ def test_associations_separately_cache 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))) + assert_equal(@post_serializer.attributes, cache_store.fetch("#{@post.cache_key}/#{adapter.cached_name}")) + assert_equal(@comment_serializer.attributes, cache_store.fetch("#{@comment.cache_key}/#{adapter.cached_name}")) end end @@ -148,9 +125,8 @@ def test_associations_cache_when_updated 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))) - + assert_equal(@post_serializer.attributes, cached_serialization(@post_serializer)) + assert_equal(@comment_serializer.attributes, cached_serialization(@comment_serializer)) # Simulating update on comments relationship with Post new_comment = Comment.new(id: 2567, body: 'ZOMG A NEW COMMENT') @@ -161,8 +137,8 @@ def test_associations_cache_when_updated 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))) + assert_equal(new_comment_serializer.attributes, cached_serialization(new_comment_serializer)) + assert_equal(@post_serializer.attributes, cached_serialization(@post_serializer)) end end @@ -177,7 +153,8 @@ def test_fragment_fetch_with_virtual_associations 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))) + key = "#{@location.cache_key}/#{adapter.cached_name}" + assert_equal({ place: 'Nowhere' }, cache_store.fetch(key)) end def test_fragment_cache_with_inheritance @@ -190,12 +167,14 @@ def test_fragment_cache_with_inheritance 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}")) + key = "#{@post.cache_key}/#{adapter.class.to_s.demodulize.underscore}" + assert_equal(@post_serializer.attributes, cache_store.fetch(key)) 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}/#{adapter.cached_name}/#{@blog.class.const_get(:FILE_DIGEST)}" + assert_equal @blog_serializer.attributes, cache_store.fetch(key) end def test_cache_digest_definition @@ -203,13 +182,13 @@ def test_cache_digest_definition end def test_object_cache_keys - serializer = ActiveModel::Serializer::CollectionSerializer.new([@comment, @comment]) + serializable = ActiveModelSerializers::SerializableResource.new([@comment, @comment]) include_tree = ActiveModel::Serializer::IncludeTree.from_include_args('*') - actual = CachedSerializer.object_cache_keys(serializer, include_tree) + actual = CachedSerializer.object_cache_keys(serializable.adapter.serializer, serializable.adapter, include_tree) assert_equal actual.size, 3 - assert actual.any? { |key| key == 'comment/1' } + assert actual.any? { |key| key == "comment/1/#{serializable.adapter.cached_name}" } assert actual.any? { |key| key =~ %r{post/post-\d+} } assert actual.any? { |key| key =~ %r{writer/author-\d+} } end @@ -224,13 +203,13 @@ def test_cached_attributes attributes.send(:cache_attributes) cached_attributes = attributes.instance_variable_get(:@cached_attributes) - assert_equal cached_attributes[@comment.cache_key], Comment.new(id: 1, body: 'ZOMG A COMMENT').attributes - assert_equal cached_attributes[@comment.post.cache_key], Post.new(id: 'post', title: 'New Post', body: 'Body').attributes + assert_equal cached_attributes["#{@comment.cache_key}/#{attributes.cached_name}"], Comment.new(id: 1, body: 'ZOMG A COMMENT').attributes + assert_equal cached_attributes["#{@comment.post.cache_key}/#{attributes.cached_name}"], 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")}" - assert_equal cached_attributes[writer_cache_key], Author.new(id: 'author', name: 'Joao M. D. Moura').attributes + assert_equal cached_attributes["#{writer_cache_key}/#{attributes.cached_name}"], Author.new(id: 'author', name: 'Joao M. D. Moura').attributes end end @@ -285,8 +264,12 @@ def test_warn_on_serializer_not_defined_in_file private + def cache_store + ActiveModelSerializers.config.cache_store + end + def render_object_with_cache(obj, options = {}) - @serializable_resource = serializable(obj, options).serializable_hash + @serializable_resource = serializable(obj, options) @serializable_resource.serializable_hash end @@ -294,16 +277,8 @@ def adapter @serializable_resource.adapter end - def cache_key_with_adapter(key) - "#{key}/#{adapter.name.underscore}" - end - - def cache_store - ActiveModelSerializers.config.cache_store - end - def cached_serialization(serializer) - cache_key = CachedSerializer.new(serializer).cache_key + cache_key = CachedSerializer.new(serializer).cache_key(adapter) cache_store.fetch(cache_key) end end diff --git a/test/fixtures/poro.rb b/test/fixtures/poro.rb index c2bddf8e9..ce6fd5fde 100644 --- a/test/fixtures/poro.rb +++ b/test/fixtures/poro.rb @@ -54,10 +54,6 @@ class ProfilePreviewSerializer < ActiveModel::Serializer Bio = Class.new(Model) Blog = Class.new(Model) do FILE_DIGEST = Digest::MD5.hexdigest(File.open(__FILE__).read) - - def digest - FILE_DIGEST - end end Role = Class.new(Model) User = Class.new(Model)