diff options
| -rw-r--r-- | activemodel/CHANGELOG.md | 16 | ||||
| -rw-r--r-- | activerecord/CHANGELOG.md | 14 | ||||
| -rw-r--r-- | activesupport/CHANGELOG.md | 33 | ||||
| -rw-r--r-- | activesupport/lib/active_support/callbacks.rb | 8 | ||||
| -rw-r--r-- | activesupport/lib/active_support/railtie.rb | 7 | ||||
| -rw-r--r-- | activesupport/test/callbacks_test.rb | 50 | ||||
| -rw-r--r-- | guides/source/configuring.md | 2 | ||||
| -rw-r--r-- | guides/source/upgrading_ruby_on_rails.md | 22 | ||||
| -rw-r--r-- | railties/CHANGELOG.md | 15 | ||||
| -rw-r--r-- | railties/lib/rails/generators/rails/app/app_generator.rb | 7 | ||||
| -rw-r--r-- | railties/lib/rails/generators/rails/app/templates/config/initializers/callback_terminator.rb | 4 | ||||
| -rw-r--r-- | railties/test/generators/app_generator_test.rb | 32 |
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] |