Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for "const" #507

Merged
merged 7 commits into from
May 23, 2018
Merged
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 @@ -14,6 +14,7 @@ class ConstraintError extends Enum
const DISALLOW = 'disallow';
const DIVISIBLE_BY = 'divisibleBy';
const ENUM = 'enum';
const CONSTANT = 'const';
const EXCLUSIVE_MINIMUM = 'exclusiveMinimum';
const EXCLUSIVE_MAXIMUM = 'exclusiveMaximum';
const FORMAT_COLOR = 'colorFormat';
Expand Down Expand Up @@ -63,6 +64,7 @@ public function getMessage()
self::DISALLOW => 'Disallowed value was matched',
self::DIVISIBLE_BY => 'Is not divisible by %d',
self::ENUM => 'Does not have a value in the enumeration %s',
self::CONSTANT => 'Does not have a value equal to %s',
self::EXCLUSIVE_MINIMUM => 'Must have a minimum value greater than %d',
self::EXCLUSIVE_MAXIMUM => 'Must have a maximum value less than %d',
self::FORMAT_COLOR => 'Invalid color',
Expand Down
54 changes: 54 additions & 0 deletions src/JsonSchema/Constraints/ConstConstraint.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?php

/*
* This file is part of the JsonSchema package.
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace JsonSchema\Constraints;

use JsonSchema\ConstraintError;
use JsonSchema\Entity\JsonPointer;

/**
* The ConstConstraint Constraints, validates an element against a constant value
*
* @author Martin Helmich <[email protected]>
*/
class ConstConstraint extends Constraint
{
/**
* {@inheritdoc}
*/
public function check(&$element, $schema = null, JsonPointer $path = null, $i = null)
{
// Only validate const if the attribute exists
if ($element instanceof UndefinedConstraint && (!isset($schema->required) || !$schema->required)) {
return;
}
$const = $schema->const;

$type = gettype($element);
$constType = gettype($const);

if ($this->factory->getConfig(self::CHECK_MODE_TYPE_CAST) && $type == 'array' && $constType == 'object') {
if ((object) $element == $const) {
return;
}
}

if ($type === gettype($const)) {
if ($type == 'object') {
if ($element == $const) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This does not correctly implement deep comparison of objects - it results in members being compared using ==.

Could you please change this implementation to ensure type-strict comparison of object properties. You will probably need to iterate them; PHP lacks a suitable native comparison operator that can do it all in one go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Following that argumentation, the enum implementation (which was my inspiration for this class) should probably be changed as well -- it uses the same comparison.

Copy link
Contributor

Choose a reason for hiding this comment

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

If enum is similarly broken, then yes - I agree with you that it needs fixing.

return;
}
} elseif ($element === $const) {
return;
}
}

$this->addError(ConstraintError::CONSTANT(), $path, array('const' => $schema->const));
}
}
16 changes: 16 additions & 0 deletions src/JsonSchema/Constraints/Constraint.php
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,22 @@ protected function checkEnum($value, $schema = null, JsonPointer $path = null, $
$this->addErrors($validator->getErrors());
}

/**
* Checks a const element
*
* @param mixed $value
* @param mixed $schema
* @param JsonPointer|null $path
* @param mixed $i
*/
protected function checkConst($value, $schema = null, JsonPointer $path = null, $i = null)
{
$validator = $this->factory->createInstanceFor('const');
$validator->check($value, $schema, $path, $i);

$this->addErrors($validator->getErrors());
}

