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

Add a priority attribute on doctrine.middleware tag and #[AsMiddleware] attribute #1676

Merged
merged 2 commits into from
Jun 15, 2023

Conversation

Okhoshi
Copy link
Contributor

@Okhoshi Okhoshi commented Jun 14, 2023

Currently, the ordering of the middlewares is only driven by the order in which they were registered in the container, which is not very predictable and sometimes impossible to modify (if one middleware is injected by another bundle for instance). By setting the priority on the doctrine.middleware tag, this ordering can be modified more easily.

@Okhoshi Okhoshi changed the base branch from 2.10.x to 2.11.x June 14, 2023 21:11
@Okhoshi Okhoshi force-pushed the middleware_priority branch from 73e2a40 to a1d5988 Compare June 14, 2023 21:13
@dmaicher
Copy link
Contributor

From my side 👍 for that feature. I will also let @ostrolucky have a look.

For finalizing it I think we should also document it in https://github.com/doctrine/DoctrineBundle/blob/2.11.x/Resources/doc/middlewares.rst

@dmaicher dmaicher added this to the 2.11.0 milestone Jun 15, 2023
@dmaicher
Copy link
Contributor

I might also be able to leverage that here 😄

https://github.com/dmaicher/doctrine-test-bundle/blob/master/src/DAMA/DoctrineTestBundle/DependencyInjection/DoctrineTestCompilerPass.php#L81

@Okhoshi
Copy link
Contributor Author

Okhoshi commented Jun 15, 2023

I might also be able to leverage that here 😄

https://github.com/dmaicher/doctrine-test-bundle/blob/master/src/DAMA/DoctrineTestBundle/DependencyInjection/DoctrineTestCompilerPass.php#L81

Happy to see it might be useful immediately 🎉

For finalizing it I think we should also document it in https://github.com/doctrine/DoctrineBundle/blob/2.11.x/Resources/doc/middlewares.rst

Adding it now 👍

Copy link
Member

@ostrolucky ostrolucky left a comment

Choose a reason for hiding this comment

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

Looks ok, but please also document it

@dmaicher dmaicher requested a review from ostrolucky June 15, 2023 07:58
@ostrolucky ostrolucky merged commit 8d983bb into doctrine:2.11.x Jun 15, 2023
@Okhoshi Okhoshi deleted the middleware_priority branch June 15, 2023 08:13
ostrolucky pushed a commit that referenced this pull request Sep 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants