1

I've set up a structure using Abstract classes for Forms, Fieldsets and InputFilters. Forms and Fieldsets have Factories while InputFilters are created and set on the Fieldsets by the FieldsetFactory (uses the MutableCreationOptionsInterface to pass along options)

The problem I have is that InputFilters are loaded for the Form, but are not used to validate the data. All input is accepted as valid.

E.g. I have a Country Entity with a name property. The name of the Country must be at least 3 chars and max 255. When the name is "ab", it's found to be valid.

Before someone asks: no error is thrown, the data is just accepted as valid.

I've been breaking my head over this for the passed few days trying to find where I've made a mistake, but cannot find it.

Also, there's quite a bit of code. I've limited it to what I think is relevant, although: if you need more, there will be more. I've removed a lot of type checking, docblocks and type hints to limit code lines/reading time ;)

module.config.php

'form_elements' => [
    'factories' => [
        CountryForm::class => CountryFormFactory::class,
        CountryFieldset::class => CountryFieldsetFactory::class,
    ],
],

Country.php

class Country extends AbstractEntity // Creates $id + getter/setter
{
    /**
     * @var string
     * @ORM\Column(name="name", type="string", length=255, nullable=false)
     */
    protected $name;

    // Other properties
    // Getters/setters
}

CountryController.php - Extends the AbstractActionController

public function editAction() // Here to show the params used to call function
{
    return parent::editAction(
        Country::class,
        CountryForm::class,
        [
            'name' => 'country',
            'options' => []
        ],
        'id',
        'admin/countries/view',
        'admin/countries',
        ['id']
    );
}

AbstractActionController.php - (goes wrong in here) - Used by CountryController#editAction()

public function editAction (
    $emEntity,
    $formName,
    $formOptions,
    $idProperty,
    $route,
    $errorRoute, array
    $routeParams = []
) {
    //Check if form is set

    $id = $this->params()->fromRoute($idProperty, null);

    /** @var AbstractEntity $entity */
    $entity = $this->getEntityManager()->getRepository($emEntity)->find($id);

    /** @var AbstractForm $form */
    $form = $this->getFormElementManager()->get($formName, (is_null($formOptions) ? [] : $formOptions));
    $form->bind($entity);

    /** @var Request $request */
    $request = $this->getRequest();
    if ($request->isPost()) {
        $form->setData($request->getPost());

        if ($form->isValid()) { // HERE IS WHERE IT GOES WRONG -> ALL IS TRUE
            try {
                $this->getEntityManager()->flush();
            } catch (\Exception $e) {
                //Print errors & return (removed, unnecessary)
            }

            return $this->redirectToRoute($route, $this->getRouteParams($entity, $routeParams));
        }
    }

    return [
        'form' => $form,
        'validationMessages' => $form->getMessages() ?: '',
    ];
}

CountryForm.php

class CountryForm extends AbstractForm
{
    // This one added for SO, does nothing but call parent#__construct, which would happen anyway
    public function __construct($name = null, array $options)
    {
        parent::__construct($name, $options);
    }

    public function init()
    {
        //Call parent initializer.
        parent::init();

        $this->add([
            'name' => 'country',
            'type' => CountryFieldset::class,
            'options' => [
                'use_as_base_fieldset' => true,
            ],
        ]);
    }
}

CountryFormFactory.php

class CountryFormFactory extends AbstractFormFactory
{
    public function createService(ServiceLocatorInterface $serviceLocator)
    {
        $serviceManager = $serviceLocator->getServiceLocator();

        /** @var EntityManager $entityManager */
        $entityManager = $serviceManager->get('Doctrine\ORM\EntityManager');

        $form = new CountryForm($this->name, $this->options);
        $form->setObjectManager($entityManager);
        $form->setTranslator($serviceManager->get('translator'));

        return $form;
    }
}

AbstractFormFactory.php - Uses MutableCreationOptionsInterface to receive options from the Controller function call: $form = $this->getFormElementManager()->get($formName, (is_null($formOptions) ? [] : $formOptions))

abstract class AbstractFormFactory implements FactoryInterface, MutableCreationOptionsInterface
{
    protected $name;
    protected $options;

    /**
     * @param array $options
     */
    public function setCreationOptions(array $options)
    {
        // Check presence of required "name" (string) parameter in $options
        $this->name = $options['name'];

        // Check presence of required "options" (array) parameter in $options
        $this->options = $options['options'];
    }
}

CountryFieldset.php - Used above by CountryForm.php as the base fieldset

class CountryFieldset extends AbstractFieldset
{
    public function init()
    {
        parent::init();

        $this->add([
            'name' => 'name',
            'required' => true,
            'type' => Text::class,
            'options' => [
                'label' => _('Name'),
            ],
        ]);
        // Other properties
    }
}

