dispatch method does not respect ShouldBeUnique on jobs

[ankurk91]

• Laravel Version: 9.48..0
• PHP Version: 8.2
• Database Driver & Version: MySql
• Cache and Queue driver - Redis v7

Description:

Steps To Reproduce:

Create a test job:

<?php

namespace App\Jobs;

use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldBeUnique;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Queue\SerializesModels;

class TestJob implements ShouldQueue, ShouldBeUnique
{
    use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;

    public function __construct()
    {
        //
    }

    public function handle()
    {
        info("Running the job");
        sleep(3);
    }
}

Then create a Controller and dispatch the job via dispatch() helper available in all Controllers

<?php

namespace App\Http\Controllers;

use App\Http\Controllers\Controller;
use App\Jobs\TestJob;

class TestController extends Controller
{
    public function index()
    {
        // TestJob::dispatch(); // works as expected 
        // dispatch(new TestJob()); // works as expected
        
        $this->dispatch(new TestJob()); // causing the issue
    }
}

Start the worker

php artisan queue:work --sleep=3

Now hit the controller via a route twice, notice worker will process 2 jobs.

The problem lies here in this trait

framework/src/Illuminate/Foundation/Bus/DispatchesJobs.php

Line 17 in 247735e

return app(Dispatcher::class)->dispatch($job);

This method is dispatching the jobs in a different way which does not respect the ShouldBeUnique contract

[driesvints]

This is intentional I feel. The former methods you used are all actions on the jobs themselves while the Dispatcher just works differently. If you want to use the ShouldBeUnique you already have two ways of making that work.

[ankurk91]

This must be documented.
There might be other unknown behaviour of using this method.

Why can't we make them work the same way??

[driesvints]

Feel free to send in a pr 👍

[driesvints]

I meant to say: send in a PR to the docs. We can't change the behavior here.

[alies-dev]

Just a note for someone who experienced the same issue (to save your time): \Illuminate\Support\Facades\Bus::dispatch also doesn't respect ShouldBeUnique (it proxies calls to the \Illuminate\Bus\Dispatcher that probably used by @ankurk91 at the original message/example: $this->dispatch())

Bus::dispatch adds a job directly to a queue:

\Illuminate\Bus\Dispatcher::dispatch($jobInstance)

while dispatch() helper creates PendingDispatch class instance that has __destruct method:

/**
 * Handle the object's destruction.
 *
 * @return void
 */
public function __destruct()
{
    if (! $this->shouldDispatch()) {
        return;
    } elseif ($this->afterResponse) {
        app(Dispatcher::class)->dispatchAfterResponse($this->job);
    } else {
        app(Dispatcher::class)->dispatch($this->job);
    }
}

and inside PendingDispatch::shouldDispatch it has this code:

/**
 * Determine if the job should be dispatched.
 *
 * @return bool
 */
protected function shouldDispatch()
{
    if (! $this->job instanceof ShouldBeUnique) {
        return true;
    }

    return (new UniqueLock(Container::getInstance()->make(Cache::class)))
                ->acquire($this->job);
}

So, dispatch has some extra behavior on the top of \Illuminate\Bus\Dispatcher