Skip to content

Commit 713b3ed

Browse files
teohhanhuiteohhanhui
authored
Merge pull request #3402 from Ocramius/fix/correct-json-schema-definitions-for-nullability-and-nested-structures
Corrected schema generation for `array<string, T>`, `object`, `?array`, `?object` and `?type`
2 parents ef98086 + 9d17089 commit 713b3ed

File tree

4 files changed

+298
-21
lines changed

4 files changed

+298
-21
lines changed

‎src/JsonSchema/TypeFactory.php

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,27 @@ public function setSchemaFactory(SchemaFactoryInterface $schemaFactory): void
5050
public function getType(Type $type, string $format = 'json', ?bool $readableLink = null, ?array $serializerContext = null, Schema $schema = null): array
5151
{
5252
if ($type->isCollection()) {
53-
$subType = new Type($type->getBuiltinType(), $type->isNullable(), $type->getClassName(), false);
53+
$keyType = $type->getCollectionKeyType();
54+
$subType = $type->getCollectionValueType() ?? new Type($type->getBuiltinType(), false, $type->getClassName(), false);
5455

55-
return [
56+
if (null !== $keyType && Type::BUILTIN_TYPE_STRING === $keyType->getBuiltinType()) {
57+
return $this->addNullabilityToTypeDefinition([
58+
'type' => 'object',
59+
'additionalProperties' => $this->getType($subType, $format, $readableLink, $serializerContext, $schema),
60+
], $type, $schema);
61+
}
62+
63+
return $this->addNullabilityToTypeDefinition([
5664
'type' => 'array',
5765
'items' => $this->getType($subType, $format, $readableLink, $serializerContext, $schema),
58-
];
66+
], $type, $schema);
5967
}
6068

69+
return $this->addNullabilityToTypeDefinition($this->makeBasicType($type, $format, $readableLink, $serializerContext, $schema), $type, $schema);
70+
}
71+
72+
private function makeBasicType(Type $type, string $format = 'json', ?bool $readableLink = null, ?array $serializerContext = null, Schema $schema = null): array
73+
{
6174
switch ($type->getBuiltinType()) {
6275
case Type::BUILTIN_TYPE_INT:
6376
return ['type' => 'integer'];
@@ -75,7 +88,7 @@ public function getType(Type $type, string $format = 'json', ?bool $readableLink
7588
/**
7689
* 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.
7790
*/
78-
private function getClassType(?string $className, string $format = 'json', ?bool $readableLink = null, ?array $serializerContext = null, ?Schema $schema = null): array
91+
private function getClassType(?string $className, string $format, ?bool $readableLink, ?array $serializerContext, ?Schema $schema): array
7992
{
8093
if (null === $className) {
8194
return ['type' => 'string'];
@@ -125,4 +138,29 @@ private function getClassType(?string $className, string $format = 'json', ?bool
125138

126139
return ['$ref' => $subSchema['$ref']];
127140
}
141+
142+
/**
143+
* @param array<string, mixed> $jsonSchema
144+
*
145+
* @return array<string, mixed>
146+
*/
147+
private function addNullabilityToTypeDefinition(array $jsonSchema, Type $type, ?Schema $schema): array
148+
{
149+
if ($schema && Schema::VERSION_SWAGGER === $schema->getVersion()) {
150+
return $jsonSchema;
151+
}
152+
153+
if (!$type->isNullable()) {
154+
return $jsonSchema;
155+
}
156+
157+
if (\array_key_exists('$ref', $jsonSchema)) {
158+
return [
159+
'nullable' => true,
160+
'anyOf' => [$jsonSchema],
161+
];
162+
}
163+
164+
return array_merge($jsonSchema, ['nullable' => true]);
165+
}
128166
}

‎tests/JsonSchema/TypeFactoryTest.php

Lines changed: 234 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,225 @@ class TypeFactoryTest extends TestCase
2929
public function testGetType(array $schema, Type $type): void
3030
{
3131
$typeFactory = new TypeFactory();
32-
$this->assertSame($schema, $typeFactory->getType($type));
32+
$this->assertEquals($schema, $typeFactory->getType($type, 'json', null, null, new Schema()));
3333
}
3434

3535
public function typeProvider(): iterable
3636
{
3737
yield [['type' => 'integer'], new Type(Type::BUILTIN_TYPE_INT)];
38+
yield [['nullable' => true, 'type' => 'integer'], new Type(Type::BUILTIN_TYPE_INT, true)];
3839
yield [['type' => 'number'], new Type(Type::BUILTIN_TYPE_FLOAT)];
40+
yield [['nullable' => true, 'type' => 'number'], new Type(Type::BUILTIN_TYPE_FLOAT, true)];
3941
yield [['type' => 'boolean'], new Type(Type::BUILTIN_TYPE_BOOL)];
42+
yield [['nullable' => true, 'type' => 'boolean'], new Type(Type::BUILTIN_TYPE_BOOL, true)];
43+
yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_STRING)];
44+
yield [['nullable' => true, 'type' => 'string'], new Type(Type::BUILTIN_TYPE_STRING, true)];
4045
yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT)];
46+
yield [['nullable' => true, 'type' => 'string'], 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 [['nullable' => true, 'type' => 'string', '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' => 'string', 'format' => 'iri-reference'], new Type(Type::BUILTIN_TYPE_OBJECT, false, Dummy::class)];
51+
yield [['nullable' => true, 'type' => 'string', 'format' => 'iri-reference'], 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+
['nullable' => true, 'type' => 'array', 'items' => ['type' => 'string']],
55+
new Type(Type::BUILTIN_TYPE_STRING, true, null, true),
56+
];
57+
58+
yield 'array can contain nullable values' => [
59+
[
60+
'type' => 'array',
61+
'items' => [
62+
'nullable' => true,
63+
'type' => 'string',
64+
],
65+
],
66+
new Type(Type::BUILTIN_TYPE_STRING, false, null, true, null, new Type(Type::BUILTIN_TYPE_STRING, true, null, false)),
67+
];
68+
69+
yield 'map with string keys becomes an object' => [
70+
['type' => 'object', 'additionalProperties' => ['type' => 'string']],
71+
new Type(
72+
Type::BUILTIN_TYPE_STRING,
73+
false,
74+
null,
75+
true,
76+
new Type(Type::BUILTIN_TYPE_STRING, false, null, false)
77+
),
78+
];
79+
80+
yield 'nullable map with string keys becomes a nullable object' => [
81+
[
82+
'nullable' => true,
83+
'type' => 'object',
84+
'additionalProperties' => ['type' => 'string'],
85+
],
86+
new Type(
87+
Type::BUILTIN_TYPE_STRING,
88+
true,
89+
null,
90+
true,
91+
new Type(Type::BUILTIN_TYPE_STRING, false, null, false),
92+
new Type(Type::BUILTIN_TYPE_STRING, false, null, false)
93+
),
94+
];
95+
96+
yield 'map value type will be considered' => [
97+
['type' => 'object', 'additionalProperties' => ['type' => 'integer']],
98+
new Type(
99+
Type::BUILTIN_TYPE_ARRAY,
100+
false,
101+
null,
102+
true,
103+
new Type(Type::BUILTIN_TYPE_STRING, false, null, false),
104+
new Type(Type::BUILTIN_TYPE_INT, false, null, false)
105+
),
106+
];
107+
108+
yield 'map value type nullability will be considered' => [
109+
[
110+
'type' => 'object',
111+
'additionalProperties' => [
112+
'nullable' => true,
113+
'type' => 'integer',
114+
],
115+
],
116+
new Type(
117+
Type::BUILTIN_TYPE_ARRAY,
118+
false,
119+
null,
120+
true,
121+
new Type(Type::BUILTIN_TYPE_STRING, false, null, false),
122+
new Type(Type::BUILTIN_TYPE_INT, true, null, false)
123+
),
124+
];
125+
126+
yield 'nullable map can contain nullable values' => [
127+
[
128+
'nullable' => true,
129+
'type' => 'object',
130+
'additionalProperties' => [
131+
'nullable' => true,
132+
'type' => 'integer',
133+
],
134+
],
135+
new Type(
136+
Type::BUILTIN_TYPE_ARRAY,
137+
true,
138+
null,
139+
true,
140+
new Type(Type::BUILTIN_TYPE_STRING, false, null, false),
141+
new Type(Type::BUILTIN_TYPE_INT, true, null, false)
142+
),
143+
];
144+
}
145+
146+
/** @dataProvider openAPIV2typeProvider */
147+
public function testGetTypeWithOpenAPIV2Syntax(array $schema, Type $type): void
148+
{
149+
$typeFactory = new TypeFactory();
150+
$this->assertSame($schema, $typeFactory->getType($type, 'json', null, null, new Schema(Schema::VERSION_SWAGGER)));
151+
}
152+
153+
public function openAPIV2typeProvider(): iterable
154+
{
155+
yield [['type' => 'integer'], new Type(Type::BUILTIN_TYPE_INT)];
156+
yield [['type' => 'integer'], new Type(Type::BUILTIN_TYPE_INT, true)];
157+
yield [['type' => 'number'], new Type(Type::BUILTIN_TYPE_FLOAT)];
158+
yield [['type' => 'number'], new Type(Type::BUILTIN_TYPE_FLOAT, true)];
159+
yield [['type' => 'boolean'], new Type(Type::BUILTIN_TYPE_BOOL)];
160+
yield [['type' => 'boolean'], new Type(Type::BUILTIN_TYPE_BOOL, true)];
161+
yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_STRING)];
162+
yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_STRING, true)];
163+
yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT)];
164+
yield [['type' => 'string'], new Type(Type::BUILTIN_TYPE_OBJECT, true)];
165+
yield [['type' => 'string', 'format' => 'date-time'], new Type(Type::BUILTIN_TYPE_OBJECT, false, \DateTimeImmutable::class)];
166+
yield [['type' => 'string', 'format' => 'date-time'], new Type(Type::BUILTIN_TYPE_OBJECT, true, \DateTimeImmutable::class)];
167+
yield [['type' => 'string', 'format' => 'duration'], new Type(Type::BUILTIN_TYPE_OBJECT, false, \DateInterval::class)];
168+
yield [['type' => 'string', 'format' => 'iri-reference'], new Type(Type::BUILTIN_TYPE_OBJECT, false, Dummy::class)];
169+
yield [['type' => 'string', 'format' => 'iri-reference'], new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class)];
170+
yield [['type' => 'array', 'items' => ['type' => 'string']], new Type(Type::BUILTIN_TYPE_STRING, false, null, true)];
171+
yield 'array can be itself nullable, but ignored in OpenAPI V2' => [
172+
['type' => 'array', 'items' => ['type' => 'string']],
173+
new Type(Type::BUILTIN_TYPE_STRING, true, null, true),
174+
];
175+
176+
yield 'array can contain nullable values, but ignored in OpenAPI V2' => [
177+
[
178+
'type' => 'array',
179+
'items' => ['type' => 'string'],
180+
],
181+
new Type(Type::BUILTIN_TYPE_STRING, false, null, true, null, new Type(Type::BUILTIN_TYPE_STRING, true, null, false)),
182+
];
183+
184+
yield 'map with string keys becomes an object' => [
185+
['type' => 'object', 'additionalProperties' => ['type' => 'string']],
186+
new Type(
187+
Type::BUILTIN_TYPE_STRING,
188+
false,
189+
null,
190+
true,
191+
new Type(Type::BUILTIN_TYPE_STRING, false, null, false)
192+
),
193+
];
194+
195+
yield 'nullable map with string keys becomes a nullable object, but ignored in OpenAPI V2' => [
196+
[
197+
'type' => 'object',
198+
'additionalProperties' => ['type' => 'string'],
199+
],
200+
new Type(
201+
Type::BUILTIN_TYPE_STRING,
202+
true,
203+
null,
204+
true,
205+
new Type(Type::BUILTIN_TYPE_STRING, false, null, false),
206+
new Type(Type::BUILTIN_TYPE_STRING, false, null, false)
207+
),
208+
];
209+
210+
yield 'map value type will be considered' => [
211+
['type' => 'object', 'additionalProperties' => ['type' => 'integer']],
212+
new Type(
213+
Type::BUILTIN_TYPE_ARRAY,
214+
false,
215+
null,
216+
true,
217+
new Type(Type::BUILTIN_TYPE_STRING, false, null, false),
218+
new Type(Type::BUILTIN_TYPE_INT, false, null, false)
219+
),
220+
];
221+
222+
yield 'map value type nullability will be considered, but ignored in OpenAPI V2' => [
223+
[
224+
'type' => 'object',
225+
'additionalProperties' => ['type' => 'integer'],
226+
],
227+
new Type(
228+
Type::BUILTIN_TYPE_ARRAY,
229+
false,
230+
null,
231+
true,
232+
new Type(Type::BUILTIN_TYPE_STRING, false, null, false),
233+
new Type(Type::BUILTIN_TYPE_INT, true, null, false)
234+
),
235+
];
236+
237+
yield 'nullable map can contain nullable values, but ignored in OpenAPI V2' => [
238+
[
239+
'type' => 'object',
240+
'additionalProperties' => ['type' => 'integer'],
241+
],
242+
new Type(
243+
Type::BUILTIN_TYPE_ARRAY,
244+
true,
245+
null,
246+
true,
247+
new Type(Type::BUILTIN_TYPE_STRING, false, null, false),
248+
new Type(Type::BUILTIN_TYPE_INT, true, null, false)
249+
),
250+
];
45251
}
46252

