Conditionally fall back to JSON-Schema compatible with Swagger/OpenAPI v2

Ocramius · Ocramius · commit 03edb554d3dd · 2020-02-20T12:11:36.000+01:00
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.
diff --git 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
@@ -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<string, mixed>
      */
-    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
@@ -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
@@ -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
@@ -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,
                         ]),
                     ],
                 ]),

Original file line number	Diff line number	Diff line change
`@@ -258,19 +258,19 @@ private function getMetadata(string $className, string $type = Schema::TYPE_OUTP`
`258`	`258`
`259`	`259`	`return [`
`260`	`260`	`$resourceMetadata,`
`261`		`- $serializerContext ?? $this->getSerializerContext($resourceMetadata, $type, $operationType, $operationName),`
	`261`	`+ $this->getSerializerContext($resourceMetadata, $type, $operationType, $operationName, $serializerContext),`
`262`	`262`	`$inputOrOutput['class'],`
`263`	`263`	`];`
`264`	`264`	`}`
`265`	`265`
`266`		`- private function getSerializerContext(ResourceMetadata $resourceMetadata, string $type = Schema::TYPE_OUTPUT, ?string $operationType, ?string $operationName): array`
	`266`	`+ private function getSerializerContext(ResourceMetadata $resourceMetadata, string $type = Schema::TYPE_OUTPUT, ?string $operationType, ?string $operationName, ?array $previousSerializerContext): array`
`267`	`267`	`{`
`268`	`268`	`$attribute = Schema::TYPE_OUTPUT === $type ? 'normalization_context' : 'denormalization_context';`
`269`	`269`
`270`	`270`	`if (null === $operationType \|\| null === $operationName) {`
`271`		`- return $resourceMetadata->getAttribute($attribute, []);`
	`271`	`+ return array_merge($resourceMetadata->getAttribute($attribute, []), (array) $previousSerializerContext);`
`272`	`272`	`}`
`273`	`273`
`274`		`- return $resourceMetadata->getTypedOperationAttribute($operationType, $operationName, $attribute, [], true);`
	`274`	`+ return array_merge($resourceMetadata->getTypedOperationAttribute($operationType, $operationName, $attribute, [], true), (array) $previousSerializerContext);`
`275`	`275`	`}`
`276`	`276`	`}`