Skip to content

Commit

Permalink
Handle kwargs separately for true Ruby 3.0 support
Browse files Browse the repository at this point in the history
  • Loading branch information
smudge committed Nov 29, 2021
1 parent 77c5061 commit 459cea1
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 23 deletions.
8 changes: 4 additions & 4 deletions lib/delayed/message_sending.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ def initialize(payload_class, target, options)
@options = options
end

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
end

Expand All @@ -36,7 +36,7 @@ def handle_asynchronously(method, opts = {}) # rubocop:disable Metrics/Perceived
punctuation = $1 # rubocop:disable Style/PerlBackrefs
with_method = "#{aliased_method}_with_delay#{punctuation}"
without_method = "#{aliased_method}_without_delay#{punctuation}"
define_method(with_method) do |*args|
define_method(with_method) do |*args, **kwargs|
curr_opts = opts.clone
curr_opts.each_key do |key|
next unless (val = curr_opts[key]).is_a?(Proc)
Expand All @@ -47,7 +47,7 @@ def handle_asynchronously(method, opts = {}) # rubocop:disable Metrics/Perceived
val.call
end
end
delay(curr_opts).__send__(without_method, *args)
delay(curr_opts).__send__(without_method, *args, **kwargs)
end

alias_method without_method, method
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
13 changes: 10 additions & 3 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 !her_model?(object) && 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 @@ -23,7 +24,13 @@ def display_name
end

def perform
object.send(method_name, *args) if object
return unless object

if kwargs.nil? # TODO: Remove this branch in the next major release. (It will be 'nil' for jobs enqueued before we split out kwargs.)
object.send(method_name, *args)
else
object.send(method_name, *args, **kwargs)
end
end

def method(sym)
Expand Down
2 changes: 1 addition & 1 deletion spec/delayed/job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ def create_job(opts = {})
context 'large handler' do
before do
text = 'Lorem ipsum dolor sit amet. ' * 1000
@job = described_class.enqueue Delayed::PerformableMethod.new(text, :length, {})
@job = described_class.enqueue Delayed::PerformableMethod.new(text, :length, [], {})
end

it 'has an id' do
Expand Down
6 changes: 4 additions & 2 deletions spec/message_sending_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ def tell!(_arg, _kwarg:); end
job = obj.tell!('a', kwarg: 'b')
expect(job.payload_object.class).to eq(Delayed::PerformableMethod)
expect(job.payload_object.method_name).to eq(:tell_without_delay!)
expect(job.payload_object.args).to eq(['a', { kwarg: 'b' }])
expect(job.payload_object.args).to eq(['a'])
expect(job.payload_object.kwargs).to eq(kwarg: 'b')
}.to change { Delayed::Job.count }.by(1)
end

Expand Down Expand Up @@ -91,7 +92,8 @@ def tell(arg, kwarg:)
job = FairyTail.new.delay.tell('arg', kwarg: 'kwarg')
expect(job.payload_object.class).to eq(Delayed::PerformableMethod)
expect(job.payload_object.method_name).to eq(:tell)
expect(job.payload_object.args).to eq(['arg', { kwarg: 'kwarg' }])
expect(job.payload_object.args).to eq(['arg'])
expect(job.payload_object.kwargs).to eq(kwarg: 'kwarg')
}.to change { Delayed::Job.count }.by(1)
end

Expand Down
8 changes: 5 additions & 3 deletions spec/performable_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def signup(email, beta_tester: false)
mailer = MyMailer.new
email = double('email', deliver: true)
allow(mailer).to receive(:mail).and_return(email)
mailer_job = described_class.new(mailer, :signup, ['john@example.com'])
mailer_job = described_class.new(mailer, :signup, ['john@example.com'], {})

