[Messenger] Allow handler locator to be set #60753
Conversation
| { | ||
| $container = $this->getContainerBuilder($busId = 'message_bus'); | ||
|
|
||
| $container->register($busId. '.messenger.handlers_locator', \stdClass::class); |
There was a problem hiding this comment.
stdClass isn't the right class here but wasn't sure it was worth the effort to create a fake HandlersLocatorInterface but happy to add it if you would prefer
The `HandleMessageMiddleware` has a dependency on a `HandlersLocatorInterface` but this gets registered by the messenger pass. As far as I can see there isn't a way for an application to provide it's own implementation. By checking if the service already exists before registering then we can allow applications to define their own
maxbaldanza
force-pushed
the
change-handlers-locator
branch
from
fcff4ae to
8b63b8f
Compare
|
I'm mixed here: a similar change could potentially apply to many compiler passes. |
Yeah I think that's a fair comment. I could define a compiler pass to do the same but it'd be great if the Symfony framework provided a way to be able to customise that without a compiler pass given there's an interface there.
My use case isn’t about customising the handler mapping — I want to replace the entire Given Symfony has an interface there then it'd be great to be able to swap out the implementation some way. Open to adjust the approach if there's a preferred pattern |
| if (!$container->has($locatorId)) { | ||
| $container->register($locatorId, HandlersLocator::class) |
There was a problem hiding this comment.
Could it be interesting to give the handlers locator to the existing definition?
| if (!$container->has($locatorId)) { | |
| $container->register($locatorId, HandlersLocator::class) | |
| ($container->hasDefinition($locatorId) ? $container->getDefinition($locatorId) : $container->register($locatorId, HandlersLocator::class)) |
There was a problem hiding this comment.
Could be but that means the first argument needs to be always be an array and if a new argument is added in Symfony in the future for the HandlersLocator then it could be classed as a breaking change because the arguments defined at application level would need to match.
For example, say a bool is now needed as a second param and someone has already defined the service with a different argument then you'd end up with an error: Argument #2 ($foo) must be of type string, bool given
I think it's safer with my suggestion but happy to change it if you prefer.
| ->setArgument(0, $handlersLocatorMappingByBus[$bus] ?? []) | ||
| ; | ||
| } | ||
|
|
||
| if ($container->has($handleMessageId = $bus.'.middleware.handle_message')) { |
There was a problem hiding this comment.
while at it:
| if ($container->has($handleMessageId = $bus.'.middleware.handle_message')) { | |
| if ($container->hasDefinition($handleMessageId = $bus.'.middleware.handle_message')) { |
Hi,
I'd like to use the
HandleMessageMiddlewarebut use a different locator to get the handlers of the messages. TheHandleMessageMiddlewareuses an interface but I can't see how to define a different implementation as the compiler pass sets this argument per bus.I could define my own version of the
handle_messagemiddleware and provide the dependancy but this means I can no longer use thedefault_middlewareconfiguration.By checking if the locator already exists it means the application could define the
HandlersLocatorInterfaceimplementation that is used by defining something like this in their services.yaml:{$busName}.messenger.handlers_locator: '@Application\Middleware\HandlersLocator'Would be great to hear if there's maybe a better way to do this or if I've missed a detail on how to provide the
HandlersLocatorInterface