MONGOID-5751 avoid unnecessary autosaves of unchanged subtrees

lib/mongoid/association/referenced/auto_save.rb

@@ -32,13 +32,24 @@ def __autosaving__
           Threaded.exit_autosave(self)
         end
 
-        # Check if there is changes for auto-saving
+        # Check if there are changes for auto-saving. Returns true if the
+        # document is new, changed, or marked for destruction, or if any
+        # in-memory referenced child with autosave: true recursively
+        # satisfies the same condition.
         #
-        # @example Return true if there is changes on self or in
-        #           autosaved associations.
-        #   document.changed_for_autosave?
-        def changed_for_autosave?(doc)
-          doc.new_record? || doc.changed? || doc.marked_for_destruction?
+        # The seen set prevents infinite recursion when autosave associations
+        # form a cycle (e.g. a belongs_to with autosave: true whose target
+        # has a has_many with autosave: true pointing back).
+        #
+        # @param [ Document ] doc The document to check.
+        # @param [ Set ] seen Documents already visited (cycle guard).
+        #
+        # @return [ true | false ] Whether the document needs autosaving.
+        def changed_for_autosave?(doc, seen = Set.new)
+          return false unless seen.add?(doc)
+
+          doc.new_record? || doc.changed? || doc.marked_for_destruction? ||
+            autosave_children_changed?(doc, seen)
         end
 
         # Define the autosave method on an association's owning class for
@@ -60,6 +71,8 @@ def self.define_autosave!(association)
                 __autosaving__ do
                   if assoc_value = ivar(association.name)
                     Array(assoc_value).each do |doc|
+                      next unless changed_for_autosave?(doc)
+
                       pc = doc.persistence_context? ? doc.persistence_context : persistence_context.for_child(doc)
                       doc.with(pc) do |d|
                         d.save
@@ -72,6 +85,35 @@ def self.define_autosave!(association)
             klass.after_persist_parent save_method, unless: :autosaved?
           end
         end
+
+        private
+
+        # Returns true if any in-memory referenced child with autosave: true
+        # needs saving.
+        #
+        # @param [ Document ] doc The document whose children to check.
+        # @param [ Set ] seen Cycle guard passed through from changed_for_autosave?.
+        #
+        # @return [ true | false ]
+        def autosave_children_changed?(doc, seen)
+          if Mongoid.autosave_saves_unchanged_documents?
+            Mongoid::Warnings.warn_autosave_saves_unchanged_documents
+            return true
+          end
+
+          doc.class.relations.values.select { |a| a.autosave? && !a.embedded? }.any? do |assoc|
+            (assoc_value = doc.ivar(assoc.name)) &&
+              in_memory_docs(assoc_value).any? { |child| changed_for_autosave?(child, seen) }
+          end
+        end
+
+        # Returns the in-memory documents for an association value without
+        # triggering a database load of any unloaded documents. Association
+        # proxies expose in_memory for this purpose; a plain document (which
+        # belongs_to can store directly in the ivar) is itself in-memory.
+        def in_memory_docs(assoc_value)
+          assoc_value.respond_to?(:in_memory) ? assoc_value.in_memory : [ assoc_value ]
+        end
       end
     end
   end

lib/mongoid/config.rb

@@ -125,6 +125,16 @@ module Config
     # in Mongoid 10.
     option :allow_reparenting_via_nested_attributes, default: true
 
+    # When this flag is true, any documents in associations with `autosave: true`
+    # will be saved even if they have not been changed. When this flag is false,
+    # only autosaved documents that have been changed will be saved. The default
+    # is `true`.
+    #
+    # This option will default to `false` in Mongoid 9.1, and will be removed
+    # in Mongoid 10, with the only behavior at that point being as if this
+    # option were set to `false`.
+    option :autosave_saves_unchanged_documents, default: true
+
     # When this flag is false, a document will become read-only only once the
     # #readonly! method is called, and an error will be raised on attempting
     # to save or update such documents, instead of just on delete. When this

lib/mongoid/warnings.rb

@@ -34,5 +34,8 @@ def warning(id, message)
     warning :legacy_readonly, 'The readonly! method will only mark the document readonly when the legacy_readonly feature flag is switched off.'
     warning :mutable_ids, 'Ignoring updates to immutable attribute `_id`. Please set Mongoid::Config.immutable_ids to true and update your code so that `_id` is never updated.'
     warning :reparenting_via_nested_attributes, 'Reparenting documents via nested attributes is insecure and is deprecated. Set Mongoid.allow_reparenting_via_nested_attributes to false and update your code to avoid reparenting documents via nested attributes.'
