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.3] Single middleware pipeline #14097

Merged
merged 2 commits into from
Jun 22, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/Illuminate/Foundation/Http/Kernel.php
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ protected function sendRequestThroughRouter($request)
public function terminate($request, $response)
{
$middlewares = $this->app->shouldSkipMiddleware() ? [] : array_merge(
$this->gatherRouteMiddlewares($request),
$this->gatherRouteMiddleware($request),
$this->middleware
);

Expand All @@ -182,10 +182,10 @@ public function terminate($request, $response)
* @param \Illuminate\Http\Request $request
* @return array
*/
protected function gatherRouteMiddlewares($request)
protected function gatherRouteMiddleware($request)
{
if ($route = $request->route()) {
return $this->router->gatherRouteMiddlewares($route);
return $this->router->gatherRouteMiddleware($route);
}

return [];
Expand Down
101 changes: 19 additions & 82 deletions src/Illuminate/Routing/ControllerDispatcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,6 @@

namespace Illuminate\Routing;

use Illuminate\Http\Request;
use Illuminate\Support\Collection;
use Illuminate\Container\Container;

class ControllerDispatcher
{
use RouteDependencyResolverTrait;
Expand All @@ -17,83 +13,51 @@ class ControllerDispatcher
*/
protected $router;

/**
* The IoC container instance.
*
* @var \Illuminate\Container\Container
*/
protected $container;

/**
* Create a new controller dispatcher instance.
*
* @param \Illuminate\Routing\Router $router
* @param \Illuminate\Container\Container $container
* @return void
*/
public function __construct(Router $router,
Container $container = null)
public function __construct(Router $router)
{
$this->router = $router;
$this->container = $container;
}
Copy link
Member

Choose a reason for hiding this comment

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

https://github.com/laravel/framework/blob/5.3/src/Illuminate/Routing/RouteDependencyResolverTrait.php#L85

RouteDependencyResolverTrait is using $container from ControllerDispatcher.

Copy link
Member

Choose a reason for hiding this comment

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

screen shot 2016-06-22 at 10 11 20 pm

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed here: #14108


/**
* Dispatch a request to a given controller and method.
*
* @param \Illuminate\Routing\Route $route
* @param \Illuminate\Http\Request $request
* @param string $controller
* @param mixed $controller
* @param string $method
* @return mixed
*/
public function dispatch(Route $route, Request $request, $controller, $method)
{
$instance = $this->makeController($controller);

return $this->callWithinStack($instance, $route, $request, $method);
}

/**
* Make a controller instance via the IoC container.
*
* @param string $controller
* @return mixed
*/
protected function makeController($controller)
public function dispatch(Route $route, $controller, $method)
{
Controller::setRouter($this->router);

return $this->container->make($controller);
return $this->call($controller, $route, $method);
}

/**
* Call the given controller instance method.
*
* @param \Illuminate\Routing\Controller $instance
* @param \Illuminate\Routing\Controller $controller
* @param \Illuminate\Routing\Route $route
* @param \Illuminate\Http\Request $request
* @param string $method
* @return mixed
*/
protected function callWithinStack($instance, $route, $request, $method)
protected function call($controller, $route, $method)
{
$shouldSkipMiddleware = $this->container->bound('middleware.disable') &&
$this->container->make('middleware.disable') === true;
$parameters = $this->resolveClassMethodDependencies(
$route->parametersWithoutNulls(), $controller, $method
);

$middleware = $shouldSkipMiddleware ? [] : $this->getMiddleware($instance, $method);
if (method_exists($controller, 'callAction')) {
return $controller->callAction($method, $parameters);
}

// Here we will make a stack onion instance to execute this request in, which gives
// us the ability to define middlewares on controllers. We will return the given
// response back out so that "after" filters can be run after the middlewares.
return (new Pipeline($this->container))
->send($request)
->through($middleware)
->then(function ($request) use ($instance, $route, $method) {
return $this->router->prepareResponse(
$request, $this->call($instance, $route, $method)
);
});
return call_user_func_array([$controller, $method], $parameters);
}

/**
Expand All @@ -103,21 +67,15 @@ protected function callWithinStack($instance, $route, $request, $method)
* @param string $method
* @return array
*/
public function getMiddleware($instance, $method)
public static function getMiddleware($controller, $method)
{
$results = new Collection;

if (! method_exists($instance, 'getMiddleware')) {
if (! method_exists($controller, 'getMiddleware')) {
return [];
}

foreach ($instance->getMiddleware() as $name => $options) {
if (! $this->methodExcludedByOptions($method, $options)) {
$results[] = $this->router->resolveMiddlewareClassName($name);
}
}

return $results->flatten()->all();
return collect($controller->getMiddleware())->reject(function ($options, $name) use ($method) {
return static::methodExcludedByOptions($method, $options);
})->keys()->all();
}

/**
Expand All @@ -127,30 +85,9 @@ public function getMiddleware($instance, $method)
* @param array $options
* @return bool
*/
public function methodExcludedByOptions($method, array $options)
protected static function methodExcludedByOptions($method, array $options)
{
return (isset($options['only']) && ! in_array($method, (array) $options['only'])) ||
(! empty($options['except']) && in_array($method, (array) $options['except']));
}

/**
* Call the given controller instance method.
*
* @param \Illuminate\Routing\Controller $instance
* @param \Illuminate\Routing\Route $route
* @param string $method
* @return mixed
*/
protected function call($instance, $route, $method)
{
$parameters = $this->resolveClassMethodDependencies(
$route->parametersWithoutNulls(), $instance, $method
);

if (method_exists($instance, 'callAction')) {
return $instance->callAction($method, $parameters);
}

return call_user_func_array([$instance, $method], $parameters);
}
}
83 changes: 66 additions & 17 deletions src/Illuminate/Routing/Route.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ class Route
*/
protected $router;

/**
* The controller instance.
*
* @var mixed
*/
protected $controller;

/**
* The container instance used by the route.
*
Expand Down Expand Up @@ -133,23 +140,32 @@ public function run(Request $request)
$this->container = $this->container ?: new Container;

try {
if (! is_string($this->action['uses'])) {
return $this->runCallable($request);
if ($this->isControllerAction()) {
return $this->runController();
}

return $this->runController($request);
return $this->runCallable();
} catch (HttpResponseException $e) {
return $e->getResponse();
}
}

/**
* Checks whether the route's action is a controller.
*
* @return bool
*/
protected function isControllerAction()
{
return is_string($this->action['uses']);
}

/**
* Run the route action and return the response.
*
* @param \Illuminate\Http\Request $request
* @return mixed
*/
protected function runCallable(Request $request)
protected function runCallable()
{
$parameters = $this->resolveMethodDependencies(
$this->parametersWithoutNulls(), new ReflectionFunction($this->action['uses'])
Expand All @@ -161,17 +177,41 @@ protected function runCallable(Request $request)
/**
* Run the route action and return the response.
*
* @param \Illuminate\Http\Request $request
* @return mixed
*
* @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException
*/
protected function runController(Request $request)
protected function runController()
{
return (new ControllerDispatcher($this->router))->dispatch(
$this, $this->getController(), $this->getControllerMethod()
);
}

/**
* Get the controller instance for the route.
*
* @return mixed
*/
protected function getController()
{
list($class, $method) = explode('@', $this->action['uses']);
list($class) = explode('@', $this->action['uses']);

if (! $this->controller) {
$this->controller = $this->container->make($class);
}

return $this->controller;
}

return (new ControllerDispatcher($this->router, $this->container))
->dispatch($this, $request, $class, $method);
/**
* Get the controller method used for the route.
*
* @return string
*/
protected function getControllerMethod()
{
return explode('@', $this->action['uses'])[1];
}

/**
Expand Down Expand Up @@ -226,6 +266,16 @@ protected function extractOptionalParameters()
return isset($matches[1]) ? array_fill_keys($matches[1], null) : [];
}

/**
* Get all middleware, including the ones from the controller.
*
* @return array
*/
public function gatherMiddleware()
{
return array_merge($this->middleware(), $this->controllerMiddleware());
}

/**
* Get or set the middlewares attached to the route.
*
Expand All @@ -250,18 +300,17 @@ public function middleware($middleware = null)
}

/**
* Get the controller middleware for the route.
* Get the middleware for the route's controller.
*
* @return array
*/
protected function controllerMiddleware()
public function controllerMiddleware()
{
list($class, $method) = explode('@', $this->action['uses']);

$controller = $this->container->make($class);
if (! $this->isControllerAction()) {
return [];
}

return (new ControllerDispatcher($this->router, $this->container))
->getMiddleware($controller, $method);
return ControllerDispatcher::getMiddleware($this->getController(), $this->getControllerMethod());
}

/**
Expand Down
15 changes: 6 additions & 9 deletions src/Illuminate/Routing/Router.php
Original file line number Diff line number Diff line change
Expand Up @@ -592,9 +592,7 @@ public function dispatch(Request $request)
{
$this->currentRequest = $request;

$response = $this->dispatchToRoute($request);

return $this->prepareResponse($request, $response);
return $this->dispatchToRoute($request);
}

/**
Expand Down Expand Up @@ -633,15 +631,14 @@ protected function runRouteWithinStack(Route $route, Request $request)
$shouldSkipMiddleware = $this->container->bound('middleware.disable') &&
$this->container->make('middleware.disable') === true;

$middleware = $shouldSkipMiddleware ? [] : $this->gatherRouteMiddlewares($route);
$middleware = $shouldSkipMiddleware ? [] : $this->gatherRouteMiddleware($route);

return (new Pipeline($this->container))
->send($request)
->through($middleware)
->then(function ($request) use ($route) {
return $this->prepareResponse(
$request,
$route->run($request)
$request, $route->run($request)
);
});
}
Expand All @@ -652,10 +649,10 @@ protected function runRouteWithinStack(Route $route, Request $request)
* @param \Illuminate\Routing\Route $route
* @return array
*/
public function gatherRouteMiddlewares(Route $route)
public function gatherRouteMiddleware(Route $route)
{
$middleware = Collection::make($route->middleware())->map(function ($name) {
return Collection::make($this->resolveMiddlewareClassName($name));
$middleware = collect($route->gatherMiddleware())->map(function ($name) {
return (array) $this->resolveMiddlewareClassName($name);
})->flatten();

return $this->sortMiddleware($middleware)->all();
Expand Down
2 changes: 1 addition & 1 deletion tests/Routing/RoutingRouteTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,7 @@ public function testMiddlewarePrioritySorting()
SubstituteBindings::class,
Placeholder2::class,
Placeholder3::class,
], $router->gatherRouteMiddlewares($route));
], $router->gatherRouteMiddleware($route));
}

public function testModelBinding()
Expand Down