Merge pull request #38267 from kamipo/fix_kwargs_warning_in_activejob

Fix keyword arguments warnings in Active Job

actionmailer/lib/action_mailer/base.rb

@@ -611,6 +611,7 @@ def method_missing(method_name, *args)
           super
         end
       end
+      ruby2_keywords(:method_missing) if respond_to?(:ruby2_keywords, true)
 
       def respond_to_missing?(method, include_all = false)
         action_methods.include?(method.to_s) || super

actionmailer/lib/action_mailer/delivery_job.rb

@@ -23,6 +23,7 @@ class DeliveryJob < ActiveJob::Base # :nodoc:
     def perform(mailer, mail_method, delivery_method, *args) #:nodoc:
       mailer.constantize.public_send(mail_method, *args).send(delivery_method)
     end
+    ruby2_keywords(:perform) if respond_to?(:ruby2_keywords, true)
 
     private
       # "Deserialize" the mailer class name by hand in case another argument

actionmailer/lib/action_mailer/mail_delivery_job.rb

@@ -13,9 +13,14 @@ class MailDeliveryJob < ActiveJob::Base # :nodoc:
 
     rescue_from StandardError, with: :handle_exception_with_mailer_class
 
-    def perform(mailer, mail_method, delivery_method, args:, params: nil) #:nodoc:
+    def perform(mailer, mail_method, delivery_method, args:, kwargs: nil, params: nil)
       mailer_class = params ? mailer.constantize.with(params) : mailer.constantize
-      mailer_class.public_send(mail_method, *args).send(delivery_method)
+      message = if kwargs
+        mailer_class.public_send(mail_method, *args, **kwargs)
+      else
+        mailer_class.public_send(mail_method, *args)
+      end
+      message.send(delivery_method)
     end
 
     private

actionmailer/lib/action_mailer/message_delivery.rb

@@ -23,6 +23,7 @@ def initialize(mailer_class, action, *args) #:nodoc:
       @processed_mailer = nil
       @mail_message = nil
     end
+    ruby2_keywords(:initialize) if respond_to?(:ruby2_keywords, true)
 
     # Method calls are delegated to the Mail::Message that's ready to deliver.
     def __getobj__ #:nodoc:
@@ -136,16 +137,14 @@ def enqueue_delivery(delivery_method, options = {})
             "method*, or 3. use a custom Active Job instead of #deliver_later."
         else
           job = @mailer_class.delivery_job
-          args = arguments_for(job, delivery_method)
-          job.set(options).perform_later(*args)
-        end
-      end
 
-      def arguments_for(delivery_job, delivery_method)
-        if delivery_job <= MailDeliveryJob
-          [@mailer_class.name, @action.to_s, delivery_method.to_s, args: @args]
-        else
-          [@mailer_class.name, @action.to_s, delivery_method.to_s, *@args]
+          if job <= MailDeliveryJob
+            job.set(options).perform_later(
+              @mailer_class.name, @action.to_s, delivery_method.to_s, args: @args)
+          else
+            job.set(options).perform_later(
+              @mailer_class.name, @action.to_s, delivery_method.to_s, *@args)
+          end
         end
       end
   end

actionmailer/lib/action_mailer/parameterized.rb

@@ -115,6 +115,7 @@ def method_missing(method_name, *args)
             super
           end
         end
+        ruby2_keywords(:method_missing) if respond_to?(:ruby2_keywords, true)
 
         def respond_to_missing?(method, include_all = false)
           @mailer.respond_to?(method, include_all)
@@ -125,13 +126,15 @@ class DeliveryJob < ActionMailer::DeliveryJob # :nodoc:
       def perform(mailer, mail_method, delivery_method, params, *args)
         mailer.constantize.with(params).public_send(mail_method, *args).send(delivery_method)
       end
+      ruby2_keywords(:perform) if respond_to?(:ruby2_keywords, true)
     end
 
     class MessageDelivery < ActionMailer::MessageDelivery # :nodoc:
       def initialize(mailer_class, action, params, *args)
         super(mailer_class, action, *args)
         @params = params
       end
+      ruby2_keywords(:initialize) if respond_to?(:ruby2_keywords, true)
 
       private
         def processed_mailer
