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

Fix job_class evaluation bug #47

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/sidekiq-status/client_middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def initialize(opts = {})
def call(worker_class, msg, queue, redis_pool=nil)

# Determine the actual job class
klass = msg["args"][0]["job_class"] || worker_class rescue worker_class
klass = (!msg["args"][0].is_a?(String) && msg["args"][0]["job_class"]) || worker_class rescue worker_class
job_class = if klass.is_a?(Class)
klass
elsif Module.const_defined?(klass)
Expand Down
2 changes: 1 addition & 1 deletion lib/sidekiq-status/server_middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def call(worker, msg, queue)
expiry = @expiration

# Determine the actual job class
klass = msg["args"][0]["job_class"] || msg["class"] rescue msg["class"]
klass = (!msg["args"][0].is_a?(String) && msg["args"][0]["job_class"]) || msg["class"] rescue msg["class"]
job_class = klass.is_a?(Class) ? klass : Module.const_get(klass)

# Bypass unless this is a Sidekiq::Status::Worker job
Expand Down
8 changes: 8 additions & 0 deletions spec/lib/sidekiq-status/client_middleware_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@
end
end

context "when first argument is a string containing substring 'job_class'" do
it "uses the constantized class name" do
expect(StubJob.perform_async 'a string with job_class inside').to eq(job_id)
expect(redis.hget("sidekiq:status:#{job_id}", :status)).to eq('queued')
expect(Sidekiq::Status::queued?(job_id)).to be_truthy
expect(Sidekiq::Status::get_all(job_id)).to include('worker' => 'StubJob')
end
end
end

describe "with :expiration parameter" do
Expand Down
13 changes: 13 additions & 0 deletions spec/lib/sidekiq-status/server_middleware_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,19 @@
expect(Sidekiq::Status::failed?(job_id)).to be_truthy
end

context "when first argument is a string containing substring 'job_class'" do
it "uses the default class name" do
allow(SecureRandom).to receive(:hex).once.and_return(job_id)
start_server do
expect(capture_status_updates(3) {
expect(ConfirmationJob.perform_async 'a string with job_class inside').to eq(job_id)
}).to eq([job_id]*3)
end
expect(redis.hget("sidekiq:status:#{job_id}", :status)).to eq('complete')
expect(Sidekiq::Status::get_all(job_id)).to include('worker' => 'ConfirmationJob')
end
end

context "when Sidekiq::Status::Worker is not included in the job" do
it "should not set a failed status" do
allow(SecureRandom).to receive(:hex).once.and_return(job_id)
Expand Down