AbstractFieldset.php

abstract class AbstractFieldset extends Fieldset
{
    use InputFilterAwareTrait;
    use TranslatorAwareTrait;

    protected $entityManager;

    public function __construct(EntityManager $entityManager, $name)
    {
        parent::__construct($name);

        $this->setEntityManager($entityManager);
    }

    public function init()
    {
        $this->add([
            'name' => 'id',
            'type' => Hidden::class,
        ]);
    }

    // Getters/setters for $entityManager
}

CountryFieldsetFactory.php - IN HERE THE INPUTFILTER IS SET ONTO THE FIELDSET

class CountryFieldsetFactory extends AbstractFieldsetFactory
{
    public function createService(ServiceLocatorInterface $serviceLocator)
    {
        parent::createService($serviceLocator);

        /** @var CountryRepository $entityRepository */
        $entityRepository = $this->getEntityManager()->getRepository(Country::class);

        $fieldset = new CountryFieldset($this->getEntityManager(), $this->name);
        $fieldset->setHydrator(new DoctrineObject($this->getServiceManager()->get('doctrine.entitymanager.orm_default'), false));
        $fieldset->setObject(new Country());
        $fieldset->setTranslator($this->getTranslator());

        // HERE THE INPUTFILTER IS SET ONTO THE FIELDSET THAT WAS JUST CREATED
        $fieldset->setInputFilter(
            $this->getServiceManager()->get('InputFilterManager')->get(
                CountryInputFilter::class,
                [ // These are the options read by the MutableCreationOptionsInterface
                    'object_manager' => $this->getEntityManager(),
                    'object_repository' => $entityRepository,
                    'translator' => $this->getTranslator(),
                ]
            )
        );

        return $fieldset;
    }
}

AbstractFieldsetFactory.php

abstract class AbstractFieldsetFactory implements FactoryInterface, MutableCreationOptionsInterface
{

    protected $serviceManager;
    protected $entityManager;
    protected $translator;
    protected $name;


    public function setCreationOptions(array $options)
    {
        $this->name = $options['name'];
    }

    public function createService(ServiceLocatorInterface $serviceLocator)
    {
        /** @var ServiceLocator $serviceManager */
        $this->serviceManager = $serviceLocator->getServiceLocator();

        /** @var EntityManager $entityManager */
        $this->entityManager = $this->getServiceManager()->get('Doctrine\ORM\EntityManager');

        /** @var Translator $translator */
        $this->translator = $this->getServiceManager()->get('translator');
    }

    // Getters/setters for properties
}

CountryFieldsetInputFilter.php

class CountryInputFilter extends AbstractInputFilter
{
    public function init()
    {
        parent::init();

        $this->add([
            'name' => 'name',
            'required' => true,
            'filters' => [
                ['name' => StringTrim::class],
                ['name' => StripTags::class],
            ],
            'validators' => [
                [
                    'name' => StringLength::class,
                    'options' => [
                        'min' => 3, // This is just completely ignored
                        'max' => 255,
                    ],
                ],
            ],
        ]);

        // More adding
    }
}

AbstractFieldsetInputFilter.php - Last one! :)

abstract class AbstractInputFilter extends InputFilter
{
    use TranslatorAwareTrait;

    protected $repository;
    protected $objectManager;

    public function __construct(array $options)
    {
        // Check if ObjectManager|EntityManager for InputFilter is set
        $this->setObjectManager($options['object_manager']);

        // Check if EntityRepository instance for InputFilter is set
        $this->setRepository($options['object_repository']);

        // Check for presence of translator so as to translate return messages
        $this->setTranslator($options['translator']);
    }

    public function init()
    {
        $this->add([
            'name' => 'id',
            'required' => false,
            'filters' => [
                ['name' => ToInt::class],
            ],
            'validators' => [
                ['name' => IsInt::class],
            ],
        ]);
    }

    //Getters/setters for properties
}

Any help would very much be appreciated. Hopefully you're not too overloaded with the code above. But I've been mashing this issue back'n'forth for about 3-4 days and haven't stumbled onto what's going wrong.

To sum up:

In the above a CountryForm is created. It uses the CountryFieldset which gets preloaded (in CountryFieldsetFactory) with the CountryInputFilter.

When it comes to validating the data, everything is accepted as valid. E.g. - Country name "ab" is valid, though StringLength validator has 'min' => 3, defined as option.

rkeet
  • 3,406
  • 2
  • 23
  • 49

2 Answers2

1

Since you've got all the classes set up already there is another approach (from @AlexP), by constructing and adding the InputFilters of the Fieldsets to the Forms InputFilter. Instead of using the InputFilterSpecifications.

