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

support read_multi #1372

Merged
merged 1 commit into from
Feb 11, 2016
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
38 changes: 36 additions & 2 deletions lib/active_model/serializer/adapter/attributes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ class Attributes < Base
def initialize(serializer, options = {})
super
@include_tree = IncludeTree.from_include_args(options[:include] || '*')
@cached_attributes = options[:cache_attributes] || {}
end

def serializable_hash(options = nil)
Expand All @@ -24,9 +25,38 @@ def fragment_cache(cached_hash, non_cached_hash)
private

def serializable_hash_for_collection(options)
cache_attributes

Copy link
Member

Choose a reason for hiding this comment

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

I think this both blocks of code added in this diff should be their own method. Maybe something like (pardon the naming... could be better)

def serializable_hash_for_collection(options)
  options.merge!(batch_cache(options))
  serializer.map { |s| Attributes.new(s, instance_options).serializable_hash(options) }
end
def batch_cache(options)
  return {} unless options[:batch_cache].blank? && ActiveModelSerializers.config.cache_store.present?
  if keys = CachedSerializer.object_cache_keys(serializer, @include_tree)
    { batch_cache: ActiveModelSerializers.config.cache_store.read_multi(*keys) }
  else
    {}
  end
end
def batch_cached_serializer(options)
  return nil unless options[:batch_cache].present?
  cache_key = CachedSerializer.new(serializer).cache_key
  options[:batch_cache][cache_key]
end
def resource_object_for(options)
  if batch_cached_serializer_attributes = batch_cached_serializer(options)
    batch_cached_serializer_attributes
  else
    cache_check(serializer) do
      serializer.attributes(options[:fields])
    end
end

def resource_object_for(options)

serializer.map { |s| Attributes.new(s, instance_options).serializable_hash(options) }
end

# Read cache from cache_store
# @return [Hash]
def cache_read_multi
return {} if ActiveModelSerializers.config.cache_store.blank?

keys = CachedSerializer.object_cache_keys(serializer, @include_tree)

return {} if keys.blank?

ActiveModelSerializers.config.cache_store.read_multi(*keys)
end

# Set @cached_attributes
def cache_attributes
return if @cached_attributes.present?

@cached_attributes = cache_read_multi
end

# Get attributes from @cached_attributes
# @return [Hash] cached attributes
def cached_attributes(cached_serializer)
return yield unless cached_serializer.cached?

@cached_attributes.fetch(cached_serializer.cache_key) { yield }
end

def serializable_hash_for_single_resource(options)
resource = resource_object_for(options)
relationships = resource_relationships(options)
Expand Down Expand Up @@ -56,8 +86,12 @@ def include_meta(json)
end

def resource_object_for(options)
cache_check(serializer) do
serializer.attributes(options[:fields])
cached_serializer = CachedSerializer.new(serializer)

cached_attributes(cached_serializer) do
cached_serializer.cache_check(self) do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bf4 we still use cache_check method if the cache is not be hit.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it the switch from cache_check(serializer) do to cached_serializer = CachedSerializer.new(serializer); cached_serializer.cache_check(self) do that isn't immediately obvious and I'm writing rather than digging into it :)

serializer.attributes(options[:fields])
end
Copy link
Member

Choose a reason for hiding this comment

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

I don't recall why we removed the cache_check method here

end
end
end
Expand Down
36 changes: 35 additions & 1 deletion lib/active_model/serializer/adapter/cached_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,51 @@ def fragment_cached?
end

def cache_key
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.join('/')
@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)
cache_keys = []

serializers.each do |serializer|
cache_keys << object_cache_key(serializer)

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)
end
else
cache_keys << object_cache_key(association.serializer)
end
end
end

cache_keys.compact.uniq
end

# @return [String, nil] the cache_key of the serializer or nil
def self.object_cache_key(serializer)
return unless serializer.present? && serializer.object.present?

cached_serializer = new(serializer)
cached_serializer.cached? ? cached_serializer.cache_key : nil
end
end
end
end
Expand Down
32 changes: 32 additions & 0 deletions test/serializers/cache_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,38 @@ def test_cache_digest_definition
assert_equal(::Model::FILE_DIGEST, @post_serializer.class._cache_digest)
end

def test_object_cache_keys
serializer = CollectionSerializer.new([@comment, @comment])
include_tree = IncludeTree.from_include_args('*')

actual = Serializer::Adapter::CachedSerializer.object_cache_keys(serializer, include_tree)

assert_equal actual.size, 3
assert actual.any? { |key| key == 'comment/1' }
assert actual.any? { |key| key =~ %r{post/post-\d+} }
assert actual.any? { |key| key =~ %r{writer/author-\d+} }
end

def test_cached_attributes
serializer = CollectionSerializer.new([@comment, @comment])

Timecop.freeze(Time.now) do
render_object_with_cache(@comment)

attributes = ActiveModel::Serializer::Adapter::Attributes.new(serializer)
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

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
end
end

def test_serializer_file_path_on_nix
path = '/Users/git/emberjs/ember-crm-backend/app/serializers/lead_serializer.rb'
caller_line = "#{path}:1:in `<top (required)>'"
Expand Down