From e09d4d548988e7a8e2eae47e10294cf770b83fd0 Mon Sep 17 00:00:00 2001 From: Jamis Buck Date: Fri, 17 Apr 2026 10:37:58 -0600 Subject: [PATCH 1/2] MONGOID-5751 avoid unnecessary autosaves of unchanged subtrees (#6133) * MONGOID-5751 avoid unnecessary autosaves of unchanged subtrees * avoid loading HasMany proxies into memory, just to check if they're dirty * account for belongs_to sometimes storing a literal object instead of a proxy * Add Mongoid.autosave_saves_unchanged_documents feature flag to preserve legacy behavior --- .../association/referenced/auto_save.rb | 54 ++++++++-- lib/mongoid/config.rb | 10 ++ lib/mongoid/warnings.rb | 3 + spec/mongoid/association/auto_save_spec.rb | 98 +++++++++++++++++++ 4 files changed, 159 insertions(+), 6 deletions(-) diff --git a/lib/mongoid/association/referenced/auto_save.rb b/lib/mongoid/association/referenced/auto_save.rb index 43a55118ea..5a097e8624 100644 --- a/lib/mongoid/association/referenced/auto_save.rb +++ b/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 diff --git a/lib/mongoid/config.rb b/lib/mongoid/config.rb index f16e61a8e4..37bbf3a2e1 100644 --- a/lib/mongoid/config.rb +++ b/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 false. + # + # 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 diff --git a/lib/mongoid/warnings.rb b/lib/mongoid/warnings.rb index 8811e64022..a2ce2bf28e 100644 --- a/lib/mongoid/warnings.rb +++ b/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 diff --git a/spec/mongoid/association/auto_save_spec.rb b/spec/mongoid/association/auto_save_spec.rb index bb660688ca..fd3e5d8d42 100644 --- a/spec/mongoid/association/auto_save_spec.rb +++ b/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 From bc6e88f305f6205c4d7a348e9baeabc4908ba999 Mon Sep 17 00:00:00 2001 From: Jamis Buck Date: Fri, 17 Apr 2026 13:56:41 -0600 Subject: [PATCH 2/2] Update lib/mongoid/config.rb Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- lib/mongoid/config.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/mongoid/config.rb b/lib/mongoid/config.rb index 37bbf3a2e1..7de48736b6 100644 --- a/lib/mongoid/config.rb +++ b/lib/mongoid/config.rb @@ -128,11 +128,11 @@ module Config # 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 false. + # is `true`. # - # This option will default to 'false' in Mongoid 9.1, and will be removed + # 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 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