+    warning :autosave_saves_unchanged_documents, "Autosave associations are currently configured to save documents even if they haven't changed. " \
+                                                 'This legacy behavior is deprecated. Set Mongoid.autosave_saves_unchanged_documents to false to ' \
+                                                 'skip saving unchanged documents in autosave associations.'
   end
 end

spec/mongoid/association/auto_save_spec.rb

@@ -5,6 +5,53 @@
 require_relative './referenced/has_many_models'
 require_relative './referenced/has_one_models'
 
+# Models for the MONGOID-5751 regression test: after_save callbacks must not
+# fire for pre-existing, unchanged documents when autosave: true cascades a
+# save from a parent to its children.
+module AutoSaveMONGOID5751
+  class Table
+    include Mongoid::Document
+
+    has_many :rows, autosave: true, class_name: 'AutoSaveMONGOID5751::Row',
+                    inverse_of: :table
+
+    class << self
+      attr_accessor :after_save_count
+    end
+    self.after_save_count = 0
+
+    after_save { self.class.after_save_count += 1 }
+  end
+
+  class Row
+    include Mongoid::Document
+
+    belongs_to :table, class_name: 'AutoSaveMONGOID5751::Table', inverse_of: :rows
+    has_many :cells, autosave: true, class_name: 'AutoSaveMONGOID5751::Cell',
+                     inverse_of: :row
+
+    class << self
+      attr_accessor :after_save_count
+    end
+    self.after_save_count = 0
+
+    after_save { self.class.after_save_count += 1 }
+  end
+
+  class Cell
+    include Mongoid::Document
+
+    belongs_to :row, class_name: 'AutoSaveMONGOID5751::Row', inverse_of: :cells
+
+    class << self
+      attr_accessor :after_save_count
+    end
+    self.after_save_count = 0
+
+    after_save { self.class.after_save_count += 1 }
+  end
+end
+
 describe Mongoid::Association::Referenced::AutoSave do
 
   describe ".auto_save" do
@@ -399,6 +446,57 @@ class Harvest
           expect(harvest.reload.season).to eq('Fall')
         end
       end
+
+      # Regression test for MONGOID-5751: after_save must not fire for
+      # pre-existing, unchanged documents that are merely loaded into memory
+      # as a side-effect of the autosave traversal.
+      context 'when a parent with existing children has a new child added' do
+        before do
+          # Persist a table with 3 pre-existing rows, each with 3 cells.
+          table = AutoSaveMONGOID5751::Table.create!
+          3.times do
+            row = table.rows.create!
+            3.times { row.cells.create! }
+          end
+
+          # Reset counters so only the saves triggered by the call below are
+          # measured.
+          AutoSaveMONGOID5751::Table.after_save_count = 0
+          AutoSaveMONGOID5751::Row.after_save_count = 0
+          AutoSaveMONGOID5751::Cell.after_save_count = 0
+
+          # Reload the table fresh from the database, then append exactly one
+          # new row (with one new cell) and persist.
+          reloaded = AutoSaveMONGOID5751::Table.find(table.id)
+          new_row = reloaded.rows.build
+          new_row.cells.build
+          reloaded.save!
+        end
+
+        it 'fires after_save once for the parent table' do
+          expect(AutoSaveMONGOID5751::Table.after_save_count).to eq(1)
+        end
+
+        it 'fires after_save only for the newly added cell' do
+          expect(AutoSaveMONGOID5751::Cell.after_save_count).to eq(1)
+        end
+
+        context 'when autosave_saves_unchanged_documents is true' do
+          config_override :autosave_saves_unchanged_documents, true
+
+          it 'fires after_save for all new and pre-existing rows' do
+            expect(AutoSaveMONGOID5751::Row.after_save_count).to eq(4)
+          end
+        end
+
+        context 'when autosave_saves_unchanged_documents is false' do
+          config_override :autosave_saves_unchanged_documents, false
+
+          it 'fires after_save only for the newly added row, not for pre-existing rows' do
+            expect(AutoSaveMONGOID5751::Row.after_save_count).to eq(1)
+          end
+        end
+      end
     end
   end
 end