MONGOID-5136 Fix touch with custom field on embedded documents. In addition:

- Perform all touch operations on the document in a single write operation (MONGOID-5142)
- Synchronize the timestamp of all touches on the document (parent+child will have same timestamp)
- Remove touches from the atomic set operations, so that they are no longer there when saving the next time.
- Cleanup/refactor touch method to be much more readable

Author: johnnyshields

lib/mongoid/atomic.rb

@@ -367,29 +367,5 @@ def generate_atomic_updates(mods, doc)
       mods.add_to_set(doc.atomic_array_add_to_sets)
       mods.pull_all(doc.atomic_array_pulls)
     end
-
-    # Get the atomic updates for a touch operation. Should only include the
-    # updated_at field and the optional extra field.
-    #
-    # @api private
-    #
-    # @example Get the touch atomic updates.
-    #   document.touch_atomic_updates
-    #
-    # @param [ Symbol ] field The optional field.
-    #
-    # @return [ Hash ] The atomic updates.
-    #
-    # @since 3.0.6
-    def touch_atomic_updates(field = nil)
-      updates = atomic_updates
-      return {} unless atomic_updates.key?("$set")
-      touches = {}
-      updates["$set"].each_pair do |key, value|
-        key_regex = /updated_at|u_at#{"|" + field if field.present?}/
-        touches.merge!({ key => value }) if key =~ key_regex
-      end
-      { "$set" => touches }
-    end
   end
 end

lib/mongoid/touchable.rb

@@ -25,38 +25,70 @@ module InstanceMethods
       # @since 3.0.0
       def touch(field = nil)
         return false if _root.new_record?
-        current = Time.now
-        field = database_field_name(field)
-        write_attribute(:updated_at, current) if respond_to?("updated_at=")
-        write_attribute(field, current) if field
-
-        # If the document being touched is embedded, touch its parents
-        # all the way through the composition hierarchy to the root object,
-        # because when an embedded document is changed the write is actually
-        # performed by the composition root. See MONGOID-3468.
-        if _parent
-          # This will persist updated_at on this document as well as parents.
-          # TODO support passing the field name to the parent's touch method;
-          # I believe it should be read out of
-          # _association.inverse_association.options but inverse_association
-          # seems to not always/ever be set here. See MONGOID-5014.
-          _parent.touch
-        else
-          # If the current document is not embedded, it is composition root
-          # and we need to persist the write here.
-          touches = touch_atomic_updates(field)
-          unless touches["$set"].blank?
-            selector = atomic_selector
-            _root.collection.find(selector).update_one(positionally(selector, touches), session: _session)
-          end
+
+        touches = __gather_touch_updates(Time.now, field)
+        unless touches.blank?
+          selector = _root.atomic_selector
+          _root.collection.find(selector).update_one(positionally(selector, '$set' => touches), session: _session)
         end
 
-        # Callbacks are invoked on the composition root first and on the
-        # leaf-most embedded document last.
-        # TODO add tests, see MONGOID-5015.
-        run_callbacks(:touch)
+        __run_touch_callbacks_from_root
         true
       end
+
+      # Recursively sets touchable fields on the current document and each of its
+      # parents, including the root node. Returns the combined atomic $set
+      # operations to be performed on the root document.
+      #
+      # @param [ Time ] now The timestamp used for synchronizing the touched time.
+      # @param [ Symbol ] field The name of an additional field to update.
+      #
+      # @return [ Hash<String, Time> ] The touch operations to perform as an atomic $set.
+      #
+      # @api private
+      def __gather_touch_updates(now, field = nil)
+        field = database_field_name(field)
+        write_attribute(:updated_at, now) if respond_to?("updated_at=")
+        write_attribute(field, now) if field
+
+        touches = __extract_touches_from_atomic_sets(field) || {}
+        touches.merge!(_parent.__gather_touch_updates(now) || {}) if _parent
+        touches
+      end
+
+      # Recursively runs :touch callbacks for the document and its parents,
+      # beginning with the root document and cascading through each successive
+      # child document.
+      #
+      # @api private
+      #
+      # TODO add tests, see MONGOID-5015.
+      def __run_touch_callbacks_from_root
+        _parent.__run_touch_callbacks_from_root if _parent
+        run_callbacks(:touch)
+      end
+
+      # Extract and remove the atomic updates for the touch operation(s)
+      # from the currently enqueued atomic $set operations.
+      #
+      # @api private
+      #
+      # @param [ Symbol ] field The optional field.
+      #
+      # @return [ Hash ] The field-value pairs to update atomically.
+      def __extract_touches_from_atomic_sets(field = nil)
+        updates = atomic_updates['$set']
+        return {} unless updates
+
+        touchable_keys = %w(updated_at u_at)
+        touchable_keys << field.to_s if field.present?
+
+        updates.keys.each_with_object({}) do |key, touches|
+          if touchable_keys.include?(key.split('.').last)
+            touches[key] = updates.delete(key)
+          end
+        end
+      end
     end
 
     extend self

spec/mongoid/touchable_spec.rb

@@ -132,6 +132,24 @@
         include_examples 'updates the child'
         include_examples 'updates the parent when :touch is true'
         include_examples 'updates the parent when :touch is not set'
+
+        context 'when also updating an additional field' do
+          it 'persists the update to the additional field' do
+            entrance
+            update_time
+            entrance.touch(:last_used_at)
+
+            entrance.reload
+            building.reload
+
+            # This is the assertion we want.
+            expect(entrance.last_used_at).to eq update_time
+
+            # Check other timestamps for good measure.
+            expect(entrance.updated_at).to eq update_time
+            expect(building.updated_at).to eq update_time
+          end
+        end
       end
 
       context "when the document is referenced" do

spec/mongoid/touchable_spec_models.rb

@@ -16,6 +16,8 @@ class Entrance
       include Mongoid::Timestamps
 
       embedded_in :building
+
+      field :last_used_at, type: Time
     end
 
     class Floor