Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/JsonSchema/ConstraintError.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ class ConstraintError extends Enum
public const PROPERTY_NAMES = 'propertyNames';
public const TYPE = 'type';
public const UNIQUE_ITEMS = 'uniqueItems';
public const UNEVALUATED_PROPERTIES = 'unevaluatedProperties';
public const CONTENT_MEDIA_TYPE = 'contentMediaType';
public const CONTENT_ENCODING = 'contentEncoding';

Expand Down Expand Up @@ -122,6 +123,7 @@ public function getMessage()
self::PROPERTY_NAMES => 'Property name %s is invalid',
self::TYPE => '%s value found, but %s is required',
self::UNIQUE_ITEMS => 'There are no duplicates allowed in the array',
self::UNEVALUATED_PROPERTIES => 'The property %s is not evaluated and the definition does not allow unevaluated properties',
self::CONTENT_MEDIA_TYPE => 'Value is not valid with content media type',
self::CONTENT_ENCODING => 'Value is not valid with content encoding',
];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public function check(&$value, $schema = null, ?JsonPointer $path = null, $i = n
$this->checkForKeyword('anyOf', $value, $schema, $path, $i);
$this->checkForKeyword('oneOf', $value, $schema, $path, $i);
$this->checkForKeyword('ifThenElse', $value, $schema, $path, $i);
$this->checkForKeyword('unevaluatedProperties', $value, $schema, $path, $i);

$this->checkForKeyword('additionalProperties', $value, $schema, $path, $i);
$this->checkForKeyword('items', $value, $schema, $path, $i);
Expand Down
1 change: 1 addition & 0 deletions src/JsonSchema/Constraints/Drafts/Draft2019/Factory.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class Factory extends \JsonSchema\Constraints\Factory
protected $constraintMap = [
'schema' => Draft2019Constraint::class,
'additionalProperties' => AdditionalPropertiesConstraint::class,
'unevaluatedProperties' => UnevaluatedPropertiesConstraint::class,
'additionalItems' => AdditionalItemsConstraint::class,
'dependentSchemas' => DependentSchemasConstraint::class,
'dependentRequired' => DependentRequiredConstraint::class,
Expand Down

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: With this keyword now supported we should alos enable the test in

'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties schema: with invalid unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties false: with unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties with adjacent properties: with unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties with adjacent patternProperties: with unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties with nested properties: with additional properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties with nested patternProperties: with additional properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties with anyOf: when one matches and has unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties with anyOf: when two match and has unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties with oneOf: with unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties with not: with unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties with if/then/else: when if is true and has unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties with if/then/else: when if is false and has unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties with if/then/else, then not defined: when if is true and has no unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties with if/then/else, then not defined: when if is true and has unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties with if/then/else, then not defined: when if is false and has unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties with if/then/else, else not defined: when if is true and has unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties with if/then/else, else not defined: when if is false and has no unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties with if/then/else, else not defined: when if is false and has unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties with dependentSchemas: with unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties with boolean schemas: with unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties with $ref: with unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties can\'t see inside cousins: always fails is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties can\'t see inside cousins (reverse order): always fails is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: nested unevaluatedProperties, outer true, inner false, properties outside: with no nested unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: nested unevaluatedProperties, outer true, inner false, properties outside: with nested unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: nested unevaluatedProperties, outer true, inner false, properties inside: with nested unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: cousin unevaluatedProperties, true and false, true with properties: with no nested unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: cousin unevaluatedProperties, true and false, true with properties: with nested unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: cousin unevaluatedProperties, true and false, false with properties: with nested unevaluated properties is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: property is evaluated in an uncle schema to unevaluatedProperties: uncle keyword evaluation is not significant is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: in-place applicator siblings, allOf has unevaluated: base case: both properties present is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: in-place applicator siblings, allOf has unevaluated: in place applicator siblings, foo is missing is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: in-place applicator siblings, anyOf has unevaluated: base case: both properties present is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: in-place applicator siblings, anyOf has unevaluated: in place applicator siblings, bar is missing is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties + single cyclic ref: Unevaluated on 1st level is invalid is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties + single cyclic ref: Unevaluated on 2nd level is invalid is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties + single cyclic ref: Unevaluated on 3rd level is invalid is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: dynamic evalation inside nested refs: xx + foo is invalid is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties not affected by propertyNames: string property is invalid is expected to be invalid',
'[draft2019-09/unevaluatedProperties.json]: unevaluatedProperties can see annotations from if without then and else: invalid in case if is evaluated is expected to be invalid',

Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
<?php

declare(strict_types=1);

namespace JsonSchema\Constraints\Drafts\Draft2019;

use JsonSchema\ConstraintError;
use JsonSchema\Constraints\ConstraintInterface;
use JsonSchema\Entity\ErrorBagProxy;
use JsonSchema\Entity\JsonPointer;

/**
* Proof-of-concept support for unevaluatedProperties.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Before merging we should remove the proof on concept comment.

Suggested change
* Proof-of-concept support for unevaluatedProperties.

*
* The current validator does not carry annotations between applicators, so this
* first implementation derives the evaluated property names from properties,
* patternProperties, and allOf branches in the current schema.
Comment on lines +15 to +17

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: The comment doesn't need to inform about this being the first implementation

Suggested change
* The current validator does not carry annotations between applicators, so this
* first implementation derives the evaluated property names from properties,
* patternProperties, and allOf branches in the current schema.
* The current validator does not carry annotations between applicators, so this
* constraint derives the evaluated property names from properties,
* patternProperties, and allOf branches in the current schema.

*/
class UnevaluatedPropertiesConstraint implements ConstraintInterface
{
use ErrorBagProxy;

/** @var Factory */
private $factory;

public function __construct(?Factory $factory = null)
{
$this->factory = $factory ?: new Factory();
$this->initialiseErrorBag($this->factory);
}

public function check(&$value, $schema = null, ?JsonPointer $path = null, $i = null): void
{
if (!is_object($schema) || !property_exists($schema, 'unevaluatedProperties') || !is_object($value)) {
return;
}

if ($schema->unevaluatedProperties === true) {
return;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: This should also evaluate the availability and value of additionalProperties, see the tests in the test suite and the spec.

$evaluated = $this->collectEvaluatedProperties($schema, $value);
$unevaluated = array_diff_key(get_object_vars($value), array_flip($evaluated));
if (!$unevaluated) {
return;
}

$basePath = $path ?? new JsonPointer('');
foreach ($unevaluated as $propertyName => $propertyValue) {
$propertyPath = $basePath->withPropertyPaths(array_merge($basePath->getPropertyPaths(), [$propertyName]));

if (is_object($schema->unevaluatedProperties)) {
$propertyConstraint = $this->factory->createInstanceFor('schema');
$propertyConstraint->check($propertyValue, $schema->unevaluatedProperties, $propertyPath, $i);
if ($propertyConstraint->isValid()) {
continue;
}

$this->addErrors($propertyConstraint->getErrors());
continue;
}

$this->addError(ConstraintError::UNEVALUATED_PROPERTIES(), $propertyPath, ['found' => $propertyName]);
}
}

/**
* @param object $schema
* @param object $value
*
* @return array<int, string>
*/
private function collectEvaluatedProperties($schema, object $value): array
{
if (!is_object($schema)) {
return [];
}

Comment on lines +67 to +78

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Since $object was already verified as being of type object on line 43 we can safely narrow the type of the function to object and skip the check as well as the doc types since we use the native typing. This requires a additional check on line 91 where we do a recursive call.

Suggested change
/**
* @param object $schema
* @param object $value
*
* @return array<int, string>
*/
private function collectEvaluatedProperties($schema, object $value): array
{
if (!is_object($schema)) {
return [];
}
/**
* @return array<int, string>
*/
private function collectEvaluatedProperties(object $schema, object $value): array
{

$evaluated = [];

if (isset($schema->properties) && is_object($schema->properties)) {
$evaluated = array_merge($evaluated, array_keys(get_object_vars($schema->properties)));
}

if (isset($schema->patternProperties) && is_object($schema->patternProperties)) {
foreach (get_object_vars($value) as $propertyName => $_) {
foreach (array_keys(get_object_vars($schema->patternProperties)) as $pattern) {
if (preg_match($this->createPregMatchPattern($pattern), (string) $propertyName)) {
$evaluated[] = $propertyName;
break;
}
}
}
}

if (isset($schema->allOf) && is_array($schema->allOf)) {
foreach ($schema->allOf as $branch) {
$evaluated = array_merge($evaluated, $this->collectEvaluatedProperties($branch, $value));
}
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: This should include (either now or at a later state) anyOf, oneOf, if/then/else and more keywords right? Curious to learn what would be your suggested approach?


return array_values(array_unique($evaluated));
}

private function createPregMatchPattern(string $pattern): string
{
$pattern = str_replace('\\p{digit}', '\\p{Nd}', $pattern);
$pattern = str_replace('\\p{Letter}', '\\p{L}', $pattern);

return '/' . str_replace('/', '\\/', $pattern) . '/u';
}
}
47 changes: 47 additions & 0 deletions tests/Constraints/UnevaluatedPropertiesTest.php

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: I think this file would belong in test/Constraints/Drafts/Draft2019/UnevaluatedPropertiesConstraintTest.php

Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php

declare(strict_types=1);

namespace JsonSchema\Tests\Constraints;

use JsonSchema\DraftIdentifiers;
use JsonSchema\Constraints\Constraint;

class UnevaluatedPropertiesTest extends BaseTestCase
{
protected $schemaSpec = DraftIdentifiers::DRAFT_2019_09;

public function getInvalidTests(): \Generator
{
yield [
'{"hello":"world","world":"hello","unexpected":true}',
'{
"$schema":"' . DraftIdentifiers::DRAFT_2019_09 . '",
"type":"object",
"unevaluatedProperties":false,
"allOf":[
{"properties":{"hello":{"type":"string"}},"required":["hello"]},
{"properties":{"world":{"type":"string"}},"required":["world"]}
]
}',
Constraint::CHECK_MODE_STRICT,
];
}

public function getValidTests(): \Generator
{
yield [
'{"hello":"world","world":"hello"}',
'{
"$schema":"' . DraftIdentifiers::DRAFT_2019_09 . '",
"type":"object",
"unevaluatedProperties":false,
"allOf":[
{"properties":{"hello":{"type":"string"}},"required":["hello"]},
{"properties":{"world":{"type":"string"}},"required":["world"]}
]
}',
Constraint::CHECK_MODE_STRICT,
];
}
}