Skip to content

Commit 3ed4c4b

Browse files
wol-softwol-soft
committed
Add property metadata to schema definition property cache to ensure generating separate properties if the metadata differs to provide distinct validators (fixes #86).
1 parent 789fe36 commit 3ed4c4b

File tree

5 files changed

+173
-1
lines changed

5 files changed

+173
-1
lines changed

‎src/Model/SchemaDefinition/SchemaDefinition.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,9 @@ public function resolveReference(
6565
$jsonSchema = $jsonSchema[$segment];
6666
}
6767

68-
$key = implode('-', $originalPath);
68+
// if the properties point to the same definition and share identical metadata the generated property can be
69+
// recycled. Otherwise, a new property must be generated as diverging metadata lead to different validators.
70+
$key = implode('-', [...$originalPath, $propertyMetaDataCollection->getHash($propertyName)]);
6971

7072
if (!$this->resolvedPaths->offsetExists($key)) {
7173
// create a dummy entry for the path first. If the path is used recursive the recursive usages will point

‎src/PropertyProcessor/PropertyMetaDataCollection.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,12 @@ public function getAttributeDependencies(string $attribute): ?array
3939
{
4040
return $this->dependencies[$attribute] ?? null;
4141
}
42+
43+
public function getHash(string $attribute): string
44+
{
45+
return md5(json_encode([
46+
$this->getAttributeDependencies($attribute),
47+
$this->isAttributeRequired($attribute),
48+
]));
49+
}
4250
}

‎tests/Issues/Issue/Issue86Test.php

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace PHPModelGenerator\Tests\Issues\Issue;
6+
7+
use PHPModelGenerator\Exception\Dependency\InvalidSchemaDependencyException;
8+
use PHPModelGenerator\Exception\Generic\InvalidTypeException;
9+
use PHPModelGenerator\Exception\Object\RequiredValueException;
10+
use PHPModelGenerator\Tests\Issues\AbstractIssueTestCase;
11+
12+
class Issue86Test extends AbstractIssueTestCase
13+
{
14+
/**
15+
* @dataProvider validRefDataProvider
16+
*/
17+
public function testDifferentRequiredConfigurationForReferencedObject(array $input, bool $implicitNull): void
18+
{
19+
$className = $this->generateClassFromFile('ref.json', implicitNull: $implicitNull);
20+
21+
$object = new $className($input);
22+
$this->assertSame($input['required'], $object->getRequired());
23+
$this->assertSame($input['optional'] ?? null, $object->getOptional());
24+
}
25+
26+
public function validRefDataProvider(): array
27+
{
28+
return [
29+
'both properties provided' => [['required' => 'Hello', 'optional' => 'World'], true],
30+
'optional property not provided' => [['required' => 'Hello'], false],
31+
'optional property null' => [['required' => 'Hello', 'optional' => null], true],
32+
];
33+
}
34+
/**
35+
* @dataProvider invalidRefDataProvider
36+
*/
37+
public function testDifferentRequiredConfigurationForReferencedObjectWithInvalidInput(
38+
array $input,
39+
bool $implicitNull,
40+
string $expectedException,
41+
): void {
42+
$className = $this->generateClassFromFile('ref.json', implicitNull: $implicitNull);
43+
44+
$this->expectException($expectedException);
45+
46+
new $className($input);
47+
}
48+
49+
public function invalidRefDataProvider(): array
50+
{
51+
return [
52+
'required property not provided' => [['optional' => 'World'], true, RequiredValueException::class],
53+
'optional property null - implicit null disabled' => [['required' => 'Hello', 'optional' => null], false, InvalidTypeException::class],
54+
'required property wrong type' => [['required' => 1, 'optional' => 'World'], true, InvalidTypeException::class],
55+
'optional property wrong type' => [['required' => 'Hello', 'optional' => 1], true, InvalidTypeException::class],
56+
];
57+
}
58+
59+
/**
60+
* @dataProvider validSchemaDependencyDataProvider
61+
*/
62+
public function testDifferentSchemaDependenciesForReferencedObject(array $input): void
63+
{
64+
$className = $this->generateClassFromFile('schemaDependency.json');
65+
66+
$object = new $className($input);
67+
$this->assertSame($input['property1'] ?? null, $object->getProperty1());
68+
$this->assertSame($input['property2'] ?? null, $object->getProperty2());
69+
$this->assertSame($input['property3'] ?? null, $object->getProperty3());
70+
}
71+
72+
public function validSchemaDependencyDataProvider(): array
73+
{
74+
return [
75+
'No properties provided' => [[]],
76+
'Property 3 without dependency' => [['property3' => true]],
77+
'Dependency Property 1' => [['property1' => 'Hello', 'property3' => 'World']],
78+
'Dependency Property 2' => [['property2' => 'Hello', 'property3' => 1]],
79+
];
80+
}
81+
82+
/**
83+
* @dataProvider invalidSchemaDependencyDataProvider
84+
*/
85+
public function testDifferentSchemaDependenciesForReferencedObjectWithInvalidInput(array $input): void
86+
{
87+
$className = $this->generateClassFromFile('schemaDependency.json');
88+
89+
$this->expectException(InvalidSchemaDependencyException::class);
90+
91+
new $className($input);
92+
}
93+
94+
public function invalidSchemaDependencyDataProvider(): array
95+
{
96+
return [
97+
'Dependency missing property 1' => [['property1' => 'Hello']],
98+
'Dependency missing property 2' => [['property2' => 'Hello']],
99+
'Dependency Property 1 invalid type' => [['property1' => 'Hello', 'property3' => 1]],
100+
'Dependency Property 2 invalid type' => [['property2' => 'Hello', 'property3' => 'World']],
101+
'Both dependencies required 1' => [['property1' => 'Hello', 'property2' => 'Hello', 'property3' => 'World']],
102+
'Both dependencies required 2' => [['property1' => 'Hello', 'property2' => 'Hello', 'property3' => 1]],
103+
];
104+
}
105+
}

‎tests/Schema/Issues/86/ref.json

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"definitions": {
3+
"stringProperty": {
4+
"type": "string"
5+
}
6+
},
7+
"type": "object",
8+
"properties": {
9+
"required": {
10+
"$ref": "#/definitions/stringProperty"
11+
},
12+
"optional": {
13+
"$ref": "#/definitions/stringProperty"
14+
}
15+
},
16+
"required": [
17+
"required"
18+
]
19+
}

‎tests/Schema/Issues/86/schemaDependency.json

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
{
2+
"definitions": {
3+
"stringProperty": {
4+
"type": "string"
5+
}
6+
},
7+
"type": "object",
8+
"properties": {
9+
"property1": {
10+
"$ref": "#/definitions/stringProperty"
11+
},
12+
"property2": {
13+
"$ref": "#/definitions/stringProperty"
14+
}
15+
},
16+
"dependencies": {
17+
"property1": {
18+
"properties": {
19+
"property3": {
20+
"type": "string"
21+
}
22+
},
23+
"required": [
24+
"property3"
25+
]
26+
},
27+
"property2": {
28+
"properties": {
29+
"property3": {
30+
"type": "integer"
31+
}
32+
},
33+
"required": [
34+
"property3"
35+
]
36+
}
37+
}
38+
}

0 commit comments

Comments
 (0)