Merge branch 'hotfix/155-inputfilter-precedence'

Close #156 Fixes #155
zendframework · Dec 4, 2017 · 178d21a · 178d21a
2 parents afd3662 + a6ba46a
commit 178d21a
Show file tree

Hide file tree

Showing 6 changed files with 188 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,33 @@
 
 All notable changes to this project will be documented in this file, in reverse chronological order by release.
 
+## 2.7.6 - 2017-12-04
+
+### Added
+
+- Nothing.
+
+### Changed
+
+- Nothing.
+
+### Deprecated
+
+- Nothing.
+
+### Removed
+
+- Nothing.
+
+### Fixed
+
+- [#156](https://github.com/zendframework/zend-inputfilter/pull/156) fixes an
+  issue introduced in 2.7.5 whereby the filter and validator chains composed in
+  inputs pulled from the `InputFilterPluginManager` were not receiving the
+  default filter and validator plugin manager instances. A solution was created
+  that preserves the original behavior as well as the bugfix that created the
+  regression.
+
 ## 2.7.5 - 2017-11-07
 
 ### Added

diff --git a/src/CollectionInputFilter.php b/src/CollectionInputFilter.php
@@ -166,8 +166,9 @@ public function setData($data)
 
     /**
      * {@inheritdoc}
+     * @param mixed $context Ignored, but present to retain signature compatibility.
      */
-    public function isValid()
+    public function isValid($context = null)
     {
         $this->collectionMessages = [];
         $inputFilter = $this->getInputFilter();

diff --git a/src/Factory.php b/src/Factory.php
@@ -189,12 +189,9 @@ public function createInput($inputSpecification)
             ));
         }
 
-        if (! $managerInstance && $this->defaultFilterChain) {
-            $input->setFilterChain(clone $this->defaultFilterChain);
-        }
-        if (! $managerInstance && $this->defaultValidatorChain) {
-            $input->setValidatorChain(clone $this->defaultValidatorChain);
-        }
+        $managerInstance
+            ? $this->injectFilterAndValidatorChainsWithPluginManagers($input)
+            : $this->injectDefaultFilterAndValidatorChains($input);
 
         foreach ($inputSpecification as $key => $value) {
             switch ($key) {
@@ -434,4 +431,47 @@ protected function populateValidators(ValidatorChain $chain, $validators)
             );
         }
     }
+
+    /**
+     * Inject the default filter and validator chains into the input, if present.
+     *
+     * This ensures custom plugins are made available to the input instance.
+     *
+     * @param InputInterface $input
+     * @return void
+     */
+    protected function injectDefaultFilterAndValidatorChains(InputInterface $input)
+    {
+        if ($this->defaultFilterChain) {
+            $input->setFilterChain(clone $this->defaultFilterChain);
+        }
+
+        if ($this->defaultValidatorChain) {
+            $input->setValidatorChain(clone $this->defaultValidatorChain);
+        }
+    }
+
+    /**
+     * Inject filter and validator chains with the plugin managers from
+     * the default chains, if present.
+     *
+     * This ensures custom plugins are made available to the input instance.
+     *
+     * @param InputInterface $input
+     * @return void
+     */
+    protected function injectFilterAndValidatorChainsWithPluginManagers(InputInterface $input)
+    {
+        if ($this->defaultFilterChain) {
+            $input->getFilterChain()
+                ? $input->getFilterChain()->setPluginManager($this->defaultFilterChain->getPluginManager())
+                : $input->setFilterChain(clone $this->defaultFilterChain);
+        }
+
+        if ($this->defaultValidatorChain) {
+            $input->getValidatorChain()
+                ? $input->getValidatorChain()->setPluginManager($this->defaultValidatorChain->getPluginManager())
+                : $input->setValidatorChain(clone $this->defaultValidatorChain);
+        }
+    }
 }
diff --git a/test/CollectionInputFilterTest.php b/test/CollectionInputFilterTest.php
@@ -19,7 +19,7 @@
 use Zend\InputFilter\Exception\RuntimeException;
 use Zend\InputFilter\Input;
 use Zend\InputFilter\InputFilter;
