From 0254213bb945ade4b4c3a8f292352759f2ab6e8d Mon Sep 17 00:00:00 2001 From: Erayd Date: Sun, 4 Mar 2018 15:41:52 +1300 Subject: [PATCH] Remove large array type-specific performance optimisation Fixes #441. The bug in #441 was caused by refactoring of the optimisation which introduced a type-checking error. Noting the performance impact is negligible for all cases other than large arrays of strings or numbers, and introduces significant cognitive complexity to a project that is extremely short of maintainers, removing it seems like the best course of action. The performance improvement provided by this optimisation was approximately 40%, however it also carried a number of other problematic bugs - if it were to be reintroduced at a later date with those bugs fixed (mainly the skipping of much of the validation logic for optimised items, even in cases where that logic might be necessary), it would not have such a significant impact. --- .../Constraints/CollectionConstraint.php | 56 +++++-------------- 1 file changed, 15 insertions(+), 41 deletions(-) diff --git a/src/JsonSchema/Constraints/CollectionConstraint.php b/src/JsonSchema/Constraints/CollectionConstraint.php index 18f0efd9..825d4531 100644 --- a/src/JsonSchema/Constraints/CollectionConstraint.php +++ b/src/JsonSchema/Constraints/CollectionConstraint.php @@ -66,53 +66,27 @@ protected function validateItems(&$value, $schema = null, JsonPointer $path = nu { if (is_object($schema->items)) { // just one type definition for the whole array + foreach ($value as $k => &$v) { + $initErrors = $this->getErrors(); - if (isset($schema->items->type) - && ( - $schema->items->type == 'string' - || $schema->items->type == 'number' - || $schema->items->type == 'integer' - ) - && !isset($schema->additionalItems) - ) { - // performance optimization - $type = $schema->items->type; - $typeValidator = $this->factory->createInstanceFor('type'); - $validator = $this->factory->createInstanceFor($type === 'integer' ? 'number' : $type); - - foreach ($value as $k => &$v) { - $k_path = $this->incrementPath($path, $k); - $typeValidator->check($v, $schema->items, $k_path, $i); + // First check if its defined in "items" + $this->checkUndefined($v, $schema->items, $path, $k); - $validator->check($v, $schema->items, $k_path, $i); + // Recheck with "additionalItems" if the first test fails + if (count($initErrors) < count($this->getErrors()) && (isset($schema->additionalItems) && $schema->additionalItems !== false)) { + $secondErrors = $this->getErrors(); + $this->checkUndefined($v, $schema->additionalItems, $path, $k); } - unset($v); /* remove dangling reference to prevent any future bugs - * caused by accidentally using $v elsewhere */ - $this->addErrors($typeValidator->getErrors()); - $this->addErrors($validator->getErrors()); - } else { - foreach ($value as $k => &$v) { - $initErrors = $this->getErrors(); - - // First check if its defined in "items" - $this->checkUndefined($v, $schema->items, $path, $k); - - // Recheck with "additionalItems" if the first test fails - if (count($initErrors) < count($this->getErrors()) && (isset($schema->additionalItems) && $schema->additionalItems !== false)) { - $secondErrors = $this->getErrors(); - $this->checkUndefined($v, $schema->additionalItems, $path, $k); - } - // Reset errors if needed - if (isset($secondErrors) && count($secondErrors) < count($this->getErrors())) { - $this->errors = $secondErrors; - } elseif (isset($secondErrors) && count($secondErrors) === count($this->getErrors())) { - $this->errors = $initErrors; - } + // Reset errors if needed + if (isset($secondErrors) && count($secondErrors) < count($this->getErrors())) { + $this->errors = $secondErrors; + } elseif (isset($secondErrors) && count($secondErrors) === count($this->getErrors())) { + $this->errors = $initErrors; } - unset($v); /* remove dangling reference to prevent any future bugs - * caused by accidentally using $v elsewhere */ } + unset($v); /* remove dangling reference to prevent any future bugs + * caused by accidentally using $v elsewhere */ } else { // Defined item type definitions foreach ($value as $k => &$v) {