[8.x] Add JobQueued event

[vdauchy]

This PR aims to add a new JobQueued event dispatched when a job is added to a queue.

The main reason for this is to complete the existing events list: JobProcessing, JobProcessed, JobFailed and JobExceptionOccurred which let users track the jobs.

Looking for packages or tutorials to track queued job status will give for example: https://github.com/imTigger/laravel-job-status or https://blog.rafter.app/tracking-queued-job-chains-in-laravel/ .

In both cases the trick is made by creating the tracking record to the database inside the job's constructor.

Having a JobQueue event aims to remove this DB call from the job's constructor.

So to track a job a simple EventSubscriber class could be created as:

public function subscribe(Dispatcher $dispatcher) {
    $dispatcher->listen(JobQueued::class, /* Handler */);
    $dispatcher->listen(JobProcessing::class, /* Handler */);
    $dispatcher->listen(JobProcessed::class, /* Handler */ );
    $dispatcher->listen(JobFailed::class, /* Handler */);
    $dispatcher->listen(JobExceptionOccurred::class, /* Handler */);
}

--

Following: #32881

[vdauchy]

@taylorotwell To be sure, following your comment (#32881 (comment)) my next step shall be to refactor:

Dispatcher::__construct(Container $container, Closure $queueResolver = null)

to

Dispatcher::__construct(Container $container, Illuminate\Contracts\Events\Dispatcher $eventsDispatcher, Closure $queueResolver = null)

[taylorotwell]

Is there a big reason you didn't fire event from Queue::push? Why fire it in Bus?

[vdauchy]

At the beginning I put it in Dispatcher::dispatchToQueue() but because of the multi returns I choose to move one level up to Dispatcher::dispatch().

Now I see the method: public function dispatchSync($command, $handler = null) that calls dispatchToQueue and so the event would not be triggered.

Now why not Queue::push, the main reasons are:

• Illuminate\Queue\Queue is an abstract which does not implement push() so I would need the add it and then add a parent::push() to all concrete classes, which is a lot of code changes for the original PR.
• Same logic need to be applied to Queue::later()
• How to make sure that custom return $command->queue($queue, $command); is handle in Dispatcher::dispatchToQueue ?

I note now that the Queue::push() would be a better place for Queue::bulk() to send the event properly too...

If you want this to be implemented in Illuminate\Queue\Queue let me know if you expect a setEventsDispatcher() to be added or if using the container is ok.

[taylorotwell]

Yeah there is generally some inconsistency here. The other job events fire for bulk queued jobs but this doesn't.

[vdauchy]

Fine, then I will revert my changes and go on Illuminate\Queue\Queue.

I will add both Queue::push() and Queue::later() with raiseJobQueuedEvent() to this abstract class and add the parent::push() and parent::later() calls to all concretes.

For the event dispatch I will use the container the same way you did in SyncQueue class.

[vdauchy]

@taylorotwell My latest commit (10022f6) is a revert from my first implementation with the logic being moved to queue as you asked.

To track the queued job we always need the jobId returned by Queue::push() and Queue::later().

Due to this, calling a parent implementation was not possible in a clean way, so to avoid code duplication I made the choice to reduce the visibility and rename all concrete Queue::push() and Queue::later() and added them to the abstract class Illuminate\Queue\Queue.

This way they act as proxy to the old method and make sure the event is dispatched with the returned jobId.

This commit does not change any tests (so they fail), let me know is this solution is valid for you, then I will try to go trough them.

[taylorotwell]

I think you would have better luck just using the base queue createPayload() method to fire the event. All the queue drivers call it during push and during bulk.

[vdauchy]

Sure createPayload() is called everywhere the event shall be sent.

However does that means that a function named createPayload shall be responsible to send an event jobQueued ? To me it sound like an unexpected behavior.

Doing it this way will sure reduce the amount of changed code. But as the current solution, no interface is changed and the same tests using mocks will fail. So I'm not sure about the benefits of using createPayload().

[taylorotwell]

Going to close this for now as I don't want to make this many changes to support it.

[vdauchy]

Sure, maybe refactoring the inheritance of Queue by a composition (proxy pattern) of Queue(Engine) where the engine could be any of the existing queue could be possible later...

This would help moving functions like createStringPayload(), createObjectPayload(), bulk(), raiseJobQueuedEvent() out of the engine themselves...

In the meanwhile if you want this feature to be done inside createPayload I would be happy to help, just tell me so it would not be done in vain.