Allow specifying middlewares at the controller level. #15558
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ADmad
force-pushed
the
controller-middlewares
branch
3 times, most recently
from
5a68500
to
33e6953
Compare
othercorey
reviewed
May 26, 2021
markstory
reviewed
May 27, 2021
| * @return void | ||
| * @psalm-var array{?only: string|array, ?except: string|array} | ||
| */ | ||
| public function middleware($middleware, array $options = []) |
There was a problem hiding this comment.
Should this method raise an exception if it is called after middleware have already been applied? It would be frustrating to use this method in a beforeRender and then think that Cake is 'broken' when you're using it wrong.
There was a problem hiding this comment.
Could be start with just documenting the fact that middlewares must be set from initialize() only?
I don't think we would be able to check if the middleware queue was already created without adding a new property.
There was a problem hiding this comment.
Yeah we would need another property to track whether or not middleware could be added.
While we can setup route based middlewares at times setting them can be a bit cumbersome. If for e.g. you want a middleware to run for just one action you would have to setup an extra route for it. Middlewares for controllers provides a good conveninece and also replacing some components with middlewares.
ADmad
force-pushed
the
controller-middlewares
branch
from
33e6953
to
32a5ab3
Compare
|
Can this be merged? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While we can setup route based middlewares at times setting them can be
a bit cumbersome. If for e.g. you want a middleware to run for just one
action you would have to setup an extra route for it.
Middlewares for controllers provides a good convenience and also allow replacing
some components with middlewares.
P. S. Laravel allows setting up middleware for controller in a similar way.