From aa044cdad90a7df1d6e077961b123ee70533aed6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20Kr=C3=A4mer?= Date: Mon, 6 Jan 2025 19:07:39 +0100 Subject: [PATCH] Refactoring the conversion of exceptions --- readme.md | 10 +- .../ExceptionConverterInterface.php | 22 ++++ .../GenericThrowableConverter.php | 59 ++++++++++ .../HttpExceptionConverter.php | 43 ++++++++ .../ValidationFailedExceptionConverter.php | 59 ++++++++++ ...xceptionToProblemDetailsKernelListener.php | 57 ---------- ...hrowableToProblemDetailsKernelListener.php | 49 +++------ ...sToProblemDetailsKernelEventSubscriber.php | 82 -------------- .../GenericThrowableConverterTest.php | 59 ++++++++++ .../HttpExceptionConverterTest.php | 54 +++++++++ ...alidationFailedExceptionConverterTest.php} | 103 +++++++----------- ...tionToProblemDetailsKernelListenerTest.php | 72 ------------ ...ableToProblemDetailsKernelListenerTest.php | 8 +- 13 files changed, 362 insertions(+), 315 deletions(-) create mode 100644 src/ExceptionConversion/ExceptionConverterInterface.php create mode 100644 src/ExceptionConversion/GenericThrowableConverter.php create mode 100644 src/ExceptionConversion/HttpExceptionConverter.php create mode 100644 src/ExceptionConversion/ValidationFailedExceptionConverter.php delete mode 100644 src/HttpExceptionToProblemDetailsKernelListener.php delete mode 100644 src/Validation/ValidationErrorsToProblemDetailsKernelEventSubscriber.php create mode 100644 tests/Unit/ExceptionConversion/GenericThrowableConverterTest.php create mode 100644 tests/Unit/ExceptionConversion/HttpExceptionConverterTest.php rename tests/Unit/{ValidationErrorsToProblemDetailsKernelEventSubscriberTest.php => ExceptionConversion/ValidationFailedExceptionConverterTest.php} (54%) delete mode 100644 tests/Unit/HttpExceptionToProblemDetailsKernelListenerTest.php diff --git a/readme.md b/readme.md index 0b04d49..b089801 100644 --- a/readme.md +++ b/readme.md @@ -17,11 +17,9 @@ composer require phauthentic/problem-details-symfony-bundle ```php class ExampleController { - private ProblemDetailsFactoryInterface $problemDetailsFactory; - - public function __construct(ProblemDetailsFactoryInterface $problemDetailsFactory) - { - $this->problemDetailsFactory = $problemDetailsFactory; + public function __construct( + private ProblemDetailsFactoryInterface $problemDetailsFactory + ) { } /** @@ -33,8 +31,10 @@ class ExampleController type: 'https://example.net/validation-error', detail: 'Your request is not valid.', status: 422, + title: 'Validation Error', ); } +} ``` ## Problem Details Example diff --git a/src/ExceptionConversion/ExceptionConverterInterface.php b/src/ExceptionConversion/ExceptionConverterInterface.php new file mode 100644 index 0000000..43a3b49 --- /dev/null +++ b/src/ExceptionConversion/ExceptionConverterInterface.php @@ -0,0 +1,22 @@ + + * Phauthentic\Symfony\ProblemDetails\ThrowableToProblemDetailsKernelListener: + * arguments: ['%kernel.environment%', { }] + * tags: + * - { name: kernel.event_listener, event: kernel.exception, priority: -20 } + * + * + * @link https://www.rfc-editor.org/rfc/rfc9457.html + */ +class GenericThrowableConverter implements ExceptionConverterInterface +{ + /** + * @param ProblemDetailsFactoryInterface $problemDetailsFactory + * @param string $environment + * @param array $mappers + */ + public function __construct( + protected ProblemDetailsFactoryInterface $problemDetailsFactory, + protected string $environment = 'prod', + protected array $mappers = [] + ) { + } + + public function canHandle(Throwable $throwable): bool + { + return true; + } + + public function convertExceptionToErrorDetails(Throwable $throwable, ExceptionEvent $event): Response + { + $extensions = []; + if ($this->environment === 'dev' || $this->environment === 'test') { + $extensions['trace'] = $throwable->getTrace(); + } + + return $this->problemDetailsFactory->createResponse( + status: Response::HTTP_INTERNAL_SERVER_ERROR, + title: $throwable->getMessage(), + extensions: $extensions, + ); + } +} diff --git a/src/ExceptionConversion/HttpExceptionConverter.php b/src/ExceptionConversion/HttpExceptionConverter.php new file mode 100644 index 0000000..5326081 --- /dev/null +++ b/src/ExceptionConversion/HttpExceptionConverter.php @@ -0,0 +1,43 @@ +problemDetailsFactory->createResponse( + status: $throwable->getStatusCode(), + type: 'https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/' . $throwable->getStatusCode(), + title: $throwable->getMessage() + ); + } +} diff --git a/src/ExceptionConversion/ValidationFailedExceptionConverter.php b/src/ExceptionConversion/ValidationFailedExceptionConverter.php new file mode 100644 index 0000000..715de76 --- /dev/null +++ b/src/ExceptionConversion/ValidationFailedExceptionConverter.php @@ -0,0 +1,59 @@ +getPrevious(); + } + + return $throwable instanceof ValidationFailedException; + } + + private function extractValidationFailedException(Throwable $throwable): ValidationFailedException + { + if ($throwable instanceof UnprocessableEntityHttpException) { + $throwable = $throwable->getPrevious(); + } + + if ($throwable instanceof ValidationFailedException) { + return $throwable; + } + + throw new RuntimeException('ValidationFailedException not found'); + } + + public function convertExceptionToErrorDetails(Throwable $throwable, ExceptionEvent $event): Response + { + $throwable = $this->extractValidationFailedException($throwable); + + $errors = $this->validationErrorsBuilder->buildErrors($throwable); + + return $this->problemDetailsResponseFactory->createResponseFromKernelExceptionEvent($event, $errors); + } +} diff --git a/src/HttpExceptionToProblemDetailsKernelListener.php b/src/HttpExceptionToProblemDetailsKernelListener.php deleted file mode 100644 index 75f2b75..0000000 --- a/src/HttpExceptionToProblemDetailsKernelListener.php +++ /dev/null @@ -1,57 +0,0 @@ - - * Phauthentic\Symfony\ProblemDetails\HttpExceptionToProblemDetailsKernelListener: - * tags: - * - { name: kernel.event_listener, event: kernel.exception, priority: -10 } - * - * - * @link https://www.rfc-editor.org/rfc/rfc9457.html - */ -class HttpExceptionToProblemDetailsKernelListener -{ - public function __construct( - protected ProblemDetailsFactoryInterface $problemDetailsFactory - ) { - } - - public function onKernelException(ExceptionEvent $event): void - { - $exception = $event->getThrowable(); - - if ($this->isAHttpException($exception)) { - /** @var HttpException $exception */ - $event->setResponse($this->buildResponse($exception)); - } - } - - private function isAHttpException(Throwable $exception): bool - { - return $exception instanceof HttpException; - } - - private function buildResponse(HttpException $httpException): Response - { - return $this->problemDetailsFactory->createResponse( - status: $httpException->getStatusCode(), - type: 'https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/' . $httpException->getStatusCode(), - title: $httpException->getMessage() - ); - } -} diff --git a/src/ThrowableToProblemDetailsKernelListener.php b/src/ThrowableToProblemDetailsKernelListener.php index ac1c865..12cc7a2 100644 --- a/src/ThrowableToProblemDetailsKernelListener.php +++ b/src/ThrowableToProblemDetailsKernelListener.php @@ -4,15 +4,14 @@ namespace Phauthentic\Symfony\ProblemDetails; -use Symfony\Component\HttpFoundation\JsonResponse; -use Symfony\Component\HttpFoundation\Response; +use InvalidArgumentException; +use Phauthentic\Symfony\ProblemDetails\ExceptionConversion\ExceptionConverterInterface; use Symfony\Component\HttpKernel\Event\ExceptionEvent; -use Throwable; /** * Handles Thowable and converts it into a Problem Details HTTP response. * - * Notice that you might need to adjust the priority of the listener in your services.yaml file to make sure it is + * Notice that you might need to adjust the priority of the converters in your services.yaml file to make sure it is * executed in the right order if you have other listeners. * * @@ -27,15 +26,14 @@ class ThrowableToProblemDetailsKernelListener { /** - * @param ProblemDetailsFactoryInterface $problemDetailsFactory - * @param string $environment - * @param array $mappers + * @param array $exceptionConverters */ public function __construct( - protected ProblemDetailsFactoryInterface $problemDetailsFactory, - protected string $environment = 'prod', - protected array $mappers = [] + protected array $exceptionConverters = [] ) { + if (empty($this->exceptionConverters)) { + throw new InvalidArgumentException('No exception converter passed!'); + } } public function onKernelException(ExceptionEvent $event): void @@ -44,37 +42,26 @@ public function onKernelException(ExceptionEvent $event): void return; } - $throwable = $event->getThrowable(); + $this->processConverters($event); + } - $class = get_class($throwable); - if (isset($this->mappers[$class])) { - $mapper = $this->mappers[$class]; - $response = $mapper($throwable); + private function processConverters(ExceptionEvent $event): void + { + $throwable = $event->getThrowable(); + foreach ($this->exceptionConverters as $exceptionConverter) { + if (!$exceptionConverter->canHandle($throwable)) { + continue; + } + $response = $exceptionConverter->convertExceptionToErrorDetails($throwable, $event); $event->setResponse($response); return; } - - $event->setResponse($this->buildResponse($throwable)); } private function isNotAJsonRequest(ExceptionEvent $event): bool { return $event->getRequest()->getPreferredFormat() !== 'json'; } - - private function buildResponse(Throwable $throwable): Response - { - $extensions = []; - if ($this->environment === 'dev' || $this->environment === 'test') { - $extensions['trace'] = $throwable->getTrace(); - } - - return $this->problemDetailsFactory->createResponse( - status: Response::HTTP_INTERNAL_SERVER_ERROR, - title: $throwable->getMessage(), - extensions: $extensions - ); - } } diff --git a/src/Validation/ValidationErrorsToProblemDetailsKernelEventSubscriber.php b/src/Validation/ValidationErrorsToProblemDetailsKernelEventSubscriber.php deleted file mode 100644 index cda9d6c..0000000 --- a/src/Validation/ValidationErrorsToProblemDetailsKernelEventSubscriber.php +++ /dev/null @@ -1,82 +0,0 @@ - 'onException' - ]; - } - - private function isNotAJsonRequest(ExceptionEvent $event): bool - { - return $event->getRequest()->getPreferredFormat() !== 'json'; - } - - public function extractValidationFailedException(ExceptionEvent $event): ?ValidationFailedException - { - $throwAble = $event->getThrowable(); - if ($throwAble instanceof UnprocessableEntityHttpException) { - $throwAble = $throwAble->getPrevious(); - } - - if ($throwAble instanceof ValidationFailedException) { - return $throwAble; - } - - return null; - } - - /** - * Specification: - * - Checks if the request is not a JSON request. - * - If not return. - * - Checks if the exception is a ValidationFailedException. - * - If not return. - * - Converts a ValidationFailedException to an array of errors. - * - Creates a ProblemDetails response containing the errors. - */ - public function onException(ExceptionEvent $event): void - { - if ($this->isNotAJsonRequest($event)) { - return; - } - - $validationFailedException = $this->extractValidationFailedException($event); - if (!$validationFailedException) { - return; - } - - $errors = $this->validationErrorsBuilder->buildErrors($validationFailedException); - $response = $this->problemDetailsResponseFactory->createResponseFromKernelExceptionEvent($event, $errors); - - $event->setResponse($response); - } -} diff --git a/tests/Unit/ExceptionConversion/GenericThrowableConverterTest.php b/tests/Unit/ExceptionConversion/GenericThrowableConverterTest.php new file mode 100644 index 0000000..8234c6e --- /dev/null +++ b/tests/Unit/ExceptionConversion/GenericThrowableConverterTest.php @@ -0,0 +1,59 @@ +converter = new GenericThrowableConverter( + problemDetailsFactory: new ProblemDetailsFactory() + ); + } + + public function testConvertExceptionToErrorDetails(): void + { + $this->assertTrue($this->converter->canHandle(new Exception())); + } + + public function testConvert(): void + { + // Arrange + $exception = new Exception('Some error'); + $event = new ExceptionEvent( + $this->createMock(HttpKernelInterface::class), + new Request(), + HttpKernelInterface::MAIN_REQUEST, + $exception + ); + + // Act + $response = $this->converter->convertExceptionToErrorDetails($exception, $event); + + // Assert + $data = json_decode($response->getContent(), true, 512, JSON_THROW_ON_ERROR); + $this->assertEquals([ + 'status' => 500, + 'type' => 'about:blank', + 'title' => 'Some error', + ], $data); + } +} diff --git a/tests/Unit/ExceptionConversion/HttpExceptionConverterTest.php b/tests/Unit/ExceptionConversion/HttpExceptionConverterTest.php new file mode 100644 index 0000000..35b8d46 --- /dev/null +++ b/tests/Unit/ExceptionConversion/HttpExceptionConverterTest.php @@ -0,0 +1,54 @@ +createMock(HttpKernelInterface::class), + new Request(), + HttpKernelInterface::MAIN_REQUEST, + $exception + ); + + // Act + $response = $converter->convertExceptionToErrorDetails($exception, $event); + + // Assert + $data = json_decode($response->getContent(), true, 512, JSON_THROW_ON_ERROR); + $this->assertEquals([ + 'status' => 404, + 'type' => 'https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/404', + 'title' => 'Not Found', + ], $data); + } + + public function testCanHandle(): void + { + // Arrange + $converter = new HttpExceptionConverter(new ProblemDetailsFactory()); + + // Act + $this->assertFalse($converter->canHandle(new Exception('Some other exception'))); + $this->assertTrue($converter->canHandle(new HttpException(404, 'Not Found'))); + } +} diff --git a/tests/Unit/ValidationErrorsToProblemDetailsKernelEventSubscriberTest.php b/tests/Unit/ExceptionConversion/ValidationFailedExceptionConverterTest.php similarity index 54% rename from tests/Unit/ValidationErrorsToProblemDetailsKernelEventSubscriberTest.php rename to tests/Unit/ExceptionConversion/ValidationFailedExceptionConverterTest.php index b85fd54..7219ad4 100644 --- a/tests/Unit/ValidationErrorsToProblemDetailsKernelEventSubscriberTest.php +++ b/tests/Unit/ExceptionConversion/ValidationFailedExceptionConverterTest.php @@ -2,38 +2,46 @@ declare(strict_types=1); -namespace Phauthentic\Symfony\ProblemDetails\Tests\Unit; +namespace Phauthentic\Symfony\ProblemDetails\Tests\Unit\ExceptionConversion; -use JsonException; +use Exception; +use Phauthentic\Symfony\ProblemDetails\ExceptionConversion\ValidationFailedExceptionConverter; +use Phauthentic\Symfony\ProblemDetails\FromExceptionEventFactoryInterface; use Phauthentic\Symfony\ProblemDetails\ProblemDetailsFactory; use Phauthentic\Symfony\ProblemDetails\Validation\ValidationErrorsBuilder; -use Phauthentic\Symfony\ProblemDetails\Validation\ValidationErrorsToProblemDetailsKernelEventSubscriber; use PHPUnit\Framework\Attributes\Test; -use PHPUnit\Framework\MockObject\Exception; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Event\ExceptionEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; -use Symfony\Component\HttpKernel\KernelEvents; -use Symfony\Component\Validator\Exception\ValidationFailedException; -use Symfony\Component\Validator\ConstraintViolationList; use Symfony\Component\Validator\ConstraintViolation; +use Symfony\Component\Validator\ConstraintViolationList; +use Symfony\Component\Validator\Exception\ValidationFailedException; -class ValidationErrorsToProblemDetailsKernelEventSubscriberTest extends TestCase +/** + * + */ +class ValidationFailedExceptionConverterTest extends TestCase { - /** - * @throws Exception - * @throws JsonException - */ + protected ValidationFailedExceptionConverter $converter; + + public function setUp(): void + { + parent::setUp(); + + $this->converter = new ValidationFailedExceptionConverter( + validationErrorsBuilder: new ValidationErrorsBuilder(), + problemDetailsResponseFactory: new ProblemDetailsFactory() + ); + } + #[Test] - public function testOnException(): void + public function testConvertExceptionToErrorDetails(): void { // Arrange $violations = $this->getConstraintViolationList(); $exception = new ValidationFailedException('Validation failed', $violations); - $listener = $this->getErrorsToProblemDetailsKernelEventSubscriber(); - $kernel = $this->createMock(HttpKernelInterface::class); $request = new Request([], [], [], [], [], ['REQUEST_URI' => '/profile/1']); $request->headers->add(['Accept' => 'application/json']); @@ -41,10 +49,9 @@ public function testOnException(): void $event = new ExceptionEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST, $exception); // Act - $listener->onException($event); + $response = $this->converter->convertExceptionToErrorDetails($exception, $event); // Assert - $response = $event->getResponse(); $this->assertNotNull($response); $this->assertEquals(422, $response->getStatusCode()); $this->assertJsonStringEqualsJsonString( @@ -68,10 +75,21 @@ public function testOnException(): void ); } - /** - * @return ConstraintViolationList - * @throws Exception - */ + #[Test] + public function testCanHandle(): void + { + $this->assertFalse($this->converter->canHandle( + new Exception('Some other exception') + )); + + $this->assertTrue($this->converter->canHandle( + new ValidationFailedException( + 'Validation failed', + $this->getConstraintViolationList() + ) + )); + } + private function getConstraintViolationList(): ConstraintViolationList { $violation1 = $this->createMock(ConstraintViolation::class); @@ -84,47 +102,4 @@ private function getConstraintViolationList(): ConstraintViolationList return new ConstraintViolationList([$violation1, $violation2]); } - - #[Test] - public function testGetSubscribedEvents(): void - { - $events = ValidationErrorsToProblemDetailsKernelEventSubscriber::getSubscribedEvents(); - - $this->assertEquals([KernelEvents::EXCEPTION => 'onException'], $events); - } - - #[Test] - public function testOnExceptionNotValidationFailedException(): void - { - // Arrange - $exception = new \Exception('Some other exception'); - - $listener = $this->getErrorsToProblemDetailsKernelEventSubscriber(); - - $kernel = $this->createMock(HttpKernelInterface::class); - $request = new Request([], [], [], [], [], ['REQUEST_URI' => '/profile/1']); - $request->headers->add(['Accept' => 'application/json']); - $event = new ExceptionEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST, $exception); - - // Act - $listener->onException($event); - - // Assert - $response = $event->getResponse(); - $this->assertNull($response); - } - - /** - * @return ValidationErrorsToProblemDetailsKernelEventSubscriber - */ - public function getErrorsToProblemDetailsKernelEventSubscriber(): ValidationErrorsToProblemDetailsKernelEventSubscriber - { - $validationErrorsBuilder = new ValidationErrorsBuilder(); - $problemDetailsResponseFactory = new ProblemDetailsFactory(); - - return new ValidationErrorsToProblemDetailsKernelEventSubscriber( - $validationErrorsBuilder, - $problemDetailsResponseFactory - ); - } } diff --git a/tests/Unit/HttpExceptionToProblemDetailsKernelListenerTest.php b/tests/Unit/HttpExceptionToProblemDetailsKernelListenerTest.php deleted file mode 100644 index 3c534ab..0000000 --- a/tests/Unit/HttpExceptionToProblemDetailsKernelListenerTest.php +++ /dev/null @@ -1,72 +0,0 @@ -createMock(HttpKernelInterface::class); - $request = new Request( - server: ['HTTP_ACCEPT' => 'application/json'] - ); - $event = new ExceptionEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST, $exception); - $listener = new HttpExceptionToProblemDetailsKernelListener( - new ProblemDetailsFactory() - ); - - // Act - $listener->onKernelException($event); - - // Assert - $response = $event->getResponse(); - $this->assertInstanceOf(JsonResponse::class, $response); - $this->assertEquals(404, $response->getStatusCode()); - $this->assertJsonStringEqualsJsonString( - json_encode([ - 'type' => 'https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/404', - 'title' => 'Not Found', - 'status' => 404, - ], JSON_THROW_ON_ERROR), - $response->getContent() - ); - } - - public function testOnKernelExceptionWithNonHttpException(): void - { - // Arrange - $exception = new Exception('Some other exception'); - $kernel = $this->createMock(HttpKernelInterface::class); - $request = new Request( - server: ['HTTP_ACCEPT' => 'application/json'] - ); - $event = new ExceptionEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST, $exception); - $listener = new HttpExceptionToProblemDetailsKernelListener( - new ProblemDetailsFactory() - ); - - // Act - $listener->onKernelException($event); - - // Assert - $response = $event->getResponse(); - $this->assertNull($response); - } -} diff --git a/tests/Unit/ThrowableToProblemDetailsKernelListenerTest.php b/tests/Unit/ThrowableToProblemDetailsKernelListenerTest.php index d42d39c..6a0f490 100644 --- a/tests/Unit/ThrowableToProblemDetailsKernelListenerTest.php +++ b/tests/Unit/ThrowableToProblemDetailsKernelListenerTest.php @@ -5,17 +5,16 @@ namespace Phauthentic\Symfony\ProblemDetails\Tests\Unit; use Exception; +use Phauthentic\Symfony\ProblemDetails\ExceptionConversion\GenericThrowableConverter; use Phauthentic\Symfony\ProblemDetails\ProblemDetailsFactory; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; use Phauthentic\Symfony\ProblemDetails\ThrowableToProblemDetailsKernelListener; -use RuntimeException; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpKernel\Event\ExceptionEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\HttpFoundation\Request; -use Throwable; /** * @@ -44,8 +43,9 @@ public function testOnKernelException(string $environment, bool $shouldHaveTrace $event = new ExceptionEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST, $throwable); $listener = new ThrowableToProblemDetailsKernelListener( - new ProblemDetailsFactory(), - $environment + [ + new GenericThrowableConverter(new ProblemDetailsFactory(), $environment), + ] ); // Act