Input fallback feature should work when required and optional

zendframework · Sep 1, 2015 · 44cd418 · 44cd418
1 parent 152ee5b
commit 44cd418
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 20 deletions.
diff --git a/src/ArrayInput.php b/src/ArrayInput.php
@@ -64,16 +64,16 @@ public function isValid($context = null)
         $required = $this->isRequired();
         $hasFallback = $this->hasFallback();
 
-        if (! $hasValue && $required && !$hasFallback) {
-            $this->setErrorMessage('Value is required');
-            return false;
-        }
-
-        if (! $hasValue && $required && $hasFallback) {
+        if (! $hasValue && $hasFallback) {
             $this->setValue($this->getFallbackValue());
             return true;
         }
 
+        if (! $hasValue && $required) {
+            $this->setErrorMessage('Value is required');
+            return false;
+        }
+
         if (!$this->continueIfEmpty() && !$this->allowEmpty()) {
             $this->injectNotEmptyValidator();
         }
@@ -90,7 +90,7 @@ public function isValid($context = null)
             if (!$result) {
                 if ($hasFallback) {
                     $this->setValue($this->getFallbackValue());
-                    $result = true;
+                    return true;
                 }
                 break;
             }

diff --git a/src/Input.php b/src/Input.php
@@ -377,16 +377,16 @@ public function isValid($context = null)
         $allowEmpty      = $this->allowEmpty();
         $continueIfEmpty = $this->continueIfEmpty();
 
-        if (! $hasValue && $required && ! $this->hasFallback()) {
-            $this->setErrorMessage('Value is required');
-            return false;
-        }
-
-        if (! $hasValue && $required && $this->hasFallback()) {
+        if (! $hasValue && $this->hasFallback()) {
             $this->setValue($this->getFallbackValue());
             return true;
         }
 
+        if (! $hasValue && $required) {
+            $this->setErrorMessage('Value is required');
+            return false;
+        }
+
         if ($empty && ! $required && ! $continueIfEmpty) {
             return true;
         }

diff --git a/test/ArrayInputTest.php b/test/ArrayInputTest.php
@@ -218,9 +218,9 @@ public function fallbackValueVsIsValidProvider()
     {
         $dataSets = parent::fallbackValueVsIsValidProvider();
         array_walk($dataSets, function (&$set) {
-            $set[0] = [$set[0]]; // Wrap fallback value into an array.
-            $set[1] = [$set[1]]; // Wrap value into an array.
-            $set[3] = [$set[3]]; // Wrap expected value into an array.
+            $set[1] = [$set[1]]; // Wrap fallback value into an array.
+            $set[2] = [$set[2]]; // Wrap value into an array.
+            $set[4] = [$set[4]]; // Wrap expected value into an array.
         });
 
         return $dataSets;

diff --git a/test/FileInputTest.php b/test/FileInputTest.php
@@ -312,6 +312,7 @@ public function testRequiredNotEmptyValidatorNotAddedWhenOneExists()
     }
 
     public function testFallbackValueVsIsValidRules(
+        $required = null,
         $fallbackValue = null,
         $originalValue = null,
         $isValid = null,
@@ -322,6 +323,7 @@ public function testFallbackValueVsIsValidRules(
 
 
     public function testFallbackValueVsIsValidRulesWhenValueNotSet(
+        $required = null,
         $fallbackValue = null,
         $originalValue = null,
         $isValid = null,

diff --git a/test/InputTest.php b/test/InputTest.php
@@ -115,11 +115,12 @@ public function testSetFallbackValue($fallbackValue)
     /**
      * @dataProvider fallbackValueVsIsValidProvider
      */
-    public function testFallbackValueVsIsValidRules($fallbackValue, $originalValue, $isValid, $expectedValue)
+    public function testFallbackValueVsIsValidRules($required, $fallbackValue, $originalValue, $isValid, $expectedValue)
     {
         $input = $this->input;
         $input->setContinueIfEmpty(true);
 
+        $input->setRequired($required);
         $input->setValidatorChain($this->createValidatorChainMock($isValid));
         $input->setFallbackValue($fallbackValue);
         $input->setValue($originalValue);
@@ -137,13 +138,14 @@ public function testFallbackValueVsIsValidRules($fallbackValue, $originalValue,
     /**
      * @dataProvider fallbackValueVsIsValidProvider
      */
-    public function testFallbackValueVsIsValidRulesWhenValueNotSet($fallbackValue, $originalValue, $isValid)
+    public function testFallbackValueVsIsValidRulesWhenValueNotSet($required, $fallbackValue, $originalValue, $isValid)
     {
         $expectedValue = $fallbackValue; // Should always return the fallback value
 
         $input = $this->input;
         $input->setContinueIfEmpty(true);
 
+        $input->setRequired($required);
         $input->setValidatorChain($this->createValidatorChainMock($isValid));
         $input->setFallbackValue($fallbackValue);
 
@@ -931,6 +933,7 @@ public function testResetValueReturnsInputValueToDefaultValue($value)
 
     public function fallbackValueVsIsValidProvider()
     {
+        $required = true;
         $isValid = true;
 
         $originalValue = 'fooValue';
@@ -939,8 +942,10 @@ public function fallbackValueVsIsValidProvider()
         // @codingStandardsIgnoreStart
         return [
             // Description => [$inputIsRequired, $fallbackValue, $originalValue, $isValid, $expectedValue]
-            'Input: Invalid. getValue: fallback' => [$fallbackValue, $originalValue, !$isValid, $fallbackValue],
-            'Input: Valid. getValue: original' =>   [$fallbackValue, $originalValue,  $isValid, $originalValue],
+            'Required: T, Input: Invalid. getValue: fallback' => [ $required, $fallbackValue, $originalValue, !$isValid, $fallbackValue],
+            'Required: T, Input: Valid. getValue: original' =>   [ $required, $fallbackValue, $originalValue,  $isValid, $originalValue],
+            'Required: F, Input: Invalid. getValue: fallback' => [!$required, $fallbackValue, $originalValue, !$isValid, $fallbackValue],
+            'Required: F, Input: Valid. getValue: original' =>   [!$required, $fallbackValue, $originalValue,  $isValid, $originalValue],
         ];
         // @codingStandardsIgnoreEnd
     }