1 files changed, 57 insertions, 96 deletions

diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb
index e8ab3a7db5..d43fde03a9 100644
--- a/activesupport/lib/active_support/callbacks.rb
+++ b/activesupport/lib/active_support/callbacks.rb
@@ -4,6 +4,7 @@ require 'active_support/core_ext/array/extract_options'
 require 'active_support/core_ext/class/attribute'
 require 'active_support/core_ext/kernel/reporting'
 require 'active_support/core_ext/kernel/singleton_class'
+require 'active_support/core_ext/module/attribute_accessors'
 require 'active_support/core_ext/string/filters'
 require 'active_support/deprecation'
 require 'thread'
@@ -66,6 +67,12 @@ module ActiveSupport
 
     CALLBACK_FILTER_TYPES = [:before, :after, :around]
 
+    # If true, Active Record and Active Model callbacks returning +false+ will
+    # halt the entire callback chain and display a deprecation message.
+    # If false, callback chains will only be halted by calling +throw :abort+.
+    # Defaults to +true+.
+    mattr_accessor(:halt_and_display_warning_on_return_false) { true }
+
     # Runs the callbacks for the given event.
     #
     # Calls the before and around callbacks in the order they were set, yields
@@ -80,8 +87,12 @@ module ActiveSupport
     #     save
     #   end
     def run_callbacks(kind, &block)
-      callbacks = send("_#{kind}_callbacks")
+      send "_run_#{kind}_callbacks", &block
+    end
+
+    private
 
+    def __run_callbacks__(callbacks, &block)
       if callbacks.empty?
         yield if block_given?
       else
@@ -91,8 +102,6 @@ module ActiveSupport
       end
     end
 
-    private
-
     # A hook invoked every time a before callback is halted.
     # This can be overridden in AS::Callback implementors in order
     # to provide better debugging/logging.
@@ -124,14 +133,10 @@ module ActiveSupport
         def self.build(callback_sequence, user_callback, user_conditions, chain_config, filter)
           halted_lambda = chain_config[:terminator]
 
-          if chain_config.key?(:terminator) && user_conditions.any?
+          if user_conditions.any?
             halting_and_conditional(callback_sequence, user_callback, user_conditions, halted_lambda, filter)
-          elsif chain_config.key? :terminator
-            halting(callback_sequence, user_callback, halted_lambda, filter)
-          elsif user_conditions.any?
-            conditional(callback_sequence, user_callback, user_conditions)
           else
-            simple callback_sequence, user_callback
+            halting(callback_sequence, user_callback, halted_lambda, filter)
           end
         end
 
@@ -173,42 +178,15 @@ module ActiveSupport
           end
         end
         private_class_method :halting
-
-        def self.conditional(callback_sequence, user_callback, user_conditions)
-          callback_sequence.before do |env|
-            target = env.target
-            value  = env.value
-
-            if user_conditions.all? { |c| c.call(target, value) }
-              user_callback.call target, value
-            end
-
-            env
-          end
-        end
-        private_class_method :conditional
-
-        def self.simple(callback_sequence, user_callback)
-          callback_sequence.before do |env|
-            user_callback.call env.target, env.value
-
-            env
-          end
-        end
-        private_class_method :simple
       end
 
       class After
         def self.build(callback_sequence, user_callback, user_conditions, chain_config)
           if chain_config[:skip_after_callbacks_if_terminated]
-            if chain_config.key?(:terminator) && user_conditions.any?
+            if user_conditions.any?
               halting_and_conditional(callback_sequence, user_callback, user_conditions)
-            elsif chain_config.key?(:terminator)
-              halting(callback_sequence, user_callback)
-            elsif user_conditions.any?
-              conditional callback_sequence, user_callback, user_conditions
             else
-              simple callback_sequence, user_callback
+              halting(callback_sequence, user_callback)
             end
           else
             if user_conditions.any?
@@ -271,14 +249,10 @@ module ActiveSupport
 
       class Around
         def self.build(callback_sequence, user_callback, user_conditions, chain_config)
-          if chain_config.key?(:terminator) && user_conditions.any?
+          if user_conditions.any?
             halting_and_conditional(callback_sequence, user_callback, user_conditions)
-          elsif chain_config.key? :terminator
-            halting(callback_sequence, user_callback)
-          elsif user_conditions.any?
-            conditional(callback_sequence, user_callback, user_conditions)
           else
-            simple(callback_sequence, user_callback)
+            halting(callback_sequence, user_callback)
           end
         end
 
@@ -316,33 +290,6 @@ module ActiveSupport
           end
         end
         private_class_method :halting
-
-        def self.conditional(callback_sequence, user_callback, user_conditions)
-          callback_sequence.around do |env, &run|
-            target = env.target
-            value  = env.value
-
-            if user_conditions.all? { |c| c.call(target, value) }
-              user_callback.call(target, value) {
-                run.call.value
-              }
-              env
-            else
-              run.call
-            end
-          end
-        end
-        private_class_method :conditional
-
-        def self.simple(callback_sequence, user_callback)
-          callback_sequence.around do |env, &run|
-            user_callback.call(env.target, env.value) {
-              run.call.value
-            }
-            env
-          end
-        end
-        private_class_method :simple
       end
     end
 
@@ -510,12 +457,6 @@ module ActiveSupport
 
       attr_reader :name, :config
 
-      # If true, any callback returning +false+ will halt the entire callback
-      # chain and display a deprecation message. If false, callback chains will
-      # only be halted by calling +throw :abort+. Defaults to +true+.
-      class_attribute :halt_and_display_warning_on_return_false
-      self.halt_and_display_warning_on_return_false = true
-
       def initialize(name, config)
         @name = name
         @config = {
@@ -596,23 +537,12 @@ module ActiveSupport
         Proc.new do |target, result_lambda|
           terminate = true
           catch(:abort) do
-            result = result_lambda.call if result_lambda.is_a?(Proc)
-            if halt_and_display_warning_on_return_false && result == false
-              display_deprecation_warning_for_false_terminator
-            else
-              terminate = false
-            end
+            result_lambda.call if result_lambda.is_a?(Proc)
+            terminate = false
           end
           terminate
         end
       end
-
-      def display_deprecation_warning_for_false_terminator
-        ActiveSupport::Deprecation.warn(<<-MSG.squish)
-          Returning `false` in a callback will not implicitly halt a callback chain in the next release of Rails.
-          To explicitly halt a callback chain, please use `throw :abort` instead.
-        MSG
-      end
     end
 
     module ClassMethods
@@ -739,14 +669,15 @@ module ActiveSupport
       #   callback chain, preventing following before and around callbacks from
       #   being called and the event from being triggered.
       #   This should be a lambda to be executed.
-      #   The current object and the return result of the callback will be called
-      #   with the lambda.
+      #   The current object and the result lambda of the callback will be provided
+      #   to the terminator lambda.
       #
-      #     define_callbacks :validate, terminator: ->(target, result) { result == false }
+      #     define_callbacks :validate, terminator: ->(target, result_lambda) { result_lambda.call == false }
       #
       #   In this example, if any before validate callbacks returns +false+,
       #   any successive before and around callback is not executed.
-      #   Defaults to +false+, meaning no value halts the chain.
+      #
+      #   The default terminator halts the chain when a callback throws +:abort+.
       #
       # * <tt>:skip_after_callbacks_if_terminated</tt> - Determines if after
       #   callbacks should be terminated by the <tt>:terminator</tt> option. By
@@ -806,18 +737,48 @@ module ActiveSupport
         names.each do |name|
           class_attribute "_#{name}_callbacks"
           set_callbacks name, CallbackChain.new(name, options)
+
+          module_eval <<-RUBY, __FILE__, __LINE__ + 1
+            def _run_#{name}_callbacks(&block)
+              __run_callbacks__(_#{name}_callbacks, &block)
+            end
+          RUBY
         end
       end
 
       protected
 
-      def get_callbacks(name)
+      def get_callbacks(name) # :nodoc:
         send "_#{name}_callbacks"
       end
 
-      def set_callbacks(name, callbacks)
+      def set_callbacks(name, callbacks) # :nodoc:
         send "_#{name}_callbacks=", callbacks
       end
+
+      def deprecated_false_terminator # :nodoc:
+        Proc.new do |target, result_lambda|
+          terminate = true
+          catch(:abort) do
+            result = result_lambda.call if result_lambda.is_a?(Proc)
+            if Callbacks.halt_and_display_warning_on_return_false && result == false
+              display_deprecation_warning_for_false_terminator
+            else
+              terminate = false
+            end
+          end
+          terminate
+        end
+      end
+
+      private
+
+      def display_deprecation_warning_for_false_terminator
+        ActiveSupport::Deprecation.warn(<<-MSG.squish)
+          Returning `false` in Active Record and Active Model callbacks will not implicitly halt a callback chain in the next release of Rails.
+          To explicitly halt the callback chain, please use `throw :abort` instead.
+        MSG
+      end
     end
   end
 end