So add the input filters to your input_filters config key:

'form_elements' => [
    'factories' => [
        CountryForm::class => CountryFormFactory::class,
        CountryFieldset::class => CountryFieldsetFactory::class,
    ],
],
'input_filters' => [
    'factories' => [
        CountryInputFilter::class => CountryInputFilterFactory::class,
        CountryFieldsetInputFilter::class => CountryFieldsetInputFilterFactory::class,
    ],
],

Factory classes:

class CountryInputFilterFactory implements FactoryInterface
{
    public function createService(ServiceLocatorInterface $serviceLocator)
    {
        $serviceManager = $serviceLocator->getServiceLocator();

        $inputFilter = new CountryInputFilter(
            $serviceLocator->get(CountryFieldsetInputFilter::class),
            $serviceManager()->get('Doctrine\ORM\EntityManager'),
            $serviceManager()->get('translator')
        );

        return $inputFilter;
    }
}

class CountryFieldsetInputFilterFactory implements FactoryInterface
{
    public function createService(ServiceLocatorInterface $serviceLocator)
    {
        $serviceManager = $serviceLocator->getServiceLocator();

        return new CountryFieldsetInputFilter(
            $serviceManager()->get('Doctrine\ORM\EntityManager'),
            $serviceManager()->get('translator')
        );
    }
}

class CountryFormFactory implements AbstractFormFactory
{
    public function createService(ServiceLocatorInterface $serviceLocator)
    {
        $serviceManager = $serviceLocator->getServiceLocator();

        $form = new CountryForm($this->name, $this->options);
        $form->setObjectManager($serviceManager->get('Doctrine\ORM\EntityManager'));
        $form->setTranslator($serviceManager->get('translator'));

        $form->setInputFilter($serviceManager->get('InputFilterManager')->get(CountryInputFilterFactory::class));
        return $form;
    }
}

Form:

class CountryForm extends AbstractForm
{
    const COUNTRY_FIELDSET_NAME = 'country';

    // This one added for SO, does nothing but call parent#__construct, which would happen anyway
    public function __construct($name = null, array $options)
    {
        parent::__construct($name, $options);
    }

    public function init()
    {
        //Call parent initializer.
        parent::init();

        $this->add([
            'name' => self::COUNTRY_FIELDSET_NAME,
            'type' => CountryFieldset::class,
            'options' => [
                'use_as_base_fieldset' => true,
            ],
        ]);
    }
}

InputFilters:

class CountryInputFilter extends AbstractInputFilter
{
    /** @var CountryFieldsetInputFilter  */
    protected $countryFieldsetInputFilter;

    public function __construct(CountryFieldsetInputFilter $filter, $objectManager, $translator)
    {
        $this->countryFieldsetInputFilter = $filter;
        // other dependencies
    }

    public function init()
    {
        $this->add($this->countryFieldsetInputFilter, CountryForm::COUNTRY_FIELDSET_NAME);
    }
}

class CountryFieldsetInputFilter extends AbstractInputFilter
{
    public function __construct($objectManager, $translator)
    {
        // dependencies
    }

    public function init()
    {
        $this->add([
            // configuration
        ]);
    }
}

Note that I injected the dependencies to the InputFilters per argument by instance instead of an array holding the instances.

