From a28009beb1558b837953b965a5890a38018f02a1 Mon Sep 17 00:00:00 2001 From: Jakov Knezovic Date: Fri, 28 Aug 2026 17:16:52 +0200 Subject: [PATCH] fix(jsonschema): handle partial update required properties --- src/JsonSchema/DefinitionNameFactory.php | 4 + src/JsonSchema/SchemaFactory.php | 10 ++- .../Tests/DefinitionNameFactoryTest.php | 2 + src/JsonSchema/Tests/SchemaFactoryTest.php | 75 +++++++++++++++++++ .../OpenApiPartialUpdateResource.php | 33 ++++++++ tests/Functional/OpenApiTest.php | 24 ++++++ 6 files changed, 146 insertions(+), 2 deletions(-) create mode 100644 tests/Fixtures/TestBundle/ApiResource/OpenApiPartialUpdateResource.php diff --git a/src/JsonSchema/DefinitionNameFactory.php b/src/JsonSchema/DefinitionNameFactory.php index 2396f9424d5..360f223357f 100644 --- a/src/JsonSchema/DefinitionNameFactory.php +++ b/src/JsonSchema/DefinitionNameFactory.php @@ -79,6 +79,10 @@ public function create(string $className, string $format = 'json', ?string $inpu $name = $parts ? \sprintf('%s-%s', $prefix, implode('_', $parts)) : $prefix; } + if ($serializerContext[SchemaFactory::PARTIAL_UPDATE] ?? false) { + $name .= '.partial'; + } + if (false === ($serializerContext['gen_id'] ?? true)) { $name .= '_noid'; } diff --git a/src/JsonSchema/SchemaFactory.php b/src/JsonSchema/SchemaFactory.php index 1157b458cb3..7e529743e72 100644 --- a/src/JsonSchema/SchemaFactory.php +++ b/src/JsonSchema/SchemaFactory.php @@ -47,6 +47,7 @@ final class SchemaFactory implements SchemaFactoryInterface, SchemaFactoryAwareI private ?SchemaFactoryInterface $schemaFactory = null; // Edge case where the related resource is not readable (for example: NotExposed) but we have groups to read the whole related object public const OPENAPI_DEFINITION_NAME = 'openapi_definition_name'; + public const PARTIAL_UPDATE = 'partial_update'; public function __construct(ResourceMetadataCollectionFactoryInterface $resourceMetadataFactory, private readonly PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, private readonly PropertyMetadataFactoryInterface $propertyMetadataFactory, private readonly ?NameConverterInterface $nameConverter = null, ?ResourceClassResolverInterface $resourceClassResolver = null, ?array $distinctFormats = null, private ?DefinitionNameFactoryInterface $definitionNameFactory = null) { @@ -95,12 +96,17 @@ public function buildSchema(string $className, string $format = 'json', string $ } $isJsonMergePatch = 'json' === $format && 'PATCH' === $method && Schema::TYPE_INPUT === $type; + $isNonStandardPut = Schema::TYPE_INPUT === $type && 'PUT' === $method && !($operation?->getExtraProperties()['standard_put'] ?? true); + $isPartialUpdate = $isJsonMergePatch || $isNonStandardPut; $definitionFormat = match (true) { default => $format, $isJsonMergePatch => 'merge-patch+json', }; - $definitionName = $this->definitionNameFactory->create($className, $definitionFormat, $inputOrOutputClass, $operation, $serializerContext + ['schema_type' => $type]); + $definitionName = $this->definitionNameFactory->create($className, $definitionFormat, $inputOrOutputClass, $operation, $serializerContext + [ + 'schema_type' => $type, + self::PARTIAL_UPDATE => $isPartialUpdate && !$isJsonMergePatch, + ]); if (!isset($schema['$ref']) && !isset($schema['type'])) { $ref = $this->getSchemaUriPrefix($version).$definitionName; @@ -154,7 +160,7 @@ public function buildSchema(string $className, string $format = 'json', string $ } $normalizedPropertyName = $this->nameConverter ? $this->nameConverter->normalize($propertyName, $inputOrOutputClass, $format, $serializerContext) : $propertyName; - if ($propertyMetadata->isRequired() && !$isJsonMergePatch) { + if ($propertyMetadata->isRequired() && !$isPartialUpdate) { $definition['required'][] = $normalizedPropertyName; } diff --git a/src/JsonSchema/Tests/DefinitionNameFactoryTest.php b/src/JsonSchema/Tests/DefinitionNameFactoryTest.php index b297b8fae9f..f9a20ab2a2a 100644 --- a/src/JsonSchema/Tests/DefinitionNameFactoryTest.php +++ b/src/JsonSchema/Tests/DefinitionNameFactoryTest.php @@ -34,6 +34,8 @@ public static function providerDefinitions(): iterable yield ['Dummy.jsonapi', Dummy::class, 'jsonapi']; yield ['Dummy.jsonhal', Dummy::class, 'jsonhal']; yield ['Dummy.jsonld', Dummy::class, 'jsonld']; + yield ['Dummy.partial', Dummy::class, 'json', null, null, [SchemaFactory::PARTIAL_UPDATE => true]]; + yield ['Dummy.jsonld.partial', Dummy::class, 'jsonld', null, null, [SchemaFactory::PARTIAL_UPDATE => true]]; yield ['Dummy.DtoOutput', Dummy::class, 'json', DtoOutput::class]; yield ['Dummy.DtoOutput.jsonapi', Dummy::class, 'jsonapi', DtoOutput::class]; diff --git a/src/JsonSchema/Tests/SchemaFactoryTest.php b/src/JsonSchema/Tests/SchemaFactoryTest.php index 10bee202881..fb1382c9185 100644 --- a/src/JsonSchema/Tests/SchemaFactoryTest.php +++ b/src/JsonSchema/Tests/SchemaFactoryTest.php @@ -29,6 +29,8 @@ use ApiPlatform\Metadata\ApiResource; use ApiPlatform\Metadata\Get; use ApiPlatform\Metadata\Operations; +use ApiPlatform\Metadata\Patch; +use ApiPlatform\Metadata\Post; use ApiPlatform\Metadata\Property\Factory\PropertyMetadataFactoryInterface; use ApiPlatform\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface; use ApiPlatform\Metadata\Property\PropertyNameCollection; @@ -731,4 +733,77 @@ public function testBuildSchemaForAssociativeArray(): void $this->assertSame('object', $definitions[$rootDefinitionKey]['properties']['bar']['type']); $this->assertSame('string', $definitions[$rootDefinitionKey]['properties']['bar']['additionalProperties']); } + + public function testPartialUpdateRequiredPropertiesUseDistinctDefinitions(): void + { + $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataCollectionFactoryInterface::class); + + $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); + $propertyNameCollectionFactoryProphecy->create(NotAResource::class, Argument::cetera())->willReturn(new PropertyNameCollection(['foo'])); + + $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); + $propertyMetadataFactoryProphecy->create(NotAResource::class, 'foo', Argument::cetera())->willReturn( + (new ApiProperty()) + ->withNativeType(Type::string()) + ->withReadable(true) + ->withWritable(true) + ->withRequired(true) + ->withSchema(['type' => 'string']) + ); + + $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); + $resourceClassResolverProphecy->isResourceClass(NotAResource::class)->willReturn(true); + + $schemaFactory = new SchemaFactory( + resourceMetadataFactory: $resourceMetadataFactoryProphecy->reveal(), + propertyNameCollectionFactory: $propertyNameCollectionFactoryProphecy->reveal(), + propertyMetadataFactory: $propertyMetadataFactoryProphecy->reveal(), + resourceClassResolver: $resourceClassResolverProphecy->reveal(), + ); + + $postSchema = $schemaFactory->buildSchema( + NotAResource::class, + 'jsonld', + Schema::TYPE_INPUT, + new Post(class: NotAResource::class, shortName: 'RequiredResource'), + ); + $postDefinitionKey = $postSchema->getRootDefinitionKey(); + + $partialPutSchema = new Schema(Schema::VERSION_OPENAPI); + $partialPutSchema->setDefinitions($postSchema->getDefinitions()); + $partialPutSchema = $schemaFactory->buildSchema( + NotAResource::class, + 'jsonld', + Schema::TYPE_INPUT, + new Put(class: NotAResource::class, shortName: 'RequiredResource', extraProperties: ['standard_put' => false]), + $partialPutSchema, + ); + $partialPutDefinitionKey = $partialPutSchema->getRootDefinitionKey(); + + $standardPutSchema = $schemaFactory->buildSchema( + NotAResource::class, + 'jsonld', + Schema::TYPE_INPUT, + new Put(class: NotAResource::class, shortName: 'RequiredResource'), + ); + $standardPutDefinitionKey = $standardPutSchema->getRootDefinitionKey(); + + self::assertSame('RequiredResource.jsonld', $postDefinitionKey); + self::assertSame('RequiredResource.jsonld.partial', $partialPutDefinitionKey); + self::assertSame('RequiredResource.jsonld', $standardPutDefinitionKey); + self::assertNotSame($postDefinitionKey, $partialPutDefinitionKey); + self::assertSame(['foo'], $postSchema->getDefinitions()[$postDefinitionKey]['required']); + self::assertArrayNotHasKey('required', $partialPutSchema->getDefinitions()[$partialPutDefinitionKey]); + self::assertSame(['foo'], $standardPutSchema->getDefinitions()[$standardPutDefinitionKey]['required']); + + $patchSchema = $schemaFactory->buildSchema( + NotAResource::class, + 'json', + Schema::TYPE_INPUT, + new Patch(class: NotAResource::class, shortName: 'RequiredResource'), + ); + + self::assertSame('RequiredResource.jsonMergePatch', $patchSchema->getRootDefinitionKey()); + self::assertArrayNotHasKey('required', $patchSchema->getDefinitions()[$patchSchema->getRootDefinitionKey()]); + } } diff --git a/tests/Fixtures/TestBundle/ApiResource/OpenApiPartialUpdateResource.php b/tests/Fixtures/TestBundle/ApiResource/OpenApiPartialUpdateResource.php new file mode 100644 index 00000000000..9eb728dbf9a --- /dev/null +++ b/tests/Fixtures/TestBundle/ApiResource/OpenApiPartialUpdateResource.php @@ -0,0 +1,33 @@ + + * + * 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; + +use ApiPlatform\Metadata\ApiProperty; +use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\Patch; +use ApiPlatform\Metadata\Post; +use ApiPlatform\Metadata\Put; + +#[ApiResource(operations: [ + new Post(uriTemplate: '/openapi_partial_update_resources', inputFormats: ['json' => ['application/json']]), + new Put(uriTemplate: '/openapi_partial_update_resources/{id}', inputFormats: ['json' => ['application/json']], extraProperties: ['standard_put' => false]), + new Patch(uriTemplate: '/openapi_partial_update_resources/{id}', inputFormats: ['json' => ['application/json']]), +])] +final class OpenApiPartialUpdateResource +{ + public ?int $id = null; + + #[ApiProperty(required: true)] + public ?string $name = null; +} diff --git a/tests/Functional/OpenApiTest.php b/tests/Functional/OpenApiTest.php index 1a59307a44f..51d1400d775 100644 --- a/tests/Functional/OpenApiTest.php +++ b/tests/Functional/OpenApiTest.php @@ -22,6 +22,7 @@ use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\Issue7064\DeprecatedPutUser; use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\Issue7064\DeprecatedPutUserAction; use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\Issue8143\ReferenceResponse; +use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\OpenApiPartialUpdateResource; use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\ParentAttribute; use ApiPlatform\Tests\Fixtures\TestBundle\Entity\AbstractDummy; use ApiPlatform\Tests\Fixtures\TestBundle\Entity\CircularReference; @@ -112,6 +113,7 @@ public static function getResources(): array DeprecatedPutUser::class, DeprecatedPutUserAction::class, ReferenceResponse::class, + OpenApiPartialUpdateResource::class, ]; } @@ -152,6 +154,28 @@ public function testDeprecatedPutDoesNotLeakIntoNestedResourceSchema(): void $this->assertArrayNotHasKey('deprecated', $schemas[$userName], 'Nested DeprecatedPutUser schema must not be marked deprecated because of the deprecated PUT operation'); } + public function testPartialUpdateSchemasDoNotRequireProperties(): void + { + $response = self::createClient()->request('GET', '/docs', [ + 'headers' => ['Accept' => 'application/vnd.openapi+json'], + ]); + + $json = $response->toArray(); + $schemas = $json['components']['schemas']; + $postSchema = $json['paths']['/openapi_partial_update_resources']['post']['requestBody']['content']['application/json']['schema']; + $putSchema = $json['paths']['/openapi_partial_update_resources/{id}']['put']['requestBody']['content']['application/json']['schema']; + $patchSchema = $json['paths']['/openapi_partial_update_resources/{id}']['patch']['requestBody']['content']['application/json']['schema']; + $postName = substr($postSchema['$ref'], \strlen('#/components/schemas/')); + $putName = substr($putSchema['$ref'], \strlen('#/components/schemas/')); + $patchName = substr($patchSchema['$ref'], \strlen('#/components/schemas/')); + + self::assertNotSame($postName, $putName); + self::assertNotSame($postName, $patchName); + self::assertSame(['name'], $schemas[$postName]['required']); + self::assertArrayNotHasKey('required', $schemas[$putName]); + self::assertArrayNotHasKey('required', $schemas[$patchName]); + } + public function testErrorsAreDocumented(): void { $response = self::createClient()->request('GET', '/docs', [