Fix failing tests in Zend\InputFilter

- Cleaned up BaseInputFilterTest to use a data provider; makes isolating failing cases simpler. - Rewrote logic in validateInputs() to capture discrete use cases; more verbose, but easier to determine what is happening. - Made executive decision: non-required input that is empty is valid.
zendframework · Mar 26, 2013 · ef30a66 · ef30a66
1 parent a4055c6
commit ef30a66
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 25 deletions.
diff --git a/src/BaseInputFilter.php b/src/BaseInputFilter.php
@@ -169,37 +169,96 @@ protected function validateInputs(array $inputs)
         $valid               = true;
 
         foreach ($inputs as $name) {
-            $input = $this->inputs[$name];
-            // Check for missing data on non-required input
-            if ((!array_key_exists($name, $this->data)
-                    || (null === $this->data[$name]))
+            $input      = $this->inputs[$name];
+            $dataExists = array_key_exists($name, $this->data);
+
+            // key doesn't exist, but input is not required; valid
+            if (!$dataExists
                 && $input instanceof InputInterface
                 && !$input->isRequired()
             ) {
                 $this->validInputs[$name] = $input;
                 continue;
             }
-            if (!array_key_exists($name, $this->data)
-                || (null === $this->data[$name])
-                || (is_string($this->data[$name]) && strlen($this->data[$name]) === 0)
-                // Single and Multi File Uploads
-                || (is_array($this->data[$name])
-                    && isset($this->data[$name]['error']) && $this->data[$name]['error'] === UPLOAD_ERR_NO_FILE)
-                || (is_array($this->data[$name]) && count($this->data[$name]) === 1
-                    && isset($this->data[$name][0]) && is_array($this->data[$name][0])
-                    && isset($this->data[$name][0]['error']) && $this->data[$name][0]['error'] === UPLOAD_ERR_NO_FILE)
+
+            // key doesn't exist, input is required, allows empty; valid
+            if (!$dataExists
+                && $input instanceof InputInterface
+                && $input->isRequired()
+                && $input->allowEmpty()
             ) {
-                if ($input instanceof InputInterface) {
-                    // - test if input allows empty
-                    if ($input->allowEmpty()) {
-                        $this->validInputs[$name] = $input;
-                        continue;
-                    }
-                }
-                // make sure we have a value (empty) for validation
+                $this->validInputs[$name] = $input;
+                continue;
+            }
+
+            // key exists, is null, input is not required; valid
+            if ($dataExists
+                && null === $this->data[$name]
+                && $input instanceof InputInterface
+                && !$input->isRequired()
+            ) {
+                $this->validInputs[$name] = $input;
+                continue;
+            }
+
+            // key exists, is null, input is required, allows empty; valid
+            if ($dataExists
+                && null === $this->data[$name]
+                && $input instanceof InputInterface
+                && $input->isRequired()
+                && $input->allowEmpty()
+            ) {
+                $this->validInputs[$name] = $input;
+                continue;
+            }
+
+            // key exists, empty string, input is not required, allows empty; valid
+            if ($dataExists
+                && '' === $this->data[$name]
+                && $input instanceof InputInterface
+                && !$input->isRequired()
+            ) {
+                $this->validInputs[$name] = $input;
+                continue;
+            }
+
+            // key exists, empty string, input is required, allows empty; valid
+            if ($dataExists
+                && '' === $this->data[$name]
+                && $input instanceof InputInterface
+                && $input->isRequired()
+                && $input->allowEmpty()
+            ) {
+                $this->validInputs[$name] = $input;
+                continue;
+            }
+
+            // key exists, is array representing file, no file present, input not
+            // required or allows empty; valid
+            if ($dataExists
+                && is_array($this->data[$name])
+                && (
+                    (isset($this->data[$name]['error'])
+                        && $this->data[$name]['error'] === UPLOAD_ERR_NO_FILE)
+                    || (count($this->data[$name]) === 1
+                        && isset($this->data[$name][0])
+                        && is_array($this->data[$name][0])
+                        && isset($this->data[$name][0]['error'])
+                        && $this->data[$name][0]['error'] === UPLOAD_ERR_NO_FILE)
+                )
+                && $input instanceof InputInterface
+                && (!$input->isRequired() || $input->allowEmpty())
+            ) {
+                $this->validInputs[$name] = $input;
+                continue;
+            }
+
+            // make sure we have a value (empty) for validation
+            if (!$dataExists) {
                 $this->data[$name] = null;
             }
 
+            // Validate an input filter
             if ($input instanceof InputFilterInterface) {
                 if (!$input->isValid()) {
                     $this->invalidInputs[$name] = $input;
@@ -209,6 +268,8 @@ protected function validateInputs(array $inputs)
                 $this->validInputs[$name] = $input;
                 continue;
             }
+
+            // Validate an input
             if ($input instanceof InputInterface) {
                 if (!$input->isValid($this->data)) {
                     // Validation failure

diff --git a/test/BaseInputFilterTest.php b/test/BaseInputFilterTest.php
@@ -156,11 +156,11 @@ public function dataSets()
                 array(
                     'foo' => ' bazbat ',
                     'bar' => '12345',
-                    'baz' => '',
+                    'baz' => 'thisistoolong',
                     'nest' => array(
                         'foo' => ' bazbat ',
                         'bar' => '12345',
-                        'baz' => '',
+                        'baz' => 'thisistoolong',
                     ),
                 ),
                 false,
@@ -184,6 +184,7 @@ public function dataSets()
 
     /**
      * @dataProvider dataSets
+     * @group fmlife
      */
     public function testCanValidateEntireDataset($dataset, $expected)
     {

diff --git a/test/CollectionInputFilterTest.php b/test/CollectionInputFilterTest.php
@@ -42,7 +42,6 @@ public function getBaseInputFilter()
 
         $baz = new Input();
         $baz->setRequired(false);
-        $baz->setAllowEmpty(true);
         $baz->getFilterChain()->attachByName('stringtrim');
         $baz->getValidatorChain()->attach(new Validator\StringLength(1, 6));
 
@@ -69,7 +68,6 @@ public function getChildInputFilter()
 
         $baz = new Input();
         $baz->setRequired(false);
-        $baz->setAllowEmpty(true);
         $baz->getFilterChain()->attachByName('stringtrim');
         $baz->getValidatorChain()->attach(new Validator\StringLength(1, 6));