Kwido
  • 1,382
  • 8
  • 21
  • Hmm this does indeed look good, working on implementing it at the moment. I'm assuming that with the config that you've provided it would also not be a problem to change it to: `'input_filters' => [ 'factories' => [ CountryFieldsetInputFilter::class => CountryFieldsetInputFilterFactory::class, ], ],`, right? Currently all Factories have their own classes instead of in the module configs. – rkeet Jun 02 '17 at 09:17
  • @Nukeface yes it works just like any other configuration | serviceManager instance. – Kwido Jun 02 '17 at 09:20
  • I think I have your setup correctly modified to my application, using Factory classes. I'm seeing the InputFilter registered in the form, however, it does not have a name. E.g. - In the `$form` object (CountryForm), there's this: `filter (InputFilter) => [ 'inputs' => [ "no key here (num/alpha)" => (CountryFieldsetInputFilter) => [ the filter here ], ], ],`. The `CountryFormFactory` is exactly like your example (I checked for the possibility to add Fieldset name, but cannot with `setInputFilter()`). The FieldsetFactory is also the same, though in its own class. – rkeet Jun 02 '17 at 09:57
  • So I now have the same structure for Fieldsets as for InputFilters - (Form -> Fieldset -> Fieldset & Form -> InputFilter -> InputFilter). But even with this setup it does not validate. Any ideas? Also, I could update question with refactored classes and debugging screenshots if that would help. – rkeet Jun 02 '17 at 09:59
  • I will update my answer in that case, so the question remains the same and the other answers will still apply to your question. The code you posted within your comments is hard to read and doesn't really clearifies your debugging, but I think I understand what you're trying to say. – Kwido Jun 02 '17 at 10:07
  • Wow thanks for the update, that helped out a lot! I now have it working for Form using a Fieldset. Quickly noticed that the current setup doesn't (yet) supported nested fieldsets. Would you maybe have a way for that as well? (was hoping that would automatically work, which was why I left it out of the question, but a Country has Coordinates (OneToOne)). – rkeet Jun 02 '17 at 12:27
  • Just add one InputFilter to another InputFilter like we did with the fieldset inputfilter by adding it to the form inputfilter. We can nest it as much as we want. As long as you stick with and match the form element names and keep the same structure within your inputfilters as your form/fieldsets. Feel free to accept the answer if it helped solving your problem. – Kwido Jun 02 '17 at 12:39
  • Following your comment above I've been trying to add the `CoordinatesInputFilter` to the `CountryInputFilter. However, no joy. In `CountryFormFactory` I hadded 2 lines: `$coordinatesInputFilter = $this->getInputFilterPluginManager()->get(CoordinatesInputFilter::class);`, and then went to add it to the `$countryInputFilter` like so: `$countryInputFilter->get('country')->add($coordinatesInputFilter, 'coordinates')`. It shows up in the `inputs` property of the `$countryInputFilter`, however it does not validate the nested Fieldset, even though the structure is the same. Any clues? – rkeet Jun 02 '17 at 13:37
  • Your `CountryFieldset` has an element "coordiantes" which is another fieldset: `CoordinatesFieldset`. In that case you should inject the `CoordiantesInputFilter` into the `CountryFieldsetInputFilter` and within the `::init()` do something like `$this->add($this->coordinatesInputFilter, 'coordinates');`. Notice that "coordinates", the second argument, is the name you use within your Fieldset itself to link to the CountryFieldset. Name as within `$this->add(['name' => __FIELDSET_NAME__, 'type' => CountryFieldset::class])` – Kwido Jun 02 '17 at 13:58
  • Quick message, time to go home. That last bit did point me in the right direction for the nested fieldsets. I was trying to do it via the factories, though should just have been what you said, pass the nested InputFilter along to the constructor, set it on the class and use it in the `init()`. Thanks a bunch for your help yesterday/today! It now works. With proper separation of all concerns, makes me very happy! :) – rkeet Jun 02 '17 at 15:09
0
$fieldset->setInputFilter($inputFilter); 

Assuming that $fieldset is an instance of Zend\Form\Fieldset, the method doesn't exist. This is because you need to set the input filter on the form (Zend\Form\Form).

If I was to modify your code I would do so in the following way. Modify the CountryForm to provide the input filter via the form. This can be done without needing to define a custom input filter for each form, by using the Zend\InputFilter\InputFilterProviderInterface and referencing your custom input filter under the type key.

When the form element manager creates the form, it will also inject the input filter manager which will be able to find the custom CountryInputFilter.

For example:

use Zend\InputFilter\InputFilterProviderInterface;

class CountryForm extends AbstractForm implements InputFilterProviderInterface
{

    public function init() 
    { 
        //.. add the country fieldset here
    }

    /**
     * getInputFilterSpecification
     *
     * Return the form's input filter specification array.
     *
     * @return array
     */
    public function getInputFilterSpecification()
    {
        return [
            // Refers to the country element (happens to be a fieldset)
            'country' => [

                // Tell the input filter factory what type of input filter.
                'type' => 'Country\\InputFilter\\CountryInputFilter',

                // You could even overload/change the default defined in the input filter
                // here. This is useful as sometimes the requirements of the FORM are 
                // different from what the FIELDSET expects.
                // [
                //     'name'        => 'name',
                //     'required'    => false,
                //     'allow_empty' => true,
                // ],
            ],
        ];
    }

}
AlexP
  • 9,906
  • 1
  • 24
  • 43
  • Thanks, I'll be giving that a shot first thing tomorrow. One thing though. The InputFilters require the presence of an `object_repository`, `object_manager` and `translator`. How would you go about passing them along using this method? – rkeet Jun 01 '17 at 14:29
  • @Nukeface The filter will be loaded from the input filter manager; this means that should you require dependencies you can create a `CountryInputFilterFactory` to inject them. If you do so the `$options`, which are currently going via the `AbstractInputFilter ::__construct($options)` would then be passed to the factory (e.g. `CountryInputFilterFactory::__construct($options)`. – AlexP Jun 01 '17 at 15:19