@@ -145,9 +148,15 @@ def enqueue_delivery(delivery_method, options = {})
           if processed?
             super
           else
-            job  = delivery_job_class
-            args = arguments_for(job, delivery_method)
-            job.set(options).perform_later(*args)
+            job = delivery_job_class
+
+            if job <= MailDeliveryJob
+              job.set(options).perform_later(
+                @mailer_class.name, @action.to_s, delivery_method.to_s, params: @params, args: @args)
+            else
+              job.set(options).perform_later(
+                @mailer_class.name, @action.to_s, delivery_method.to_s, @params, *@args)
+            end
           end
         end
 
@@ -158,14 +167,6 @@ def delivery_job_class
             Parameterized::DeliveryJob
           end
         end
-
-        def arguments_for(delivery_job, delivery_method)
-          if delivery_job <= MailDeliveryJob
-            [@mailer_class.name, @action.to_s, delivery_method.to_s, params: @params, args: @args]
-          else
-            [@mailer_class.name, @action.to_s, delivery_method.to_s, @params, *@args]
-          end
-        end
     end
   end
 end

actionmailer/test/mailers/delayed_mailer.rb

@@ -22,6 +22,10 @@ def test_message(*)
     mail(from: "test-sender@test.com", to: "test-receiver@test.com", subject: "Test Subject", body: "Test Body")
   end
 
+  def test_kwargs(argument:)
+    mail(from: "test-sender@test.com", to: "test-receiver@test.com", subject: "Test Subject", body: "Test Body")
+  end
+
   def test_raise(klass_name)
     raise klass_name.constantize, "boom"
   end

actionmailer/test/message_delivery_test.rb

@@ -164,4 +164,11 @@ def to_global_id(options = {})
     assert_equal DelayedMailer, DelayedMailer.last_rescue_from_instance
     assert_equal "Error while trying to deserialize arguments: boom, missing find", DelayedMailer.last_error.message
   end
+
+  test "allows for keyword arguments" do
+    assert_performed_with(job: ActionMailer::MailDeliveryJob, args: ["DelayedMailer", "test_kwargs", "deliver_now", args: [argument: 1]]) do
+      message = DelayedMailer.test_kwargs(argument: 1)
+      message.deliver_later
+    end
+  end
 end

activejob/lib/active_job/arguments.rb

@@ -52,6 +52,8 @@ def deserialize(arguments)
       # :nodoc:
       SYMBOL_KEYS_KEY = "_aj_symbol_keys"
       # :nodoc:
+      RUBY2_KEYWORDS_KEY = "_aj_ruby2_keywords"
+      # :nodoc:
       WITH_INDIFFERENT_ACCESS_KEY = "_aj_hash_with_indifferent_access"
       # :nodoc:
       OBJECT_SERIALIZER_KEY = "_aj_serialized"
@@ -60,10 +62,39 @@ def deserialize(arguments)
       RESERVED_KEYS = [
         GLOBALID_KEY, GLOBALID_KEY.to_sym,
         SYMBOL_KEYS_KEY, SYMBOL_KEYS_KEY.to_sym,
+        RUBY2_KEYWORDS_KEY, RUBY2_KEYWORDS_KEY.to_sym,
         OBJECT_SERIALIZER_KEY, OBJECT_SERIALIZER_KEY.to_sym,
         WITH_INDIFFERENT_ACCESS_KEY, WITH_INDIFFERENT_ACCESS_KEY.to_sym,
       ]
-      private_constant :PERMITTED_TYPES, :RESERVED_KEYS, :GLOBALID_KEY, :SYMBOL_KEYS_KEY, :WITH_INDIFFERENT_ACCESS_KEY
+      private_constant :PERMITTED_TYPES, :RESERVED_KEYS, :GLOBALID_KEY,
+        :SYMBOL_KEYS_KEY, :RUBY2_KEYWORDS_KEY, :WITH_INDIFFERENT_ACCESS_KEY
+
+      unless Hash.respond_to?(:ruby2_keywords_hash?) && Hash.respond_to?(:ruby2_keywords_hash)
+        using Module.new {
+          refine Hash do
+            class << Hash
+              if RUBY_VERSION >= "2.7"
+                def ruby2_keywords_hash?(hash)
+                  !new(*[hash]).default.equal?(hash)
+                end
+              else
+                def ruby2_keywords_hash?(hash)
+                  false
+                end
+              end
+
+              def ruby2_keywords_hash(hash)
+                _ruby2_keywords_hash(**hash)
+              end
+
+              private def _ruby2_keywords_hash(*args)
+                args.last
+              end
+              ruby2_keywords(:_ruby2_keywords_hash) if respond_to?(:ruby2_keywords, true)
+            end
+          end
+        }
+      end
 
       def serialize_argument(argument)
         case argument
