Add config to halt callback chain on return false

This stems from [a comment](rails#17227 (comment)) by @dhh.
In summary:

* New Rails 5.0 apps will not accept `return false` as a way to halt callback chains, and will not display a deprecation warning.
* Existing apps ported to Rails 5.0 will still accept `return false` as a way to halt callback chains, albeit with a deprecation warning.

For this purpose, this commit introduces a Rails configuration option:

```ruby
config.active_support.halt_callback_chains_on_return_false
```

For new Rails 5.0 apps, this option will be set to `false` by a new initializer
`config/initializers/callback_terminator.rb`:

```ruby
Rails.application.config.active_support.halt_callback_chains_on_return_false = false
```

For existing apps ported to Rails 5.0, the initializers above will not exist.
Even running `rake rails:update` will not create this initializer.

Since the default value of `halt_callback_chains_on_return_false` is set to
`true`, these apps will still accept `return true` as a way to halt callback
chains, displaying a deprecation warning.

Developers will be able to switch to the new behavior (and stop the warning)
by manually adding the line above to their `config/application.rb`.

A gist with the suggested release notes to add to Rails 5.0 after this
commit is available at https://gist.github.com/claudiob/614c59409fb7d11f2931

12 files changed, 191 insertions, 19 deletions

diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md
index b52ec8a67f..d643a08235 100644
--- a/activemodel/CHANGELOG.md
+++ b/activemodel/CHANGELOG.md
@@ -1,10 +1,12 @@
-*   Deprecate returning `false` as a way to halt callback chains.
-
-    Returning `false` in a `before_` callback will display a
-    deprecation warning explaining that the preferred method to halt a callback
-    chain is to explicitly `throw(:abort)`.
-
-    *claudiob*
+*   Change the way in which callback chains can be halted.
+
+    The preferred method to halt a callback chain from now on is to explicitly
+    `throw(:abort)`.
+    In the past, returning `false` in an ActiveModel or ActiveModel::Validations
+    `before_` callback had the side effect of halting the callback chain.
+    This is not recommended anymore and, depending on the value of the
+    `config.active_support.halt_callback_chains_on_return_false` option, will
+    either not work at all or display a deprecation warning.
 
 
 Please check [4-2-stable](https://github.com/rails/rails/blob/4-2-stable/activemodel/CHANGELOG.md) for previous changes.
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 60313a0a02..83224c522c 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,8 +1,12 @@
-*   Deprecate returning `false` as a way to halt callback chains.
-
-    Returning `false` in a `before_` callback will display a
-    deprecation warning explaining that the preferred method to halt a callback
-    chain is to explicitly `throw(:abort)`.
+*   Change the way in which callback chains can be halted.
+
+    The preferred method to halt a callback chain from now on is to explicitly
+    `throw(:abort)`.
+    In the past, returning `false` in an ActiveRecord `before_` callback had the
+    side effect of halting the callback chain.
+    This is not recommended anymore and, depending on the value of the
+    `config.active_support.halt_callback_chains_on_return_false` option, will
+    either not work at all or display a deprecation warning.
 
     *claudiob*
 
diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 30e2bdd1e1..f59ad476ae 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,8 +1,30 @@
-*   Deprecate returning `false` as a way to halt callback chains.
+*   Change the way in which callback chains can be halted.
 
-    Returning `false` in a callback will display a deprecation warning
-    explaining that the preferred method to halt a callback chain is to
-    explicitly `throw(:abort)`.
+    The preferred method to halt a callback chain from now on is to explicitly
+    `throw(:abort)`.
+    In the past, returning `false` in an ActiveSupport callback had the side
+    effect of halting the callback chain. This is not recommended anymore and,
+    depending on the value of
+    `Callbacks::CallbackChain.halt_and_display_warning_on_return_false`, will
+    either not work at all or display a deprecation warning.
+
+
+*   Add Callbacks::CallbackChain.halt_and_display_warning_on_return_false
+
+    Setting `Callbacks::CallbackChain.halt_and_display_warning_on_return_false`
+    to true will let an app support the deprecated way of halting callback
+    chains by returning `false`.
+
+    Setting the value to false will tell the app to ignore any `false` value
+    returned by callbacks, and only halt the chain upon `throw(:abort)`.
+
+    The value can also be set with the Rails configuration option
+    `config.active_support.halt_callback_chains_on_return_false`.
+
+    When the configuration option is missing, its value is `true`, so older apps
+    ported to Rails 5.0 will not break (but display a deprecation warning).
+    For new Rails 5.0 apps, its value is set to `false` in an initializer, so
+    these apps will support the new behavior by default.
 
     *claudiob*
 
@@ -13,8 +35,7 @@
 
     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`.
+    terminator's expectation.
 
     *claudiob*
 
diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb
index 7dd97eb1ab..0f1de8b076 100644
--- a/activesupport/lib/active_support/callbacks.rb
+++ b/activesupport/lib/active_support/callbacks.rb
@@ -516,6 +516,12 @@ 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 = {
@@ -597,7 +603,7 @@ module ActiveSupport
           terminate = true
           catch(:abort) do
             result = result_lambda.call if result_lambda.is_a?(Proc)
-            if result == false
+            if halt_and_display_warning_on_return_false && result == false
               display_deprecation_warning_for_false_terminator
             else
               terminate = false
diff --git a/activesupport/lib/active_support/railtie.rb b/activesupport/lib/active_support/railtie.rb
index 133aa6a054..6eba24b569 100644
--- a/activesupport/lib/active_support/railtie.rb
+++ b/activesupport/lib/active_support/railtie.rb
@@ -13,6 +13,13 @@ module ActiveSupport
       end
     end
 
+    initializer "active_support.halt_callback_chains_on_return_false", after: :load_config_initializers do |app|
+      if app.config.active_support.key? :halt_callback_chains_on_return_false
+        ActiveSupport::Callbacks::CallbackChain.halt_and_display_warning_on_return_false = \
+          app.config.active_support.halt_callback_chains_on_return_false
+      end
+    end
+
     # Sets the default value for Time.zone
     # If assigned value cannot be matched to a TimeZone, an exception will be raised.
     initializer "active_support.initialize_time_zone" do |app|
diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb
index 73b00e80f6..f6abef8cee 100644
--- a/activesupport/test/callbacks_test.rb
+++ b/activesupport/test/callbacks_test.rb
@@ -571,6 +571,17 @@ module CallbacksTest
     set_save_callbacks
   end
 
+  class CallbackFalseTerminator < AbstractCallbackTerminator
+    define_callbacks :save
+
+    def second
+      @history << "second"
+      false
+    end
+
+    set_save_callbacks
+  end
+
   class CallbackObject
     def before(caller)
       caller.record << "before"
@@ -754,6 +765,45 @@ module CallbacksTest
     end
   end
 
+  class CallbackFalseTerminatorWithoutConfigTest < ActiveSupport::TestCase
+    def test_returning_false_halts_callback_if_config_variable_is_not_set
+      obj = CallbackFalseTerminator.new
+      assert_deprecated do
+        obj.save
+        assert_equal :second, obj.halted
+        assert !obj.saved
+      end
+    end
+  end
+
+  class CallbackFalseTerminatorWithConfigTrueTest < ActiveSupport::TestCase
+    def setup
+      ActiveSupport::Callbacks::CallbackChain.halt_and_display_warning_on_return_false = true
+    end
+
+    def test_returning_false_halts_callback_if_config_variable_is_true
+      obj = CallbackFalseTerminator.new
+      assert_deprecated do
+        obj.save
+        assert_equal :second, obj.halted
+        assert !obj.saved
+      end
+    end
+  end
+
+  class CallbackFalseTerminatorWithConfigFalseTest < ActiveSupport::TestCase
+    def setup
+      ActiveSupport::Callbacks::CallbackChain.halt_and_display_warning_on_return_false = false
+    end
+
+    def test_returning_false_does_not_halt_callback_if_config_variable_is_false
+      obj = CallbackFalseTerminator.new
+      obj.save
+      assert_equal nil, obj.halted
+      assert obj.saved
+    end
+  end
+
   class HyphenatedKeyTest < ActiveSupport::TestCase
     def test_save
       obj = HyphenatedCallbacks.new
diff --git a/guides/source/configuring.md b/guides/source/configuring.md
index 0a375d7cb8..5baab8a4b5 100644
--- a/guides/source/configuring.md
+++ b/guides/source/configuring.md
@@ -507,6 +507,8 @@ There are a few configuration options available in Active Support:
 
 * `config.active_support.time_precision` sets the precision of JSON encoded time values. Defaults to `3`.
 
+* `config.active_support.halt_callback_chains_on_return_false` specifies whether ActiveRecord, ActiveModel and ActiveModel::Validations callback chains can be halted by returning `false` in a 'before' callback. Defaults to `true`.
+
 * `ActiveSupport::Logger.silencer` is set to `false` to disable the ability to silence logging in a block. The default is `true`.
 
 * `ActiveSupport::Cache::Store.logger` specifies the logger to use within cache store operations.
diff --git a/guides/source/upgrading_ruby_on_rails.md b/guides/source/upgrading_ruby_on_rails.md
index 0b9f59bb46..c0c94475e0 100644
--- a/guides/source/upgrading_ruby_on_rails.md
+++ b/guides/source/upgrading_ruby_on_rails.md
@@ -53,6 +53,28 @@ Don't forget to review the difference, to see if there were any unexpected chang
 Upgrading from Rails 4.2 to Rails 5.0
 -------------------------------------
 
+### Halting callback chains by returning `false`
+
+In Rails 4.2, when a 'before' callback returns `false` in ActiveRecord,
+ActiveModel and ActiveModel::Validations, then the entire callback chain
+is halted. In other words, successive 'before' callbacks are not executed,
+and neither is the action wrapped in callbacks.
+
+In Rails 5.0, returning `false` in a callback will not have this side effect
+of halting the callback chain. Instead, callback chains must be explicitly
+halted by calling `throw(:abort)`.
+
+When you upgrade from Rails 4.2 to Rails 5.0, returning `false` in a callback
+will still halt the callback chain, but you will receive a deprecation warning
+about this upcoming change.
+
+When you are ready, you can opt into the new behavior and remove the deprecation
+warning by adding the following configuration to your `config/application.rb`:
+
+    config.active_support.halt_callback_chains_on_return_false = false
+
+See [#17227](https://github.com/rails/rails/pull/17227) for more details.
+
 Upgrading from Rails 4.1 to Rails 4.2
 -------------------------------------
 
diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md
index d583be5e73..9165019d0d 100644
--- a/railties/CHANGELOG.md
+++ b/railties/CHANGELOG.md
@@ -1,3 +1,18 @@
+*   Add `config/initializers/callback_terminator.rb`
+
+    Newly generated Rails apps have a new initializer called
+    `callback_terminator.rb` which sets the value of the configuration option
+    `config.active_support.halt_callback_chains_on_return_false` to `false`.
+
+    As a result, new Rails apps do not halt callback chains when a callback
+    returns `false`; only when they are explicitly halted with `throw(:abort)`.
+
+    The terminator is *not* added when running `rake rails:update`, so returning
+    `false` will still work on old apps ported to Rails 5, displaying a
+    deprecation warning to prompt users to update their code to the new syntax.
+
+    *claudiob*
+
 *   Add `--skip-action-mailer` option to the app generator.
 
     *claudiob*
diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb
index 1ff1f970b5..c235fbc7f7 100644
--- a/railties/lib/rails/generators/rails/app/app_generator.rb
+++ b/railties/lib/rails/generators/rails/app/app_generator.rb
@@ -88,12 +88,19 @@ module Rails
 
     def config_when_updating
       cookie_serializer_config_exist = File.exist?('config/initializers/cookies_serializer.rb')
+      callback_terminator_config_exist = File.exist?('config/initializers/callback_terminator.rb')
 
       config
 
+      unless callback_terminator_config_exist
+        remove_file 'config/initializers/callback_terminator.rb'
+      end
+
       unless cookie_serializer_config_exist
         gsub_file 'config/initializers/cookies_serializer.rb', /json/, 'marshal'
       end
+
+
     end
 
     def database_yml
diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/callback_terminator.rb b/railties/lib/rails/generators/rails/app/templates/config/initializers/callback_terminator.rb
new file mode 100644
index 0000000000..e63022da91
--- /dev/null
+++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/callback_terminator.rb
@@ -0,0 +1,4 @@
+# Be sure to restart your server when you modify this file.
+
+# Do not halt callback chains when a callback returns false.
+Rails.application.config.active_support.halt_callback_chains_on_return_false = false
diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb
index ac8f735cec..e30c9a7b2f 100644
--- a/railties/test/generators/app_generator_test.rb
+++ b/railties/test/generators/app_generator_test.rb
@@ -160,6 +160,38 @@ class AppGeneratorTest < Rails::Generators::TestCase
     assert_file("#{app_root}/config/initializers/cookies_serializer.rb", /Rails\.application\.config\.action_dispatch\.cookies_serializer = :json/)
   end
 
+  def test_rails_update_does_not_create_callback_terminator_initializer
+    app_root = File.join(destination_root, 'myapp')
+    run_generator [app_root]
+
+    FileUtils.rm("#{app_root}/config/initializers/callback_terminator.rb")
+
+    Rails.application.config.root = app_root
+    Rails.application.class.stubs(:name).returns("Myapp")
+    Rails.application.stubs(:is_a?).returns(Rails::Application)
+
+    generator = Rails::Generators::AppGenerator.new ["rails"], { with_dispatchers: true }, destination_root: app_root, shell: @shell
+    generator.send(:app_const)
+    quietly { generator.send(:update_config_files) }
+    assert_no_file "#{app_root}/config/initializers/callback_terminator.rb"
+  end
+
+  def test_rails_update_does_not_remove_callback_terminator_initializer_if_already_present
+    app_root = File.join(destination_root, 'myapp')
+    run_generator [app_root]
+
+    FileUtils.touch("#{app_root}/config/initializers/callback_terminator.rb")
+
+    Rails.application.config.root = app_root
+    Rails.application.class.stubs(:name).returns("Myapp")
+    Rails.application.stubs(:is_a?).returns(Rails::Application)
+
+    generator = Rails::Generators::AppGenerator.new ["rails"], { with_dispatchers: true }, destination_root: app_root, shell: @shell
+    generator.send(:app_const)
+    quietly { generator.send(:update_config_files) }
+    assert_file "#{app_root}/config/initializers/callback_terminator.rb"
+  end
+
   def test_rails_update_set_the_cookie_serializer_to_marchal_if_it_is_not_already_configured
     app_root = File.join(destination_root, 'myapp')
     run_generator [app_root]