From ba552764344bc0a3c25b8576ec11f127ceaa16da Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 14 May 2013 16:03:09 -0700 Subject: deprecating string based terminators --- actionpack/lib/abstract_controller/callbacks.rb | 4 +++- activemodel/lib/active_model/callbacks.rb | 2 +- activemodel/lib/active_model/validations/callbacks.rb | 5 ++++- activerecord/lib/active_record/transactions.rb | 4 +++- activesupport/lib/active_support/callbacks.rb | 15 +++++++++++---- activesupport/test/callbacks_test.rb | 2 +- 6 files changed, 23 insertions(+), 9 deletions(-) diff --git a/actionpack/lib/abstract_controller/callbacks.rb b/actionpack/lib/abstract_controller/callbacks.rb index 599fff81c2..19cfd7dae1 100644 --- a/actionpack/lib/abstract_controller/callbacks.rb +++ b/actionpack/lib/abstract_controller/callbacks.rb @@ -8,7 +8,9 @@ module AbstractController include ActiveSupport::Callbacks included do - define_callbacks :process_action, :terminator => "response_body", :skip_after_callbacks_if_terminated => true + define_callbacks :process_action, + terminator: ->(controller,_) { controller.response_body }, + skip_after_callbacks_if_terminated: true end # Override AbstractController::Base's process_action to run the diff --git a/activemodel/lib/active_model/callbacks.rb b/activemodel/lib/active_model/callbacks.rb index 94d2181e4d..8b09f8b203 100644 --- a/activemodel/lib/active_model/callbacks.rb +++ b/activemodel/lib/active_model/callbacks.rb @@ -100,7 +100,7 @@ module ActiveModel def define_model_callbacks(*callbacks) options = callbacks.extract_options! options = { - terminator: "result == false", + terminator: ->(_,result) { result == 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 8de36b4f8b..cabb9482f2 100644 --- a/activemodel/lib/active_model/validations/callbacks.rb +++ b/activemodel/lib/active_model/validations/callbacks.rb @@ -22,7 +22,10 @@ module ActiveModel included do include ActiveSupport::Callbacks - define_callbacks :validation, terminator: "result == false", skip_after_callbacks_if_terminated: true, scope: [:kind, :name] + define_callbacks :validation, + terminator: ->(_,result) { result == false }, + skip_after_callbacks_if_terminated: true, + scope: [:kind, :name] end module ClassMethods diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index a5955ccba4..77634b40bb 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -10,7 +10,9 @@ module ActiveRecord end included do - define_callbacks :commit, :rollback, :terminator => "result == false", :scope => [:kind, :name] + define_callbacks :commit, :rollback, + terminator: ->(_, result) { result == false }, + scope: [:kind, :name] end # = Active Record Transactions diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index 5b8a300528..429e4f60a4 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -108,11 +108,11 @@ module ActiveSupport class Before def self.build(next_callback, user_callback, user_conditions, chain_config, filter) + halted_lambda = chain_config[:terminator] + if chain_config.key?(:terminator) && user_conditions.any? - halted_lambda = class_eval "lambda { |result| #{chain_config[:terminator]} }", __FILE__, __LINE__ halting_and_conditional(next_callback, user_callback, user_conditions, halted_lambda, filter) elsif chain_config.key? :terminator - halted_lambda = class_eval "lambda { |result| #{chain_config[:terminator]} }", __FILE__, __LINE__ halting(next_callback, user_callback, halted_lambda, filter) elsif user_conditions.any? conditional(next_callback, user_callback, user_conditions) @@ -131,7 +131,7 @@ module ActiveSupport if !halted && user_conditions.all? { |c| c.call(target, value) } result = user_callback.call target, value - env.halted = target.instance_exec result, &halted_lambda + env.halted = halted_lambda.call(target, result) if env.halted target.send :halted_callback_hook, filter end @@ -148,7 +148,7 @@ module ActiveSupport if !halted result = user_callback.call target, value - env.halted = target.instance_exec result, &halted_lambda + env.halted = halted_lambda.call(target, result) if env.halted target.send :halted_callback_hook, filter end @@ -752,6 +752,13 @@ module ActiveSupport # would call Audit#save. def define_callbacks(*callbacks) config = callbacks.last.is_a?(Hash) ? callbacks.pop : {} + if config.key?(:terminator) && String === config[:terminator] + ActiveSupport::Deprecation.warn "String based terminators are deprecated, please use a lambda" + value = config[:terminator] + l = class_eval "lambda { |result| #{value} }", __FILE__, __LINE__ + config[:terminator] = lambda { |target, result| target.instance_exec(result, &l) } + end + callbacks.each do |callback| class_attribute "_#{callback}_callbacks" set_callbacks callback, CallbackChain.new(callback, config) diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb index 86057f7cbb..5fce1eeafc 100644 --- a/activesupport/test/callbacks_test.rb +++ b/activesupport/test/callbacks_test.rb @@ -525,7 +525,7 @@ module CallbacksTest class CallbackTerminator include ActiveSupport::Callbacks - define_callbacks :save, :terminator => "result == :halt" + define_callbacks :save, :terminator => ->(_,result) { result == :halt } set_callback :save, :before, :first set_callback :save, :before, :second -- cgit v1.2.3