Skip to content

Commit 155fa00

Browse files
committed
Set successfully_enqueued in Job.enqueue
Even though this will be overwritten by Active Job when called through `perform_later`. This is necessary for recurring tasks, where we run the `enqueue` callbacks manually and call `Job.enqueue`. We rely on `successfully_enqueued?` to know whether we need to record the recurring execution. We were setting that no matter what after calling `Job.enqueue` because we were relying on that to raise (which would be the case for Active Record errors). In the case of discarding it because of concurrency controls, the job is simply not persisted but no error is raised, which means we were trying to record a recurring execution without a persisted job, which would raise. Fixes #598
1 parent 7a7ae43 commit 155fa00

File tree

3 files changed

+31
-5
lines changed

3 files changed

+31
-5
lines changed

app/models/solid_queue/job.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ def enqueue(active_job, scheduled_at: Time.current)
3030

3131
create_from_active_job(active_job).tap do |enqueued_job|
3232
active_job.provider_job_id = enqueued_job.id if enqueued_job.persisted?
33+
active_job.successfully_enqueued = enqueued_job.persisted?
3334
end
3435
end
3536

app/models/solid_queue/recurring_task.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,6 @@ def enqueue_and_record(run_at:)
130130
active_job.run_callbacks(:enqueue) do
131131
Job.enqueue(active_job)
132132
end
133-
active_job.successfully_enqueued = true
134133
end
135134
end
136135
end

test/models/solid_queue/recurring_task_test.rb

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,14 @@ def perform
4141
end
4242
end
4343

44+
class JobWithConcurrencyControlsAndDiscard < ApplicationJob
45+
limits_concurrency key: -> { true }, on_conflict: :discard
46+
47+
def perform
48+
JobBuffer.add "job_with_concurrency_controls"
49+
end
50+
end
51+
4452
test "job without arguments" do
4553
task = recurring_task_with(class_name: "JobWithoutArguments")
4654
enqueue_and_assert_performed_with_result task, "job_without_arguments"
@@ -88,6 +96,20 @@ def perform
8896
end
8997
end
9098

99+
test "error when enqueuing job because of concurrency controls and discard" do
100+
JobWithConcurrencyControlsAndDiscard.perform_later
101+
102+
task = recurring_task_with(class_name: "JobWithConcurrencyControlsAndDiscard")
103+
assert_no_difference -> { SolidQueue::Job.count } do
104+
task.enqueue(at: Time.now)
105+
end
106+
107+
# Run the enqueued job that was preventing a new one from being enqueued
108+
run_all_jobs_inline
109+
110+
enqueue_and_assert_performed_with_result task, "job_with_concurrency_controls"
111+
end
112+
91113
test "error when enqueuing job using another adapter that raises ActiveJob::EnqueueError" do
92114
ActiveJob::QueueAdapters::AsyncAdapter.any_instance.stubs(:enqueue).raises(ActiveJob::EnqueueError)
93115
previous_size = JobBuffer.size
@@ -174,10 +196,7 @@ def enqueue_and_assert_performed_with_result(task, result)
174196
end
175197

176198
assert_difference -> { JobBuffer.size }, +1 do
177-
SolidQueue::Worker.new(queues: "*").tap do |worker|
178-
worker.mode = :inline
179-
worker.start
180-
end
199+
run_all_jobs_inline
181200
end
182201

183202
assert_equal result, JobBuffer.last_value
@@ -192,4 +211,11 @@ def recurring_task_with(class_name: nil, **options)
192211

193212
SolidQueue::RecurringTask.from_configuration("task-id", **options)
194213
end
214+
215+
def run_all_jobs_inline
216+
SolidQueue::Worker.new(queues: "*").tap do |worker|
217+
worker.mode = :inline
218+
worker.start
219+
end
220+
end
195221
end

0 commit comments

Comments
 (0)