Skip to content

Commit

Permalink
Do not instantiate the controller just for fetching the middleware
Browse files Browse the repository at this point in the history
  • Loading branch information
X-Coder264 committed Jan 7, 2022
1 parent bb4035b commit ce2412a
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 15 deletions.
4 changes: 2 additions & 2 deletions src/Illuminate/Routing/Contracts/ControllerDispatcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ public function dispatch(Route $route, $controller, $method);
/**
* Get the middleware for the controller instance.
*
* @param \Illuminate\Routing\Controller $controller
* @param string $controllerClass
* @param string $method
* @return array
*/
public function getMiddleware($controller, $method);
public function getMiddleware(string $controllerClass, string $method): array;
}
10 changes: 5 additions & 5 deletions src/Illuminate/Routing/Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ abstract class Controller
*
* @var array
*/
protected $middleware = [];
protected static $middleware = [];

/**
* Register middleware on the controller.
Expand All @@ -20,10 +20,10 @@ abstract class Controller
* @param array $options
* @return \Illuminate\Routing\ControllerMiddlewareOptions
*/
public function middleware($middleware, array $options = [])
public static function middleware($middleware, array $options = [])
{
foreach ((array) $middleware as $m) {
$this->middleware[] = [
static::$middleware[] = [
'middleware' => $m,
'options' => &$options,
];
Expand All @@ -37,9 +37,9 @@ public function middleware($middleware, array $options = [])
*
* @return array
*/
public function getMiddleware()
public static function getMiddleware()
{
return $this->middleware;
return static::$middleware;
}

/**
Expand Down
8 changes: 4 additions & 4 deletions src/Illuminate/Routing/ControllerDispatcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,17 +51,17 @@ public function dispatch(Route $route, $controller, $method)
/**
* Get the middleware for the controller instance.
*
* @param \Illuminate\Routing\Controller $controller
* @param string $controllerClass
* @param string $method
* @return array
*/
public function getMiddleware($controller, $method)
public function getMiddleware(string $controllerClass, string $method): array
{
if (! method_exists($controller, 'getMiddleware')) {
if (! method_exists($controllerClass, 'getMiddleware')) {
return [];
}

return collect($controller->getMiddleware())->reject(function ($data) use ($method) {
return collect($controllerClass::getMiddleware())->reject(function ($data) use ($method) {
return static::methodExcludedByOptions($method, $data['options']);
})->pluck('middleware')->all();
}
Expand Down
18 changes: 14 additions & 4 deletions src/Illuminate/Routing/Route.php
Original file line number Diff line number Diff line change
Expand Up @@ -270,14 +270,24 @@ protected function runController()
public function getController()
{
if (! $this->controller) {
$class = $this->parseControllerCallback()[0];

$this->controller = $this->container->make(ltrim($class, '\\'));
$this->controller = $this->container->make($this->getControllerClass());
}

return $this->controller;
}

/**
* Get the controller class for the route.
*
* @return string
*/
public function getControllerClass(): string
{
$class = $this->parseControllerCallback()[0];

return ltrim($class, '\\');
}

/**
* Get the controller method used for the route.
*
Expand Down Expand Up @@ -1068,7 +1078,7 @@ public function controllerMiddleware()
}

return $this->controllerDispatcher()->getMiddleware(
$this->getController(), $this->getControllerMethod()
$this->getControllerClass(), $this->getControllerMethod()
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

namespace Illuminate\Tests\Integration\Routing\Fixtures;

use Illuminate\Http\RedirectResponse;
use Illuminate\Http\Request;
use Illuminate\Http\Response;
use Illuminate\Routing\Controller;

class ControllerWithStaticallyDefinedMiddleware extends Controller
{
public function __construct()
{
$_SERVER['controller_with_statically_defined_middleware_was_constructed'] = true;
}

public static function getMiddleware(): array
{
static::middleware('auth');

static::middleware(function (Request $request): RedirectResponse {
return new RedirectResponse('https://www.foo.com');
});

return parent::getMiddleware();
}

public function __invoke(): Response
{
return new Response('foobar');
}
}
37 changes: 37 additions & 0 deletions tests/Integration/Routing/RoutingMiddlewareTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php

namespace Illuminate\Tests\Integration\Routing;

use Illuminate\Auth\GenericUser;
use Illuminate\Contracts\Routing\Registrar;
use Illuminate\Tests\Integration\Routing\Fixtures\ControllerWithStaticallyDefinedMiddleware;
use Orchestra\Testbench\TestCase;

final class RoutingMiddlewareTest extends TestCase
{
protected function tearDown(): void
{
unset($_SERVER['controller_with_statically_defined_middleware_was_constructed']);

parent::tearDown();
}

public function testControllerIsNotInstantiatedWhenStaticallyDefiningMiddlewareOnIt()
{
$this->withoutExceptionHandling();
$this->actingAs(new GenericUser([]));

$_SERVER['controller_with_statically_defined_middleware_was_constructed'] = false;

/** @var Registrar $router */
$router = $this->app->make(Registrar::class);

$router->get('test-route', ControllerWithStaticallyDefinedMiddleware::class);

$response = $this->get('test-route');

$response->assertRedirect('https://www.foo.com');

$this->assertFalse($_SERVER['controller_with_statically_defined_middleware_was_constructed']);
}
}

0 comments on commit ce2412a

Please sign in to comment.