Permalink
Browse files

Add config to halt callback chain on return false

This stems from [a comment](#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
  • Loading branch information...
claudiob committed Dec 24, 2014
1 parent bb78af7 commit 9c65c539e2caa4590aded1975aead008f8135da4
@@ -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.
@@ -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*
@@ -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*
@@ -516,6 +516,12 @@ class CallbackChain #:nodoc:#
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 @@ def default_terminator
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
@@ -13,6 +13,13 @@ class Railtie < Rails::Railtie # :nodoc:
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|
@@ -571,6 +571,17 @@ def second
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 @@ def test_block_never_called_if_abort_is_thrown
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
@@ -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.
@@ -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
-------------------------------------
@@ -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*
@@ -88,12 +88,19 @@ def config
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
@@ -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
@@ -160,6 +160,38 @@ def test_rails_update_keep_the_cookie_serializer_if_it_is_already_configured
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]

0 comments on commit 9c65c53

Please sign in to comment.