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

Fixes order in which default initializers are injected #119

Merged
merged 7 commits into from
Sep 14, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 32 additions & 3 deletions src/FormElementManager/FormElementManagerV2Polyfill.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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;
}
}
44 changes: 28 additions & 16 deletions src/FormElementManager/FormElementManagerV3Polyfill.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down Expand Up @@ -299,4 +283,32 @@ 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.
*
* {@inheritDoc}
*/
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;
}
}
126 changes: 126 additions & 0 deletions test/Integration/ServiceManagerTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
<?php
/**
* @link http://github.com/zendframework/zend-form for the canonical source repository
* @copyright Copyright (c) 2016 Zend Technologies USA Inc. (http://www.zend.com)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copyright wording inconsistency

* @license http://framework.zend.com/license/new-bsd New BSD License
*/

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
{
public function testInitInitializerShouldBeCalledAfterAllOtherInitializers()
{
// 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');

$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([
'factories' => [
'InitializableElement' => function () use ($element) {
return $element->reveal();
},
],
'initializers' => [
$initializer->reveal(),
],
]);

$formElementManagerConfig->configureServiceManager($formElementManager);

$formElementManager->get('InitializableElement');
}

public function testInjectFactoryInitializerShouldTriggerBeforeInitInitializer()
{
// 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());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not $this->assert... as we have in other places? I'd change it to keep consistency.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because $this gets bound to the prophecy.

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([
'factories' => [
'MyForm' => function () {
return new TestAsset\Form();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ocramius Had to use a test asset here. When I tried mocking, I was hitting out-of-memory errors. Worked fine for the initializers, but not for the form.

},
],
'initializers' => [
$initializer->reveal(),
],
]);

$formElementManagerConfig->configureServiceManager($formElementManager);

/** @var TestAsset\Form */
$form = $formElementManager->get('MyForm');
$this->assertSame($formElementManager, $form->elementManagerAtInit);
}
}
26 changes: 26 additions & 0 deletions test/Integration/TestAsset/Form.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php
/**
* @link http://github.com/zendframework/zend-form for the canonical source repository
* @copyright Copyright (c) 2016 Zend Technologies USA Inc. (http://www.zend.com)
* @license http://framework.zend.com/license/new-bsd New BSD License
*/

namespace ZendTest\Form\Integration\TestAsset;

use Zend\Form\Form as BaseForm;

class Form extends BaseForm
{
/**
* @param null|\Zend\Form\FormElementManager
*/
public $elementManagerAtInit;

/**
* {@inheritDoc}
*/
public function init()
{
$this->elementManagerAtInit = $this->getFormFactory()->getFormElementManager();
}
}