Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

FormElementManagerV3Polyfill: initializers BC break #100

Closed
Slamdunk opened this issue Jul 4, 2016 · 4 comments
Closed

FormElementManagerV3Polyfill: initializers BC break #100

Slamdunk opened this issue Jul 4, 2016 · 4 comments

Comments

@Slamdunk
Copy link
Contributor

Slamdunk commented Jul 4, 2016

Let's say you have this code (some FQCN are shortened for readibility):

// zend-mvc config
$config = array(
    'form_elements' => array(
        'invokables' => array(
            'MyForm' => 'MyForm',
        ),
        'initializers' => array(
            'MyInitializer',
        ),
    ),
);

class MyInitializer implements InitializerInterface
{
    public function __invoke(ContainerInterface $container, $instance)
    {
        $instance->dependancy = 1;
    }
}

class MyForm implements ElementInterface, InitializableInterface
{
    public $dependancy = 0;

    public function init()
    {
        var_dump($this->dependancy);
    }
}

This will output different if using SM v2 or SM v3:

int(1) // SM v2
int(0) // SM v3

Because by default v2 adds initializers on top of the stack leaving callElementInit for last, instead v3 always add initializers on bottom.

Although we all know that initializers should be avoided in favour of factories and constructor dependency injection, as of yet there is no way to forward port from SM v2 to SM v3 a full zf modulemanager+mvc stack, because in v3 FormElementManager is always configured after construction, and so init will always be called before custom initializers.

A temporary solution would be to move initializers config key away and create a custom FormElementManagerFactory to pass them into the constructor.

A (very raw) functioning example:

