From 319a8c6370c05bdde3603cb69c2a1916cc232f5b Mon Sep 17 00:00:00 2001 From: Ryan Jentzsch Date: Tue, 13 Aug 2019 14:13:57 -0600 Subject: [PATCH 1/7] Add setLogErrorRenderer() to ErrorHandler Closes #2797 --- Slim/Handlers/ErrorHandler.php | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/Slim/Handlers/ErrorHandler.php b/Slim/Handlers/ErrorHandler.php index f1975b533..7dd553caf 100644 --- a/Slim/Handlers/ErrorHandler.php +++ b/Slim/Handlers/ErrorHandler.php @@ -53,6 +53,11 @@ class ErrorHandler implements ErrorHandlerInterface 'text/plain' => PlainTextErrorRenderer::class, ]; + /** + * @var ErrorRendererInterface|string|callable + */ + protected $logErrorRenderer = PlainTextErrorRenderer::class; + /** * @var bool */ @@ -259,6 +264,16 @@ public function setDefaultErrorRenderer(string $contentType, $errorRenderer): vo $this->defaultErrorRenderer = $errorRenderer; } + /** + * Set the renderer for the error logger + * + * @param ErrorRendererInterface|string|callable $logErrorRenderer + */ + public function setLogErrorRenderer($logErrorRenderer): void + { + $this->logErrorRenderer = $logErrorRenderer; + } + /** * Write to the error log if $logErrors has been set to true * @@ -266,7 +281,8 @@ public function setDefaultErrorRenderer(string $contentType, $errorRenderer): vo */ protected function writeToErrorLog(): void { - $renderer = new PlainTextErrorRenderer(); + /** @var ErrorRendererInterface $renderer */ + $renderer = $this->callableResolver->resolve($this->logErrorRenderer); $error = $renderer->__invoke($this->exception, $this->logErrorDetails); $error .= "\nView in rendered output by enabling the \"displayErrorDetails\" setting.\n"; $this->logError($error); From f8ac89b8c6c8c48fdb88eb1344e8d08e0d5a7e01 Mon Sep 17 00:00:00 2001 From: Ryan Jentzsch Date: Tue, 13 Aug 2019 17:12:01 -0600 Subject: [PATCH 2/7] Fix $renderer calling `->invoke()` directly. --- Slim/Handlers/ErrorHandler.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Slim/Handlers/ErrorHandler.php b/Slim/Handlers/ErrorHandler.php index 7dd553caf..550634244 100644 --- a/Slim/Handlers/ErrorHandler.php +++ b/Slim/Handlers/ErrorHandler.php @@ -281,9 +281,8 @@ public function setLogErrorRenderer($logErrorRenderer): void */ protected function writeToErrorLog(): void { - /** @var ErrorRendererInterface $renderer */ - $renderer = $this->callableResolver->resolve($this->logErrorRenderer); - $error = $renderer->__invoke($this->exception, $this->logErrorDetails); + $renderer = $this->logErrorRenderer; + $error = $renderer($this->exception, $this->logErrorDetails); $error .= "\nView in rendered output by enabling the \"displayErrorDetails\" setting.\n"; $this->logError($error); } From ed583730c9054aa8eb1760b5e575f34b7eddce82 Mon Sep 17 00:00:00 2001 From: Ryan Jentzsch Date: Tue, 13 Aug 2019 17:16:36 -0600 Subject: [PATCH 3/7] Fix $renderer calling `->invoke()` directly (attempt #2) --- Slim/Handlers/ErrorHandler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Slim/Handlers/ErrorHandler.php b/Slim/Handlers/ErrorHandler.php index 550634244..d5072078c 100644 --- a/Slim/Handlers/ErrorHandler.php +++ b/Slim/Handlers/ErrorHandler.php @@ -281,7 +281,7 @@ public function setLogErrorRenderer($logErrorRenderer): void */ protected function writeToErrorLog(): void { - $renderer = $this->logErrorRenderer; + $renderer = $this->callableResolver->resolve($this->logErrorRenderer); $error = $renderer($this->exception, $this->logErrorDetails); $error .= "\nView in rendered output by enabling the \"displayErrorDetails\" setting.\n"; $this->logError($error); From d57b92f3319b94be8525ff2eaec9db96a7e93e77 Mon Sep 17 00:00:00 2001 From: Adrian Suter Date: Fri, 16 Aug 2019 22:42:15 +0200 Subject: [PATCH 4/7] Remove `Slim\DeferredCallable` --- Slim/DeferredCallable.php | 46 ---------------------- tests/DeferredCallableTest.php | 71 ---------------------------------- tests/Routing/RouteTest.php | 6 +-- 3 files changed, 2 insertions(+), 121 deletions(-) delete mode 100644 Slim/DeferredCallable.php delete mode 100644 tests/DeferredCallableTest.php diff --git a/Slim/DeferredCallable.php b/Slim/DeferredCallable.php deleted file mode 100644 index 1fb899d03..000000000 --- a/Slim/DeferredCallable.php +++ /dev/null @@ -1,46 +0,0 @@ -callable = $callable; - $this->callableResolver = $resolver; - } - - public function __invoke(...$args) - { - /** @var callable $callable */ - $callable = $this->callable; - if ($this->callableResolver) { - $callable = $this->callableResolver->resolve($callable); - } - - return $callable(...$args); - } -} diff --git a/tests/DeferredCallableTest.php b/tests/DeferredCallableTest.php deleted file mode 100644 index e0bd4d0d6..000000000 --- a/tests/DeferredCallableTest.php +++ /dev/null @@ -1,71 +0,0 @@ -prophesize(ContainerInterface::class); - $containerProphecy->has('CallableTest')->willReturn(true); - $containerProphecy->get('CallableTest')->willReturn(new CallableTest()); - $resolver = new CallableResolver($containerProphecy->reveal()); - - $deferred = new DeferredCallable('CallableTest:toCall', $resolver); - $deferred(); - - $this->assertEquals(1, CallableTest::$CalledCount); - } - - public function testItBindsClosuresToContainer() - { - $containerProphecy = $this->prophesize(ContainerInterface::class); - $resolver = new CallableResolver($containerProphecy->reveal()); - - $assertCalled = $this->getMockBuilder('StdClass')->setMethods(['foo'])->getMock(); - $assertCalled - ->expects($this->once()) - ->method('foo'); - - $test = $this; - $closure = function () use ($containerProphecy, $test, $assertCalled) { - $assertCalled->foo(); - $test->assertSame($containerProphecy->reveal(), $this); - }; - - $deferred = new DeferredCallable($closure, $resolver); - $deferred(); - } - - public function testItReturnsInvokedCallableResponse() - { - $containerProphecy = $this->prophesize(ContainerInterface::class); - $resolver = new CallableResolver($containerProphecy->reveal()); - - $test = $this; - $foo = 'foo'; - $bar = 'bar'; - - $closure = function ($param) use ($test, $foo, $bar) { - $test->assertEquals($foo, $param); - return $bar; - }; - - $deferred = new DeferredCallable($closure, $resolver); - - $response = $deferred($foo); - $this->assertEquals($bar, $response); - } -} diff --git a/tests/Routing/RouteTest.php b/tests/Routing/RouteTest.php index 301b51625..a672b44a2 100644 --- a/tests/Routing/RouteTest.php +++ b/tests/Routing/RouteTest.php @@ -17,10 +17,8 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\StreamInterface; -use Psr\Http\Message\UriInterface; use Psr\Http\Server\RequestHandlerInterface; use Slim\CallableResolver; -use Slim\DeferredCallable; use Slim\Handlers\Strategies\RequestHandler; use Slim\Handlers\Strategies\RequestResponse; use Slim\Interfaces\CallableResolverInterface; @@ -390,7 +388,7 @@ public function testControllerMethodAsStringResolvesWithoutContainer() $callableResolver = new CallableResolver(); $responseFactory = $this->getResponseFactory(); - $deferred = new DeferredCallable('\Slim\Tests\Mocks\CallableTest:toCall', $callableResolver); + $deferred = $callableResolver->resolve('\Slim\Tests\Mocks\CallableTest:toCall'); $route = new Route(['GET'], '/', $deferred, $responseFactory, $callableResolver); CallableTest::$CalledCount = 0; @@ -410,7 +408,6 @@ public function testControllerMethodAsStringResolvesWithContainer() $callableResolverProphecy = $this->prophesize(CallableResolverInterface::class); $callable = 'CallableTest:toCall'; - $deferred = new DeferredCallable($callable, $callableResolverProphecy->reveal()); $callableResolverProphecy ->resolve($callable) @@ -426,6 +423,7 @@ public function testControllerMethodAsStringResolvesWithContainer() }) ->shouldBeCalledOnce(); + $deferred = $callableResolverProphecy->reveal()->resolve($callable); $callableResolverProphecy ->resolve($deferred) ->willReturn($deferred) From eea0503dfeba9bfb83234d42aa000e13da9fcff0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20B=C3=A9rub=C3=A9?= Date: Sat, 17 Aug 2019 21:21:22 -0600 Subject: [PATCH 5/7] change $logErrorRenderer property order --- Slim/Handlers/ErrorHandler.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Slim/Handlers/ErrorHandler.php b/Slim/Handlers/ErrorHandler.php index d5072078c..8ac1755df 100644 --- a/Slim/Handlers/ErrorHandler.php +++ b/Slim/Handlers/ErrorHandler.php @@ -42,6 +42,11 @@ class ErrorHandler implements ErrorHandlerInterface */ protected $defaultErrorRenderer = HtmlErrorRenderer::class; + /** + * @var ErrorRendererInterface|string|callable + */ + protected $logErrorRenderer = PlainTextErrorRenderer::class; + /** * @var array */ @@ -53,11 +58,6 @@ class ErrorHandler implements ErrorHandlerInterface 'text/plain' => PlainTextErrorRenderer::class, ]; - /** - * @var ErrorRendererInterface|string|callable - */ - protected $logErrorRenderer = PlainTextErrorRenderer::class; - /** * @var bool */ From 9efd5cb211beb019e9b0a799a710fcb11d3dc05f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20B=C3=A9rub=C3=A9?= Date: Sat, 17 Aug 2019 21:21:41 -0600 Subject: [PATCH 6/7] add test for setLogErrorRenderer method --- tests/Handlers/ErrorHandlerTest.php | 31 ++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/tests/Handlers/ErrorHandlerTest.php b/tests/Handlers/ErrorHandlerTest.php index df5d897df..f6456f505 100644 --- a/tests/Handlers/ErrorHandlerTest.php +++ b/tests/Handlers/ErrorHandlerTest.php @@ -11,6 +11,9 @@ use Psr\Http\Message\ResponseInterface; use ReflectionClass; +use ReflectionMethod; +use ReflectionProperty; +use RuntimeException; use Slim\Error\Renderers\HtmlErrorRenderer; use Slim\Error\Renderers\JsonErrorRenderer; use Slim\Error\Renderers\PlainTextErrorRenderer; @@ -18,6 +21,7 @@ use Slim\Exception\HttpMethodNotAllowedException; use Slim\Exception\HttpNotFoundException; use Slim\Handlers\ErrorHandler; +use Slim\Interfaces\CallableResolverInterface; use Slim\Tests\Mocks\MockCustomException; use Slim\Tests\TestCase; @@ -322,7 +326,7 @@ public function testDefaultErrorRenderer() ->withHeader('Accept', 'application/unknown'); $handler = new ErrorHandler($this->getCallableResolver(), $this->getResponseFactory()); - $exception = new \RuntimeException(); + $exception = new RuntimeException(); /** @var ResponseInterface $res */ $res = $handler->__invoke($request, $exception, true, true, true); @@ -330,4 +334,29 @@ public function testDefaultErrorRenderer() $this->assertTrue($res->hasHeader('Content-Type')); $this->assertEquals('text/html', $res->getHeaderLine('Content-Type')); } + + public function testLogErrorRenderer() + { + $renderer = function () { + return 'error'; + }; + + $callableResolverProphecy = $this->prophesize(CallableResolverInterface::class); + $callableResolverProphecy + ->resolve('logErrorRenderer') + ->willReturn($renderer) + ->shouldBeCalledOnce(); + + $handler = new ErrorHandler($callableResolverProphecy->reveal(), $this->getResponseFactory()); + $handler->setLogErrorRenderer('logErrorRenderer'); + + $exception = new RuntimeException(); + $exceptionProperty = new ReflectionProperty($handler, 'exception'); + $exceptionProperty->setAccessible(true); + $exceptionProperty->setValue($handler, $exception); + + $writeToErrorLogMethod = new ReflectionMethod($handler, 'writeToErrorLog'); + $writeToErrorLogMethod->setAccessible(true); + $writeToErrorLogMethod->invoke($handler); + } } From 10b11bd8a8d50fb1b1cb9178fe17d155e431c378 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20B=C3=A9rub=C3=A9?= Date: Sat, 17 Aug 2019 21:33:51 -0600 Subject: [PATCH 7/7] fix code style errors --- tests/Handlers/ErrorHandlerTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Handlers/ErrorHandlerTest.php b/tests/Handlers/ErrorHandlerTest.php index f6456f505..8993cc3ab 100644 --- a/tests/Handlers/ErrorHandlerTest.php +++ b/tests/Handlers/ErrorHandlerTest.php @@ -337,9 +337,9 @@ public function testDefaultErrorRenderer() public function testLogErrorRenderer() { - $renderer = function () { - return 'error'; - }; + $renderer = function () { + return ''; + }; $callableResolverProphecy = $this->prophesize(CallableResolverInterface::class); $callableResolverProphecy