diff --git a/CODING-CHALLENGE-8.md b/CODING-CHALLENGE-8.md new file mode 100644 index 0000000..1e0c963 --- /dev/null +++ b/CODING-CHALLENGE-8.md @@ -0,0 +1,13 @@ +# RESTful Webservices in Symfony + +## Coding Challenge 8 - Error Handling + +### Tasks + +- centralize the error handling + +### Solution + +- throw a `ValidationFailedException` on validation errors +- introduce an `ExceptionListener` to catch the `ValidationFailedException` +- in the `ExceptionListener` create an error representation of the caught exception and fill the response content with it diff --git a/composer.json b/composer.json index 23b8d21..cbf94a0 100644 --- a/composer.json +++ b/composer.json @@ -22,6 +22,7 @@ "symfony/property-info": "6.3.*", "symfony/runtime": "6.3.*", "symfony/serializer": "6.3.*", + "symfony/validator": "6.3.*", "symfony/yaml": "6.3.*", "webmozart/assert": "^1.11", "willdurand/negotiation": "^3.1" diff --git a/composer.lock b/composer.lock index e5832d7..d232182 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "5596bf499dc7ce184640fa9df83b1cb1", + "content-hash": "fab137f8283c2f0c56444153bddecb62", "packages": [ { "name": "brick/math", @@ -4459,6 +4459,180 @@ ], "time": "2023-07-05T08:41:27+00:00" }, + { + "name": "symfony/translation-contracts", + "version": "v3.3.0", + "source": { + "type": "git", + "url": "https://github.com/symfony/translation-contracts.git", + "reference": "02c24deb352fb0d79db5486c0c79905a85e37e86" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/translation-contracts/zipball/02c24deb352fb0d79db5486c0c79905a85e37e86", + "reference": "02c24deb352fb0d79db5486c0c79905a85e37e86", + "shasum": "" + }, + "require": { + "php": ">=8.1" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-main": "3.4-dev" + }, + "thanks": { + "name": "symfony/contracts", + "url": "https://github.com/symfony/contracts" + } + }, + "autoload": { + "psr-4": { + "Symfony\\Contracts\\Translation\\": "" + }, + "exclude-from-classmap": [ + "/Test/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Nicolas Grekas", + "email": "p@tchwork.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Generic abstractions related to translation", + "homepage": "https://symfony.com", + "keywords": [ + "abstractions", + "contracts", + "decoupling", + "interfaces", + "interoperability", + "standards" + ], + "support": { + "source": "https://github.com/symfony/translation-contracts/tree/v3.3.0" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2023-05-30T17:17:10+00:00" + }, + { + "name": "symfony/validator", + "version": "v6.3.4", + "source": { + "type": "git", + "url": "https://github.com/symfony/validator.git", + "reference": "0c8435154920b9bbe93bece675234c244cadf73b" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/validator/zipball/0c8435154920b9bbe93bece675234c244cadf73b", + "reference": "0c8435154920b9bbe93bece675234c244cadf73b", + "shasum": "" + }, + "require": { + "php": ">=8.1", + "symfony/deprecation-contracts": "^2.5|^3", + "symfony/polyfill-ctype": "~1.8", + "symfony/polyfill-mbstring": "~1.0", + "symfony/polyfill-php83": "^1.27", + "symfony/translation-contracts": "^2.5|^3" + }, + "conflict": { + "doctrine/annotations": "<1.13", + "doctrine/lexer": "<1.1", + "symfony/dependency-injection": "<5.4", + "symfony/expression-language": "<5.4", + "symfony/http-kernel": "<5.4", + "symfony/intl": "<5.4", + "symfony/property-info": "<5.4", + "symfony/translation": "<5.4", + "symfony/yaml": "<5.4" + }, + "require-dev": { + "doctrine/annotations": "^1.13|^2", + "egulias/email-validator": "^2.1.10|^3|^4", + "symfony/cache": "^5.4|^6.0", + "symfony/config": "^5.4|^6.0", + "symfony/console": "^5.4|^6.0", + "symfony/dependency-injection": "^5.4|^6.0", + "symfony/expression-language": "^5.4|^6.0", + "symfony/finder": "^5.4|^6.0", + "symfony/http-client": "^5.4|^6.0", + "symfony/http-foundation": "^5.4|^6.0", + "symfony/http-kernel": "^5.4|^6.0", + "symfony/intl": "^5.4|^6.0", + "symfony/mime": "^5.4|^6.0", + "symfony/property-access": "^5.4|^6.0", + "symfony/property-info": "^5.4|^6.0", + "symfony/translation": "^5.4|^6.0", + "symfony/yaml": "^5.4|^6.0" + }, + "type": "library", + "autoload": { + "psr-4": { + "Symfony\\Component\\Validator\\": "" + }, + "exclude-from-classmap": [ + "/Tests/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Fabien Potencier", + "email": "fabien@symfony.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Provides tools to validate values", + "homepage": "https://symfony.com", + "support": { + "source": "https://github.com/symfony/validator/tree/v6.3.4" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2023-08-17T15:49:05+00:00" + }, { "name": "symfony/var-dumper", "version": "v6.3.4", diff --git a/config/packages/validator.yaml b/config/packages/validator.yaml new file mode 100644 index 0000000..0201281 --- /dev/null +++ b/config/packages/validator.yaml @@ -0,0 +1,13 @@ +framework: + validation: + email_validation_mode: html5 + + # Enables validator auto-mapping support. + # For instance, basic validation constraints will be inferred from Doctrine's metadata. + #auto_mapping: + # App\Entity\: [] + +when@test: + framework: + validation: + not_compromised_password: false diff --git a/config/services.yaml b/config/services.yaml index cdad506..334535c 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -26,3 +26,11 @@ services: # add more service definitions when explicit configuration is needed # please note that last definitions always *replace* previous ones + + + +when@test: + services: + Psr\Log\NullLogger: ~ + # Disable logger to avoid showing errors during tests + logger: '@Psr\Log\NullLogger' \ No newline at end of file diff --git a/src/ArgumentValueResolver/CreateAttendeeModelResolver.php b/src/ArgumentValueResolver/CreateAttendeeModelResolver.php index d99a1bf..ac35296 100644 --- a/src/ArgumentValueResolver/CreateAttendeeModelResolver.php +++ b/src/ArgumentValueResolver/CreateAttendeeModelResolver.php @@ -8,12 +8,15 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Controller\ArgumentValueResolverInterface; use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata; +use Symfony\Component\HttpKernel\Exception\UnprocessableEntityHttpException; use Symfony\Component\Serializer\SerializerInterface; +use Symfony\Component\Validator\Validator\ValidatorInterface; final class CreateAttendeeModelResolver implements ArgumentValueResolverInterface { public function __construct( private readonly SerializerInterface $serializer, + private readonly ValidatorInterface $validator ) { } @@ -29,10 +32,23 @@ public function resolve(Request $request, ArgumentMetadata $argument): iterable { // it returns an iterable, because the controller method argument could be variadic - yield $this->serializer->deserialize( - $request->getContent(), - CreateAttendeeModel::class, - $request->getRequestFormat(), - ); + try { + $model = $this->serializer->deserialize( + $request->getContent(), + CreateAttendeeModel::class, + $request->getRequestFormat(), + ); + } catch (\Exception $exception) { + throw new UnprocessableEntityHttpException(); + } + + $validationErrors = $this->validator->validate($model); + + if (\count($validationErrors) > 0) { + // throw a UnprocessableEntityHttpException for now, we will introduce proper ApiExceptions later + throw new UnprocessableEntityHttpException(); + } + + yield $model; } } diff --git a/src/ArgumentValueResolver/UpdateAttendeeModelResolver.php b/src/ArgumentValueResolver/UpdateAttendeeModelResolver.php index 8f033af..4aa7197 100644 --- a/src/ArgumentValueResolver/UpdateAttendeeModelResolver.php +++ b/src/ArgumentValueResolver/UpdateAttendeeModelResolver.php @@ -8,12 +8,15 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Controller\ArgumentValueResolverInterface; use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata; +use Symfony\Component\HttpKernel\Exception\UnprocessableEntityHttpException; use Symfony\Component\Serializer\SerializerInterface; +use Symfony\Component\Validator\Validator\ValidatorInterface; final class UpdateAttendeeModelResolver implements ArgumentValueResolverInterface { public function __construct( private readonly SerializerInterface $serializer, + private readonly ValidatorInterface $validator ) { } @@ -27,10 +30,23 @@ public function supports(Request $request, ArgumentMetadata $argument): bool */ public function resolve(Request $request, ArgumentMetadata $argument): iterable { - yield $this->serializer->deserialize( - $request->getContent(), - UpdateAttendeeModel::class, - $request->getRequestFormat(), - ); + try { + $model = $this->serializer->deserialize( + $request->getContent(), + UpdateAttendeeModel::class, + $request->getRequestFormat(), + ); + } catch (\Exception $exception) { + throw new UnprocessableEntityHttpException(); + } + + $validationErrors = $this->validator->validate($model); + + if (\count($validationErrors) > 0) { + // throw a UnprocessableEntityHttpException for now, we will introduce proper ApiExceptions later + throw new UnprocessableEntityHttpException(); + } + + yield $model; } } diff --git a/src/Domain/Model/CreateAttendeeModel.php b/src/Domain/Model/CreateAttendeeModel.php index 277501b..691675c 100644 --- a/src/Domain/Model/CreateAttendeeModel.php +++ b/src/Domain/Model/CreateAttendeeModel.php @@ -4,9 +4,18 @@ namespace App\Domain\Model; +use Symfony\Component\Validator\Constraints\Email; +use Symfony\Component\Validator\Constraints\NotBlank; + final class CreateAttendeeModel { + #[NotBlank] public string $firstname; + + #[NotBlank] public string $lastname; + + #[NotBlank] + #[Email] public string $email; } diff --git a/src/Domain/Model/UpdateAttendeeModel.php b/src/Domain/Model/UpdateAttendeeModel.php index 98a6d82..cf2b7f1 100644 --- a/src/Domain/Model/UpdateAttendeeModel.php +++ b/src/Domain/Model/UpdateAttendeeModel.php @@ -4,9 +4,18 @@ namespace App\Domain\Model; +use Symfony\Component\Validator\Constraints\Email; +use Symfony\Component\Validator\Constraints\NotBlank; + final class UpdateAttendeeModel { + #[NotBlank(allowNull: true)] public ?string $firstname = null; + + #[NotBlank(allowNull: true)] public ?string $lastname = null; + + #[NotBlank(allowNull: true)] + #[Email] public ?string $email = null; } diff --git a/symfony.lock b/symfony.lock index b1e4a3d..204fd77 100644 --- a/symfony.lock +++ b/symfony.lock @@ -160,6 +160,18 @@ "config/routes.yaml" ] }, + "symfony/validator": { + "version": "6.3", + "recipe": { + "repo": "github.com/symfony/recipes", + "branch": "main", + "version": "5.3", + "ref": "c32cfd98f714894c4f128bb99aa2530c1227603c" + }, + "files": [ + "config/packages/validator.yaml" + ] + }, "theofidry/alice-data-fixtures": { "version": "1.6", "recipe": { diff --git a/tests/Controller/Attendee/CreateControllerTest.php b/tests/Controller/Attendee/CreateControllerTest.php index 70509e6..0618017 100644 --- a/tests/Controller/Attendee/CreateControllerTest.php +++ b/tests/Controller/Attendee/CreateControllerTest.php @@ -38,4 +38,28 @@ public function test_it_should_create_an_attendee(): void static::assertSame('Paulsen', $expectedAttendee->getLastname()); static::assertSame('paul@paulsen.de', $expectedAttendee->getEmail()); } + + /** + * @dataProvider provideUnprocessableAttendeeData + */ + public function test_it_should_throw_an_UnprocessableEntityHttpException(string $requestBody): void + { + $this->browser->request('POST', '/attendees', [], [], [], $requestBody); + + static::assertResponseStatusCodeSame(422); + static::assertStringContainsString( + 'UnprocessableEntityHttpException', + $this->browser->getResponse()->getContent() + ); + } + + public static function provideUnprocessableAttendeeData(): \Generator + { + yield 'no data' => ['']; + yield 'empty data' => ['{}']; + yield 'missing firstname' => ['{"lastname": "Paulsen", "email": "paul@paulsen.de"}']; + yield 'missing lastname' => ['{"firstname": "Paul", "email": "paul@paulsen.de"}']; + yield 'missing email' => ['{"firstname": "Paul", "lastname": "Paulsen"}']; + yield 'wrong email' => ['{"firstname": "Paul", "lastname": "Paulsen", "email": "paulpaulsende"}']; + } } diff --git a/tests/Controller/Attendee/UpdateControllerTest.php b/tests/Controller/Attendee/UpdateControllerTest.php index 5f11654..c2fe9b3 100644 --- a/tests/Controller/Attendee/UpdateControllerTest.php +++ b/tests/Controller/Attendee/UpdateControllerTest.php @@ -40,4 +40,31 @@ public function test_it_should_update_an_attendee(): void static::assertSame('Paulsen', $expectedAttendee->getLastname()); static::assertSame('paul@paulsen.de', $expectedAttendee->getEmail()); } + + /** + * @dataProvider provideUnprocessableAttendeeData + */ + public function test_it_should_throw_an_UnprocessableEntityHttpException(string $requestBody): void + { + $this->loadFixtures([ + __DIR__.'/fixtures/update_attendee.yaml', + ]); + + $attendeesBefore = static::getContainer()->get(AttendeeRepository::class)->findAll(); + static::assertCount(1, $attendeesBefore); + + $this->browser->request('PUT', '/attendees/b38aa3e4-f9de-4dca-aaeb-3ec36a9feb6c', [], [], [], $requestBody); + + static::assertResponseStatusCodeSame(422); + static::assertStringContainsString( + 'UnprocessableEntityHttpException', + $this->browser->getResponse()->getContent() + ); + } + + public static function provideUnprocessableAttendeeData(): \Generator + { + yield 'no data' => ['']; + yield 'wrong email' => ['{"email": "paulpaulsende"}']; + } }