diff --git a/activejob/CHANGELOG.md b/activejob/CHANGELOG.md index 1d3e89c184d10b243278a48d6be100c6464b768f..644d5d79a3e6a42d995e1f0a9461bd27b7f68f16 100644 --- a/activejob/CHANGELOG.md +++ b/activejob/CHANGELOG.md @@ -1,3 +1,17 @@ +* Don't run `after_enqueue` and `after_perform` callbacks if the callback chain is halted. + + class MyJob < ApplicationJob + before_enqueue { throw(:abort) } + after_enqueue { # won't enter here anymore } + end + + `after_enqueue` and `after_perform` callbacks will no longer run if the callback chain is halted. + This behaviour is a breaking change and won't take effect until Rails 6.2. + To enable this behaviour in your app right now, you can add in your app's configuration file + `config.active_job.skip_after_callbacks_if_terminated = true` + + *Edouard Chin* + * Fix enqueuing and performing incorrect logging message. Jobs will no longer always log "Enqueued MyJob" or "Performed MyJob" when they actually didn't get enqueued/performed. diff --git a/activejob/lib/active_job/callbacks.rb b/activejob/lib/active_job/callbacks.rb index 6b82ea6cab5a7bd867fde148755ed9bb86f831c9..e393da81a2f62e1a72bf02c39fc3419c9bac423a 100644 --- a/activejob/lib/active_job/callbacks.rb +++ b/activejob/lib/active_job/callbacks.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true require "active_support/callbacks" +require "active_support/core_ext/object/with_options" +require "active_support/core_ext/module/attribute_accessors" module ActiveJob # = Active Job Callbacks @@ -27,16 +29,33 @@ class << self end included do - define_callbacks :perform - define_callbacks :enqueue + class_attribute :return_false_on_aborted_enqueue, instance_accessor: false, instance_predicate: false, default: false + cattr_accessor :skip_after_callbacks_if_terminated, instance_accessor: false, default: false - class_attribute :return_false_on_aborted_enqueue, instance_accessor: false, instance_predicate: false - self.return_false_on_aborted_enqueue = false + with_options(skip_after_callbacks_if_terminated: skip_after_callbacks_if_terminated) do + define_callbacks :perform + define_callbacks :enqueue + end end # These methods will be included into any Active Job object, adding # callbacks for +perform+ and +enqueue+ methods. module ClassMethods + def inherited(klass) + unless skip_after_callbacks_if_terminated + ActiveSupport::Deprecation.warn(<<~EOM) + In Rails 6.2, ActiveJob's `after_enqueue` and `after_perform` callbacks will no longer run in case the + callback chain is halted (i.e. `throw(:abort)` is thrown in a before_enqueue callback). + To enable this behaviour right now, add in your application configuration file + `config.active_job.skip_after_callbacks_if_terminated = true`. + EOM + end + + klass.get_callbacks(:enqueue).config[:skip_after_callbacks_if_terminated] = skip_after_callbacks_if_terminated + klass.get_callbacks(:perform).config[:skip_after_callbacks_if_terminated] = skip_after_callbacks_if_terminated + super + end + # Defines a callback that will get called right before the # job's perform method is executed. # diff --git a/activejob/test/cases/callbacks_test.rb b/activejob/test/cases/callbacks_test.rb index 895edb34a558a80b5b00565168fa2af81da2d9e0..87dc7634ae97d024a4f2d0ffa9c7ad7075846b62 100644 --- a/activejob/test/cases/callbacks_test.rb +++ b/activejob/test/cases/callbacks_test.rb @@ -43,8 +43,63 @@ class CallbacksTest < ActiveSupport::TestCase ActiveJob::Base.return_false_on_aborted_enqueue = prev end + test "#enqueue does not run after_enqueue callbacks when skip_after_callbacks_if_terminated is true" do + prev = ActiveJob::Base.skip_after_callbacks_if_terminated + ActiveJob::Base.skip_after_callbacks_if_terminated = true + reload_job + job = AbortBeforeEnqueueJob.new + job.enqueue + + assert_nil(job.flag) + ensure + ActiveJob::Base.skip_after_callbacks_if_terminated = prev + end + + test "#enqueue does run after_enqueue callbacks when skip_after_callbacks_if_terminated is false" do + prev = ActiveJob::Base.skip_after_callbacks_if_terminated + ActiveJob::Base.skip_after_callbacks_if_terminated = false + reload_job + job = AbortBeforeEnqueueJob.new + job.enqueue + + assert_equal("after_enqueue", job.flag) + ensure + ActiveJob::Base.skip_after_callbacks_if_terminated = prev + end + + test "#perform does not run after_perform callbacks when skip_after_callbacks_if_terminated is true" do + prev = ActiveJob::Base.skip_after_callbacks_if_terminated + ActiveJob::Base.skip_after_callbacks_if_terminated = true + reload_job + job = AbortBeforeEnqueueJob.new + job.perform_now + + assert_nil(job.flag) + ensure + ActiveJob::Base.skip_after_callbacks_if_terminated = prev + end + + test "#perform does run after_perform callbacks when skip_after_callbacks_if_terminated is false" do + prev = ActiveJob::Base.skip_after_callbacks_if_terminated + ActiveJob::Base.skip_after_callbacks_if_terminated = false + reload_job + job = AbortBeforeEnqueueJob.new + job.perform_now + + assert_equal("after_perform", job.flag) + ensure + ActiveJob::Base.skip_after_callbacks_if_terminated = prev + end + test "#enqueue returns self when the job was enqueued" do job = CallbackJob.new assert_equal job, job.enqueue end + + private + def reload_job + Object.send(:remove_const, :AbortBeforeEnqueueJob) + $LOADED_FEATURES.delete($LOADED_FEATURES.grep(%r{jobs/abort_before_enqueue_job}).first) + require "jobs/abort_before_enqueue_job" + end end diff --git a/activejob/test/jobs/abort_before_enqueue_job.rb b/activejob/test/jobs/abort_before_enqueue_job.rb index 8fb4d8078c90ef8b3ba90a10527c763006d5639d..24206abd2e991eef82f390388884a21c50eb8776 100644 --- a/activejob/test/jobs/abort_before_enqueue_job.rb +++ b/activejob/test/jobs/abort_before_enqueue_job.rb @@ -4,7 +4,11 @@ class AbortBeforeEnqueueJob < ActiveJob::Base MyError = Class.new(StandardError) before_enqueue :throw_or_raise + after_enqueue { self.flag = "after_enqueue" } before_perform { throw(:abort) } + after_perform { self.flag = "after_perform" } + + attr_accessor :flag def perform raise "This should never be called" diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index 5189698d7337f9ce6c670f5c453b27913eed7ca4..d57a86006dd5ed2d9a4ca3c832eea2b9a46b08f4 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -163,6 +163,10 @@ def load_defaults(target_version) if respond_to?(:active_storage) active_storage.track_variants = true end + + if respond_to?(:active_job) + active_job.skip_after_callbacks_if_terminated = true + end else raise "Unknown version #{target_version.to_s.inspect}" end diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_1.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_1.rb.tt index cf534ac1f81d50879107881095985fc4bc9c0914..e161bf6593fc9e8f71979f35691a012aff55172b 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_1.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_1.rb.tt @@ -11,3 +11,5 @@ # Track Active Storage variants in the database. # Rails.application.config.active_storage.track_variants = true + +# Rails.application.config.active_job.skip_after_callbacks_if_terminated = true diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 4de07ebb47ccc80f505c403f60cd9e9ff8d29701..7084185a3357ed5abd27f5638770564c0ba45103 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -2277,6 +2277,21 @@ class ::DummySerializer < ActiveJob::Serializers::ObjectSerializer; end assert_equal false, ActiveJob::Base.return_false_on_aborted_enqueue end + test "ActiveJob::Base.skip_after_callbacks_if_terminated is true by default" do + app "development" + + assert_equal true, ActiveJob::Base.skip_after_callbacks_if_terminated + end + + test "ActiveJob::Base.skip_after_callbacks_if_terminated is false in the 6.0 defaults" do + remove_from_config '.*config\.load_defaults.*\n' + add_to_config 'config.load_defaults "6.0"' + + app "development" + + assert_equal false, ActiveJob::Base.skip_after_callbacks_if_terminated + end + test "ActiveStorage.queues[:analysis] is :active_storage_analysis by default" do app "development"