Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Messenger] Activation middleware decorator #27320

Merged
merged 1 commit into from Jul 8, 2018
Merged

[Messenger] Activation middleware decorator #27320

merged 1 commit into from Jul 8, 2018

Conversation

ogizanagi
Copy link
Member

@ogizanagi ogizanagi commented May 20, 2018

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets part of #26901
License MIT
Doc PR TODO: symfony/symfony-docs#10035

A small middleware decorator that can be wired using DI decoration to enable/disable a middleware on an arbitrary condition. This can be used to keep the same middleware stack across env but enable/disable some of them through this.

@sroze
Copy link
Contributor

sroze commented May 30, 2018

How would someone use it? Manually, as in registers a service that decorates the generated middleware?

@ogizanagi
Copy link
Member Author

@sroze : Exactly. It'll just leverage DI capabilities.

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.

Makes sense to me 👍 (we definitely need a documentation PR for this though :))

/**
* @param Envelope $message
*/
public function handle($message, callable $next)
Copy link
Contributor

Choose a reason for hiding this comment

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

PHP allows you to rename $message to $envelope here, right?

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

LGTM, with one naming question.

* file that was distributed with this source code.
*/

namespace Symfony\Component\Messenger\Middleware\Enhancers;
Copy link
Member

Choose a reason for hiding this comment

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

Is the Enhancers sub-namespace required? Nesting hurts discovery IMHO, better remove it?

Copy link
Member Author

Choose a reason for hiding this comment

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

This was mainly echoing Symfony\Component\Messenger\Transport\Enhancers which are decorators. But of course I'm fine with removing this namespace.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd keep it that way; it helps when exploring the code base I believe.

@sroze
Copy link
Contributor

sroze commented Jul 8, 2018

Thank you @ogizanagi.

@sroze sroze merged commit 6e43838 into symfony:master Jul 8, 2018
sroze added a commit that referenced this pull request Jul 8, 2018
This PR was merged into the 4.2-dev branch.

Discussion
----------

[Messenger] Activation middleware decorator

| Q             | A
| ------------- | ---
| Branch?       | master <!-- see below -->
| Bug fix?      | no
| New feature?  | yes <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | part of #26901   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | TODO

A small middleware decorator that can be wired using DI decoration to enable/disable a middleware on an arbitrary condition. This can be used to keep the same middleware stack across env but enable/disable some of them through this.

Commits
-------

6e43838 [Messenger] Activation middleware decorator
@ogizanagi ogizanagi deleted the messenger/activation_middleware_decorator branch July 8, 2018 09:27
@kbond
Copy link
Member

kbond commented Jul 9, 2018

Is there a doc PR for this?

@ogizanagi
Copy link
Member Author

ogizanagi commented Jul 9, 2018

@kbond : Not yet. Feel free to open one if you want. Otherwise it'll go in my TODO list anyway :)

EDIT: I've created symfony/symfony-docs#10035 to help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants