Merge pull request #17227 from claudiob/explicitly-abort-callbacks

Introduce explicit way of halting callback chains by throwing :abort. Deprecate current implicit behavior of halting callback chains by returning `false` in apps ported to Rails 5.0. Completely remove that behavior in brand new Rails 5.0 apps.

Conflicts:
	railties/CHANGELOG.md

6 files changed, 27 insertions, 22 deletions

diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index d7f655d00c..2a782c06d0 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -17,7 +17,7 @@ module ActiveRecord
           unless empty?
             record = klass.human_attribute_name(reflection.name).downcase
             owner.errors.add(:base, :"restrict_dependent_destroy.many", record: record)
-            false
+            throw(:abort)
           end
 
         else
diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb
index 74b8c53758..41a75b820e 100644
--- a/activerecord/lib/active_record/associations/has_one_association.rb
+++ b/activerecord/lib/active_record/associations/has_one_association.rb
@@ -13,7 +13,7 @@ module ActiveRecord
           if load_target
             record = klass.human_attribute_name(reflection.name).downcase
             owner.errors.add(:base, :"restrict_dependent_destroy.one", record: record)
-            false
+            throw(:abort)
           end
 
         else
diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index c39b045a5e..fa6c5e9e8c 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -200,13 +200,19 @@ module ActiveRecord
             after_create save_method
             after_update save_method
           else
-            define_non_cyclic_method(save_method) { save_belongs_to_association(reflection) }
+            define_non_cyclic_method(save_method) { throw(:abort) if save_belongs_to_association(reflection) == false }
             before_save save_method
           end
 
           if reflection.validate? && !method_defined?(validation_method)
             method = (collection ? :validate_collection_association : :validate_single_association)
-            define_non_cyclic_method(validation_method) { send(method, reflection) }
+            define_non_cyclic_method(validation_method) do
+              send(method, reflection)
+              # TODO: remove the following line as soon as the return value of
+              # callbacks is ignored, that is, returning `false` does not
+              # display a deprecation warning or halts the callback chain.
+              true
+            end
             validate validation_method
           end
         end
diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb
index 497ce8c15c..f44e5af5de 100644
--- a/activerecord/lib/active_record/callbacks.rb
+++ b/activerecord/lib/active_record/callbacks.rb
@@ -192,14 +192,14 @@ module ActiveRecord
   #
   # == <tt>before_validation*</tt> returning statements
   #
-  # If the returning value of a +before_validation+ callback can be evaluated to +false+, the process will be
+  # If the +before_validation+ callback throws +:abort+, the process will be
   # aborted and <tt>Base#save</tt> will return +false+. If Base#save! is called it will raise a
   # ActiveRecord::RecordInvalid exception. Nothing will be appended to the errors object.
   #
   # == Canceling callbacks
   #
-  # If a <tt>before_*</tt> callback returns +false+, all the later callbacks and the associated action are
-  # cancelled. If an <tt>after_*</tt> callback returns +false+, all the later callbacks are cancelled.
+  # If a <tt>before_*</tt> callback throws +:abort+, all the later callbacks and
+  # the associated action are cancelled.
   # Callbacks are generally run in the order they are defined, with the exception of callbacks defined as
   # methods on the model, which are called last.
   #
diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index f53c5f17ef..cf6673db2e 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -113,9 +113,9 @@ module ActiveRecord
     # the current time. However, if you supply <tt>touch: false</tt>, these
     # timestamps will not be updated.
     #
-    # There's a series of callbacks associated with +save+. If any of the
-    # <tt>before_*</tt> callbacks return +false+ the action is cancelled and
-    # +save+ returns +false+. See ActiveRecord::Callbacks for further
+    # There's a series of callbacks associated with #save. If any of the
+    # <tt>before_*</tt> callbacks throws +:abort+ the action is cancelled and
+    # #save returns +false+. See ActiveRecord::Callbacks for further
     # details.
     #
     # Attributes marked as readonly are silently ignored if the record is
@@ -139,9 +139,9 @@ module ActiveRecord
     # the current time. However, if you supply <tt>touch: false</tt>, these
     # timestamps will not be updated.
     #
-    # There's a series of callbacks associated with <tt>save!</tt>. If any of
-    # the <tt>before_*</tt> callbacks return +false+ the action is cancelled
-    # and <tt>save!</tt> raises ActiveRecord::RecordNotSaved. See
+    # There's a series of callbacks associated with #save!. If any of
+    # the <tt>before_*</tt> callbacks throws +:abort+ the action is cancelled
+    # and #save! raises ActiveRecord::RecordNotSaved. See
     # ActiveRecord::Callbacks for further details.
     #
     # Attributes marked as readonly are silently ignored if the record is
@@ -171,10 +171,10 @@ module ActiveRecord
     # Deletes the record in the database and freezes this instance to reflect
     # that no changes should be made (since they can't be persisted).
     #
-    # There's a series of callbacks associated with <tt>destroy</tt>. If
-    # the <tt>before_destroy</tt> callback return +false+ the action is cancelled
-    # and <tt>destroy</tt> returns +false+. See
-    # ActiveRecord::Callbacks for further details.
+    # There's a series of callbacks associated with #destroy. If the
+    # <tt>before_destroy</tt> callback throws +:abort+ the action is cancelled
+    # and #destroy returns +false+.
+    # See ActiveRecord::Callbacks for further details.
     def destroy
       raise ReadOnlyRecord, "#{self.class} is marked as readonly" if readonly?
       destroy_associations
@@ -186,10 +186,10 @@ module ActiveRecord
     # Deletes the record in the database and freezes this instance to reflect
     # that no changes should be made (since they can't be persisted).
     #
-    # There's a series of callbacks associated with <tt>destroy!</tt>. If
-    # the <tt>before_destroy</tt> callback return +false+ the action is cancelled
-    # and <tt>destroy!</tt> raises ActiveRecord::RecordNotDestroyed. See
-    # ActiveRecord::Callbacks for further details.
+    # There's a series of callbacks associated with #destroy!. If the
+    # <tt>before_destroy</tt> callback throws +:abort+ the action is cancelled
+    # and #destroy! raises ActiveRecord::RecordNotDestroyed.
+    # See ActiveRecord::Callbacks for further details.
     def destroy!
       destroy || raise(ActiveRecord::RecordNotDestroyed, self)
     end
diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb
index de701edca0..31ca90fb58 100644
--- a/activerecord/lib/active_record/transactions.rb
+++ b/activerecord/lib/active_record/transactions.rb
@@ -17,7 +17,6 @@ module ActiveRecord
 
     included do
       define_callbacks :commit, :rollback,
-                       terminator: ->(_, result) { result == false },
                        scope: [:kind, :name]
 
       mattr_accessor :raise_in_transactional_callbacks, instance_writer: false