diff --git a/app/Jobs/CategorizeUncategorizedTransactionsJob.php b/app/Jobs/CategorizeUncategorizedTransactionsJob.php index dcea2e7d..c39694f9 100644 --- a/app/Jobs/CategorizeUncategorizedTransactionsJob.php +++ b/app/Jobs/CategorizeUncategorizedTransactionsJob.php @@ -5,6 +5,7 @@ namespace App\Jobs; use App\Models\User; use App\Services\Ai\AiCategorizationGate; use App\Services\Ai\AiCategorizer; +use Illuminate\Contracts\Queue\ShouldBeUnique; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Queue\Queueable; use Illuminate\Support\Facades\Cache; @@ -15,11 +16,17 @@ use Throwable; * consent outside of onboarding. Progress is written to the cache so the * transactions page can poll it and surface live progress while the batch runs. * + * De-duplicated per user: a second dispatch (double "Enable AI" click, or + * re-enabling consent while a run is still in flight) would read the same + * pending-transaction snapshot on a concurrent worker and re-bill the model for + * work already underway — the exact harm tries=1 targets but cannot prevent, + * since tries only bounds re-attempts of one dispatch, not duplicate dispatches. + * * ponytail: mirrors CategorizeOnboardingTransactionsJob's selection + chunking; * kept separate so the onboarding pass stays progress-free. Fold the two * together if a third caller ever needs the same loop. */ -class CategorizeUncategorizedTransactionsJob implements ShouldQueue +class CategorizeUncategorizedTransactionsJob implements ShouldBeUnique, ShouldQueue { use Queueable; @@ -34,8 +41,19 @@ class CategorizeUncategorizedTransactionsJob implements ShouldQueue */ public int $tries = 1; + /** + * Safety TTL for the unique lock in case a worker dies mid-run; comfortably + * longer than a full run. + */ + public int $uniqueFor = 1800; + public function __construct(public User $user, public string $jobId) {} + public function uniqueId(): string + { + return $this->user->id; + } + public function viaQueue(): string { return (string) config('ai_categorization.queue'); diff --git a/config/queue.php b/config/queue.php index 4fe599aa..bcc38761 100644 --- a/config/queue.php +++ b/config/queue.php @@ -40,7 +40,14 @@ return [ 'connection' => env('DB_QUEUE_CONNECTION'), 'table' => env('DB_QUEUE_TABLE', 'jobs'), 'queue' => env('DB_QUEUE', 'default'), - 'retry_after' => (int) env('DB_QUEUE_RETRY_AFTER', 90), + // retry_after must exceed the longest job $timeout on this connection + // (currently 600s). Otherwise a long job outlives its reservation, the + // queue hands it to a second worker, and a tries=1 job dies with + // MaxAttemptsExceededException (and re-runs its side effects). Each + // job's execution ceiling — its own $timeout, or the worker --timeout + // when a job declares none — must stay below this value. QueueConfigTest + // guards the $timeout side. + 'retry_after' => (int) env('DB_QUEUE_RETRY_AFTER', 900), 'after_commit' => true, ], diff --git a/tests/Feature/Ai/CategorizeUncategorizedTransactionsTest.php b/tests/Feature/Ai/CategorizeUncategorizedTransactionsTest.php index 854025e6..9edf1be3 100644 --- a/tests/Feature/Ai/CategorizeUncategorizedTransactionsTest.php +++ b/tests/Feature/Ai/CategorizeUncategorizedTransactionsTest.php @@ -9,6 +9,7 @@ use App\Models\Transaction; use App\Models\User; use App\Services\Ai\CategorizeTransactions; use App\Services\Ai\CategoryCatalog; +use Illuminate\Contracts\Queue\ShouldBeUnique; use Illuminate\Support\Facades\Bus; use Illuminate\Support\Facades\Cache; @@ -218,3 +219,11 @@ it('categorizes the most recent transactions first', function () { expect($order)->toBe([$newest->id, $middle->id, $oldest->id]); }); + +it('de-duplicates the backfill per user so a concurrent dispatch cannot double-bill', function () { + $user = User::factory()->create(); + $job = new CategorizeUncategorizedTransactionsJob($user, 'job-x'); + + expect($job)->toBeInstanceOf(ShouldBeUnique::class) + ->and($job->uniqueId())->toBe($user->id); +}); diff --git a/tests/Feature/QueueConfigTest.php b/tests/Feature/QueueConfigTest.php new file mode 100644 index 00000000..55b3e4f6 --- /dev/null +++ b/tests/Feature/QueueConfigTest.php @@ -0,0 +1,41 @@ +flatMap(fn (string $dir): array => File::allFiles(app_path($dir))) + ->map(function ($file): string { + $relative = Str::of($file->getPathname()) + ->after(app_path().DIRECTORY_SEPARATOR) + ->replace(DIRECTORY_SEPARATOR, '\\') + ->replaceLast('.php', ''); + + return 'App\\'.$relative; + }) + ->filter(fn (string $class): bool => class_exists($class)) + ->mapWithKeys(function (string $class): array { + $timeout = (new ReflectionClass($class))->getDefaultProperties()['timeout'] ?? null; + + return [$class => $timeout]; + }) + ->filter(fn ($timeout): bool => is_int($timeout) && $timeout >= $retryAfter); + + expect($offenders->all())->toBe( + [], + 'These jobs have a $timeout >= retry_after ('.$retryAfter.'s); raise DB_QUEUE_RETRY_AFTER above the longest job timeout.' + ); +});