framework icon indicating copy to clipboard operation
framework copied to clipboard

ShouldBeUnique Does Not Universally Prevent Duplicates

Open kellerjmrtn opened this issue 8 months ago • 1 comments

Laravel Version

10.48.12 (although I believe relevant code has not changed in 11.x)

PHP Version

8.1.28

Database Driver & Version

Redis 7.2.5 (although issue should exist with other cache drivers)

Description

When dispatching jobs, the ShouldBeUnique behavior is only respected when dispatching through the PendingDispatch class and the Illuminate\Console\Scheduling\Schedule class (the latter fix was added in #39302). However, it is not respected when using the Illuminate\Bus\Dispatcher class or individual queue drivers (Queue::push( ... ), etc). This behavior can be confusing because depending on how you queue the job, it's easily possible to queue duplicates, despite the job implementing ShouldBeUnique

I found a previous PR (#50381) that attempted to move the ShouldBeUnique check into the Dispatcher class. Although that still wouldn't guarantee uniqueness when using Queue::push(). I was also able to find some other issues which seem to indicate that people are generally expecting ShouldBeUnique to work universally:

  • #39113
  • #45781
  • #42297
  • #48882

There's also a mention in the original PR for the ShouldBeUnique feature mentioning how this could possibly be moved into the enqueueUsing() logic in the Queue class? This seems like it would probably fix all of the above issues

Further, the Dispatcher class and Queue enqueueUsing() method already check other "expressive" interfaces like ShouldQueue, ShouldQueueAfterCommit, etc, so it seems reasonable to also check ShouldBeUnique there

I would be willing to open a PR for this if desired. Was hesitant to do so as a similar PR (#50381) has already been closed

Steps To Reproduce

Create a job which implements ShouldBeUnique

<?php

namespace App\Jobs;

use Illuminate\Contracts\Queue\ShouldBeUnique;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;

class UniqueJob implements ShouldQueue, ShouldBeUnique
{
    use Dispatchable;

    ...
}

Confirm that ShouldBeUnique works correctly when using PendingDispatch. In tinker

Queue::size(); // Returns 0
UniqueJob::dispatch();
Queue::size(); // Returns 1. May have to call this multiple times, presumably to trigger the `__destruct()` method in `PendingDispatch`, but it will queue the job eventually
UniqueJob::dispatch();
Queue::size(); // Still returns 1, as `ShouldBeUnique` prevents queuing

Confirm that using Queue::push(new UniqueJob()), app(Dispatcher::class)->dispatch(new UniqueJob()), Bus::dispatch(new UniqueJob()) do not work the same way

Queue::size(); // Returns 0
Queue::push(new UniqueJob());
Queue::size(); // Returns 1
Queue::push(new UniqueJob());
Queue::size(); // Returns 2. One would expect a "unique" job to not be queued again

kellerjmrtn avatar Jun 13 '24 20:06 kellerjmrtn