diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a1e69ecfeb..eb49be24e7 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -293,6 +293,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 34fb684940..fbd0e5246e 100644 --- a/lib/mongoid/interceptable.rb +++ b/lib/mongoid/interceptable.rb @@ -141,9 +141,13 @@ def run_callbacks(kind, with_children: true, &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 @@ -218,9 +222,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 1c75b0c94c..ab2ed5574f 100644 --- a/spec/mongoid/interceptable_spec.rb +++ b/spec/mongoid/interceptable_spec.rb @@ -388,6 +388,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 71e7680899..7b72afee8b 100644 --- a/spec/mongoid/interceptable_spec_models.rb +++ b/spec/mongoid/interceptable_spec_models.rb @@ -1,10 +1,12 @@ 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 @@ -15,6 +17,8 @@ module CallbackTracking extend ActiveSupport::Concern included do + field :name, type: String + %i( validation save create update ).each do |what| @@ -34,196 +38,128 @@ 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 + before_save :test_mongoid_state - attr_accessor :callback_registry + private + + # Helps test that cascading child callbacks have access to the Mongoid + # state objects; if the implementation uses fiber-local (instead of truly + # thread-local) variables, the related tests will fail because the + # cascading child callbacks use fibers to linearize the recursion. + 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