From 42aa478996fbbdf6069861e062e58dd646e57de2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20B=C3=A9rub=C3=A9?= Date: Mon, 5 Aug 2019 12:40:13 -0600 Subject: [PATCH 1/4] add Slim callable pattern as static for re-use in MiddlewareDispatcher --- Slim/CallableResolver.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Slim/CallableResolver.php b/Slim/CallableResolver.php index 8df0f7e7e..3c4963e46 100644 --- a/Slim/CallableResolver.php +++ b/Slim/CallableResolver.php @@ -21,6 +21,11 @@ */ final class CallableResolver implements CallableResolverInterface { + /** + * @var string + */ + public static $callablePattern = '!^([^\:]+)\:([a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)$!'; + /** * @var ContainerInterface|null */ @@ -55,14 +60,13 @@ public function resolve($toResolve): callable $instance = null; $method = null; - // check for slim callable as "class:method" - $callablePattern = '!^([^\:]+)\:([a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)$!'; - if (preg_match($callablePattern, $toResolve, $matches)) { + // Check for Slim callable as `class:method` + if (preg_match(self::$callablePattern, $toResolve, $matches)) { $class = $matches[1]; $method = $matches[2]; } - if ($this->container instanceof ContainerInterface && $this->container->has($class)) { + if ($this->container && $this->container->has($class)) { $instance = $this->container->get($class); } else { if (!class_exists($class)) { @@ -91,7 +95,7 @@ public function resolve($toResolve): callable )); } - if ($this->container instanceof ContainerInterface && $resolved instanceof Closure) { + if ($this->container && $resolved instanceof Closure) { $resolved = $resolved->bindTo($this->container); } From 2066752f8a67d0ac96d6afd3ce74ceb2b5cd6250 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20B=C3=A9rub=C3=A9?= Date: Mon, 5 Aug 2019 12:40:41 -0600 Subject: [PATCH 2/4] add Slim callable support for middleware dispatcher deffered callables and bind closures to container --- Slim/MiddlewareDispatcher.php | 58 ++++++++++++++++++++++++++--------- 1 file changed, 44 insertions(+), 14 deletions(-) diff --git a/Slim/MiddlewareDispatcher.php b/Slim/MiddlewareDispatcher.php index 9a032a86b..73b71645f 100644 --- a/Slim/MiddlewareDispatcher.php +++ b/Slim/MiddlewareDispatcher.php @@ -9,6 +9,7 @@ namespace Slim; +use Closure; use Psr\Container\ContainerInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; @@ -159,24 +160,48 @@ public function __construct( public function handle(ServerRequestInterface $request): ResponseInterface { $resolved = $this->middleware; - if ($this->container && $this->container->has($this->middleware)) { - $resolved = $this->container->get($this->middleware); - if ($resolved instanceof MiddlewareInterface) { - return $resolved->process($request, $this->next); + $instance = null; + $method = null; + + // Check for Slim callable as `class:method` + if (preg_match(CallableResolver::$callablePattern, $resolved, $matches)) { + $resolved = $matches[1]; + $method = $matches[2]; + } + + if ($this->container && $this->container->has($resolved)) { + $instance = $this->container->get($resolved); + if ($instance instanceof MiddlewareInterface) { + return $instance->process($request, $this->next); + } + } else if (!function_exists($resolved)) { + if (!class_exists($resolved)) { + throw new RuntimeException(sprintf('Middleware %s does not exist', $resolved)); } + $instance = new $resolved($this->container); + } + + if ($instance && $instance instanceof MiddlewareInterface) { + return $instance->process($request, $this->next); + } + + $callable = $instance ?? $resolved; + if ($instance && $method) { + $callable = [$instance, $method]; } - if (is_subclass_of($resolved, MiddlewareInterface::class)) { - /** @var MiddlewareInterface $resolved */ - $resolved = new $resolved($this->container); - return $resolved->process($request, $this->next); + + if (!is_callable($callable)) { + throw new RuntimeException(sprintf( + 'Middleware %s is not resolvable', + $this->middleware + )); } - if (is_callable($resolved)) { - return $resolved($request, $this->next); + + if ($this->container && $callable instanceof Closure) { + $callable = $callable->bindTo($this->container); } - throw new RuntimeException(sprintf( - '%s is not resolvable', - $this->middleware - )); + + return $callable($request, $this->next); } }; @@ -196,6 +221,11 @@ public function handle(ServerRequestInterface $request): ResponseInterface public function addCallable(callable $middleware): self { $next = $this->tip; + + if ($this->container && $middleware instanceof Closure) { + $middleware = $middleware->bindTo($this->container); + } + $this->tip = new class($middleware, $next) implements RequestHandlerInterface { private $middleware; From 7613c9208cc79f2da53714a57519807cb86dca19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20B=C3=A9rub=C3=A9?= Date: Mon, 5 Aug 2019 12:41:06 -0600 Subject: [PATCH 3/4] add test coverage for Slim callable and closure binding --- tests/MiddlewareDispatcherTest.php | 56 ++++++++++++++++++- tests/Mocks/MockMiddlewareSlimCallable.php | 27 +++++++++ tests/Mocks/MockMiddlewareWithConstructor.php | 6 ++ 3 files changed, 87 insertions(+), 2 deletions(-) create mode 100644 tests/Mocks/MockMiddlewareSlimCallable.php diff --git a/tests/MiddlewareDispatcherTest.php b/tests/MiddlewareDispatcherTest.php index 41bde6224..7ad56740c 100644 --- a/tests/MiddlewareDispatcherTest.php +++ b/tests/MiddlewareDispatcherTest.php @@ -16,6 +16,7 @@ use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; use Slim\MiddlewareDispatcher; +use Slim\Tests\Mocks\MockMiddlewareSlimCallable; use Slim\Tests\Mocks\MockMiddlewareWithConstructor; use Slim\Tests\Mocks\MockMiddlewareWithoutConstructor; use Slim\Tests\Mocks\MockRequestHandler; @@ -74,6 +75,57 @@ public function testDeferredResolvedCallable() $this->assertEquals(1, $handler->getCalledCount()); } + public function testDeferredResolvedSlimCallable() + { + $handler = new MockRequestHandler(); + $middlewareDispatcher = new MiddlewareDispatcher($handler, null); + $middlewareDispatcher->addDeferred(MockMiddlewareSlimCallable::class . ':custom'); + + $request = $this->createServerRequest('/'); + $middlewareDispatcher->handle($request); + + $this->assertEquals(1, $handler->getCalledCount()); + } + + public function testDeferredResolvedClosureIsBoundToContainer() + { + $containerProphecy = $this->prophesize(ContainerInterface::class); + + $self = $this; + $callable = function (ServerRequestInterface $request, RequestHandlerInterface $handler) use ($self, $containerProphecy) { + $self->assertSame($containerProphecy->reveal(), $this); + return $handler->handle($request); + }; + + $containerProphecy->has('callable')->willReturn(true); + $containerProphecy->get('callable')->willReturn($callable); + + $handler = new MockRequestHandler(); + $middlewareDispatcher = new MiddlewareDispatcher($handler, $containerProphecy->reveal()); + $middlewareDispatcher->addDeferred('callable'); + + $request = $this->createServerRequest('/'); + $middlewareDispatcher->handle($request); + } + + public function testAddCallableBindsClosureToContainer() + { + $containerProphecy = $this->prophesize(ContainerInterface::class); + + $self = $this; + $callable = function (ServerRequestInterface $request, RequestHandlerInterface $handler) use ($self, $containerProphecy) { + $self->assertSame($containerProphecy->reveal(), $this); + return $handler->handle($request); + }; + + $handler = new MockRequestHandler(); + $middlewareDispatcher = new MiddlewareDispatcher($handler, $containerProphecy->reveal()); + $middlewareDispatcher->addCallable($callable); + + $request = $this->createServerRequest('/'); + $middlewareDispatcher->handle($request); + } + public function testResolvableReturnsInstantiatedObject() { MockMiddlewareWithoutConstructor::$CalledCount = 0; @@ -91,7 +143,7 @@ public function testResolvableReturnsInstantiatedObject() /** * @expectedException \RuntimeException - * @expectedExceptionMessage MiddlewareInterfaceNotImplemented is not resolvable + * @expectedExceptionMessage Middleware MiddlewareInterfaceNotImplemented is not resolvable */ public function testResolveThrowsExceptionWhenResolvableDoesNotImplementMiddlewareInterface() { @@ -109,7 +161,7 @@ public function testResolveThrowsExceptionWhenResolvableDoesNotImplementMiddlewa /** * @expectedException \RuntimeException - * @expectedExceptionMessage Unresolvable::class is not resolvable + * @expectedExceptionMessage Middleware Unresolvable::class does not exist */ public function testResolveThrowsExceptionWithoutContainerAndUnresolvableClass() { diff --git a/tests/Mocks/MockMiddlewareSlimCallable.php b/tests/Mocks/MockMiddlewareSlimCallable.php new file mode 100644 index 000000000..ca165ba25 --- /dev/null +++ b/tests/Mocks/MockMiddlewareSlimCallable.php @@ -0,0 +1,27 @@ +handle($request); + } +} diff --git a/tests/Mocks/MockMiddlewareWithConstructor.php b/tests/Mocks/MockMiddlewareWithConstructor.php index e47bf7b1d..89592cb2e 100644 --- a/tests/Mocks/MockMiddlewareWithConstructor.php +++ b/tests/Mocks/MockMiddlewareWithConstructor.php @@ -1,4 +1,10 @@ Date: Mon, 5 Aug 2019 12:42:54 -0600 Subject: [PATCH 4/4] fix code style errors --- Slim/MiddlewareDispatcher.php | 2 +- tests/MiddlewareDispatcherTest.php | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/Slim/MiddlewareDispatcher.php b/Slim/MiddlewareDispatcher.php index 73b71645f..7d4906da1 100644 --- a/Slim/MiddlewareDispatcher.php +++ b/Slim/MiddlewareDispatcher.php @@ -174,7 +174,7 @@ public function handle(ServerRequestInterface $request): ResponseInterface if ($instance instanceof MiddlewareInterface) { return $instance->process($request, $this->next); } - } else if (!function_exists($resolved)) { + } elseif (!function_exists($resolved)) { if (!class_exists($resolved)) { throw new RuntimeException(sprintf('Middleware %s does not exist', $resolved)); } diff --git a/tests/MiddlewareDispatcherTest.php b/tests/MiddlewareDispatcherTest.php index 7ad56740c..9b51c5ce5 100644 --- a/tests/MiddlewareDispatcherTest.php +++ b/tests/MiddlewareDispatcherTest.php @@ -92,7 +92,13 @@ public function testDeferredResolvedClosureIsBoundToContainer() $containerProphecy = $this->prophesize(ContainerInterface::class); $self = $this; - $callable = function (ServerRequestInterface $request, RequestHandlerInterface $handler) use ($self, $containerProphecy) { + $callable = function ( + ServerRequestInterface $request, + RequestHandlerInterface $handler + ) use ( + $self, + $containerProphecy + ) { $self->assertSame($containerProphecy->reveal(), $this); return $handler->handle($request); }; @@ -113,7 +119,13 @@ public function testAddCallableBindsClosureToContainer() $containerProphecy = $this->prophesize(ContainerInterface::class); $self = $this; - $callable = function (ServerRequestInterface $request, RequestHandlerInterface $handler) use ($self, $containerProphecy) { + $callable = function ( + ServerRequestInterface $request, + RequestHandlerInterface $handler + ) use ( + $self, + $containerProphecy + ) { $self->assertSame($containerProphecy->reveal(), $this); return $handler->handle($request); };