Skip to content

Commit dc64343

Browse files
wol-softwol-soft
committed
Inherited properties are partially nullable (#28)
BC break: function signatures for allOf compositions don't accept null any longer if `implicitNull` is not enabled or the property is required
1 parent 5614b69 commit dc64343

11 files changed

+240
-23
lines changed

‎src/Model/Validator/AbstractComposedPropertyValidator.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,16 @@
1414
abstract class AbstractComposedPropertyValidator extends PropertyTemplateValidator
1515
{
1616
/** @var string */
17-
protected $composedProcessor;
17+
protected $compositionProcessor;
1818
/** @var CompositionPropertyDecorator[] */
1919
protected $composedProperties;
2020

2121
/**
2222
* @return string
2323
*/
24-
public function getComposedProcessor(): string
24+
public function getCompositionProcessor(): string
2525
{
26-
return $this->composedProcessor;
26+
return $this->compositionProcessor;
2727
}
2828

2929
/**

‎src/Model/Validator/ComposedPropertyValidator.php

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,24 +21,24 @@ class ComposedPropertyValidator extends AbstractComposedPropertyValidator
2121
*
2222
* @param PropertyInterface $property
2323
* @param CompositionPropertyDecorator[] $composedProperties
24-
* @param string $composedProcessor
24+
* @param string $compositionProcessor
2525
* @param array $validatorVariables
2626
*/
2727
public function __construct(
2828
PropertyInterface $property,
2929
array $composedProperties,
30-
string $composedProcessor,
30+
string $compositionProcessor,
3131
array $validatorVariables
3232
) {
3333
parent::__construct(
3434
$property,
3535
DIRECTORY_SEPARATOR . 'Validator' . DIRECTORY_SEPARATOR . 'ComposedItem.phptpl',
3636
$validatorVariables,
37-
$this->getExceptionByProcessor($composedProcessor),
37+
$this->getExceptionByProcessor($compositionProcessor),
3838
['&$succeededCompositionElements', '&$compositionErrorCollection']
3939
);
4040

41-
$this->composedProcessor = $composedProcessor;
41+
$this->compositionProcessor = $compositionProcessor;
4242
$this->composedProperties = $composedProperties;
4343
}
4444

@@ -77,11 +77,11 @@ public function withoutNestedCompositionValidation(): self
7777
/**
7878
* Parse the composition type (allOf, anyOf, ...) from the given processor and get the corresponding exception class
7979
*
80-
* @param string $composedProcessor
80+
* @param string $compositionProcessor
8181
*
8282
* @return string
8383
*/
84-
private function getExceptionByProcessor(string $composedProcessor): string
84+
private function getExceptionByProcessor(string $compositionProcessor): string
8585
{
8686
return str_replace(
8787
DIRECTORY_SEPARATOR,
@@ -90,7 +90,7 @@ private function getExceptionByProcessor(string $composedProcessor): string
9090
) . '\\' . str_replace(
9191
'Processor',
9292
'',
93-
substr($composedProcessor, strrpos($composedProcessor, '\\') + 1)
93+
substr($compositionProcessor, strrpos($compositionProcessor, '\\') + 1)
9494
) . 'Exception';
9595
}
9696
}

‎src/Model/Validator/ConditionalPropertyValidator.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public function __construct(
3535
ConditionalException::class
3636
);
3737

38-
$this->composedProcessor = IfProcessor::class;
38+
$this->compositionProcessor = IfProcessor::class;
3939
$this->composedProperties = $composedProperties;
4040
}
4141
}

‎src/PropertyProcessor/Property/BaseProcessor.php

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
use PHPModelGenerator\Model\Validator\PropertyNamesValidator;
2424
use PHPModelGenerator\Model\Validator\PropertyTemplateValidator;
2525
use PHPModelGenerator\Model\Validator\PropertyValidator;
26+
use PHPModelGenerator\PropertyProcessor\ComposedValue\AllOfProcessor;
2627
use PHPModelGenerator\PropertyProcessor\ComposedValue\ComposedPropertiesInterface;
2728
use PHPModelGenerator\PropertyProcessor\PropertyMetaDataCollection;
2829
use PHPModelGenerator\PropertyProcessor\PropertyFactory;
@@ -266,7 +267,7 @@ protected function transferComposedPropertiesToSchema(PropertyInterface $propert
266267
: $validator
267268
);
268269

