[Messenger] internal cleanups #28908
Conversation
src/Symfony/Component/Messenger/Asynchronous/Routing/AbstractSenderLocator.php
Show resolved
Hide resolved
b9ca4e7 to
0dac059
Compare
src/Symfony/Component/Messenger/Asynchronous/Routing/AbstractSenderLocator.php
Show resolved
Hide resolved
| */ | ||
| abstract class AbstractHandlerLocator implements HandlerLocatorInterface | ||
| { | ||
| public function resolve($message): callable | ||
| public function getHandler(Envelope $envelope, bool $allowNoHandler = false): ?callable |
There was a problem hiding this comment.
$allowNoHandler is not used for now, but I'd like we add it right now in the interface and replace AllowNoHandlerMiddleware later on (using exceptions as a signaling mean is making the "no-handler" path slow for no real reasons.)
There was a problem hiding this comment.
It's internal, so you can add it later, as part of another PR please 😉
There was a problem hiding this comment.
(agreed, removed)
365ff57 to
23435f2
Compare
23435f2 to
3e3182c
Compare
|
|
||
| if ($sender) { | ||
| $sender->send($envelope); | ||
|
|
||
| if (!$this->mustSendAndHandle($envelope->getMessage())) { |
There was a problem hiding this comment.
Keeping the private function increases the readability, I'd keep it.
There was a problem hiding this comment.
Honeslty, I've had a hard time understanding what this was about. The private method didn't help me. I think the comment would make a better job. I'd keep the change :)
src/Symfony/Component/Messenger/Asynchronous/Routing/AbstractSenderLocator.php
Show resolved
Hide resolved
| */ | ||
| abstract class AbstractHandlerLocator implements HandlerLocatorInterface | ||
| { | ||
| public function resolve($message): callable | ||
| public function getHandler(Envelope $envelope, bool $allowNoHandler = false): ?callable |
There was a problem hiding this comment.
It's internal, so you can add it later, as part of another PR please 😉
| */ | ||
| public function send(Envelope $envelope); |
There was a problem hiding this comment.
Sneaky. Surely sender will want to return things. It very similar to the : void on the MessageBusInterface. Sounds reasonable if we provide a clear path for the use-cases returning something.
There was a problem hiding this comment.
We don't have any use case in core for this, and the code is extensible enough so that people can write their own way of getter something back (ie a middleware + an extended SendInterface of their own, that's nothing standard/core anyway).
Better close the interface to ease maintainance from core team's pov.
0b3dc43 to
b45836e
Compare
|
PR rebased and comments addressed, ready. |
| } | ||
|
|
||
| /** | ||
| * {@inheritdoc} |
There was a problem hiding this comment.
This should have stayed as before
b45836e to
4a3edd0
Compare
| usort($messages, function (array $a, array $b): int { | ||
| return $a[1] > $b[1] ? 1 : -1; | ||
| }); | ||
| usort($messages, function ($a, $b) { return $a[1] <=> $b[1]; }); |
|
Thank you @nicolas-grekas. |
This PR was merged into the 4.2-dev branch. Discussion ---------- [Messenger] internal cleanups | Q | A | ------------- | --- | Branch? | 4.2 | Bug fix? | no | New feature? | no | BC breaks? | yes | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - From the updated changelog: * `MessengerDataCollector::getMessages()` returns an iterable, not just an array anymore * `AbstractHandlerLocator` is now internal * `HandlerLocatorInterface::resolve()` has been replaced by `getHandler(Envelope $envelope)` * `SenderLocatorInterface::getSenderForMessage()` has been replaced by `getSender(Envelope $envelope)` * `SenderInterface::send()` returns `void` + some internal simplifications Commits ------- 4a3edd0 [Messenger] internal cleanups
From the updated changelog:
MessengerDataCollector::getMessages()returns an iterable, not just an array anymoreAbstractHandlerLocatoris now internalHandlerLocatorInterface::resolve()has been replaced bygetHandler(Envelope $envelope)SenderLocatorInterface::getSenderForMessage()has been replaced bygetSender(Envelope $envelope)SenderInterface::send()returnsvoid