Skip to content

performance improvements #310

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 10, 2016
Merged

performance improvements #310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 12 additions & 4 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,31 @@
sudo: false
language: php

cache:
directories:
- $HOME/.composer/cache

matrix:
fast_finish: true
include:
- php: 5.3
- php: 5.4
- php: 5.5
- php: 5.6
- php: 7.0
env: WITH_COVERAGE=true
- php: 7
- php: 7.1
- php: 'nightly'
- php: hhvm
allow_failures:
- php: 'nightly'

before_install:
- if [[ "$WITH_COVERAGE" != "true" && "$TRAVIS_PHP_VERSION" != "hhvm" ]]; then phpenv config-rm xdebug.ini; fi
- if [[ "$WITH_COVERAGE" != "true" && "$TRAVIS_PHP_VERSION" != "hhvm" && "$TRAVIS_PHP_VERSION" != "nightly" && "$TRAVIS_PHP_VERSION" != "7.1" ]]; then phpenv config-rm xdebug.ini; fi
- composer selfupdate

install:
- travis_retry composer install --no-interaction --prefer-source
- travis_retry composer install --no-interaction --prefer-dist

script:
- if [[ "$WITH_COVERAGE" == "true" ]]; then vendor/bin/phpunit --coverage-text; else vendor/bin/phpunit; fi
- if [[ "$WITH_COVERAGE" == "true" ]]; then composer coverage; else composer test; fi
4 changes: 4 additions & 0 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,9 @@
"branch-alias": {
"dev-master": "3.0.x-dev"
}
},
"scripts": {
"test" : "vendor/bin/phpunit",
"coverage" : "vendor/bin/phpunit --coverage-text"
}
}
2 changes: 1 addition & 1 deletion src/JsonSchema/Constraints/CollectionConstraint.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ protected function validateItems($value, $schema = null, JsonPointer $path = nul
// Treat when we have more schema definitions than values, not for empty arrays
if (count($value) > 0) {
for ($k = count($value); $k < count($schema->items); $k++) {
$this->checkUndefined(new UndefinedConstraint(), $schema->items[$k], $path, $k);
$this->checkUndefined($this->factory->createInstanceFor('undefined'), $schema->items[$k], $path, $k);
}
}
}
Expand Down
90 changes: 18 additions & 72 deletions src/JsonSchema/Constraints/Constraint.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@
*/
abstract class Constraint implements ConstraintInterface
{
protected $schemaStorage;
protected $checkMode = self::CHECK_MODE_NORMAL;
protected $uriRetriever;
protected $errors = array();
protected $inlineSchemaProperty = '$schema';

Expand All @@ -33,70 +30,16 @@ abstract class Constraint implements ConstraintInterface
const CHECK_MODE_COERCE = 0x00000004;

/**
* @var null|Factory
* @var Factory
*/
private $factory;
protected $factory;

/**
* @param int $checkMode
* @param SchemaStorage $schemaStorage
* @param UriRetrieverInterface $uriRetriever
* @param Factory $factory
*/
public function __construct(
$checkMode = self::CHECK_MODE_NORMAL,
SchemaStorage $schemaStorage = null,
UriRetrieverInterface $uriRetriever = null,
Factory $factory = null
) {
$this->checkMode = $checkMode;
$this->uriRetriever = $uriRetriever;
$this->factory = $factory;
$this->schemaStorage = $schemaStorage;
}

/**
* @return UriRetrieverInterface $uriRetriever
*/
public function getUriRetriever()
{
if (is_null($this->uriRetriever)) {
$this->setUriRetriever(new UriRetriever);
}

return $this->uriRetriever;
}

/**
* @return Factory
*/
public function getFactory()
public function __construct(Factory $factory = null)
Copy link

Choose a reason for hiding this comment

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

you should consider providing a BC layer for the constructor signature. Otherwise, this force to bump to 4.0 already. Supporting the old signature with a deprecation warning (building a factory from the older arguments) would make the upgrade path easier

Copy link
Contributor Author

Choose a reason for hiding this comment

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

im slightly against supporting BC...i guess nobody is using custom constraints out there so its all part of the internals...im ok with a BC break here

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 injecting a few constraints, at least until this gets merged: #308

That said, changing my client code isn't a big deal. When can we expect these PRs to get merged? What are we waiting on?

{
if (!$this->factory) {
$this->factory = new Factory($this->getSchemaStorage(), $this->getUriRetriever(), $this->checkMode);
}

return $this->factory;
}

/**
* @return SchemaStorage
*/
public function getSchemaStorage()
{
if (is_null($this->schemaStorage)) {
$this->schemaStorage = new SchemaStorage($this->getUriRetriever());
}

return $this->schemaStorage;
}

/**
* @param UriRetrieverInterface $uriRetriever
*/
public function setUriRetriever(UriRetrieverInterface $uriRetriever)
{
$this->uriRetriever = $uriRetriever;
$this->factory = $factory ? : new Factory();
}

/**
Expand Down Expand Up @@ -124,7 +67,9 @@ public function addError(JsonPointer $path = null, $message, $constraint='', arr
*/
public function addErrors(array $errors)
{
$this->errors = array_merge($this->errors, $errors);
if ($errors) {
$this->errors = array_merge($this->errors, $errors);
}
}

/**
Expand Down Expand Up @@ -182,7 +127,7 @@ protected function incrementPath(JsonPointer $path = null, $i)
*/
protected function checkArray($value, $schema = null, JsonPointer $path = null, $i = null)
{
$validator = $this->getFactory()->createInstanceFor('collection');
$validator = $this->factory->createInstanceFor('collection');
$validator->check($value, $schema, $path, $i);

$this->addErrors($validator->getErrors());
Expand All @@ -199,7 +144,7 @@ protected function checkArray($value, $schema = null, JsonPointer $path = null,
*/
protected function checkObject($value, $schema = null, JsonPointer $path = null, $i = null, $patternProperties = null)
{
$validator = $this->getFactory()->createInstanceFor('object');
$validator = $this->factory->createInstanceFor('object');
$validator->check($value, $schema, $path, $i, $patternProperties);

$this->addErrors($validator->getErrors());
Expand All @@ -215,7 +160,7 @@ protected function checkObject($value, $schema = null, JsonPointer $path = null,
*/
protected function checkType($value, $schema = null, JsonPointer $path = null, $i = null)
{
$validator = $this->getFactory()->createInstanceFor('type');
$validator = $this->factory->createInstanceFor('type');
$validator->check($value, $schema, $path, $i);

$this->addErrors($validator->getErrors());
Expand All @@ -231,8 +176,9 @@ protected function checkType($value, $schema = null, JsonPointer $path = null, $
*/
protected function checkUndefined($value, $schema = null, JsonPointer $path = null, $i = null)
{
$validator = $this->getFactory()->createInstanceFor('undefined');
$validator->check($value, $this->schemaStorage->resolveRefSchema($schema), $path, $i);
$validator = $this->factory->createInstanceFor('undefined');

$validator->check($value, $this->factory->getSchemaStorage()->resolveRefSchema($schema), $path, $i);

$this->addErrors($validator->getErrors());
}
Expand All @@ -247,7 +193,7 @@ protected function checkUndefined($value, $schema = null, JsonPointer $path = nu
*/
protected function checkString($value, $schema = null, JsonPointer $path = null, $i = null)
{
$validator = $this->getFactory()->createInstanceFor('string');
$validator = $this->factory->createInstanceFor('string');
$validator->check($value, $schema, $path, $i);

$this->addErrors($validator->getErrors());
Expand All @@ -263,7 +209,7 @@ protected function checkString($value, $schema = null, JsonPointer $path = null,
*/
protected function checkNumber($value, $schema = null, JsonPointer $path = null, $i = null)
{
$validator = $this->getFactory()->createInstanceFor('number');
$validator = $this->factory->createInstanceFor('number');
$validator->check($value, $schema, $path, $i);

$this->addErrors($validator->getErrors());
Expand All @@ -279,7 +225,7 @@ protected function checkNumber($value, $schema = null, JsonPointer $path = null,
*/
protected function checkEnum($value, $schema = null, JsonPointer $path = null, $i = null)
{
$validator = $this->getFactory()->createInstanceFor('enum');
$validator = $this->factory->createInstanceFor('enum');
$validator->check($value, $schema, $path, $i);

$this->addErrors($validator->getErrors());
Expand All @@ -295,7 +241,7 @@ protected function checkEnum($value, $schema = null, JsonPointer $path = null, $
*/
protected function checkFormat($value, $schema = null, JsonPointer $path = null, $i = null)
{
$validator = $this->getFactory()->createInstanceFor('format');
$validator = $this->factory->createInstanceFor('format');
$validator->check($value, $schema, $path, $i);

$this->addErrors($validator->getErrors());
Expand All @@ -308,7 +254,7 @@ protected function checkFormat($value, $schema = null, JsonPointer $path = null,
*/
protected function getTypeCheck()
{
return $this->getFactory()->getTypeCheck();
return $this->factory->getTypeCheck();
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/JsonSchema/Constraints/EnumConstraint.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public function check($element, $schema = null, JsonPointer $path = null, $i = n

foreach ($schema->enum as $enum) {
$enumType = gettype($enum);
if (($this->checkMode & self::CHECK_MODE_TYPE_CAST) && $type == "array" && $enumType == "object") {
if (($this->factory->getCheckMode() & self::CHECK_MODE_TYPE_CAST) && $type == "array" && $enumType == "object") {
if ((object)$element == $enum) {
return;
}
Expand Down
27 changes: 16 additions & 11 deletions src/JsonSchema/Constraints/Factory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,17 +129,22 @@ public function setConstraintClass($name, $class)
*/
public function createInstanceFor($constraintName)
{
if (array_key_exists($constraintName, $this->constraintMap)) {
if (!isset($this->instanceCache[$constraintName])) {
$this->instanceCache[$constraintName] = new $this->constraintMap[$constraintName](
$this->checkMode,
$this->schemaStorage,
$this->uriRetriever,
$this
);
}
return clone $this->instanceCache[$constraintName];
if (!isset($this->constraintMap[$constraintName])) {
throw new InvalidArgumentException('Unknown constraint ' . $constraintName);
}
throw new InvalidArgumentException('Unknown constraint ' . $constraintName);

if (!isset($this->instanceCache[$constraintName])) {
$this->instanceCache[$constraintName] = new $this->constraintMap[$constraintName]($this);
}

return clone $this->instanceCache[$constraintName];
}

/**
* @return int
*/
public function getCheckMode()
{
return $this->checkMode;
}
}
19 changes: 10 additions & 9 deletions src/JsonSchema/Constraints/ObjectConstraint.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ public function validatePatternProperties($element, JsonPointer $path = null, $p
public function validateElement($element, $matches, $objectDefinition = null, JsonPointer $path = null, $additionalProp = null)
{
$this->validateMinMaxConstraint($element, $objectDefinition, $path);

foreach ($element as $i => $value) {
$definition = $this->getProperty($objectDefinition, $i);

Expand All @@ -105,7 +106,7 @@ public function validateElement($element, $matches, $objectDefinition = null, Js
$this->addError($path, "The presence of the property " . $i . " requires that " . $require . " also be present", 'requires');
}

$property = $this->getProperty($element, $i, new UndefinedConstraint());
$property = $this->getProperty($element, $i, $this->factory->createInstanceFor('undefined'));
if (is_object($property)) {
$this->validateMinMaxConstraint(!($property instanceof UndefinedConstraint) ? $property : $element, $definition, $path);
}
Expand All @@ -121,17 +122,17 @@ public function validateElement($element, $matches, $objectDefinition = null, Js
*/
public function validateDefinition($element, $objectDefinition = null, JsonPointer $path = null)
{
$default = $this->getFactory()->createInstanceFor('undefined');
$undefinedConstraint = $this->factory->createInstanceFor('undefined');

foreach ($objectDefinition as $i => $value) {
$property = $this->getProperty($element, $i, $default);
$property = $this->getProperty($element, $i, $undefinedConstraint);
$definition = $this->getProperty($objectDefinition, $i);

if($this->checkMode & Constraint::CHECK_MODE_TYPE_CAST){
if($this->factory->getCheckMode() & Constraint::CHECK_MODE_TYPE_CAST){
if(!($property instanceof Constraint)) {
$property = $this->coerce($property, $definition);

if($this->checkMode & Constraint::CHECK_MODE_COERCE) {
if($this->factory->getCheckMode() & Constraint::CHECK_MODE_COERCE) {
if (is_object($element)) {
$element->{$i} = $property;
} else {
Expand Down Expand Up @@ -228,10 +229,10 @@ protected function coerce($value, $definition)
*/
protected function getProperty($element, $property, $fallback = null)
{
if (is_array($element) /*$this->checkMode == self::CHECK_MODE_TYPE_CAST*/) {
return array_key_exists($property, $element) ? $element[$property] : $fallback;
} elseif (is_object($element)) {
return property_exists($element, $property) ? $element->$property : $fallback;
if (is_array($element) && (isset($element[$property]) || array_key_exists($property, $element)) /*$this->checkMode == self::CHECK_MODE_TYPE_CAST*/) {
return $element[$property];
} elseif (is_object($element) && property_exists($element, $property)) {
return $element->$property;
}

return $fallback;
Expand Down
2 changes: 1 addition & 1 deletion src/JsonSchema/Constraints/TypeConstraint.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ protected function validateTypesArray($value, array $type, &$validTypesWording,
// with a new type constraint
if (is_object($tp)) {
if (!$isValid) {
$validator = $this->getFactory()->createInstanceFor('type');
$validator = $this->factory->createInstanceFor('type');
$subSchema = new \stdClass();
$subSchema->type = $tp;
$validator->check($value, $subSchema, $path, null);
Expand Down
4 changes: 2 additions & 2 deletions src/JsonSchema/Constraints/UndefinedConstraint.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public function validateTypes($value, $schema = null, JsonPointer $path, $i = nu
if ($this->getTypeCheck()->isObject($value)) {
$this->checkObject(
$value,
isset($schema->properties) ? $this->schemaStorage->resolveRefSchema($schema->properties) : $schema,
isset($schema->properties) ? $this->factory->getSchemaStorage()->resolveRefSchema($schema->properties) : $schema,
$path,
isset($schema->additionalProperties) ? $schema->additionalProperties : null,
isset($schema->patternProperties) ? $schema->patternProperties : null
Expand Down Expand Up @@ -267,7 +267,7 @@ protected function validateDependencies($value, $dependencies, JsonPointer $path
protected function validateUri($schema, $schemaUri = null)
{
$resolver = new UriResolver();
$retriever = $this->getUriRetriever();
$retriever = $this->factory->getUriRetriever();

$jsonSchema = null;
if ($resolver->isValid($schemaUri)) {
Expand Down
2 changes: 1 addition & 1 deletion src/JsonSchema/Validator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class Validator extends Constraint
*/
public function check($value, $schema = null, JsonPointer $path = null, $i = null)
{
$validator = $this->getFactory()->createInstanceFor('schema');
$validator = $this->factory->createInstanceFor('schema');
$validator->check($value, $schema);

$this->addErrors(array_unique($validator->getErrors(), SORT_REGULAR));
Expand Down
Loading