Permalink
Browse files

merged branch bschussek/issue5221 (PR #5319)

Commits
-------

a38232a [Form] Fixed: FormTypeInterface::getParent() supports returning FormTypeInterface instances again

Discussion
----------

[Form] Fixed: FormTypeInterface::getParent() supports returning FormTypeInterface instances again

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #5221
Todo: -

---------------------------------------------------------------------------

by stof at 2012-08-22T14:14:55Z

the return value of the getParent method should be updated in the phpdoc of the FormTypeInterface to mention the FormTypeInterface .And the description of the method should be updated to explain than returning an instance is discouraged as it implies a performance penalty and does not support using type extensions (if the comment in the factory also applies to the unregistered parent)

---------------------------------------------------------------------------

by henrikbjorn at 2012-08-22T14:22:00Z

Wasn't TypeExtensions supported before? This means that Csrf will not be applied?

---------------------------------------------------------------------------

by stof at 2012-08-22T14:23:50Z

@henrikbjorn the csrf extension is targeting the FormType, which is registered in the form registry. What is not supported is having a type extension targeting an unregistered type

---------------------------------------------------------------------------

by bschussek at 2012-08-22T14:39:53Z

@stof Exactly. I find it a bit unlogical to register an extension for something that is not registered.

---------------------------------------------------------------------------

by henrikbjorn at 2012-08-22T14:39:57Z

Okay. That wasn't what i got from reading the comment :)

---------------------------------------------------------------------------

by bschussek at 2012-08-22T14:44:27Z

@stof Updated.
  • Loading branch information...
2 parents a27e1d8 + 78bf8be commit a132adde62247f5d19e191adc3a411f852667a52 @fabpot fabpot committed Aug 22, 2012
Showing with 182 additions and 26 deletions.
  1. +29 −10 FormFactory.php
  2. +33 −14 FormRegistry.php
  3. +5 −1 FormTypeInterface.php
  4. +32 −0 Tests/Fixtures/FooSubTypeWithParentInstance.php
  5. +53 −1 Tests/FormFactoryTest.php
  6. +30 −0 Tests/FormRegistryTest.php
View
@@ -80,16 +80,7 @@ public function createNamedBuilder($name, $type = 'form', $data = null, array $o
}
if ($type instanceof FormTypeInterface) {
- // An unresolved type instance was passed. Type extensions
- // are not supported for these. If you want to use type
- // extensions, you should create form extensions or register
- // your type in the Dependency Injection configuration instead.
- $parentType = $type->getParent();
- $type = $this->resolvedTypeFactory->createResolvedType(
- $type,
- array(),
- $parentType ? $this->registry->getType($parentType) : null
- );
+ $type = $this->resolveType($type);
} elseif (is_string($type)) {
$type = $this->registry->getType($type);
} elseif (!$type instanceof ResolvedFormTypeInterface) {
@@ -196,4 +187,32 @@ public function getType($name)
{
return $this->registry->getType($name)->getInnerType();
}
+
+ /**
+ * Wraps a type into a ResolvedFormTypeInterface implementation and connects
+ * it with its parent type.
+ *
+ * @param FormTypeInterface $type The type to resolve.
+ *
+ * @return ResolvedFormTypeInterface The resolved type.
+ */
+ private function resolveType(FormTypeInterface $type)
+ {
+ $parentType = $type->getParent();
+
+ if ($parentType instanceof FormTypeInterface) {
+ $parentType = $this->resolveType($parentType);
+ } elseif (null !== $parentType) {
+ $parentType = $this->registry->getType($parentType);
+ }
+
+ return $this->resolvedTypeFactory->createResolvedType(
+ $type,
+ // Type extensions are not supported for unregistered type instances,
+ // i.e. type instances that are passed to the FormFactory directly,
+ // nor for their parents, if getParent() also returns a type instance.
+ array(),
+ $parentType
+ );
+ }
}
View
@@ -95,25 +95,44 @@ public function getType($name)
throw new FormException(sprintf('Could not load type "%s"', $name));
}
- $parentType = $type->getParent();
- $typeExtensions = array();
+ $this->resolveAndAddType($type);
+ }
- foreach ($this->extensions as $extension) {
- /* @var FormExtensionInterface $extension */
- $typeExtensions = array_merge(
- $typeExtensions,
- $extension->getTypeExtensions($name)
- );
- }
+ return $this->types[$name];
+ }
+
+ /**
+ * Wraps a type into a ResolvedFormTypeInterface implementation and connects
+ * it with its parent type.
+ *
+ * @param FormTypeInterface $type The type to resolve.
+ *
+ * @return ResolvedFormTypeInterface The resolved type.
+ */
+ private function resolveAndAddType(FormTypeInterface $type)
+ {
+ $parentType = $type->getParent();
- $this->addType($this->resolvedTypeFactory->createResolvedType(
- $type,
+ if ($parentType instanceof FormTypeInterface) {
+ $this->resolveAndAddType($parentType);
+ $parentType = $parentType->getName();
+ }
+
+ $typeExtensions = array();
+
+ foreach ($this->extensions as $extension) {
+ /* @var FormExtensionInterface $extension */
+ $typeExtensions = array_merge(
$typeExtensions,
- $parentType ? $this->getType($parentType) : null
- ));
+ $extension->getTypeExtensions($type->getName())
+ );
}
- return $this->types[$name];
+ $this->addType($this->resolvedTypeFactory->createResolvedType(
+ $type,
+ $typeExtensions,
+ $parentType ? $this->getType($parentType) : null
+ ));
}
/**
@@ -78,7 +78,11 @@ public function setDefaultOptions(OptionsResolverInterface $resolver);
/**
* Returns the name of the parent type.
*
- * @return string|null The name of the parent type if any, null otherwise.
+ * You can also return a type instance from this method, although doing so
+ * is discouraged because it leads to a performance penalty. The support
+ * for returning type instances may be dropped from future releases.
+ *
+ * @return string|null|FormTypeInterface The name of the parent type if any, null otherwise.
*/
public function getParent();
@@ -0,0 +1,32 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Form\Tests\Fixtures;
+
+use Symfony\Component\Form\AbstractType;
+use Symfony\Component\Form\FormBuilder;
+use Symfony\Component\Form\FormBuilderInterface;
+use Symfony\Component\Form\FormFactoryInterface;
+use Symfony\Component\EventDispatcher\EventDispatcher;
+use Symfony\Component\OptionsResolver\OptionsResolverInterface;
+
+class FooSubTypeWithParentInstance extends AbstractType
+{
+ public function getName()
+ {
+ return 'foo_sub_type_parent_instance';
+ }
+
+ public function getParent()
+ {
+ return new FooType();
+ }
+}
@@ -21,6 +21,8 @@
use Symfony\Component\Form\Tests\Fixtures\AuthorType;
use Symfony\Component\Form\Tests\Fixtures\TestExtension;
use Symfony\Component\Form\Tests\Fixtures\FooType;
+use Symfony\Component\Form\Tests\Fixtures\FooSubType;
+use Symfony\Component\Form\Tests\Fixtures\FooSubTypeWithParentInstance;
use Symfony\Component\Form\Tests\Fixtures\FooTypeBarExtension;
use Symfony\Component\Form\Tests\Fixtures\FooTypeBazExtension;
@@ -139,7 +141,7 @@ public function testCreateNamedBuilderWithTypeName()
public function testCreateNamedBuilderWithTypeInstance()
{
$options = array('a' => '1', 'b' => '2');
- $type = $this->getMockType();
+ $type = new FooType();
$resolvedType = $this->getMockResolvedType();
$this->resolvedTypeFactory->expects($this->once())
@@ -155,6 +157,56 @@ public function testCreateNamedBuilderWithTypeInstance()
$this->assertSame('BUILDER', $this->factory->createNamedBuilder('name', $type, null, $options));
}
+ public function testCreateNamedBuilderWithTypeInstanceWithParentType()
+ {
+ $options = array('a' => '1', 'b' => '2');
+ $type = new FooSubType();
+ $resolvedType = $this->getMockResolvedType();
+ $parentResolvedType = $this->getMockResolvedType();
+
+ $this->registry->expects($this->once())
+ ->method('getType')
+ ->with('foo')
+ ->will($this->returnValue($parentResolvedType));
+
+ $this->resolvedTypeFactory->expects($this->once())
+ ->method('createResolvedType')
+ ->with($type, array(), $parentResolvedType)
+ ->will($this->returnValue($resolvedType));
+
+ $resolvedType->expects($this->once())
+ ->method('createBuilder')
+ ->with($this->factory, 'name', $options)
+ ->will($this->returnValue('BUILDER'));
+
+ $this->assertSame('BUILDER', $this->factory->createNamedBuilder('name', $type, null, $options));
+ }
+
+ public function testCreateNamedBuilderWithTypeInstanceWithParentTypeInstance()
+ {
+ $options = array('a' => '1', 'b' => '2');
+ $type = new FooSubTypeWithParentInstance();
+ $resolvedType = $this->getMockResolvedType();
+ $parentResolvedType = $this->getMockResolvedType();
+
+ $this->resolvedTypeFactory->expects($this->at(0))
+ ->method('createResolvedType')
+ ->with($type->getParent())
+ ->will($this->returnValue($parentResolvedType));
+
+ $this->resolvedTypeFactory->expects($this->at(1))
+ ->method('createResolvedType')
+ ->with($type, array(), $parentResolvedType)
+ ->will($this->returnValue($resolvedType));
+
+ $resolvedType->expects($this->once())
+ ->method('createBuilder')
+ ->with($this->factory, 'name', $options)
+ ->will($this->returnValue('BUILDER'));
+
+ $this->assertSame('BUILDER', $this->factory->createNamedBuilder('name', $type, null, $options));
+ }
+
public function testCreateNamedBuilderWithResolvedTypeInstance()
{
$options = array('a' => '1', 'b' => '2');
@@ -12,6 +12,7 @@
namespace Symfony\Component\Form;
use Symfony\Component\Form\Tests\Fixtures\TestExtension;
+use Symfony\Component\Form\Tests\Fixtures\FooSubTypeWithParentInstance;
use Symfony\Component\Form\Tests\Fixtures\FooSubType;
use Symfony\Component\Form\Tests\Fixtures\FooTypeBazExtension;
use Symfony\Component\Form\Tests\Fixtures\FooTypeBarExtension;
@@ -153,6 +154,35 @@ public function testGetTypeConnectsParent()
$this->assertSame($resolvedType, $this->registry->getType('foo_sub_type'));
}
+ public function testGetTypeConnectsParentIfGetParentReturnsInstance()
+ {
+ $type = new FooSubTypeWithParentInstance();
+ $parentResolvedType = $this->getMock('Symfony\Component\Form\ResolvedFormTypeInterface');
+ $resolvedType = $this->getMock('Symfony\Component\Form\ResolvedFormTypeInterface');
+
+ $this->extension1->addType($type);
+
+ $this->resolvedTypeFactory->expects($this->at(0))
+ ->method('createResolvedType')
+ ->with($this->isInstanceOf('Symfony\Component\Form\Tests\Fixtures\FooType'))
+ ->will($this->returnValue($parentResolvedType));
+
+ $this->resolvedTypeFactory->expects($this->at(1))
+ ->method('createResolvedType')
+ ->with($type, array(), $parentResolvedType)
+ ->will($this->returnValue($resolvedType));
+
+ $parentResolvedType->expects($this->any())
+ ->method('getName')
+ ->will($this->returnValue('foo'));
+
+ $resolvedType->expects($this->any())
+ ->method('getName')
+ ->will($this->returnValue('foo_sub_type_parent_instance'));
+
+ $this->assertSame($resolvedType, $this->registry->getType('foo_sub_type_parent_instance'));
+ }
+
/**
* @expectedException Symfony\Component\Form\Exception\FormException
*/

0 comments on commit a132add

Please sign in to comment.