Split $objectDefinition into $schema and $properties (#411)

Object validation attempts to use a single variable to store both the
object definition, and its properties. This causes validation to be
incomplete where "properties" is not set, but "additionalProperties" is.

This commit fixes both bugs in issue #353.

Author: erayd

src/JsonSchema/Constraints/Constraint.php

@@ -78,13 +78,15 @@ protected function checkArray(&$value, $schema = null, JsonPointer $path = null,
      * @param mixed            $value
      * @param mixed            $schema
      * @param JsonPointer|null $path
-     * @param mixed            $i
+     * @param mixed            $properties
+     * @param mixed            $additionalProperties
      * @param mixed            $patternProperties
      */
-    protected function checkObject(&$value, $schema = null, JsonPointer $path = null, $i = null, $patternProperties = null, $appliedDefaults = array())
+    protected function checkObject(&$value, $schema = null, JsonPointer $path = null, $properties = null,
+        $additionalProperties = null, $patternProperties = null, $appliedDefaults = array())
     {
         $validator = $this->factory->createInstanceFor('object');
-        $validator->check($value, $schema, $path, $i, $patternProperties, $appliedDefaults);
+        $validator->check($value, $schema, $path, $properties, $additionalProperties, $patternProperties, $appliedDefaults);
 
         $this->addErrors($validator->getErrors());
     }

src/JsonSchema/Constraints/ObjectConstraint.php

@@ -28,7 +28,8 @@ class ObjectConstraint extends Constraint
     /**
      * {@inheritdoc}
      */
-    public function check(&$element, $definition = null, JsonPointer $path = null, $additionalProp = null, $patternProperties = null, $appliedDefaults = array())
+    public function check(&$element, $schema = null, JsonPointer $path = null, $properties = null,
+        $additionalProp = null, $patternProperties = null, $appliedDefaults = array())
     {
         if ($element instanceof UndefinedConstraint) {
             return;
@@ -38,16 +39,17 @@ public function check(&$element, $definition = null, JsonPointer $path = null, $
 
         $matches = array();
         if ($patternProperties) {
+            // validate the element pattern properties
             $matches = $this->validatePatternProperties($element, $path, $patternProperties);
         }
 
-        if ($definition) {
-            // validate the definition properties
-            $this->validateDefinition($element, $definition, $path);
+        if ($properties) {
+            // validate the element properties
+            $this->validateProperties($element, $properties, $path);
         }
 
-        // additional the element properties
-        $this->validateElement($element, $matches, $definition, $path, $additionalProp);
+        // validate additional element properties & constraints
+        $this->validateElement($element, $matches, $schema, $path, $properties, $additionalProp);
     }
 
     public function validatePatternProperties($element, JsonPointer $path = null, $patternProperties)
@@ -82,18 +84,20 @@ public function validatePatternProperties($element, JsonPointer $path = null, $p
     /**
      * Validates the element properties
      *
-     * @param \stdClass        $element          Element to validate
-     * @param array            $matches          Matches from patternProperties (if any)
-     * @param \stdClass        $objectDefinition ObjectConstraint definition
-     * @param JsonPointer|null $path             Path to test?
-     * @param mixed            $additionalProp   Additional properties
+     * @param \StdClass        $element        Element to validate
+     * @param array            $matches        Matches from patternProperties (if any)
+     * @param \StdClass        $schema         ObjectConstraint definition
+     * @param JsonPointer|null $path           Current test path
+     * @param \StdClass        $properties     Properties
+     * @param mixed            $additionalProp Additional properties
      */
-    public function validateElement($element, $matches, $objectDefinition = null, JsonPointer $path = null, $additionalProp = null)
+    public function validateElement($element, $matches, $schema = null, JsonPointer $path = null,
+        $properties = null, $additionalProp = null)
     {
-        $this->validateMinMaxConstraint($element, $objectDefinition, $path);
+        $this->validateMinMaxConstraint($element, $schema, $path);
 
         foreach ($element as $i => $value) {
-            $definition = $this->getProperty($objectDefinition, $i);
+            $definition = $this->getProperty($properties, $i);
 
             // no additional properties allowed
             if (!in_array($i, $matches) && $additionalProp === false && $this->inlineSchemaProperty !== $i && !$definition) {
@@ -128,17 +132,17 @@ public function validateElement($element, $matches, $objectDefinition = null, Js
     /**
      * Validates the definition properties
      *
-     * @param \stdClass        $element          Element to validate
-     * @param \stdClass        $objectDefinition ObjectConstraint definition
-     * @param JsonPointer|null $path             Path?
+     * @param \stdClass        $element    Element to validate
+     * @param \stdClass        $properties Property definitions
+     * @param JsonPointer|null $path       Path?
      */
-    public function validateDefinition(&$element, $objectDefinition = null, JsonPointer $path = null)
+    public function validateProperties(&$element, $properties = null, JsonPointer $path = null)
     {
         $undefinedConstraint = $this->factory->createInstanceFor('undefined');
 
-        foreach ($objectDefinition as $i => $value) {
+        foreach ($properties as $i => $value) {
             $property = &$this->getProperty($element, $i, $undefinedConstraint);
-            $definition = $this->getProperty($objectDefinition, $i);
+            $definition = $this->getProperty($properties, $i);
 
             if (is_object($definition)) {
                 // Undefined constraint will check for is_object() and quit if is not - so why pass it?

src/JsonSchema/Constraints/UndefinedConstraint.php

@@ -73,8 +73,9 @@ public function validateTypes(&$value, $schema = null, JsonPointer $path, $i = n
                                                 // is not set (i.e. don't use $this->getTypeCheck() here).
             $this->checkObject(
                 $value,
-                isset($schema->properties) ? $this->factory->getSchemaStorage()->resolveRefSchema($schema->properties) : $schema,
+                $schema,
                 $path,
+                isset($schema->properties) ? $schema->properties : null,
                 isset($schema->additionalProperties) ? $schema->additionalProperties : null,
                 isset($schema->patternProperties) ? $schema->patternProperties : null,
                 $this->appliedDefaults