-use Zend\Validator\EmailAddress;
+use Zend\Validator\Digits;
 use Zend\Validator\NotEmpty;
 
 /**
@@ -512,10 +512,10 @@ public function testCollectionValidationDoesNotReuseMessagesBetweenInputs()
     {
         $inputFilter = new InputFilter();
         $inputFilter->add([
-            'name' => 'email',
+            'name' => 'phone',
             'required' => true,
             'validators' => [
-                ['name' => EmailAddress::class],
+                ['name' => Digits::class],
                 ['name' => NotEmpty::class],
             ],
         ]);
@@ -535,7 +535,7 @@ public function testCollectionValidationDoesNotReuseMessagesBetweenInputs()
                 'name' => 'Tom',
             ],
             [
-                'email' => 'tom@tom',
+                'phone' => 'tom@tom',
                 'name' => 'Tom',
             ],
         ]);
@@ -547,27 +547,25 @@ public function testCollectionValidationDoesNotReuseMessagesBetweenInputs()
         $this->assertFalse($isValid);
         $this->assertCount(2, $messages);
 
-        $this->assertArrayHasKey('email', $messages[0]);
-        $this->assertCount(1, $messages[0]['email']);
-        $this->assertContains('Value is required and can\'t be empty', $messages[0]['email']);
+        $this->assertArrayHasKey('phone', $messages[0]);
+        $this->assertCount(1, $messages[0]['phone']);
+        $this->assertContains('Value is required and can\'t be empty', $messages[0]['phone']);
 
-        $this->assertArrayHasKey('email', $messages[1]);
-        $this->assertCount(3, $messages[1]['email']);
-        $this->assertNotContains('Value is required and can\'t be empty', $messages[1]['email']);
-        $this->assertContains('\'tom\' is not a valid hostname for the email address', $messages[1]['email']);
-        $this->assertContains('The input does not match the expected structure for a DNS hostname', $messages[1]['email']);
-        $this->assertContains('The input appears to be a local network name but local network names are not allowed', $messages[1]['email']);
+        $this->assertArrayHasKey('phone', $messages[1]);
+        $this->assertCount(1, $messages[1]['phone']);
+        $this->assertNotContains('Value is required and can\'t be empty', $messages[1]['phone']);
+        $this->assertContains('The input must contain only digits', $messages[1]['phone']);
         // @codingStandardsIgnoreEnd
     }
 
     public function testCollectionValidationUsesCustomInputErrorMessages()
     {
         $inputFilter = new InputFilter();
         $inputFilter->add([
-            'name' => 'email',
+            'name' => 'phone',
             'required' => true,
             'validators' => [
-                ['name' => EmailAddress::class],
+                ['name' => Digits::class],
                 ['name' => NotEmpty::class],
             ],
             'error_message' => 'CUSTOM ERROR MESSAGE',
@@ -588,7 +586,7 @@ public function testCollectionValidationUsesCustomInputErrorMessages()
                 'name' => 'Tom',
             ],
             [
-                'email' => 'tom@tom',
+                'phone' => 'tom@tom',
                 'name' => 'Tom',
             ],
         ]);
@@ -600,14 +598,14 @@ public function testCollectionValidationUsesCustomInputErrorMessages()
         $this->assertFalse($isValid);
         $this->assertCount(2, $messages);
 
-        $this->assertArrayHasKey('email', $messages[0]);
-        $this->assertCount(1, $messages[0]['email']);
-        $this->assertContains('CUSTOM ERROR MESSAGE', $messages[0]['email']);
-        $this->assertNotContains('Value is required and can\'t be empty', $messages[0]['email']);
+        $this->assertArrayHasKey('phone', $messages[0]);
+        $this->assertCount(1, $messages[0]['phone']);
+        $this->assertContains('CUSTOM ERROR MESSAGE', $messages[0]['phone']);
+        $this->assertNotContains('Value is required and can\'t be empty', $messages[0]['phone']);
 
-        $this->assertArrayHasKey('email', $messages[1]);
-        $this->assertCount(1, $messages[1]['email']);
-        $this->assertContains('CUSTOM ERROR MESSAGE', $messages[1]['email']);
+        $this->assertArrayHasKey('phone', $messages[1]);
+        $this->assertCount(1, $messages[1]['phone']);
+        $this->assertContains('CUSTOM ERROR MESSAGE', $messages[1]['phone']);
     }
 
     public function testDuplicatedErrorMessages()

diff --git a/test/FactoryTest.php b/test/FactoryTest.php
@@ -13,6 +13,7 @@
 use PHPUnit_Framework_MockObject_MockObject as MockObject;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
+use ReflectionProperty;
 use Zend\Filter;
 use Zend\InputFilter\CollectionInputFilter;
 use Zend\InputFilter\Exception\InvalidArgumentException;
@@ -999,9 +1000,11 @@ public function testClearDefaultValidatorChain()
 
     /**
      * @see https://github.com/zendframework/zend-inputfilter/issues/8
+     * @see https://github.com/zendframework/zend-inputfilter/issues/155
      */
     public function testWhenCreateInputPullsInputFromThePluginManagerItMustNotOverwriteFilterAndValidatorChains()
     {
+
         $input = $this->prophesize(InputInterface::class);
         $input->setFilterChain(Argument::any())->shouldNotBeCalled();
         $input->setValidatorChain(Argument::any())->shouldNotBeCalled();
@@ -1015,6 +1018,23 @@ public function testWhenCreateInputPullsInputFromThePluginManagerItMustNotOverwr
 
         $factory = new Factory($pluginManager->reveal());
 
+        $r = new ReflectionProperty($factory, 'defaultFilterChain');
+        $r->setAccessible(true);
+        $defaultFilterChain = $r->getValue($factory);
+
+        $filterChain = $this->prophesize(Filter\FilterChain::class);
+        $filterChain->setPluginManager($defaultFilterChain->getPluginManager())->shouldBeCalled();
+
+        $r = new ReflectionProperty($factory, 'defaultValidatorChain');
+        $r->setAccessible(true);
+        $defaultValidatorChain = $r->getValue($factory);
+
+        $validatorChain = $this->prophesize(Validator\ValidatorChain::class);
+        $validatorChain->setPluginManager($defaultValidatorChain->getPluginManager())->shouldBeCalled();
+
+        $input->getFilterChain()->will([$filterChain, 'reveal'])->shouldBeCalled();
+        $input->getValidatorChain()->will([$validatorChain, 'reveal'])->shouldBeCalled();
+
         $this->assertSame($input->reveal(), $factory->createInput($spec));
     }
 