269-
if (!is_a($validator->getComposedProcessor(), ComposedPropertiesInterface::class, true)) {
270+
if (!is_a($validator->getCompositionProcessor(), ComposedPropertiesInterface::class, true)) {
270271
continue;
271272
}
272273

@@ -283,16 +284,44 @@ protected function transferComposedPropertiesToSchema(PropertyInterface $propert
283284

284285
foreach ($composedProperty->getNestedSchema()->getProperties() as $property) {
285286
$this->schema->addProperty(
286-
(clone $property)
287-
->setRequired(false)
288-
->filterValidators(function (Validator $validator): bool {
289-
return is_a($validator->getValidator(), PropertyTemplateValidator::class);
290-
})
287+
$this->cloneTransferredProperty($property, $validator->getCompositionProcessor())
291288
);
292289

293290
$composedProperty->appendAffectedObjectProperty($property);
294291
}
295292
}
296293
}
297294
}
295+
296+
/**
297+
* Clone the provided property to transfer it to a schema. Sets the nullability and required flag based on the
298+
* composition processor used to set up the composition
299+
*
300+
* @param PropertyInterface $property
301+
* @param string $compositionProcessor
302+
*
303+
* @return PropertyInterface
304+
*/
305+
private function cloneTransferredProperty(
306+
PropertyInterface $property,
307+
string $compositionProcessor
308+
): PropertyInterface {
309+
$transferredProperty = (clone $property)
310+
->filterValidators(function (Validator $validator): bool {
311+
return is_a($validator->getValidator(), PropertyTemplateValidator::class);
312+
});
313+
314+
if (!is_a($compositionProcessor, AllOfProcessor::class, true)) {
315+
$transferredProperty->setRequired(false);
316+
317+
if ($transferredProperty->getType()) {
318+
$transferredProperty->setType(
319+
new PropertyType($transferredProperty->getType()->getName(), true),
320+
new PropertyType($transferredProperty->getType(true)->getName(), true)
321+
);
322+
}
323+
}
324+
325+
return $transferredProperty;
326+
}
298327
}

‎tests/ComposedValue/ComposedAllOfTest.php

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,47 @@ public function testNotProvidedObjectLevelAllOfNotMatchingAnyOptionThrowsAnExcep
8282
new $className([]);
8383
}
8484

