diff --git a/Slim/Error/Renderers/HtmlErrorRenderer.php b/Slim/Error/Renderers/HtmlErrorRenderer.php index 00789db21..84a29a951 100644 --- a/Slim/Error/Renderers/HtmlErrorRenderer.php +++ b/Slim/Error/Renderers/HtmlErrorRenderer.php @@ -22,7 +22,7 @@ class HtmlErrorRenderer extends AbstractErrorRenderer * @param bool $displayErrorDetails * @return string */ - public function render(Throwable $exception, bool $displayErrorDetails): string + public function __invoke(Throwable $exception, bool $displayErrorDetails): string { $title = 'Slim Application Error'; diff --git a/Slim/Error/Renderers/JsonErrorRenderer.php b/Slim/Error/Renderers/JsonErrorRenderer.php index 55c17bc0c..9236120c6 100644 --- a/Slim/Error/Renderers/JsonErrorRenderer.php +++ b/Slim/Error/Renderers/JsonErrorRenderer.php @@ -22,7 +22,7 @@ class JsonErrorRenderer extends AbstractErrorRenderer * @param bool $displayErrorDetails * @return string */ - public function render(Throwable $exception, bool $displayErrorDetails): string + public function __invoke(Throwable $exception, bool $displayErrorDetails): string { $error = ['message' => $exception->getMessage()]; diff --git a/Slim/Error/Renderers/PlainTextErrorRenderer.php b/Slim/Error/Renderers/PlainTextErrorRenderer.php index 141efcec9..de3142cb9 100644 --- a/Slim/Error/Renderers/PlainTextErrorRenderer.php +++ b/Slim/Error/Renderers/PlainTextErrorRenderer.php @@ -22,7 +22,7 @@ class PlainTextErrorRenderer extends AbstractErrorRenderer * @param bool $displayErrorDetails * @return string */ - public function render(Throwable $exception, bool $displayErrorDetails): string + public function __invoke(Throwable $exception, bool $displayErrorDetails): string { $text = "Slim Application Error:\n"; $text .= $this->formatExceptionFragment($exception); diff --git a/Slim/Error/Renderers/XmlErrorRenderer.php b/Slim/Error/Renderers/XmlErrorRenderer.php index 11ff065df..f8ff683d7 100644 --- a/Slim/Error/Renderers/XmlErrorRenderer.php +++ b/Slim/Error/Renderers/XmlErrorRenderer.php @@ -22,7 +22,7 @@ class XmlErrorRenderer extends AbstractErrorRenderer * @param bool $displayErrorDetails * @return string */ - public function render(Throwable $exception, bool $displayErrorDetails): string + public function __invoke(Throwable $exception, bool $displayErrorDetails): string { $xml = "\n {$exception->getMessage()}\n"; diff --git a/Slim/Handlers/ErrorHandler.php b/Slim/Handlers/ErrorHandler.php index 273ae5919..5db4b5a2b 100644 --- a/Slim/Handlers/ErrorHandler.php +++ b/Slim/Handlers/ErrorHandler.php @@ -19,6 +19,7 @@ use Slim\Error\Renderers\XmlErrorRenderer; use Slim\Exception\HttpException; use Slim\Exception\HttpMethodNotAllowedException; +use Slim\Interfaces\CallableResolverInterface; use Slim\Interfaces\ErrorHandlerInterface; use Slim\Interfaces\ErrorRendererInterface; use Throwable; @@ -32,12 +33,12 @@ class ErrorHandler implements ErrorHandlerInterface { /** - * @var string + * @var ErrorRendererInterface|string|callable */ protected $defaultErrorRenderer = HtmlErrorRenderer::class; /** - * @var string[] + * @var array */ protected $errorRenderers = [ 'application/json' => JsonErrorRenderer::class, @@ -83,14 +84,14 @@ class ErrorHandler implements ErrorHandlerInterface protected $exception; /** - * @var ErrorRendererInterface|string|null + * @var int */ - protected $renderer; + protected $statusCode; /** - * @var int + * @var CallableResolverInterface */ - protected $statusCode; + protected $callableResolver; /** * @var ResponseFactoryInterface @@ -98,10 +99,12 @@ class ErrorHandler implements ErrorHandlerInterface protected $responseFactory; /** - * @param ResponseFactoryInterface $responseFactory + * @param CallableResolverInterface $callableResolver + * @param ResponseFactoryInterface $responseFactory */ - public function __construct(ResponseFactoryInterface $responseFactory) + public function __construct(CallableResolverInterface $callableResolver, ResponseFactoryInterface $responseFactory) { + $this->callableResolver = $callableResolver; $this->responseFactory = $responseFactory; } @@ -131,7 +134,6 @@ public function __invoke( $this->method = $request->getMethod(); $this->statusCode = $this->determineStatusCode(); $this->contentType = $this->determineContentType($request); - $this->renderer = $this->determineRenderer(); if ($logErrors) { $this->writeToErrorLog(); @@ -201,46 +203,29 @@ protected function determineContentType(ServerRequestInterface $request): string /** * Determine which renderer to use based on content type - * Overloaded $renderer from calling class takes precedence over all * - * @return ErrorRendererInterface + * @return callable * * @throws RuntimeException */ - protected function determineRenderer(): ErrorRendererInterface + protected function determineRenderer(): callable { - $renderer = $this->renderer; - - if ($renderer !== null - && ( - (is_string($renderer) && !class_exists($renderer)) - || !in_array(ErrorRendererInterface::class, class_implements($renderer), true) - ) - ) { - throw new RuntimeException( - 'Non compliant error renderer provided (%s). ' . - 'Renderer must implement the ErrorRendererInterface' - ); - } - - if ($renderer === null) { - if (array_key_exists($this->contentType, $this->errorRenderers)) { - $renderer = $this->errorRenderers[$this->contentType]; - } else { - $renderer = $this->defaultErrorRenderer; - } + if (array_key_exists($this->contentType, $this->errorRenderers)) { + $renderer = $this->errorRenderers[$this->contentType]; + } else { + $renderer = $this->defaultErrorRenderer; } - return new $renderer(); + return $this->callableResolver->resolve($renderer); } /** * Register an error renderer for a specific content-type * - * @param string $contentType The content-type this renderer should be registered to - * @param string $errorRenderer The error renderer class name + * @param string $contentType The content-type this renderer should be registered to + * @param ErrorRendererInterface|string|callable $errorRenderer The error renderer */ - public function registerErrorRenderer(string $contentType, string $errorRenderer): void + public function registerErrorRenderer(string $contentType, $errorRenderer): void { $this->errorRenderers[$contentType] = $errorRenderer; } @@ -248,9 +233,9 @@ public function registerErrorRenderer(string $contentType, string $errorRenderer /** * Set the default error renderer * - * @param string $errorRenderer The default error renderer class name + * @param ErrorRendererInterface|string|callable $errorRenderer The default error renderer */ - public function setDefaultErrorRenderer(string $errorRenderer): void + public function setDefaultErrorRenderer($errorRenderer): void { $this->defaultErrorRenderer = $errorRenderer; } @@ -263,7 +248,7 @@ public function setDefaultErrorRenderer(string $errorRenderer): void protected function writeToErrorLog(): void { $renderer = new PlainTextErrorRenderer(); - $error = $renderer->render($this->exception, $this->logErrorDetails); + $error = $renderer->__invoke($this->exception, $this->logErrorDetails); $error .= "\nView in rendered output by enabling the \"displayErrorDetails\" setting.\n"; $this->logError($error); } @@ -292,9 +277,8 @@ protected function respond(): ResponseInterface $response = $response->withHeader('Allow', $allowedMethods); } - /** @var ErrorRendererInterface $renderer */ - $renderer = $this->renderer; - $body = $renderer->render($this->exception, $this->displayErrorDetails); + $renderer = $this->determineRenderer(); + $body = call_user_func($renderer, $this->exception, $this->displayErrorDetails); $response->getBody()->write($body); return $response; diff --git a/Slim/Interfaces/ErrorRendererInterface.php b/Slim/Interfaces/ErrorRendererInterface.php index 3055c4dc1..1ec0f4da9 100644 --- a/Slim/Interfaces/ErrorRendererInterface.php +++ b/Slim/Interfaces/ErrorRendererInterface.php @@ -18,5 +18,5 @@ interface ErrorRendererInterface * @param bool $displayErrorDetails * @return string */ - public function render(Throwable $exception, bool $displayErrorDetails): string; + public function __invoke(Throwable $exception, bool $displayErrorDetails): string; } diff --git a/Slim/Middleware/ErrorMiddleware.php b/Slim/Middleware/ErrorMiddleware.php index d609cadb0..ae255fc7b 100644 --- a/Slim/Middleware/ErrorMiddleware.php +++ b/Slim/Middleware/ErrorMiddleware.php @@ -136,7 +136,7 @@ public function getDefaultErrorHandler() return $this->callableResolver->resolve($this->defaultErrorHandler); } - return new ErrorHandler($this->responseFactory); + return new ErrorHandler($this->callableResolver, $this->responseFactory); } /** diff --git a/tests/Error/AbstractErrorRendererTest.php b/tests/Error/AbstractErrorRendererTest.php index 9e1432659..e3d917494 100644 --- a/tests/Error/AbstractErrorRendererTest.php +++ b/tests/Error/AbstractErrorRendererTest.php @@ -25,7 +25,7 @@ public function testHTMLErrorRendererDisplaysErrorDetails() { $exception = new RuntimeException('Oops..'); $renderer = new HtmlErrorRenderer(); - $output = $renderer->render($exception, true); + $output = $renderer->__invoke($exception, true); $this->assertRegExp('/.*The application could not run because of the following error:.*/', $output); } @@ -34,7 +34,7 @@ public function testHTMLErrorRendererNoErrorDetails() { $exception = new RuntimeException('Oops..'); $renderer = new HtmlErrorRenderer(); - $output = $renderer->render($exception, false); + $output = $renderer->__invoke($exception, false); $this->assertRegExp('/.*A website error has occurred. Sorry for the temporary inconvenience.*/', $output); } @@ -66,7 +66,7 @@ public function testJSONErrorRendererDisplaysErrorDetails() $method->setAccessible(true); $fragment = $method->invoke($renderer, $exception); - $output = json_encode(json_decode($renderer->render($exception, true))); + $output = json_encode(json_decode($renderer->__invoke($exception, true))); $expectedString = json_encode(['message' => 'Oops..', 'exception' => [$fragment]]); $this->assertEquals($output, $expectedString); @@ -77,7 +77,7 @@ public function testJSONErrorRendererDoesNotDisplayErrorDetails() $exception = new Exception('Oops..'); $renderer = new JsonErrorRenderer(); - $output = json_encode(json_decode($renderer->render($exception, false))); + $output = json_encode(json_decode($renderer->__invoke($exception, false))); $this->assertEquals($output, json_encode(['message' => 'Oops..'])); } @@ -92,7 +92,7 @@ public function testJSONErrorRendererDisplaysPreviousError() $method = $reflectionRenderer->getMethod('formatExceptionFragment'); $method->setAccessible(true); - $output = json_encode(json_decode($renderer->render($exception, true))); + $output = json_encode(json_decode($renderer->__invoke($exception, true))); $fragments = [ $method->invoke($renderer, $exception), @@ -112,7 +112,7 @@ public function testXMLErrorRendererDisplaysErrorDetails() $renderer = new XmlErrorRenderer(); /** @var stdClass $output */ - $output = simplexml_load_string($renderer->render($exception, true)); + $output = simplexml_load_string($renderer->__invoke($exception, true)); $this->assertEquals($output->message[0], 'Ooops...'); $this->assertEquals((string) $output->exception[0]->type, 'Exception'); @@ -142,7 +142,7 @@ public function testPlainTextErrorRendererDisplaysErrorDetails() $exception = new Exception('Ooops...', 0, $previousException); $renderer = new PlainTextErrorRenderer(); - $output = $renderer->render($exception, true); + $output = $renderer->__invoke($exception, true); $this->assertRegExp('/Ooops.../', $output); } diff --git a/tests/Handlers/ErrorHandlerTest.php b/tests/Handlers/ErrorHandlerTest.php index f5f9cb74b..9863cc850 100644 --- a/tests/Handlers/ErrorHandlerTest.php +++ b/tests/Handlers/ErrorHandlerTest.php @@ -19,50 +19,10 @@ use Slim\Exception\HttpNotFoundException; use Slim\Handlers\ErrorHandler; use Slim\Tests\Mocks\MockCustomException; -use Slim\Tests\Mocks\MockErrorRenderer; use Slim\Tests\TestCase; class ErrorHandlerTest extends TestCase { - public function testDetermineContentTypeMethodDoesNotThrowExceptionWhenPassedValidRenderer() - { - $handler = $this - ->getMockBuilder(ErrorHandler::class) - ->disableOriginalConstructor() - ->getMock(); - $class = new ReflectionClass(ErrorHandler::class); - - $reflectionProperty = $class->getProperty('renderer'); - $reflectionProperty->setAccessible(true); - $reflectionProperty->setValue($handler, MockErrorRenderer::class); - - $method = $class->getMethod('determineRenderer'); - $method->setAccessible(true); - $method->invoke($handler); - - $this->addToAssertionCount(1); - } - - /** - * @expectedException \RuntimeException - */ - public function testDetermineContentTypeMethodThrowsExceptionWhenPassedAnInvalidRenderer() - { - $handler = $this - ->getMockBuilder(ErrorHandler::class) - ->disableOriginalConstructor() - ->getMock(); - $class = new ReflectionClass(ErrorHandler::class); - - $reflectionProperty = $class->getProperty('renderer'); - $reflectionProperty->setAccessible(true); - $reflectionProperty->setValue($handler, 'NonExistentRenderer::class'); - - $method = $class->getMethod('determineRenderer'); - $method->setAccessible(true); - $method->invoke($handler); - } - public function testDetermineRenderer() { $handler = $this @@ -71,6 +31,10 @@ public function testDetermineRenderer() ->getMock(); $class = new ReflectionClass(ErrorHandler::class); + $callableResolverProperty = $class->getProperty('callableResolver'); + $callableResolverProperty->setAccessible(true); + $callableResolverProperty->setValue($handler, $this->getCallableResolver()); + $reflectionProperty = $class->getProperty('contentType'); $reflectionProperty->setAccessible(true); $reflectionProperty->setValue($handler, 'application/json'); @@ -79,20 +43,24 @@ public function testDetermineRenderer() $method->setAccessible(true); $renderer = $method->invoke($handler); - $this->assertInstanceOf(JsonErrorRenderer::class, $renderer); + $this->assertIsCallable($renderer); + $this->assertInstanceOf(JsonErrorRenderer::class, $renderer[0]); $reflectionProperty->setValue($handler, 'application/xml'); $renderer = $method->invoke($handler); - $this->assertInstanceOf(XmlErrorRenderer::class, $renderer); + $this->assertIsCallable($renderer); + $this->assertInstanceOf(XmlErrorRenderer::class, $renderer[0]); $reflectionProperty->setValue($handler, 'text/plain'); $renderer = $method->invoke($handler); - $this->assertInstanceOf(PlainTextErrorRenderer::class, $renderer); + $this->assertIsCallable($renderer); + $this->assertInstanceOf(PlainTextErrorRenderer::class, $renderer[0]); // Test the default error renderer $reflectionProperty->setValue($handler, 'text/unknown'); $renderer = $method->invoke($handler); - $this->assertInstanceOf(HtmlErrorRenderer::class, $renderer); + $this->assertIsCallable($renderer); + $this->assertInstanceOf(HtmlErrorRenderer::class, $renderer[0]); } public function testDetermineStatusCode() @@ -260,7 +228,7 @@ public function testAcceptableMediaTypeIsNotFirstInList() public function testRegisterErrorRenderer() { - $handler = new ErrorHandler($this->getResponseFactory()); + $handler = new ErrorHandler($this->getCallableResolver(), $this->getResponseFactory()); $handler->registerErrorRenderer('application/slim', PlainTextErrorRenderer::class); $reflectionClass = new ReflectionClass(ErrorHandler::class); @@ -273,7 +241,7 @@ public function testRegisterErrorRenderer() public function testSetDefaultErrorRenderer() { - $handler = new ErrorHandler($this->getResponseFactory()); + $handler = new ErrorHandler($this->getCallableResolver(), $this->getResponseFactory()); $handler->setDefaultErrorRenderer(PlainTextErrorRenderer::class); $reflectionClass = new ReflectionClass(ErrorHandler::class); @@ -287,7 +255,7 @@ public function testSetDefaultErrorRenderer() public function testOptions() { $request = $this->createServerRequest('/', 'OPTIONS'); - $handler = new ErrorHandler($this->getResponseFactory()); + $handler = new ErrorHandler($this->getCallableResolver(), $this->getResponseFactory()); $exception = new HttpMethodNotAllowedException($request); $exception->setAllowedMethods(['POST', 'PUT']); @@ -306,7 +274,10 @@ public function testWriteToErrorLog() ->withHeader('Accept', 'application/json'); $handler = $this->getMockBuilder(ErrorHandler::class) - ->setConstructorArgs(['responseFactory' => $this->getResponseFactory()]) + ->setConstructorArgs([ + 'callableResolver' => $this->getCallableResolver(), + 'responseFactory' => $this->getResponseFactory(), + ]) ->setMethods(['writeToErrorLog', 'logError']) ->getMock(); diff --git a/tests/Mocks/MockErrorRenderer.php b/tests/Mocks/MockErrorRenderer.php deleted file mode 100644 index 04dcb5237..000000000 --- a/tests/Mocks/MockErrorRenderer.php +++ /dev/null @@ -1,25 +0,0 @@ -getStreamFactory(); } + /** + * @param ContainerInterface|null $container + * + * @return CallableResolverInterface + */ + protected function getCallableResolver(?ContainerInterface $container = null): CallableResolverInterface + { + return new CallableResolver($container); + } + /** * @param string $uri * @param string $method