Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
This patch removes the change introduced in 2.7.1's
`Zend\Form\Factory::create()` method that aggregated options and passed
them to the `FormElementManager::get()` call. This usage was incorrect,
as it creates an order-of-operations problem: most instances call their
own `setOptions()` method from the constructor if options are present,
and many of the actions this executes assume things like a populated
`FormElementManager`, using a default instance if none is found!

As such, reverting those changes to pre-2.7.1 usage fixes the issues
reported in zendframework#66. Since the changes were done under an erroneous
assumption, a test related to the incorrect functionality was also
removed.
  • Loading branch information
weierophinney committed May 1, 2016
1 parent 60de7d8 commit 8660a5d
Show file tree
Hide file tree
Showing 5 changed files with 4 additions and 39 deletions.
7 changes: 1 addition & 6 deletions src/Factory.php
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,7 @@ public function create($spec)
$spec = $this->validateSpecification($spec, __METHOD__);
$type = isset($spec['type']) ? $spec['type'] : Element::class;

$creationOptions = [];
if (isset($spec['options']) && is_array($spec['options'])) {
$creationOptions = $spec['options'];
}

$element = $this->getFormElementManager()->get($type, $creationOptions);
$element = $this->getFormElementManager()->get($type);

if ($element instanceof FormInterface) {
return $this->configureForm($element, $spec);
Expand Down
1 change: 0 additions & 1 deletion test/Annotation/AnnotationBuilderFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

use Interop\Container\ContainerInterface;
use PHPUnit_Framework_TestCase as TestCase;
use Prophecy\Argument;
use ReflectionProperty;
use stdClass;
use Zend\EventManager\EventManagerInterface;
Expand Down
29 changes: 1 addition & 28 deletions test/FactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,10 @@
namespace ZendTest\Form;

use PHPUnit_Framework_TestCase as TestCase;
use DateTime;
use Zend\Filter;
use Zend\Form;
use Zend\Form\Factory as FormFactory;
use Zend\Form\FormElementManager;
use Zend\Form\FormInterface;
use Zend\ServiceManager\ServiceManager;
use Zend\Hydrator\HydratorPluginManager;

Expand Down Expand Up @@ -758,32 +756,6 @@ public function testCanCreateFormWithNullElements()
$this->assertTrue($form->has('bat'));
}

public function testOptionsArePassedAsCreationOptionsToFactories()
{
$formManager = $this->factory->getFormElementManager();
$formManager->setFactory('customCreatedForm', TestAsset\CustomCreatedFormFactory::class);

/* @var $form TestAsset\CustomCreatedForm */
$form = $this->factory->create([
'name' => 'some_name',
'type' => 'customCreatedForm',
'options' => [
'created' => '2016-02-19',
'some_other_option' => 1234
]
]);

$this->assertInstanceOf(FormInterface::class, $form);
$this->assertInstanceOf(TestAsset\CustomCreatedForm::class, $form);
$this->assertSame('some_name', $form->getName());
$this->assertSame(1234, $form->getOption('some_other_option'));

/* @var $created DateTime */
$created = $form->getCreated();
$this->assertInstanceOf(DateTime::class, $created);
$this->assertSame('2016-02-19', $created->format('Y-m-d'));
}

public function testCanCreateWithConstructionLogicInOptions()
{
$formManager = $this->factory->getFormElementManager();
Expand All @@ -802,6 +774,7 @@ public function testCanCreateWithConstructionLogicInOptions()
$this->assertInstanceOf(Form\Element\Collection::class, $collection);

$targetElement = $collection->getTargetElement();

$this->assertInstanceOf(TestAsset\FieldsetWithDependency::class, $targetElement);
$this->assertInstanceOf(TestAsset\InputFilter::class, $targetElement->getDependency());
}
Expand Down
2 changes: 1 addition & 1 deletion test/TestAsset/FieldsetWithDependency.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class FieldsetWithDependency extends Fieldset

public function __construct($name = null, $options = [])
{
parent::__construct('fielset_with_dependency', $options);
parent::__construct('fieldset_with_dependency', $options);
}

public function init()
Expand Down
4 changes: 1 addition & 3 deletions test/TestAsset/FieldsetWithDependencyFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ public function __invoke(ContainerInterface $container, $name, array $options =
}

$form = new FieldsetWithDependency($name, $options);
$dependency = new InputFilter();

$form->setDependency($dependency);
$form->setDependency(new InputFilter());

return $form;
}
Expand Down

0 comments on commit 8660a5d

Please sign in to comment.