Skip to content

Commit

Permalink
When objects are sideloaded multiple times, serialize them only once
Browse files Browse the repository at this point in the history
To achieve this, we make the following change when sideloading: Instead
of serializing associations and discarding duplicate *hashes*, we
memorize the *objects* (records) that we have already serialized, and
only serialize those that are new.

This change is mostly transparent, and brings down serialization time
from 3.1 seconds to 1.0 seconds on my set of sample data.

There is one change in the behavior: If you sideload the same object
multiple times, and it yields different hashes, like so:

    embed :ids, include: true
    has_many :comments
    has_many :recent_comments, root: comments, serializer: CommentShortSerializer

then previously, it would be included multiple times, whereas now, the
first hash wins. (I haven't actually tested this.) I don't know that
either option is preferable. It's not covered by the test suite, and I
think it's an edge case that is OK to ignore entirely.
  • Loading branch information
joliss committed Oct 29, 2012
1 parent 28ee88c commit ee3cec3
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 13 deletions.
18 changes: 10 additions & 8 deletions lib/active_model/serializer.rb
Expand Up @@ -310,7 +310,7 @@ def include!(name, options={})
if association.embed_in_root? && hash.nil?
raise IncludeError.new(self.class, association.name)
elsif association.embed_in_root? && association.embeddable?
merge_association hash, association.root, association.serialize_many, unique_values
merge_association hash, association.root, association.serializables, unique_values
end
elsif association.embed_objects?
node[association.key] = association.serialize
Expand All @@ -326,13 +326,15 @@ def include!(name, options={})
# a unique list of all of the objects that are already in the Array. This
# avoids the need to scan through the Array looking for entries every time
# we want to merge a new list of values.
def merge_association(hash, key, value, unique_values)
if current_value = unique_values[key]
current_value.merge! value
hash[key] = current_value.to_a
elsif value
hash[key] = value
unique_values[key] = OrderedSet.new(value)
def merge_association(hash, key, serializables, unique_values)
already_serialized = (unique_values[key] ||= {})
serializable_hashes = (hash[key] ||= [])

serializables.each do |serializable|
unless already_serialized.include? serializable.object
already_serialized[serializable.object] = true
serializable_hashes << serializable.serializable_hash
end
end
end

Expand Down
11 changes: 8 additions & 3 deletions lib/active_model/serializer/associations.rb
Expand Up @@ -99,7 +99,12 @@ def serialize
find_serializable(item).serializable_hash
end
end
alias serialize_many serialize

def serializables
associated_object.map do |item|
find_serializable(item)
end
end

def serialize_ids
# Use pluck or select_columns if available
Expand Down Expand Up @@ -149,9 +154,9 @@ def serialize
end
end

def serialize_many
def serializables
object = associated_object
value = object && find_serializable(object).serializable_hash
value = object && find_serializable(object)
value ? [value] : []
end

Expand Down
23 changes: 23 additions & 0 deletions test/association_test.rb
Expand Up @@ -284,6 +284,29 @@ def test_embed_ids_include_true_for_has_one_associations
]
}, @root_hash)
end

def test_embed_ids_include_true_does_not_serialize_multiple_times
@post.recent_comment = @comment

@post_serializer_class.class_eval do
has_one :comment, :embed => :ids, :include => true
has_one :recent_comment, :embed => :ids, :include => true, :root => :comments
end

# Count how often the @comment record is serialized.
serialized_times = 0
@comment.class_eval do
define_method :read_attribute_for_serialization, lambda { |name|
serialized_times += 1 if name == :body
super(name)
}
end

include_bare! :comment
include_bare! :recent_comment

assert_equal 1, serialized_times
end
end

class InclusionTest < AssociationTest
Expand Down
6 changes: 4 additions & 2 deletions test/serializer_test.rb
Expand Up @@ -73,11 +73,13 @@ def serializable_hash

class CommentSerializer
def initialize(comment, options={})
@comment = comment
@object = comment
end

attr_reader :object

def serializable_hash
{ :title => @comment.read_attribute_for_serialization(:title) }
{ :title => @object.read_attribute_for_serialization(:title) }
end

def as_json(options=nil)
Expand Down

0 comments on commit ee3cec3

Please sign in to comment.