From 70a407ecc01d3566051094f996f5ea8b72a9ed7a Mon Sep 17 00:00:00 2001 From: Jacob Dreesen Date: Thu, 9 Jun 2016 22:53:31 +0200 Subject: [PATCH 1/2] Allow short circuiting the controller in a before route middleware This brings the bridge in line with the behavior of Silex. --- src/MiddlewareListener.php | 11 +++++------ tests/MiddlewareTest.php | 31 +++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/src/MiddlewareListener.php b/src/MiddlewareListener.php index b349ce0..8379b58 100644 --- a/src/MiddlewareListener.php +++ b/src/MiddlewareListener.php @@ -2,10 +2,9 @@ namespace DI\Bridge\Silex; -use DI\Bridge\Silex\Application; -use Symfony\Component\HttpKernel\Event\GetResponseEvent; -use Symfony\Component\HttpKernel\Event\FilterResponseEvent; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpKernel\Event\FilterResponseEvent; +use Symfony\Component\HttpKernel\Event\GetResponseEvent; /** * Replacement for the Silex MiddlewareListener to allow arbitrary injection into middleware functions. @@ -21,7 +20,7 @@ class MiddlewareListener extends \Silex\EventListener\MiddlewareListener /** * @param Application $app The application - * @param CallbackInvoker $callbackInvoker The invoker that handles injecting middlewares + * @param CallbackInvoker $callbackInvoker The invoker that handles injecting middleware */ public function __construct(Application $app, CallbackInvoker $callbackInvoker) { @@ -38,7 +37,6 @@ public function onKernelRequest(GetResponseEvent $event) } foreach ((array) $route->getOption('_before_middlewares') as $callback) { - $middleware = $this->app['callback_resolver']->resolveCallback($callback); $ret = $this->callbackInvoker->call($middleware, [ // type hints @@ -50,6 +48,8 @@ public function onKernelRequest(GetResponseEvent $event) if ($ret instanceof Response) { $event->setResponse($ret); + + return; } elseif (null !== $ret) { throw new \RuntimeException(sprintf('A before middleware for route "%s" returned an invalid response value. Must return null or an instance of Response.', $routeName)); } @@ -66,7 +66,6 @@ public function onKernelResponse(FilterResponseEvent $event) } foreach ((array) $route->getOption('_after_middlewares') as $callback) { - $middleware = $this->app['callback_resolver']->resolveCallback($callback); $ret = $this->callbackInvoker->call($middleware, [ // type hints diff --git a/tests/MiddlewareTest.php b/tests/MiddlewareTest.php index 055d023..7545e78 100644 --- a/tests/MiddlewareTest.php +++ b/tests/MiddlewareTest.php @@ -99,4 +99,35 @@ public function should_fall_back_to_silex_default_behaviour() $response = $application->handle($request); $this->assertEquals('Hello john', $response->getContent()); } + + /** + * @test + */ + public function should_allow_short_circuiting_the_controller_in_before_route_middleware() + { + $application = $this->createApplication(); + $request = Request::create('/'); + + $shouldBeCalled = $this->createSpyCallback(); + $shouldBeCalled->expects($this->once())->method('__invoke'); + + $shouldNotBeCalled = $this->createSpyCallback(); + $shouldNotBeCalled->expects($this->never())->method('__invoke'); + + // route middleware + $application + ->get('/', $shouldNotBeCalled) + ->before(function () { + return new Response('Expected result'); + }) + ->before($shouldNotBeCalled) + ->after($shouldBeCalled); + + $response = $application->handle($request); + $this->assertEquals('Expected result', $response->getContent()); + } + + private function createSpyCallback() { + return $this->getMockBuilder('stdClass')->setMethods(['__invoke'])->getMock(); + } } From af3ff2e7b31c22a21b3d30f731d745cca5a6c351 Mon Sep 17 00:00:00 2001 From: Jacob Dreesen Date: Thu, 9 Jun 2016 23:16:16 +0200 Subject: [PATCH 2/2] Fix short circuiting the controller in a before application middleware - Added missing use statement for the Response - Removed broken code in the finish middleware that tries to set the response (finish is called after the response has been sent) --- src/Application.php | 16 ++-------------- tests/MiddlewareTest.php | 28 ++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/src/Application.php b/src/Application.php index bcec5a6..85c6162 100644 --- a/src/Application.php +++ b/src/Application.php @@ -4,11 +4,10 @@ use DI\Bridge\Silex\Container\ContainerInteropProxy; use DI\Bridge\Silex\Controller\ControllerResolver; -use DI\Bridge\Silex\MiddlewareListener; -use DI\Bridge\Silex\ConverterListener; use Silex\EventListener\LocaleListener; use Silex\EventListener\StringToResponseListener; use Silex\LazyUrlMatcher; +use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Kernel; use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\HttpKernel\KernelEvents; @@ -21,7 +20,6 @@ use DI\ContainerBuilder; use Interop\Container\ContainerInterface; use Invoker\CallableResolver; -use Invoker\Reflection\CallableReflection; use Invoker\ParameterResolver\AssociativeArrayResolver; use Invoker\ParameterResolver\Container\TypeHintContainerResolver; use Invoker\ParameterResolver\ResolverChain; @@ -168,7 +166,6 @@ public function before($callback, $priority = 0) if ($ret instanceof Response) { $event->setResponse($ret); } - }, $priority); } @@ -197,18 +194,16 @@ public function after($callback, $priority = 0) } elseif (null !== $ret) { throw new \RuntimeException('An after middleware returned an invalid response value. Must return null or an instance of Response.'); } - }, $priority); } public function finish($callback, $priority = 0) { $this->on(KernelEvents::TERMINATE, function (PostResponseEvent $event) use ($callback) { - $request = $event->getRequest(); $response = $event->getResponse(); $middleware = $this['callback_resolver']->resolveCallback($callback); - $ret = $this->callbackInvoker->call($middleware, [ + $this->callbackInvoker->call($middleware, [ // type hints 'Symfony\Component\HttpFoundation\Request' => $request, 'Symfony\Component\HttpFoundation\Response' => $response, @@ -217,13 +212,6 @@ public function finish($callback, $priority = 0) 1 => $response, 2 => $this, ]); - - if ($ret instanceof Response) { - $event->setResponse($ret); - } elseif (null !== $ret) { - throw new \RuntimeException('An after middleware returned an invalid response value. Must return null or an instance of Response.'); - } - }, $priority); } } diff --git a/tests/MiddlewareTest.php b/tests/MiddlewareTest.php index 7545e78..99a7786 100644 --- a/tests/MiddlewareTest.php +++ b/tests/MiddlewareTest.php @@ -100,6 +100,34 @@ public function should_fall_back_to_silex_default_behaviour() $this->assertEquals('Hello john', $response->getContent()); } + /** + * @test + */ + public function should_allow_short_circuiting_the_controller_in_before_application_middleware() + { + $application = $this->createApplication(); + $request = Request::create('/'); + + $shouldBeCalled = $this->createSpyCallback(); + $shouldBeCalled->expects($this->once())->method('__invoke'); + + $shouldNotBeCalled = $this->createSpyCallback(); + $shouldNotBeCalled->expects($this->never())->method('__invoke'); + + $application->get('/', $shouldNotBeCalled); + + // application middleware + $application->before(function () { + return new Response('Expected result'); + }); + $application->before($shouldNotBeCalled); + $application->after($shouldBeCalled); + $application->finish($shouldBeCalled); + + $response = $application->handle($request); + $this->assertEquals('Expected result', $response->getContent()); + } + /** * @test */