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

Use a more precise generated cache key #939

Merged
merged 1 commit into from
Jun 26, 2015
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
4 changes: 3 additions & 1 deletion lib/active_model/serializer/adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ def cache_key
end

def object_cache_key
(@klass._cache_key) ? "#{@klass._cache_key}/#{@cached_serializer.object.id}-#{@cached_serializer.object.updated_at}" : @cached_serializer.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

def meta
Expand Down
14 changes: 6 additions & 8 deletions test/action_controller/serialization_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,12 @@ def render_fragment_changed_object_with_except_cache_enabled

def render_fragment_changed_object_with_relationship
comment = Comment.new({ id: 1, body: 'ZOMG A COMMENT' })
comment2 = Comment.new({ id: 1, body: 'ZOMG AN UPDATED-BUT-NOT-CACHE-EXPIRED COMMENT' })
author = Author.new(id: 1, name: 'Joao Moura.')
post = Post.new({ id: 1, title: 'New Post', body: 'Body', comments: [comment], author: author })
post2 = Post.new({ id: 1, title: 'New Post2', body: 'Body2', comments: [comment], author: author })
like = Like.new({ id: 1, post: post, time: 3.days.ago })
like = Like.new({ id: 1, likeable: comment, time: 3.days.ago })

generate_cached_serializer(like)
like.post = post2
like.likable = comment2
like.time = DateTime.now.to_s

render json: like
Expand Down Expand Up @@ -229,7 +228,7 @@ def test_render_with_cache_enable
assert_equal expected.to_json, @response.body

get :render_changed_object_with_cache_enabled
assert_equal expected.to_json, @response.body
assert_not_equal expected.to_json, @response.body
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This used to be able to be assert_equal because the cache key wasn't a high enough resolution to detect the change. This was actually incorrect (and/or bad caching), so at a minimum I changed the test to assert on the correct result, which it now receives.


ActionController::Base.cache_store.clear
get :render_changed_object_with_cache_enabled
Expand Down Expand Up @@ -291,10 +290,9 @@ def test_render_fragment_changed_object_with_relationship
expected_return = {
"id"=>1,
"time"=>DateTime.now.to_s,
"post" => {
"likeable" => {
"id"=>1,
"title"=>"New Post",
"body"=>"Body"
"body"=>"ZOMG A COMMENT"
}
}

Expand Down
13 changes: 9 additions & 4 deletions test/fixtures/poro.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ def initialize(hash={})
end

def cache_key
"#{self.class.name.downcase}/#{self.id}-#{self.updated_at}"
"#{self.class.name.downcase}/#{self.id}-#{self.updated_at.strftime("%Y%m%d%H%M%S%9N")}"
end

def cache_key_with_digest
"#{cache_key}/#{FILE_DIGEST}"
end

def updated_at
@attributes[:updated_at] ||= DateTime.now.to_time.to_i
@attributes[:updated_at] ||= DateTime.now.to_time
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This usage of #to_i was one of the root causes behind the tests passing when they shouldn't have, and (incidentally) the tests not testing for real-world scenarios where updated_at isn't an integer, but is an instance of Time (or more likely an ActiveSupport::TimeWithZone instance.)

end

def read_attribute_for_serialization(name)
Expand Down Expand Up @@ -69,14 +69,19 @@ class ProfilePreviewSerializer < ActiveModel::Serializer

Post = Class.new(Model)
Like = Class.new(Model)
Comment = Class.new(Model)
Author = Class.new(Model)
Bio = Class.new(Model)
Blog = Class.new(Model)
Role = Class.new(Model)
User = Class.new(Model)
Location = Class.new(Model)
Place = Class.new(Model)
Comment = Class.new(Model) do
# Uses a custom non-time-based cache key
def cache_key
"#{self.class.name.downcase}/#{self.id}"
end
end

module Spam; end
Spam::UnrelatedLink = Class.new(Model)
Expand Down Expand Up @@ -143,7 +148,7 @@ def slug
LikeSerializer = Class.new(ActiveModel::Serializer) do
attributes :id, :time

belongs_to :post
belongs_to :likeable
end

LocationSerializer = Class.new(ActiveModel::Serializer) do
Expand Down
4 changes: 2 additions & 2 deletions test/serializers/cache_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def test_cache_key_definition
def test_cache_key_interpolation_with_updated_at
author = render_object_with_cache(@author)
assert_equal(nil, ActionController::Base.cache_store.fetch(@author.cache_key))
assert_equal(@author_serializer.attributes.to_json, ActionController::Base.cache_store.fetch("#{@author_serializer.class._cache_key}/#{@author_serializer.object.id}-#{@author_serializer.object.updated_at}").to_json)
assert_equal(@author_serializer.attributes.to_json, ActionController::Base.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)
end

def test_default_cache_key_fallback
Expand Down Expand Up @@ -85,7 +85,7 @@ def test_associations_cache_when_updated
# Generate a new Cache of Post object and each objects related to it.
render_object_with_cache(@post)

# Check if if cache the objects separately
# Check if it cached the objects separately
assert_equal(@post_serializer.attributes, ActionController::Base.cache_store.fetch(@post.cache_key))
assert_equal(@comment_serializer.attributes, ActionController::Base.cache_store.fetch(@comment.cache_key))

Expand Down