8000 Add a messenger middleware to enforce null results by polc · Pull Request #28758 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Add a messenger middleware to enforce null results #28758

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

Add a messenger middleware to enforce null results #28758

wants to merge 5 commits into from

Conversation

polc
Copy link
Contributor
@polc polc commented Oct 7, 2018 •
edited
Loading
Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #27839
License MIT
Doc PR symfony/symfony-docs#10442

A little middleware to make sure a message bus doesn't return any result.

@polc polc requested a review from sroze as a code owner October 7, 2018 10:42
sroze
sroze suggested changes Oct 7, 2018
View reviewed changes
Copy link
Contributor
@sroze sroze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable to have this in core to me. Can you also send a documentation pull-request? 🙏

src/Symfony/Component/Messenger/Middleware/EnforceNullResultMiddleware.php Outdated
{
$result = $next($message);
if (null !== $result) {
throw new NonNullResultException(sprintf('Non null result for message %s.', \get_class($message)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

- Non null result for message %s.
+ Non null result for message "%s": at least one handler returned something but this is prohibited by this middleware.

@polc
Copy link
Contributor Author
polc commented Oct 7, 2018

Thanks for the suggestion ! Doc: symfony/symfony-docs#10442

@javiereguiluz
Copy link
Member

@sroze do you like the proposed name? EnforceNullResultMiddleware I find it a bit long and convoluted. What about NoResultMiddleware or NullResultMiddleware or ...?

@ogizanagi
Copy link
Contributor
ogizanagi commented Oct 8, 2018 •
edited
Loading

@javiereguiluz : For myself, I find EnforceNullResultMiddleware really clear about the intention. NoResultMiddleware or NullResultMiddleware describe facts, not purposes (it also seems consistent with AllowNoHandlerMiddleware).

@nicolas-grekas nicolas-grekas added this to the next milestone Oct 14, 2018
@Koc Koc mentioned this pull request Oct 18, 2018
Merged
@sroze
Copy link
Contributor
sroze commented Oct 21, 2018

This will stay the case, as the messenger bus now returns null (see #28909). Closing :)

@sroze sroze closed this Oct 21, 2018
@polc polc deleted the feature/enforce-null-result-middleware branch October 21, 2018 13:18
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.2 Nov 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sroze sroze sroze requested changes

@azjezz azjezz azjezz approved these changes

Assignees
No one assigned
Labels
Feature Messenger Status: Reviewed
Projects
None yet
Milestone
4.2
Development

Successfully merging this pull request may close these issues.
7 participants
@polc @javiereguiluz @ogizanagi @sroze @azjezz @nicolas-grekas @carsonbot
0