ShouldBeUnique Does Not Universally Prevent Duplicates

[kellerjmrtn]

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:

• Unique Jobs are not unique when using Dependency Injection #39113
• dispatch method does not respect ShouldBeUnique on jobs #45781
• Unable to prevent the creation of the same in a queue #42297
• Batched Unique Jobs Can Be Dispatched With The Same UniqueId Values #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

[github-actions]

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

[driesvints]

As Taylor said we're tabling this one for now.

[shawnnaquin]

@driesvints I'm nobody to question the maintenance team. But I did want to point out:

(While I'm not intimately familiar with the internals)

Taylor's comments makes it seem as though only undocumented dispatch methods are affected.

But that's not the case.

Using ShouldBeUnique as documented, with the documented Bus::chain, and Bus::batch methods does not guarantee uniqueness.

The only way to guarantee a unique job is going through PendingDispatch, which Bus::chain and Bus::batch do not.

I'm pretty confident, This is a broken feature. (10x, 11x, and 12x)