MONGOID-3291 Clear inverse associations when setting across all association types (#5288)

* MONGOID-3291 add tests for has_many, belongs_to, fix has_many

* MONGOID-3291 test HABTM fix belongs_to

* MONGOID-3291 remove HABTM test

* MONGOID-3291 add has_many test

* MONGOID-3291 dont clear belongs_to with has_any inverse

* MONGOID-3291 fix belongs_to association resolving

* MONGOID-3291 use create instead of new

* MONGOID-3291 add embeds_one test

* MONGOID-3291 fix tests

* MONGOID-3291 add param to inverse

* MONGOID-3291 reformat

* MONGOID-3291 fix tests

* MONGOID-3291 pend test

* MONGOID-3291 remove associated only when relation is in memory

* MONGOID-3291 fix embeds_one

* MONGOID-3291 use methods instead of is_a?

* MONGOID-3291 implement embeds_many and embedded_in

* Revert "MONGOID-3291 use methods instead of is_a?"

This reverts commit 48e344b.

* MONGOID-3291 fix tests

* MONGOID-3291 fix test

* MONGOID-3291 remove embeds_many specific method and use the general one

* MONGOID-3291 generalize methods

* MONGOID-3291 clean up association methods

* MONGOID-3291 further abstraction

* MONGOID-3291 update test descriptions

Author: neilshweky

lib/mongoid/association/bindable.rb

@@ -60,6 +60,53 @@ def check_inverse!(doc)
         end
       end
 
+      # Remove the associated document from the inverse's association.
+      #
+      # @param [ Document ] doc The document to remove.
+      def remove_associated(doc)
+        if inverse = _association.inverse(doc)
+          if _association.many?
+            remove_associated_many(doc, inverse)
+          elsif _association.in_to?
+            remove_associated_in_to(doc, inverse)
+          end
+        end
+      end
+
+      # Remove the associated document from the inverse's association.
+      #
+      # This method removes the associated on *_many relationships.
+      #
+      # @param [ Document ] doc The document to remove.
+      # @param [ Symbol ] inverse The name of the inverse.
+      def remove_associated_many(doc, inverse)
+        # We only want to remove the inverse association when the inverse
+        # document is in memory.
+        if inv = doc.ivar(inverse)
+          # This first condition is needed because when assigning the
+          # embeds_many association using the same embeds_many
+          # association, we delete from the array we are about to assign.
+          if _base != inv && (associated = inv.ivar(_association.name))
+            associated.delete(doc)
+          end
+        end
+      end
+
+      # Remove the associated document from the inverse's association.
+      #
+      # This method removes associated on belongs_to and embedded_in
+      # associations.
+      #
+      # @param [ Document ] doc The document to remove.
+      # @param [ Symbol ] inverse The name of the inverse.
+      def remove_associated_in_to(doc, inverse)
+        # We only want to remove the inverse association when the inverse
+        # document is in memory.
+        if associated = doc.ivar(inverse)
+          associated.send(_association.setter, nil)
+        end
+      end
+
       # Set the id of the related document in the foreign key field on the
       # keyed document.
       #
@@ -134,6 +181,7 @@ def bind_inverse(doc, inverse)
       # @param [ Document ] doc The document to bind.
       def bind_from_relational_parent(doc)
         check_inverse!(doc)
+        remove_associated(doc)
         bind_foreign_key(doc, record_id(_base))
         bind_polymorphic_type(doc, _base.class.name)
         bind_inverse(doc, _base)

lib/mongoid/association/embedded/embedded_in/binding.rb

@@ -26,6 +26,7 @@ def bind_one
               if _base.embedded_many?
                 _target.do_or_do_not(_association.inverse(_target)).push(_base)
               else
+                remove_associated(_target)
                 _target.do_or_do_not(_association.inverse_setter(_target), _base)
               end
             end

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

@@ -19,6 +19,7 @@ class Binding
           def bind_one(doc)
             doc.parentize(_base)
             binding do
+              remove_associated(doc)
               doc.do_or_do_not(_association.inverse_setter(_target), _base)
             end
           end

lib/mongoid/association/embedded/embeds_one/buildable.rb

@@ -25,11 +25,25 @@ module Buildable
           #
           # @return [ Document ] A single document.
           def build(base, object, _type = nil, selected_fields = nil)
-            return object unless object.is_a?(Hash)
-            if _loading? && base.persisted?
-              Factory.execute_from_db(klass, object, nil, selected_fields, execute_callbacks: false)
+            if object.is_a?(Hash)
+              if _loading? && base.persisted?
+                Factory.execute_from_db(klass, object, nil, selected_fields, execute_callbacks: false)
+              else
+                Factory.build(klass, object)
+              end
             else
-              Factory.build(klass, object)
+              clear_associated(object)
+              object
+            end
+          end
+
+          private
+
+          def clear_associated(doc)
+            if doc && (inv = inverse(doc))
+              if associated = doc.ivar(inv)
+                associated.substitute(nil)
+              end
             end
           end
         end

lib/mongoid/association/referenced/belongs_to/binding.rb

@@ -28,6 +28,7 @@ def bind_one
                   if _base.referenced_many?
                     _target.__send__(inverse).push(_base)
                   else
+                    remove_associated(_target)
                     _target.set_relation(inverse, _base)
                   end
                 end

spec/integration/associations/scope_option_spec.rb

@@ -49,7 +49,7 @@
     it 'initially associates the documents in-memory' do
       expect(trainer1.animal).to eq animal2
       expect(trainer2.animal).to eq animal3
-      expect(animal1.trainer).to eq trainer1
+      expect(animal1.trainer).to be_nil
       expect(animal2.trainer).to eq trainer1
       expect(animal3.trainer).to eq trainer2
     end

spec/mongoid/association/embedded/embedded_in/binding_spec.rb

@@ -58,7 +58,8 @@
         end
 
         it "does nothing" do
-          expect(name).to receive(:namable=).never
+          expect(name).to receive(:namable=).with(person).never
+          expect(name).to receive(:namable=).with(nil).once
           binding.bind_one
         end
       end

spec/mongoid/association/embedded/embedded_in/buildable_spec.rb

@@ -33,4 +33,58 @@
       end
     end
   end
+
+  context 'when the object is already associated with another object' do
+
+    context "when inverse is embeds_many" do
+
+      let(:appointment1) do
+        Appointment.new
+      end
+
+      let(:appointment2) do
+        Appointment.new
+      end
+
+      let(:person) do
+        Person.create!
+      end
+
+      before do
+        appointment1.person = person
+        appointment2.person = person
+      end
+
+      it 'does not clear the object of its previous association' do
+        expect(appointment1.person).to eq(person)
+        expect(appointment2.person).to eq(person)
+        expect(person.appointments).to eq([appointment1, appointment2])
+      end
+    end
+
+    context "when inverse is embeds_one" do
+
+      let(:scribe1) do
+        Scribe.new
+      end
+
+      let(:scribe2) do
+        Scribe.new
+      end
+
+      let(:owner) do
+        Owner.create!
+      end
+
+      before do
+        scribe1.owner = owner
+        scribe2.owner = owner
+      end
+
+      it 'clears the object of its previous association' do
+        expect(scribe1.owner).to be_nil
+        expect(scribe2.owner).to eq(owner)
+      end
+    end
+  end
 end

spec/mongoid/association/embedded/embeds_many/buildable_spec.rb

@@ -103,4 +103,116 @@
       end
     end
   end
+
+  context 'when the object is already associated with another object' do
+
+    context "when using <<" do
+
+      let(:person1) do
+        Person.new
+      end
+
+      let(:person2) do
+        Person.new
+      end
+
+      let(:appointment) do
+        Appointment.new
+      end
+
+      before do
+        person1.appointments << appointment
+        person2.appointments << appointment
+      end
+
+      it 'clears the object of its previous association' do
+        expect(person1.appointments).to eq([])
+        expect(person2.appointments).to eq([appointment])
+      end
+    end
+
+    context "when using concat" do
+
+      let(:person1) do
+        Person.new
+      end
+
+      let(:person2) do
+        Person.new
+      end
+
+      let(:appointment) do
+        Appointment.new
+      end
+
+      before do
+        person1.appointments.concat([appointment])
+        person2.appointments.concat([appointment])
+      end
+
+      it 'clears the object of its previous association' do
+        expect(person1.appointments).to eq([])
+        expect(person2.appointments).to eq([appointment])
+      end
+    end
+
+    context "when using =" do
+
+      let(:person1) do
+        Person.new
+      end
+
+      let(:person2) do
+        Person.new
+      end
+
+      let(:appointment) do
+        Appointment.new
+      end
+
+      let(:apts) { [ appointment ] }
+
+      before do
+        person1.appointments = apts
+        person2.appointments = apts
+        expect(apts).to eq([ appointment ])
+      end
+
+      it 'clears the object of its previous association' do
+        expect(person1.appointments).to eq([])
+        expect(person2.appointments).to eq([appointment])
+      end
+    end
+
+    context "when using = on the same document twice" do
+
+      let(:person1) do
+        Person.new
+      end
+
+      let(:person2) do
+        Person.new
+      end
+
+      let(:appointment1) do
+        Appointment.new
+      end
+
+      let(:appointment2) do
+        Appointment.new
+      end
+
+      let(:apts) { [ appointment1, appointment2 ] }
+
+      before do
+        person1.appointments = apts
+        person1.appointments = person1.appointments.reverse
+        expect(apts).to eq([ appointment1, appointment2  ])
+      end
+
+      it 'clears the object of its previous association' do
+        expect(person1.appointments).to eq([ appointment2, appointment1 ])
+      end
+    end
+  end
 end

spec/mongoid/association/embedded/embeds_one/buildable_spec.rb

@@ -78,4 +78,29 @@
       end
     end
   end
+
+  context 'when the object is already associated with another object' do
+
+    let(:owner1) do
+      Owner.create!
+    end
+
+    let(:owner2) do
+      Owner.create!
+    end
+
+    let(:scribe) do
+      Scribe.new
+    end
+
+    before do
+      owner1.scribe = scribe
+      owner2.scribe = scribe
+    end
+
+    it 'clears the object of its previous association' do
+      expect(owner1.scribe).to be_nil
+      expect(owner2.scribe).to eq(scribe)
+    end
+  end
 end