47253
public function testGetClassType(): void
@@ -59,4 +265,30 @@ public function testGetClassType(): void
59265

60266
$this->assertSame(['$ref' => 'ref'], $typeFactory->getType(new Type(Type::BUILTIN_TYPE_OBJECT, false, Dummy::class), 'jsonld', true, ['foo' => 'bar'], new Schema()));
61267
}
268+
269+
public function testGetClassTypeWithNullability(): void
270+
{
271+
$schemaFactory = $this->createMock(SchemaFactoryInterface::class);
272+
273+
$schemaFactory
274+
->method('buildSchema')
275+
->willReturnCallback(static function (): Schema {
276+
$schema = new Schema();
277+
278+
$schema['$ref'] = 'the-ref-name';
279+
$schema['description'] = 'more stuff here';
280+
281+
return $schema;
282+
});
283+
284+
$typeFactory = new TypeFactory();
285+
$typeFactory->setSchemaFactory($schemaFactory);
286+
287+
self::assertSame([
288+
'nullable' => true,
289+
'anyOf' => [
290+
['$ref' => 'the-ref-name'],
291+
],
292+
], $typeFactory->getType(new Type(Type::BUILTIN_TYPE_OBJECT, true, Dummy::class), 'jsonld', true, ['foo' => 'bar'], new Schema()));
293+
}
62294
}

