Skip to content

Commit

Permalink
Add property metadata to schema definition property cache to ensure g…
Browse files Browse the repository at this point in the history
…enerating separate properties if the metadata differs to provide distinct validators (fixes #86).
  • Loading branch information
wol-soft committed Nov 18, 2024
1 parent 789fe36 commit 3ed4c4b
Show file tree
Hide file tree
Showing 5 changed files with 173 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/Model/SchemaDefinition/SchemaDefinition.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ public function resolveReference(
$jsonSchema = $jsonSchema[$segment];
}

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

if (!$this->resolvedPaths->offsetExists($key)) {
// create a dummy entry for the path first. If the path is used recursive the recursive usages will point
Expand Down
8 changes: 8 additions & 0 deletions src/PropertyProcessor/PropertyMetaDataCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,12 @@ public function getAttributeDependencies(string $attribute): ?array
{
return $this->dependencies[$attribute] ?? null;
}

public function getHash(string $attribute): string
{
return md5(json_encode([
$this->getAttributeDependencies($attribute),
$this->isAttributeRequired($attribute),
]));
}
}
105 changes: 105 additions & 0 deletions tests/Issues/Issue/Issue86Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
<?php

declare(strict_types=1);

namespace PHPModelGenerator\Tests\Issues\Issue;

use PHPModelGenerator\Exception\Dependency\InvalidSchemaDependencyException;
use PHPModelGenerator\Exception\Generic\InvalidTypeException;
use PHPModelGenerator\Exception\Object\RequiredValueException;
use PHPModelGenerator\Tests\Issues\AbstractIssueTestCase;

class Issue86Test extends AbstractIssueTestCase
{
/**
* @dataProvider validRefDataProvider
*/
public function testDifferentRequiredConfigurationForReferencedObject(array $input, bool $implicitNull): void
{
$className = $this->generateClassFromFile('ref.json', implicitNull: $implicitNull);

$object = new $className($input);
$this->assertSame($input['required'], $object->getRequired());
$this->assertSame($input['optional'] ?? null, $object->getOptional());
}

public function validRefDataProvider(): array
{
return [
'both properties provided' => [['required' => 'Hello', 'optional' => 'World'], true],
'optional property not provided' => [['required' => 'Hello'], false],
'optional property null' => [['required' => 'Hello', 'optional' => null], true],
];
}
/**
* @dataProvider invalidRefDataProvider
*/
public function testDifferentRequiredConfigurationForReferencedObjectWithInvalidInput(
array $input,
bool $implicitNull,
string $expectedException,
): void {
$className = $this->generateClassFromFile('ref.json', implicitNull: $implicitNull);

$this->expectException($expectedException);

new $className($input);
}

public function invalidRefDataProvider(): array
{
return [
'required property not provided' => [['optional' => 'World'], true, RequiredValueException::class],
'optional property null - implicit null disabled' => [['required' => 'Hello', 'optional' => null], false, InvalidTypeException::class],
'required property wrong type' => [['required' => 1, 'optional' => 'World'], true, InvalidTypeException::class],
'optional property wrong type' => [['required' => 'Hello', 'optional' => 1], true, InvalidTypeException::class],
];
}

/**
* @dataProvider validSchemaDependencyDataProvider
*/
public function testDifferentSchemaDependenciesForReferencedObject(array $input): void
{
$className = $this->generateClassFromFile('schemaDependency.json');

$object = new $className($input);
$this->assertSame($input['property1'] ?? null, $object->getProperty1());
$this->assertSame($input['property2'] ?? null, $object->getProperty2());
$this->assertSame($input['property3'] ?? null, $object->getProperty3());
}

public function validSchemaDependencyDataProvider(): array
{
return [
'No properties provided' => [[]],
'Property 3 without dependency' => [['property3' => true]],
'Dependency Property 1' => [['property1' => 'Hello', 'property3' => 'World']],
'Dependency Property 2' => [['property2' => 'Hello', 'property3' => 1]],
];
}

/**
* @dataProvider invalidSchemaDependencyDataProvider
*/
public function testDifferentSchemaDependenciesForReferencedObjectWithInvalidInput(array $input): void
{
$className = $this->generateClassFromFile('schemaDependency.json');

$this->expectException(InvalidSchemaDependencyException::class);

new $className($input);
}

public function invalidSchemaDependencyDataProvider(): array
{
return [
'Dependency missing property 1' => [['property1' => 'Hello']],
'Dependency missing property 2' => [['property2' => 'Hello']],
'Dependency Property 1 invalid type' => [['property1' => 'Hello', 'property3' => 1]],
'Dependency Property 2 invalid type' => [['property2' => 'Hello', 'property3' => 'World']],
'Both dependencies required 1' => [['property1' => 'Hello', 'property2' => 'Hello', 'property3' => 'World']],
'Both dependencies required 2' => [['property1' => 'Hello', 'property2' => 'Hello', 'property3' => 1]],
];
}
}
19 changes: 19 additions & 0 deletions tests/Schema/Issues/86/ref.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"definitions": {
"stringProperty": {
"type": "string"
}
},
"type": "object",
"properties": {
"required": {
"$ref": "#/definitions/stringProperty"
},
"optional": {
"$ref": "#/definitions/stringProperty"
}
},
"required": [
"required"
]
}
38 changes: 38 additions & 0 deletions tests/Schema/Issues/86/schemaDependency.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
{
"definitions": {
"stringProperty": {
"type": "string"
}
},
"type": "object",
"properties": {
"property1": {
"$ref": "#/definitions/stringProperty"
},
"property2": {
"$ref": "#/definitions/stringProperty"
}
},
"dependencies": {
"property1": {
"properties": {
"property3": {
"type": "string"
}
},
"required": [
"property3"
]
},
"property2": {
"properties": {
"property3": {
"type": "integer"
}
},
"required": [
"property3"
]
}
}
}

0 comments on commit 3ed4c4b

Please sign in to comment.