Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include adapter in cache key #1644

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions lib/active_model/serializer/caching.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions lib/active_model_serializers/adapter/attributes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?

Expand All @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions lib/active_model_serializers/adapter/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 10 additions & 9 deletions lib/active_model_serializers/cached_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand All @@ -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

Expand All @@ -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
Expand All @@ -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
123 changes: 104 additions & 19 deletions test/cache_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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

Expand All @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down