Skip to content

Commit 44f44de

Browse files
OcramiusOcramius
committed
Corrected schema generation for array<string, T>, 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<string, T> 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<string, T>` as `{"type": "object", "additionalProperties": {"type": "T"}}` * `array<string, ?T> as `{"type": object, "additionalProperties": {"type": ["T", "null"]}}` * `null|array<string, T>` as `{"type": ["object", "null"], "additionalProperties": {"type": "T"}}` * `array<int, T>` 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"}}`
1 parent 40f4f74 commit 44f44de

File tree

3 files changed

+190
-11
lines changed

3 files changed

+190
-11
lines changed

‎src/JsonSchema/TypeFactory.php

Lines changed: 73 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
use ApiPlatform\Core\Util\ResourceClassInfoTrait;
1818
use Ramsey\Uuid\UuidInterface;
1919
use Symfony\Component\PropertyInfo\Type;
20+
use function array_merge;
21+
use function array_unique;
22+
use function array_values;
2023

2124
/**
2225
* {@inheritdoc}
@@ -50,14 +53,37 @@ public function setSchemaFactory(SchemaFactoryInterface $schemaFactory): void
5053
public function getType(Type $type, string $format = 'json', ?bool $readableLink = null, ?array $serializerContext = null, Schema $schema = null): array
5154
{
5255
if ($type->isCollection()) {
53-
$subType = new Type($type->getBuiltinType(), $type->isNullable(), $type->getClassName(), false);
54-
55-
return [
56-
'type' => 'array',
57-
'items' => $this->getType($subType, $format, $readableLink, $serializerContext, $schema),
58-
];
56+
$keyType = $type->getCollectionKeyType();
57+
$subType = $type->getCollectionValueType()
58+
?? new Type($type->getBuiltinType(), false, $type->getClassName(), false);
59+
60+
if (null !== $keyType && Type::BUILTIN_TYPE_STRING === $keyType->getBuiltinType()) {
61+
return $this->addNullabilityToTypeDefinition(
62+
[
63+
'type' => 'object',
64+
'additionalProperties' => $this->getType($subType, $format, $readableLink, $serializerContext, $schema),
65+
],
66+
$type
67+
);
68+
}
69+
70+
return $this->addNullabilityToTypeDefinition(
71+
[
72+
'type' => 'array',
73+
'items' => $this->getType($subType, $format, $readableLink, $serializerContext, $schema),
74+
],
75+
$type
76+
);
5977
}
6078

79+
return $this->addNullabilityToTypeDefinition(
80+
$this->makeBasicType($type, $format, $readableLink, $serializerContext, $schema),
81+
$type
82+
);
83+
}
84+
85+
private function makeBasicType(Type $type, string $format = 'json', ?bool $readableLink = null, ?array $serializerContext = null, Schema $schema = null): array
86+
{
6187
switch ($type->getBuiltinType()) {
6288
case Type::BUILTIN_TYPE_INT:
6389
return ['type' => 'integer'];
@@ -78,7 +104,7 @@ public function getType(Type $type, string $format = 'json', ?bool $readableLink
78104
private function getClassType(?string $className, string $format = 'json', ?bool $readableLink = null, ?array $serializerContext = null, ?Schema $schema = null): array
79105
{
80106
if (null === $className) {
81-
return ['type' => 'string'];
107+
return ['type' => 'object'];
82108
}
83109

84110
if (is_a($className, \DateTimeInterface::class, true)) {
@@ -102,7 +128,7 @@ private function getClassType(?string $className, string $format = 'json', ?bool
102128

103129
// Skip if $schema is null (filters only support basic types)
104130
if (null === $schema) {
105-
return ['type' => 'string'];
131+
return ['type' => 'object'];
106132
}
107133

108134
if ($this->isResourceClass($className) && true !== $readableLink) {
@@ -125,4 +151,43 @@ private function getClassType(?string $className, string $format = 'json', ?bool
125151

126152
return ['$ref' => $subSchema['$ref']];
127153
}
154+
155+
/**
156+
* @param array<string, mixed> $jsonSchema
157+
*
158+
* @return array<string, mixed>
159+
*
160+
* @psalm-param array{type=: string|list<string>} $jsonSchema
161+
*
162+
* @psalm-return array{type=: string|list<string>, $ref=: string}
163+
*/
164+
private function addNullabilityToTypeDefinition(array $jsonSchema, Type $type): array
165+
{
166+
if (!$type->isNullable()) {
167+
return $jsonSchema;
168+
}
169+
170+
if (!\array_key_exists('type', $jsonSchema)) {
171+
return [
172+
'oneOf' => [
173+
['type' => 'null'],
174+
$jsonSchema,
175+
],
176+
];
177+
}
178+
179+
return array_merge($jsonSchema, ['type' => $this->addNullToTypes((array) $jsonSchema['type'])]);
180+
}
181+
182+
/**
183+
* @param string[] $types
184+
*
185+
* @return string[]
186+
*
187+
* @psalm-param list<string> $types
188+
*/
189+
private function addNullToTypes(array $types): array
190+
{
191+
return array_values(array_unique(array_merge($types, ['null'])));
192+
}
128193
}

