MONGOID-4397 update_attributes does not reflect changes to embedded documents in the attributes hash (#5232)

* MONGOID-4397 potential fix

* MONGOID-4397 old implementation

* Revert "MONGOID-4397 old implementation"

This reverts commit 252b2f1.

* MONGOID-4397 second implementation moved to binding

* MONGOID-4397 finish embeds_one/many tests

* MONGOID-4397 fix tests

* MONGOID-4397 fix tests

* MONGOID-4397 take hash assignments into account

Author: neilshweky

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

@@ -95,6 +95,7 @@ def build(attributes = {}, type = nil)
           # @return [ self ] The empty association.
           def clear
             batch_clear(_target.dup)
+            update_attributes_hash
             self
           end
 
@@ -146,6 +147,7 @@ def delete(document)
                   doc.delete(suppress: true)
                   unbind_one(doc)
                 end
+                update_attributes_hash
               end
               reindex
               doc
@@ -257,6 +259,7 @@ def initialize(base, target, association)
                 integrate(doc)
                 doc._index = index
               end
+              update_attributes_hash
               @_unscoped = _target.dup
               @_target = scope(_target)
             end
@@ -292,6 +295,8 @@ def pop(count = nil)
               end
             else
               delete(_target[-1])
+            end.tap do
+              update_attributes_hash
             end
           end
 
@@ -315,6 +320,8 @@ def shift(count = nil)
               end
             else
               delete(_target[0])
+            end.tap do
+              update_attributes_hash
             end
           end
 
@@ -329,6 +336,7 @@ def shift(count = nil)
           # @return [ Many ] The proxied association.
           def substitute(docs)
             batch_replace(docs)
+            update_attributes_hash
             self
           end
 
@@ -366,6 +374,7 @@ def append(document)
             end
             _unscoped.push(document)
             integrate(document)
+            update_attributes_hash
             document._index = _unscoped.size - 1
             execute_callback :after_add, document
           end
@@ -399,6 +408,7 @@ def criteria
           def delete_one(document)
             _target.delete_one(document)
             _unscoped.delete_one(document)
+            update_attributes_hash
             reindex
           end
 
@@ -487,6 +497,7 @@ def remove_all(conditions = {}, method = :delete)
             criteria = where(conditions || {})
             removed = criteria.size
             batch_remove(criteria, method)
+            update_attributes_hash
             removed
           end
 
@@ -519,6 +530,17 @@ def as_attributes
             _unscoped.map { |doc| doc.send(:as_attributes) }
           end
 
+          # Update the _base's attributes hash with the _target's attributes
+          #
+          # @api private
+          def update_attributes_hash
+            if !_target.empty?
+              _base.attributes.merge!(_association.store_as => _target.map(&:attributes))
+            else
+              _base.attributes.delete(_association.store_as)
+            end
+          end
+
           class << self
 
             # Returns true if the association is an embedded one. In this case

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

@@ -31,6 +31,7 @@ def initialize(base, target, association)
               characterize_one(_target)
               bind_one
               characterize_one(_target)
+              update_attributes_hash(_target)
               _base._reset_memoized_descendants!
               _target.save if persistable?
             end
@@ -80,11 +81,15 @@ def substitute(replacement)
                 end
               end
               unbind_one
-              return nil unless replacement
+              unless replacement
+                update_attributes_hash(replacement)
+                return nil
+              end
               replacement = Factory.build(klass, replacement) if replacement.is_a?(::Hash)
               self._target = replacement
               characterize_one(_target)
               bind_one
+              update_attributes_hash(_target)
               _target.save if persistable?
             end
             self
@@ -112,6 +117,20 @@ def persistable?
             _base.persisted? && !_binding? && !_building? && !_assigning?
           end
 
+          # Update the _base's attributes hash with the _target's attributes
+          #
+          # @param replacement [ Document | nil ] The doc to use to update the
+          #   attributes hash.
+          #
+          # @api private
+          def update_attributes_hash(replacement)
+            if replacement
+              _base.attributes.merge!(_association.store_as => replacement.attributes)
+            else
+              _base.attributes.delete(_association.store_as)
+            end
+          end
+
           class << self
 
             # Returns true if the association is an embedded one. In this case

lib/mongoid/traversable.rb

@@ -233,6 +233,7 @@ def parentize(document)
     def remove_child(child)
       name = child.association_name
       if child.embedded_one?
+        self.attributes.delete(child._association.store_as)
         remove_ivar(name)
       else
         relation = send(name)

spec/mongoid/attributes/nested_spec_models.rb

@@ -23,3 +23,26 @@ class NestedPost
   has_many :comments, class_name: "NestedComment"
   accepts_nested_attributes_for :comments
 end
+
+class NestedBook
+  include Mongoid::Document
+
+  embeds_one :cover, class_name: "NestedCover"
+  embeds_many :pages, class_name: "NestedPage"
+
+  accepts_nested_attributes_for :cover, :pages
+end
+
+class NestedCover
+  include Mongoid::Document
+
+  field :title, type: String
+  embedded_in :book, class_name: "NestedBook"
+end
+
+class NestedPage
+  include Mongoid::Document
+
+  field :number, type: Integer
+  embedded_in :book, class_name: "NestedBook"
+end