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

[5.0] Implemented Kernel::addMiddleware method to add middleware at runtime. #7834

Merged
merged 1 commit into from Mar 3, 2015

Conversation

jasonlewis
Copy link
Contributor

As per title and previous issue #6211.

This first checks to see if the middleware is already in the stack before adding it. This allows a custom order if it's required, otherwise it just adds it to the end.

@jasonlewis
Copy link
Contributor Author

The only other thing I was tempted to do is perhaps bind the kernel to the container as kernel, this would avoid having to do the following.

$this->app['Illuminate\Contracts\Http\Kernel']->addMiddleware('FooMiddleware');

It would instead be...

$this->app['kernel']->addMiddleware('FooMiddleware');

* Add a new middleware if it does not already exist.
*
* @param string $middleware
* @return \Illuminate\Foundation\Http\Kernel
Copy link
Member

Choose a reason for hiding this comment

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

should just @return $this i think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aye, fixed. Cheers mate.

Signed-off-by: Jason Lewis <jason.lewis1991@gmail.com>
@barryvdh
Copy link
Contributor

barryvdh commented Mar 3, 2015

Would it perhaps make sense to make it possible to determine the order of the middleware? Eg. public function addMiddleware($middleware, $prepend = false) or something, so you can add a second parameter to put the middleware in front of the list, instead of the end.

@lucasmichot
Copy link
Contributor

Makes sense @barryvdh ,
I would go further and say we should have addMiddleware, appendMiddleware, prependMiddleware

@barryvdh
Copy link
Contributor

barryvdh commented Mar 3, 2015

Wouldn't add and append have the same effect?

@lucasmichot
Copy link
Contributor

Yes mistyped addMiddleware => appendMiddleware, prependMiddleware

@jasonlewis
Copy link
Contributor Author

Yeah I was contemplating that as well. Go that route you reckon?

@taylorotwell
Copy link
Member

To keep our terms consistent we may want to use "pushMiddleware" for adding to the end (like session push method), and then prependMiddleware for adding to beginning.

@taylorotwell taylorotwell merged commit 3ea6991 into laravel:5.0 Mar 3, 2015
@kaidesu
Copy link

kaidesu commented Mar 4, 2015

pushMiddleware and prependMiddleware would be ideal for sure.

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

Successfully merging this pull request may close these issues.

None yet

6 participants