MONGOID-4889 Optimize batch assignment of embedded documents (#6008) (#6010)

* MONGOID-4889 Optimize batch assignment of embedded documents

* rubocop appeasement

* simplify some refactoring artifacts

* improve the name of the extracted method

Author: jamis

.rubocop.yml

@@ -64,7 +64,7 @@ Layout/SpaceInsidePercentLiteralDelimiters:
   Enabled: false
 
 Metrics/ClassLength:
-  Max: 200
+  Enabled: false
 
 Metrics/ModuleLength:
   Enabled: false

lib/mongoid/association/embedded/batchable.rb

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

lib/mongoid/association/embedded/embeds_many/proxy.rb

@@ -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.

lib/mongoid/association/referenced/has_many/proxy.rb

@@ -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
@@ -588,4 +585,3 @@ def save_or_delay(doc, docs, inserts)
     end
   end
 end
-# rubocop:enable Metrics/ClassLength

spec/integration/associations/embeds_many_spec.rb

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