feat(jsonapi): spec-compliant PATCH conflicts and member names (#8752)

* feat(jsonapi): 409 on PATCH id/type mismatch

Fixes #8750

* feat(jsonapi): opt-in valid reserved member names

Fixes #8749

Author: soyuka

src/JsonApi/JsonSchema/SchemaFactory.php

@@ -82,7 +82,7 @@ final class SchemaFactory implements SchemaFactoryInterface, SchemaFactoryAwareI
 
     private readonly ResourceLinkageResolver $resourceLinkageResolver;
 
-    public function __construct(private readonly SchemaFactoryInterface $schemaFactory, private readonly PropertyMetadataFactoryInterface $propertyMetadataFactory, ResourceClassResolverInterface $resourceClassResolver, ?ResourceMetadataCollectionFactoryInterface $resourceMetadataFactory = null, private ?DefinitionNameFactoryInterface $definitionNameFactory = null, ?ResourceLinkageResolver $resourceLinkageResolver = null)
+    public function __construct(private readonly SchemaFactoryInterface $schemaFactory, private readonly PropertyMetadataFactoryInterface $propertyMetadataFactory, ResourceClassResolverInterface $resourceClassResolver, ?ResourceMetadataCollectionFactoryInterface $resourceMetadataFactory = null, private ?DefinitionNameFactoryInterface $definitionNameFactory = null, ?ResourceLinkageResolver $resourceLinkageResolver = null, private readonly bool $strictMemberNames = false)
     {
         if (!$definitionNameFactory) {
             $this->definitionNameFactory = new DefinitionNameFactory();
@@ -327,7 +327,7 @@ private function buildDefinitionPropertiesSchema(string $key, string $className,
                     $relatedDefinitions[$ref] = ['$ref' => $ref];
                 }
                 // A relationship literally named "relationships"/"included" is prefixed at runtime too.
-                $relationshipName = ReservedAttributeNameConverter::JSON_API_RESERVED_ATTRIBUTES[$propertyName] ?? $propertyName;
+                $relationshipName = ReservedAttributeNameConverter::getReservedAttributes($this->strictMemberNames)[$propertyName] ?? $propertyName;
                 if ($isOne) {
                     $relationships[$relationshipName]['properties']['data'] = [
                         'oneOf' => [
@@ -346,7 +346,7 @@ private function buildDefinitionPropertiesSchema(string $key, string $className,
 
             // Reserved names (id, type, links, relationships, included) are prefixed by the
             // ReservedAttributeNameConverter at runtime; mirror that single source of truth here.
-            $attributes[ReservedAttributeNameConverter::JSON_API_RESERVED_ATTRIBUTES[$propertyName] ?? $propertyName] = $property;
+            $attributes[ReservedAttributeNameConverter::getReservedAttributes($this->strictMemberNames)[$propertyName] ?? $propertyName] = $property;
         }
 
         $replacement = self::PROPERTY_PROPS;

src/JsonApi/Serializer/ItemNormalizerTrait.php

@@ -58,6 +58,11 @@ public function denormalize(mixed $data, string $type, ?string $format = null, a
         $this->assertValidRequestDocument($data, $operation, $type, $context);
 
         $isPostOperation = $operation instanceof HttpOperation && 'POST' === $operation->getMethod();
+        if (!$isPostOperation && $operation instanceof HttpOperation && \is_object($context[AbstractItemNormalizer::OBJECT_TO_POPULATE] ?? null)) {
+            if (isset($data['data']['id'])) {
+                $this->assertIdMatchesEndpoint((string) $data['data']['id'], $context[AbstractItemNormalizer::OBJECT_TO_POPULATE], $operation, $type);
+            }
+        }
         $allowClientGeneratedId = true === ($context[ItemNormalizer::ALLOW_CLIENT_GENERATED_ID] ?? $this->defaultContext[ItemNormalizer::ALLOW_CLIENT_GENERATED_ID] ?? false);
 
         // Avoid issues with proxies if we populated the object
@@ -87,7 +92,7 @@ public function denormalize(mixed $data, string $type, ?string $format = null, a
 
         // Surface the client-generated id so the entity setter receives it.
         if ($isPostOperation && $allowClientGeneratedId && isset($data['data']['id'])) {
-            $dataToDenormalize[ReservedAttributeNameConverter::JSON_API_RESERVED_ATTRIBUTES['id']] = $data['data']['id'];
+            $dataToDenormalize[$this->nameConverter?->normalize('id', $type, self::FORMAT) ?? 'id'] = $data['data']['id'];
         }
 
         return parent::denormalize($dataToDenormalize, $type, $format, $context);
@@ -113,7 +118,7 @@ private function assertValidRequestDocument(mixed $data, mixed $operation, strin
             throw new BadRequestException('The resource object must have a "type" member.');
         }
 
-        if (!$operation instanceof HttpOperation || 'POST' !== $operation->getMethod()) {
+        if (!$operation instanceof HttpOperation) {
             return;
         }
 
@@ -123,6 +128,23 @@ private function assertValidRequestDocument(mixed $data, mixed $operation, strin
         }
     }
 
+    private function assertIdMatchesEndpoint(string $id, object $object, HttpOperation $operation, string $resourceClass): void
+    {
+        if ($this->useIriAsId) {
+            $requested = $id;
+            $actual = $this->iriConverter->getIriFromResource($object);
+        } elseif (\count($operation->getUriVariables() ?? []) > 1) {
+            return;
+        } else {
+            $requested = $this->reconstructIri($resourceClass, $id, $operation);
+            $actual = $this->iriConverter->getIriFromResource($object, UrlGeneratorInterface::ABS_PATH, $operation);
+        }
+
+        if (parse_url($requested, \PHP_URL_PATH) !== parse_url($actual, \PHP_URL_PATH)) {
+            throw new ConflictException(\sprintf('The resource object id "%s" does not match the id of this endpoint.', $id));
+        }
+    }
+
     private function getResourceShortName(string $resourceClass): string
     {
         if ($this->resourceClassResolver->isResourceClass($resourceClass)) {

src/JsonApi/Serializer/ReservedAttributeNameConverter.php

@@ -31,8 +31,21 @@ final class ReservedAttributeNameConverter implements NameConverterInterface
         'included' => '_included',
     ];
 
-    public function __construct(private readonly ?NameConverterInterface $nameConverter = null)
+    public const STRICT_JSON_API_RESERVED_ATTRIBUTES = [
+        'id' => 'resourceId',
+        'type' => 'resourceType',
+    ];
+
+    public function __construct(private readonly ?NameConverterInterface $nameConverter = null, private readonly bool $strictMemberNames = false)
+    {
+    }
+
+    /**
+     * @return array<string, string>
+     */
+    public static function getReservedAttributes(bool $strictMemberNames = false): array
     {
+        return $strictMemberNames ? self::STRICT_JSON_API_RESERVED_ATTRIBUTES : self::JSON_API_RESERVED_ATTRIBUTES;
     }
 
     /**
@@ -48,8 +61,9 @@ public function normalize(string $propertyName, ?string $class = null, ?string $
             return $propertyName;
         }
 
-        if (isset(self::JSON_API_RESERVED_ATTRIBUTES[$propertyName])) {
-            $propertyName = self::JSON_API_RESERVED_ATTRIBUTES[$propertyName];
+        $reserved = self::getReservedAttributes($this->strictMemberNames);
+        if (isset($reserved[$propertyName])) {
+            $propertyName = $reserved[$propertyName];
         }
 
         return $propertyName;
@@ -60,8 +74,9 @@ public function normalize(string $propertyName, ?string $class = null, ?string $
      */
     public function denormalize(string $propertyName, ?string $class = null, ?string $format = null, array $context = []): string
     {
-        if (\in_array($propertyName, self::JSON_API_RESERVED_ATTRIBUTES, true)) {
-            $propertyName = substr($propertyName, 1);
+        $original = array_search($propertyName, self::getReservedAttributes($this->strictMemberNames), true);
+        if (false !== $original) {
+            $propertyName = $original;
         }
 
         if (null !== $this->nameConverter) {

src/JsonApi/Tests/Serializer/ReservedAttributeNameConverterTest.php

@@ -58,4 +58,29 @@ public function testDenormalize(string $expectedPropertyName, string $propertyNa
     {
         $this->assertSame($expectedPropertyName, $this->reservedAttributeNameConverter->denormalize($propertyName));
     }
+
+    public static function strictPropertiesProvider(): array
+    {
+        return [
+            ['id', 'resourceId'],
+            ['type', 'resourceType'],
+            ['links', 'links'],
+            ['relationships', 'relationships'],
+            ['included', 'included'],
+            ['foo', 'foo'],
+            ['nameConverted', 'name_converted'],
+        ];
+    }
+
+    #[DataProvider('strictPropertiesProvider')]
+    public function testStrictMemberNamesNormalize(string $propertyName, string $expectedPropertyName): void
+    {
+        $this->assertSame($expectedPropertyName, (new ReservedAttributeNameConverter(new CustomConverter(), true))->normalize($propertyName));
+    }
+
+    #[DataProvider('strictPropertiesProvider')]
+    public function testStrictMemberNamesDenormalize(string $expectedPropertyName, string $propertyName): void
+    {
+        $this->assertSame($expectedPropertyName, (new ReservedAttributeNameConverter(new CustomConverter(), true))->denormalize($propertyName));
+    }
 }

src/Laravel/ApiPlatformProvider.php

@@ -976,6 +976,7 @@ public function register(): void
                 $app->make(ResourceClassResolverInterface::class),
                 $app->make(ResourceMetadataCollectionFactoryInterface::class),
                 $app->make(DefinitionNameFactoryInterface::class),
+                strictMemberNames: (bool) $app['config']->get('api-platform.jsonapi.strict_member_names', false),
             );
         });
         $this->app->singleton(HydraSchemaFactory::class, static function (Application $app) {
@@ -1052,7 +1053,7 @@ public function register(): void
         });
 
         $this->app->singleton(ReservedAttributeNameConverter::class, static function (Application $app) {
-            return new ReservedAttributeNameConverter($app->make(NameConverterInterface::class));
+            return new ReservedAttributeNameConverter($app->make(NameConverterInterface::class), (bool) $app['config']->get('api-platform.jsonapi.strict_member_names', false));
         });
 
         if (interface_exists(FieldsBuilderEnumInterface::class)) {

src/Laravel/config/api-platform.php

@@ -96,8 +96,13 @@
 
         // Require the "type" member of JSON:API request resource objects (400
         // when missing) and reject a "type" that does not match the endpoint
-        // resource on POST (409 Conflict). Off by default.
+        // resource on POST, PUT and PATCH (409 Conflict). Off by default.
         'strict_resource_type' => false,
+
+        // Expose the "id" and "type" properties as "resourceId" and "resourceType"
+        // attributes instead of "_id" and "_type", which are not valid JSON:API
+        // member names. Off by default.
+        'strict_member_names' => false,
     ],
 
     'graphql' => [

src/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php

@@ -719,6 +719,10 @@ private function registerJsonApiConfiguration(ContainerBuilder $container, array
         $loader->load('state/jsonapi.php');
 
         $useIriAsId = $config['jsonapi']['use_iri_as_id'];
+        $strictMemberNames = $config['jsonapi']['strict_member_names'] ?? false;
+
+        $container->getDefinition('api_platform.jsonapi.name_converter.reserved_attribute_name')->replaceArgument(1, $strictMemberNames);
+        $container->getDefinition('api_platform.jsonapi.json_schema.schema_factory')->replaceArgument(6, $strictMemberNames);
 
         $itemNormalizer = $container->getDefinition('api_platform.jsonapi.normalizer.item');
         $itemNormalizer->replaceArgument(7, [

src/Symfony/Bundle/DependencyInjection/Configuration.php

@@ -111,7 +111,11 @@ public function getConfigTreeBuilder(): TreeBuilder
                         ->end()
                         ->booleanNode('strict_resource_type')
                             ->defaultFalse()
-                            ->info('Require the "type" member of JSON:API request resource objects (400 when missing) and reject a "type" that does not match the endpoint resource on POST (409 Conflict), per https://jsonapi.org/format/#crud-creating-responses-409.')
+                            ->info('Require the "type" member of JSON:API request resource objects (400 when missing) and reject a "type" that does not match the endpoint resource on POST, PUT and PATCH (409 Conflict), per https://jsonapi.org/format/#crud-creating-responses-409.')
+                        ->end()
+                        ->booleanNode('strict_member_names')
+                            ->defaultFalse()
+                            ->info('Expose the "id" and "type" properties as "resourceId" and "resourceType" attributes instead of "_id" and "_type", which are not valid member names per https://jsonapi.org/format/#document-member-names. The "links", "relationships" and "included" properties are no longer renamed.')
                         ->end()
                     ->end()
                 ->end()

src/Symfony/Bundle/Resources/config/jsonapi.php

@@ -42,14 +42,15 @@
             service('api_platform.metadata.resource.metadata_collection_factory')->ignoreOnInvalid(),
             service('api_platform.json_schema.definition_name_factory')->ignoreOnInvalid(),
             service('api_platform.jsonapi.resource_linkage_resolver'),
+            false,
         ]);
 
     $services->set('api_platform.jsonapi.encoder', JsonEncoder::class)
         ->args(['jsonapi'])
         ->tag('serializer.encoder');
 
     $services->set('api_platform.jsonapi.name_converter.reserved_attribute_name', ReservedAttributeNameConverter::class)
-        ->args([service('api_platform.name_converter')->ignoreOnInvalid()]);
+        ->args([service('api_platform.name_converter')->ignoreOnInvalid(), false]);
 
     $services->set('api_platform.jsonapi.normalizer.entrypoint', EntrypointNormalizer::class)
         ->args([

tests/Fixtures/TestBundle/ApiResource/JsonApi/PatchableDummy.php

@@ -0,0 +1,63 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <dunglas@gmail.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\JsonApi;
+
+use ApiPlatform\JsonApi\Serializer\ItemNormalizer;
+use ApiPlatform\Metadata\ApiProperty;
+use ApiPlatform\Metadata\ApiResource;
+use ApiPlatform\Metadata\Get;
+use ApiPlatform\Metadata\Operation;
+use ApiPlatform\Metadata\Patch;
+
+#[ApiResource(
+    shortName: 'JsonApiPatchableDummy',
+    formats: ['jsonapi' => ['application/vnd.api+json']],
+    operations: [
+        new Get(
+            uriTemplate: '/jsonapi_patchable_dummies/{id}',
+            provider: [self::class, 'provide'],
+        ),
+        new Patch(
+            uriTemplate: '/jsonapi_patchable_dummies/{id}',
+            provider: [self::class, 'provide'],
+            processor: [self::class, 'process'],
+        ),
+        new Patch(
+            uriTemplate: '/jsonapi_strict_patchable_dummies/{id}',
+            denormalizationContext: [ItemNormalizer::STRICT_RESOURCE_TYPE => true],
+            provider: [self::class, 'provide'],
+            processor: [self::class, 'process'],
+        ),
+    ],
+)]
+class PatchableDummy
+{
+    #[ApiProperty(identifier: true)]
+    public int $id = 1;
+
+    public string $name = 'original';
+
+    public static function provide(Operation $operation, array $uriVariables = []): self
+    {
+        $resource = new self();
+        $resource->id = (int) $uriVariables['id'];
+
+        return $resource;
+    }
+
+    public static function process(self $data): self
+    {
+        return $data;
+    }
+}