From ba3372669380c929525ca1b130cefc260843042c Mon Sep 17 00:00:00 2001 From: Erayd Date: Mon, 13 Mar 2017 14:48:33 +1300 Subject: [PATCH 1/9] Improve performance - don't loop over everything if already valid --- src/JsonSchema/Constraints/TypeConstraint.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/JsonSchema/Constraints/TypeConstraint.php b/src/JsonSchema/Constraints/TypeConstraint.php index 096f5485..66c493c0 100644 --- a/src/JsonSchema/Constraints/TypeConstraint.php +++ b/src/JsonSchema/Constraints/TypeConstraint.php @@ -82,6 +82,11 @@ public function check(&$value = null, $schema = null, JsonPointer $path = null, protected function validateTypesArray(&$value, array $type, &$validTypesWording, &$isValid, $path) { foreach ($type as $tp) { + // already valid, so no need to waste cycles looping over everything + if ($isValid) { + return; + } + // $tp can be an object, if it's a schema instead of a simple type, validate it // with a new type constraint if (is_object($tp)) { From 7f7a2b1a98e34a60dd40b25664ffb7b739751178 Mon Sep 17 00:00:00 2001 From: Erayd Date: Mon, 13 Mar 2017 14:55:34 +1300 Subject: [PATCH 2/9] Don't coerce already-valid types (fixes #379) --- src/JsonSchema/Constraints/TypeConstraint.php | 19 ++++++++++++------- tests/Constraints/CoerciveTest.php | 3 +++ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/JsonSchema/Constraints/TypeConstraint.php b/src/JsonSchema/Constraints/TypeConstraint.php index 66c493c0..59bdd6d5 100644 --- a/src/JsonSchema/Constraints/TypeConstraint.php +++ b/src/JsonSchema/Constraints/TypeConstraint.php @@ -44,16 +44,23 @@ public function check(&$value = null, $schema = null, JsonPointer $path = null, { $type = isset($schema->type) ? $schema->type : null; $isValid = false; + $coerce = $this->factory->getConfig(self::CHECK_MODE_COERCE_TYPES); $wording = array(); if (is_array($type)) { - $this->validateTypesArray($value, $type, $wording, $isValid, $path); + $this->validateTypesArray($value, $type, $wording, $isValid, $path, false); + if (!$isValid && $coerce) { + $this->validateTypesArray($value, $type, $wording, $isValid, $path, true); + } } elseif (is_object($type)) { $this->checkUndefined($value, $type, $path); return; } else { - $isValid = $this->validateType($value, $type); + $isValid = $this->validateType($value, $type, false); + if (!$isValid && $coerce) { + $isValid = $this->validateType($value, $type, true); + } } if ($isValid === false) { @@ -79,7 +86,7 @@ public function check(&$value = null, $schema = null, JsonPointer $path = null, * @param bool $isValid The current validation value * @param $path */ - protected function validateTypesArray(&$value, array $type, &$validTypesWording, &$isValid, $path) + protected function validateTypesArray(&$value, array $type, &$validTypesWording, &$isValid, $path, $coerce = false) { foreach ($type as $tp) { // already valid, so no need to waste cycles looping over everything @@ -103,7 +110,7 @@ protected function validateTypesArray(&$value, array $type, &$validTypesWording, $this->validateTypeNameWording($tp); $validTypesWording[] = self::$wording[$tp]; if (!$isValid) { - $isValid = $this->validateType($value, $tp); + $isValid = $this->validateType($value, $tp, $coerce); } } } @@ -162,7 +169,7 @@ protected function validateTypeNameWording($type) * * @return bool */ - protected function validateType(&$value, $type) + protected function validateType(&$value, $type, $coerce = false) { //mostly the case for inline schema if (!$type) { @@ -181,8 +188,6 @@ protected function validateType(&$value, $type) return $this->getTypeCheck()->isArray($value); } - $coerce = $this->factory->getConfig(Constraint::CHECK_MODE_COERCE_TYPES); - if ('integer' === $type) { if ($coerce) { $value = $this->toInteger($value); diff --git a/tests/Constraints/CoerciveTest.php b/tests/Constraints/CoerciveTest.php index e4dd173d..f27e0d0f 100644 --- a/tests/Constraints/CoerciveTest.php +++ b/tests/Constraints/CoerciveTest.php @@ -70,6 +70,7 @@ public function testValidCoerceCases($input, $schema, $errors = array()) $this->assertTrue(gettype($value->multitype1) == 'boolean'); $this->assertTrue(gettype($value->multitype2) == 'double'); $this->assertTrue(gettype($value->multitype3) == 'integer'); + $this->assertTrue(gettype($value->multitype4) == 'string'); $this->assertTrue($value->number === 1.5); $this->assertTrue($value->integer === 1); @@ -146,6 +147,7 @@ public function getValidCoerceTests() "multitype1": "false", "multitype2": "1.2", "multitype3": "7", + "multitype4": "45", "arrayOfIntegers":["-1","0","1"], "tupleTyping":["1","2.2","true"], "any1": 2.6, @@ -175,6 +177,7 @@ public function getValidCoerceTests() "multitype1": {"type":["boolean","integer","number"]}, "multitype2": {"type":["boolean","integer","number"]}, "multitype3": {"type":["boolean","integer","number"]}, + "multitype4": {"type":["boolean","integer","string"]}, "arrayOfIntegers":{ "items":{ "type":"integer" From d70c8d67083c35b6682aef449d944798ea503236 Mon Sep 17 00:00:00 2001 From: Erayd Date: Tue, 14 Mar 2017 12:55:23 +1300 Subject: [PATCH 3/9] Add remaining coercion cases & rewrite tests * Add all remaining coercion cases from ajv matrix * Rewrite the coercion tests to tidy things up a bit --- src/JsonSchema/Constraints/TypeConstraint.php | 105 ++++- tests/Constraints/CoerciveTest.php | 398 +++++++----------- tests/Constraints/OfPropertiesTest.php | 8 +- 3 files changed, 252 insertions(+), 259 deletions(-) diff --git a/src/JsonSchema/Constraints/TypeConstraint.php b/src/JsonSchema/Constraints/TypeConstraint.php index 59bdd6d5..abbbda8b 100644 --- a/src/JsonSchema/Constraints/TypeConstraint.php +++ b/src/JsonSchema/Constraints/TypeConstraint.php @@ -69,8 +69,8 @@ public function check(&$value = null, $schema = null, JsonPointer $path = null, $wording[] = self::$wording[$type]; } $this->addError(ConstraintError::TYPE(), $path, array( - 'expected' => gettype($value), - 'found' => $this->implodeWith($wording, ', ', 'or') + 'found' => gettype($value), + 'expected' => $this->implodeWith($wording, ', ', 'or') )); } } @@ -185,6 +185,10 @@ protected function validateType(&$value, $type, $coerce = false) } if ('array' === $type) { + if ($coerce) { + $value = $this->toArray($value); + } + return $this->getTypeCheck()->isArray($value); } @@ -213,10 +217,18 @@ protected function validateType(&$value, $type, $coerce = false) } if ('string' === $type) { + if ($coerce) { + $value = $this->toString($value); + } + return is_string($value); } if ('null' === $type) { + if ($coerce) { + $value = $this->toNull($value); + } + return is_null($value); } @@ -232,19 +244,21 @@ protected function validateType(&$value, $type, $coerce = false) */ protected function toBoolean($value) { - if ($value === 'true') { + if ($value === 1 || $value === 'true') { return true; } - - if ($value === 'false') { + if (is_null($value) || $value === 0 || $value === 'false') { return false; } + if ($this->getTypeCheck()->isArray($value) && count($value) === 1) { + return $this->toBoolean(reset($value)); + } return $value; } /** - * Converts a numeric string to a number. For example, "4" becomes 4. + * Converts a value to a number. For example, "4.5" becomes 4.5. * * @param mixed $value the value to convert to a number * @@ -255,14 +269,89 @@ protected function toNumber($value) if (is_numeric($value)) { return $value + 0; // cast to number } + if (is_bool($value) || is_null($value)) { + return (int) $value; + } + if ($this->getTypeCheck()->isArray($value) && count($value) === 1) { + return $this->toNumber(reset($value)); + } return $value; } + /** + * Converts a value to an integer. For example, "4" becomes 4. + * + * @param mixed $value + * + * @return int|mixed + */ protected function toInteger($value) { - if (is_numeric($value) && (int) $value == $value) { - return (int) $value; // cast to number + $numberValue = $this->toNumber($value); + if (is_numeric($numberValue) && (int) $numberValue == $numberValue) { + return (int) $numberValue; // cast to number + } + + return $value; + } + + /** + * Converts a value to an array containing that value. For example, [4] becomes 4. + * + * @param mixed $value + * + * @return array|mixed + */ + protected function toArray($value) + { + if (is_scalar($value) || is_null($value)) { + return array($value); + } + + return $value; + } + + /** + * Convert a value to a string representation of that value. For example, null becomes "". + * + * @param mixed $value + * + * @return string|mixed + */ + protected function toString($value) + { + if (is_numeric($value)) { + return "$value"; + } + if ($value === true) { + return 'true'; + } + if ($value === false) { + return 'false'; + } + if (is_null($value)) { + return ''; + } + if ($this->getTypeCheck()->isArray($value) && count($value) === 1) { + return $this->toString(reset($value)); + } + } + + /** + * Convert a value to a null. For example, 0 becomes null. + * + * @param mixed $value + * + * @return null|mixed + */ + protected function toNull($value) + { + if ($value === 0 || $value === false || $value === '') { + return null; + } + if ($this->getTypeCheck()->isArray($value) && count($value) === 1) { + return $this->toNull(reset($value)); } return $value; diff --git a/tests/Constraints/CoerciveTest.php b/tests/Constraints/CoerciveTest.php index f27e0d0f..aad14552 100644 --- a/tests/Constraints/CoerciveTest.php +++ b/tests/Constraints/CoerciveTest.php @@ -11,113 +11,182 @@ use JsonSchema\Constraints\Constraint; use JsonSchema\Constraints\Factory; -use JsonSchema\SchemaStorage; -use JsonSchema\Uri\UriResolver; +use JsonSchema\Constraints\TypeCheck\LooseTypeCheck; use JsonSchema\Validator; -class CoerciveTest extends BasicTypesTest +class CoerciveTest extends VeryBaseTestCase { - protected $schemaSpec = 'http://json-schema.org/draft-03/schema#'; - protected $validateSchema = true; + protected $factory = null; - /** - * @dataProvider getValidCoerceTests - */ - public function testValidCoerceCasesUsingAssoc($input, $schema) + public function setUp() { - $checkMode = Constraint::CHECK_MODE_TYPE_CAST | Constraint::CHECK_MODE_COERCE_TYPES; - - $schemaStorage = new SchemaStorage($this->getUriRetrieverMock(json_decode($schema))); - $schema = $schemaStorage->getSchema('http://www.my-domain.com/schema.json'); - - $validator = new Validator(new Factory($schemaStorage, null, $checkMode)); - - $value = json_decode($input, true); - - $validator->validate($value, $schema, $checkMode); - $this->assertTrue($validator->isValid(), print_r($validator->getErrors(), true)); + $this->factory = new Factory(); + $this->factory->setConfig(Constraint::CHECK_MODE_TYPE_CAST | Constraint::CHECK_MODE_COERCE_TYPES); } - /** - * @dataProvider getValidCoerceTests - */ - public function testValidCoerceCases($input, $schema, $errors = array()) + public function dataCoerceCases() { - $checkMode = Constraint::CHECK_MODE_TYPE_CAST | Constraint::CHECK_MODE_COERCE_TYPES; - - $schemaStorage = new SchemaStorage($this->getUriRetrieverMock(json_decode($schema))); - $schema = $schemaStorage->getSchema('http://www.my-domain.com/schema.json'); + // check type conversions + $types = array( + // toType + 'string' => array( + // fromType fromValue toValue valid Test Number + array('string', '"string"', 'string', true), // #0 + array('integer', '45', '45', true), // #1 + array('boolean', 'true', 'true', true), // #2 + array('boolean', 'false', 'false', true), // #3 + array('NULL', 'null', '', true), // #4 + array('array', '[45]', '45', true), // #5 + array('object', '{"a":"b"}', null, false), // #6 + array('array', '[{"a":"b"}]', null, false), // #7 + ), + 'integer' => array( + array('string', '"45"', 45, true), // #8 + array('integer', '45', 45, true), // #9 + array('boolean', 'true', 1, true), // #10 + array('boolean', 'false', 0, true), // #11 + array('NULL', 'null', 0, true), // #12 + array('array', '["-45"]', -45, true), // #13 + array('object', '{"a":"b"}', null, false), // #14 + ), + 'boolean' => array( + array('string', '"true"', true, true), // #15 + array('integer', '1', true, true), // #16 + array('boolean', 'true', true, true), // #17 + array('NULL', 'null', false, true), // #18 + array('array', '["true"]', true, true), // #19 + array('object', '{"a":"b"}', null, false), // #20 + array('string', '""', null, false), // #21 + array('string', '"ABC"', null, false), // #22 + array('integer', '2', null, false), // #23 + ), + 'NULL' => array( + array('string', '""', null, true), // #24 + array('integer', '0', null, true), // #25 + array('boolean', 'false', null, true), // #26 + array('NULL', 'null', null, true), // #27 + array('array', '[0]', null, true), // #28 + array('object', '{"a":"b"}', null, false), // #29 + array('string', '"null"', null, false), // #30 + array('integer', '-1', null, false), // #31 + ), + 'array' => array( + array('string', '"ABC"', array('ABC'), true), // #32 + array('integer', '45', array(45), true), // #33 + array('boolean', 'true', array(true), true), // #34 + array('NULL', 'null', array(null), true), // #35 + array('array', '["ABC"]', array('ABC'), true), // #36 + array('object', '{"a":"b"}', null, false), // #37 + ), + ); - $validator = new Validator(new Factory($schemaStorage, null, $checkMode)); - $value = json_decode($input); + $tests = array(); + foreach ($types as $toType => $testCases) { + foreach ($testCases as $testCase) { + $tests[] = array( + sprintf('{"properties":{"propertyOne":{"type":"%s"}}}', strtolower($toType)), + sprintf('{"propertyOne":%s}', $testCase[1]), + $testCase[0], + $toType, + $testCase[2], + $testCase[3] + ); + } + } - $this->assertTrue(gettype($value->number) == 'string'); - $this->assertTrue(gettype($value->integer) == 'string'); - $this->assertTrue(gettype($value->boolean) == 'string'); + // #38 check post-coercion validation (to array) + $tests[] = array( + '{"properties":{"propertyOne":{"type":"array","items":[{"type":"number"}]}}}', + '{"propertyOne":"ABC"}', + 'string', null, null, false + ); - $validator->validate($value, $schema, $checkMode); + // #39 check post-coercion validation (from array) + $tests[] = array( + '{"properties":{"propertyOne":{"type":"number"}}}', + '{"propertyOne":["ABC"]}', + 'array', null, null, false + ); - $this->assertTrue(gettype($value->number) == 'double'); - $this->assertTrue(gettype($value->integer) == 'integer'); - $this->assertTrue(gettype($value->negativeInteger) == 'integer'); - $this->assertTrue(gettype($value->boolean) == 'boolean'); + // #40 check multiple types (first valid) + $tests[] = array( + '{"properties":{"propertyOne":{"type":["number", "string"]}}}', + '{"propertyOne":42}', + 'integer', 'integer', 42, true + ); - $this->assertTrue($value->number === 1.5); - $this->assertTrue($value->integer === 1); - $this->assertTrue($value->negativeInteger === -2); - $this->assertTrue($value->boolean === true); + // #41 check multiple types (last valid) + $tests[] = array( + '{"properties":{"propertyOne":{"type":["number", "string"]}}}', + '{"propertyOne":"42"}', + 'string', 'string', '42', true + ); - $this->assertTrue(gettype($value->multitype1) == 'boolean'); - $this->assertTrue(gettype($value->multitype2) == 'double'); - $this->assertTrue(gettype($value->multitype3) == 'integer'); - $this->assertTrue(gettype($value->multitype4) == 'string'); + // #42 check the meaning of life + $tests[] = array( + '{"properties":{"propertyOne":{"type":"any"}}}', + '{"propertyOne":"42"}', + 'string', 'string', '42', true + ); - $this->assertTrue($value->number === 1.5); - $this->assertTrue($value->integer === 1); - $this->assertTrue($value->negativeInteger === -2); - $this->assertTrue($value->boolean === true); + // #43 check turple coercion + $tests[] = array( + '{"properties":{"propertyOne":{"type":"array","items":[{"type":"number"},{"type":"string"}]}}}', + '{"propertyOne":["42", 42]}', + 'array', 'array', array(42, '42'), true + ); - $this->assertTrue($validator->isValid(), print_r($validator->getErrors(), true)); + return $tests; } - /** - * @dataProvider getInvalidCoerceTests - */ - public function testInvalidCoerceCases($input, $schema, $errors = array()) + /** @dataProvider dataCoerceCases **/ + public function testCoerceCases($schema, $data, $startType, $endType, $endValue, $valid, $assoc = false) { - $checkMode = Constraint::CHECK_MODE_TYPE_CAST | Constraint::CHECK_MODE_COERCE_TYPES; + $validator = new Validator($this->factory); - $schemaStorage = new SchemaStorage($this->getUriRetrieverMock(json_decode($schema))); - $schema = $schemaStorage->getSchema('http://www.my-domain.com/schema.json'); + $schema = json_decode($schema); + $data = json_decode($data, $assoc); - $validator = new Validator(new Factory($schemaStorage, null, $checkMode)); - $value = json_decode($input); - $validator->validate($value, $schema, $checkMode); - - if (array() !== $errors) { - $this->assertEquals($errors, $validator->getErrors(), print_r($validator->getErrors(), true)); + // check initial type + $type = gettype(LooseTypeCheck::propertyGet($data, 'propertyOne')); + if ($assoc && $type == 'array' && $startType == 'object') { + $type = 'object'; + } + $this->assertEquals($startType, $type, "Incorrect type '$type': expected '$startType'"); + + $validator->validate($data, $schema); + + // check validity + if ($valid) { + $this->assertTrue( + $validator->isValid(), + 'Validation failed: ' . json_encode($validator->getErrors(), \JSON_PRETTY_PRINT) + ); + + // check end type + $type = gettype(LooseTypeCheck::propertyGet($data, 'propertyOne')); + $this->assertEquals($endType, $type, "Incorrect type '$type': expected '$endType'"); + + // check end value + $value = LooseTypeCheck::propertyGet($data, 'propertyOne'); + $this->assertTrue( + $value === $endValue, + sprintf( + "Incorrect value '%s': expected '%s'", + is_scalar($value) ? $value : gettype($value), + is_scalar($endValue) ? $endValue : gettype($endValue) + ) + ); + } else { + $this->assertFalse($validator->isValid(), 'Validation succeeded, but should have failed'); + $this->assertEquals(1, count($validator->getErrors())); } - $this->assertFalse($validator->isValid(), print_r($validator->getErrors(), true)); } - /** - * @dataProvider getInvalidCoerceTests - */ - public function testInvalidCoerceCasesUsingAssoc($input, $schema, $errors = array()) + /** @dataProvider dataCoerceCases **/ + public function testCoerceCasesUsingAssoc($schema, $data, $startType, $endType, $endValue, $valid) { - $checkMode = Constraint::CHECK_MODE_TYPE_CAST | Constraint::CHECK_MODE_COERCE_TYPES; - - $schemaStorage = new SchemaStorage($this->getUriRetrieverMock(json_decode($schema))); - $schema = $schemaStorage->getSchema('http://www.my-domain.com/schema.json'); - - $validator = new Validator(new Factory($schemaStorage, null, $checkMode)); - $value = json_decode($input, true); - $validator->validate($value, $schema, $checkMode); - - if (array() !== $errors) { - $this->assertEquals($errors, $validator->getErrors(), print_r($validator->getErrors(), true)); - } - $this->assertFalse($validator->isValid(), print_r($validator->getErrors(), true)); + $this->testCoerceCases($schema, $data, $startType, $endType, $endValue, $valid, true); } public function testCoerceAPI() @@ -128,169 +197,4 @@ public function testCoerceAPI() $v->coerce($input, $schema); $this->assertEquals('{"propertyOne":10}', json_encode($input)); } - - public function getValidCoerceTests() - { - return array( - array( - '{ - "string":"string test", - "number":"1.5", - "integer":"1", - "negativeInteger":"-2", - "boolean":"true", - "object":{}, - "array":[], - "null":null, - "any": "string", - "allOf": "1", - "multitype1": "false", - "multitype2": "1.2", - "multitype3": "7", - "multitype4": "45", - "arrayOfIntegers":["-1","0","1"], - "tupleTyping":["1","2.2","true"], - "any1": 2.6, - "any2": 4, - "any3": false, - "any4": {}, - "any5": [], - "any6": null - }', - '{ - "type":"object", - "properties":{ - "string":{"type":"string"}, - "number":{"type":"number"}, - "integer":{"type":"integer"}, - "negativeInteger":{"type":"integer"}, - "boolean":{"type":"boolean"}, - "object":{"type":"object"}, - "array":{"type":"array"}, - "null":{"type":"null"}, - "any": {"type":"any"}, - "allOf" : {"allOf":[{ - "type" : "string" - },{ - "type" : "integer" - }]}, - "multitype1": {"type":["boolean","integer","number"]}, - "multitype2": {"type":["boolean","integer","number"]}, - "multitype3": {"type":["boolean","integer","number"]}, - "multitype4": {"type":["boolean","integer","string"]}, - "arrayOfIntegers":{ - "items":{ - "type":"integer" - } - }, - "tupleTyping":{ - "type":"array", - "items":[ - {"type":"integer"}, - {"type":"number"} - ], - "additionalItems":{"type":"boolean"} - }, - "any1": {"type":"any"}, - "any2": {"type":"any"}, - "any3": {"type":"any"}, - "any4": {"type":"any"}, - "any5": {"type":"any"}, - "any6": {"type":"any"} - }, - "additionalProperties":false - }', - ), - ); - } - - public function getInvalidCoerceTests() - { - return array( - array( - '{ - "string":null - }', - '{ - "type":"object", - "properties": { - "string":{"type":"string"} - }, - "additionalProperties":false - }', - ), - array( - '{ - "number":"five" - }', - '{ - "type":"object", - "properties": { - "number":{"type":"number"} - }, - "additionalProperties":false - }', - ), - array( - '{ - "integer":"5.2" - }', - '{ - "type":"object", - "properties": { - "integer":{"type":"integer"} - }, - "additionalProperties":false - }', - ), - array( - '{ - "boolean":"0" - }', - '{ - "type":"object", - "properties": { - "boolean":{"type":"boolean"} - }, - "additionalProperties":false - }', - ), - array( - '{ - "object":null - }', - '{ - "type":"object", - "properties": { - "object":{"type":"object"} - }, - "additionalProperties":false - }', - ), - array( - '{ - "array":null - }', - '{ - "type":"object", - "properties": { - "array":{"type":"array"} - }, - "additionalProperties":false - }', - ), - array( - '{ - "null":1 - }', - '{ - "type":"object", - "properties": { - "null":{"type":"null"} - }, - "additionalProperties":false - }', - ), - ); - } } diff --git a/tests/Constraints/OfPropertiesTest.php b/tests/Constraints/OfPropertiesTest.php index 192369c6..ff8bded3 100644 --- a/tests/Constraints/OfPropertiesTest.php +++ b/tests/Constraints/OfPropertiesTest.php @@ -83,8 +83,8 @@ public function getInvalidTests() 'constraint' => array( 'name' => 'type', 'params' => array( - 'expected' => 'array', - 'found' => 'a string' + 'expected' => 'a string', + 'found' => 'array' ) ), 'context' => Validator::ERROR_DOCUMENT_VALIDATION @@ -96,8 +96,8 @@ public function getInvalidTests() 'constraint' => array( 'name' => 'type', 'params' => array( - 'expected' => 'array', - 'found' => 'a number' + 'expected' => 'a number', + 'found' => 'array' ) ), 'context' => Validator::ERROR_DOCUMENT_VALIDATION From a775d4d1afa8d0bbf78edad9123cf3544ecc26e0 Mon Sep 17 00:00:00 2001 From: Erayd Date: Tue, 14 Mar 2017 13:31:20 +1300 Subject: [PATCH 4/9] Add CHECK_MODE_EARLY_COERCE If set, falls back to the old behavior of coercing to the first compatible type, regardless of whether another already-valid type might be available. --- README.md | 9 +++++++-- src/JsonSchema/Constraints/Constraint.php | 1 + src/JsonSchema/Constraints/TypeConstraint.php | 9 +++++---- tests/Constraints/CoerciveTest.php | 18 +++++++++++++++--- 4 files changed, 28 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 8df14db7..62b05abb 100644 --- a/README.md +++ b/README.md @@ -186,14 +186,19 @@ third argument to `Validator::validate()`, or can be provided as the third argum | `Constraint::CHECK_MODE_NORMAL` | Validate in 'normal' mode - this is the default | | `Constraint::CHECK_MODE_TYPE_CAST` | Enable fuzzy type checking for associative arrays and objects | | `Constraint::CHECK_MODE_COERCE_TYPES` | Convert data types to match the schema where possible | +| `Constraint::CHECK_MODE_EARLY_COERCE` | Apply type coercion as soon as possible | | `Constraint::CHECK_MODE_APPLY_DEFAULTS` | Apply default values from the schema if not set | | `Constraint::CHECK_MODE_ONLY_REQUIRED_DEFAULTS` | When applying defaults, only set values that are required | | `Constraint::CHECK_MODE_EXCEPTIONS` | Throw an exception immediately if validation fails | | `Constraint::CHECK_MODE_DISABLE_FORMAT` | Do not validate "format" constraints | | `Constraint::CHECK_MODE_VALIDATE_SCHEMA` | Validate the schema as well as the provided document | -Please note that using `Constraint::CHECK_MODE_COERCE_TYPES` or `Constraint::CHECK_MODE_APPLY_DEFAULTS` -will modify your original data. +Please note that using `CHECK_MODE_COERCE_TYPES` or `CHECK_MODE_APPLY_DEFAULTS` will modify your +original data. + +`CHECK_MODE_EARLY_COERCE` has no effect unless used in combination with `CHECK_MODE_COERCE_TYPES`. If +enabled, type coercion will occur as soon as possible, even if the value may already be of another valid +type. ## Running the tests diff --git a/src/JsonSchema/Constraints/Constraint.php b/src/JsonSchema/Constraints/Constraint.php index b7f3bb42..05e3efa2 100644 --- a/src/JsonSchema/Constraints/Constraint.php +++ b/src/JsonSchema/Constraints/Constraint.php @@ -31,6 +31,7 @@ abstract class Constraint extends BaseConstraint implements ConstraintInterface const CHECK_MODE_APPLY_DEFAULTS = 0x00000008; const CHECK_MODE_EXCEPTIONS = 0x00000010; const CHECK_MODE_DISABLE_FORMAT = 0x00000020; + const CHECK_MODE_EARLY_COERCE = 0x00000040; const CHECK_MODE_ONLY_REQUIRED_DEFAULTS = 0x00000080; const CHECK_MODE_VALIDATE_SCHEMA = 0x00000100; diff --git a/src/JsonSchema/Constraints/TypeConstraint.php b/src/JsonSchema/Constraints/TypeConstraint.php index abbbda8b..5bfe08a9 100644 --- a/src/JsonSchema/Constraints/TypeConstraint.php +++ b/src/JsonSchema/Constraints/TypeConstraint.php @@ -45,11 +45,12 @@ public function check(&$value = null, $schema = null, JsonPointer $path = null, $type = isset($schema->type) ? $schema->type : null; $isValid = false; $coerce = $this->factory->getConfig(self::CHECK_MODE_COERCE_TYPES); + $earlyCoerce = $this->factory->getConfig(self::CHECK_MODE_EARLY_COERCE); $wording = array(); if (is_array($type)) { - $this->validateTypesArray($value, $type, $wording, $isValid, $path, false); - if (!$isValid && $coerce) { + $this->validateTypesArray($value, $type, $wording, $isValid, $path, $coerce && $earlyCoerce); + if (!$isValid && $coerce && !$earlyCoerce) { $this->validateTypesArray($value, $type, $wording, $isValid, $path, true); } } elseif (is_object($type)) { @@ -57,8 +58,8 @@ public function check(&$value = null, $schema = null, JsonPointer $path = null, return; } else { - $isValid = $this->validateType($value, $type, false); - if (!$isValid && $coerce) { + $isValid = $this->validateType($value, $type, $coerce && $earlyCoerce); + if (!$isValid && $coerce && !$earlyCoerce) { $isValid = $this->validateType($value, $type, true); } } diff --git a/tests/Constraints/CoerciveTest.php b/tests/Constraints/CoerciveTest.php index aad14552..53310da7 100644 --- a/tests/Constraints/CoerciveTest.php +++ b/tests/Constraints/CoerciveTest.php @@ -136,12 +136,22 @@ public function dataCoerceCases() 'array', 'array', array(42, '42'), true ); + // #44 check early coercion + $tests[] = array( + '{"properties":{"propertyOne":{"type":["object", "number", "string"]}}}', + '{"propertyOne":"42"}', + 'string', 'integer', 42, true, true + ); + return $tests; } /** @dataProvider dataCoerceCases **/ - public function testCoerceCases($schema, $data, $startType, $endType, $endValue, $valid, $assoc = false) + public function testCoerceCases($schema, $data, $startType, $endType, $endValue, $valid, $early = false, $assoc = false) { + if ($early) { + $this->factory->addConfig(Constraint::CHECK_MODE_EARLY_COERCE); + } $validator = new Validator($this->factory); $schema = json_decode($schema); @@ -181,12 +191,14 @@ public function testCoerceCases($schema, $data, $startType, $endType, $endValue, $this->assertFalse($validator->isValid(), 'Validation succeeded, but should have failed'); $this->assertEquals(1, count($validator->getErrors())); } + + $this->factory->removeConfig(Constraint::CHECK_MODE_EARLY_COERCE); } /** @dataProvider dataCoerceCases **/ - public function testCoerceCasesUsingAssoc($schema, $data, $startType, $endType, $endValue, $valid) + public function testCoerceCasesUsingAssoc($schema, $data, $startType, $endType, $endValue, $valid, $early = false) { - $this->testCoerceCases($schema, $data, $startType, $endType, $endValue, $valid, true); + $this->testCoerceCases($schema, $data, $startType, $endType, $endValue, $valid, $early, true); } public function testCoerceAPI() From 2ea432b7068ab267e400b006a58d2ae807cfebfd Mon Sep 17 00:00:00 2001 From: Erayd Date: Tue, 14 Mar 2017 13:32:36 +1300 Subject: [PATCH 5/9] Add multiple-type test that requires coercion --- tests/Constraints/CoerciveTest.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/Constraints/CoerciveTest.php b/tests/Constraints/CoerciveTest.php index 53310da7..b1b5e043 100644 --- a/tests/Constraints/CoerciveTest.php +++ b/tests/Constraints/CoerciveTest.php @@ -143,6 +143,13 @@ public function dataCoerceCases() 'string', 'integer', 42, true, true ); + // #45 check multiple types (none valid) + $tests[] = array( + '{"properties":{"propertyOne":{"type":["number", "boolean"]}}}', + '{"propertyOne":"42"}', + 'string', 'integer', 42, true + ); + return $tests; } From 4ca054141de86996ff3bf0c5c217ca01f5d067b7 Mon Sep 17 00:00:00 2001 From: Erayd Date: Tue, 14 Mar 2017 13:36:42 +1300 Subject: [PATCH 6/9] \JSON_PRETTY_PRINT doesn't exist in PHP-5.3, so work around this --- tests/Constraints/CoerciveTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Constraints/CoerciveTest.php b/tests/Constraints/CoerciveTest.php index b1b5e043..dca3f26c 100644 --- a/tests/Constraints/CoerciveTest.php +++ b/tests/Constraints/CoerciveTest.php @@ -175,9 +175,10 @@ public function testCoerceCases($schema, $data, $startType, $endType, $endValue, // check validity if ($valid) { + $prettyPrint = defined('\JSON_PRETTY_PRINT') ? constant('\JSON_PRETTY_PRINT') : 0; $this->assertTrue( $validator->isValid(), - 'Validation failed: ' . json_encode($validator->getErrors(), \JSON_PRETTY_PRINT) + 'Validation failed: ' . json_encode($validator->getErrors(), $prettyPrint) ); // check end type From ab0e8ba53a90344bae8c900160b9a13801b0dd17 Mon Sep 17 00:00:00 2001 From: Erayd Date: Tue, 14 Mar 2017 16:14:05 +1300 Subject: [PATCH 7/9] Various PR cleanup stuff * Fix whitespace * Turn $early into $extraFlags * Change "string" to "ABC" in string test * Update README.md description of CHECK_MODE_EARLY_COERCE --- README.md | 4 ++-- tests/Constraints/CoerciveTest.php | 15 +++++---------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 62b05abb..aabb2eb6 100644 --- a/README.md +++ b/README.md @@ -197,8 +197,8 @@ Please note that using `CHECK_MODE_COERCE_TYPES` or `CHECK_MODE_APPLY_DEFAULTS` original data. `CHECK_MODE_EARLY_COERCE` has no effect unless used in combination with `CHECK_MODE_COERCE_TYPES`. If -enabled, type coercion will occur as soon as possible, even if the value may already be of another valid -type. +enabled, the validator will use (and coerce) the first compatible type it encounters, even if the +schema defines another type that matches directly and does not require coercion. ## Running the tests diff --git a/tests/Constraints/CoerciveTest.php b/tests/Constraints/CoerciveTest.php index dca3f26c..2470a429 100644 --- a/tests/Constraints/CoerciveTest.php +++ b/tests/Constraints/CoerciveTest.php @@ -31,7 +31,7 @@ public function dataCoerceCases() // toType 'string' => array( // fromType fromValue toValue valid Test Number - array('string', '"string"', 'string', true), // #0 + array('string', '"ABC"', 'ABC', true), // #0 array('integer', '45', '45', true), // #1 array('boolean', 'true', 'true', true), // #2 array('boolean', 'false', 'false', true), // #3 @@ -55,7 +55,7 @@ public function dataCoerceCases() array('boolean', 'true', true, true), // #17 array('NULL', 'null', false, true), // #18 array('array', '["true"]', true, true), // #19 - array('object', '{"a":"b"}', null, false), // #20 + array('object', '{"a":"b"}', null, false), // #20 array('string', '""', null, false), // #21 array('string', '"ABC"', null, false), // #22 array('integer', '2', null, false), // #23 @@ -140,7 +140,7 @@ public function dataCoerceCases() $tests[] = array( '{"properties":{"propertyOne":{"type":["object", "number", "string"]}}}', '{"propertyOne":"42"}', - 'string', 'integer', 42, true, true + 'string', 'integer', 42, true, Constraint::CHECK_MODE_EARLY_COERCE ); // #45 check multiple types (none valid) @@ -154,11 +154,8 @@ public function dataCoerceCases() } /** @dataProvider dataCoerceCases **/ - public function testCoerceCases($schema, $data, $startType, $endType, $endValue, $valid, $early = false, $assoc = false) + public function testCoerceCases($schema, $data, $startType, $endType, $endValue, $valid, $extraFlags = 0, $assoc = false) { - if ($early) { - $this->factory->addConfig(Constraint::CHECK_MODE_EARLY_COERCE); - } $validator = new Validator($this->factory); $schema = json_decode($schema); @@ -171,7 +168,7 @@ public function testCoerceCases($schema, $data, $startType, $endType, $endValue, } $this->assertEquals($startType, $type, "Incorrect type '$type': expected '$startType'"); - $validator->validate($data, $schema); + $validator->validate($data, $schema, $this->factory->getConfig() | $extraFlags); // check validity if ($valid) { @@ -199,8 +196,6 @@ public function testCoerceCases($schema, $data, $startType, $endType, $endValue, $this->assertFalse($validator->isValid(), 'Validation succeeded, but should have failed'); $this->assertEquals(1, count($validator->getErrors())); } - - $this->factory->removeConfig(Constraint::CHECK_MODE_EARLY_COERCE); } /** @dataProvider dataCoerceCases **/ From c8f4b507bf94c68ec0cf19337c0604c73487e446 Mon Sep 17 00:00:00 2001 From: Erayd Date: Tue, 14 Mar 2017 16:18:54 +1300 Subject: [PATCH 8/9] Move loop after complex tests definition --- tests/Constraints/CoerciveTest.php | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/Constraints/CoerciveTest.php b/tests/Constraints/CoerciveTest.php index 2470a429..7ad4cdd5 100644 --- a/tests/Constraints/CoerciveTest.php +++ b/tests/Constraints/CoerciveTest.php @@ -80,20 +80,6 @@ public function dataCoerceCases() ), ); - $tests = array(); - foreach ($types as $toType => $testCases) { - foreach ($testCases as $testCase) { - $tests[] = array( - sprintf('{"properties":{"propertyOne":{"type":"%s"}}}', strtolower($toType)), - sprintf('{"propertyOne":%s}', $testCase[1]), - $testCase[0], - $toType, - $testCase[2], - $testCase[3] - ); - } - } - // #38 check post-coercion validation (to array) $tests[] = array( '{"properties":{"propertyOne":{"type":"array","items":[{"type":"number"}]}}}', @@ -150,6 +136,20 @@ public function dataCoerceCases() 'string', 'integer', 42, true ); + $tests = array(); + foreach ($types as $toType => $testCases) { + foreach ($testCases as $testCase) { + $tests[] = array( + sprintf('{"properties":{"propertyOne":{"type":"%s"}}}', strtolower($toType)), + sprintf('{"propertyOne":%s}', $testCase[1]), + $testCase[0], + $toType, + $testCase[2], + $testCase[3] + ); + } + } + return $tests; } From a21741dc4bf6deea18e5096b9c4f416038c8dd0d Mon Sep 17 00:00:00 2001 From: Erayd Date: Tue, 14 Mar 2017 16:52:23 +1300 Subject: [PATCH 9/9] Move test #39 to grid #15 --- tests/Constraints/CoerciveTest.php | 56 +++++++++++++----------------- 1 file changed, 25 insertions(+), 31 deletions(-) diff --git a/tests/Constraints/CoerciveTest.php b/tests/Constraints/CoerciveTest.php index 7ad4cdd5..9a910c8a 100644 --- a/tests/Constraints/CoerciveTest.php +++ b/tests/Constraints/CoerciveTest.php @@ -48,52 +48,46 @@ public function dataCoerceCases() array('NULL', 'null', 0, true), // #12 array('array', '["-45"]', -45, true), // #13 array('object', '{"a":"b"}', null, false), // #14 + array('array', '["ABC"]', null, false), // #15 ), 'boolean' => array( - array('string', '"true"', true, true), // #15 - array('integer', '1', true, true), // #16 - array('boolean', 'true', true, true), // #17 - array('NULL', 'null', false, true), // #18 - array('array', '["true"]', true, true), // #19 - array('object', '{"a":"b"}', null, false), // #20 - array('string', '""', null, false), // #21 - array('string', '"ABC"', null, false), // #22 - array('integer', '2', null, false), // #23 + array('string', '"true"', true, true), // #16 + array('integer', '1', true, true), // #17 + array('boolean', 'true', true, true), // #18 + array('NULL', 'null', false, true), // #19 + array('array', '["true"]', true, true), // #20 + array('object', '{"a":"b"}', null, false), // #21 + array('string', '""', null, false), // #22 + array('string', '"ABC"', null, false), // #23 + array('integer', '2', null, false), // #24 ), 'NULL' => array( - array('string', '""', null, true), // #24 - array('integer', '0', null, true), // #25 - array('boolean', 'false', null, true), // #26 - array('NULL', 'null', null, true), // #27 - array('array', '[0]', null, true), // #28 - array('object', '{"a":"b"}', null, false), // #29 - array('string', '"null"', null, false), // #30 - array('integer', '-1', null, false), // #31 + array('string', '""', null, true), // #25 + array('integer', '0', null, true), // #26 + array('boolean', 'false', null, true), // #27 + array('NULL', 'null', null, true), // #28 + array('array', '[0]', null, true), // #29 + array('object', '{"a":"b"}', null, false), // #30 + array('string', '"null"', null, false), // #31 + array('integer', '-1', null, false), // #32 ), 'array' => array( - array('string', '"ABC"', array('ABC'), true), // #32 - array('integer', '45', array(45), true), // #33 - array('boolean', 'true', array(true), true), // #34 - array('NULL', 'null', array(null), true), // #35 - array('array', '["ABC"]', array('ABC'), true), // #36 - array('object', '{"a":"b"}', null, false), // #37 + array('string', '"ABC"', array('ABC'), true), // #33 + array('integer', '45', array(45), true), // #34 + array('boolean', 'true', array(true), true), // #35 + array('NULL', 'null', array(null), true), // #36 + array('array', '["ABC"]', array('ABC'), true), // #37 + array('object', '{"a":"b"}', null, false), // #38 ), ); - // #38 check post-coercion validation (to array) + // #39 check post-coercion validation (to array) $tests[] = array( '{"properties":{"propertyOne":{"type":"array","items":[{"type":"number"}]}}}', '{"propertyOne":"ABC"}', 'string', null, null, false ); - // #39 check post-coercion validation (from array) - $tests[] = array( - '{"properties":{"propertyOne":{"type":"number"}}}', - '{"propertyOne":["ABC"]}', - 'array', null, null, false - ); - // #40 check multiple types (first valid) $tests[] = array( '{"properties":{"propertyOne":{"type":["number", "string"]}}}',