/**
* Checks format of an element
*
Expand Down
1 change: 1 addition & 0 deletions src/JsonSchema/Constraints/Factory.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class Factory
'string' => 'JsonSchema\Constraints\StringConstraint',
'number' => 'JsonSchema\Constraints\NumberConstraint',
'enum' => 'JsonSchema\Constraints\EnumConstraint',
'const' => 'JsonSchema\Constraints\ConstConstraint',
'format' => 'JsonSchema\Constraints\FormatConstraint',
'schema' => 'JsonSchema\Constraints\SchemaConstraint',
'validator' => 'JsonSchema\Validator'
Expand Down
5 changes: 5 additions & 0 deletions src/JsonSchema/Constraints/UndefinedConstraint.php
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ public function validateTypes(&$value, $schema = null, JsonPointer $path, $i = n
if (isset($schema->enum)) {
$this->checkEnum($value, $schema, $path, $i);
}

// check const
if (isset($schema->const)) {
$this->checkConst($value, $schema, $path, $i);
}
}

/**
Expand Down
43 changes: 42 additions & 1 deletion tests/Constraints/CoerciveTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,48 @@ public function dataCoerceCases()
'string', 'integer', 42, true
);

$tests = array();
// #46 check coercion with "const"
$tests[] = array(
'{"properties":{"propertyOne":{"type":"string","const":"42"}}}',
'{"propertyOne":42}',
'integer', 'string', '42', true
);

// #47 check coercion with "const"
$tests[] = array(
'{"properties":{"propertyOne":{"type":"number","const":42}}}',
'{"propertyOne":"42"}',
'string', 'integer', 42, true
);

// #48 check boolean coercion with "const"
$tests[] = array(
'{"properties":{"propertyOne":{"type":"boolean","const":false}}}',
'{"propertyOne":"false"}',
'string', 'boolean', false, true
);

// #49 check boolean coercion with "const"
$tests[] = array(
'{"properties":{"propertyOne":{"type":"boolean","const":true}}}',
'{"propertyOne":"true"}',
'string', 'boolean', true, true
);

// #50 check boolean coercion with "const"
$tests[] = array(
Copy link
Collaborator

@shmax shmax Apr 17, 2018

Choose a reason for hiding this comment

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

I was hoping to see a false...

("true" == true) // true
("false" == true) // true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here you go...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Beautiful, thank you

'{"properties":{"propertyOne":{"type":"boolean","const":true}}}',
'{"propertyOne":1}',
'integer', 'boolean', true, true
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please also add some deep object tests. With and without mismatched property types.


// #51 check boolean coercion with "const"
$tests[] = array(
'{"properties":{"propertyOne":{"type":"boolean","const":false}}}',
'{"propertyOne":"false"}',
'string', 'boolean', false, true
);

foreach ($types as $toType => $testCases) {
foreach ($testCases as $testCase) {
$tests[] = array(
Expand Down
98 changes: 98 additions & 0 deletions tests/Constraints/ConstTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
<?php

/*
* This file is part of the JsonSchema package.
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace JsonSchema\Tests\Constraints;

class ConstTest extends BaseTestCase
{
protected $schemaSpec = 'http://json-schema.org/draft-06/schema#';
protected $validateSchema = true;

public function getInvalidTests()
{
return array(
array(
'{"value":"foo"}',
'{
"type":"object",
"properties":{
"value":{"type":"string","const":"bar"}
},
"additionalProperties":false
}'
),
array(
'{"value":5}',
'{
"type":"object",
"properties":{
"value":{"type":"integer","const":6}
},
"additionalProperties":false
}'
),
array(
'{"value":false}',
'{
"type":"object",
"properties":{
"value":{"type":"boolean","const":true}
},
"additionalProperties":false
}'
),
);
}

public function getValidTests()
{
return array(
array(
'{"value":"bar"}',
'{
"type":"object",
"properties":{
"value":{"type":"string","const":"bar"}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm a little curious about the potential use cases covered by this implementation. Ten minutes ago I had never heard of the const keyword, but from what I've read since then its main utility comes from interacting with the $data concept proposed in v5. Given that we don't currently support $data or relative JSON pointers, how much use is there in supporting a keyword that guarantees that an input value is precisely equal to some other value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not find any mention of the $data concept in the recent spec v7. From my understanding, const is presumably nothing more than a more expressive alternative to a one-element enum:

6.1.3. const
The value of this keyword MAY be of any type, including null.

An instance validates successfully against this keyword if its value is equal to the value of the keyword.

My use case for const is when using a "constant" property as a type discriminator when using polymorphic types:

{
  "$oneOf": [
    {
      "properties": {
        "paymentType": {"type": "string", "const": "directDebit"},
        "iban": {"type": ...}
       }
    },
    {
      "properties": {
        "paymentType": {"type: "string", "const": "creditCard"},
        "cardNumber": {...}
      }
  ]
}

In this case, documents like {"payment": {"paymentType": "directDebit", "iban": "DE1234567890"}} and {"payment": {"paymentType": "creditCard", "cardNumber": "123456789"}} would match the schema, while something like {"payment": {"paymentType": "creditCard", "iban": "DE1234567890"}} would not.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see. Okay. 👍

},
"additionalProperties":false
}'
),
array(
'{"value":false}',
'{
"type":"object",
"properties":{
"value":{"type":"boolean","const":false}
},
"additionalProperties":false
}'
),
array(
'{"value":true}',
'{
"type":"object",
"properties":{
"value":{"type":"boolean","const":true}
},
"additionalProperties":false
}'
),
array(
'{"value":5}',
'{
"type":"object",
"properties":{
"value":{"type":"integer","const":5}
},
"additionalProperties":false
}'
),
);
}
}