expect(email).to receive(:deliver)
mailer_job.perform
Expand All @@ -34,7 +34,8 @@ def signup(email, beta_tester: false)
job = MyMailer.delay.signup('john@example.com', beta_tester: true)
expect(job.payload_object.class).to eq(Delayed::PerformableMailer)
expect(job.payload_object.method_name).to eq(:signup)
expect(job.payload_object.args).to eq(['john@example.com', { beta_tester: true }])
expect(job.payload_object.args).to eq(['john@example.com'])
expect(job.payload_object.kwargs).to eq(beta_tester: true)
}.to change { Delayed::Job.count }.by(1)
end
end
Expand All @@ -53,13 +54,14 @@ def signup(email, beta_tester: false)
describe 'delay' do
it 'enqueues a PerformableEmail job' do
expect {
job = MyMailer.with(foo: 1, bar: 2).delay.signup('john@example.com')
job = MyMailer.with(foo: 1, bar: 2).delay.signup('john@example.com', beta_tester: false)
expect(job.payload_object.class).to eq(Delayed::PerformableMailer)
expect(job.payload_object.object.class).to eq(described_class)
expect(job.payload_object.object.instance_variable_get('@mailer')).to eq(MyMailer)
expect(job.payload_object.object.instance_variable_get('@params')).to eq(foo: 1, bar: 2)
expect(job.payload_object.method_name).to eq(:signup)
expect(job.payload_object.args).to eq(['john@example.com'])
expect(job.payload_object.kwargs).to eq(beta_tester: false)
}.to change { Delayed::Job.count }.by(1)
end
end
Expand Down
18 changes: 9 additions & 9 deletions spec/performable_method_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def foo(arg, kwarg:)
end

before do
@method = described_class.new(test_class.new, :foo, ['a', { kwarg: 'b' }])
@method = described_class.new(test_class.new, :foo, ['a'], { kwarg: 'b' })
end

context 'with the persisted record cannot be found' do
Expand All @@ -35,7 +35,7 @@ def foo(arg, kwarg:)

it "raises a NoMethodError if target method doesn't exist" do
expect {
described_class.new(Object, :method_that_does_not_exist, [])
described_class.new(Object, :method_that_does_not_exist, [], {})
}.to raise_error(NoMethodError)
end

Expand All @@ -44,29 +44,29 @@ def foo(arg, kwarg:)
def private_method; end
private :private_method
end
expect { described_class.new(clazz.new, :private_method, []) }.not_to raise_error
expect { described_class.new(clazz.new, :private_method, [], {}) }.not_to raise_error
end

context 'when it receives an object that is not persisted' do
let(:object) { double(persisted?: false, expensive_operation: true) }

it 'raises an ArgumentError' do
expect { described_class.new(object, :expensive_operation, []) }.to raise_error ArgumentError
expect { described_class.new(object, :expensive_operation, [], {}) }.to raise_error ArgumentError
end

it 'does not raise ArgumentError if the object acts like a Her model' do
allow(object.class).to receive(:save_existing).and_return(true)
expect { described_class.new(object, :expensive_operation, []) }.not_to raise_error
expect { described_class.new(object, :expensive_operation, [], {}) }.not_to raise_error
end
end

describe 'display_name' do
it 'returns class_name#method_name for instance methods' do
expect(described_class.new('foo', :count, ['o']).display_name).to eq('String#count')
expect(described_class.new('foo', :count, ['o'], {}).display_name).to eq('String#count')
end

it 'returns class_name.method_name for class methods' do
expect(described_class.new(Class, :inspect, []).display_name).to eq('Class.inspect')
expect(described_class.new(Class, :inspect, [], {}).display_name).to eq('Class.inspect')
end
end

Expand Down Expand Up @@ -95,7 +95,7 @@ def private_method; end
end

it 'delegates failure hook to object' do
method = described_class.new('object', :size, [])
method = described_class.new('object', :size, [], {})
expect(method.object).to receive(:failure)
method.failure
end
Expand Down Expand Up @@ -125,7 +125,7 @@ def private_method; end
end

it 'delegates failure hook to object' do
method = described_class.new('object', :size, [])
method = described_class.new('object', :size, [], {})
expect(method.object).to receive(:failure)
method.failure
end
Expand Down

0 comments on commit 459cea1

Please sign in to comment.