diff --git a/test/InputFilterAbstractServiceFactoryTest.php b/test/InputFilterAbstractServiceFactoryTest.php
@@ -12,11 +12,14 @@
 
 use PHPUnit_Framework_MockObject_MockObject as MockObject;
 use PHPUnit\Framework\TestCase;
+use Zend\Filter;
 use Zend\Filter\FilterPluginManager;
+use Zend\InputFilter\FileInput;
 use Zend\InputFilter\InputFilterAbstractServiceFactory;
 use Zend\InputFilter\InputFilterInterface;
 use Zend\InputFilter\InputFilterPluginManager;
 use Zend\ServiceManager\ServiceManager;
+use Zend\Validator;
 use Zend\Validator\ValidatorInterface;
 use Zend\Validator\ValidatorPluginManager;
 
@@ -301,4 +304,73 @@ public function testAllowsPassingNonPluginManagerContainerToFactoryWithServiceMa
         $inputFilter = call_user_func_array([$this->factory, $create], $args);
         $this->assertInstanceOf(InputFilterInterface::class, $inputFilter);
     }
+
+    /**
+     * @see https://github.com/zendframework/zend-inputfilter/issues/155
+     */
+    public function testWillUseCustomFiltersWhenProvided()
+    {
+        $filter = $this->prophesize(Filter\FilterInterface::class)->reveal();
+
+        $filters = new FilterPluginManager($this->services);
+        $filters->setService('CustomFilter', $filter);
+
+        $validators = new ValidatorPluginManager($this->services);
+
+        $this->services->setService('FilterManager', $filters);
+        $this->services->setService('ValidatorManager', $validators);
+
+        $this->services->setService('config', [
+            'input_filter_specs' => [
+                'test' => [
+                    [
+                        'name' => 'a-file-element',
+                        'type' => FileInput::class,
+                        'required' => true,
+                        'validators' => [
+                            [
+                                'name' => Validator\File\UploadFile::class,
+                                'options' => [
+                                    'breakchainonfailure' => true,
+                                ],
+                            ],
+                            [
+                                'name' => Validator\File\Size::class,
+                                'options' => [
+                                    'breakchainonfailure' => true,
+                                    'max' => '6GB',
+                                ],
+                            ],
+                            [
+                                'name' => Validator\File\Extension::class,
+                                'options' => [
+                                    'breakchainonfailure' => true,
+                                    'extension' => 'csv,zip',
+                                ],
+                            ],
+                        ],
+                        'filters' => [
+                            ['name' => 'CustomFilter'],
+                        ],
+                    ],
+                ],
+            ],
+        ]);
+
+        $this->services->get('InputFilterManager')
+            ->addAbstractFactory(InputFilterAbstractServiceFactory::class);
+
+        $inputFilter = $this->services->get('InputFilterManager')->get('test');
+        $this->assertInstanceOf(InputFilterInterface::class, $inputFilter);
+
+        $input = $inputFilter->get('a-file-element');
+        $this->assertInstanceOf(FileInput::class, $input);
+
+        $filters = $input->getFilterChain();
+        $this->assertCount(1, $filters);
+
+        $callback = $filters->getFilters()->top();
+        $this->assertInternalType('array', $callback);
+        $this->assertSame($filter, $callback[0]);
+    }
 }