Skip to content

Commit

Permalink
Include adapter in cache key
Browse files Browse the repository at this point in the history
Confirm caching attributes with different key json_api vs. attributes adapter

Adapted from @kevintyll's original test
#1644 (comment)
  • Loading branch information
kevintyll authored and bf4 committed Apr 1, 2016
1 parent e118599 commit 16a3f93
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 30 deletions.
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

0 comments on commit 16a3f93

Please sign in to comment.