Merge pull request rails#37950 from Edouard-chin/ec-activejob-too-much-deprecation

Fix deprecation being thrown at boot time:

Author: rafaelfranca

activejob/lib/active_job/callbacks.rb

@@ -42,15 +42,6 @@ class << self
     # 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

activejob/lib/active_job/enqueuing.rb

@@ -63,6 +63,14 @@ def enqueue(options = {})
       if successfully_enqueued
         self
       else
+        if !self.class.skip_after_callbacks_if_terminated && _enqueue_callbacks.any? { |c| c.kind == :after }
+          ActiveSupport::Deprecation.warn(<<~EOM)
+            In Rails 6.2, ActiveJob's `after_enqueue` 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
         if self.class.return_false_on_aborted_enqueue
           false
         else

activejob/lib/active_job/execution.rb

@@ -35,9 +35,23 @@ def perform_now
       self.executions = (executions || 0) + 1
 
       deserialize_arguments_if_needed
+      successfully_performed = false
+
       run_callbacks :perform do
         perform(*arguments)
+
+        successfully_performed = true
+      end
+
+      if !successfully_performed && !self.class.skip_after_callbacks_if_terminated && _perform_callbacks.any? { |c| c.kind == :after }
+        ActiveSupport::Deprecation.warn(<<~EOM)
+          In Rails 6.2, ActiveJob's `after_perform` callbacks will no longer run in case the
+          callback chain is halted (i.e. `throw(:abort)` is thrown in a before_perform callback).
+          To enable this behaviour right now, add in your application configuration file
+          `config.active_job.skip_after_callbacks_if_terminated = true`.
+        EOM
       end
+      successfully_performed
     rescue => exception
       rescue_with_handler(exception) || raise
     end

activejob/test/cases/callbacks_test.rb

@@ -27,7 +27,10 @@ class CallbacksTest < ActiveSupport::TestCase
   test "#enqueue returns false when before_enqueue aborts callback chain and return_false_on_aborted_enqueue = true" do
     prev = ActiveJob::Base.return_false_on_aborted_enqueue
     ActiveJob::Base.return_false_on_aborted_enqueue = true
-    assert_equal false, AbortBeforeEnqueueJob.new.enqueue
+
+    ActiveSupport::Deprecation.silence do
+      assert_equal false, AbortBeforeEnqueueJob.new.enqueue
+    end
   ensure
     ActiveJob::Base.return_false_on_aborted_enqueue = prev
   end
@@ -48,7 +51,9 @@ class CallbacksTest < ActiveSupport::TestCase
     ActiveJob::Base.skip_after_callbacks_if_terminated = true
     reload_job
     job = AbortBeforeEnqueueJob.new
-    job.enqueue
+    ActiveSupport::Deprecation.silence do
+      job.enqueue
+    end
 
     assert_nil(job.flag)
   ensure
@@ -60,13 +65,31 @@ class CallbacksTest < ActiveSupport::TestCase
     ActiveJob::Base.skip_after_callbacks_if_terminated = false
     reload_job
     job = AbortBeforeEnqueueJob.new
-    job.enqueue
+    assert_deprecated(/`after_enqueue` callbacks will no longer run/) do
+      job.enqueue
+    end
 
     assert_equal("after_enqueue", job.flag)
   ensure
     ActiveJob::Base.skip_after_callbacks_if_terminated = prev
   end
 
+  test "#enqueue does not throw a deprecation warning when skip_after_callbacks_if_terminated_is false but job has no after callbacks" do
+    prev = ActiveJob::Base.skip_after_callbacks_if_terminated
+    ActiveJob::Base.skip_after_callbacks_if_terminated = false
+
+    job = Class.new(ActiveJob::Base) do
+      before_enqueue { throw(:abort) }
+      self.return_false_on_aborted_enqueue = true
+    end.new
+
+    assert_not_deprecated do
+      job.enqueue
+    end
+  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
@@ -84,13 +107,30 @@ class CallbacksTest < ActiveSupport::TestCase
     ActiveJob::Base.skip_after_callbacks_if_terminated = false
     reload_job
     job = AbortBeforeEnqueueJob.new
-    job.perform_now
+    assert_deprecated(/`after_perform` callbacks will no longer run/) do
+      job.perform_now
+    end
 
     assert_equal("after_perform", job.flag)
   ensure
     ActiveJob::Base.skip_after_callbacks_if_terminated = prev
   end
 
+  test "#perform does not throw a deprecation warning when skip_after_callbacks_if_terminated_is false but job has no after callbacks" do
+    prev = ActiveJob::Base.skip_after_callbacks_if_terminated
+    ActiveJob::Base.skip_after_callbacks_if_terminated = false
+
+    job = Class.new(ActiveJob::Base) do
+      before_perform { throw(:abort) }
+    end
+
+    assert_not_deprecated do
+      job.perform_now
+    end
+  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

activejob/test/helper.rb

@@ -12,6 +12,7 @@
   require "support/integration/helper"
 else
   ActiveJob::Base.logger = Logger.new(nil)
+  ActiveJob::Base.skip_after_callbacks_if_terminated = true
   require "adapters/#{@adapter}"
 end