diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 73b62b7f3b..458d6c54b9 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -152,6 +152,13 @@ jobs: uses: actions/checkout@v2 with: submodules: recursive + + # the default python 3.8 doesn't cut it, and causes mongo-orchestration + # to fail in mongodb-labs/drivers-evergreen-tools. + - uses: actions/setup-python@v5 + with: + python-version: '3.13' + - id: start-mongodb name: start mongodb uses: mongodb-labs/drivers-evergreen-tools@master diff --git a/lib/mongoid/interceptable.rb b/lib/mongoid/interceptable.rb index 3e946740be..9a3d39a503 100644 --- a/lib/mongoid/interceptable.rb +++ b/lib/mongoid/interceptable.rb @@ -152,9 +152,13 @@ def run_callbacks(kind, with_children: true, skip_if: nil, &block) # @api private def _mongoid_run_child_callbacks(kind, children: nil, &block) if Mongoid::Config.around_callbacks_for_embeds - _mongoid_run_child_callbacks_with_around(kind, children: children, &block) + _mongoid_run_child_callbacks_with_around(kind, + children: children, + &block) else - _mongoid_run_child_callbacks_without_around(kind, children: children, &block) + _mongoid_run_child_callbacks_without_around(kind, + children: children, + &block) end end @@ -235,9 +239,6 @@ def _mongoid_run_child_before_callbacks(kind, children: [], callback_list: []) return false if env.halted env.value = !env.halted callback_list << [next_sequence, env] - if (grandchildren = child.send(:cascadable_children, kind)) - _mongoid_run_child_before_callbacks(kind, children: grandchildren, callback_list: callback_list) - end end callback_list end diff --git a/spec/mongoid/interceptable_spec.rb b/spec/mongoid/interceptable_spec.rb index f13b44040b..90c7ecb59f 100644 --- a/spec/mongoid/interceptable_spec.rb +++ b/spec/mongoid/interceptable_spec.rb @@ -389,6 +389,84 @@ class TestClass end end end + + context 'with embedded grandchildren' do + IS = InterceptableSpec + + context 'when creating' do + let(:registry) { IS::CallbackRegistry.new(only: %i[ before_save ]) } + + let(:expected_calls) do + [ + # the parent + [ IS::CbParent, :before_save ], + + # the immediate child of the parent + [ IS::CbCascadedNode, :before_save ], + + # the grandchild of the parent + [ IS::CbCascadedNode, :before_save ], + ] + end + + let!(:parent) do + parent = IS::CbParent.new(registry) + child = IS::CbCascadedNode.new(registry) + grandchild = IS::CbCascadedNode.new(registry) + + child.cb_cascaded_nodes = [ grandchild ] + parent.cb_cascaded_nodes = [ child ] + + parent.tap(&:save) + end + + it 'should cascade callbacks to grandchildren' do + expect(registry.calls).to be == expected_calls + end + end + + context 'when updating' do + let(:registry) { IS::CallbackRegistry.new(only: %i[ before_update ]) } + + let(:expected_calls) do + [ + # the parent + [ IS::CbParent, :before_update ], + + # the immediate child of the parent + [ IS::CbCascadedNode, :before_update ], + + # the grandchild of the parent + [ IS::CbCascadedNode, :before_update ], + ] + end + + let!(:parent) do + parent = IS::CbParent.new(nil) + child = IS::CbCascadedNode.new(nil) + grandchild = IS::CbCascadedNode.new(nil) + + child.cb_cascaded_nodes = [ grandchild ] + parent.cb_cascaded_nodes = [ child ] + + parent.save + + parent.callback_registry = registry + child.callback_registry = registry + grandchild.callback_registry = registry + + parent.name = 'updated' + child.name = 'updated' + grandchild.name = 'updated' + + parent.tap(&:save) + end + + it 'should cascade callbacks to grandchildren' do + expect(registry.calls).to be == expected_calls + end + end + end end describe ".before_destroy" do diff --git a/spec/mongoid/interceptable_spec_models.rb b/spec/mongoid/interceptable_spec_models.rb index a6b87957e3..48b4fba536 100644 --- a/spec/mongoid/interceptable_spec_models.rb +++ b/spec/mongoid/interceptable_spec_models.rb @@ -1,11 +1,13 @@ # rubocop:todo all module InterceptableSpec class CallbackRegistry - def initialize + def initialize(only: []) @calls = [] + @only = only end def record_call(cls, cb) + return unless @only.empty? || @only.any? { |pat| pat == cb } @calls << [cls, cb] end @@ -16,6 +18,8 @@ module CallbackTracking extend ActiveSupport::Concern included do + field :name, type: String + %i( validation save create update ).each do |what| @@ -35,199 +39,110 @@ module CallbackTracking end end end - end - - class CbHasOneParent - include Mongoid::Document - has_one :child, autosave: true, class_name: "CbHasOneChild", inverse_of: :parent + attr_accessor :callback_registry - def initialize(callback_registry) + def initialize(callback_registry, *args, **kwargs) @callback_registry = callback_registry - super() + super(*args, **kwargs) end + end - attr_accessor :callback_registry - + module RootInsertable def insert_as_root @callback_registry&.record_call(self.class, :insert_into_database) super end + end + class CbHasOneParent + include Mongoid::Document include CallbackTracking + include RootInsertable + + has_one :child, autosave: true, class_name: "CbHasOneChild", inverse_of: :parent end class CbHasOneChild include Mongoid::Document + include CallbackTracking belongs_to :parent, class_name: "CbHasOneParent", inverse_of: :child - - def initialize(callback_registry) - @callback_registry = callback_registry - super() - end - - attr_accessor :callback_registry - - include CallbackTracking end class CbHasManyParent include Mongoid::Document + include CallbackTracking + include RootInsertable has_many :children, autosave: true, class_name: "CbHasManyChild", inverse_of: :parent - - def initialize(callback_registry) - @callback_registry = callback_registry - super() - end - - attr_accessor :callback_registry - - def insert_as_root - @callback_registry&.record_call(self.class, :insert_into_database) - super - end - - include CallbackTracking end class CbHasManyChild include Mongoid::Document + include CallbackTracking belongs_to :parent, class_name: "CbHasManyParent", inverse_of: :children - - def initialize(callback_registry) - @callback_registry = callback_registry - super() - end - - attr_accessor :callback_registry - - include CallbackTracking end class CbEmbedsOneParent include Mongoid::Document + include CallbackTracking + include RootInsertable field :name embeds_one :child, cascade_callbacks: true, class_name: "CbEmbedsOneChild", inverse_of: :parent - - def initialize(callback_registry) - @callback_registry = callback_registry - super() - end - - attr_accessor :callback_registry - - def insert_as_root - @callback_registry&.record_call(self.class, :insert_into_database) - super - end - - include CallbackTracking end class CbEmbedsOneChild include Mongoid::Document + include CallbackTracking field :age embedded_in :parent, class_name: "CbEmbedsOneParent", inverse_of: :child - - def initialize(callback_registry) - @callback_registry = callback_registry - super() - end - - attr_accessor :callback_registry - - include CallbackTracking end class CbEmbedsManyParent include Mongoid::Document + include CallbackTracking + include RootInsertable embeds_many :children, cascade_callbacks: true, class_name: "CbEmbedsManyChild", inverse_of: :parent - - def initialize(callback_registry) - @callback_registry = callback_registry - super() - end - - attr_accessor :callback_registry - - def insert_as_root - @callback_registry&.record_call(self.class, :insert_into_database) - super - end - - include CallbackTracking end class CbEmbedsManyChild include Mongoid::Document + include CallbackTracking embedded_in :parent, class_name: "CbEmbedsManyParent", inverse_of: :children - - def initialize(callback_registry) - @callback_registry = callback_registry - super() - end - - attr_accessor :callback_registry - - include CallbackTracking end class CbParent include Mongoid::Document - - def initialize(callback_registry) - @callback_registry = callback_registry - super() - end - - attr_accessor :callback_registry + include CallbackTracking embeds_many :cb_children embeds_many :cb_cascaded_children, cascade_callbacks: true - - include CallbackTracking + embeds_many :cb_cascaded_nodes, cascade_callbacks: true, as: :parent end class CbChild include Mongoid::Document + include CallbackTracking embedded_in :cb_parent - - def initialize(callback_registry, options) - @callback_registry = callback_registry - super(options) - end - - attr_accessor :callback_registry - - include CallbackTracking end class CbCascadedChild include Mongoid::Document + include CallbackTracking embedded_in :cb_parent - def initialize(callback_registry, options) - @callback_registry = callback_registry - super(options) - end - - attr_accessor :callback_registry - before_save :test_mongoid_state - include CallbackTracking - private # Helps test that cascading child callbacks have access to the Mongoid @@ -238,6 +153,15 @@ def test_mongoid_state Mongoid::Threaded.stack('interceptable').push(self) end end + + class CbCascadedNode + include Mongoid::Document + include CallbackTracking + + embedded_in :parent, polymorphic: true + + embeds_many :cb_cascaded_nodes, cascade_callbacks: true, as: :parent + end end class InterceptableBand