diff --git a/CODING-CHALLENGE-8.md b/CODING-CHALLENGE-8.md new file mode 100644 index 0000000..3a118f6 --- /dev/null +++ b/CODING-CHALLENGE-8.md @@ -0,0 +1,14 @@ +# 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 4363406..3d9f1dd 100644 --- a/composer.json +++ b/composer.json @@ -1,7 +1,7 @@ { "type": "project", "license": "proprietary", - "minimum-stability": "dev", + "minimum-stability": "stable", "prefer-stable": true, "require": { "php": ">=8", @@ -26,6 +26,8 @@ "symfony/proxy-manager-bridge": "5.4.*", "symfony/runtime": "5.4.*", "symfony/serializer": "5.4.*", + "symfony/string": "5.4.*", + "symfony/validator": "5.4.*", "symfony/yaml": "5.4.*", "webmozart/assert": "^1.10", "willdurand/negotiation": "^3.0" diff --git a/composer.lock b/composer.lock index b547555..c4c4cba 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": "d66fc5de0e131e7972b58a13ac74bd11", + "content-hash": "39a815772e16e8c23ba4c5f5f7c53fa0", "packages": [ { "name": "brick/math", @@ -1957,30 +1957,30 @@ }, { "name": "psr/log", - "version": "2.0.0", + "version": "1.1.4", "source": { "type": "git", "url": "https://github.com/php-fig/log.git", - "reference": "ef29f6d262798707a9edd554e2b82517ef3a9376" + "reference": "d49695b909c3b7628b6289db5479a1c204601f11" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/php-fig/log/zipball/ef29f6d262798707a9edd554e2b82517ef3a9376", - "reference": "ef29f6d262798707a9edd554e2b82517ef3a9376", + "url": "https://api.github.com/repos/php-fig/log/zipball/d49695b909c3b7628b6289db5479a1c204601f11", + "reference": "d49695b909c3b7628b6289db5479a1c204601f11", "shasum": "" }, "require": { - "php": ">=8.0.0" + "php": ">=5.3.0" }, "type": "library", "extra": { "branch-alias": { - "dev-master": "2.0.x-dev" + "dev-master": "1.1.x-dev" } }, "autoload": { "psr-4": { - "Psr\\Log\\": "src" + "Psr\\Log\\": "Psr/Log/" } }, "notification-url": "https://packagist.org/downloads/", @@ -2001,9 +2001,9 @@ "psr-3" ], "support": { - "source": "https://github.com/php-fig/log/tree/2.0.0" + "source": "https://github.com/php-fig/log/tree/1.1.4" }, - "time": "2021-07-14T16:41:46+00:00" + "time": "2021-05-03T11:20:27+00:00" }, { "name": "ramsey/collection", @@ -5014,6 +5014,196 @@ ], "time": "2021-11-24T10:02:00+00:00" }, + { + "name": "symfony/translation-contracts", + "version": "v2.5.0", + "source": { + "type": "git", + "url": "https://github.com/symfony/translation-contracts.git", + "reference": "d28150f0f44ce854e942b671fc2620a98aae1b1e" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/translation-contracts/zipball/d28150f0f44ce854e942b671fc2620a98aae1b1e", + "reference": "d28150f0f44ce854e942b671fc2620a98aae1b1e", + "shasum": "" + }, + "require": { + "php": ">=7.2.5" + }, + "suggest": { + "symfony/translation-implementation": "" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-main": "2.5-dev" + }, + "thanks": { + "name": "symfony/contracts", + "url": "https://github.com/symfony/contracts" + } + }, + "autoload": { + "psr-4": { + "Symfony\\Contracts\\Translation\\": "" + } + }, + "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/v2.5.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": "2021-08-17T14:20:01+00:00" + }, + { + "name": "symfony/validator", + "version": "v5.4.0", + "source": { + "type": "git", + "url": "https://github.com/symfony/validator.git", + "reference": "68db3401621f75b285cf54ac83e3b89066e08f8d" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/validator/zipball/68db3401621f75b285cf54ac83e3b89066e08f8d", + "reference": "68db3401621f75b285cf54ac83e3b89066e08f8d", + "shasum": "" + }, + "require": { + "php": ">=7.2.5", + "symfony/deprecation-contracts": "^2.1|^3", + "symfony/polyfill-ctype": "~1.8", + "symfony/polyfill-mbstring": "~1.0", + "symfony/polyfill-php73": "~1.0", + "symfony/polyfill-php80": "^1.16", + "symfony/translation-contracts": "^1.1|^2|^3" + }, + "conflict": { + "doctrine/annotations": "<1.13", + "doctrine/cache": "<1.11", + "doctrine/lexer": "<1.0.2", + "phpunit/phpunit": "<5.4.3", + "symfony/dependency-injection": "<4.4", + "symfony/expression-language": "<5.1", + "symfony/http-kernel": "<4.4", + "symfony/intl": "<4.4", + "symfony/property-info": "<5.3", + "symfony/translation": "<4.4", + "symfony/yaml": "<4.4" + }, + "require-dev": { + "doctrine/annotations": "^1.13", + "doctrine/cache": "^1.11|^2.0", + "egulias/email-validator": "^2.1.10|^3", + "symfony/cache": "^4.4|^5.0|^6.0", + "symfony/config": "^4.4|^5.0|^6.0", + "symfony/console": "^4.4|^5.0|^6.0", + "symfony/dependency-injection": "^4.4|^5.0|^6.0", + "symfony/expression-language": "^5.1|^6.0", + "symfony/finder": "^4.4|^5.0|^6.0", + "symfony/http-client": "^4.4|^5.0|^6.0", + "symfony/http-foundation": "^4.4|^5.0|^6.0", + "symfony/http-kernel": "^4.4|^5.0|^6.0", + "symfony/intl": "^4.4|^5.0|^6.0", + "symfony/mime": "^4.4|^5.0|^6.0", + "symfony/property-access": "^4.4|^5.0|^6.0", + "symfony/property-info": "^5.3|^6.0", + "symfony/translation": "^4.4|^5.0|^6.0", + "symfony/yaml": "^4.4|^5.0|^6.0" + }, + "suggest": { + "egulias/email-validator": "Strict (RFC compliant) email validation", + "psr/cache-implementation": "For using the mapping cache.", + "symfony/config": "", + "symfony/expression-language": "For using the Expression validator and the ExpressionLanguageSyntax constraints", + "symfony/http-foundation": "", + "symfony/intl": "", + "symfony/property-access": "For accessing properties within comparison constraints", + "symfony/property-info": "To automatically add NotNull and Type constraints", + "symfony/translation": "For translating validation errors.", + "symfony/yaml": "" + }, + "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/v5.4.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": "2021-11-29T15:30:56+00:00" + }, { "name": "symfony/var-dumper", "version": "v5.4.0", @@ -5733,16 +5923,16 @@ }, { "name": "hautelook/alice-bundle", - "version": "dev-master", + "version": "2.9.0", "source": { "type": "git", "url": "https://github.com/theofidry/AliceBundle.git", - "reference": "ada5be333c79c494330d91d790b1f6674350c3f6" + "reference": "17c5199b2a6efbc1383b0afe1cddfa3c176b7b6f" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/theofidry/AliceBundle/zipball/ada5be333c79c494330d91d790b1f6674350c3f6", - "reference": "ada5be333c79c494330d91d790b1f6674350c3f6", + "url": "https://api.github.com/repos/theofidry/AliceBundle/zipball/17c5199b2a6efbc1383b0afe1cddfa3c176b7b6f", + "reference": "17c5199b2a6efbc1383b0afe1cddfa3c176b7b6f", "shasum": "" }, "require": { @@ -5751,7 +5941,7 @@ "doctrine/orm": "^2.5.11", "doctrine/persistence": "^1.3.4 || ^2.0", "php": "^7.3 || ^8.0", - "psr/log": "^1.0 || ^2.0 || ^3.0", + "psr/log": "^1.0", "symfony/finder": "^3.4 || ^4.0 || ^5.0", "symfony/framework-bundle": "^3.4.24 || ^4.0 || ^5.0", "theofidry/alice-data-fixtures": "^1.4" @@ -5761,7 +5951,6 @@ "phpunit/phpunit": "^8.5", "symfony/phpunit-bridge": "^5.1" }, - "default-branch": true, "type": "symfony-bundle", "extra": { "branch-alias": { @@ -5799,8 +5988,7 @@ "symfony" ], "support": { - "issues": "https://github.com/theofidry/AliceBundle/issues", - "source": "https://github.com/theofidry/AliceBundle/tree/master" + "source": "https://github.com/theofidry/AliceBundle/tree/2.9.0" }, "funding": [ { @@ -5808,7 +5996,7 @@ "type": "github" } ], - "time": "2021-11-04T08:18:45+00:00" + "time": "2021-02-23T08:45:57+00:00" }, { "name": "myclabs/deep-copy", @@ -8125,29 +8313,28 @@ }, { "name": "theofidry/alice-data-fixtures", - "version": "dev-master", + "version": "1.4.0", "source": { "type": "git", "url": "https://github.com/theofidry/AliceDataFixtures.git", - "reference": "9e94af9bb1bd47b0e6069d3760d002f182e5763f" + "reference": "bcfdf64bc940eb4a7b40b46d9ca5251e5692cc11" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/theofidry/AliceDataFixtures/zipball/9e94af9bb1bd47b0e6069d3760d002f182e5763f", - "reference": "9e94af9bb1bd47b0e6069d3760d002f182e5763f", + "url": "https://api.github.com/repos/theofidry/AliceDataFixtures/zipball/bcfdf64bc940eb4a7b40b46d9ca5251e5692cc11", + "reference": "bcfdf64bc940eb4a7b40b46d9ca5251e5692cc11", "shasum": "" }, "require": { "nelmio/alice": "^3.5", - "php": "^7.4 || ^8.0", - "psr/log": "^1 || ^2 || ^3" + "php": "^7.3 | ^8.0", + "psr/log": "^1.0" }, "conflict": { "doctrine/orm": "<2.6.3", - "doctrine/persistence": "<1.4", "illuminate/database": "<5.5", "ocramius/proxy-manager": "<2.1", - "symfony/framework-bundle": "<4.4 || >5.0,<5.3", + "symfony/framework-bundle": "<3.4", "zendframework/zend-code": "<3.3.1" }, "require-dev": { @@ -8169,7 +8356,6 @@ "jackalope/jackalope-doctrine-dbal": "To use Doctrine with the PHPCR flavour", "ocramius/proxy-manager": "To avoid database connection on kernel boot" }, - "default-branch": true, "type": "library", "extra": { "bamarni-bin": { @@ -8206,7 +8392,7 @@ ], "support": { "issues": "https://github.com/theofidry/AliceDataFixtures/issues", - "source": "https://github.com/theofidry/AliceDataFixtures/tree/master" + "source": "https://github.com/theofidry/AliceDataFixtures/tree/1.4.0" }, "funding": [ { @@ -8214,7 +8400,7 @@ "type": "github" } ], - "time": "2021-10-17T09:13:11+00:00" + "time": "2021-01-25T10:07:17+00:00" }, { "name": "theseer/tokenizer", @@ -8268,7 +8454,7 @@ } ], "aliases": [], - "minimum-stability": "dev", + "minimum-stability": "stable", "stability-flags": [], "prefer-stable": true, "prefer-lowest": false, diff --git a/config/packages/test/validator.yaml b/config/packages/test/validator.yaml new file mode 100644 index 0000000..1e5ab78 --- /dev/null +++ b/config/packages/test/validator.yaml @@ -0,0 +1,3 @@ +framework: + validation: + not_compromised_password: false diff --git a/config/packages/validator.yaml b/config/packages/validator.yaml new file mode 100644 index 0000000..350786a --- /dev/null +++ b/config/packages/validator.yaml @@ -0,0 +1,8 @@ +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\: [] diff --git a/src/ArgumentValueResolver/CreateAttendeeModelResolver.php b/src/ArgumentValueResolver/CreateAttendeeModelResolver.php index be4493f..7eec489 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 SerializerInterface $serializer, + private ValidatorInterface $validator, ) { } @@ -27,10 +30,23 @@ public function supports(Request $request, ArgumentMetadata $argument): bool */ public function resolve(Request $request, ArgumentMetadata $argument) { - 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 6182dea..3ba6ace 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 SerializerInterface $serializer, + private ValidatorInterface $validator, ) { } @@ -27,10 +30,23 @@ public function supports(Request $request, ArgumentMetadata $argument): bool */ public function resolve(Request $request, ArgumentMetadata $argument) { - 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 BadRequestHttpException 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 5f95e5b..c7bafd9 100644 --- a/symfony.lock +++ b/symfony.lock @@ -461,6 +461,22 @@ "symfony/string": { "version": "v5.4.0-RC1" }, + "symfony/translation-contracts": { + "version": "v3.0.0" + }, + "symfony/validator": { + "version": "5.4", + "recipe": { + "repo": "github.com/symfony/recipes", + "branch": "master", + "version": "4.3", + "ref": "3eb8df139ec05414489d55b97603c5f6ca0c44cb" + }, + "files": [ + "config/packages/test/validator.yaml", + "config/packages/validator.yaml" + ] + }, "symfony/var-dumper": { "version": "v5.4.0-RC1" }, diff --git a/tests/Controller/Attendee/CreateControllerTest.php b/tests/Controller/Attendee/CreateControllerTest.php index 70509e6..9f09894 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 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..fea8162 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 function provideUnprocessableAttendeeData(): \Generator + { + yield 'no data' => ['']; + yield 'wrong email' => ['{"email": "paulpaulsende"}']; + } }