zendframework / zend-inputfilter

InputFilter component from Zend Framework
BSD 3-Clause "New" or "Revised" License
64 stars 50 forks source link

Ensure `input_filters` config is honored in non-zend-mvc contexts #137

Closed weierophinney closed 7 years ago

weierophinney commented 7 years ago

Per https://discourse.zendframework.com/t/validatormanager-not-calling-custom-validator-factory/109/5?u=matthew the input_filters config key is not honored currently unless the application is within a zend-mvc context. This is due to the fact that Zend\InputFilter\Module wires configuration for the Zend\ModuleManager\Listener\ServiceListener in order to push merged service configuration into the plugin during bootstrap; no similar logic is available when not in a zend-mvc context, however.

This patch fixes that situation by modifying the InputFilterPluginManagerFactory to do the following:

weierophinney commented 7 years ago

Failing tests are against latest target, and are due to PHPUnit version mismatch issues; failures only affect InputFilterPluginManagerCompatibilityTest; I've run the new tests manually with latest dependencies, and they pass as expected.

svycka commented 7 years ago

@weierophinney does this mean that we no longer require to write our own factory for zend-expressive? also do you plan to add this to other plugin managers like validator, filter, view, controllers and others?

froschdesign commented 7 years ago

@svycka

also do you plan to add this to other plugin managers like validator, filter, view, controllers and others?

Already done:

zendframework/zend-filter/pull/56 https://github.com/zendframework/zend-form/pull/164 https://github.com/zendframework/zend-validator/pull/168 https://github.com/zendframework/zend-log/pull/74 https://github.com/zendframework/zend-i18n/pull/74 https://github.com/zendframework/zend-hydrator/pull/59

svycka commented 7 years ago

oh nice didn't see this

mtymek commented 7 years ago

I would also add the factory for Zend\InputFilter\Factory that injects filter and validator plugin managers to appropriate chain.

Here's how I implemented it in my library:

use Interop\Container\ContainerInterface;
use Zend\Filter\FilterPluginManager;
use Zend\InputFilter\Factory;
use Zend\InputFilter\InputFilterPluginManager;
use Zend\Validator\ValidatorPluginManager;

class InputFilterFactoryFactory
{
    public function __invoke(ContainerInterface $container)
    {
        $factory = new Factory($container->get(InputFilterPluginManager::class));
        $factory->getDefaultFilterChain()->setPluginManager($container->get(FilterPluginManager::class));
        $factory->getDefaultValidatorChain()->setPluginManager($container->get(ValidatorPluginManager::class));

        return $factory;
    }
}

This is required to configure populate input filter using add method with array definition:

class ContactInputFilter extends InputFilter
{
    public function __construct()
    {
        $this->add(
            [
                'name' => 'first_name',
                'filters' => [
                    ['name' => 'Foo'],
                ],
                'validators' => [
                    ['name' => 'Bar'],
                ],
            ]
        );

        $this->add(
            [
                'name' => 'content',
                'filters' => [
                    ['name' => 'StringTrim'],
                ],
            ]
        );
    }
}
svycka commented 7 years ago

@mtymek are you sure this is required? I don't remember doing this. Maybe you not added filter and validators config providers to config?

froschdesign commented 7 years ago

@mtymek Do you mean these code lines:

mtymek commented 7 years ago

@froschdesign lines you mentioned will only work under zend-mvc, because FQCN is not used when pulling plugin manager for the container:

$factory->getDefaultFilterChain()->setPluginManager($container->get('FilterManager'));

My code uses FilterPluginManager::class instead of just 'FilterManager'. Do you see the nuance? ;-)

svycka commented 7 years ago

@mtymek then I think this should be solved here https://github.com/zendframework/zend-filter/blob/master/src/ConfigProvider.php#L33 by adding FilterPluginManager::class and making 'FilterManager' as alias. This would solve this no?

froschdesign commented 7 years ago

@mtymek

lines you mentioned will only work under zend-mvc

…or you have registered already these managers.

mtymek commented 7 years ago

@svycka indeed, this could work as well. Maybe it would be enough to add the aliases to ConfigProvider class?

@froschdesign I don't understand your remark. Can you elaborate?

weierophinney commented 7 years ago

@mtymek — regarding this:

$factory->getDefaultFilterChain()->setPluginManager($container->get(FilterPluginManager::class));
$factory->getDefaultValidatorChain()->setPluginManager($container->get(ValidatorPluginManager::class));

I'm not sure why you need that at all. zend-filter defines the FilterManager service in its ConfigProvider, and zend-validator defines ValidatorManager in its own. InputFilterPluginManager pulls these two services when populating the Factory instance with the default plugin managers. Your changes above are using service names we do not define in the existing ConfigProvider definitions, which means they shouldn't work at all... unless your module is defining them.

I agree that we should likely add the FQCN as the service name pointing to the factory, and the shorter names as aliases to those, but after the proposed changes in this PR and the new tags I've created on the other components, it should all work seamlessly.

mtymek commented 7 years ago

Right :) The reason you see it is that is that I working on my lib before ConfigProvider was available in zend-inputfilter. Sorry for adding more confusion!

weierophinney commented 7 years ago

@mtymek No worries! Glad it's all sorted!