diff --git a/lib/delayed/message_sending.rb b/lib/delayed/message_sending.rb index a2808fd39..34cc4f6ee 100644 --- a/lib/delayed/message_sending.rb +++ b/lib/delayed/message_sending.rb @@ -7,8 +7,8 @@ def initialize(payload_class, target, options) 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_mailer.rb b/lib/delayed/performable_mailer.rb index 8535c452d..7c5d6d7b0 100644 --- a/lib/delayed/performable_mailer.rb +++ b/lib/delayed/performable_mailer.rb @@ -3,7 +3,7 @@ module Delayed class PerformableMailer < PerformableMethod def perform - mailer = object.send(method_name, *args) + mailer = super mailer.respond_to?(:deliver_now) ? mailer.deliver_now : mailer.deliver end end diff --git a/lib/delayed/performable_method.rb b/lib/delayed/performable_method.rb index 96a28b056..0fbb4f038 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 @@ def initialize(object, method_name, args) self.object = object self.args = args + self.kwargs = kwargs self.method_name = method_name.to_sym end @@ -22,18 +23,42 @@ def display_name end end - def perform - object.send(method_name, *args) if object + def kwargs + # Default to a hash so that we can handle deserializing jobs that were + # created before kwargs was available. + @kwargs || {} end - def method(sym) - object.method(sym) + # In ruby 3 we need to explicitly separate regular args from the keyword-args. + if RUBY_VERSION >= '3.0' + def perform + object.send(method_name, *args, **kwargs) if object + end + else + # On ruby 2, rely on the implicit conversion from a hash to kwargs + def perform + return unless object + if kwargs.present? + object.send(method_name, *args, kwargs) + else + object.send(method_name, *args) + end + end end - # rubocop:disable MethodMissing - def method_missing(symbol, *args) - object.send(symbol, *args) + def method(sym) + object.method(sym) end + method_def = [] + location = caller_locations(1, 1).first + file = location.path + line = location.lineno + definition = RUBY_VERSION >= '2.7' ? '...' : '*args, &block' + method_def << + "def method_missing(#{definition})" \ + " object.send(#{definition})" \ + 'end' + module_eval(method_def.join(';'), file, line) # rubocop:enable MethodMissing def respond_to?(symbol, include_private = false) diff --git a/lib/delayed/psych_ext.rb b/lib/delayed/psych_ext.rb index 00350a453..049a18d12 100644 --- a/lib/delayed/psych_ext.rb +++ b/lib/delayed/psych_ext.rb @@ -5,7 +5,8 @@ def encode_with(coder) coder.map = { 'object' => object, 'method_name' => method_name, - 'args' => args + 'args' => args, + 'kwargs' => kwargs } end end diff --git a/spec/message_sending_spec.rb b/spec/message_sending_spec.rb index ca58edcc1..281f8b287 100644 --- a/spec/message_sending_spec.rb +++ b/spec/message_sending_spec.rb @@ -64,12 +64,17 @@ def spin; end 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 @@ def tell 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 a4d700322..c5d3e7803 100644 --- a/spec/performable_method_spec.rb +++ b/spec/performable_method_spec.rb @@ -1,4 +1,5 @@ require 'helper' +require 'action_controller/metal/strong_parameters' if ActionPack::VERSION::MAJOR >= 5 describe Delayed::PerformableMethod do describe 'perform' do @@ -22,6 +23,117 @@ end end + describe 'perform with hash object' do + before do + @method = Delayed::PerformableMethod.new('foo', :count, [{:o => true}]) + end + + it 'calls the method on the object' do + expect(@method.object).to receive(:count).with(:o => true) + @method.perform + 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}]) + end + + it 'calls the method on the object' do + expect(@method.object).to receive(:count).with({:o => true}, :o2 => true) + @method.perform + end + end + + if ActionPack::VERSION::MAJOR >= 5 + describe 'perform with params object' do + before do + @params = ActionController::Parameters.new(:person => { + :name => 'Francesco', + :age => 22, + :role => 'admin' + }) + + @method = Delayed::PerformableMethod.new('foo', :count, [@params]) + end + + it 'calls the method on the object' do + expect(@method.object).to receive(:count).with(@params) + @method.perform + end + end + + describe 'perform with sample object and params object' do + before do + @params = ActionController::Parameters.new(:person => { + :name => 'Francesco', + :age => 22, + :role => 'admin' + }) + + klass = Class.new do + def test_method(_o1, _o2) + true + end + end + + @method = Delayed::PerformableMethod.new(klass.new, :test_method, ['o', @params]) + end + + it 'calls the method on the object' do + expect(@method.object).to receive(:test_method).with('o', @params) + @method.perform + end + + it 'calls the method on the object (real)' do + expect(@method.perform).to be true + end + end + end + + describe 'perform with sample object and hash object' do + before do + @method = Delayed::PerformableMethod.new('foo', :count, ['o', {:o => true}]) + end + + it 'calls the method on the object' do + expect(@method.object).to receive(:count).with('o', :o => true) + @method.perform + end + end + + describe 'perform with hash to named parameters' do + before do + klass = Class.new do + def test_method(name:, any:) + true if name && any + end + end + + @method = Delayed::PerformableMethod.new(klass.new, :test_method, [], :name => 'name', :any => 'any') + end + + it 'calls the method on the object' do + expect(@method.object).to receive(:test_method).with(:name => 'name', :any => 'any') + @method.perform + end + + it 'calls the method on the object (real)' do + expect(@method.perform).to be true + end + end + it "raises a NoMethodError if target method doesn't exist" do expect do Delayed::PerformableMethod.new(Object, :method_that_does_not_exist, [])