From 56640b4de4e3245b087ca16e0f3bec751c6d6fb7 Mon Sep 17 00:00:00 2001 From: Ryan Wallace Date: Tue, 6 Feb 2018 14:53:26 -0500 Subject: [PATCH 1/2] Define callbacks on descendants. We set callbacks on all descendants, so we need to make sure that they are also defined on all descendants as well. --- activesupport/lib/active_support/callbacks.rb | 36 ++++++++++--------- .../test/callback_inheritance_test.rb | 10 ++++++ 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index 0ed4681b7d..cece3eb19f 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -806,28 +806,30 @@ module ActiveSupport def define_callbacks(*names) options = names.extract_options! - names.each do |name| - name = name.to_sym + ([self] + ActiveSupport::DescendantsTracker.descendants(self)).each do |target| + names.each do |name| + name = name.to_sym - set_callbacks name, CallbackChain.new(name, options) + target.set_callbacks name, CallbackChain.new(name, options) - module_eval <<-RUBY, __FILE__, __LINE__ + 1 - def _run_#{name}_callbacks(&block) - run_callbacks #{name.inspect}, &block - end + target.module_eval <<-RUBY, __FILE__, __LINE__ + 1 + def _run_#{name}_callbacks(&block) + run_callbacks #{name.inspect}, &block + end - def self._#{name}_callbacks - get_callbacks(#{name.inspect}) - end + def self._#{name}_callbacks + get_callbacks(#{name.inspect}) + end - def self._#{name}_callbacks=(value) - set_callbacks(#{name.inspect}, value) - end + def self._#{name}_callbacks=(value) + set_callbacks(#{name.inspect}, value) + end - def _#{name}_callbacks - __callbacks[#{name.inspect}] - end - RUBY + def _#{name}_callbacks + __callbacks[#{name.inspect}] + end + RUBY + end end end diff --git a/activesupport/test/callback_inheritance_test.rb b/activesupport/test/callback_inheritance_test.rb index 015e17deb9..5633b6e2b8 100644 --- a/activesupport/test/callback_inheritance_test.rb +++ b/activesupport/test/callback_inheritance_test.rb @@ -176,3 +176,13 @@ class DynamicInheritedCallbacks < ActiveSupport::TestCase assert_equal 1, child.count end end + +class DynamicDefinedCallbacks < ActiveSupport::TestCase + def test_callbacks_should_be_performed_once_in_child_class_after_dynamic_define + GrandParent.define_callbacks(:foo) + GrandParent.set_callback(:foo, :before, :before1) + parent = Parent.new("foo") + parent.run_callbacks(:foo) + assert_equal %w(before1), parent.log + end +end From 4efba09e88c5f4cdd78102c1d43ceea2c242a435 Mon Sep 17 00:00:00 2001 From: Ryan Wallace Date: Sat, 24 Feb 2018 21:32:11 +0100 Subject: [PATCH 2/2] No need to define methods on descendants. Addresses feedback from https://github.com/rails/rails/pull/31913#issuecomment-365983580 --- activesupport/lib/active_support/callbacks.rb | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index cece3eb19f..de42a075e1 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -806,30 +806,30 @@ module ActiveSupport def define_callbacks(*names) options = names.extract_options! - ([self] + ActiveSupport::DescendantsTracker.descendants(self)).each do |target| - names.each do |name| - name = name.to_sym + names.each do |name| + name = name.to_sym + ([self] + ActiveSupport::DescendantsTracker.descendants(self)).each do |target| target.set_callbacks name, CallbackChain.new(name, options) - - target.module_eval <<-RUBY, __FILE__, __LINE__ + 1 - def _run_#{name}_callbacks(&block) - run_callbacks #{name.inspect}, &block - end - - def self._#{name}_callbacks - get_callbacks(#{name.inspect}) - end - - def self._#{name}_callbacks=(value) - set_callbacks(#{name.inspect}, value) - end - - def _#{name}_callbacks - __callbacks[#{name.inspect}] - end - RUBY end + + module_eval <<-RUBY, __FILE__, __LINE__ + 1 + def _run_#{name}_callbacks(&block) + run_callbacks #{name.inspect}, &block + end + + def self._#{name}_callbacks + get_callbacks(#{name.inspect}) + end + + def self._#{name}_callbacks=(value) + set_callbacks(#{name.inspect}, value) + end + + def _#{name}_callbacks + __callbacks[#{name.inspect}] + end + RUBY end end