// new config
$config = array(
    'form_elements' => array(
        'invokables' => array(
            'MyForm' => 'MyForm',
        ),
    'form_elements_initializers' => array(
        'MyInitializer',
    ),
    'service_manager' => array(
        'factories' => array(
            'FormElementManager' => 'MyFormElementManagerFactory',
        ),
    ),
);

class MyFormElementManagerFactory implements FactoryInterface
{
    public function __invoke(ContainerInterface $container, $name, array $options = null)
    {
        $config = $container->get('Config');

        $options = array(
            'initializers' => $config['form_elements_initializers'],
        );

        return new FormElementManager($container, $options);
    }
}
@weierophinney
Copy link
Member

I cannot recreate this issue, and I think it may have to do with how you're creating your initializer.

I've written the following bits to test it.

First, test/TestAsset/InitializableElement.php:

namespace ZendTest\Form\TestAsset;

use Zend\Form\ElementInterface;
use Zend\Stdlib\InitializableInterface;

class InitializableElement implements ElementInterface, InitializableInterface
{
    public $dependency = 0;

    public $dependencyAtTimeOfInit;

    public function init()
    {
        $this->dependencyAtTimeOfInit = $this->dependency;
    }

    /**
     * {@inheritDoc}
     */
    public function setName($name)
    {
    }

    /**
     * {@inheritDoc}
     */
    public function getName()
    {
    }

    /**
     * {@inheritDoc}
     */
    public function setOptions($options)
    {
    }

    /**
     * {@inheritDoc}
     */
    public function setOption($key, $value)
    {
    }

    /**
     * {@inheritDoc}
     */
    public function getOptions()
    {
    }

    /**
     * {@inheritDoc}
     */
    public function getOption($option)
    {
    }

    /**
     * {@inheritDoc}
     */
    public function setAttribute($key, $value)
    {
    }

    /**
     * {@inheritDoc}
     */
    public function getAttribute($key)
    {
    }

    /**
     * {@inheritDoc}
     */
    public function hasAttribute($key)
    {
    }

    /**
     * {@inheritDoc}
     */
    public function setAttributes($arrayOrTraversable)
    {
    }

    /**
     * {@inheritDoc}
     */
    public function getAttributes()
    {
    }

    /**
     * {@inheritDoc}
     */
    public function setValue($value)
    {
    }

    /**
     * {@inheritDoc}
     */
    public function getValue()
    {
    }

    /**
     * {@inheritDoc}
     */
    public function setLabel($label)
    {
    }

    /**
     * {@inheritDoc}
     */
    public function getLabel()
    {
    }

    /**
     * {@inheritDoc}
     */
    public function setMessages($messages)
    {
    }

    /**
     * {@inheritDoc}
     */
    public function getMessages()
    {
    }
}

Next, I added a test case to test/FormElementManagerTest.php:

    public function testInitInitializerShouldBeCalledAfterAllOtherInitializers()
    {
        $initializer = function ($one, $two) {
            if ($one instanceof ElementInterface) {
                // v2
                $instance = $one;
                $container = $two;
            } else {
                // v3
                $instance = $two;
                $container = $one;
            }

            if (! property_exists($instance, 'dependency')) {
                return;
            }

            $instance->dependency = 1;
        };

        $config = [
            'aliases' => [
                'Foo' => TestAsset\InitializableElement::class,
            ],
            'invokables' => [
                TestAsset\InitializableElement::class => TestAsset\InitializableElement::class,
            ],
            'initializers' => [
                $initializer,
            ],
        ];

        $manager = new FormElementManager(new ServiceManager(), $config);
        $foo = $manager->get('Foo');
        $this->assertInstanceOf(TestAsset\InitializableElement::class, $foo);
        $this->assertEquals(1, $foo->dependency);
        $this->assertEquals(1, $foo->dependencyAtTimeOfInit);
    }

The idea here is to mimic what you did with your var_dump() call, by setting a second property when init() is called; we can then assert that it has been updated after initialization.

I ran this both using the composer.lock (which uses zend-servicemanager 3.0.3), and against the lowest supported dependencies (which uses zend-servicemanager 2.7.5). The test ran correctly in both cases.

The two differences I see between my test setup and your examples are:

  • I used an alias. That should have no effect on the test, however.
  • My initializer is testing for v2 vs v3 arguments, and varying which is used as the instance vs the element accordingly before attempting to set the property.

If you look here:

you can see that the v3 polyfill attempts to recreate the stacking order such that (a) the injectFactory() initializer is always first in the stack (i.e., on the top, first to execute), and (b) the callElementInit() initializer is always last on the stack after configuration (i.e., on the bottom, last to execute). This was introduced in #67 precisely to address the BC breaks you're reporting here — because we recognized that factory-aware fieldsets and forms require injection of the form element factory early, and that init() should be called after all other initialization occurs.

Right now, I cannot reproduce using the code you've provided. If you can provide a reproduce case, I'll look into it further.

@Slamdunk
Copy link
Contributor Author

Slamdunk commented Jul 6, 2016

Hi, I'm sorry I wasn't clear enough on original post on how this is not a bug related to zend-form itself, but an integration bug on a full stack mvc + module-manager + form, and because those are not dependancies (dev-)required in this component, originally I couldn't make a test.

Btw, here it is the full reproducible bug: https://github.com/Slamdunk/zend-form-issue-100

The main sources of this bug are the fact that in a stack like this, the FormElementManager is configured after instantiation:

@Slamdunk
Copy link
Contributor Author

Slamdunk commented Jul 22, 2016

@weierophinney let me know if the reproducible bug provided is clear enough. This bug is forcing me to change all my projects.

@Slamdunk
Copy link
Contributor Author

Slamdunk commented Sep 13, 2016

Bug reproduced in #117, closing

weierophinney added a commit to weierophinney/zend-form that referenced this issue Sep 13, 2016
Reproduce issue zendframework#100 without mvc and modulemanager dependencies
weierophinney added a commit to weierophinney/zend-form that referenced this issue Sep 13, 2016
Uses the test from zendframework#117 to find a solution for zendframework#100 by overriding `configure()`
to first remove the default initializers from the initializer stack if present,
and then push them on in the appropriate positions once configuration is
complete.
weierophinney added a commit that referenced this issue Sep 14, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants