From a9724dfd51f061ec5fe87a7718278f8fb0d303c8 Mon Sep 17 00:00:00 2001 From: Filippo Tessarotto Date: Tue, 13 Sep 2016 10:04:18 +0200 Subject: [PATCH 1/6] Reproduce issue #100 without mvc and modulemanager dependancies --- test/Integration/ServiceManagerTest.php | 48 +++++++++++++++++++ .../TestAsset/DependancyInitializer.php | 31 ++++++++++++ .../TestAsset/InitializableElement.php | 24 ++++++++++ 3 files changed, 103 insertions(+) create mode 100644 test/Integration/ServiceManagerTest.php create mode 100644 test/Integration/TestAsset/DependancyInitializer.php create mode 100644 test/Integration/TestAsset/InitializableElement.php diff --git a/test/Integration/ServiceManagerTest.php b/test/Integration/ServiceManagerTest.php new file mode 100644 index 00000000..c867845a --- /dev/null +++ b/test/Integration/ServiceManagerTest.php @@ -0,0 +1,48 @@ + [ + 'FormElementManager' => FormElementManagerFactory::class, + ], + ]); + + $serviceManager = new ServiceManager(); + $serviceManagerConfig->configureServiceManager($serviceManager); + + $formElementManagerConfig = new Config([ + 'invokables' => [ + 'InitializableElement' => TestAsset\InitializableElement::class, + ], + 'initializers' => [ + TestAsset\DependancyInitializer::class, + ], + ]); + + $formElementManager = $serviceManager->get('FormElementManager'); + $formElementManagerConfig->configureServiceManager($formElementManager); + + $element = $formElementManager->get('InitializableElement'); + + $this->assertSame(1, $element->dependency); + $this->assertSame(1, $element->dependencyAtTimeOfInit); + } +} diff --git a/test/Integration/TestAsset/DependancyInitializer.php b/test/Integration/TestAsset/DependancyInitializer.php new file mode 100644 index 00000000..5a79d975 --- /dev/null +++ b/test/Integration/TestAsset/DependancyInitializer.php @@ -0,0 +1,31 @@ +dependency = 1; + } + + public function initialize($instance, ServiceLocatorInterface $serviceLocator) + { + return $this($serviceLocator, $instance); + } +} diff --git a/test/Integration/TestAsset/InitializableElement.php b/test/Integration/TestAsset/InitializableElement.php new file mode 100644 index 00000000..289374a8 --- /dev/null +++ b/test/Integration/TestAsset/InitializableElement.php @@ -0,0 +1,24 @@ +dependencyAtTimeOfInit = $this->dependency; + } +} From 81c5d3fe1097f5736e15e41f29efcf928ae2f117 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Tue, 13 Sep 2016 16:34:27 -0500 Subject: [PATCH 2/6] Fixes order in which default initializers are injected Uses the test from #117 to find a solution for #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. --- .../FormElementManagerV3Polyfill.php | 45 ++++++++++++------- 1 file changed, 29 insertions(+), 16 deletions(-) diff --git a/src/FormElementManager/FormElementManagerV3Polyfill.php b/src/FormElementManager/FormElementManagerV3Polyfill.php index 28f80b4d..c5fe1ac1 100644 --- a/src/FormElementManager/FormElementManagerV3Polyfill.php +++ b/src/FormElementManager/FormElementManagerV3Polyfill.php @@ -202,22 +202,6 @@ class FormElementManagerV3Polyfill extends AbstractPluginManager */ protected $instanceOf = ElementInterface::class; - /** - * Constructor - * - * Overrides parent constructor in order to add the initializer methods injectFactory() - * and callElementInit(). - * - * @param ContainerInterface $parentLocator - * @param null|array $config - */ - public function __construct(ContainerInterface $parentLocator = null, array $config = []) - { - $this->addInitializer([$this, 'injectFactory']); - parent::__construct($parentLocator, $config); - $this->addInitializer([$this, 'callElementInit']); - } - /** * Inject the factory to any element that implements FormFactoryAwareInterface * @@ -299,4 +283,33 @@ public function validate($plugin) )); } } + + /** + * Overrides parent::configure in order to ensure default initializers are in expected positions. + * + * Always pushes `injectFactory` to top of initializer stack, and + * `callElementInit` to the bottom. + * + * @param array $config + * @return self + */ + public function configure(array $config) + { + $firstInitializer = [$this, 'injectFactory']; + $lastInitializer = [$this, 'callElementInit']; + + foreach ([$firstInitializer, $lastInitializer] as $default) { + if (false === ($index = array_search($default, $this->initializers))) { + continue; + } + unset($this->initializers[$index]); + } + + parent::configure($config); + + array_unshift($this->initializers, $firstInitializer); + array_push($this->initializers, $lastInitializer); + + return $this; + } } From 83ab4fea6507c5fee711b74261c66ebced68a2a2 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Tue, 13 Sep 2016 16:51:17 -0500 Subject: [PATCH 3/6] Incorporated feedback - Ensure consistent license on new files, with correct year - `s/Dependancy/Dependency/g` - Added dockblocks on new class methods and properties --- test/Integration/ServiceManagerTest.php | 8 +++----- ...itializer.php => DependencyInitializer.php} | 18 +++++++++++++----- .../TestAsset/InitializableElement.php | 17 +++++++++++++---- 3 files changed, 29 insertions(+), 14 deletions(-) rename test/Integration/TestAsset/{DependancyInitializer.php => DependencyInitializer.php} (54%) diff --git a/test/Integration/ServiceManagerTest.php b/test/Integration/ServiceManagerTest.php index c867845a..e2acdd8b 100644 --- a/test/Integration/ServiceManagerTest.php +++ b/test/Integration/ServiceManagerTest.php @@ -1,9 +1,7 @@ TestAsset\InitializableElement::class, ], 'initializers' => [ - TestAsset\DependancyInitializer::class, + TestAsset\DependencyInitializer::class, ], ]); diff --git a/test/Integration/TestAsset/DependancyInitializer.php b/test/Integration/TestAsset/DependencyInitializer.php similarity index 54% rename from test/Integration/TestAsset/DependancyInitializer.php rename to test/Integration/TestAsset/DependencyInitializer.php index 5a79d975..5d5b1b2d 100644 --- a/test/Integration/TestAsset/DependancyInitializer.php +++ b/test/Integration/TestAsset/DependencyInitializer.php @@ -1,9 +1,7 @@ dependency = 1; } + /** + * @param \Zend\Form\Element\ElementInterface $instance + * @param ServiceLocatorInterface $serviceLocator + * @return void + */ public function initialize($instance, ServiceLocatorInterface $serviceLocator) { return $this($serviceLocator, $instance); diff --git a/test/Integration/TestAsset/InitializableElement.php b/test/Integration/TestAsset/InitializableElement.php index 289374a8..9abbe54f 100644 --- a/test/Integration/TestAsset/InitializableElement.php +++ b/test/Integration/TestAsset/InitializableElement.php @@ -1,9 +1,7 @@ dependencyAtTimeOfInit = $this->dependency; From 0365eabc60a834a08243e4eab7155a2b43f03f06 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Tue, 13 Sep 2016 17:27:31 -0500 Subject: [PATCH 4/6] Refactor tests Removes need for additional test asset classes by using mock objects to test behavior of initializers. Adds a test to verify that the plugin manager injected into a form's factory is the application form element manager. --- test/Integration/ServiceManagerTest.php | 90 +++++++++++++++++-- .../TestAsset/DependencyInitializer.php | 39 -------- .../TestAsset/InitializableElement.php | 33 ------- 3 files changed, 83 insertions(+), 79 deletions(-) delete mode 100644 test/Integration/TestAsset/DependencyInitializer.php delete mode 100644 test/Integration/TestAsset/InitializableElement.php diff --git a/test/Integration/ServiceManagerTest.php b/test/Integration/ServiceManagerTest.php index e2acdd8b..edee2a95 100644 --- a/test/Integration/ServiceManagerTest.php +++ b/test/Integration/ServiceManagerTest.php @@ -8,8 +8,13 @@ namespace ZendTest\Form\Integration; use PHPUnit_Framework_TestCase as TestCase; +use Prophecy\Argument; +use Zend\Form\Element; +use Zend\Form\Form; +use Zend\Form\FormElementManager; use Zend\Form\FormElementManagerFactory; use Zend\ServiceManager\Config; +use Zend\ServiceManager\InitializerInterface; use Zend\ServiceManager\ServiceManager; class ServiceManagerTest extends TestCase @@ -26,21 +31,92 @@ public function testInitInitializerShouldBeCalledAfterAllOtherInitializers() $serviceManager = new ServiceManager(); $serviceManagerConfig->configureServiceManager($serviceManager); + $formElementManager = $serviceManager->get('FormElementManager'); + + $test = 0; + $spy = function () use (&$test) { + TestCase::assertEquals(1, $test); + }; + + $element = $this->prophesize(Element::class); + $element->init()->will($spy); + + $initializer = $this->prophesize(InitializerInterface::class); + $incrementTest = function () use (&$test) { + $test += 1; + }; + + if (method_exists($serviceManager, 'configure')) { + $initializer->__invoke( + $serviceManager, + $element->reveal() + )->will($incrementTest)->shouldBeCalled(); + } else { + $initializer->initialize( + $element->reveal(), + $formElementManager + )->will($incrementTest)->shouldBeCalled(); + } + $formElementManagerConfig = new Config([ - 'invokables' => [ - 'InitializableElement' => TestAsset\InitializableElement::class, + 'factories' => [ + 'InitializableElement' => function () use ($element) { + return $element->reveal(); + }, ], 'initializers' => [ - TestAsset\DependencyInitializer::class, + $initializer->reveal(), ], ]); - $formElementManager = $serviceManager->get('FormElementManager'); $formElementManagerConfig->configureServiceManager($formElementManager); - $element = $formElementManager->get('InitializableElement'); + $formElementManager->get('InitializableElement'); + } + + public function testInjectFactoryInitializerShouldBeFirst() + { + // Reproducing the behaviour of a full stack MVC + ModuleManager + $serviceManagerConfig = new Config([ + 'factories' => [ + 'FormElementManager' => FormElementManagerFactory::class, + ], + ]); + + $serviceManager = new ServiceManager(); + $serviceManagerConfig->configureServiceManager($serviceManager); + + $formElementManager = $serviceManager->get('FormElementManager'); + + $initializer = $this->prophesize(InitializerInterface::class); + $formElementManagerAssertion = function ($form) use ($formElementManager) { + TestCase::assertInstanceOf(Form::class, $form); + TestCase::assertSame($formElementManager, $form->getFormFactory()->getFormElementManager()); + return true; + }; + if (method_exists($serviceManager, 'configure')) { + $initializer->__invoke( + $serviceManager, + Argument::that($formElementManagerAssertion) + )->shouldBeCalled(); + } else { + $initializer->initialize( + Argument::that($formElementManagerAssertion), + $formElementManager + )->shouldBeCalled(); + } + + $formElementManagerConfig = new Config([ + 'invokables' => [ + 'MyForm' => Form::class, + ], + 'initializers' => [ + $initializer->reveal(), + ], + ]); + + $formElementManagerConfig->configureServiceManager($formElementManager); - $this->assertSame(1, $element->dependency); - $this->assertSame(1, $element->dependencyAtTimeOfInit); + $formElementManager->get('Form'); } } diff --git a/test/Integration/TestAsset/DependencyInitializer.php b/test/Integration/TestAsset/DependencyInitializer.php deleted file mode 100644 index 5d5b1b2d..00000000 --- a/test/Integration/TestAsset/DependencyInitializer.php +++ /dev/null @@ -1,39 +0,0 @@ -dependency = 1; - } - - /** - * @param \Zend\Form\Element\ElementInterface $instance - * @param ServiceLocatorInterface $serviceLocator - * @return void - */ - public function initialize($instance, ServiceLocatorInterface $serviceLocator) - { - return $this($serviceLocator, $instance); - } -} diff --git a/test/Integration/TestAsset/InitializableElement.php b/test/Integration/TestAsset/InitializableElement.php deleted file mode 100644 index 9abbe54f..00000000 --- a/test/Integration/TestAsset/InitializableElement.php +++ /dev/null @@ -1,33 +0,0 @@ -dependencyAtTimeOfInit = $this->dependency; - } -} From 6e7d469b8bebf957dc0f95e19d43627da7066054 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 14 Sep 2016 08:28:55 -0500 Subject: [PATCH 5/6] Ensure injectFactory is called before callElementInit in all versions Adds a unit test to ensure that the `injectFactory()` initializer is injected prior to the `callElementInit()` initializer, and updates the V2 polyfill to ensure the behavior is correct. A new test asset was created to help spy on behavior. --- .../FormElementManagerV2Polyfill.php | 35 +++++++++++++++++-- test/Integration/ServiceManagerTest.php | 14 +++++--- test/Integration/TestAsset/Form.php | 26 ++++++++++++++ 3 files changed, 67 insertions(+), 8 deletions(-) create mode 100644 test/Integration/TestAsset/Form.php diff --git a/src/FormElementManager/FormElementManagerV2Polyfill.php b/src/FormElementManager/FormElementManagerV2Polyfill.php index 727ef5ed..3b08e680 100644 --- a/src/FormElementManager/FormElementManagerV2Polyfill.php +++ b/src/FormElementManager/FormElementManagerV2Polyfill.php @@ -175,10 +175,11 @@ class FormElementManagerV2Polyfill extends AbstractPluginManager */ public function __construct($configInstanceOrParentLocator = null, array $v3config = []) { - parent::__construct($configInstanceOrParentLocator, $v3config); + // Provide default initializers, ensuring correct order + array_unshift($this->initializers, [$this, 'injectFactory']); + array_push($this->initializers, [$this, 'callElementInit']); - $this->addInitializer([$this, 'injectFactory']); - $this->addInitializer([$this, 'callElementInit'], false); + parent::__construct($configInstanceOrParentLocator, $v3config); } /** @@ -261,4 +262,32 @@ public function validatePlugin($plugin) )); } } + + /** + * Overrides parent::addInitializer in order to ensure default initializers are in expected positions. + * + * Always pushes `injectFactory` to top of initializer stack, and + * `callElementInit` to the bottom. + * + * {@inheritDoc} + */ + public function addInitializer($initializer, $topOfStack = true) + { + $firstInitializer = [$this, 'injectFactory']; + $lastInitializer = [$this, 'callElementInit']; + + foreach ([$firstInitializer, $lastInitializer] as $default) { + if (false === ($index = array_search($default, $this->initializers))) { + continue; + } + unset($this->initializers[$index]); + } + + parent::addInitializer($initializer, $topOfStack); + + array_unshift($this->initializers, $firstInitializer); + array_push($this->initializers, $lastInitializer); + + return $this; + } } diff --git a/test/Integration/ServiceManagerTest.php b/test/Integration/ServiceManagerTest.php index edee2a95..26660245 100644 --- a/test/Integration/ServiceManagerTest.php +++ b/test/Integration/ServiceManagerTest.php @@ -1,6 +1,6 @@ get('InitializableElement'); } - public function testInjectFactoryInitializerShouldBeFirst() + public function testInjectFactoryInitializerShouldTriggerBeforeInitInitializer() { // Reproducing the behaviour of a full stack MVC + ModuleManager $serviceManagerConfig = new Config([ @@ -107,8 +107,10 @@ public function testInjectFactoryInitializerShouldBeFirst() } $formElementManagerConfig = new Config([ - 'invokables' => [ - 'MyForm' => Form::class, + 'factories' => [ + 'MyForm' => function () { + return new TestAsset\Form(); + }, ], 'initializers' => [ $initializer->reveal(), @@ -117,6 +119,8 @@ public function testInjectFactoryInitializerShouldBeFirst() $formElementManagerConfig->configureServiceManager($formElementManager); - $formElementManager->get('Form'); + /** @var TestAsset\Form */ + $form = $formElementManager->get('MyForm'); + $this->assertSame($formElementManager, $form->elementManagerAtInit); } } diff --git a/test/Integration/TestAsset/Form.php b/test/Integration/TestAsset/Form.php new file mode 100644 index 00000000..681e0b78 --- /dev/null +++ b/test/Integration/TestAsset/Form.php @@ -0,0 +1,26 @@ +elementManagerAtInit = $this->getFormFactory()->getFormElementManager(); + } +} From b8396978e3ab61588332ecfda4e674e37d0a4bb3 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 14 Sep 2016 09:33:28 -0500 Subject: [PATCH 6/6] Use `{@inheritDoc}` on `configure()` override --- src/FormElementManager/FormElementManagerV3Polyfill.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/FormElementManager/FormElementManagerV3Polyfill.php b/src/FormElementManager/FormElementManagerV3Polyfill.php index c5fe1ac1..6d087283 100644 --- a/src/FormElementManager/FormElementManagerV3Polyfill.php +++ b/src/FormElementManager/FormElementManagerV3Polyfill.php @@ -290,8 +290,7 @@ public function validate($plugin) * Always pushes `injectFactory` to top of initializer stack, and * `callElementInit` to the bottom. * - * @param array $config - * @return self + * {@inheritDoc} */ public function configure(array $config) {