From e72d49c3e9697ff0580ca5de457773cf3d9e4d40 Mon Sep 17 00:00:00 2001 From: soyuka Date: Fri, 25 Oct 2024 12:41:35 +0200 Subject: [PATCH] fix(laravel): jsonapi error serialization --- src/JsonApi/Serializer/ErrorNormalizer.php | 29 +++++- .../ReservedAttributeNameConverter.php | 5 ++ src/Laravel/ApiPlatformProvider.php | 14 ++- src/Laravel/ApiResource/Error.php | 16 ++-- src/Laravel/ApiResource/ValidationError.php | 14 +-- src/Laravel/State/ValidateProvider.php | 9 +- src/Laravel/Tests/EloquentTest.php | 8 +- src/Laravel/Tests/JsonApiTest.php | 88 +++++++++++++++++-- .../app/ApiResource/RuleValidation.php | 29 ++++++ src/State/ApiResource/Error.php | 8 +- 10 files changed, 191 insertions(+), 29 deletions(-) create mode 100644 src/Laravel/workbench/app/ApiResource/RuleValidation.php diff --git a/src/JsonApi/Serializer/ErrorNormalizer.php b/src/JsonApi/Serializer/ErrorNormalizer.php index c6a5997283b..af619b81e46 100644 --- a/src/JsonApi/Serializer/ErrorNormalizer.php +++ b/src/JsonApi/Serializer/ErrorNormalizer.php @@ -37,9 +37,34 @@ public function normalize(mixed $object, ?string $format = null, array $context $jsonApiObject = $this->itemNormalizer->normalize($object, $format, $context); $error = $jsonApiObject['data']['attributes']; $error['id'] = $jsonApiObject['data']['id']; - $error['type'] = $jsonApiObject['data']['id']; + if (isset($error['type'])) { + $error['links'] = ['type' => $error['type']]; + } + + if (!isset($error['code']) && method_exists($object, 'getId')) { + $error['code'] = $object->getId(); + } + + if (!isset($error['violations'])) { + return ['errors' => [$error]]; + } + + $errors = []; + foreach ($error['violations'] as $violation) { + $e = ['detail' => $violation['message']] + $error; + if (isset($error['links']['type'])) { + $type = $error['links']['type']; + $e['links']['type'] = \sprintf('%s/%s', $type, $violation['propertyPath']); + $e['id'] = str_replace($type, $e['links']['type'], $e['id']); + } + if (isset($e['code'])) { + $e['code'] = \sprintf('%s/%s', $error['code'], $violation['propertyPath']); + } + unset($e['violations']); + $errors[] = $e; + } - return ['errors' => [$error]]; + return ['errors' => $errors]; } /** diff --git a/src/JsonApi/Serializer/ReservedAttributeNameConverter.php b/src/JsonApi/Serializer/ReservedAttributeNameConverter.php index e25ad566dc9..1c9e5239e8b 100644 --- a/src/JsonApi/Serializer/ReservedAttributeNameConverter.php +++ b/src/JsonApi/Serializer/ReservedAttributeNameConverter.php @@ -13,6 +13,7 @@ namespace ApiPlatform\JsonApi\Serializer; +use ApiPlatform\Metadata\Exception\ProblemExceptionInterface; use Symfony\Component\Serializer\NameConverter\AdvancedNameConverterInterface; use Symfony\Component\Serializer\NameConverter\NameConverterInterface; @@ -44,6 +45,10 @@ public function normalize(string $propertyName, ?string $class = null, ?string $ $propertyName = $this->nameConverter->normalize($propertyName, $class, $format, $context); } + if ($class && is_a($class, ProblemExceptionInterface::class, true)) { + return $propertyName; + } + if (isset(self::JSON_API_RESERVED_ATTRIBUTES[$propertyName])) { $propertyName = self::JSON_API_RESERVED_ATTRIBUTES[$propertyName]; } diff --git a/src/Laravel/ApiPlatformProvider.php b/src/Laravel/ApiPlatformProvider.php index a9725ebdbea..a71a43ebb92 100644 --- a/src/Laravel/ApiPlatformProvider.php +++ b/src/Laravel/ApiPlatformProvider.php @@ -58,6 +58,7 @@ use ApiPlatform\JsonApi\JsonSchema\SchemaFactory as JsonApiSchemaFactory; use ApiPlatform\JsonApi\Serializer\CollectionNormalizer as JsonApiCollectionNormalizer; use ApiPlatform\JsonApi\Serializer\EntrypointNormalizer as JsonApiEntrypointNormalizer; +use ApiPlatform\JsonApi\Serializer\ErrorNormalizer as JsonApiErrorNormalizer; use ApiPlatform\JsonApi\Serializer\ItemNormalizer as JsonApiItemNormalizer; use ApiPlatform\JsonApi\Serializer\ObjectNormalizer as JsonApiObjectNormalizer; use ApiPlatform\JsonApi\Serializer\ReservedAttributeNameConverter; @@ -907,6 +908,10 @@ public function register(): void return new ReservedAttributeNameConverter($app->make(NameConverterInterface::class)); }); + if (interface_exists(FieldsBuilderEnumInterface::class)) { + $this->registerGraphQl($this->app); + } + $this->app->singleton(JsonApiEntrypointNormalizer::class, function (Application $app) { return new JsonApiEntrypointNormalizer( $app->make(ResourceMetadataCollectionFactoryInterface::class), @@ -946,9 +951,11 @@ public function register(): void ); }); - if (interface_exists(FieldsBuilderEnumInterface::class)) { - $this->registerGraphQl($this->app); - } + $this->app->singleton(JsonApiErrorNormalizer::class, function (Application $app) { + return new JsonApiErrorNormalizer( + $app->make(JsonApiItemNormalizer::class), + ); + }); $this->app->singleton(JsonApiObjectNormalizer::class, function (Application $app) { return new JsonApiObjectNormalizer( @@ -985,6 +992,7 @@ public function register(): void $list->insert($app->make(JsonApiEntrypointNormalizer::class), -800); $list->insert($app->make(JsonApiCollectionNormalizer::class), -985); $list->insert($app->make(JsonApiItemNormalizer::class), -890); + $list->insert($app->make(JsonApiErrorNormalizer::class), -790); $list->insert($app->make(JsonApiObjectNormalizer::class), -995); if (interface_exists(FieldsBuilderEnumInterface::class)) { diff --git a/src/Laravel/ApiResource/Error.php b/src/Laravel/ApiResource/Error.php index fc4a2963a48..69e0bbc4e1c 100644 --- a/src/Laravel/ApiResource/Error.php +++ b/src/Laravel/ApiResource/Error.php @@ -52,7 +52,7 @@ name: '_api_errors_jsonapi', outputFormats: ['jsonapi' => ['application/vnd.api+json']], normalizationContext: ['groups' => ['jsonapi'], 'skip_null_values' => true], - uriTemplate: '/errros/{status}.jsonapi' + uriTemplate: '/errors/{status}.jsonapi' ), ], graphQlOperations: [] @@ -124,6 +124,12 @@ public function getStatusCode(): int return $this->status; } + #[Groups(['jsonapi'])] + public function getId(): string + { + return (string) $this->status; + } + /** * @param array $headers */ @@ -132,7 +138,7 @@ public function setHeaders(array $headers): void $this->headers = $headers; } - #[Groups(['jsonld', 'jsonproblem'])] + #[Groups(['jsonld', 'jsonproblem', 'jsonapi'])] public function getType(): string { return $this->type; @@ -149,7 +155,7 @@ public function setType(string $type): void $this->type = $type; } - #[Groups(['jsonld', 'jsonproblem'])] + #[Groups(['jsonld', 'jsonproblem', 'jsonapi'])] public function getStatus(): ?int { return $this->status; @@ -160,13 +166,13 @@ public function setStatus(int $status): void $this->status = $status; } - #[Groups(['jsonld', 'jsonproblem'])] + #[Groups(['jsonld', 'jsonproblem', 'jsonapi'])] public function getDetail(): ?string { return $this->detail; } - #[Groups(['jsonld', 'jsonproblem'])] + #[Groups(['jsonld', 'jsonproblem', 'jsonapi'])] public function getInstance(): ?string { return $this->instance; diff --git a/src/Laravel/ApiResource/ValidationError.php b/src/Laravel/ApiResource/ValidationError.php index 0eb36413462..92197a74e2b 100644 --- a/src/Laravel/ApiResource/ValidationError.php +++ b/src/Laravel/ApiResource/ValidationError.php @@ -86,25 +86,25 @@ public function getId(): string } #[SerializedName('description')] - #[Groups(['jsonapi', 'jsonld', 'json'])] + #[Groups(['jsonld', 'json'])] public function getDescription(): string { return $this->detail; } - #[Groups(['jsonld', 'json'])] + #[Groups(['jsonld', 'json', 'jsonapi'])] public function getType(): string { return '/validation_errors/'.$this->id; } - #[Groups(['jsonld', 'json'])] + #[Groups(['jsonld', 'json', 'jsonapi'])] public function getTitle(): ?string { return 'Validation Error'; } - #[Groups(['jsonld', 'json'])] + #[Groups(['jsonld', 'json', 'jsonapi'])] private string $detail; public function getDetail(): ?string @@ -117,7 +117,7 @@ public function setDetail(string $detail): void $this->detail = $detail; } - #[Groups(['jsonld', 'json'])] + #[Groups(['jsonld', 'json', 'jsonapi'])] public function getStatus(): ?int { return $this->status; @@ -128,7 +128,7 @@ public function setStatus(int $status): void $this->status = $status; } - #[Groups(['jsonld', 'json'])] + #[Groups(['jsonld', 'json', 'jsonapi'])] public function getInstance(): ?string { return null; @@ -138,7 +138,7 @@ public function getInstance(): ?string * @return array */ #[SerializedName('violations')] - #[Groups(['json', 'jsonld'])] + #[Groups(['json', 'jsonld', 'jsonapi'])] public function getViolations(): array { return $this->violations; diff --git a/src/Laravel/State/ValidateProvider.php b/src/Laravel/State/ValidateProvider.php index 499c4a0e7ef..5e065ffd1b4 100644 --- a/src/Laravel/State/ValidateProvider.php +++ b/src/Laravel/State/ValidateProvider.php @@ -74,7 +74,14 @@ public function provide(Operation $operation, array $uriVariables = [], array $c return $body; } - $validator = Validator::make($request->request->all(), $rules); + // In Symfony, validation is done on the Resource object (here $body) using Deserialization before Validation + // Here, we did not deserialize yet, we validate on the raw body before. + $validationBody = $request->request->all(); + if ('jsonapi' === $request->getRequestFormat()) { + $validationBody = $validationBody['data']['attributes']; + } + + $validator = Validator::make($validationBody, $rules); if ($validator->fails()) { throw $this->getValidationError($validator, new ValidationException($validator)); } diff --git a/src/Laravel/Tests/EloquentTest.php b/src/Laravel/Tests/EloquentTest.php index 23960b7ad20..ad847eeced9 100644 --- a/src/Laravel/Tests/EloquentTest.php +++ b/src/Laravel/Tests/EloquentTest.php @@ -386,11 +386,11 @@ public function testRangeGreaterThanEqualFilter(): void 'Content-Type' => ['application/merge-patch+json'], ] ); - $response = $this->get('api/books?isbn_range[gte]='.$updated['isbn'], ['Accept' => ['application/ld+json']]); - $this->assertSame($response->json()['member'][0]['@id'], $bookBefore['@id']); - $this->assertSame($response->json()['member'][1]['@id'], $bookAfter['@id']); - $this->assertSame($response->json()['totalItems'], 2); + $json = $response->json(); + $this->assertSame($json['member'][0]['@id'], $bookBefore['@id']); + $this->assertSame($json['member'][1]['@id'], $bookAfter['@id']); + $this->assertSame($json['totalItems'], 2); } public function testWrongOrderFilter(): void diff --git a/src/Laravel/Tests/JsonApiTest.php b/src/Laravel/Tests/JsonApiTest.php index 74833c3ebc3..95492a9b0c6 100644 --- a/src/Laravel/Tests/JsonApiTest.php +++ b/src/Laravel/Tests/JsonApiTest.php @@ -39,6 +39,7 @@ protected function defineEnvironment($app): void tap($app['config'], function (Repository $config): void { $config->set('api-platform.formats', ['jsonapi' => ['application/vnd.api+json']]); $config->set('api-platform.docs_formats', ['jsonapi' => ['application/vnd.api+json']]); + $config->set('api-platform.resources', [app_path('Models'), app_path('ApiResource')]); $config->set('app.debug', true); }); } @@ -48,13 +49,15 @@ public function testGetEntrypoint(): void $response = $this->get('/api/', ['accept' => ['application/vnd.api+json']]); $response->assertStatus(200); $response->assertHeader('content-type', 'application/vnd.api+json; charset=utf-8'); - $this->assertJsonContains([ - 'links' => [ - 'self' => 'http://localhost/api', - 'book' => 'http://localhost/api/books', + $this->assertJsonContains( + [ + 'links' => [ + 'self' => 'http://localhost/api', + 'book' => 'http://localhost/api/books', + ], ], - ], - $response->json()); + $response->json() + ); } public function testGetCollection(): void @@ -209,4 +212,77 @@ public function testRelationWithGroups(): void $this->assertArrayHasKey('relation', $content['data']['relationships']); $this->assertArrayHasKey('data', $content['data']['relationships']['relation']); } + + public function testValidateJsonApi(): void + { + $response = $this->postJson( + '/api/issue6745/rule_validations', + [ + 'data' => [ + 'type' => 'string', + 'attributes' => ['max' => 3], + ], + ], + [ + 'accept' => 'application/vnd.api+json', + 'content_type' => 'application/vnd.api+json', + ] + ); + + $response->assertStatus(422); + $response->assertHeader('content-type', 'application/vnd.api+json; charset=utf-8'); + $json = $response->json(); + $this->assertJsonContains([ + 'errors' => [ + [ + 'detail' => 'The prop field is required.', + 'title' => 'Validation Error', + 'status' => 422, + 'code' => '58350900e0fc6b8e/prop', + ], + [ + 'detail' => 'The max field must be less than 2.', + 'title' => 'Validation Error', + 'status' => 422, + 'code' => '58350900e0fc6b8e/max', + ], + ], + ], $json); + + $this->assertArrayHasKey('id', $json['errors'][0]); + $this->assertArrayHasKey('links', $json['errors'][0]); + $this->assertArrayHasKey('type', $json['errors'][0]['links']); + + $response = $this->postJson( + '/api/issue6745/rule_validations', + [ + 'data' => [ + 'type' => 'string', + 'attributes' => [ + 'prop' => 1, + 'max' => 1, + ], + ], + ], + [ + 'accept' => 'application/vnd.api+json', + 'content_type' => 'application/vnd.api+json', + ] + ); + $response->assertStatus(201); + } + + public function testNotFound(): void + { + $response = $this->get('/api/books/notfound', headers: ['accept' => 'application/vnd.api+json']); + $response->assertStatus(404); + $response->assertHeader('content-type', 'application/vnd.api+json; charset=utf-8'); + + $this->assertJsonContains([ + 'links' => ['type' => '/errors/404'], + 'title' => 'An error occurred', + 'status' => 404, + 'detail' => 'Not Found', + ], $response->json()['errors'][0]); + } } diff --git a/src/Laravel/workbench/app/ApiResource/RuleValidation.php b/src/Laravel/workbench/app/ApiResource/RuleValidation.php new file mode 100644 index 00000000000..2922b5d66cd --- /dev/null +++ b/src/Laravel/workbench/app/ApiResource/RuleValidation.php @@ -0,0 +1,29 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Workbench\App\ApiResource; + +use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\Post; + +#[ApiResource( + uriTemplate: '/issue6745/rule_validations', + operations: [new Post()], + rules: ['prop' => 'required', 'max' => 'lt:2'] +)] +class RuleValidation +{ + public function __construct(public int $prop, public ?int $max = null) + { + } +} diff --git a/src/State/ApiResource/Error.php b/src/State/ApiResource/Error.php index 4a5061a65a1..ec7e816824f 100644 --- a/src/State/ApiResource/Error.php +++ b/src/State/ApiResource/Error.php @@ -90,6 +90,12 @@ public function __construct( } } + #[Groups(['jsonapi'])] + public function getId(): string + { + return (string) $this->status; + } + #[SerializedName('trace')] #[Groups(['trace'])] public ?array $originalTrace = null; @@ -129,7 +135,7 @@ public function setHeaders(array $headers): void $this->headers = $headers; } - #[Groups(['jsonld', 'jsonproblem'])] + #[Groups(['jsonld', 'jsonproblem', 'jsonapi'])] public function getType(): string { return $this->type;