Skip to content

Commit ffedb2b

Browse files
authored
Fix ActiveJob keyword arguments support (#961) (#962)
ActiveJob jobs with keyword arguments were broken in Shoryuken 7.0 because the SQSSendMessageParametersSupport module's initialize method did not properly forward keyword arguments to the base class. Use argument forwarding (...) to ensure all arguments including keyword arguments are correctly passed through when prepending the module to ActiveJob::Base. Fixes #961
1 parent b0022cd commit ffedb2b

File tree

5 files changed

+149
-4
lines changed

5 files changed

+149
-4
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
## [Unreleased]
22

3+
- Fix: ActiveJob keyword arguments support
4+
- Jobs with keyword arguments were broken due to improper argument forwarding in `SQSSendMessageParametersSupport#initialize`
5+
- Use Ruby's argument forwarding (`...`) to properly pass all arguments including keyword arguments
6+
- [#962](https://github.com/ruby-shoryuken/shoryuken/pull/962)
7+
38
- Fix: Replace `ArgumentError` with custom `FifoDelayNotSupportedError` for FIFO delay errors
49
- Provides more specific error type for programmatic error handling
510
- [#957](https://github.com/ruby-shoryuken/shoryuken/pull/957)

lib/active_job/extensions.rb

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,10 @@ module SQSSendMessageParametersAccessor
2020
module SQSSendMessageParametersSupport
2121
# Initializes a new ActiveJob instance with empty SQS parameters
2222
#
23-
# @param arguments [Array] the job arguments
24-
def initialize(*arguments)
25-
super(*arguments)
23+
# Uses argument forwarding (...) to properly pass all arguments including
24+
# keyword arguments to the base class.
25+
def initialize(...)
26+
super(...)
2627
self.sqs_send_message_parameters = {}
2728
end
2829

lib/shoryuken/version.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,5 @@
22

33
module Shoryuken
44
# Current version of the Shoryuken gem
5-
VERSION = '7.0.0'
5+
VERSION = '7.0.1'
66
end
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
# frozen_string_literal: true
2+
3+
# Integration test for ActiveJob keyword arguments support
4+
# Regression test for: https://github.com/ruby-shoryuken/shoryuken/issues/961
5+
#
6+
# In Shoryuken 7.0, the SQSSendMessageParametersSupport module's initialize method
7+
# breaks keyword argument passing to ActiveJob jobs because it lacks ruby2_keywords.
8+
9+
setup_localstack
10+
setup_active_job
11+
12+
queue_name = DT.queue
13+
create_test_queue(queue_name)
14+
15+
# Job that accepts keyword arguments - this was broken in Shoryuken 7.0
16+
class KeywordArgumentsTestJob < ActiveJob::Base
17+
def perform(name:, count:, enabled: false)
18+
DT[:executions] << {
19+
name: name,
20+
count: count,
21+
enabled: enabled,
22+
job_id: job_id
23+
}
24+
end
25+
end
26+
27+
KeywordArgumentsTestJob.queue_as(queue_name)
28+
29+
Shoryuken.add_group('default', 1)
30+
Shoryuken.add_queue(queue_name, 1, 'default')
31+
Shoryuken.register_worker(queue_name, Shoryuken::ActiveJob::JobWrapper)
32+
33+
# Enqueue jobs with keyword arguments
34+
# This is where the bug manifests - the job instantiation fails
35+
KeywordArgumentsTestJob.perform_later(name: 'first', count: 1)
36+
KeywordArgumentsTestJob.perform_later(name: 'second', count: 2, enabled: true)
37+
38+
poll_queues_until(timeout: 30) do
39+
DT[:executions].size >= 2
40+
end
41+
42+
assert_equal(2, DT[:executions].size, "Expected 2 job executions, got #{DT[:executions].size}")
43+
44+
# Find the executions by name
45+
first_exec = DT[:executions].find { |e| e[:name] == 'first' }
46+
second_exec = DT[:executions].find { |e| e[:name] == 'second' }
47+
48+
assert(first_exec, "Expected to find 'first' job execution")
49+
assert(second_exec, "Expected to find 'second' job execution")
50+
51+
# Verify keyword arguments were passed correctly
52+
assert_equal('first', first_exec[:name])
53+
assert_equal(1, first_exec[:count])
54+
assert_equal(false, first_exec[:enabled])
55+
56+
assert_equal('second', second_exec[:name])
57+
assert_equal(2, second_exec[:count])
58+
assert_equal(true, second_exec[:enabled])
59+
60+
# Verify job IDs
61+
job_ids = DT[:executions].map { |e| e[:job_id] }
62+
assert(job_ids.all? { |id| id && !id.empty? }, "All jobs should have job IDs")
63+
assert_equal(2, job_ids.uniq.size, "All job IDs should be unique")

spec/lib/active_job/extensions_spec.rb

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# frozen_string_literal: true
22

33
require 'spec_helper'
4+
require 'active_job'
45

56
# Skip this spec if ActiveSupport is not available, as the extensions require it
67
if defined?(ActiveSupport)
@@ -69,6 +70,81 @@ def enqueue(options = {})
6970
expect(job_class.method(:new)).to respond_to(:ruby2_keywords) if RUBY_VERSION >= '2.7'
7071
end
7172
end
73+
74+
# Regression test for https://github.com/ruby-shoryuken/shoryuken/issues/961
75+
# ActiveJob with keyword arguments was broken in Shoryuken 7.0 because
76+
# the SQSSendMessageParametersSupport module's initialize method did not
77+
# properly forward keyword arguments using ruby2_keywords.
78+
context 'with keyword arguments' do
79+
let(:base_class_with_kwargs) do
80+
Class.new do
81+
attr_accessor :sqs_send_message_parameters
82+
attr_reader :received_name, :received_count, :received_enabled
83+
84+
def initialize(name:, count:, enabled: false)
85+
@received_name = name
86+
@received_count = count
87+
@received_enabled = enabled
88+
end
89+
end
90+
end
91+
92+
let(:job_class_with_kwargs) do
93+
Class.new(base_class_with_kwargs) do
94+
prepend Shoryuken::ActiveJob::SQSSendMessageParametersSupport
95+
end
96+
end
97+
98+
it 'properly forwards keyword arguments to the base class' do
99+
job = job_class_with_kwargs.new(name: 'test_name', count: 42, enabled: true)
100+
101+
expect(job.received_name).to eq('test_name')
102+
expect(job.received_count).to eq(42)
103+
expect(job.received_enabled).to eq(true)
104+
expect(job.sqs_send_message_parameters).to eq({})
105+
end
106+
107+
it 'properly forwards keyword arguments with default values' do
108+
job = job_class_with_kwargs.new(name: 'another_test', count: 10)
109+
110+
expect(job.received_name).to eq('another_test')
111+
expect(job.received_count).to eq(10)
112+
expect(job.received_enabled).to eq(false)
113+
expect(job.sqs_send_message_parameters).to eq({})
114+
end
115+
end
116+
117+
# Regression test for https://github.com/ruby-shoryuken/shoryuken/issues/961
118+
# Test with a mix of positional and keyword arguments
119+
context 'with mixed positional and keyword arguments' do
120+
let(:base_class_mixed) do
121+
Class.new do
122+
attr_accessor :sqs_send_message_parameters
123+
attr_reader :received_id, :received_data, :received_options
124+
125+
def initialize(id, data, options: {})
126+
@received_id = id
127+
@received_data = data
128+
@received_options = options
129+
end
130+
end
131+
end
132+
133+
let(:job_class_mixed) do
134+
Class.new(base_class_mixed) do
135+
prepend Shoryuken::ActiveJob::SQSSendMessageParametersSupport
136+
end
137+
end
138+
139+
it 'properly forwards mixed arguments to the base class' do
140+
job = job_class_mixed.new('job_id', { key: 'value' }, options: { retry: true })
141+
142+
expect(job.received_id).to eq('job_id')
143+
expect(job.received_data).to eq({ key: 'value' })
144+
expect(job.received_options).to eq({ retry: true })
145+
expect(job.sqs_send_message_parameters).to eq({})
146+
end
147+
end
72148
end
73149

74150
describe '#enqueue' do

0 commit comments

Comments
 (0)