From 4d1c2b88a9ece5d5e95182d6203b98edaa8208ae Mon Sep 17 00:00:00 2001 From: Takashi Kanemoto Date: Sat, 20 Jul 2024 23:45:28 +0900 Subject: [PATCH 1/3] feat: separate the JSON-LD schema for input/output and fix the types for JSON-LD specific properties --- src/Hydra/JsonSchema/SchemaFactory.php | 31 ++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/src/Hydra/JsonSchema/SchemaFactory.php b/src/Hydra/JsonSchema/SchemaFactory.php index f51af53e3d7..75144baee28 100644 --- a/src/Hydra/JsonSchema/SchemaFactory.php +++ b/src/Hydra/JsonSchema/SchemaFactory.php @@ -27,7 +27,6 @@ final class SchemaFactory implements SchemaFactoryInterface, SchemaFactoryAwareInterface { private const BASE_PROP = [ - 'readOnly' => true, 'type' => 'string', ]; private const BASE_PROPS = [ @@ -36,7 +35,6 @@ final class SchemaFactory implements SchemaFactoryInterface, SchemaFactoryAwareI ]; private const BASE_ROOT_PROPS = [ '@context' => [ - 'readOnly' => true, 'oneOf' => [ ['type' => 'string'], [ @@ -74,18 +72,43 @@ public function buildSchema(string $className, string $format = 'jsonld', string return $schema; } - if ('input' === $type) { - return $schema; + if (($key = $schema->getRootDefinitionKey() ?? $schema->getItemsDefinitionKey()) !== null) { + $postfix = '.'.$type; + $definitions = $schema->getDefinitions(); + $definitions[$key.$postfix] = $definitions[$key]; + unset($definitions[$key]); + + if (($schema['type'] ?? '') === 'array') { + $schema['items']['$ref'] .= $postfix; + } else { + $schema['$ref'] .= $postfix; + } } $definitions = $schema->getDefinitions(); if ($key = $schema->getRootDefinitionKey()) { $definitions[$key]['properties'] = self::BASE_ROOT_PROPS + ($definitions[$key]['properties'] ?? []); + if (Schema::TYPE_OUTPUT === $type) { + foreach (array_keys(self::BASE_ROOT_PROPS) as $property) { + $definitions[$key]['required'] ??= []; + if (!\in_array($property, $definitions[$key]['required'], true)) { + $definitions[$key]['required'][] = $property; + } + } + } return $schema; } if ($key = $schema->getItemsDefinitionKey()) { $definitions[$key]['properties'] = self::BASE_PROPS + ($definitions[$key]['properties'] ?? []); + if (Schema::TYPE_OUTPUT === $type) { + foreach (array_keys(self::BASE_PROPS) as $property) { + $definitions[$key]['required'] ??= []; + if (!\in_array($property, $definitions[$key]['required'], true)) { + $definitions[$key]['required'][] = $property; + } + } + } } if (($schema['type'] ?? '') === 'array') { From a42ca678807bada009716f998786dbe25bb99805 Mon Sep 17 00:00:00 2001 From: Takashi Kanemoto Date: Sat, 20 Jul 2024 23:45:53 +0900 Subject: [PATCH 2/3] Fix and add tests --- features/openapi/docs.feature | 8 +-- tests/Hydra/JsonSchema/SchemaFactoryTest.php | 50 +++++++++++++++++-- .../Command/JsonSchemaGenerateCommandTest.php | 42 ++++++++-------- .../Bundle/Command/OpenApiCommandTest.php | 2 +- 4 files changed, 73 insertions(+), 29 deletions(-) diff --git a/features/openapi/docs.feature b/features/openapi/docs.feature index 980b896959a..4f98c666751 100644 --- a/features/openapi/docs.feature +++ b/features/openapi/docs.feature @@ -153,7 +153,7 @@ Feature: Documentation support And the JSON node "paths./related_dummies/{id}/related_to_dummy_friends.get.parameters" should have 6 elements # Subcollection - check schema - And the JSON node "paths./related_dummies/{id}/related_to_dummy_friends.get.responses.200.content.application/ld+json.schema.properties.hydra:member.items.$ref" should be equal to "#/components/schemas/RelatedToDummyFriend.jsonld-fakemanytomany" + And the JSON node "paths./related_dummies/{id}/related_to_dummy_friends.get.responses.200.content.application/ld+json.schema.properties.hydra:member.items.$ref" should be equal to "#/components/schemas/RelatedToDummyFriend.jsonld-fakemanytomany.output" # Deprecations And the JSON node "paths./dummies.get.deprecated" should be false @@ -165,8 +165,8 @@ Feature: Documentation support And the JSON node "paths./deprecated_resources/{id}.patch.deprecated" should be true # Formats - And the OpenAPI class "Dummy.jsonld" exists - And the "@id" property exists for the OpenAPI class "Dummy.jsonld" + And the OpenAPI class "Dummy.jsonld.output" exists + And the "@id" property exists for the OpenAPI class "Dummy.jsonld.output" And the JSON node "paths./dummies.get.responses.200.content.application/ld+json" should be equal to: """ { @@ -176,7 +176,7 @@ Feature: Documentation support "hydra:member": { "type": "array", "items": { - "$ref": "#/components/schemas/Dummy.jsonld" + "$ref": "#/components/schemas/Dummy.jsonld.output" } }, "hydra:totalItems": { diff --git a/tests/Hydra/JsonSchema/SchemaFactoryTest.php b/tests/Hydra/JsonSchema/SchemaFactoryTest.php index b1a6029503d..109c0cc398b 100644 --- a/tests/Hydra/JsonSchema/SchemaFactoryTest.php +++ b/tests/Hydra/JsonSchema/SchemaFactoryTest.php @@ -21,6 +21,7 @@ use ApiPlatform\Metadata\ApiResource; use ApiPlatform\Metadata\Get; use ApiPlatform\Metadata\GetCollection; +use ApiPlatform\Metadata\Post; use ApiPlatform\Metadata\Property\Factory\PropertyMetadataFactoryInterface; use ApiPlatform\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface; use ApiPlatform\Metadata\Property\PropertyNameCollection; @@ -49,6 +50,7 @@ protected function setUp(): void $propertyNameCollectionFactory = $this->prophesize(PropertyNameCollectionFactoryInterface::class); $propertyNameCollectionFactory->create(Dummy::class, ['enable_getter_setter_extraction' => true, 'schema_type' => Schema::TYPE_OUTPUT])->willReturn(new PropertyNameCollection()); + $propertyNameCollectionFactory->create(Dummy::class, ['enable_getter_setter_extraction' => true, 'schema_type' => Schema::TYPE_INPUT])->willReturn(new PropertyNameCollection()); $propertyMetadataFactory = $this->prophesize(PropertyMetadataFactoryInterface::class); $definitionNameFactory = new DefinitionNameFactory(['jsonapi' => true, 'jsonhal' => true, 'jsonld' => true]); @@ -69,7 +71,12 @@ public function testBuildSchema(): void $resultSchema = $this->schemaFactory->buildSchema(Dummy::class); $this->assertTrue($resultSchema->isDefined()); - $this->assertSame('Dummy.jsonld', $resultSchema->getRootDefinitionKey()); + $this->assertSame('Dummy.jsonld.output', $resultSchema->getRootDefinitionKey()); + + $resultSchema = $this->schemaFactory->buildSchema(Dummy::class, 'jsonld', Schema::TYPE_INPUT, new Post()); + + $this->assertTrue($resultSchema->isDefined()); + $this->assertSame('Dummy.jsonld.input', $resultSchema->getRootDefinitionKey()); } public function testCustomFormatBuildSchema(): void @@ -94,7 +101,6 @@ public function testHasRootDefinitionKeyBuildSchema(): void $this->assertArrayHasKey('@context', $properties); $this->assertEquals( [ - 'readOnly' => true, 'oneOf' => [ ['type' => 'string'], [ @@ -122,7 +128,7 @@ public function testHasRootDefinitionKeyBuildSchema(): void public function testSchemaTypeBuildSchema(): void { $resultSchema = $this->schemaFactory->buildSchema(Dummy::class, 'jsonld', Schema::TYPE_OUTPUT, new GetCollection()); - $definitionName = 'Dummy.jsonld'; + $definitionName = 'Dummy.jsonld.output'; $this->assertNull($resultSchema->getRootDefinitionKey()); // @noRector @@ -151,6 +157,12 @@ public function testSchemaTypeBuildSchema(): void $this->assertArrayNotHasKey('@context', $properties); $this->assertArrayHasKey('@type', $properties); $this->assertArrayHasKey('@id', $properties); + + $resultSchema = $this->schemaFactory->buildSchema(Dummy::class, 'jsonld', Schema::TYPE_INPUT, new Post()); + $definitionName = 'Dummy.jsonld.input'; + + $this->assertSame($definitionName, $resultSchema->getRootDefinitionKey()); + $this->assertFalse(isset($resultSchema['properties'])); } public function testHasHydraViewNavigationBuildSchema(): void @@ -168,4 +180,36 @@ public function testHasHydraViewNavigationBuildSchema(): void $this->assertArrayHasKey('hydra:previous', $resultSchema['properties']['hydra:view']['properties']); $this->assertArrayHasKey('hydra:next', $resultSchema['properties']['hydra:view']['properties']); } + + public function testRequiredBasePropertiesBuildSchema(): void + { + $resultSchema = $this->schemaFactory->buildSchema(Dummy::class); + $definitions = $resultSchema->getDefinitions(); + $rootDefinitionKey = $resultSchema->getRootDefinitionKey(); + + $this->assertTrue(isset($definitions[$rootDefinitionKey])); + $this->assertTrue(isset($definitions[$rootDefinitionKey]['required'])); + $requiredProperties = $resultSchema['definitions'][$rootDefinitionKey]['required']; + $this->assertContains('@context', $requiredProperties); + $this->assertContains('@id', $requiredProperties); + $this->assertContains('@type', $requiredProperties); + + $resultSchema = $this->schemaFactory->buildSchema(Dummy::class, 'jsonld', Schema::TYPE_OUTPUT, new GetCollection()); + $definitions = $resultSchema->getDefinitions(); + $itemsDefinitionKey = array_key_first($definitions->getArrayCopy()); + + $this->assertTrue(isset($definitions[$itemsDefinitionKey])); + $this->assertTrue(isset($definitions[$itemsDefinitionKey]['required'])); + $requiredProperties = $resultSchema['definitions'][$itemsDefinitionKey]['required']; + $this->assertNotContains('@context', $requiredProperties); + $this->assertContains('@id', $requiredProperties); + $this->assertContains('@type', $requiredProperties); + + $resultSchema = $this->schemaFactory->buildSchema(Dummy::class, 'jsonld', Schema::TYPE_INPUT, new Post()); + $definitions = $resultSchema->getDefinitions(); + $itemsDefinitionKey = array_key_first($definitions->getArrayCopy()); + + $this->assertTrue(isset($definitions[$itemsDefinitionKey])); + $this->assertFalse(isset($definitions[$itemsDefinitionKey]['required'])); + } } diff --git a/tests/JsonSchema/Command/JsonSchemaGenerateCommandTest.php b/tests/JsonSchema/Command/JsonSchemaGenerateCommandTest.php index c84baa4aafc..e29b170560b 100644 --- a/tests/JsonSchema/Command/JsonSchemaGenerateCommandTest.php +++ b/tests/JsonSchema/Command/JsonSchemaGenerateCommandTest.php @@ -76,9 +76,9 @@ public function testExecuteWithJsonldTypeInput(): void $this->tester->run(['command' => 'api:json-schema:generate', 'resource' => $this->entityClass, '--operation' => '_api_/dummies{._format}_post', '--format' => 'jsonld', '--type' => 'input']); $result = $this->tester->getDisplay(); - $this->assertStringNotContainsString('@id', $result); - $this->assertStringNotContainsString('@context', $result); - $this->assertStringNotContainsString('@type', $result); + $this->assertStringContainsString('@id', $result); + $this->assertStringContainsString('@context', $result); + $this->assertStringContainsString('@type', $result); } /** @@ -103,24 +103,24 @@ public function testArraySchemaWithReference(): void $result = $this->tester->getDisplay(); $json = json_decode($result, associative: true); - $this->assertEquals($json['definitions']['BagOfTests.jsonld-write']['properties']['tests'], [ + $this->assertEquals($json['definitions']['BagOfTests.jsonld-write.input']['properties']['tests'], [ 'type' => 'string', 'foo' => 'bar', ]); - $this->assertEquals($json['definitions']['BagOfTests.jsonld-write']['properties']['nonResourceTests'], [ + $this->assertEquals($json['definitions']['BagOfTests.jsonld-write.input']['properties']['nonResourceTests'], [ 'type' => 'array', 'items' => [ - '$ref' => '#/definitions/NonResourceTestEntity.jsonld-write', + '$ref' => '#/definitions/NonResourceTestEntity.jsonld-write.input', ], ]); - $this->assertEquals($json['definitions']['BagOfTests.jsonld-write']['properties']['description'], [ + $this->assertEquals($json['definitions']['BagOfTests.jsonld-write.input']['properties']['description'], [ 'maxLength' => 255, ]); - $this->assertEquals($json['definitions']['BagOfTests.jsonld-write']['properties']['type'], [ - '$ref' => '#/definitions/TestEntity.jsonld-write', + $this->assertEquals($json['definitions']['BagOfTests.jsonld-write.input']['properties']['type'], [ + '$ref' => '#/definitions/TestEntity.jsonld-write.input', ]); } @@ -130,14 +130,14 @@ public function testArraySchemaWithMultipleUnionTypesJsonLd(): void $result = $this->tester->getDisplay(); $json = json_decode($result, associative: true); - $this->assertEquals($json['definitions']['Nest.jsonld']['properties']['owner']['anyOf'], [ - ['$ref' => '#/definitions/Wren.jsonld'], - ['$ref' => '#/definitions/Robin.jsonld'], + $this->assertEquals($json['definitions']['Nest.jsonld.output']['properties']['owner']['anyOf'], [ + ['$ref' => '#/definitions/Wren.jsonld.output'], + ['$ref' => '#/definitions/Robin.jsonld.output'], ['type' => 'null'], ]); - $this->assertArrayHasKey('Wren.jsonld', $json['definitions']); - $this->assertArrayHasKey('Robin.jsonld', $json['definitions']); + $this->assertArrayHasKey('Wren.jsonld.output', $json['definitions']); + $this->assertArrayHasKey('Robin.jsonld.output', $json['definitions']); } public function testArraySchemaWithMultipleUnionTypesJsonApi(): void @@ -185,7 +185,7 @@ public function testArraySchemaWithTypeFactory(): void $result = $this->tester->getDisplay(); $json = json_decode($result, associative: true); - $this->assertEquals($json['definitions']['Foo.jsonld']['properties']['expiration'], ['type' => 'string', 'format' => 'date']); + $this->assertEquals($json['definitions']['Foo.jsonld.output']['properties']['expiration'], ['type' => 'string', 'format' => 'date']); } /** @@ -197,7 +197,7 @@ public function testWritableNonResourceRef(): void $result = $this->tester->getDisplay(); $json = json_decode($result, associative: true); - $this->assertEquals($json['definitions']['SaveProduct.jsonld']['properties']['codes']['items']['$ref'], '#/definitions/ProductCode.jsonld'); + $this->assertEquals($json['definitions']['SaveProduct.jsonld.input']['properties']['codes']['items']['$ref'], '#/definitions/ProductCode.jsonld.input'); } /** @@ -209,8 +209,8 @@ public function testOpenApiResourceRefIsNotOverwritten(): void $result = $this->tester->getDisplay(); $json = json_decode($result, associative: true); - $this->assertEquals('#/definitions/DummyFriend', $json['definitions']['Issue6299.Issue6299OutputDto.jsonld']['properties']['itemDto']['$ref']); - $this->assertEquals('#/definitions/DummyDate', $json['definitions']['Issue6299.Issue6299OutputDto.jsonld']['properties']['collectionDto']['items']['$ref']); + $this->assertEquals('#/definitions/DummyFriend', $json['definitions']['Issue6299.Issue6299OutputDto.jsonld.output']['properties']['itemDto']['$ref']); + $this->assertEquals('#/definitions/DummyDate', $json['definitions']['Issue6299.Issue6299OutputDto.jsonld.output']['properties']['collectionDto']['items']['$ref']); } /** @@ -222,7 +222,7 @@ public function testSubSchemaJsonLd(): void $result = $this->tester->getDisplay(); $json = json_decode($result, associative: true); - $this->assertArrayHasKey('@id', $json['definitions']['ThirdLevel.jsonld-friends']['properties']); + $this->assertArrayHasKey('@id', $json['definitions']['ThirdLevel.jsonld-friends.output']['properties']); } public function testJsonApiIncludesSchema(): void @@ -280,7 +280,7 @@ public function testBackedEnumExamplesAreNotLost(): void $this->tester->run(['command' => 'api:json-schema:generate', 'resource' => 'ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\Issue6317\Issue6317', '--type' => 'output', '--format' => 'jsonld']); $result = $this->tester->getDisplay(); $json = json_decode($result, associative: true); - $properties = $json['definitions']['Issue6317.jsonld']['properties']; + $properties = $json['definitions']['Issue6317.jsonld.output']['properties']; $this->assertArrayHasKey('example', $properties['id']); $this->assertArrayHasKey('example', $properties['name']); @@ -295,7 +295,7 @@ public function testResourceWithEnumPropertiesSchema(): void $this->tester->run(['command' => 'api:json-schema:generate', 'resource' => 'ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\ResourceWithEnumProperty', '--type' => 'output', '--format' => 'jsonld']); $result = $this->tester->getDisplay(); $json = json_decode($result, associative: true); - $properties = $json['definitions']['ResourceWithEnumProperty.jsonld']['properties']; + $properties = $json['definitions']['ResourceWithEnumProperty.jsonld.output']['properties']; $this->assertSame( [ diff --git a/tests/Symfony/Bundle/Command/OpenApiCommandTest.php b/tests/Symfony/Bundle/Command/OpenApiCommandTest.php index cf68f461997..b09d7ae7f65 100644 --- a/tests/Symfony/Bundle/Command/OpenApiCommandTest.php +++ b/tests/Symfony/Bundle/Command/OpenApiCommandTest.php @@ -134,7 +134,7 @@ public function testBackedEnumExamplesAreNotLost(): void }; $assertExample($json['components']['schemas']['Issue6317']['properties'], 'id'); - $assertExample($json['components']['schemas']['Issue6317.jsonld']['properties'], 'id'); + $assertExample($json['components']['schemas']['Issue6317.jsonld.output']['properties'], 'id'); $assertExample($json['components']['schemas']['Issue6317.jsonapi']['properties']['data']['properties']['attributes']['properties'], '_id'); $assertExample($json['components']['schemas']['Issue6317.jsonhal']['properties'], 'id'); } From 23af961845008f9ef741ff68f57eeed97b87d475 Mon Sep 17 00:00:00 2001 From: Takashi Kanemoto Date: Mon, 22 Jul 2024 00:43:29 +0900 Subject: [PATCH 3/3] make only required in sub-schema --- src/Hydra/JsonSchema/SchemaFactory.php | 39 ++++++++++++-------- tests/Hydra/JsonSchema/SchemaFactoryTest.php | 4 +- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/Hydra/JsonSchema/SchemaFactory.php b/src/Hydra/JsonSchema/SchemaFactory.php index 75144baee28..e7172c9e0cc 100644 --- a/src/Hydra/JsonSchema/SchemaFactory.php +++ b/src/Hydra/JsonSchema/SchemaFactory.php @@ -88,27 +88,13 @@ public function buildSchema(string $className, string $format = 'jsonld', string $definitions = $schema->getDefinitions(); if ($key = $schema->getRootDefinitionKey()) { $definitions[$key]['properties'] = self::BASE_ROOT_PROPS + ($definitions[$key]['properties'] ?? []); - if (Schema::TYPE_OUTPUT === $type) { - foreach (array_keys(self::BASE_ROOT_PROPS) as $property) { - $definitions[$key]['required'] ??= []; - if (!\in_array($property, $definitions[$key]['required'], true)) { - $definitions[$key]['required'][] = $property; - } - } - } + $this->makeJsonLdKeywordPropertiesRequired($definitions, $key, $type, true, null === $operation); return $schema; } if ($key = $schema->getItemsDefinitionKey()) { $definitions[$key]['properties'] = self::BASE_PROPS + ($definitions[$key]['properties'] ?? []); - if (Schema::TYPE_OUTPUT === $type) { - foreach (array_keys(self::BASE_PROPS) as $property) { - $definitions[$key]['required'] ??= []; - if (!\in_array($property, $definitions[$key]['required'], true)) { - $definitions[$key]['required'][] = $property; - } - } - } + $this->makeJsonLdKeywordPropertiesRequired($definitions, $key, $type, false, null === $operation); } if (($schema['type'] ?? '') === 'array') { @@ -211,4 +197,25 @@ public function setSchemaFactory(SchemaFactoryInterface $schemaFactory): void $this->schemaFactory->setSchemaFactory($schemaFactory); } } + + private function makeJsonLdKeywordPropertiesRequired(\ArrayObject $definitions, string $key, string $type, bool $isRoot, bool $isSubSchema): void + { + if (Schema::TYPE_INPUT === $type) { + return; + } + + $definitions[$key]['required'] ??= []; + + $requiredProperties = match (true) { + $isSubSchema => ['@type'], + $isRoot => ['@context', '@id', '@type'], + default => ['@id', '@type'], + }; + + foreach ($requiredProperties as $property) { + if (!\in_array($property, $definitions[$key]['required'], true)) { + $definitions[$key]['required'][] = $property; + } + } + } } diff --git a/tests/Hydra/JsonSchema/SchemaFactoryTest.php b/tests/Hydra/JsonSchema/SchemaFactoryTest.php index 109c0cc398b..17c96d6f970 100644 --- a/tests/Hydra/JsonSchema/SchemaFactoryTest.php +++ b/tests/Hydra/JsonSchema/SchemaFactoryTest.php @@ -190,8 +190,8 @@ public function testRequiredBasePropertiesBuildSchema(): void $this->assertTrue(isset($definitions[$rootDefinitionKey])); $this->assertTrue(isset($definitions[$rootDefinitionKey]['required'])); $requiredProperties = $resultSchema['definitions'][$rootDefinitionKey]['required']; - $this->assertContains('@context', $requiredProperties); - $this->assertContains('@id', $requiredProperties); + $this->assertNotContains('@context', $requiredProperties); + $this->assertNotContains('@id', $requiredProperties); $this->assertContains('@type', $requiredProperties); $resultSchema = $this->schemaFactory->buildSchema(Dummy::class, 'jsonld', Schema::TYPE_OUTPUT, new GetCollection());