diff --git a/CHANGELOG.md b/CHANGELOG.md index 8340262f1..1eb04825d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ Breaking changes: Features: +- [#1644](https://github.com/rails-api/active_model_serializers/pull/1644) Include adapter name in cache key so + that the same serializer can be cached per adapter. (@bf4 via #1346 by @kevintyll) - [#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 diff --git a/lib/active_model/serializer/caching.rb b/lib/active_model/serializer/caching.rb index 0d2160af9..8577fdcae 100644 --- a/lib/active_model/serializer/caching.rb +++ b/lib/active_model/serializer/caching.rb @@ -58,6 +58,10 @@ def digest_caller_file(caller_line) ''.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. 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 cc6092d6b..71eb59abf 100644 --- a/lib/active_model_serializers/adapter/base.rb +++ b/lib/active_model_serializers/adapter/base.rb @@ -15,6 +15,10 @@ def initialize(serializer, options = {}) @instance_options = options end + def cached_name + @cached_name ||= self.class.name.demodulize.underscore + 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 9eea39673..c9b561206 100644 --- a/lib/active_model_serializers/cached_serializer.rb +++ b/lib/active_model_serializers/cached_serializer.rb @@ -9,7 +9,7 @@ def initialize(serializer) 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? @@ -27,12 +27,13 @@ def fragment_cached? @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 << adapter_instance.cached_name + parts << @klass._cache_digest unless @klass._skip_digest? @cache_key = parts.join('/') end @@ -54,19 +55,19 @@ def object_cache_key # @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 @@ -75,11 +76,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 d5049656e..283f35b80 100644 --- a/test/cache_test.rb +++ b/test/cache_test.rb @@ -102,12 +102,13 @@ def test_cache_key_interpolation_with_updated_at_when_cache_key_is_not_defined_o 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")}" + key = "#{key}/#{adapter.cached_name}" 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) - key = @comment.cache_key + key = "#{@comment.cache_key}/#{adapter.cached_name}" assert_equal(@comment_serializer.attributes.to_json, cache_store.fetch(key).to_json) end @@ -138,9 +139,9 @@ def test_associations_separately_cache Timecop.freeze(Time.current) do render_object_with_cache(@post) - key = @post.cache_key + key = "#{@post.cache_key}/#{adapter.cached_name}" assert_equal(@post_serializer.attributes, cache_store.fetch(key)) - key = @comment.cache_key + key = "#{@comment.cache_key}/#{adapter.cached_name}" assert_equal(@comment_serializer.attributes, cache_store.fetch(key)) end end @@ -151,9 +152,9 @@ def test_associations_cache_when_updated render_object_with_cache(@post) # Check if it cached the objects separately - key = @post.cache_key + key = "#{@post.cache_key}/#{adapter.cached_name}" assert_equal(@post_serializer.attributes, cache_store.fetch(key)) - key = @comment.cache_key + key = "#{@comment.cache_key}/#{adapter.cached_name}" assert_equal(@comment_serializer.attributes, cache_store.fetch(key)) # Simulating update on comments relationship with Post @@ -165,9 +166,9 @@ def test_associations_cache_when_updated render_object_with_cache(@post) # Check if the the new comment was cached - key = new_comment.cache_key + key = "#{new_comment.cache_key}/#{adapter.cached_name}" assert_equal(new_comment_serializer.attributes, cache_store.fetch(key)) - key = @post.cache_key + key = "#{@post.cache_key}/#{adapter.cached_name}" assert_equal(@post_serializer.attributes, cache_store.fetch(key)) end end @@ -183,7 +184,8 @@ def test_fragment_fetch_with_virtual_associations hash = render_object_with_cache(@location) assert_equal(hash, expected_result) - assert_equal({ place: 'Nowhere' }, cache_store.fetch(@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 @@ -194,9 +196,87 @@ def test_fragment_cache_with_inheritance refute_includes(base.keys, :special_attribute) end + def test_uses_adapter_in_cache_key + render_object_with_cache(@post) + key = "#{@post.cache_key}/#{adapter.class.to_s.demodulize.underscore}" + assert_equal(@post_serializer.attributes, cache_store.fetch(key)) + end + + # Based on original failing test by @kevintyll + # rubocop:disable Metrics/AbcSize + def test_a_serializer_rendered_by_two_adapter_returns_differently_cached_attributes + Object.const_set(:Alert, Class.new(ActiveModelSerializers::Model) do + attr_accessor :id, :status, :resource, :started_at, :ended_at, :updated_at, :created_at + end) + Object.const_set(:UncachedAlertSerializer, Class.new(ActiveModel::Serializer) do + attributes :id, :status, :resource, :started_at, :ended_at, :updated_at, :created_at + end) + Object.const_set(:AlertSerializer, Class.new(UncachedAlertSerializer) do + cache + end) + + alert = Alert.new( + id: 1, + status: 'fail', + resource: 'resource-1', + started_at: Time.new(2016, 3, 31, 21, 36, 35, 0), + ended_at: nil, + updated_at: Time.new(2016, 3, 31, 21, 27, 35, 0), + created_at: Time.new(2016, 3, 31, 21, 37, 35, 0) + ) + + expected_cached_attributes = { + id: 1, + status: 'fail', + resource: 'resource-1', + started_at: alert.started_at, + ended_at: nil, + updated_at: alert.updated_at, + created_at: alert.created_at + } + expected_cached_jsonapi_attributes = { + id: '1', + type: 'alerts', + attributes: { + status: 'fail', + resource: 'resource-1', + started_at: alert.started_at, + ended_at: nil, + updated_at: alert.updated_at, + created_at: alert.created_at + } + } + + # Assert attributes are serialized correctly + serializable_alert = serializable(alert, serializer: AlertSerializer, adapter: :attributes) + attributes_serialization = serializable_alert.as_json + assert_equal expected_cached_attributes, alert.attributes + assert_equal alert.attributes, attributes_serialization + attributes_cache_key = CachedSerializer.new(serializable_alert.adapter.serializer).cache_key(serializable_alert.adapter) + assert_equal attributes_serialization, cache_store.fetch(attributes_cache_key) + + serializable_alert = serializable(alert, serializer: AlertSerializer, adapter: :json_api) + jsonapi_cache_key = CachedSerializer.new(serializable_alert.adapter.serializer).cache_key(serializable_alert.adapter) + # Assert cache keys differ + refute_equal attributes_cache_key, jsonapi_cache_key + # Assert (cached) serializations differ + jsonapi_serialization = serializable_alert.as_json + assert_equal alert.status, jsonapi_serialization.fetch(:data).fetch(:attributes).fetch(:status) + serializable_alert = serializable(alert, serializer: UncachedAlertSerializer, adapter: :json_api) + assert_equal serializable_alert.as_json, jsonapi_serialization + + cached_serialization = cache_store.fetch(jsonapi_cache_key) + assert_equal expected_cached_jsonapi_attributes, cached_serialization + ensure + Object.send(:remove_const, :Alert) + Object.send(:remove_const, :AlertSerializer) + Object.send(:remove_const, :UncachedAlertSerializer) + end + # rubocop:enable Metrics/AbcSize + def test_uses_file_digest_in_cache_key render_object_with_cache(@blog) - key = "#{@blog.cache_key}/#{::Model::FILE_DIGEST}" + key = "#{@blog.cache_key}/#{adapter.cached_name}/#{::Model::FILE_DIGEST}" assert_equal(@blog_serializer.attributes, cache_store.fetch(key)) end @@ -205,13 +285,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{author/author-\d+} } end @@ -226,13 +306,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.cache_key - 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 @@ -287,16 +367,21 @@ 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(obj, options).serializable_hash + @serializable_resource = serializable(obj, options) + @serializable_resource.serializable_hash end - def cache_store - ActiveModelSerializers.config.cache_store + def adapter + @serializable_resource.adapter 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