‎tests/JsonSchema/TypeFactoryTest.php

Lines changed: 113 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,89 @@ public function testGetType(array $schema, Type $type): void
3535
public function typeProvider(): iterable
3636
{
3737
yield [['type' => 'integer'], new Type(Type::BUILTIN_TYPE_INT)];
38+
yield [['type' => ['integer', 'null']], new Type(Type::BUILTIN_TYPE_INT, true)];
3839
yield [['type' => 'number'], new Type(Type::BUILTIN_TYPE_FLOAT)];
40+
yield [['type' => ['number', 'null']], new Type(Type::BUILTIN_TYPE_FLOAT, true)];
3941
yield [['type' => 'boolean'], new Type(Type::BUILTIN_TYPE_BOOL)];
40-
yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT)];
42+
yield [['type' => ['boolean', 'null']], new Type(Type::BUILTIN_TYPE_BOOL, true)];
43+
yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_STRING)];
44+
yield [['type' => ['string', 'null']], new Type(Type::BUILTIN_TYPE_STRING, true)];
45+
yield [['type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT)];
46+
yield [['type' => ['object', 'null']], new Type(Type::BUILTIN_TYPE_OBJECT, true)];
4147
yield [['type' => 'string', 'format' => 'date-time'], new Type(Type::BUILTIN_TYPE_OBJECT, false, \DateTimeImmutable::class)];
48+
yield [['type' => ['string', 'null'], 'format' => 'date-time'], new Type(Type::BUILTIN_TYPE_OBJECT, true, \DateTimeImmutable::class)];
4249
yield [['type' => 'string', 'format' => 'duration'], new Type(Type::BUILTIN_TYPE_OBJECT, false, \DateInterval::class)];
43-
yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT, false, Dummy::class)];
50+
yield [['type' => 'object'], new Type(Type::BUILTIN_TYPE_OBJECT, false, Dummy::class)];
51+
yield [['type' => ['object', 'null']], new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class)];
4452
yield [['type' => 'array', 'items' => ['type' => 'string']], new Type(Type::BUILTIN_TYPE_STRING, false, null, true)];
53+
yield 'array can be itself nullable' => [
54+
['type' => ['array', 'null'], 'items' => ['type' => 'string']],
55+
new Type(Type::BUILTIN_TYPE_STRING, true, null, true),
56+
];
57+
58+
yield 'array can contain nullable values' => [
59+
['type' => 'array', 'items' => ['type' => ['string', 'null']]],
60+
new Type(Type::BUILTIN_TYPE_STRING, false, null, true, null, new Type(Type::BUILTIN_TYPE_STRING, true, null, false)),
61+
];
62+
63+
yield 'map with string keys becomes an object' => [
64+
['type' => 'object', 'additionalProperties' => ['type' => 'string']],
65+
new Type(
66+
Type::BUILTIN_TYPE_STRING,
67+
false,
68+
null,
69+
true,
70+
new Type(Type::BUILTIN_TYPE_STRING, false, null, false)
71+
),
72+
];
73+
74+
yield 'nullable map with string keys becomes a nullable object' => [
75+
['type' => ['object', 'null'], 'additionalProperties' => ['type' => 'string']],
76+
new Type(
77+
Type::BUILTIN_TYPE_STRING,
78+
true,
79+
null,
80+
true,
81+
new Type(Type::BUILTIN_TYPE_STRING, false, null, false),
82+
new Type(Type::BUILTIN_TYPE_STRING, false, null, false)
83+
),
84+
];
85+
86+
yield 'map value type will be considered' => [
87+
['type' => 'object', 'additionalProperties' => ['type' => 'integer']],
88+
new Type(
89+
Type::BUILTIN_TYPE_ARRAY,
90+
false,
91+
null,
92+
true,
93+
new Type(Type::BUILTIN_TYPE_STRING, false, null, false),
94+
new Type(Type::BUILTIN_TYPE_INT, false, null, false)
95+
),
96+
];
97+
98+
yield 'map value type nullability will be considered' => [
99+
['type' => 'object', 'additionalProperties' => ['type' => ['integer', 'null']]],
100+
new Type(
101+
Type::BUILTIN_TYPE_ARRAY,
102+
false,
103+
null,
104+
true,
105+
new Type(Type::BUILTIN_TYPE_STRING, false, null, false),
106+
new Type(Type::BUILTIN_TYPE_INT, true, null, false)
107+
),
108+
];
109+
110+
yield 'nullable map can contain nullable values' => [
111+
['type' => ['object', 'null'], 'additionalProperties' => ['type' => ['integer', 'null']]],
112+
new Type(
113+
Type::BUILTIN_TYPE_ARRAY,
114+
true,
115+
null,
116+
true,
117+
new Type(Type::BUILTIN_TYPE_STRING, false, null, false),
118+
new Type(Type::BUILTIN_TYPE_INT, true, null, false)
119+
),
120+
];
45121
}
46122

