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

Ensure job context does not leak into extensions whenperform_now is called within another job #1336

Merged
merged 1 commit into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/good_job/active_job_extensions/batches.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Batches
extend ActiveSupport::Concern

def batch
@_batch ||= CurrentThread.execution&.batch&.to_batch
@_batch ||= CurrentThread.execution&.batch&.to_batch if CurrentThread.execution.present? && CurrentThread.execution.active_job_id == job_id
end
alias batch? batch
end
Expand Down
2 changes: 1 addition & 1 deletion lib/good_job/active_job_extensions/concurrency.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def deserialize(job_data)
key = job.good_job_concurrency_key
next if key.blank?

if CurrentThread.execution.blank?
if CurrentThread.execution.blank? || CurrentThread.execution.active_job_id != job_id
logger.debug("Ignoring concurrency limits because the job is executed with `perform_now`.")
next
end
Expand Down
1 change: 1 addition & 0 deletions sorbet/rbi/todo.rbi
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ module ::TestJob::Error; end
module ::TestJob::ExpectedError; end
module ::TestJob::RunError; end
module ::TestJob::SuccessCallbackJob; end
module ::WrapperJob; end
module GoodJob::Job::ERROR_EVENT_INTERRUPTED; end
module GoodJob::Job::ERROR_EVENT_RETRIED; end
module Rails::Server; end
21 changes: 20 additions & 1 deletion spec/lib/good_job/active_job_extensions/batches_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
include GoodJob::ActiveJobExtensions::Batches

def perform
RESULTS << batch.properties[:some_property]
RESULTS << batch.properties[:some_property] if batch
end
end)
end
Expand All @@ -31,5 +31,24 @@ def perform

expect(RESULTS).to eq %w[Apple Apple]
end

it "does not leak batch into perform_now" do
stub_const("WrapperJob", Class.new(ActiveJob::Base) do
include GoodJob::ActiveJobExtensions::Batches

def perform
TestJob.perform_now
end
end)

batch = Rails.application.executor.wrap do
GoodJob::Batch.enqueue(some_property: "Apple") do
WrapperJob.perform_later
end
end

expect(batch).to be_finished
expect(RESULTS).to eq []
end
end
end
12 changes: 12 additions & 0 deletions spec/lib/good_job/active_job_extensions/concurrency_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,18 @@ def perform(name:)
TestJob.perform_now(name: "Alice")
expect(JOB_PERFORMED).to be_true
end

it 'is ignored when the job is executed inside another job' do
stub_const("WrapperJob", Class.new(ApplicationJob) do
def perform
TestJob.perform_now(name: "Alice")
end
end)

WrapperJob.perform_later
GoodJob.perform_inline
expect(JOB_PERFORMED).to be_true
end
end

describe '#enqueue_throttle' do
Expand Down