aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--actionpack/lib/abstract_controller/callbacks.rb2
-rw-r--r--activemodel/lib/active_model/callbacks.rb2
-rw-r--r--activemodel/lib/active_model/validations/callbacks.rb2
-rw-r--r--activerecord/lib/active_record/transactions.rb2
-rw-r--r--activesupport/CHANGELOG.md12
-rw-r--r--activesupport/lib/active_support/callbacks.rb23
-rw-r--r--activesupport/test/callbacks_test.rb39
7 files changed, 68 insertions, 14 deletions
diff --git a/actionpack/lib/abstract_controller/callbacks.rb b/actionpack/lib/abstract_controller/callbacks.rb
index ca5c80cd71..60604db447 100644
--- a/actionpack/lib/abstract_controller/callbacks.rb
+++ b/actionpack/lib/abstract_controller/callbacks.rb
@@ -9,7 +9,7 @@ module AbstractController
included do
define_callbacks :process_action,
- terminator: ->(controller,_) { controller.response_body },
+ terminator: ->(controller, result_lambda) { result_lambda.call if result_lambda.is_a?(Proc); controller.response_body },
skip_after_callbacks_if_terminated: true
end
diff --git a/activemodel/lib/active_model/callbacks.rb b/activemodel/lib/active_model/callbacks.rb
index b3d70dc515..061d96a80b 100644
--- a/activemodel/lib/active_model/callbacks.rb
+++ b/activemodel/lib/active_model/callbacks.rb
@@ -103,7 +103,7 @@ module ActiveModel
def define_model_callbacks(*callbacks)
options = callbacks.extract_options!
options = {
- terminator: ->(_,result) { result == false },
+ terminator: ->(_,result_lambda) { result_lambda.call == false },
skip_after_callbacks_if_terminated: true,
scope: [:kind, :name],
only: [:before, :around, :after]
diff --git a/activemodel/lib/active_model/validations/callbacks.rb b/activemodel/lib/active_model/validations/callbacks.rb
index 25ccabd66b..449f1a0299 100644
--- a/activemodel/lib/active_model/validations/callbacks.rb
+++ b/activemodel/lib/active_model/validations/callbacks.rb
@@ -23,7 +23,7 @@ module ActiveModel
included do
include ActiveSupport::Callbacks
define_callbacks :validation,
- terminator: ->(_,result) { result == false },
+ terminator: ->(_,result_lambda) { result_lambda.call == false },
skip_after_callbacks_if_terminated: true,
scope: [:kind, :name]
end
diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb
index de701edca0..95e9a8646e 100644
--- a/activerecord/lib/active_record/transactions.rb
+++ b/activerecord/lib/active_record/transactions.rb
@@ -17,7 +17,7 @@ module ActiveRecord
included do
define_callbacks :commit, :rollback,
- terminator: ->(_, result) { result == false },
+ terminator: ->(_, result_lambda) { result_lambda.call == false },
scope: [:kind, :name]
mattr_accessor :raise_in_transactional_callbacks, instance_writer: false
diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 3606d7e572..01bb816923 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,15 @@
+* Changes arguments and default value of CallbackChain's :terminator option
+
+ Chains of callbacks defined without an explicit `:terminator` option will
+ now be halted as soon as a `before_` callback throws `:abort`.
+
+ Chains of callbacks defined with a `:terminator` option will maintain their
+ existing behavior of halting as soon as a `before_` callback matches the
+ terminator's expectation. For instance, ActiveModel's callbacks will still
+ halt the chain when a `before_` callback returns `false`.
+
+ *claudiob*
+
* Deprecate `MissingSourceFile` in favor of `LoadError`.
`MissingSourceFile` was just an alias to `LoadError` and was not being
diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb
index d2911a254c..3a3061e536 100644
--- a/activesupport/lib/active_support/callbacks.rb
+++ b/activesupport/lib/active_support/callbacks.rb
@@ -142,8 +142,8 @@ module ActiveSupport
halted = env.halted
if !halted && user_conditions.all? { |c| c.call(target, value) }
- result = user_callback.call target, value
- env.halted = halted_lambda.call(target, result)
+ result_lambda = -> { user_callback.call target, value }
+ env.halted = halted_lambda.call(target, result_lambda)
if env.halted
target.send :halted_callback_hook, filter
end
@@ -161,8 +161,9 @@ module ActiveSupport
halted = env.halted
unless halted
- result = user_callback.call target, value
- env.halted = halted_lambda.call(target, result)
+ result_lambda = -> { user_callback.call target, value }
+ env.halted = halted_lambda.call(target, result_lambda)
+
if env.halted
target.send :halted_callback_hook, filter
end
@@ -517,7 +518,8 @@ module ActiveSupport
def initialize(name, config)
@name = name
@config = {
- :scope => [ :kind ]
+ scope: [:kind],
+ terminator: default_terminator
}.merge!(config)
@chain = []
@callbacks = nil
@@ -588,6 +590,17 @@ module ActiveSupport
@callbacks = nil
@chain.delete_if { |c| callback.duplicates?(c) }
end
+
+ def default_terminator
+ Proc.new do |target, result_lambda|
+ terminate = true
+ catch(:abort) do
+ result_lambda.call if result_lambda.is_a?(Proc)
+ terminate = false
+ end
+ terminate
+ end
+ end
end
module ClassMethods
diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb
index d19e5fd6e7..73b00e80f6 100644
--- a/activesupport/test/callbacks_test.rb
+++ b/activesupport/test/callbacks_test.rb
@@ -511,8 +511,6 @@ module CallbacksTest
set_callback :save, :before, :third
set_callback :save, :after, :first
set_callback :save, :around, :around_it
- set_callback :save, :after, :second
- set_callback :save, :around, :around_it
set_callback :save, :after, :third
end
@@ -552,16 +550,27 @@ module CallbacksTest
end
class CallbackTerminator < AbstractCallbackTerminator
- define_callbacks :save, terminator: ->(_,result) { result == :halt }
+ define_callbacks :save, terminator: ->(_, result_lambda) { result_lambda.call == :halt }
set_save_callbacks
end
class CallbackTerminatorSkippingAfterCallbacks < AbstractCallbackTerminator
- define_callbacks :save, terminator: ->(_,result) { result == :halt },
+ define_callbacks :save, terminator: ->(_, result_lambda) { result_lambda.call == :halt },
skip_after_callbacks_if_terminated: true
set_save_callbacks
end
+ class CallbackDefaultTerminator < AbstractCallbackTerminator
+ define_callbacks :save
+
+ def second
+ @history << "second"
+ throw(:abort)
+ end
+
+ set_save_callbacks
+ end
+
class CallbackObject
def before(caller)
caller.record << "before"
@@ -701,7 +710,7 @@ module CallbacksTest
def test_termination_skips_following_before_and_around_callbacks
terminator = CallbackTerminator.new
terminator.save
- assert_equal ["first", "second", "third", "second", "first"], terminator.history
+ assert_equal ["first", "second", "third", "first"], terminator.history
end
def test_termination_invokes_hook
@@ -725,6 +734,26 @@ module CallbacksTest
end
end
+ class CallbackDefaultTerminatorTest < ActiveSupport::TestCase
+ def test_default_termination
+ terminator = CallbackDefaultTerminator.new
+ terminator.save
+ assert_equal ["first", "second", "third", "first"], terminator.history
+ end
+
+ def test_default_termination_invokes_hook
+ terminator = CallbackDefaultTerminator.new
+ terminator.save
+ assert_equal :second, terminator.halted
+ end
+
+ def test_block_never_called_if_abort_is_thrown
+ obj = CallbackDefaultTerminator.new
+ obj.save
+ assert !obj.saved
+ end
+ end
+
class HyphenatedKeyTest < ActiveSupport::TestCase
def test_save
obj = HyphenatedCallbacks.new
generated by cgit v1.2.3 (git 2.25.1) at 2024-12-25 17:55:57 +0000