Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Messenger] remove AllowNoHandlerMiddleware in favor of a constructor argument on HandleMessageMiddleware#28945
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
fa653b8 tod3005e8Compareb8407df to5c6b0b6Compare| returnarray($middleware); | ||
| ->always() | ||
| ->then(function ($middleware) { | ||
| return\is_string($middleware) || !\is_int(key($middleware)) ?array($middleware) :$middleware; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
(this borrows fromProcessor::normalizeConfig() and fixes a bug actually, same as whatfixXmlConfig() does, but when no plural form exists)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Should we put\is_string($middleware) || !\is_int(key($middleware)) insideifTrue() instead of usingalways()? Would be consistent with the rest of the nodes I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
makes sense, updated
5c6b0b6 to3cab9b2CompareUh oh!
There was an error while loading.Please reload this page.
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.phpShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
| $handler($envelope->getMessage()); | ||
| $next($envelope); | ||
| }elseif (!$this->allowNoHandlers) { | ||
| thrownewNoHandlerForMessageException(sprintf('No handler for message "%s".',\get_class($envelope->getMessage()))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
will become$envelope->getDispatchKey() ?? \get_class($envelope->getMessage() right
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Correct, one PR or the other will need a rebase once merged.
src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsdShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
3cab9b2 toe447a5fCompare… argument on HandleMessageMiddleware
e447a5f toaedb281Comparesroze commentedOct 25, 2018
For the records, now, to allow no handlers on a given bus, here is the following configuration: framework:messenger:buses:events:default_middleware:allow_no_handlers |
sroze commentedOct 25, 2018
Thank you@nicolas-grekas. |
…f a constructor argument on HandleMessageMiddleware (nicolas-grekas)This PR was merged into the 4.2-dev branch.Discussion----------[Messenger] remove AllowNoHandlerMiddleware in favor of a constructor argument on HandleMessageMiddleware| 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 | -`AllowNoHandlerMiddleware` adds a frame in the call stack but its concern should be handled by `HandleMessageMiddleware` to me, and `HandlerLocatorInterface::getHandler()` should not care about whether it's an error or not to not have a middleware (this makes it on par with `SenderLocator` in this regard.)Commits-------aedb281 [Messenger] remove AllowNoHandlerMiddleware in favor of a constructor argument on HandleMessageMiddleware
…is empty, it will be `null` (sroze)This PR was merged into the 4.2-dev branch.Discussion----------[Messenger] If `framework.messenger.buses.X.middleware` is empty, it will be `null`| Q | A| ------------- | ---| Branch? | master| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#28945| License | MIT| Doc PR | øOtherwise, it would fail with the following configuration:```yamlframework: messenger: buses: events_bus: middleware:# - 'some_commented_middleware'```Commits-------91a70fc If `framework.messenger.buses.X.middleware` is empty, it will be `null`
This PR was merged into the 4.2 branch.Discussion----------[Messenger] Fix allow_no_handlers for 4.2Fixes#10689 (comment)Ref:symfony/symfony#28945Commits-------ab2684e Fix allow_no_handers for 4.2
Uh oh!
There was an error while loading.Please reload this page.
AllowNoHandlerMiddlewareadds a frame in the call stack but its concern should be handled byHandleMessageMiddlewareto me, andHandlerLocatorInterface::getHandler()should not care about whether it's an error or not to not have a middleware (this makes it on par withSenderLocatorin this regard.)