Skip to content

Commit

Permalink
Conditionally fall back to JSON-Schema compatible with Swagger/OpenAP…
Browse files Browse the repository at this point in the history 
…I 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.
  • Loading branch information
@Ocramius
Ocramius committed Feb 20, 2020
1 parent 6b6c40d commit 39578be
Show file tree
Hide file tree
Showing 5 changed files with 164 additions and 32 deletions.
8 changes: 4 additions & 4 deletions src/JsonSchema/SchemaFactory.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
30 changes: 25 additions & 5 deletions src/JsonSchema/TypeFactory.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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
);
}

Expand All @@ -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
);
}

Expand All @@ -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'];
Expand Down Expand Up @@ -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;
}
Expand Down
13 changes: 12 additions & 1 deletion src/Swagger/Serializer/DocumentationNormalizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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'])) {
Expand Down
107 changes: 107 additions & 0 deletions tests/JsonSchema/TypeFactoryTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
38 changes: 16 additions & 22 deletions tests/Swagger/Serializer/DocumentationNormalizerV2Test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.',
]),
],
Expand Down Expand Up @@ -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 $nameToNormalize === 'nameConverted'
? 'name_converted'
: $nameToNormalize;
});

$operationPathResolver = new CustomOperationPathResolver(new OperationPathResolver(new UnderscorePathSegmentNameGenerator()));

Expand All @@ -408,10 +409,6 @@ interface_exists(AdvancedNameConverterInterface::class)
* @var PropertyMetadataFactoryInterface
*/
$propertyMetadataFactory = $propertyMetadataFactoryProphecy->reveal();
/**
* @var NameConverterInterface
*/
$nameConverter = $nameConverterProphecy->reveal();

/**
* @var TypeFactoryInterface|null
Expand Down Expand Up @@ -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,
]),
],
]),
Expand Down

0 comments on commit 39578be

Please sign in to comment.