Skip to content

Commit

Permalink
Merge 3309cb2 into 6d6d34e
Browse files Browse the repository at this point in the history
  • Loading branch information
adriansuter committed Jun 26, 2019
2 parents 6d6d34e + 3309cb2 commit 275863b
Show file tree
Hide file tree
Showing 11 changed files with 71 additions and 128 deletions.
2 changes: 1 addition & 1 deletion Slim/Error/Renderers/HtmlErrorRenderer.php
Expand Up @@ -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';

Expand Down
2 changes: 1 addition & 1 deletion Slim/Error/Renderers/JsonErrorRenderer.php
Expand Up @@ -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()];

Expand Down
2 changes: 1 addition & 1 deletion Slim/Error/Renderers/PlainTextErrorRenderer.php
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion Slim/Error/Renderers/XmlErrorRenderer.php
Expand Up @@ -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 = "<error>\n <message>{$exception->getMessage()}</message>\n";

Expand Down
68 changes: 26 additions & 42 deletions Slim/Handlers/ErrorHandler.php
Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -83,25 +84,27 @@ 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
*/
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;
}

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -201,56 +203,39 @@ 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;
}

/**
* 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;
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion Slim/Interfaces/ErrorRendererInterface.php
Expand Up @@ -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;
}
2 changes: 1 addition & 1 deletion Slim/Middleware/ErrorMiddleware.php
Expand Up @@ -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);
}

/**
Expand Down
14 changes: 7 additions & 7 deletions tests/Error/AbstractErrorRendererTest.php
Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
Expand All @@ -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..']));
}
Expand All @@ -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),
Expand All @@ -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');
Expand Down Expand Up @@ -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);
}
Expand Down
67 changes: 19 additions & 48 deletions tests/Handlers/ErrorHandlerTest.php
Expand Up @@ -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
Expand All @@ -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');
Expand All @@ -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()
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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']);

Expand All @@ -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();

Expand Down

0 comments on commit 275863b

Please sign in to comment.