@@ -76,9 +107,14 @@ def serialize_argument(argument)
         when ActiveSupport::HashWithIndifferentAccess
           serialize_indifferent_hash(argument)
         when Hash
-          symbol_keys = argument.each_key.grep(Symbol).map(&:to_s)
+          symbol_keys = argument.each_key.grep(Symbol).map!(&:to_s)
+          aj_hash_key = if Hash.ruby2_keywords_hash?(argument)
+            RUBY2_KEYWORDS_KEY
+          else
+            SYMBOL_KEYS_KEY
+          end
           result = serialize_hash(argument)
-          result[SYMBOL_KEYS_KEY] = symbol_keys
+          result[aj_hash_key] = symbol_keys
           result
         when -> (arg) { arg.respond_to?(:permitted?) }
           serialize_indifferent_hash(argument.to_h)
@@ -132,6 +168,9 @@ def deserialize_hash(serialized_hash)
           result = result.with_indifferent_access
         elsif symbol_keys = result.delete(SYMBOL_KEYS_KEY)
           result = transform_symbol_keys(result, symbol_keys)
+        elsif symbol_keys = result.delete(RUBY2_KEYWORDS_KEY)
+          result = transform_symbol_keys(result, symbol_keys)
+          result = Hash.ruby2_keywords_hash(result)
         end
         result
       end

activejob/lib/active_job/configured_job.rb

@@ -10,9 +10,11 @@ def initialize(job_class, options = {})
     def perform_now(*args)
       @job_class.new(*args).perform_now
     end
+    ruby2_keywords(:perform_now) if respond_to?(:ruby2_keywords, true)
 
     def perform_later(*args)
       @job_class.new(*args).enqueue @options
     end
+    ruby2_keywords(:perform_later) if respond_to?(:ruby2_keywords, true)
   end
 end

activejob/lib/active_job/core.rb

@@ -86,6 +86,7 @@ def initialize(*arguments)
       @executions = 0
       @exception_executions = {}
     end
+    ruby2_keywords(:initialize) if respond_to?(:ruby2_keywords, true)
 
     # Returns a hash with the job data that can safely be passed to the
     # queuing adapter.

activejob/lib/active_job/enqueuing.rb

@@ -21,11 +21,13 @@ module ClassMethods
       def perform_later(*args)
         job_or_instantiate(*args).enqueue
       end
+      ruby2_keywords(:perform_later) if respond_to?(:ruby2_keywords, true)
 
       private
         def job_or_instantiate(*args) # :doc:
           args.first.is_a?(self) ? args.first : new(*args)
         end
+        ruby2_keywords(:job_or_instantiate) if respond_to?(:ruby2_keywords, true)
     end
 
     # Enqueues the job to be performed by the queue adapter.

activejob/lib/active_job/execution.rb

@@ -17,6 +17,7 @@ module ClassMethods
       def perform_now(*args)
         job_or_instantiate(*args).perform_now
       end
+      ruby2_keywords(:perform_now) if respond_to?(:ruby2_keywords, true)
 
       def execute(job_data) #:nodoc:
         ActiveJob::Callbacks.run_callbacks(:execute) do

activejob/test/cases/argument_serialization_test.rb

@@ -161,7 +161,7 @@ class ArgumentSerializationTest < ActiveSupport::TestCase
   end
 
   test "allows for keyword arguments" do
-    KwargsJob.perform_later(argument: 2)
+    KwargsJob.perform_now(argument: 2)
 
     assert_equal "Job with argument: 2", JobBuffer.last_value
   end