diff --git a/src/Controller/Admin/QueueController.php b/src/Controller/Admin/QueueController.php index 71d5676a..758cf9a4 100644 --- a/src/Controller/Admin/QueueController.php +++ b/src/Controller/Admin/QueueController.php @@ -113,8 +113,22 @@ public function index() { 'failure_message IS NOT' => null, ]) ->count(); - // Pending = total pending minus running and failed (to avoid double counting) - $pendingJobs = max(0, $totalPending - $runningJobs - $failedJobs); + // Aborted jobs (retries exhausted) carry a failure_message but are + // terminal and already excluded from getPendingCount()/getPendingStats(). + // Subtract only the still-retriable failures here so aborted rows are not + // removed twice (which would under-count, or zero out, real pending work). + $retriableFailedJobs = $this->QueuedJobs->find() + ->where([ + 'completed IS' => null, + 'failure_message IS NOT' => null, + 'OR' => [ + 'status IS' => null, + 'status !=' => $this->QueuedJobs::STATUS_ABORTED, + ], + ]) + ->count(); + // Pending = total pending minus running and still-retriable failed (to avoid double counting) + $pendingJobs = max(0, $totalPending - $runningJobs - $retriableFailedJobs); $configurations = (array)Configure::read('Queue'); diff --git a/src/Model/Table/QueuedJobsTable.php b/src/Model/Table/QueuedJobsTable.php index 1a4feb04..51b29c4b 100644 --- a/src/Model/Table/QueuedJobsTable.php +++ b/src/Model/Table/QueuedJobsTable.php @@ -210,7 +210,7 @@ public function createConfig(): JobConfig { * * @return \Queue\Model\Entity\QueuedJob Saved job entity (or the existing pending entity if `unique` deduped). */ - public function createJob(string $jobTask, array|object|null $data = null, array|JobConfig $config = []): QueuedJob { + public function createJob(string $jobTask, object|array|null $data = null, JobConfig|array $config = []): QueuedJob { if (!$config instanceof JobConfig) { $config = $this->createConfig()->fromArray($config); } @@ -844,6 +844,9 @@ public function reset(?int $id = null, bool $full = false): int { 'failure_message' => null, 'output' => null, 'memory' => null, + // Clear the terminal aborted status so a reset job counts as pending + // again (getPendingCount()/isQueued() exclude status = aborted). + 'status' => null, ]; $conditions = [ 'completed IS' => null, @@ -1103,16 +1106,35 @@ public function getPendingStats(): SelectQuery { 'failure_message', 'memory', ], - 'conditions' => [ - 'completed IS' => null, + 'conditions' => $this->pendingConditions(), + ]; + + return $this->find('all', ...$findCond); + } + + /** + * Shared WHERE conditions for "pending" jobs: not completed, due to run, and + * not terminally aborted. An aborted job keeps `completed IS NULL` but will + * never run again (retries exhausted), so it must not be reported as pending. + * + * @return array + */ + protected function pendingConditions(): array { + return [ + 'completed IS' => null, + [ 'OR' => [ 'notbefore <=' => new DateTime(), 'notbefore IS' => null, ], ], + [ + 'OR' => [ + 'status IS' => null, + 'status !=' => static::STATUS_ABORTED, + ], + ], ]; - - return $this->find('all', ...$findCond); } /** @@ -1125,13 +1147,7 @@ public function getPendingStats(): SelectQuery { */ public function getPendingCount(): int { return $this->find() - ->where([ - 'completed IS' => null, - 'OR' => [ - 'notbefore <=' => new DateTime(), - 'notbefore IS' => null, - ], - ]) + ->where($this->pendingConditions()) ->count(); } diff --git a/tests/TestCase/Model/Table/QueuedJobsTableTest.php b/tests/TestCase/Model/Table/QueuedJobsTableTest.php index acbd2aa7..57412c85 100644 --- a/tests/TestCase/Model/Table/QueuedJobsTableTest.php +++ b/tests/TestCase/Model/Table/QueuedJobsTableTest.php @@ -874,6 +874,59 @@ public function testMarkJobAborted() { $this->assertNull($reloaded->completed); } + /** + * Aborted jobs keep `completed IS NULL` but are terminal, so they must not + * be counted by the pending stats/count used on the admin dashboard. + * + * @return void + */ + public function testGetPendingCountExcludesAborted() { + $pending = $this->QueuedJobs->newEntity([ + 'key' => 'key', + 'job_task' => 'FooBar', + 'reference' => 'pending-one', + ]); + $this->QueuedJobs->saveOrFail($pending); + + $aborted = $this->QueuedJobs->newEntity([ + 'key' => 'key', + 'job_task' => 'FooBar', + 'reference' => 'aborted-one', + ]); + $this->QueuedJobs->saveOrFail($aborted); + $this->QueuedJobs->markJobAborted($aborted); + + $this->assertSame(1, $this->QueuedJobs->getPendingCount()); + + $statsRefs = $this->QueuedJobs->getPendingStats()->all()->extract('reference')->toArray(); + $this->assertContains('pending-one', $statsRefs); + $this->assertNotContains('aborted-one', $statsRefs); + } + + /** + * Resetting an aborted job for rerun must clear its terminal status so it + * counts as pending again and gets picked up. + * + * @return void + */ + public function testResetClearsAbortedStatus() { + $job = $this->QueuedJobs->newEntity([ + 'key' => 'key', + 'job_task' => 'FooBar', + 'reference' => 'reset-me', + 'attempts' => 3, + ]); + $this->QueuedJobs->saveOrFail($job); + $this->QueuedJobs->markJobAborted($job); + $this->assertSame(0, $this->QueuedJobs->getPendingCount()); + + $this->QueuedJobs->reset($job->id, true); + + $reloaded = $this->QueuedJobs->get($job->id); + $this->assertNull($reloaded->status); + $this->assertSame(1, $this->QueuedJobs->getPendingCount()); + } + /** * @return void */