Update the LIVR test suite and fix the bugs it finds

lib/Validator/LIVR.php

@@ -65,7 +65,8 @@ class LIVR
 
     public static function registerDefaultRules($rules)
     {
-        self::$DEFAULT_RULES = self::$DEFAULT_RULES + $rules;
+        // Rules with the names of registered ones replace them
+        self::$DEFAULT_RULES = array_merge(self::$DEFAULT_RULES, $rules);
         return;
     }
 
@@ -108,7 +109,8 @@ public function prepare()
         }
 
         foreach ($this->livrRules as $field => $fieldRules) {
-            if (!is_array($fieldRules) || \Validator\LIVR\Util::isAssocArray($fieldRules)) {
+            // An empty array is an empty list of rules
+            if (!is_array($fieldRules) || !\Validator\LIVR\Util::isList($fieldRules)) {
                 $fieldRules = array($fieldRules);
             }
 
@@ -276,7 +278,8 @@ private function autoTrim($data)
     {
         if (is_string($data)) {
             return trim($data);
-        } elseif (\Validator\LIVR\Util::isAssocArray($data)) {
+        } elseif (is_array($data)) {
+            // Objects and lists
             $trimmedData = array();
             foreach ($data as $key => $value) {
                 $trimmedData[$key]  = $this->autoTrim($value);

lib/Validator/LIVR/Rules/Meta.php

@@ -33,7 +33,7 @@ public static function listOf()
     {
         $first_arg = func_get_arg(0);
 
-        if (is_array($first_arg) && !\Validator\LIVR\Util::isAssocArray($first_arg)) {
+        if (\Validator\LIVR\Util::isList($first_arg)) {
             $livr         = func_get_arg(0);
             $ruleBuilders = func_get_arg(1);
         } else {
@@ -60,8 +60,9 @@ public static function listOf()
             foreach ($values as $value) {
                 $result = $validator->validate(array('field' => $value));
 
-                if ($result) {
-                    $results[] = $result['field'];
+                // The result is empty without rules
+                if ($result !== false && $result !== null) {
+                    $results[] = isset($result['field']) ? $result['field'] : null;
                     $errors[]  = null;
                 } else {
                     $results[] = null;
@@ -131,20 +132,27 @@ public static function listOfDifferentObjects($selectorField, $livrs, $ruleBuild
         }
 
         return function ($objects, $params, &$outputArr) use ($validators, $selectorField) {
+            if (!isset($objects) || $objects === '') {
+                return;
+            }
+
+            if (!\Validator\LIVR\Util::isList($objects)) {
+                return 'FORMAT_ERROR';
+            }
+
             $results   = array();
             $errors    = array();
             $hasErrors = false;
 
             foreach ($objects as $object) {
-                if (!is_array($object)
-                    || !isset($object[$selectorField])
-                    || !isset($validators[$object[$selectorField]])
-                    || !$validators[$object[$selectorField]]) {
-                    $errors[] = 'FORMAT_ERROR';
+                $validator = self::selectValidator($validators, $object, $selectorField);
+
+                if (!$validator) {
+                    $errors[]  = 'FORMAT_ERROR';
+                    $hasErrors = true;
                     continue;
                 }
 
-                $validator = $validators[ $object[$selectorField] ];
                 $result = $validator->validate($object);
 
                 if ($result) {
@@ -180,13 +188,12 @@ public static function variableObject($selectorField, $livrs, $ruleBuilders)
                 return '';
             }
 
-            if (!is_array($object)
-                || !isset($object[$selectorField])
-                || !isset($validators[$object[$selectorField]])) {
+            $validator = self::selectValidator($validators, $object, $selectorField);
+
+            if (!$validator) {
                 return 'FORMAT_ERROR';
             }
 
-            $validator = $validators[ $object[$selectorField] ];
             $result = $validator->validate($object);
 
             if ($result !== false && $result !== null) {
@@ -200,15 +207,10 @@ public static function variableObject($selectorField, $livrs, $ruleBuilders)
 
     public static function __or()
     {
-        $first_arg = func_get_arg(0);
-
-        if (is_array($first_arg) && !\Validator\LIVR\Util::isAssocArray($first_arg)) {
-            $livrs        = func_get_arg(0);
-            $ruleBuilders = func_get_arg(1);
-        } else {
-            $livrs        = func_get_args();
-            $ruleBuilders = array_pop($livrs);
-        }
+        // Each argument is a set of rules. Unlike list_of, a list in the first argument
+        // is not unwrapped, as a set of rules can be a list: { or: [['email', 'to_lc'], 'positive_integer'] }
+        $livrs        = func_get_args();
+        $ruleBuilders = array_pop($livrs);
 
         $validators = array();
 
@@ -219,10 +221,7 @@ public static function __or()
         }
 
         return function ($value, $params, &$outputArr) use ($validators) {
-            if (!isset($value) || $value === '') {
-                return;
-            }
-
+            // Empty values go to the rule sets too, so "required" and "not_empty" in them work
             $lastError = null;
             foreach ($validators as $validator) {
                 $result = $validator->validate(array('field' => $value));
@@ -241,4 +240,23 @@ public static function __or()
             }
         };
     }
+
+    /**
+     * Returns the validator for the selector value of the object,
+     * or null if the object or its selector value is wrong
+     */
+    private static function selectValidator($validators, $object, $selectorField)
+    {
+        if (!is_array($object) || !isset($object[$selectorField])) {
+            return null;
+        }
+
+        // Only strings and integers can be array keys
+        $selectorValue = $object[$selectorField];
+        if (!is_string($selectorValue) && !is_int($selectorValue)) {
+            return null;
+        }
+
+        return isset($validators[$selectorValue]) ? $validators[$selectorValue] : null;
+    }
 }

lib/Validator/LIVR/Rules/Special.php

@@ -35,7 +35,10 @@ public static function equalToField($field)
                 return 'FORMAT_ERROR';
             }
 
-            if ($value != $params[$field]) {
+            // Compared as strings: "==" finds numeric strings like "10" and "1e1" equal
+            $fieldValue = isset($params[$field]) ? $params[$field] : null;
+
+            if (!\Validator\LIVR\Util::isStringOrNumber($fieldValue) || (string) $value !== (string) $fieldValue) {
                 return 'FIELDS_NOT_EQUAL';
             }
 

phpunit.xml.dist

@@ -10,6 +10,7 @@
             <file>tests/02-livr_test_suite_aliased_rules.php</file>
             <file>tests/03-auto_trim.php</file>
             <file>tests/04-custom_filters.php</file>
+            <file>tests/05-rules_edge_cases.php</file>
         </testsuite>
     </testsuites>
 

tests/03-auto_trim.php

@@ -9,6 +9,7 @@ class AutoTrimTest extends TestCase
     private $rules = array(
         'code'      =>  array('required'),
         'password'  =>  array('required', array( 'min_length' => 3 ) ),
+        'list'      =>  array('list_of' => array('string')),
         'address'   =>  array('nested_object' => array(
                             'street' => array( array( 'min_length'=> 5 ) )
                         )
@@ -22,6 +23,7 @@ public function testPositive()
         $input = array(
             'code'      => ' A ',
             'password'  => ' 123 ',
+            'list'      => array(' aaa ', ' bbb '),
             'address'   => array(
                 'street' => ' hello '
             )
@@ -30,6 +32,7 @@ public function testPositive()
         $output = array(
             'code'      => 'A',
             'password'  => '123',
+            'list'      => array('aaa', 'bbb'),
             'address'   => array(
                 'street' => 'hello'
             )

tests/05-rules_edge_cases.php

@@ -0,0 +1,67 @@
+<?php
+
+require 'vendor/autoload.php';
+
+use PHPUnit\Framework\TestCase;
+
+// The test suite has the other edge cases of these rules
+class RulesEdgeCasesTest extends TestCase
+{
+    public function testListOfDifferentObjectsWithNotList()
+    {
+        $validator = new Validator\LIVR(array(
+            'products' => array('list_of_different_objects' => array('product_type', array(
+                'material' => array('product_type' => 'required', 'material_id' => 'positive_integer'),
+            ))),
+        ));
+
+        $this->assertEquals(array(), $validator->validate(array()), 'Should skip a missing value');
+        $this->assertEquals(
+            array('products' => ''),
+            $validator->validate(array('products' => '')),
+            'Should skip an empty value'
+        );
+
+        foreach (array('not an array', 10, array('product_type' => 'material')) as $products) {
+            $this->assertFalse($validator->validate(array('products' => $products)), 'Should return false');
+            $this->assertEquals(array('products' => 'FORMAT_ERROR'), $validator->getErrors(), 'Should return FORMAT_ERROR');
+        }
+    }
+
+    public function testRegisterDefaultRulesReplacesRules()
+    {
+        $defaultRules = Validator\LIVR::getDefaultRules();
+
+        Validator\LIVR::registerDefaultRules(array(
+            'max_length' => function ($maxLength) use ($defaultRules) {
+                $maxLengthRule = call_user_func($defaultRules['max_length'], $maxLength);
+
+                return function ($value) use ($maxLengthRule) {
+                    return $maxLengthRule($value) ? 'MY_TOO_LONG' : null;
+                };
+            },
+        ));
+
+        try {
+            $validator = new Validator\LIVR(array('phone' => array('max_length' => 10)));
+            $this->assertFalse($validator->validate(array('phone' => '123456789123456')), 'Should return false');
+            $this->assertEquals(array('phone' => 'MY_TOO_LONG'), $validator->getErrors(), 'Should use the new rule');
+        } finally {
+            Validator\LIVR::registerDefaultRules(array('max_length' => $defaultRules['max_length']));
+        }
+    }
+
+    public function testEmptyListsOfRules()
+    {
+        $validator = new Validator\LIVR(array(
+            'no_rules' => array(),
+            'list'     => array('list_of' => array()),
+        ));
+
+        $this->assertEquals(
+            array('list' => array(null, null)),
+            $validator->validate(array('no_rules' => 'x', 'list' => array(1, 2))),
+            'Should skip fields without rules'
+        );
+    }
+}

tests/test_suite/aliases_negative/02-address/aliases.json

@@ -4,7 +4,7 @@
         "rules": { "nested_object":{
             "street": "required",
             "zip": ["required", "positive_integer"],
-            "city": {"one_of": ["Kiev", "Kharkiv"] }
+            "city": {"one_of": ["Kyiv", "Kharkiv"] }
         }}
     },
 
@@ -13,7 +13,7 @@
         "rules": { "nested_object":{
             "street": "required",
             "zip": ["required", "positive_integer"],
-            "city": {"one_of": ["Kiev", "Kharkiv"] }
+            "city": {"one_of": ["Kyiv", "Kharkiv"] }
         }},
         "error": "WRONG_ADDRESS"
     }

tests/test_suite/aliases_negative/02-address/input.json

@@ -2,12 +2,12 @@
     "address": {
         "street": "",
         "zip": -1232131,
-        "city": "Moscow"
+        "city": "Lviv"
     },
     "address_custom_error": {
         "street": "My street",
         "zip": "12312",
-        "city": "Moscow"
+        "city": "Lviv"
     },
     "extra_field": 12
 }

tests/test_suite/aliases_positive/01-adult_age/input.json

@@ -3,6 +3,7 @@
     "age2": 33,
     "age3": 35,
     "age4": 36,
+    "empty_age": "",
     "age1_custom_error": 37,
     "age2_custom_error": 38,
     "age3_custom_error": 100,

tests/test_suite/aliases_positive/01-adult_age/output.json

@@ -3,6 +3,7 @@
     "age2": 33,
     "age3": 35,
     "age4": 36,
+    "empty_age": "",
     "age1_custom_error": 37,
     "age2_custom_error": 38,
     "age3_custom_error": 100,

tests/test_suite/aliases_positive/01-adult_age/rules.json

@@ -3,6 +3,8 @@
     "age2": ["adult_age"],
     "age3": [ { "adult_age": [] } ],
     "age4": { "adult_age": [] },
+    "empty_age": "adult_age",
+    "missed_age": "adult_age",
     "age1_custom_error": "adult_age_with_custom_error",
     "age2_custom_error": ["adult_age_with_custom_error"],
     "age3_custom_error": [ { "adult_age_with_custom_error": [] } ],

tests/test_suite/aliases_positive/02-address/aliases.json

@@ -4,7 +4,7 @@
         "rules": { "nested_object":{
             "street": "required",
             "zip": ["required", "positive_integer"],
-            "city": {"one_of": ["Kiev", "Kharkiv"] }
+            "city": {"one_of": ["Kyiv", "Kharkiv"] }
         }}
     },
 
@@ -13,7 +13,7 @@
         "rules": { "nested_object":{
             "street": "required",
             "zip": ["required", "positive_integer"],
-            "city": {"one_of": ["Kiev", "Kharkiv"] }
+            "city": {"one_of": ["Kyiv", "Kharkiv"] }
         }},
         "error": "WRONG_ADDRESS"
     }

tests/test_suite/aliases_positive/02-address/input.json

@@ -2,7 +2,7 @@
     "address": {
         "street": "My street",
         "zip": 1232131,
-        "city": "Kiev"
+        "city": "Kyiv"
     },
     "address_custom_error": {
         "street": "My street",

tests/test_suite/aliases_positive/02-address/output.json

@@ -2,7 +2,7 @@
     "address": {
         "street": "My street",
         "zip": 1232131,
-        "city": "Kiev"
+        "city": "Kyiv"
     },
     "address_custom_error": {
         "street": "My street",

tests/test_suite/negative/01-required/errors.json

@@ -1,5 +1,8 @@
 {
     "first_name": "REQUIRED",
     "last_name": "REQUIRED",
-    "middle_name": "REQUIRED"
+    "middle_name": "REQUIRED",
+    "constructor": "REQUIRED",
+    "toString": "REQUIRED",
+    "hasOwnProperty": "REQUIRED"
 }

tests/test_suite/negative/01-required/input.json

@@ -2,6 +2,7 @@
     "first_name": "",
     "last_name": null,
     "age": "25",
+    "__proto__": { "middle_name": "Some" },
 
     "value_is_hash":        {"test": 1},
     "value_is_empty_hash":  {},

tests/test_suite/negative/01-required/rules.json

@@ -6,5 +6,9 @@
     "value_is_hash":        "required",
     "value_is_empty_hash":  "required",
     "value_is_array":       "required",
-    "value_is_empty_array": "required"
+    "value_is_empty_array": "required",
+
+    "constructor":    "required",
+    "toString":       "required",
+    "hasOwnProperty": "required"
 }