85+
/**
86+
* @dataProvider implicitNullDataProvider
87+
*
88+
* @param bool $implicitNull
89+
*/
90+
public function testCompositionTypes(bool $implicitNull): void
91+
{
92+
$className = $this->generateClassFromFile(
93+
'ObjectLevelCompositionTypeCheck.json',
94+
(new GeneratorConfiguration())->setImmutable(false),
95+
false,
96+
$implicitNull
97+
);
98+
99+
$this->assertSame('int|null', $this->getPropertyTypeAnnotation($className, 'age'));
100+
$this->assertSame('string', $this->getPropertyTypeAnnotation($className, 'name'));
101+
102+
$this->assertSame(
103+
$implicitNull ? 'int|null' : 'int',
104+
$this->getMethodParameterTypeAnnotation($className, 'setAge')
105+
);
106+
$setAgeParamType = $this->getParameterType($className, 'setAge');
107+
$this->assertSame('int', $setAgeParamType->getName());
108+
$this->assertSame($implicitNull, $setAgeParamType->allowsNull());
109+
110+
$this->assertSame('string', $this->getMethodParameterTypeAnnotation($className, 'setName'));
111+
$setNameParamType = $this->getParameterType($className, 'setName');
112+
$this->assertSame('string', $setNameParamType->getName());
113+
$this->assertFalse($setNameParamType->allowsNull());
114+
115+
$this->assertSame('int|null', $this->getMethodReturnTypeAnnotation($className, 'getAge'));
116+
$getAgeReturnType = $this->getReturnType($className, 'getAge');
117+
$this->assertSame('int', $getAgeReturnType->getName());
118+
$this->assertTrue($getAgeReturnType->allowsNull());
119+
120+
$this->assertSame('string', $this->getMethodReturnTypeAnnotation($className, 'getName'));
121+
$getNameReturnType = $this->getReturnType($className, 'getName');
122+
$this->assertSame('string', $getNameReturnType->getName());
123+
$this->assertFalse($getNameReturnType->allowsNull());
124+
}
125+
85126
/**
86127
* Throws an exception as it's not valid against any of the given schemas
87128
*/
@@ -489,14 +530,14 @@ public function testValidationInSetterMethods(
489530

490531
// test an invalid change (only one property valid)
491532
try {
492-
$object->setIntegerProperty(null);
533+
$object->setIntegerProperty(-1);
493534
$this->fail('Exception not thrown');
494535
} catch (ErrorRegistryException | AllOfException $exception) {
495536
$this->assertStringContainsString($exceptionMessageIntegerPropertyInvalid, $exception->getMessage());
496537
}
497538

498539
try {
499-
$object->setStringProperty(null);
540+
$object->setStringProperty('');
500541
$this->fail('Exception not thrown');
501542
} catch (ErrorRegistryException | AllOfException $exception) {
502543
$this->assertStringContainsString($exceptionMessageStringPropertyInvalid, $exception->getMessage());
@@ -526,14 +567,14 @@ public function validationInSetterDataProvider(): array
526567
Requires to match all composition elements but matched 1 elements.
527568
- Composition element #1: Valid
528569
- Composition element #2: Failed
529-
* Invalid type for integerProperty. Requires int, got NULL
570+
* Value for integerProperty must not be smaller than 1
530571
ERROR
531572
,
532573
<<<ERROR
533574
declined by composition constraint.
534575
Requires to match all composition elements but matched 1 elements.
535576
- Composition element #1: Failed
536-
* Invalid type for stringProperty. Requires string, got NULL
577+
* Value for stringProperty must not be shorter than 2
537578
- Composition element #2: Valid
538579
ERROR
539580
],

‎tests/ComposedValue/ComposedAnyOfTest.php

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,44 @@ public function validEmptyAnyOfDataProvider(): array
5454
];
5555
}
5656