‎tests/Swagger/Serializer/DocumentationNormalizerV2Test.php

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -344,9 +344,10 @@ private function doTestNormalize(OperationMethodResolverInterface $operationMeth
344344
]),
345345
'dummyDate' => new \ArrayObject([
346346
'type' => 'string',
347-
'description' => 'This is a \DateTimeInterface object.',
348347
'format' => 'date-time',
349-
]), ],
348+
'description' => 'This is a \DateTimeInterface object.',
349+
]),
350+
],
350351
]),
351352
]),
352353
];
@@ -380,13 +381,19 @@ private function doTestNormalizeWithNameConverter(bool $legacy = false): void
380381
$propertyMetadataFactoryProphecy->create(Dummy::class, 'name')->shouldBeCalled()->willReturn(new PropertyMetadata(new Type(Type::BUILTIN_TYPE_STRING), 'This is a name.', true, true, null, null, false));
381382
$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));
382383

383-
$nameConverterProphecy = $this->prophesize(
384-
interface_exists(AdvancedNameConverterInterface::class)
385-
? AdvancedNameConverterInterface::class
386-
: NameConverterInterface::class
387-
);
388-
$nameConverterProphecy->normalize('name', Dummy::class, DocumentationNormalizer::FORMAT, [])->willReturn('name')->shouldBeCalled();
389-
$nameConverterProphecy->normalize('nameConverted', Dummy::class, DocumentationNormalizer::FORMAT, [])->willReturn('name_converted')->shouldBeCalled();
384+
if (interface_exists(AdvancedNameConverterInterface::class)) {
385+
$nameConverter = $this->createMock(AdvancedNameConverterInterface::class);
386+
} else {
387+
$nameConverter = $this->createMock(NameConverterInterface::class);
388+
}
389+
390+
$nameConverter->method('normalize')
391+
->with(self::logicalOr('name', 'nameConverted'))
392+
->willReturnCallback(static function (string $nameToNormalize): string {
393+
return 'nameConverted' === $nameToNormalize
394+
? 'name_converted'
395+
: $nameToNormalize;
396+
});
390397

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

@@ -402,10 +409,6 @@ interface_exists(AdvancedNameConverterInterface::class)
402409
* @var PropertyMetadataFactoryInterface
403410
*/
404411
$propertyMetadataFactory = $propertyMetadataFactoryProphecy->reveal();
405-
/**
406-
* @var NameConverterInterface
407-
*/
408-
$nameConverter = $nameConverterProphecy->reveal();
409412

410413
/**
411414
* @var TypeFactoryInterface|null

0 commit comments

Comments
 (0)