47123
public function testGetClassType(): void
@@ -59,4 +135,39 @@ public function testGetClassType(): void
59135

60136
$this->assertSame(['$ref' => 'ref'], $typeFactory->getType(new Type(Type::BUILTIN_TYPE_OBJECT, false, Dummy::class), 'jsonld', true, ['foo' => 'bar'], new Schema()));
61137
}
138+
139+
public function testGetClassTypeWithNullability(): void
140+
{
141+
$schemaFactory = $this->createMock(SchemaFactoryInterface::class);
142+
143+
$schemaFactory
144+
->method('buildSchema')
145+
->willReturnCallback(static function (): Schema {
146+
$schema = new Schema();
147+
148+
$schema['$ref'] = 'the-ref-name';
149+
$schema['description'] = 'more stuff here';
150+
151+
return $schema;
152+
});
153+
154+
$typeFactory = new TypeFactory();
155+
$typeFactory->setSchemaFactory($schemaFactory);
156+
157+
self::assertSame(
158+
[
159+
'oneOf' => [
160+
['type' => 'null'],
161+
['$ref' => 'the-ref-name'],
162+
],
163+
],
164+
$typeFactory->getType(
165+
new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class),
166+
'jsonld',
167+
true,
168+
['foo' => 'bar'],
169+
new Schema()
170+
)
171+
);
172+
}
62173
}

‎tests/Swagger/Serializer/DocumentationNormalizerV3Test.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2000,7 +2000,10 @@ public function testNormalizeWithNestedNormalizationGroups(): void
20002000
]),
20012001
'relatedDummy' => new \ArrayObject([
20022002
'description' => 'This is a related dummy \o/.',
2003-
'$ref' => '#/components/schemas/'.$relatedDummyRef,
2003+
'oneOf' => [
2004+
['type' => 'null'],
2005+
['$ref' => '#/components/schemas/'.$relatedDummyRef],
2006+
],
20042007
]),
20052008
],
20062009
]),

0 commit comments

Comments
 (0)