57+
/**
58+
* @dataProvider implicitNullDataProvider
59+
*
60+
* @param bool $implicitNull
61+
*/
62+
public function testCompositionTypes(bool $implicitNull): void
63+
{
64+
$className = $this->generateClassFromFile(
65+
'ObjectLevelCompositionTypeCheck.json',
66+
(new GeneratorConfiguration())->setImmutable(false),
67+
false,
68+
$implicitNull
69+
);
70+
71+
$this->assertSame('int|null', $this->getPropertyTypeAnnotation($className, 'age'));
72+
$this->assertSame('string|null', $this->getPropertyTypeAnnotation($className, 'name'));
73+
74+
$this->assertSame('int|null', $this->getMethodParameterTypeAnnotation($className, 'setAge'));
75+
$setAgeParamType = $this->getParameterType($className, 'setAge');
76+
$this->assertSame('int', $setAgeParamType->getName());
77+
$this->assertTrue($setAgeParamType->allowsNull());
78+
79+
$this->assertSame('string|null', $this->getMethodParameterTypeAnnotation($className, 'setName'));
80+
$setNameParamType = $this->getParameterType($className, 'setName');
81+
$this->assertSame('string', $setNameParamType->getName());
82+
$this->assertTrue($setNameParamType->allowsNull());
83+
84+
$this->assertSame('int|null', $this->getMethodReturnTypeAnnotation($className, 'getAge'));
85+
$getAgeReturnType = $this->getReturnType($className, 'getAge');
86+
$this->assertSame('int', $getAgeReturnType->getName());
87+
$this->assertTrue($getAgeReturnType->allowsNull());
88+
89+
$this->assertSame('string|null', $this->getMethodReturnTypeAnnotation($className, 'getName'));
90+
$getNameReturnType = $this->getReturnType($className, 'getName');
91+
$this->assertSame('string', $getNameReturnType->getName());
92+
$this->assertTrue($getNameReturnType->allowsNull());
93+
}
94+
5795
/**
5896
* @dataProvider propertyLevelAnyOfSchemaFileDataProvider
5997
*

‎tests/ComposedValue/ComposedOneOfTest.php

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,44 @@ public function validEmptyOneOfDataProvider(): array
5454
];
5555
}
5656

57+
/**
58+
* @dataProvider implicitNullDataProvider
59+
*
60+
* @param bool $implicitNull
61+
*/
62+
public function testCompositionTypes(bool $implicitNull): void
63+
{
64+
$className = $this->generateClassFromFile(
65+
'ObjectLevelCompositionTypeCheck.json',
66+
(new GeneratorConfiguration())->setImmutable(false),
67+
false,
68+
$implicitNull
69+
);
70+
71+
$this->assertSame('int|null', $this->getPropertyTypeAnnotation($className, 'age'));
72+
$this->assertSame('string|null', $this->getPropertyTypeAnnotation($className, 'name'));
73+
74+
$this->assertSame('int|null', $this->getMethodParameterTypeAnnotation($className, 'setAge'));
75+
$setAgeParamType = $this->getParameterType($className, 'setAge');
76+
$this->assertSame('int', $setAgeParamType->getName());
77+
$this->assertTrue($setAgeParamType->allowsNull());
78+
79+
$this->assertSame('string|null', $this->getMethodParameterTypeAnnotation($className, 'setName'));
80+
$setNameParamType = $this->getParameterType($className, 'setName');
81+
$this->assertSame('string', $setNameParamType->getName());
82+
$this->assertTrue($setNameParamType->allowsNull());
83+
84+
$this->assertSame('int|null', $this->getMethodReturnTypeAnnotation($className, 'getAge'));
85+
$getAgeReturnType = $this->getReturnType($className, 'getAge');
86+
$this->assertSame('int', $getAgeReturnType->getName());
87+
$this->assertTrue($getAgeReturnType->allowsNull());
88+
89+
$this->assertSame('string|null', $this->getMethodReturnTypeAnnotation($className, 'getName'));
90+
$getNameReturnType = $this->getReturnType($className, 'getName');
91+
$this->assertSame('string', $getNameReturnType->getName());
92+
$this->assertTrue($getNameReturnType->allowsNull());
93+
}
94+
5795
/**
5896
* @dataProvider propertyLevelOneOfSchemaFileDataProvider
5997
*

‎tests/Schema/ComposedAllOfTest/ObjectLevelCompositionRequired.json

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
"type": "object",
55
"properties": {
66
"stringProperty": {
7-
"type": "string"
7+
"type": "string",
8+
"minLength": 2
89
}
910
},
1011
"required": ["stringProperty"]
@@ -13,7 +14,8 @@
1314
"type": "object",
1415
"properties": {
1516
"integerProperty": {
16-
"type": "integer"
17+
"type": "integer",
18+
"minimum": 1
1719
}
1820
},
1921
"required": ["integerProperty"]

‎tests/Schema/ComposedAllOfTest/ObjectLevelCompositionTypeCheck.json

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{
2+
"allOf": [
3+
{
4+
"type": "object",
5+
"properties": {
6+
"age": {
7+
"type": "integer"
8+
}
9+
}
10+
},
11+
{
12+
"type": "object",
13+
"properties": {
14+
"name": {
15+
"type": "string"
16+
}
17+
},
18+
"required": [
19+
"name"
20+
]
21+
}
22+
]
23+
}

‎tests/Schema/ComposedAnyOfTest/ObjectLevelCompositionTypeCheck.json

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{
2+
"anyOf": [
3+
{
4+
"type": "object",
5+
"properties": {
6+
"age": {
7+
"type": "integer"
8+
}
9+
}
10+
},
11+
{
12+
"type": "object",
13+
"properties": {
14+
"name": {
15+
"type": "string"
16+
}
17+
},
18+
"required": [
19+
"name"
20+
]
21+
}
22+
]
23+
}

0 commit comments

Comments
 (0)