Skip to content

MONGOID-4889 Optimize batch assignment of embedded documents (backport to 9.0) #6010

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

Open
wants to merge 1 commit into
base: 9.0-stable
Choose a base branch
from
Open
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: 1 addition & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ Layout/SpaceInsidePercentLiteralDelimiters:
Enabled: false

Metrics/ClassLength:
Max: 200
Enabled: false

Metrics/ModuleLength:
Enabled: false
Expand Down
21 changes: 11 additions & 10 deletions lib/mongoid/association/embedded/batchable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -313,18 +313,19 @@ def selector
#
# @return [ Array<Hash> ] The documents as an array of hashes.
def pre_process_batch_insert(docs)
docs.map do |doc|
next unless doc
append(doc)
if persistable? && !_assigning?
self.path = doc.atomic_path unless path
if doc.valid?(:create)
doc.run_before_callbacks(:save, :create)
else
self.inserts_valid = false
[].tap do |results|
append_many(docs) do |doc|
if persistable? && !_assigning?
self.path = doc.atomic_path unless path
if doc.valid?(:create)
doc.run_before_callbacks(:save, :create)
else
self.inserts_valid = false
end
end

results << doc.send(:as_attributes)
end
doc.send(:as_attributes)
end
end

Expand Down
61 changes: 61 additions & 0 deletions lib/mongoid/association/embedded/embeds_many/proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,67 @@ def append(document)
execute_callback :after_add, document
end

# Returns a unique id for the document, which is either
# its _id or its object_id.
def id_of(doc)
doc._id || doc.object_id
end

# Optimized version of #append that handles multiple documents
# in a more efficient way.
#
# @param [ Array<Document> ] documents The documents to append.
#
# @return [ EmbedsMany::Proxy ] This proxy instance.
def append_many(documents, &block)
unique_set = process_incoming_docs(documents, &block)

_unscoped.concat(unique_set)
_target.push(*scope(unique_set))
update_attributes_hash

unique_set.each { |doc| execute_callback :after_add, doc }

self
end

# Processes the list of documents, building a list of those
# that are not already in the association, and preparing
# each unique document to be integrated into the association.
#
# The :before_add callback is executed for each unique document
# as part of this step.
#
# @param [ Array<Document> ] documents The incoming documents to
# process.
#
# @yield [ Document ] Optional block to call for each unique
# document.
#
# @return [ Array<Document> ] The list of unique documents that
# do not yet exist in the association.
def process_incoming_docs(documents, &block)
visited_docs = Set.new(_target.map { |doc| id_of(doc) })
next_index = _unscoped.size

documents.select do |doc|
next unless doc

id = id_of(doc)
next if visited_docs.include?(id)

execute_callback :before_add, doc

visited_docs.add(id)
integrate(doc)

doc._index = next_index
next_index += 1

block&.call(doc) || true
end
end

# Instantiate the binding associated with this association.
#
# @example Create the binding.
Expand Down
4 changes: 0 additions & 4 deletions lib/mongoid/association/referenced/has_many/proxy.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
# frozen_string_literal: true

# TODO: consider refactoring this Proxy class, to satisfy the following
# cops...
# rubocop:disable Metrics/ClassLength
module Mongoid
module Association
module Referenced
Expand Down Expand Up @@ -588,4 +585,3 @@ def save_or_delay(doc, docs, inserts)
end
end
end
# rubocop:enable Metrics/ClassLength
41 changes: 41 additions & 0 deletions spec/integration/associations/embeds_many_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,47 @@
include_examples 'persists correctly'
end
end

context 'including duplicates in the assignment' do
let(:canvas) do
Canvas.create!(shapes: [Shape.new])
end

shared_examples 'persists correctly' do
it 'persists correctly' do
canvas.shapes.length.should eq 2
_canvas = Canvas.find(canvas.id)
_canvas.shapes.length.should eq 2
end
end

context 'via assignment operator' do
before do
canvas.shapes = [ canvas.shapes.first, Shape.new, canvas.shapes.first ]
canvas.save!
end

include_examples 'persists correctly'
end

context 'via attributes=' do
before do
canvas.attributes = { shapes: [ canvas.shapes.first, Shape.new, canvas.shapes.first ] }
canvas.save!
end

include_examples 'persists correctly'
end

context 'via assign_attributes' do
before do
canvas.assign_attributes(shapes: [ canvas.shapes.first, Shape.new, canvas.shapes.first ])
canvas.save!
end

include_examples 'persists correctly'
end
end
end

context 'when an anonymous class defines an embeds_many association' do
Expand Down