From 8ca334706e189956a71027aa5bf8bd571dba45ea Mon Sep 17 00:00:00 2001 From: elazar Date: Fri, 3 Apr 2020 16:48:35 -0500 Subject: [PATCH 1/4] Add support for PSR-3 --- Slim/App.php | 14 +++++--- Slim/Handlers/ErrorHandler.php | 28 ++++++++++++++-- Slim/Logger.php | 22 +++++++++++++ Slim/Middleware/ErrorMiddleware.php | 17 ++++++++-- composer.json | 3 +- tests/AppTest.php | 6 +++- tests/Handlers/ErrorHandlerTest.php | 42 ++++++++++++++---------- tests/Middleware/ErrorMiddlewareTest.php | 33 ++++++++++++++----- 8 files changed, 126 insertions(+), 39 deletions(-) create mode 100644 Slim/Logger.php diff --git a/Slim/App.php b/Slim/App.php index 8fd6a6a39..173f942f0 100644 --- a/Slim/App.php +++ b/Slim/App.php @@ -16,6 +16,7 @@ use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; +use Psr\Log\LoggerInterface; use Slim\Factory\ServerRequestCreatorFactory; use Slim\Interfaces\CallableResolverInterface; use Slim\Interfaces\MiddlewareDispatcherInterface; @@ -140,23 +141,26 @@ public function addRoutingMiddleware(): RoutingMiddleware /** * Add the Slim built-in error middleware to the app middleware stack * - * @param bool $displayErrorDetails - * @param bool $logErrors - * @param bool $logErrorDetails + * @param bool $displayErrorDetails + * @param bool $logErrors + * @param bool $logErrorDetails + * @param LoggerInterface|null $logger * * @return ErrorMiddleware */ public function addErrorMiddleware( bool $displayErrorDetails, bool $logErrors, - bool $logErrorDetails + bool $logErrorDetails, + ?LoggerInterface $logger = null ): ErrorMiddleware { $errorMiddleware = new ErrorMiddleware( $this->getCallableResolver(), $this->getResponseFactory(), $displayErrorDetails, $logErrors, - $logErrorDetails + $logErrorDetails, + $logger ); $this->add($errorMiddleware); return $errorMiddleware; diff --git a/Slim/Handlers/ErrorHandler.php b/Slim/Handlers/ErrorHandler.php index a92d12f5f..0ac33c188 100644 --- a/Slim/Handlers/ErrorHandler.php +++ b/Slim/Handlers/ErrorHandler.php @@ -13,6 +13,7 @@ use Psr\Http\Message\ResponseFactoryInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; +use Psr\Log\LoggerInterface; use RuntimeException; use Slim\Error\Renderers\HtmlErrorRenderer; use Slim\Error\Renderers\JsonErrorRenderer; @@ -23,6 +24,7 @@ use Slim\Interfaces\CallableResolverInterface; use Slim\Interfaces\ErrorHandlerInterface; use Slim\Interfaces\ErrorRendererInterface; +use Slim\Logger; use Throwable; use function array_intersect; @@ -121,14 +123,24 @@ class ErrorHandler implements ErrorHandlerInterface */ protected $responseFactory; + /* + * @var LoggerInterface + */ + protected $logger; + /** * @param CallableResolverInterface $callableResolver * @param ResponseFactoryInterface $responseFactory + * @param LoggerInterface|null $logger */ - public function __construct(CallableResolverInterface $callableResolver, ResponseFactoryInterface $responseFactory) - { + public function __construct( + CallableResolverInterface $callableResolver, + ResponseFactoryInterface $responseFactory, + ?LoggerInterface $logger = null + ) { $this->callableResolver = $callableResolver; $this->responseFactory = $responseFactory; + $this->logger = $logger ?: $this->getDefaultLogger(); } /** @@ -311,7 +323,17 @@ protected function writeToErrorLog(): void */ protected function logError(string $error): void { - error_log($error); + $this->logger->error($error); + } + + /** + * Returns a default logger implementation. + * + * @return LoggerInterface + */ + protected function getDefaultLogger(): LoggerInterface + { + return new Logger(); } /** diff --git a/Slim/Logger.php b/Slim/Logger.php new file mode 100644 index 000000000..61ffffa8c --- /dev/null +++ b/Slim/Logger.php @@ -0,0 +1,22 @@ +callableResolver = $callableResolver; $this->responseFactory = $responseFactory; $this->displayErrorDetails = $displayErrorDetails; $this->logErrors = $logErrors; $this->logErrorDetails = $logErrorDetails; + $this->logger = $logger; } /** @@ -150,7 +159,11 @@ public function getErrorHandler(string $type) public function getDefaultErrorHandler() { if ($this->defaultErrorHandler === null) { - $this->defaultErrorHandler = new ErrorHandler($this->callableResolver, $this->responseFactory); + $this->defaultErrorHandler = new ErrorHandler( + $this->callableResolver, + $this->responseFactory, + $this->logger + ); } return $this->callableResolver->resolve($this->defaultErrorHandler); diff --git a/composer.json b/composer.json index ee597f1c1..a341551f8 100644 --- a/composer.json +++ b/composer.json @@ -40,7 +40,8 @@ "psr/http-factory": "^1.0", "psr/http-message": "^1.0", "psr/http-server-handler": "^1.0", - "psr/http-server-middleware": "^1.0" + "psr/http-server-middleware": "^1.0", + "psr/log": "^1.1" }, "require-dev": { "ext-simplexml": "*", diff --git a/tests/AppTest.php b/tests/AppTest.php index 63f3ee126..2c6b91372 100644 --- a/tests/AppTest.php +++ b/tests/AppTest.php @@ -19,6 +19,7 @@ use Psr\Http\Message\UriInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; +use Psr\Log\LoggerInterface; use ReflectionClass; use ReflectionProperty; use RuntimeException; @@ -724,11 +725,14 @@ public function testAddErrorMiddleware() /** @var ResponseFactoryInterface $responseFactory */ $responseFactory = $this->prophesize(ResponseFactoryInterface::class)->reveal(); + /** @var LoggerInterface $logger */ + $logger = $this->createMock(LoggerInterface::class); + // Create the app. $app = new App($responseFactory); // Add the error middleware. - $errorMiddleware = $app->addErrorMiddleware(true, true, true); + $errorMiddleware = $app->addErrorMiddleware(true, true, true, $logger); // Check that the error middleware really has been added to the tip of the app middleware stack. $middlewareDispatcherProperty = new ReflectionProperty(App::class, 'middlewareDispatcher'); diff --git a/tests/Handlers/ErrorHandlerTest.php b/tests/Handlers/ErrorHandlerTest.php index 44251229c..baaeeea76 100644 --- a/tests/Handlers/ErrorHandlerTest.php +++ b/tests/Handlers/ErrorHandlerTest.php @@ -11,6 +11,7 @@ namespace Slim\Tests\Handlers; use Psr\Http\Message\ResponseInterface; +use Psr\Log\LoggerInterface; use ReflectionClass; use ReflectionMethod; use ReflectionProperty; @@ -303,16 +304,16 @@ public function testWriteToErrorLog() ->createServerRequest('/', 'GET') ->withHeader('Accept', 'application/json'); - $handler = $this->getMockBuilder(ErrorHandler::class) - ->setConstructorArgs([ - 'callableResolver' => $this->getCallableResolver(), - 'responseFactory' => $this->getResponseFactory(), - ]) - ->setMethods(['logError']) - ->getMock(); + $logger = $this->getMockLogger(); + + $handler = new ErrorHandler( + $this->getCallableResolver(), + $this->getResponseFactory(), + $logger + ); - $handler->expects(self::once()) - ->method('logError') + $logger->expects(self::once()) + ->method('error') ->willReturnCallback(static function (string $error) { self::assertStringNotContainsString( 'set "displayErrorDetails" to true in the ErrorHandler constructor', @@ -330,16 +331,16 @@ public function testWriteToErrorLogShowTip() ->createServerRequest('/', 'GET') ->withHeader('Accept', 'application/json'); - $handler = $this->getMockBuilder(ErrorHandler::class) - ->setConstructorArgs([ - 'callableResolver' => $this->getCallableResolver(), - 'responseFactory' => $this->getResponseFactory(), - ]) - ->setMethods(['logError']) - ->getMock(); + $logger = $this->getMockLogger(); + + $handler = new ErrorHandler( + $this->getCallableResolver(), + $this->getResponseFactory(), + $logger, + ); - $handler->expects(self::once()) - ->method('logError') + $logger->expects(self::once()) + ->method('error') ->willReturnCallback(static function (string $error) { self::assertStringContainsString( 'set "displayErrorDetails" to true in the ErrorHandler constructor', @@ -391,4 +392,9 @@ public function testLogErrorRenderer() $writeToErrorLogMethod->setAccessible(true); $writeToErrorLogMethod->invoke($handler); } + + private function getMockLogger(): LoggerInterface + { + return $this->createMock(LoggerInterface::class); + } } diff --git a/tests/Middleware/ErrorMiddlewareTest.php b/tests/Middleware/ErrorMiddlewareTest.php index 8326f0960..028ee3915 100644 --- a/tests/Middleware/ErrorMiddlewareTest.php +++ b/tests/Middleware/ErrorMiddlewareTest.php @@ -15,6 +15,7 @@ use LogicException; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; +use Psr\Log\LoggerInterface; use RuntimeException; use Slim\App; use Slim\Exception\HttpNotFoundException; @@ -31,6 +32,7 @@ public function testSetErrorHandler() $responseFactory = $this->getResponseFactory(); $app = new App($responseFactory); $callableResolver = $app->getCallableResolver(); + $logger = $this->getMockLogger(); $mw = new RoutingMiddleware($app->getRouteResolver(), $app->getRouteCollector()->getRouteParser()); $app->add($mw); @@ -42,7 +44,7 @@ public function testSetErrorHandler() return $response; })->bindTo($this); - $mw2 = new ErrorMiddleware($callableResolver, $this->getResponseFactory(), false, false, false); + $mw2 = new ErrorMiddleware($callableResolver, $this->getResponseFactory(), false, false, false, $logger); $mw2->setErrorHandler($exception, $handler); $app->add($mw2); @@ -57,6 +59,7 @@ public function testSetDefaultErrorHandler() $responseFactory = $this->getResponseFactory(); $app = new App($responseFactory); $callableResolver = $app->getCallableResolver(); + $logger = $this->getMockLogger(); $mw = new RoutingMiddleware($app->getRouteResolver(), $app->getRouteCollector()->getRouteParser()); $app->add($mw); @@ -67,7 +70,7 @@ public function testSetDefaultErrorHandler() return $response; })->bindTo($this); - $mw2 = new ErrorMiddleware($callableResolver, $this->getResponseFactory(), false, false, false); + $mw2 = new ErrorMiddleware($callableResolver, $this->getResponseFactory(), false, false, false, $logger); $mw2->setDefaultErrorHandler($handler); $app->add($mw2); @@ -84,8 +87,9 @@ public function testSetDefaultErrorHandlerThrowsException() $responseFactory = $this->getResponseFactory(); $app = new App($responseFactory); $callableResolver = $app->getCallableResolver(); + $logger = $this->getMockLogger(); - $mw = new ErrorMiddleware($callableResolver, $this->getResponseFactory(), false, false, false); + $mw = new ErrorMiddleware($callableResolver, $this->getResponseFactory(), false, false, false, $logger); $mw->setDefaultErrorHandler('Uncallable'); $mw->getDefaultErrorHandler(); } @@ -95,8 +99,9 @@ public function testGetErrorHandlerWillReturnDefaultErrorHandlerForUnhandledExce $responseFactory = $this->getResponseFactory(); $app = new App($responseFactory); $callableResolver = $app->getCallableResolver(); + $logger = $this->getMockLogger(); - $middleware = new ErrorMiddleware($callableResolver, $this->getResponseFactory(), false, false, false); + $middleware = new ErrorMiddleware($callableResolver, $this->getResponseFactory(), false, false, false, $logger); $exception = MockCustomException::class; $handler = $middleware->getErrorHandler($exception); $this->assertInstanceOf(ErrorHandler::class, $handler); @@ -107,7 +112,8 @@ public function testSuperclassExceptionHandlerHandlesExceptionWithSubclassExactM $responseFactory = $this->getResponseFactory(); $app = new App($responseFactory); $callableResolver = $app->getCallableResolver(); - $mw = new ErrorMiddleware($callableResolver, $this->getResponseFactory(), false, false, false); + $logger = $this->getMockLogger(); + $mw = new ErrorMiddleware($callableResolver, $this->getResponseFactory(), false, false, false, $logger); $app->add(function ($request, $handler) { throw new LogicException('This is a LogicException...'); }); @@ -136,8 +142,9 @@ public function testSuperclassExceptionHandlerHandlesSubclassException() $responseFactory = $this->getResponseFactory(); $app = new App($responseFactory); $callableResolver = $app->getCallableResolver(); + $logger = $this->getMockLogger(); - $mw = new ErrorMiddleware($callableResolver, $this->getResponseFactory(), false, false, false); + $mw = new ErrorMiddleware($callableResolver, $this->getResponseFactory(), false, false, false, $logger); $app->add(function ($request, $handler) { throw new InvalidArgumentException('This is a subclass of LogicException...'); @@ -173,8 +180,9 @@ public function testSuperclassExceptionHandlerDoesNotHandleSubclassException() $responseFactory = $this->getResponseFactory(); $app = new App($responseFactory); $callableResolver = $app->getCallableResolver(); + $logger = $this->getMockLogger(); - $mw = new ErrorMiddleware($callableResolver, $this->getResponseFactory(), false, false, false); + $mw = new ErrorMiddleware($callableResolver, $this->getResponseFactory(), false, false, false, $logger); $app->add(function ($request, $handler) { throw new InvalidArgumentException('This is a subclass of LogicException...'); @@ -210,8 +218,9 @@ public function testHandleMultipleExceptionsAddedAsArray() $responseFactory = $this->getResponseFactory(); $app = new App($responseFactory); $callableResolver = $app->getCallableResolver(); + $logger = $this->getMockLogger(); - $mw = new ErrorMiddleware($callableResolver, $this->getResponseFactory(), false, false, false); + $mw = new ErrorMiddleware($callableResolver, $this->getResponseFactory(), false, false, false, $logger); $app->add(function ($request, $handler) { throw new InvalidArgumentException('This is an invalid argument exception...'); @@ -249,8 +258,9 @@ public function testErrorHandlerHandlesThrowables() $responseFactory = $this->getResponseFactory(); $app = new App($responseFactory); $callableResolver = $app->getCallableResolver(); + $logger = $this->getMockLogger(); - $mw = new ErrorMiddleware($callableResolver, $this->getResponseFactory(), false, false, false); + $mw = new ErrorMiddleware($callableResolver, $this->getResponseFactory(), false, false, false, $logger); $app->add(function ($request, $handler) { throw new Error('Oops..'); @@ -274,4 +284,9 @@ public function testErrorHandlerHandlesThrowables() $this->expectOutputString('Oops..'); } + + private function getMockLogger(): LoggerInterface + { + return $this->createMock(LoggerInterface::class); + } } From 7053c6d64a907298083efed87e5d463bfdaa7fdc Mon Sep 17 00:00:00 2001 From: elazar Date: Fri, 3 Apr 2020 17:00:14 -0500 Subject: [PATCH 2/4] Fix syntax error in PHP 7.2 in ErrorHandlerTest --- tests/Handlers/ErrorHandlerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Handlers/ErrorHandlerTest.php b/tests/Handlers/ErrorHandlerTest.php index baaeeea76..9d86b2c1d 100644 --- a/tests/Handlers/ErrorHandlerTest.php +++ b/tests/Handlers/ErrorHandlerTest.php @@ -336,7 +336,7 @@ public function testWriteToErrorLogShowTip() $handler = new ErrorHandler( $this->getCallableResolver(), $this->getResponseFactory(), - $logger, + $logger ); $logger->expects(self::once()) From 040a831cd54ddc6259f8646ce2049df53065e3ef Mon Sep 17 00:00:00 2001 From: elazar Date: Fri, 3 Apr 2020 23:00:18 -0500 Subject: [PATCH 3/4] Address PR comments --- Slim/Logger.php | 8 ++++++++ tests/AppTest.php | 2 +- tests/Handlers/ErrorHandlerTest.php | 10 +++++----- tests/Middleware/ErrorMiddlewareTest.php | 10 +++++----- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/Slim/Logger.php b/Slim/Logger.php index 61ffffa8c..06e97d7f1 100644 --- a/Slim/Logger.php +++ b/Slim/Logger.php @@ -1,5 +1,13 @@ prophesize(ResponseFactoryInterface::class)->reveal(); /** @var LoggerInterface $logger */ - $logger = $this->createMock(LoggerInterface::class); + $logger = $this->prophesize(LoggerInterface::class)->reveal(); // Create the app. $app = new App($responseFactory); diff --git a/tests/Handlers/ErrorHandlerTest.php b/tests/Handlers/ErrorHandlerTest.php index 9d86b2c1d..bf1d7b218 100644 --- a/tests/Handlers/ErrorHandlerTest.php +++ b/tests/Handlers/ErrorHandlerTest.php @@ -29,6 +29,11 @@ class ErrorHandlerTest extends TestCase { + private function getMockLogger(): LoggerInterface + { + return $this->createMock(LoggerInterface::class); + } + public function testDetermineRenderer() { $handler = $this @@ -392,9 +397,4 @@ public function testLogErrorRenderer() $writeToErrorLogMethod->setAccessible(true); $writeToErrorLogMethod->invoke($handler); } - - private function getMockLogger(): LoggerInterface - { - return $this->createMock(LoggerInterface::class); - } } diff --git a/tests/Middleware/ErrorMiddlewareTest.php b/tests/Middleware/ErrorMiddlewareTest.php index 028ee3915..d6289cc6f 100644 --- a/tests/Middleware/ErrorMiddlewareTest.php +++ b/tests/Middleware/ErrorMiddlewareTest.php @@ -27,6 +27,11 @@ class ErrorMiddlewareTest extends TestCase { + private function getMockLogger(): LoggerInterface + { + return $this->createMock(LoggerInterface::class); + } + public function testSetErrorHandler() { $responseFactory = $this->getResponseFactory(); @@ -284,9 +289,4 @@ public function testErrorHandlerHandlesThrowables() $this->expectOutputString('Oops..'); } - - private function getMockLogger(): LoggerInterface - { - return $this->createMock(LoggerInterface::class); - } } From 40eebcd6f599bb497c0d66b11851865f0a7834a6 Mon Sep 17 00:00:00 2001 From: elazar Date: Sat, 4 Apr 2020 09:18:42 -0500 Subject: [PATCH 4/4] Move exception FQCN to use directive in Logger --- Slim/Logger.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Slim/Logger.php b/Slim/Logger.php index 06e97d7f1..9d86ac0cf 100644 --- a/Slim/Logger.php +++ b/Slim/Logger.php @@ -11,6 +11,7 @@ namespace Slim; use Psr\Log\AbstractLogger; +use Psr\Log\InvalidArgumentException; class Logger extends AbstractLogger { @@ -21,7 +22,7 @@ class Logger extends AbstractLogger * * @return void * - * @throws \Psr\Log\InvalidArgumentException + * @throws InvalidArgumentException */ public function log($level, $message, array $context = []) {