collectiveidea / delayed_job

Database based asynchronous priority queue system -- Extracted from Shopify
http://groups.google.com/group/delayed_job
MIT License
4.81k stars 955 forks source link

Support for Ruby 3.0 added #1130

Open webhoernchen opened 3 years ago

webhoernchen commented 3 years ago

Some methods are not worked with Ruby 3.0

jdelStrother commented 3 years ago

@albus522 -

This fails if the only argument is intentionally a hash, for example sending partial request params to a job.

I started looking into this - think we'd want to capture the PerformableMethod's args and kwargs separately (see below). It's going to be painful to continue supporting ruby 2.6 for this, though - how would you feel about only supporting 2.7+ for the next release?

diff --git a/lib/delayed/message_sending.rb b/lib/delayed/message_sending.rb
index a2808fd..34cc4f6 100644
--- a/lib/delayed/message_sending.rb
+++ b/lib/delayed/message_sending.rb
@@ -7,8 +7,8 @@ module Delayed
     end

     # rubocop:disable MethodMissing
-    def method_missing(method, *args)
-      Job.enqueue({:payload_object => @payload_class.new(@target, method.to_sym, args)}.merge(@options))
+    def method_missing(method, *args, **kwargs)
+      Job.enqueue({:payload_object => @payload_class.new(@target, method.to_sym, args, kwargs)}.merge(@options))
     end
     # rubocop:enable MethodMissing
   end
diff --git a/lib/delayed/performable_method.rb b/lib/delayed/performable_method.rb
index 9f64a61..a8765f6 100644
--- a/lib/delayed/performable_method.rb
+++ b/lib/delayed/performable_method.rb
@@ -1,8 +1,8 @@
 module Delayed
   class PerformableMethod
-    attr_accessor :object, :method_name, :args
+    attr_accessor :object, :method_name, :args, :kwargs

-    def initialize(object, method_name, args)
+    def initialize(object, method_name, args, kwargs = {})
       raise NoMethodError, "undefined method `#{method_name}' for #{object.inspect}" unless object.respond_to?(method_name, true)

       if object.respond_to?(:persisted?) && !object.persisted?
@@ -11,6 +11,7 @@ module Delayed

       self.object       = object
       self.args         = args
+      self.kwargs       = kwargs
       self.method_name  = method_name.to_sym
     end

@@ -26,19 +27,9 @@ module Delayed
     # Otherwise the following error is thrown
     # ArgumentError:
     #   wrong number of arguments (given 1, expected 0; required keywords:
     if RUBY_VERSION >= '3.0'
       def perform
-        return unless object
-
-        if args_is_a_hash?
-          object.send(method_name, **args.first)
-        else
-          object.send(method_name, *args)
-        end
-      end
-
-      def args_is_a_hash?
-        args.size == 1 && args.first.is_a?(Hash)
+        object.send(method_name, *args, **kwargs) if object
       end
     else
       def perform
diff --git a/spec/message_sending_spec.rb b/spec/message_sending_spec.rb
index ca58edc..281f8b2 100644
--- a/spec/message_sending_spec.rb
+++ b/spec/message_sending_spec.rb
@@ -64,12 +64,17 @@ describe Delayed::MessageSending do

   context 'delay' do
     class FairyTail
-      attr_accessor :happy_ending
+      attr_accessor :happy_ending, :ogre, :dead
       def self.princesses; end

       def tell
         @happy_ending = true
       end
+
+      def defeat(ogre_params, dead: true)
+        @ogre = ogre_params
+        @dead = dead
+      end
     end

     after do
@@ -143,5 +148,14 @@ describe Delayed::MessageSending do
         end.to change(fairy_tail, :happy_ending).from(nil).to(true)
       end.not_to(change { Delayed::Job.count })
     end
+
+    it 'can handle a mix of params and kwargs' do
+      Delayed::Worker.delay_jobs = false
+      fairy_tail = FairyTail.new
+      expect do
+        fairy_tail.delay.defeat({:name => 'shrek'}, :dead => false)
+      end.to change(fairy_tail, :ogre).from(nil).to(:name => 'shrek').
+        and(change(fairy_tail, :dead).from(nil).to(false))
+    end
   end
 end
diff --git a/spec/performable_method_spec.rb b/spec/performable_method_spec.rb
index 17179c6..c5d3e78 100644
--- a/spec/performable_method_spec.rb
+++ b/spec/performable_method_spec.rb
@@ -34,6 +34,17 @@ describe Delayed::PerformableMethod do
     end
   end

+  describe 'perform with hash object and kwargs' do
+    before do
+      @method = Delayed::PerformableMethod.new('foo', :count, [{:o => true}], :o2 => false)
+    end
+
+    it 'calls the method on the object' do
+      expect(@method.object).to receive(:count).with({:o => true}, :o2 => false)
+      @method.perform
+    end
+  end
+
   describe 'perform with many hash objects' do
     before do
       @method = Delayed::PerformableMethod.new('foo', :count, [{:o => true}, {:o2 => true}])
@@ -110,7 +121,7 @@ describe Delayed::PerformableMethod do
         end
       end

-      @method = Delayed::PerformableMethod.new(klass.new, :test_method, [{:name => 'name', :any => 'any'}])
+      @method = Delayed::PerformableMethod.new(klass.new, :test_method, [], :name => 'name', :any => 'any')
     end

     it 'calls the method on the object' do
jacobjlevine commented 2 years ago

Hi folks. Any updates on this? As far as I can tell, it appears Delayed Job still doesn't support keyword args for background jobs in Ruby 3.

infoman commented 2 years ago

Does not work for me on Ruby 3.0.2 and Rails 6.0.4:

Something.some_method(arg, other_arg: value)
Something.delay.some_method(arg, other_arg: value)
jdelStrother commented 2 years ago

I've built on this here - https://github.com/collectiveidea/delayed_job/pull/1158 - which has been working well in production for us.