Skip to content

Commit

Permalink
Ensure job context does not leak into extensions when perform_now i…
Browse files Browse the repository at this point in the history
…s called within another job (#1336)
  • Loading branch information
bensheldon committed Apr 24, 2024
1 parent 410b34f commit 59b8c68
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 3 deletions.
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

0 comments on commit 59b8c68

Please sign in to comment.