From 57e36dc018f57ee741d5cb5841e021b315716c1e Mon Sep 17 00:00:00 2001 From: Rosa Gutierrez Date: Tue, 30 Jan 2024 16:59:52 +0100 Subject: [PATCH] Return last Solid Queue job when there are several with the same Active Job ID This applies in the case of automatic retries performed by Active Job's retrying mechanism. In that case, the failing job finishes just fine, the exception is retried and Active Job re-enqueues the job, so for Solid Queue, these look like different jobs with the same Active Job ID. Normally we'd be interested in the last one only, though, so just return that. In the future perhaps we can find a way to return all the instances if there are more. --- .../queue_adapters/solid_queue_ext.rb | 2 +- test/controllers/jobs_controller_test.rb | 45 +++++++++++++++---- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/lib/active_job/queue_adapters/solid_queue_ext.rb b/lib/active_job/queue_adapters/solid_queue_ext.rb index 0857bf10..837fbd92 100644 --- a/lib/active_job/queue_adapters/solid_queue_ext.rb +++ b/lib/active_job/queue_adapters/solid_queue_ext.rb @@ -83,7 +83,7 @@ def discard_job(job, jobs_relation) end def find_job(job_id, *) - if job = SolidQueue::Job.find_by(active_job_id: job_id) + if job = SolidQueue::Job.where(active_job_id: job_id).order(:id).last deserialize_and_proxy_solid_queue_job job end end diff --git a/test/controllers/jobs_controller_test.rb b/test/controllers/jobs_controller_test.rb index c9b5e126..bc033639 100644 --- a/test/controllers/jobs_controller_test.rb +++ b/test/controllers/jobs_controller_test.rb @@ -3,25 +3,51 @@ class MissionControl::Jobs::JobsControllerTest < ActionDispatch::IntegrationTest setup do DummyJob.queue_as :queue_1 - @job = DummyJob.perform_later(42) end test "get job details" do - get mission_control_jobs.application_job_url(@application, @job.job_id) + job = DummyJob.perform_later(42) + + get mission_control_jobs.application_job_url(@application, job.job_id) assert_response :ok - assert_includes response.body, @job.job_id - assert_includes response.body, "queue_1" + assert_select "h1", /DummyJob\s+pending/ + assert_includes response.body, job.job_id + assert_select "div.tag a", "queue_1" - get mission_control_jobs.application_job_url(@application, @job.job_id, filter: { queue_name: "queue_1" }) + get mission_control_jobs.application_job_url(@application, job.job_id, filter: { queue_name: "queue_1" }) assert_response :ok - assert_includes response.body, @job.job_id - assert_includes response.body, "queue_1" + assert_select "h1", /DummyJob\s+pending/ + assert_includes response.body, job.job_id + assert_select "div.tag a", "queue_1" + end + + test "get jobs and job details when there are multiple instances of the same job due to automatic retries" do + job = AutoRetryingJob.perform_later + perform_enqueued_jobs_async + + # Wait until the job has been executed and retried + sleep(1) + + get mission_control_jobs.application_jobs_url(@application, :finished) + assert_response :ok + + assert_select "tr.job", 2 + assert_select "tr.job", /AutoRetryingJob\s+Enqueued less than a minute ago\s+default/ + + get mission_control_jobs.application_job_url(@application, job.job_id) + assert_response :ok + + assert_select "h1", /AutoRetryingJob\s+failed\s+/ + assert_includes response.body, job.job_id + assert_select "div.is-danger", "failed" end test "redirect to queue when job doesn't exist" do - get mission_control_jobs.application_job_url(@application, @job.job_id + "0", filter: { queue_name: "queue_1" }) + job = DummyJob.perform_later(42) + + get mission_control_jobs.application_job_url(@application, job.job_id + "0", filter: { queue_name: "queue_1" }) assert_redirected_to mission_control_jobs.application_queue_path(@application, :queue_1) end @@ -32,9 +58,10 @@ class MissionControl::Jobs::JobsControllerTest < ActionDispatch::IntegrationTest travel_to 2.minutes.from_now get mission_control_jobs.application_jobs_url(@application, :scheduled) + assert_response :ok assert_select "tr.job", 2 assert_select "tr.job", /DummyJob\s+Enqueued 2 minutes ago\s+queue_1\s+in 1 minute/ - assert_select "tr.job", /DummyJob\s+Enqueued 2 minutes ago\s+queue_1\s+less than a minute ago/ + assert_select "tr.job", /DummyJob\s+Enqueued 2 minutes ago\s+queue_1\s+1 minute ago/ end end