From 3cf7970a724b2101d039e5db3172b1415a72729c Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Wed, 19 Feb 2020 22:50:44 +0100 Subject: [PATCH 1/9] Corrected schema generation for `array`, `object`, `?array`, `?object` and `?type` The previous version of the `TypeFactory` generated following **WRONG** definitions: * `null|T[]` as `{"type": array, "items": {"type": "T"}}` * `?T[]` as `{"type": array, "items": {"type": "T"}}` * `array as `{"type": array, "items": {"type": "T"}}` * `object` without explicit schema definition as `{"type": "string"}` * `?T` as `{"type": T}` The new definitions instead do fix this by mapping: * `array` as `{"type": "object", "additionalProperties": {"type": "T"}}` * `array as `{"type": object, "additionalProperties": {"type": ["T", "null"]}}` * `null|array` as `{"type": ["object", "null"], "additionalProperties": {"type": "T"}}` * `array` as `{"type": "array", "items": {"type": "T"}}` (not very precise, but list support is not yet in symfony) * `object` without explicit schema definition as `{"type": "object"}` * `?T[]` as `{"type": "array", "items": {"type": ["T", "null"]}}` * `null|T[]` as `{"type": ["array", "null"], "items": {"type": "T"}}` --- src/JsonSchema/TypeFactory.php | 81 ++++++++++-- tests/JsonSchema/TypeFactoryTest.php | 115 +++++++++++++++++- .../DocumentationNormalizerV2Test.php | 10 +- .../DocumentationNormalizerV3Test.php | 7 +- 4 files changed, 198 insertions(+), 15 deletions(-) diff --git a/src/JsonSchema/TypeFactory.php b/src/JsonSchema/TypeFactory.php index b6825ceaf70..48480bfb69e 100644 --- a/src/JsonSchema/TypeFactory.php +++ b/src/JsonSchema/TypeFactory.php @@ -17,6 +17,9 @@ use ApiPlatform\Core\Util\ResourceClassInfoTrait; use Ramsey\Uuid\UuidInterface; use Symfony\Component\PropertyInfo\Type; +use function array_merge; +use function array_unique; +use function array_values; /** * {@inheritdoc} @@ -50,14 +53,37 @@ public function setSchemaFactory(SchemaFactoryInterface $schemaFactory): void public function getType(Type $type, string $format = 'json', ?bool $readableLink = null, ?array $serializerContext = null, Schema $schema = null): array { if ($type->isCollection()) { - $subType = new Type($type->getBuiltinType(), $type->isNullable(), $type->getClassName(), false); - - return [ - 'type' => 'array', - 'items' => $this->getType($subType, $format, $readableLink, $serializerContext, $schema), - ]; + $keyType = $type->getCollectionKeyType(); + $subType = $type->getCollectionValueType() + ?? new Type($type->getBuiltinType(), false, $type->getClassName(), false); + + if (null !== $keyType && Type::BUILTIN_TYPE_STRING === $keyType->getBuiltinType()) { + return $this->addNullabilityToTypeDefinition( + [ + 'type' => 'object', + 'additionalProperties' => $this->getType($subType, $format, $readableLink, $serializerContext, $schema), + ], + $type + ); + } + + return $this->addNullabilityToTypeDefinition( + [ + 'type' => 'array', + 'items' => $this->getType($subType, $format, $readableLink, $serializerContext, $schema), + ], + $type + ); } + return $this->addNullabilityToTypeDefinition( + $this->makeBasicType($type, $format, $readableLink, $serializerContext, $schema), + $type + ); + } + + private function makeBasicType(Type $type, string $format = 'json', ?bool $readableLink = null, ?array $serializerContext = null, Schema $schema = null): array + { switch ($type->getBuiltinType()) { case Type::BUILTIN_TYPE_INT: return ['type' => 'integer']; @@ -78,7 +104,7 @@ public function getType(Type $type, string $format = 'json', ?bool $readableLink private function getClassType(?string $className, string $format = 'json', ?bool $readableLink = null, ?array $serializerContext = null, ?Schema $schema = null): array { if (null === $className) { - return ['type' => 'string']; + return ['type' => 'object']; } if (is_a($className, \DateTimeInterface::class, true)) { @@ -102,7 +128,7 @@ private function getClassType(?string $className, string $format = 'json', ?bool // Skip if $schema is null (filters only support basic types) if (null === $schema) { - return ['type' => 'string']; + return ['type' => 'object']; } if ($this->isResourceClass($className) && true !== $readableLink) { @@ -125,4 +151,43 @@ private function getClassType(?string $className, string $format = 'json', ?bool return ['$ref' => $subSchema['$ref']]; } + + /** + * @param array $jsonSchema + * + * @return array + * + * @psalm-param array{type=: string|list} $jsonSchema + * + * @psalm-return array{type=: string|list, $ref=: string} + */ + private function addNullabilityToTypeDefinition(array $jsonSchema, Type $type): array + { + if (!$type->isNullable()) { + return $jsonSchema; + } + + if (!\array_key_exists('type', $jsonSchema)) { + return [ + 'oneOf' => [ + ['type' => 'null'], + $jsonSchema, + ], + ]; + } + + return array_merge($jsonSchema, ['type' => $this->addNullToTypes((array) $jsonSchema['type'])]); + } + + /** + * @param string[] $types + * + * @return string[] + * + * @psalm-param list $types + */ + private function addNullToTypes(array $types): array + { + return array_values(array_unique(array_merge($types, ['null']))); + } } diff --git a/tests/JsonSchema/TypeFactoryTest.php b/tests/JsonSchema/TypeFactoryTest.php index 000b05aea36..4f2aaad3495 100644 --- a/tests/JsonSchema/TypeFactoryTest.php +++ b/tests/JsonSchema/TypeFactoryTest.php @@ -35,13 +35,89 @@ public function testGetType(array $schema, Type $type): void public function typeProvider(): iterable { yield [['type' => 'integer'], new Type(Type::BUILTIN_TYPE_INT)]; + yield [['type' => ['integer', 'null']], new Type(Type::BUILTIN_TYPE_INT, true)]; yield [['type' => 'number'], new Type(Type::BUILTIN_TYPE_FLOAT)]; + yield [['type' => ['number', 'null']], new Type(Type::BUILTIN_TYPE_FLOAT, true)]; yield [['type' => 'boolean'], new Type(Type::BUILTIN_TYPE_BOOL)]; - yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT)]; + yield [['type' => ['boolean', 'null']], new Type(Type::BUILTIN_TYPE_BOOL, true)]; + yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_STRING)]; + yield [['type' => ['string', 'null']], new Type(Type::BUILTIN_TYPE_STRING, true)]; + yield [['type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT)]; + yield [['type' => ['object', 'null']], new Type(Type::BUILTIN_TYPE_OBJECT, true)]; yield [['type' => 'string', 'format' => 'date-time'], new Type(Type::BUILTIN_TYPE_OBJECT, false, \DateTimeImmutable::class)]; + yield [['type' => ['string', 'null'], 'format' => 'date-time'], new Type(Type::BUILTIN_TYPE_OBJECT, true, \DateTimeImmutable::class)]; yield [['type' => 'string', 'format' => 'duration'], new Type(Type::BUILTIN_TYPE_OBJECT, false, \DateInterval::class)]; - yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT, false, Dummy::class)]; + yield [['type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT, false, Dummy::class)]; + yield [['type' => ['object', 'null']], new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class)]; yield [['type' => 'array', 'items' => ['type' => 'string']], new Type(Type::BUILTIN_TYPE_STRING, false, null, true)]; + yield 'array can be itself nullable' => [ + ['type' => ['array', 'null'], 'items' => ['type' => 'string']], + new Type(Type::BUILTIN_TYPE_STRING, true, null, true), + ]; + + yield 'array can contain nullable values' => [ + ['type' => 'array', 'items' => ['type' => ['string', 'null']]], + new Type(Type::BUILTIN_TYPE_STRING, false, null, true, null, new Type(Type::BUILTIN_TYPE_STRING, true, null, false)), + ]; + + yield 'map with string keys becomes an object' => [ + ['type' => 'object', 'additionalProperties' => ['type' => 'string']], + new Type( + Type::BUILTIN_TYPE_STRING, + false, + null, + true, + new Type(Type::BUILTIN_TYPE_STRING, false, null, false) + ), + ]; + + yield 'nullable map with string keys becomes a nullable object' => [ + ['type' => ['object', 'null'], 'additionalProperties' => ['type' => 'string']], + new Type( + Type::BUILTIN_TYPE_STRING, + true, + null, + true, + new Type(Type::BUILTIN_TYPE_STRING, false, null, false), + new Type(Type::BUILTIN_TYPE_STRING, false, null, false) + ), + ]; + + yield 'map value type will be considered' => [ + ['type' => 'object', 'additionalProperties' => ['type' => 'integer']], + new Type( + Type::BUILTIN_TYPE_ARRAY, + false, + null, + true, + new Type(Type::BUILTIN_TYPE_STRING, false, null, false), + new Type(Type::BUILTIN_TYPE_INT, false, null, false) + ), + ]; + + yield 'map value type nullability will be considered' => [ + ['type' => 'object', 'additionalProperties' => ['type' => ['integer', 'null']]], + new Type( + Type::BUILTIN_TYPE_ARRAY, + false, + null, + true, + new Type(Type::BUILTIN_TYPE_STRING, false, null, false), + new Type(Type::BUILTIN_TYPE_INT, true, null, false) + ), + ]; + + yield 'nullable map can contain nullable values' => [ + ['type' => ['object', 'null'], 'additionalProperties' => ['type' => ['integer', 'null']]], + new Type( + Type::BUILTIN_TYPE_ARRAY, + true, + null, + true, + new Type(Type::BUILTIN_TYPE_STRING, false, null, false), + new Type(Type::BUILTIN_TYPE_INT, true, null, false) + ), + ]; } public function testGetClassType(): void @@ -59,4 +135,39 @@ public function testGetClassType(): void $this->assertSame(['$ref' => 'ref'], $typeFactory->getType(new Type(Type::BUILTIN_TYPE_OBJECT, false, Dummy::class), 'jsonld', true, ['foo' => 'bar'], new Schema())); } + + public function testGetClassTypeWithNullability(): void + { + $schemaFactory = $this->createMock(SchemaFactoryInterface::class); + + $schemaFactory + ->method('buildSchema') + ->willReturnCallback(static function (): Schema { + $schema = new Schema(); + + $schema['$ref'] = 'the-ref-name'; + $schema['description'] = 'more stuff here'; + + return $schema; + }); + + $typeFactory = new TypeFactory(); + $typeFactory->setSchemaFactory($schemaFactory); + + self::assertSame( + [ + 'oneOf' => [ + ['type' => 'null'], + ['$ref' => 'the-ref-name'], + ], + ], + $typeFactory->getType( + new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class), + 'jsonld', + true, + ['foo' => 'bar'], + new Schema() + ) + ); + } } diff --git a/tests/Swagger/Serializer/DocumentationNormalizerV2Test.php b/tests/Swagger/Serializer/DocumentationNormalizerV2Test.php index 684da2d83a4..25fe3931610 100644 --- a/tests/Swagger/Serializer/DocumentationNormalizerV2Test.php +++ b/tests/Swagger/Serializer/DocumentationNormalizerV2Test.php @@ -343,10 +343,11 @@ private function doTestNormalize(OperationMethodResolverInterface $operationMeth 'description' => 'This is an initializable but not writable property.', ]), 'dummyDate' => new \ArrayObject([ - 'type' => 'string', + 'type' => ['string', 'null'], 'description' => 'This is a \DateTimeInterface object.', 'format' => 'date-time', - ]), ], + ]), + ], ]), ]), ]; @@ -2018,7 +2019,10 @@ public function testNormalizeWithNestedNormalizationGroups(): void ]), 'relatedDummy' => new \ArrayObject([ 'description' => 'This is a related dummy \o/.', - '$ref' => '#/definitions/'.$relatedDummyRef, + 'oneOf' => [ + ['type' => 'null'], + ['$ref' => '#/definitions/'.$relatedDummyRef], + ], ]), ], ]), diff --git a/tests/Swagger/Serializer/DocumentationNormalizerV3Test.php b/tests/Swagger/Serializer/DocumentationNormalizerV3Test.php index 125add993c0..c7a741ed70d 100644 --- a/tests/Swagger/Serializer/DocumentationNormalizerV3Test.php +++ b/tests/Swagger/Serializer/DocumentationNormalizerV3Test.php @@ -385,7 +385,7 @@ private function doTestNormalize(OperationMethodResolverInterface $operationMeth 'description' => 'This is an initializable but not writable property.', ]), 'dummyDate' => new \ArrayObject([ - 'type' => 'string', + 'type' => ['string', 'null'], 'description' => 'This is a \DateTimeInterface object.', 'format' => 'date-time', ]), @@ -2000,7 +2000,10 @@ public function testNormalizeWithNestedNormalizationGroups(): void ]), 'relatedDummy' => new \ArrayObject([ 'description' => 'This is a related dummy \o/.', - '$ref' => '#/components/schemas/'.$relatedDummyRef, + 'oneOf' => [ + ['type' => 'null'], + ['$ref' => '#/components/schemas/'.$relatedDummyRef], + ], ]), ], ]), From a94ca5f08f123c32ac31890fc8f0fcc8031f738e Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Wed, 19 Feb 2020 23:16:13 +0100 Subject: [PATCH 2/9] Removed `@psalm-param array{...}` annotations These annotations were too specific, since the returned type would never match anyway. Specifically, psalm cannot yet declare `ArrayType1&ArrayType2`, so an union type of two arrays is not currently something we can declare explicitly. Ref: https://github.com/api-platform/core/pull/3402#discussion_r381574780 --- src/JsonSchema/TypeFactory.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/JsonSchema/TypeFactory.php b/src/JsonSchema/TypeFactory.php index 48480bfb69e..f3e758b8ce9 100644 --- a/src/JsonSchema/TypeFactory.php +++ b/src/JsonSchema/TypeFactory.php @@ -156,10 +156,6 @@ private function getClassType(?string $className, string $format = 'json', ?bool * @param array $jsonSchema * * @return array - * - * @psalm-param array{type=: string|list} $jsonSchema - * - * @psalm-return array{type=: string|list, $ref=: string} */ private function addNullabilityToTypeDefinition(array $jsonSchema, Type $type): array { From 8e3e04303640407dc3ba21ab012f99dd3d94fb8a Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 20 Feb 2020 00:14:35 +0100 Subject: [PATCH 3/9] Using `oneOf` also for simple non-`$ref` types As per OpenAPI documentation at https://swagger.io/docs/specification/data-models/data-types/ the OpenAPI v3 documentation does not allow defining union types via `type: ['string', 'integer']`, but requires explicit usage of `oneOf` and nested type declarations or references instead. Ref: https://github.com/api-platform/core/pull/3402#issuecomment-588518155 --- src/JsonSchema/TypeFactory.php | 31 ++-------- tests/JsonSchema/TypeFactoryTest.php | 58 +++++++++++++++---- .../DocumentationNormalizerV2Test.php | 9 ++- .../DocumentationNormalizerV3Test.php | 9 ++- 4 files changed, 66 insertions(+), 41 deletions(-) diff --git a/src/JsonSchema/TypeFactory.php b/src/JsonSchema/TypeFactory.php index f3e758b8ce9..8bd1c6d86bf 100644 --- a/src/JsonSchema/TypeFactory.php +++ b/src/JsonSchema/TypeFactory.php @@ -17,9 +17,6 @@ use ApiPlatform\Core\Util\ResourceClassInfoTrait; use Ramsey\Uuid\UuidInterface; use Symfony\Component\PropertyInfo\Type; -use function array_merge; -use function array_unique; -use function array_values; /** * {@inheritdoc} @@ -163,27 +160,11 @@ private function addNullabilityToTypeDefinition(array $jsonSchema, Type $type): return $jsonSchema; } - if (!\array_key_exists('type', $jsonSchema)) { - return [ - 'oneOf' => [ - ['type' => 'null'], - $jsonSchema, - ], - ]; - } - - return array_merge($jsonSchema, ['type' => $this->addNullToTypes((array) $jsonSchema['type'])]); - } - - /** - * @param string[] $types - * - * @return string[] - * - * @psalm-param list $types - */ - private function addNullToTypes(array $types): array - { - return array_values(array_unique(array_merge($types, ['null']))); + return [ + 'oneOf' => [ + ['type' => 'null'], + $jsonSchema, + ], + ]; } } diff --git a/tests/JsonSchema/TypeFactoryTest.php b/tests/JsonSchema/TypeFactoryTest.php index 4f2aaad3495..4bef70f28b0 100644 --- a/tests/JsonSchema/TypeFactoryTest.php +++ b/tests/JsonSchema/TypeFactoryTest.php @@ -35,28 +35,36 @@ public function testGetType(array $schema, Type $type): void public function typeProvider(): iterable { yield [['type' => 'integer'], new Type(Type::BUILTIN_TYPE_INT)]; - yield [['type' => ['integer', 'null']], new Type(Type::BUILTIN_TYPE_INT, true)]; + yield [['oneOf' => [['type' => 'null'], ['type' => 'integer']]], new Type(Type::BUILTIN_TYPE_INT, true)]; yield [['type' => 'number'], new Type(Type::BUILTIN_TYPE_FLOAT)]; - yield [['type' => ['number', 'null']], new Type(Type::BUILTIN_TYPE_FLOAT, true)]; + yield [['oneOf' => [['type' => 'null'], ['type' => 'number']]], new Type(Type::BUILTIN_TYPE_FLOAT, true)]; yield [['type' => 'boolean'], new Type(Type::BUILTIN_TYPE_BOOL)]; - yield [['type' => ['boolean', 'null']], new Type(Type::BUILTIN_TYPE_BOOL, true)]; + yield [['oneOf' => [['type' => 'null'], ['type' => 'boolean']]], new Type(Type::BUILTIN_TYPE_BOOL, true)]; yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_STRING)]; - yield [['type' => ['string', 'null']], new Type(Type::BUILTIN_TYPE_STRING, true)]; + yield [['oneOf' => [['type' => 'null'], ['type' => 'string']]], new Type(Type::BUILTIN_TYPE_STRING, true)]; yield [['type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT)]; - yield [['type' => ['object', 'null']], new Type(Type::BUILTIN_TYPE_OBJECT, true)]; + yield [['oneOf' => [['type' => 'null'], ['type' => 'object']]], new Type(Type::BUILTIN_TYPE_OBJECT, true)]; yield [['type' => 'string', 'format' => 'date-time'], new Type(Type::BUILTIN_TYPE_OBJECT, false, \DateTimeImmutable::class)]; - yield [['type' => ['string', 'null'], 'format' => 'date-time'], new Type(Type::BUILTIN_TYPE_OBJECT, true, \DateTimeImmutable::class)]; + yield [['oneOf' => [['type' => 'null'], ['type' => 'string', 'format' => 'date-time']]], new Type(Type::BUILTIN_TYPE_OBJECT, true, \DateTimeImmutable::class)]; yield [['type' => 'string', 'format' => 'duration'], new Type(Type::BUILTIN_TYPE_OBJECT, false, \DateInterval::class)]; yield [['type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT, false, Dummy::class)]; - yield [['type' => ['object', 'null']], new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class)]; + yield [['oneOf' => [['type' => 'null'], ['type' => 'object']]], new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class)]; yield [['type' => 'array', 'items' => ['type' => 'string']], new Type(Type::BUILTIN_TYPE_STRING, false, null, true)]; yield 'array can be itself nullable' => [ - ['type' => ['array', 'null'], 'items' => ['type' => 'string']], + ['oneOf' => [['type' => 'null'], ['type' => 'array', 'items' => ['type' => 'string']]]], new Type(Type::BUILTIN_TYPE_STRING, true, null, true), ]; yield 'array can contain nullable values' => [ - ['type' => 'array', 'items' => ['type' => ['string', 'null']]], + [ + 'type' => 'array', + 'items' => [ + 'oneOf' => [ + ['type' => 'null'], + ['type' => 'string'], + ], + ], + ], new Type(Type::BUILTIN_TYPE_STRING, false, null, true, null, new Type(Type::BUILTIN_TYPE_STRING, true, null, false)), ]; @@ -72,7 +80,12 @@ public function typeProvider(): iterable ]; yield 'nullable map with string keys becomes a nullable object' => [ - ['type' => ['object', 'null'], 'additionalProperties' => ['type' => 'string']], + [ + 'oneOf' => [ + ['type' => 'null'], + ['type' => 'object', 'additionalProperties' => ['type' => 'string']], + ], + ], new Type( Type::BUILTIN_TYPE_STRING, true, @@ -96,7 +109,15 @@ public function typeProvider(): iterable ]; yield 'map value type nullability will be considered' => [ - ['type' => 'object', 'additionalProperties' => ['type' => ['integer', 'null']]], + [ + 'type' => 'object', + 'additionalProperties' => [ + 'oneOf' => [ + ['type' => 'null'], + ['type' => 'integer'], + ], + ], + ], new Type( Type::BUILTIN_TYPE_ARRAY, false, @@ -108,7 +129,20 @@ public function typeProvider(): iterable ]; yield 'nullable map can contain nullable values' => [ - ['type' => ['object', 'null'], 'additionalProperties' => ['type' => ['integer', 'null']]], + [ + 'oneOf' => [ + ['type' => 'null'], + [ + 'type' => 'object', + 'additionalProperties' => [ + 'oneOf' => [ + ['type' => 'null'], + ['type' => 'integer'], + ], + ], + ], + ], + ], new Type( Type::BUILTIN_TYPE_ARRAY, true, diff --git a/tests/Swagger/Serializer/DocumentationNormalizerV2Test.php b/tests/Swagger/Serializer/DocumentationNormalizerV2Test.php index 25fe3931610..6f95465c916 100644 --- a/tests/Swagger/Serializer/DocumentationNormalizerV2Test.php +++ b/tests/Swagger/Serializer/DocumentationNormalizerV2Test.php @@ -343,9 +343,14 @@ private function doTestNormalize(OperationMethodResolverInterface $operationMeth 'description' => 'This is an initializable but not writable property.', ]), 'dummyDate' => new \ArrayObject([ - 'type' => ['string', 'null'], + 'oneOf' => [ + ['type' => 'null'], + [ + 'type' => 'string', + 'format' => 'date-time', + ], + ], 'description' => 'This is a \DateTimeInterface object.', - 'format' => 'date-time', ]), ], ]), diff --git a/tests/Swagger/Serializer/DocumentationNormalizerV3Test.php b/tests/Swagger/Serializer/DocumentationNormalizerV3Test.php index c7a741ed70d..0e0d8369728 100644 --- a/tests/Swagger/Serializer/DocumentationNormalizerV3Test.php +++ b/tests/Swagger/Serializer/DocumentationNormalizerV3Test.php @@ -385,9 +385,14 @@ private function doTestNormalize(OperationMethodResolverInterface $operationMeth 'description' => 'This is an initializable but not writable property.', ]), 'dummyDate' => new \ArrayObject([ - 'type' => ['string', 'null'], + 'oneOf' => [ + ['type' => 'null'], + [ + 'type' => 'string', + 'format' => 'date-time', + ], + ], 'description' => 'This is a \DateTimeInterface object.', - 'format' => 'date-time', ]), ], ]), From f1c59cdedb7db28b5ef2b91a0aeb262337dc5e05 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 20 Feb 2020 11:38:58 +0100 Subject: [PATCH 4/9] Conditionally fall back to JSON-Schema compatible with Swagger/OpenAPI v2 OpenAPI V2 has no way to generate accurate nullable types, so we need to disable nullable `oneOf` syntax in JSON-Schema by providing some context to the `TypeFactory` when not operating under OpenAPI v3 or newer considerations. In future, Swagger/OpenAPIV2 will (finally) at some point disappear, so we will be able to get rid of these conditionals once that happens. --- src/JsonSchema/SchemaFactory.php | 8 +- src/JsonSchema/TypeFactory.php | 30 ++++- .../Serializer/DocumentationNormalizer.php | 13 ++- tests/JsonSchema/TypeFactoryTest.php | 107 ++++++++++++++++++ .../DocumentationNormalizerV2Test.php | 38 +++---- 5 files changed, 164 insertions(+), 32 deletions(-) diff --git a/src/JsonSchema/SchemaFactory.php b/src/JsonSchema/SchemaFactory.php index 9d075e38ccf..7a0eaf6f3b2 100644 --- a/src/JsonSchema/SchemaFactory.php +++ b/src/JsonSchema/SchemaFactory.php @@ -258,19 +258,19 @@ private function getMetadata(string $className, string $type = Schema::TYPE_OUTP return [ $resourceMetadata, - $serializerContext ?? $this->getSerializerContext($resourceMetadata, $type, $operationType, $operationName), + $this->getSerializerContext($resourceMetadata, $type, $operationType, $operationName, $serializerContext), $inputOrOutput['class'], ]; } - private function getSerializerContext(ResourceMetadata $resourceMetadata, string $type = Schema::TYPE_OUTPUT, ?string $operationType, ?string $operationName): array + private function getSerializerContext(ResourceMetadata $resourceMetadata, string $type = Schema::TYPE_OUTPUT, ?string $operationType, ?string $operationName, ?array $previousSerializerContext): array { $attribute = Schema::TYPE_OUTPUT === $type ? 'normalization_context' : 'denormalization_context'; if (null === $operationType || null === $operationName) { - return $resourceMetadata->getAttribute($attribute, []); + return array_merge($resourceMetadata->getAttribute($attribute, []), (array) $previousSerializerContext); } - return $resourceMetadata->getTypedOperationAttribute($operationType, $operationName, $attribute, [], true); + return array_merge($resourceMetadata->getTypedOperationAttribute($operationType, $operationName, $attribute, [], true), (array) $previousSerializerContext); } } diff --git a/src/JsonSchema/TypeFactory.php b/src/JsonSchema/TypeFactory.php index 8bd1c6d86bf..549e67d7131 100644 --- a/src/JsonSchema/TypeFactory.php +++ b/src/JsonSchema/TypeFactory.php @@ -27,6 +27,19 @@ */ final class TypeFactory implements TypeFactoryInterface { + /** + * This constant is to be provided as serializer context key to conditionally enable types to be generated in + * a format that is compatible with OpenAPI specifications **PREVIOUS** to 3.0. + * + * Without this flag being set, the generated format will only be compatible with Swagger 3.0 or newer. + * + * Once support for OpenAPI < 3.0 is gone, this constant **WILL BE REMOVED** + * + * @internal Once support for OpenAPI < 3.0 is gone, this constant **WILL BE REMOVED** - do not rely on + * it in downstream projects! + */ + public const CONTEXT_SERIALIZATION_FORMAT_OPENAPI_PRE_V3_0 = self::class.'::CONTEXT_SERIALIZATION_FORMAT_OPENAPI_PRE_V3_0'; + use ResourceClassInfoTrait; /** @@ -60,7 +73,8 @@ public function getType(Type $type, string $format = 'json', ?bool $readableLink 'type' => 'object', 'additionalProperties' => $this->getType($subType, $format, $readableLink, $serializerContext, $schema), ], - $type + $type, + (array) $serializerContext ); } @@ -69,13 +83,15 @@ public function getType(Type $type, string $format = 'json', ?bool $readableLink 'type' => 'array', 'items' => $this->getType($subType, $format, $readableLink, $serializerContext, $schema), ], - $type + $type, + (array) $serializerContext ); } return $this->addNullabilityToTypeDefinition( $this->makeBasicType($type, $format, $readableLink, $serializerContext, $schema), - $type + $type, + (array) $serializerContext ); } @@ -98,7 +114,7 @@ private function makeBasicType(Type $type, string $format = 'json', ?bool $reada /** * Gets the JSON Schema document which specifies the data type corresponding to the given PHP class, and recursively adds needed new schema to the current schema if provided. */ - private function getClassType(?string $className, string $format = 'json', ?bool $readableLink = null, ?array $serializerContext = null, ?Schema $schema = null): array + private function getClassType(?string $className, string $format, ?bool $readableLink, ?array $serializerContext, ?Schema $schema): array { if (null === $className) { return ['type' => 'object']; @@ -154,8 +170,12 @@ private function getClassType(?string $className, string $format = 'json', ?bool * * @return array */ - private function addNullabilityToTypeDefinition(array $jsonSchema, Type $type): array + private function addNullabilityToTypeDefinition(array $jsonSchema, Type $type, array $serializerContext): array { + if (\array_key_exists(self::CONTEXT_SERIALIZATION_FORMAT_OPENAPI_PRE_V3_0, $serializerContext)) { + return $jsonSchema; + } + if (!$type->isNullable()) { return $jsonSchema; } diff --git a/src/Swagger/Serializer/DocumentationNormalizer.php b/src/Swagger/Serializer/DocumentationNormalizer.php index 835fe75a740..ac1a145df59 100644 --- a/src/Swagger/Serializer/DocumentationNormalizer.php +++ b/src/Swagger/Serializer/DocumentationNormalizer.php @@ -594,6 +594,10 @@ private function getJsonSchema(bool $v3, \ArrayObject $definitions, string $reso $schema = new Schema($v3 ? Schema::VERSION_OPENAPI : Schema::VERSION_SWAGGER); $schema->setDefinitions($definitions); + if (!$v3) { + $serializerContext = array_merge([TypeFactory::CONTEXT_SERIALIZATION_FORMAT_OPENAPI_PRE_V3_0 => null], (array) $serializerContext); + } + $this->jsonSchemaFactory->buildSchema($resourceClass, $format, $type, $operationType, $operationName, $schema, $serializerContext, $forceCollection); return $schema; @@ -720,7 +724,14 @@ private function getFiltersParameters(bool $v3, string $resourceClass, string $o 'required' => $data['required'], ]; - $type = \in_array($data['type'], Type::$builtinTypes, true) ? $this->jsonSchemaTypeFactory->getType(new Type($data['type'], false, null, $data['is_collection'] ?? false)) : ['type' => 'string']; + $type = \in_array($data['type'], Type::$builtinTypes, true) + ? $this->jsonSchemaTypeFactory->getType( + new Type($data['type'], false, null, $data['is_collection'] ?? false), + 'json', + null, + $v3 ? null : [TypeFactory::CONTEXT_SERIALIZATION_FORMAT_OPENAPI_PRE_V3_0 => null] + ) + : ['type' => 'string']; $v3 ? $parameter['schema'] = $type : $parameter += $type; if ($v3 && isset($data['schema'])) { diff --git a/tests/JsonSchema/TypeFactoryTest.php b/tests/JsonSchema/TypeFactoryTest.php index 4bef70f28b0..5c524c2bb2d 100644 --- a/tests/JsonSchema/TypeFactoryTest.php +++ b/tests/JsonSchema/TypeFactoryTest.php @@ -154,6 +154,113 @@ public function typeProvider(): iterable ]; } + /** @dataProvider openAPIV2typeProvider */ + public function testGetTypeWithOpenAPIV2Syntax(array $schema, Type $type): void + { + $typeFactory = new TypeFactory(); + $this->assertSame($schema, $typeFactory->getType($type, 'json', null, [TypeFactory::CONTEXT_SERIALIZATION_FORMAT_OPENAPI_PRE_V3_0 => null])); + } + + public function openAPIV2typeProvider(): iterable + { + yield [['type' => 'integer'], new Type(Type::BUILTIN_TYPE_INT)]; + yield [['type' => 'integer'], new Type(Type::BUILTIN_TYPE_INT, true)]; + yield [['type' => 'number'], new Type(Type::BUILTIN_TYPE_FLOAT)]; + yield [['type' => 'number'], new Type(Type::BUILTIN_TYPE_FLOAT, true)]; + yield [['type' => 'boolean'], new Type(Type::BUILTIN_TYPE_BOOL)]; + yield [['type' => 'boolean'], new Type(Type::BUILTIN_TYPE_BOOL, true)]; + yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_STRING)]; + yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_STRING, true)]; + yield [['type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT)]; + yield [['type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT, true)]; + yield [['type' => 'string', 'format' => 'date-time'], new Type(Type::BUILTIN_TYPE_OBJECT, false, \DateTimeImmutable::class)]; + yield [['type' => 'string', 'format' => 'date-time'], new Type(Type::BUILTIN_TYPE_OBJECT, true, \DateTimeImmutable::class)]; + yield [['type' => 'string', 'format' => 'duration'], new Type(Type::BUILTIN_TYPE_OBJECT, false, \DateInterval::class)]; + yield [['type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT, false, Dummy::class)]; + yield [['type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class)]; + yield [['type' => 'array', 'items' => ['type' => 'string']], new Type(Type::BUILTIN_TYPE_STRING, false, null, true)]; + yield 'array can be itself nullable, but ignored in OpenAPI V2' => [ + ['type' => 'array', 'items' => ['type' => 'string']], + new Type(Type::BUILTIN_TYPE_STRING, true, null, true), + ]; + + yield 'array can contain nullable values, but ignored in OpenAPI V2' => [ + [ + 'type' => 'array', + 'items' => ['type' => 'string'], + ], + new Type(Type::BUILTIN_TYPE_STRING, false, null, true, null, new Type(Type::BUILTIN_TYPE_STRING, true, null, false)), + ]; + + yield 'map with string keys becomes an object' => [ + ['type' => 'object', 'additionalProperties' => ['type' => 'string']], + new Type( + Type::BUILTIN_TYPE_STRING, + false, + null, + true, + new Type(Type::BUILTIN_TYPE_STRING, false, null, false) + ), + ]; + + yield 'nullable map with string keys becomes a nullable object, but ignored in OpenAPI V2' => [ + [ + 'type' => 'object', + 'additionalProperties' => ['type' => 'string'], + ], + new Type( + Type::BUILTIN_TYPE_STRING, + true, + null, + true, + new Type(Type::BUILTIN_TYPE_STRING, false, null, false), + new Type(Type::BUILTIN_TYPE_STRING, false, null, false) + ), + ]; + + yield 'map value type will be considered' => [ + ['type' => 'object', 'additionalProperties' => ['type' => 'integer']], + new Type( + Type::BUILTIN_TYPE_ARRAY, + false, + null, + true, + new Type(Type::BUILTIN_TYPE_STRING, false, null, false), + new Type(Type::BUILTIN_TYPE_INT, false, null, false) + ), + ]; + + yield 'map value type nullability will be considered, but ignored in OpenAPI V2' => [ + [ + 'type' => 'object', + 'additionalProperties' => ['type' => 'integer'], + ], + new Type( + Type::BUILTIN_TYPE_ARRAY, + false, + null, + true, + new Type(Type::BUILTIN_TYPE_STRING, false, null, false), + new Type(Type::BUILTIN_TYPE_INT, true, null, false) + ), + ]; + + yield 'nullable map can contain nullable values, but ignored in OpenAPI V2' => [ + [ + 'type' => 'object', + 'additionalProperties' => ['type' => 'integer'], + ], + new Type( + Type::BUILTIN_TYPE_ARRAY, + true, + null, + true, + new Type(Type::BUILTIN_TYPE_STRING, false, null, false), + new Type(Type::BUILTIN_TYPE_INT, true, null, false) + ), + ]; + } + public function testGetClassType(): void { $schemaFactoryProphecy = $this->prophesize(SchemaFactoryInterface::class); diff --git a/tests/Swagger/Serializer/DocumentationNormalizerV2Test.php b/tests/Swagger/Serializer/DocumentationNormalizerV2Test.php index 6f95465c916..7956cdea209 100644 --- a/tests/Swagger/Serializer/DocumentationNormalizerV2Test.php +++ b/tests/Swagger/Serializer/DocumentationNormalizerV2Test.php @@ -343,13 +343,8 @@ private function doTestNormalize(OperationMethodResolverInterface $operationMeth 'description' => 'This is an initializable but not writable property.', ]), 'dummyDate' => new \ArrayObject([ - 'oneOf' => [ - ['type' => 'null'], - [ - 'type' => 'string', - 'format' => 'date-time', - ], - ], + 'type' => 'string', + 'format' => 'date-time', 'description' => 'This is a \DateTimeInterface object.', ]), ], @@ -386,13 +381,19 @@ private function doTestNormalizeWithNameConverter(bool $legacy = false): void $propertyMetadataFactoryProphecy->create(Dummy::class, 'name')->shouldBeCalled()->willReturn(new PropertyMetadata(new Type(Type::BUILTIN_TYPE_STRING), 'This is a name.', true, true, null, null, false)); $propertyMetadataFactoryProphecy->create(Dummy::class, 'nameConverted')->shouldBeCalled()->willReturn(new PropertyMetadata(new Type(Type::BUILTIN_TYPE_STRING), 'This is a converted name.', true, true, null, null, false)); - $nameConverterProphecy = $this->prophesize( - interface_exists(AdvancedNameConverterInterface::class) - ? AdvancedNameConverterInterface::class - : NameConverterInterface::class - ); - $nameConverterProphecy->normalize('name', Dummy::class, DocumentationNormalizer::FORMAT, [])->willReturn('name')->shouldBeCalled(); - $nameConverterProphecy->normalize('nameConverted', Dummy::class, DocumentationNormalizer::FORMAT, [])->willReturn('name_converted')->shouldBeCalled(); + if (interface_exists(AdvancedNameConverterInterface::class)) { + $nameConverter = $this->createMock(AdvancedNameConverterInterface::class); + } else { + $nameConverter = $this->createMock(NameConverterInterface::class); + } + + $nameConverter->method('normalize') + ->with(self::logicalOr('name', 'nameConverted')) + ->willReturnCallback(static function (string $nameToNormalize): string { + return 'nameConverted' === $nameToNormalize + ? 'name_converted' + : $nameToNormalize; + }); $operationPathResolver = new CustomOperationPathResolver(new OperationPathResolver(new UnderscorePathSegmentNameGenerator())); @@ -408,10 +409,6 @@ interface_exists(AdvancedNameConverterInterface::class) * @var PropertyMetadataFactoryInterface */ $propertyMetadataFactory = $propertyMetadataFactoryProphecy->reveal(); - /** - * @var NameConverterInterface - */ - $nameConverter = $nameConverterProphecy->reveal(); /** * @var TypeFactoryInterface|null @@ -2024,10 +2021,7 @@ public function testNormalizeWithNestedNormalizationGroups(): void ]), 'relatedDummy' => new \ArrayObject([ 'description' => 'This is a related dummy \o/.', - 'oneOf' => [ - ['type' => 'null'], - ['$ref' => '#/definitions/'.$relatedDummyRef], - ], + '$ref' => '#/definitions/'.$relatedDummyRef, ]), ], ]), From 5a079e5f358aa5424d520d4fb35f46e310846dd0 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 20 Feb 2020 14:32:24 +0100 Subject: [PATCH 5/9] Use `{"nullable": true, "anyOf": [{"$ref": ...}]}` to comply with OpenAPI 3.0 OpenAPI 3.1 is not yet released, but fixes nullability in the way we had fixed it before (via `{"oneOf": [{"type": "null"}, ...]}`) in OAI/OpenAPI-Specification#1977. Until OpenAPI 3.1 is released, things like ``{"type": ["integer", "null"]}` are not valid definitions (because `"null"` is not yet a recognized type). We'll stick to OpenAPI 3.0 for now, using: * `{"nullable": true, ...}` for simple types * `{"nullable": true, "anyOf": [{"$ref": ...}]}` for type references --- src/JsonSchema/TypeFactory.php | 14 +++-- tests/JsonSchema/TypeFactoryTest.php | 57 ++++++++----------- .../DocumentationNormalizerV3Test.php | 14 ++--- 3 files changed, 36 insertions(+), 49 deletions(-) diff --git a/src/JsonSchema/TypeFactory.php b/src/JsonSchema/TypeFactory.php index 549e67d7131..effbf633242 100644 --- a/src/JsonSchema/TypeFactory.php +++ b/src/JsonSchema/TypeFactory.php @@ -180,11 +180,13 @@ private function addNullabilityToTypeDefinition(array $jsonSchema, Type $type, a return $jsonSchema; } - return [ - 'oneOf' => [ - ['type' => 'null'], - $jsonSchema, - ], - ]; + if (\array_key_exists('$ref', $jsonSchema)) { + return [ + 'nullable' => true, + 'anyOf' => [$jsonSchema], + ]; + } + + return array_merge($jsonSchema, ['nullable' => true]); } } diff --git a/tests/JsonSchema/TypeFactoryTest.php b/tests/JsonSchema/TypeFactoryTest.php index 5c524c2bb2d..d7e2b35234a 100644 --- a/tests/JsonSchema/TypeFactoryTest.php +++ b/tests/JsonSchema/TypeFactoryTest.php @@ -29,29 +29,29 @@ class TypeFactoryTest extends TestCase public function testGetType(array $schema, Type $type): void { $typeFactory = new TypeFactory(); - $this->assertSame($schema, $typeFactory->getType($type)); + $this->assertEquals($schema, $typeFactory->getType($type)); } public function typeProvider(): iterable { yield [['type' => 'integer'], new Type(Type::BUILTIN_TYPE_INT)]; - yield [['oneOf' => [['type' => 'null'], ['type' => 'integer']]], new Type(Type::BUILTIN_TYPE_INT, true)]; + yield [['nullable' => true, 'type' => 'integer'], new Type(Type::BUILTIN_TYPE_INT, true)]; yield [['type' => 'number'], new Type(Type::BUILTIN_TYPE_FLOAT)]; - yield [['oneOf' => [['type' => 'null'], ['type' => 'number']]], new Type(Type::BUILTIN_TYPE_FLOAT, true)]; + yield [['nullable' => true, 'type' => 'number'], new Type(Type::BUILTIN_TYPE_FLOAT, true)]; yield [['type' => 'boolean'], new Type(Type::BUILTIN_TYPE_BOOL)]; - yield [['oneOf' => [['type' => 'null'], ['type' => 'boolean']]], new Type(Type::BUILTIN_TYPE_BOOL, true)]; + yield [['nullable' => true, 'type' => 'boolean'], new Type(Type::BUILTIN_TYPE_BOOL, true)]; yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_STRING)]; - yield [['oneOf' => [['type' => 'null'], ['type' => 'string']]], new Type(Type::BUILTIN_TYPE_STRING, true)]; + yield [['nullable' => true, 'type' => 'string'], new Type(Type::BUILTIN_TYPE_STRING, true)]; yield [['type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT)]; - yield [['oneOf' => [['type' => 'null'], ['type' => 'object']]], new Type(Type::BUILTIN_TYPE_OBJECT, true)]; + yield [['nullable' => true, 'type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT, true)]; yield [['type' => 'string', 'format' => 'date-time'], new Type(Type::BUILTIN_TYPE_OBJECT, false, \DateTimeImmutable::class)]; - yield [['oneOf' => [['type' => 'null'], ['type' => 'string', 'format' => 'date-time']]], new Type(Type::BUILTIN_TYPE_OBJECT, true, \DateTimeImmutable::class)]; + yield [['nullable' => true, 'type' => 'string', 'format' => 'date-time'], new Type(Type::BUILTIN_TYPE_OBJECT, true, \DateTimeImmutable::class)]; yield [['type' => 'string', 'format' => 'duration'], new Type(Type::BUILTIN_TYPE_OBJECT, false, \DateInterval::class)]; yield [['type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT, false, Dummy::class)]; - yield [['oneOf' => [['type' => 'null'], ['type' => 'object']]], new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class)]; + yield [['nullable' => true, 'type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class)]; yield [['type' => 'array', 'items' => ['type' => 'string']], new Type(Type::BUILTIN_TYPE_STRING, false, null, true)]; yield 'array can be itself nullable' => [ - ['oneOf' => [['type' => 'null'], ['type' => 'array', 'items' => ['type' => 'string']]]], + ['nullable' => true, 'type' => 'array', 'items' => ['type' => 'string']], new Type(Type::BUILTIN_TYPE_STRING, true, null, true), ]; @@ -59,10 +59,8 @@ public function typeProvider(): iterable [ 'type' => 'array', 'items' => [ - 'oneOf' => [ - ['type' => 'null'], - ['type' => 'string'], - ], + 'nullable' => true, + 'type' => 'string', ], ], new Type(Type::BUILTIN_TYPE_STRING, false, null, true, null, new Type(Type::BUILTIN_TYPE_STRING, true, null, false)), @@ -81,10 +79,9 @@ public function typeProvider(): iterable yield 'nullable map with string keys becomes a nullable object' => [ [ - 'oneOf' => [ - ['type' => 'null'], - ['type' => 'object', 'additionalProperties' => ['type' => 'string']], - ], + 'nullable' => true, + 'type' => 'object', + 'additionalProperties' => ['type' => 'string'], ], new Type( Type::BUILTIN_TYPE_STRING, @@ -112,10 +109,8 @@ public function typeProvider(): iterable [ 'type' => 'object', 'additionalProperties' => [ - 'oneOf' => [ - ['type' => 'null'], - ['type' => 'integer'], - ], + 'nullable' => true, + 'type' => 'integer', ], ], new Type( @@ -130,17 +125,11 @@ public function typeProvider(): iterable yield 'nullable map can contain nullable values' => [ [ - 'oneOf' => [ - ['type' => 'null'], - [ - 'type' => 'object', - 'additionalProperties' => [ - 'oneOf' => [ - ['type' => 'null'], - ['type' => 'integer'], - ], - ], - ], + 'nullable' => true, + 'type' => 'object', + 'additionalProperties' => [ + 'nullable' => true, + 'type' => 'integer', ], ], new Type( @@ -297,8 +286,8 @@ public function testGetClassTypeWithNullability(): void self::assertSame( [ - 'oneOf' => [ - ['type' => 'null'], + 'nullable' => true, + 'anyOf' => [ ['$ref' => 'the-ref-name'], ], ], diff --git a/tests/Swagger/Serializer/DocumentationNormalizerV3Test.php b/tests/Swagger/Serializer/DocumentationNormalizerV3Test.php index 0e0d8369728..96406c4174d 100644 --- a/tests/Swagger/Serializer/DocumentationNormalizerV3Test.php +++ b/tests/Swagger/Serializer/DocumentationNormalizerV3Test.php @@ -385,13 +385,9 @@ private function doTestNormalize(OperationMethodResolverInterface $operationMeth 'description' => 'This is an initializable but not writable property.', ]), 'dummyDate' => new \ArrayObject([ - 'oneOf' => [ - ['type' => 'null'], - [ - 'type' => 'string', - 'format' => 'date-time', - ], - ], + 'nullable' => true, + 'type' => 'string', + 'format' => 'date-time', 'description' => 'This is a \DateTimeInterface object.', ]), ], @@ -2005,8 +2001,8 @@ public function testNormalizeWithNestedNormalizationGroups(): void ]), 'relatedDummy' => new \ArrayObject([ 'description' => 'This is a related dummy \o/.', - 'oneOf' => [ - ['type' => 'null'], + 'nullable' => true, + 'anyOf' => [ ['$ref' => '#/components/schemas/'.$relatedDummyRef], ], ]), From 7870bce13d2cc2f527ee7cd6944b394cdc3e0c47 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 20 Feb 2020 14:53:45 +0100 Subject: [PATCH 6/9] Rolling back to mapping unknown `object` types as JSON-Schema `{"type": "string"}` To avoid BC breaks, we defer this fix to https://github.com/api-platform/core/issues/3403 > **API Platform version(s) affected**: 2.5.x > > **Description** > > In our tests for `TypeFactory`: > > ``` > > yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT)]; > yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT, true)]; > > // ... > > yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT, false, Dummy::class)]; > yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class)]; > ``` > > While reviewing #3402, @dunglas found a potential BC break with objects that may be used in URIs as `string`s (therefore not `objects`): > > * [#3402 (comment)](https://github.com/api-platform/core/pull/3402#discussion_r381567578) > > * [#3402 (comment)](https://github.com/api-platform/core/pull/3402#discussion_r381938614) > > > **How to reproduce** > > The test should instead convert `object` to `object`: > > ``` > > yield [['type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT)]; > yield [['type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT, true)]; > > // ... > > yield [['type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT, false, Dummy::class)]; > yield [['type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class)]; > ``` --- src/JsonSchema/TypeFactory.php | 4 ++-- tests/JsonSchema/TypeFactoryTest.php | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/JsonSchema/TypeFactory.php b/src/JsonSchema/TypeFactory.php index effbf633242..cb617f26773 100644 --- a/src/JsonSchema/TypeFactory.php +++ b/src/JsonSchema/TypeFactory.php @@ -117,7 +117,7 @@ private function makeBasicType(Type $type, string $format = 'json', ?bool $reada private function getClassType(?string $className, string $format, ?bool $readableLink, ?array $serializerContext, ?Schema $schema): array { if (null === $className) { - return ['type' => 'object']; + return ['type' => 'string']; } if (is_a($className, \DateTimeInterface::class, true)) { @@ -141,7 +141,7 @@ private function getClassType(?string $className, string $format, ?bool $readabl // Skip if $schema is null (filters only support basic types) if (null === $schema) { - return ['type' => 'object']; + return ['type' => 'string']; } if ($this->isResourceClass($className) && true !== $readableLink) { diff --git a/tests/JsonSchema/TypeFactoryTest.php b/tests/JsonSchema/TypeFactoryTest.php index d7e2b35234a..37a4fbac539 100644 --- a/tests/JsonSchema/TypeFactoryTest.php +++ b/tests/JsonSchema/TypeFactoryTest.php @@ -42,13 +42,13 @@ public function typeProvider(): iterable yield [['nullable' => true, 'type' => 'boolean'], new Type(Type::BUILTIN_TYPE_BOOL, true)]; yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_STRING)]; yield [['nullable' => true, 'type' => 'string'], new Type(Type::BUILTIN_TYPE_STRING, true)]; - yield [['type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT)]; - yield [['nullable' => true, 'type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT, true)]; + yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT)]; + yield [['nullable' => true, 'type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT, true)]; yield [['type' => 'string', 'format' => 'date-time'], new Type(Type::BUILTIN_TYPE_OBJECT, false, \DateTimeImmutable::class)]; yield [['nullable' => true, 'type' => 'string', 'format' => 'date-time'], new Type(Type::BUILTIN_TYPE_OBJECT, true, \DateTimeImmutable::class)]; yield [['type' => 'string', 'format' => 'duration'], new Type(Type::BUILTIN_TYPE_OBJECT, false, \DateInterval::class)]; - yield [['type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT, false, Dummy::class)]; - yield [['nullable' => true, 'type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class)]; + yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT, false, Dummy::class)]; + yield [['nullable' => true, 'type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class)]; yield [['type' => 'array', 'items' => ['type' => 'string']], new Type(Type::BUILTIN_TYPE_STRING, false, null, true)]; yield 'array can be itself nullable' => [ ['nullable' => true, 'type' => 'array', 'items' => ['type' => 'string']], @@ -160,13 +160,13 @@ public function openAPIV2typeProvider(): iterable yield [['type' => 'boolean'], new Type(Type::BUILTIN_TYPE_BOOL, true)]; yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_STRING)]; yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_STRING, true)]; - yield [['type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT)]; - yield [['type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT, true)]; + yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT)]; + yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT, true)]; yield [['type' => 'string', 'format' => 'date-time'], new Type(Type::BUILTIN_TYPE_OBJECT, false, \DateTimeImmutable::class)]; yield [['type' => 'string', 'format' => 'date-time'], new Type(Type::BUILTIN_TYPE_OBJECT, true, \DateTimeImmutable::class)]; yield [['type' => 'string', 'format' => 'duration'], new Type(Type::BUILTIN_TYPE_OBJECT, false, \DateInterval::class)]; - yield [['type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT, false, Dummy::class)]; - yield [['type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class)]; + yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT, false, Dummy::class)]; + yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class)]; yield [['type' => 'array', 'items' => ['type' => 'string']], new Type(Type::BUILTIN_TYPE_STRING, false, null, true)]; yield 'array can be itself nullable, but ignored in OpenAPI V2' => [ ['type' => 'array', 'items' => ['type' => 'string']], From 48afa94be088dd4351bb6630caedd1e95ae6c937 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 20 Feb 2020 16:24:12 +0100 Subject: [PATCH 7/9] Moved `public const` declarations after `use` of traits Ref: https://github.com/api-platform/core/pull/3402#discussion_r382033877 --- src/JsonSchema/TypeFactory.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/JsonSchema/TypeFactory.php b/src/JsonSchema/TypeFactory.php index cb617f26773..01cff478477 100644 --- a/src/JsonSchema/TypeFactory.php +++ b/src/JsonSchema/TypeFactory.php @@ -27,6 +27,8 @@ */ final class TypeFactory implements TypeFactoryInterface { + use ResourceClassInfoTrait; + /** * This constant is to be provided as serializer context key to conditionally enable types to be generated in * a format that is compatible with OpenAPI specifications **PREVIOUS** to 3.0. @@ -40,8 +42,6 @@ final class TypeFactory implements TypeFactoryInterface */ public const CONTEXT_SERIALIZATION_FORMAT_OPENAPI_PRE_V3_0 = self::class.'::CONTEXT_SERIALIZATION_FORMAT_OPENAPI_PRE_V3_0'; - use ResourceClassInfoTrait; - /** * @var SchemaFactoryInterface|null */ From 25697ee7fb6b0e95938590476332c5a22dede64f Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 20 Feb 2020 16:25:39 +0100 Subject: [PATCH 8/9] Squashed coalesce operation into one line Ref: https://github.com/api-platform/core/pull/3402#discussion_r382034005 --- src/JsonSchema/TypeFactory.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/JsonSchema/TypeFactory.php b/src/JsonSchema/TypeFactory.php index 01cff478477..d267f8a26d8 100644 --- a/src/JsonSchema/TypeFactory.php +++ b/src/JsonSchema/TypeFactory.php @@ -64,8 +64,7 @@ public function getType(Type $type, string $format = 'json', ?bool $readableLink { if ($type->isCollection()) { $keyType = $type->getCollectionKeyType(); - $subType = $type->getCollectionValueType() - ?? new Type($type->getBuiltinType(), false, $type->getClassName(), false); + $subType = $type->getCollectionValueType() ?? new Type($type->getBuiltinType(), false, $type->getClassName(), false); if (null !== $keyType && Type::BUILTIN_TYPE_STRING === $keyType->getBuiltinType()) { return $this->addNullabilityToTypeDefinition( From 9d17089d1c75d65122bdd092c25dce00d2bf0763 Mon Sep 17 00:00:00 2001 From: Teoh Han Hui Date: Thu, 27 Feb 2020 16:26:44 +0100 Subject: [PATCH 9/9] Revert changes related to $serializerContext as we already have Schema::getVersion() --- src/JsonSchema/SchemaFactory.php | 8 ++-- src/JsonSchema/TypeFactory.php | 47 +++++-------------- .../Serializer/DocumentationNormalizer.php | 13 +---- tests/JsonSchema/TypeFactoryTest.php | 31 +++++------- 4 files changed, 27 insertions(+), 72 deletions(-) diff --git a/src/JsonSchema/SchemaFactory.php b/src/JsonSchema/SchemaFactory.php index 7a0eaf6f3b2..9d075e38ccf 100644 --- a/src/JsonSchema/SchemaFactory.php +++ b/src/JsonSchema/SchemaFactory.php @@ -258,19 +258,19 @@ private function getMetadata(string $className, string $type = Schema::TYPE_OUTP return [ $resourceMetadata, - $this->getSerializerContext($resourceMetadata, $type, $operationType, $operationName, $serializerContext), + $serializerContext ?? $this->getSerializerContext($resourceMetadata, $type, $operationType, $operationName), $inputOrOutput['class'], ]; } - private function getSerializerContext(ResourceMetadata $resourceMetadata, string $type = Schema::TYPE_OUTPUT, ?string $operationType, ?string $operationName, ?array $previousSerializerContext): array + private function getSerializerContext(ResourceMetadata $resourceMetadata, string $type = Schema::TYPE_OUTPUT, ?string $operationType, ?string $operationName): array { $attribute = Schema::TYPE_OUTPUT === $type ? 'normalization_context' : 'denormalization_context'; if (null === $operationType || null === $operationName) { - return array_merge($resourceMetadata->getAttribute($attribute, []), (array) $previousSerializerContext); + return $resourceMetadata->getAttribute($attribute, []); } - return array_merge($resourceMetadata->getTypedOperationAttribute($operationType, $operationName, $attribute, [], true), (array) $previousSerializerContext); + return $resourceMetadata->getTypedOperationAttribute($operationType, $operationName, $attribute, [], true); } } diff --git a/src/JsonSchema/TypeFactory.php b/src/JsonSchema/TypeFactory.php index d267f8a26d8..f3d3f48a63e 100644 --- a/src/JsonSchema/TypeFactory.php +++ b/src/JsonSchema/TypeFactory.php @@ -29,19 +29,6 @@ final class TypeFactory implements TypeFactoryInterface { use ResourceClassInfoTrait; - /** - * This constant is to be provided as serializer context key to conditionally enable types to be generated in - * a format that is compatible with OpenAPI specifications **PREVIOUS** to 3.0. - * - * Without this flag being set, the generated format will only be compatible with Swagger 3.0 or newer. - * - * Once support for OpenAPI < 3.0 is gone, this constant **WILL BE REMOVED** - * - * @internal Once support for OpenAPI < 3.0 is gone, this constant **WILL BE REMOVED** - do not rely on - * it in downstream projects! - */ - public const CONTEXT_SERIALIZATION_FORMAT_OPENAPI_PRE_V3_0 = self::class.'::CONTEXT_SERIALIZATION_FORMAT_OPENAPI_PRE_V3_0'; - /** * @var SchemaFactoryInterface|null */ @@ -67,31 +54,19 @@ public function getType(Type $type, string $format = 'json', ?bool $readableLink $subType = $type->getCollectionValueType() ?? new Type($type->getBuiltinType(), false, $type->getClassName(), false); if (null !== $keyType && Type::BUILTIN_TYPE_STRING === $keyType->getBuiltinType()) { - return $this->addNullabilityToTypeDefinition( - [ - 'type' => 'object', - 'additionalProperties' => $this->getType($subType, $format, $readableLink, $serializerContext, $schema), - ], - $type, - (array) $serializerContext - ); + return $this->addNullabilityToTypeDefinition([ + 'type' => 'object', + 'additionalProperties' => $this->getType($subType, $format, $readableLink, $serializerContext, $schema), + ], $type, $schema); } - return $this->addNullabilityToTypeDefinition( - [ - 'type' => 'array', - 'items' => $this->getType($subType, $format, $readableLink, $serializerContext, $schema), - ], - $type, - (array) $serializerContext - ); + return $this->addNullabilityToTypeDefinition([ + 'type' => 'array', + 'items' => $this->getType($subType, $format, $readableLink, $serializerContext, $schema), + ], $type, $schema); } - return $this->addNullabilityToTypeDefinition( - $this->makeBasicType($type, $format, $readableLink, $serializerContext, $schema), - $type, - (array) $serializerContext - ); + return $this->addNullabilityToTypeDefinition($this->makeBasicType($type, $format, $readableLink, $serializerContext, $schema), $type, $schema); } private function makeBasicType(Type $type, string $format = 'json', ?bool $readableLink = null, ?array $serializerContext = null, Schema $schema = null): array @@ -169,9 +144,9 @@ private function getClassType(?string $className, string $format, ?bool $readabl * * @return array */ - private function addNullabilityToTypeDefinition(array $jsonSchema, Type $type, array $serializerContext): array + private function addNullabilityToTypeDefinition(array $jsonSchema, Type $type, ?Schema $schema): array { - if (\array_key_exists(self::CONTEXT_SERIALIZATION_FORMAT_OPENAPI_PRE_V3_0, $serializerContext)) { + if ($schema && Schema::VERSION_SWAGGER === $schema->getVersion()) { return $jsonSchema; } diff --git a/src/Swagger/Serializer/DocumentationNormalizer.php b/src/Swagger/Serializer/DocumentationNormalizer.php index ac1a145df59..835fe75a740 100644 --- a/src/Swagger/Serializer/DocumentationNormalizer.php +++ b/src/Swagger/Serializer/DocumentationNormalizer.php @@ -594,10 +594,6 @@ private function getJsonSchema(bool $v3, \ArrayObject $definitions, string $reso $schema = new Schema($v3 ? Schema::VERSION_OPENAPI : Schema::VERSION_SWAGGER); $schema->setDefinitions($definitions); - if (!$v3) { - $serializerContext = array_merge([TypeFactory::CONTEXT_SERIALIZATION_FORMAT_OPENAPI_PRE_V3_0 => null], (array) $serializerContext); - } - $this->jsonSchemaFactory->buildSchema($resourceClass, $format, $type, $operationType, $operationName, $schema, $serializerContext, $forceCollection); return $schema; @@ -724,14 +720,7 @@ private function getFiltersParameters(bool $v3, string $resourceClass, string $o 'required' => $data['required'], ]; - $type = \in_array($data['type'], Type::$builtinTypes, true) - ? $this->jsonSchemaTypeFactory->getType( - new Type($data['type'], false, null, $data['is_collection'] ?? false), - 'json', - null, - $v3 ? null : [TypeFactory::CONTEXT_SERIALIZATION_FORMAT_OPENAPI_PRE_V3_0 => null] - ) - : ['type' => 'string']; + $type = \in_array($data['type'], Type::$builtinTypes, true) ? $this->jsonSchemaTypeFactory->getType(new Type($data['type'], false, null, $data['is_collection'] ?? false)) : ['type' => 'string']; $v3 ? $parameter['schema'] = $type : $parameter += $type; if ($v3 && isset($data['schema'])) { diff --git a/tests/JsonSchema/TypeFactoryTest.php b/tests/JsonSchema/TypeFactoryTest.php index 37a4fbac539..7f6259501ed 100644 --- a/tests/JsonSchema/TypeFactoryTest.php +++ b/tests/JsonSchema/TypeFactoryTest.php @@ -29,7 +29,7 @@ class TypeFactoryTest extends TestCase public function testGetType(array $schema, Type $type): void { $typeFactory = new TypeFactory(); - $this->assertEquals($schema, $typeFactory->getType($type)); + $this->assertEquals($schema, $typeFactory->getType($type, 'json', null, null, new Schema())); } public function typeProvider(): iterable @@ -47,8 +47,8 @@ public function typeProvider(): iterable yield [['type' => 'string', 'format' => 'date-time'], new Type(Type::BUILTIN_TYPE_OBJECT, false, \DateTimeImmutable::class)]; yield [['nullable' => true, 'type' => 'string', 'format' => 'date-time'], new Type(Type::BUILTIN_TYPE_OBJECT, true, \DateTimeImmutable::class)]; yield [['type' => 'string', 'format' => 'duration'], new Type(Type::BUILTIN_TYPE_OBJECT, false, \DateInterval::class)]; - yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT, false, Dummy::class)]; - yield [['nullable' => true, 'type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class)]; + yield [['type' => 'string', 'format' => 'iri-reference'], new Type(Type::BUILTIN_TYPE_OBJECT, false, Dummy::class)]; + yield [['nullable' => true, 'type' => 'string', 'format' => 'iri-reference'], new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class)]; yield [['type' => 'array', 'items' => ['type' => 'string']], new Type(Type::BUILTIN_TYPE_STRING, false, null, true)]; yield 'array can be itself nullable' => [ ['nullable' => true, 'type' => 'array', 'items' => ['type' => 'string']], @@ -147,7 +147,7 @@ public function typeProvider(): iterable public function testGetTypeWithOpenAPIV2Syntax(array $schema, Type $type): void { $typeFactory = new TypeFactory(); - $this->assertSame($schema, $typeFactory->getType($type, 'json', null, [TypeFactory::CONTEXT_SERIALIZATION_FORMAT_OPENAPI_PRE_V3_0 => null])); + $this->assertSame($schema, $typeFactory->getType($type, 'json', null, null, new Schema(Schema::VERSION_SWAGGER))); } public function openAPIV2typeProvider(): iterable @@ -165,8 +165,8 @@ public function openAPIV2typeProvider(): iterable yield [['type' => 'string', 'format' => 'date-time'], new Type(Type::BUILTIN_TYPE_OBJECT, false, \DateTimeImmutable::class)]; yield [['type' => 'string', 'format' => 'date-time'], new Type(Type::BUILTIN_TYPE_OBJECT, true, \DateTimeImmutable::class)]; yield [['type' => 'string', 'format' => 'duration'], new Type(Type::BUILTIN_TYPE_OBJECT, false, \DateInterval::class)]; - yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT, false, Dummy::class)]; - yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class)]; + yield [['type' => 'string', 'format' => 'iri-reference'], new Type(Type::BUILTIN_TYPE_OBJECT, false, Dummy::class)]; + yield [['type' => 'string', 'format' => 'iri-reference'], new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class)]; yield [['type' => 'array', 'items' => ['type' => 'string']], new Type(Type::BUILTIN_TYPE_STRING, false, null, true)]; yield 'array can be itself nullable, but ignored in OpenAPI V2' => [ ['type' => 'array', 'items' => ['type' => 'string']], @@ -284,20 +284,11 @@ public function testGetClassTypeWithNullability(): void $typeFactory = new TypeFactory(); $typeFactory->setSchemaFactory($schemaFactory); - self::assertSame( - [ - 'nullable' => true, - 'anyOf' => [ - ['$ref' => 'the-ref-name'], - ], + self::assertSame([ + 'nullable' => true, + 'anyOf' => [ + ['$ref' => 'the-ref-name'], ], - $typeFactory->getType( - new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class), - 'jsonld', - true, - ['foo' => 'bar'], - new Schema() - ) - ); + ], $typeFactory->getType(new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class), 'jsonld', true, ['foo' => 'bar'], new Schema())); } }