[12.x] ShouldBeUnique Does Not Universally Prevent Duplicates

[kellerjmrtn]

Fixes #51798

This fix was suggested in the original PR which introduced ShouldBeUnique here. Specifically pulled inspiration from the gist linked in the comments of that PR.

I pulled the logic for shouldDispatch() from the prexisting method in the PendingDispatch class. However I noticed in #39302 that an additional

if (! Container::getInstance()->bound(Cache::class)) {
    throw new RuntimeException('Cache driver not available. Scheduling unique jobs not supported.');
}

Check was added before attempting to acquire a lock. I'm not sure if checking for this should be done here?

I also wasn't quite sure the best way to write tests for this (or where best to put them), as the logic is buried pretty deep into the Queue class and required a lot of mocks. I wrote a couple as a proof of concept, but would improve/add more if necessary (suggestions/help welcome).

Also potentially of note is that the shouldDispatch check happens during the callback for ShouldQueueAfterCommit jobs. This means that the uniqueness is checked after the transaction, not immediately. I would think this behavior is desired, but I'm not quite sure

Lastly, this is a breaking change, as it prevents jobs from being queued sometimes when they previously would have been, therefore I've opened the PR to master

NOTE: Removed testUniqueJobsAreNotDispatched as it seems like a faulty test to me? It's essentially dispatching the job unto the BusFake (which never executes the job, since it's a fake), attempting to run the job with the queue worker, then asserting that the job lock still exists. But if the job has already run (or run synchronously), the lock should have already been released

[taylorotwell]

I would suggest re-targeting this for 11.x. Feels more like a bug than a breaking change.

[driesvints]

@kellerjmrtn can you send this to 11.x? Thanks!