Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support ruby 3 and kwargs #1158

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
6 changes: 3 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ jobs:
strategy:
fail-fast: false
matrix:
ruby: [2.5, 2.6, 2.7, jruby, jruby-head, ruby-head]
ruby: [2.5, 2.6, 2.7, 3.0, jruby, jruby-head, ruby-head]
rails_version:
- '5.2.0'
- '5.2.5'
- '6.0.0'
- '6.1.0.rc2'
- '6.1.0'
- 'edge'
include:
#
Expand Down
6 changes: 3 additions & 3 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ platforms :jruby do
elsif ENV['RAILS_VERSION']
gem 'railties', "~> #{ENV['RAILS_VERSION']}"
else
gem 'railties', ['>= 3.0', '< 6.2']
gem 'railties', ['>= 3.0', '< 7.1']
end
end

Expand All @@ -43,8 +43,8 @@ group :test do
gem 'actionmailer', "~> #{ENV['RAILS_VERSION']}"
gem 'activerecord', "~> #{ENV['RAILS_VERSION']}"
else
gem 'actionmailer', ['>= 3.0', '< 6.2']
gem 'activerecord', ['>= 3.0', '< 6.2']
gem 'actionmailer', ['>= 3.0', '< 7.1']
gem 'activerecord', ['>= 3.0', '< 7.1']
end

gem 'rspec', '>= 3'
Expand Down
2 changes: 1 addition & 1 deletion delayed_job.gemspec
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# -*- encoding: utf-8 -*-

Gem::Specification.new do |spec|
spec.add_dependency 'activesupport', ['>= 3.0', '< 6.2']
spec.add_dependency 'activesupport', ['>= 3.0', '< 7.1']
spec.authors = ['Brandon Keepers', 'Brian Ryckbost', 'Chris Gaffney', 'David Genord II', 'Erik Michaels-Ober', 'Matt Griffin', 'Steve Richert', 'Tobias Lütke']
spec.description = 'Delayed_job (or DJ) encapsulates the common pattern of asynchronously executing longer tasks in the background. It is a direct extraction from Shopify where the job table is responsible for a multitude of core tasks.'
spec.email = ['brian@collectiveidea.com']
Expand Down
4 changes: 2 additions & 2 deletions lib/delayed/message_sending.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/delayed/performable_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 34 additions & 9 deletions lib/delayed/performable_method.rb
Original file line number Diff line number Diff line change
@@ -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?
Expand All @@ -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

Expand All @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion lib/delayed/psych_ext.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ def encode_with(coder)
coder.map = {
'object' => object,
'method_name' => method_name,
'args' => args
'args' => args,
'kwargs' => kwargs
}
end
end
Expand Down
16 changes: 15 additions & 1 deletion spec/message_sending_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
112 changes: 112 additions & 0 deletions spec/performable_method_spec.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'helper'
require 'action_controller/metal/strong_parameters' if ActionPack::VERSION::MAJOR >= 5

describe Delayed::PerformableMethod do
describe 'perform' do
Expand All @@ -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, [])
Expand Down