framework icon indicating copy to clipboard operation
framework copied to clipboard

[11.x] Introduce `OnQueue` and `OnConnection` attributes for jobs, Mailables, Notifications, queued event listeners

Open cosmastech opened this issue 10 months ago β€’ 6 comments

The problem

PHP is weird about traits. If a class uses a trait, then it cannot re-declare properties from the trait with differing values. (Though if another class uses the trait and then you extend that class, you can πŸ™ƒ)

This is problematic with setting queue/connections, where it would be great if we could just define the $queue and $connection properties.

An example with Queueable
class SyncAthletesJob implements ShouldQueue
{
    use Queueable;
    use Dispatchable;

    public $connection = "redis_medium";  // ❌ this doesn't work, as it differs from the definition in Queueable
}

Setting queue/connection is annoying

My jobs do not need to be dynamic in where they are dispatched. If it's a SyncAthletesJob, it's always on the import queue (and the import queue is always on the redis_medium connection, but that's not a problem solved here).

The two not-so-pretty ways of specifying this
class SyncAthletesJob implements ShouldQueue
{
    use Queueable;
    use Dispatchable;

    public function __construct(private TeamId $teamId)
    {
        $this->onConnection('redis_medium');
        $this->onQueue('import');
    }
}

// or when I call it
SyncAthletesJob::dispatch($teamId)->onQueue('import')->onConnection('redis_medium');

The solution

Introduce OnQueue and OnConnection attributes.

Now the above job can look clean 🧹 and I never have to worry about specifying queue/connection in calling code or inside of the constructor.

#[OnQueue(QueueEnum::IMPORT)]
#[OnConnection(QueueConnectionEnum::REDIS_MEDIUM)]
class SyncAthletesJob implements ShouldQueue
{
    use Queueable;
    use Dispatchable;

    public function __construct(private TeamId $teamId) { }
}

Bonus: when I call dispatch(new SyncAthletesJob($teamId)) I get the same benefit.

Todos

  • [x] Works with queued jobs
  • [x] Works with Mailables
  • [x] Works on queued event listeners
  • [x] Works on Notifications

Follow ups

  • ~~Does this need added for queued event listeners as well? If so, we could share these attributes, but leaving them in Foundation/Bus doesn't really make sense.~~ Answered: move to Illuminate/Foundation/Queue

  • Additional attributes for #[OnDasher], #[OnDancer], #[OnPrancer], #[OnVixen], et cetera

cosmastech avatar Jan 17 '25 13:01 cosmastech

An On prefix makes me think, it's an event listener. In would sound more naturally to me.

shaedrich avatar Jan 18 '25 14:01 shaedrich

An On prefix makes me think, it's an event listener. In would sound more naturally to me.

But then how would I make my super funny joke about #[OnDasher], #[OnDancer]?

cosmastech avatar Jan 18 '25 15:01 cosmastech

I see, touchΓ© πŸ˜‚πŸ‘πŸ»

shaedrich avatar Jan 18 '25 15:01 shaedrich

Yet another way to slow down the framework.

Jacobs63 avatar Jan 21 '25 15:01 Jacobs63

Yet another way to slow down the framework.

If you want super fast, maybe, you shouldn't use Laravel in the first place πŸ™„

shaedrich avatar Jan 21 '25 15:01 shaedrich

@cosmastech before merging this we would likely need a way to do the same for listeners, mail, and notifications. So it would probably need to go in the Queue namespace.

That being said - I'm not sure if we're getting tons of benefit here.

taylorotwell avatar Jan 22 '25 21:01 taylorotwell