From 6b176406479f620bcd3bc74bc03696a1a3527368 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Wed, 25 Jul 2012 13:42:52 +0200 Subject: [PATCH 1/2] [Form] Fixed caching of block names when types of forms with the same unique block ID differ --- .../Component/Form/AbstractRendererEngine.php | 2 +- .../Form/Extension/Core/Type/FormType.php | 7 +++++ .../Form/Tests/AbstractDivLayoutTest.php | 24 +++++++++++++++++ .../Tests/Fixtures/AlternatingRowType.php | 27 +++++++++++++++++++ 4 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 src/Symfony/Component/Form/Tests/Fixtures/AlternatingRowType.php diff --git a/src/Symfony/Component/Form/AbstractRendererEngine.php b/src/Symfony/Component/Form/AbstractRendererEngine.php index 5d7416d2a8..9e16b44cfe 100644 --- a/src/Symfony/Component/Form/AbstractRendererEngine.php +++ b/src/Symfony/Component/Form/AbstractRendererEngine.php @@ -21,7 +21,7 @@ abstract class AbstractRendererEngine implements FormRendererEngineInterface /** * The variable in {@link FormView} used as cache key. */ - const CACHE_KEY_VAR = 'full_block_name'; + const CACHE_KEY_VAR = 'cache_key'; /** * @var array diff --git a/src/Symfony/Component/Form/Extension/Core/Type/FormType.php b/src/Symfony/Component/Form/Extension/Core/Type/FormType.php index 5deadf429a..c96ed1382f 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/FormType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/FormType.php @@ -129,6 +129,13 @@ class FormType extends AbstractType 'compound' => $form->getConfig()->getCompound(), 'types' => $types, 'translation_domain' => $translationDomain, + // Using the block name here speeds up performance in collection + // forms, where each entry has the same full block name. + // Including the type is important too, because if rows of a + // collection form have different types (dynamically), they should + // be rendered differently. + // https://github.com/symfony/symfony/issues/5038 + 'cache_key' => $fullBlockName . '_' . $form->getConfig()->getType()->getName(), )); } diff --git a/src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php b/src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php index 75e8e193e4..163539522b 100644 --- a/src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php +++ b/src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Form\Tests; use Symfony\Component\Form\FormError; +use Symfony\Component\Form\Tests\Fixtures\AlternatingRowType; abstract class AbstractDivLayoutTest extends AbstractLayoutTest { @@ -283,6 +284,29 @@ abstract class AbstractDivLayoutTest extends AbstractLayoutTest ); } + // https://github.com/symfony/symfony/issues/5038 + public function testCollectionWithAlternatingRowTypes() + { + $data = array( + array('title' => 'a'), + array('title' => 'b'), + ); + $form = $this->factory->createNamed('name', 'collection', $data, array( + 'type' => new AlternatingRowType(), + )); + + $this->assertWidgetMatchesXpath($form->createView(), array(), +'/div + [ + ./div[./div/div/input[@type="text"][@value="a"]] + /following-sibling::div[./div/div/textarea[.="b"]] + ] + [count(./div[./div/div/input])=1] + [count(./div[./div/div/textarea])=1] +' + ); + } + public function testEmptyCollection() { $form = $this->factory->createNamed('name', 'collection', array(), array( diff --git a/src/Symfony/Component/Form/Tests/Fixtures/AlternatingRowType.php b/src/Symfony/Component/Form/Tests/Fixtures/AlternatingRowType.php new file mode 100644 index 0000000000..e53d19ffe9 --- /dev/null +++ b/src/Symfony/Component/Form/Tests/Fixtures/AlternatingRowType.php @@ -0,0 +1,27 @@ +getFormFactory(); + + $builder->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event) use ($formFactory) { + $form = $event->getForm(); + $type = $form->getName() % 2 === 0 ? 'text' : 'textarea'; + $form->add($formFactory->createNamed('title', $type)); + }); + } + + public function getName() + { + return 'alternating_row'; + } +} From eeb66dd2efa8cfd12619d0b294c5c2decba49ca4 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Wed, 25 Jul 2012 13:48:48 +0200 Subject: [PATCH 2/2] [Form] Renamed the internal FormView variables "types" and "full_block_name" --- .../Form/Extension/Core/Type/FormType.php | 57 ++++++++++--------- src/Symfony/Component/Form/FormRenderer.php | 7 +-- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/src/Symfony/Component/Form/Extension/Core/Type/FormType.php b/src/Symfony/Component/Form/Extension/Core/Type/FormType.php index c96ed1382f..5650537917 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/FormType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/FormType.php @@ -71,11 +71,11 @@ class FormType extends AbstractType if ('' !== ($parentFullName = $view->parent->vars['full_name'])) { $id = sprintf('%s_%s', $view->parent->vars['id'], $name); $fullName = sprintf('%s[%s]', $parentFullName, $name); - $fullBlockName = sprintf('%s_%s', $view->parent->vars['full_block_name'], $blockName); + $uniqueBlockPrefix = sprintf('%s_%s', $view->parent->vars['unique_block_prefix'], $blockName); } else { $id = $name; $fullName = $name; - $fullBlockName = '_' . $blockName; + $uniqueBlockPrefix = '_' . $blockName; } // Complex fields are read-only if they themselves or their parents are. @@ -89,7 +89,7 @@ class FormType extends AbstractType } else { $id = $name; $fullName = $name; - $fullBlockName = '_' . $blockName; + $uniqueBlockPrefix = '_' . $blockName; // Strip leading underscores and digits. These are allowed in // form names, but not in HTML4 ID attributes. @@ -97,45 +97,46 @@ class FormType extends AbstractType $id = ltrim($id, '_0123456789'); } - $types = array(); + $blockPrefixes = array(); for ($type = $form->getConfig()->getType(); null !== $type; $type = $type->getParent()) { - array_unshift($types, $type->getName()); + array_unshift($blockPrefixes, $type->getName()); } + $blockPrefixes[] = $uniqueBlockPrefix; if (!$translationDomain) { $translationDomain = 'messages'; } $view->vars = array_replace($view->vars, array( - 'form' => $view, - 'id' => $id, - 'name' => $name, - 'full_name' => $fullName, - 'full_block_name' => $fullBlockName, - 'read_only' => $readOnly, - 'errors' => $form->getErrors(), - 'valid' => $form->isBound() ? $form->isValid() : true, - 'value' => $form->getViewData(), - 'data' => $form->getNormData(), - 'disabled' => $form->isDisabled(), - 'required' => $form->isRequired(), - 'max_length' => $options['max_length'], - 'pattern' => $options['pattern'], - 'size' => null, - 'label' => $options['label'], - 'multipart' => false, - 'attr' => $options['attr'], - 'label_attr' => $options['label_attr'], - 'compound' => $form->getConfig()->getCompound(), - 'types' => $types, - 'translation_domain' => $translationDomain, + 'form' => $view, + 'id' => $id, + 'name' => $name, + 'full_name' => $fullName, + 'read_only' => $readOnly, + 'errors' => $form->getErrors(), + 'valid' => $form->isBound() ? $form->isValid() : true, + 'value' => $form->getViewData(), + 'data' => $form->getNormData(), + 'disabled' => $form->isDisabled(), + 'required' => $form->isRequired(), + 'max_length' => $options['max_length'], + 'pattern' => $options['pattern'], + 'size' => null, + 'label' => $options['label'], + 'multipart' => false, + 'attr' => $options['attr'], + 'label_attr' => $options['label_attr'], + 'compound' => $form->getConfig()->getCompound(), + 'block_prefixes' => $blockPrefixes, + 'unique_block_prefix' => $uniqueBlockPrefix, + 'translation_domain' => $translationDomain, // Using the block name here speeds up performance in collection // forms, where each entry has the same full block name. // Including the type is important too, because if rows of a // collection form have different types (dynamically), they should // be rendered differently. // https://github.com/symfony/symfony/issues/5038 - 'cache_key' => $fullBlockName . '_' . $form->getConfig()->getType()->getName(), + 'cache_key' => $uniqueBlockPrefix . '_' . $form->getConfig()->getType()->getName(), )); } diff --git a/src/Symfony/Component/Form/FormRenderer.php b/src/Symfony/Component/Form/FormRenderer.php index d464d314ee..bfadb6c93c 100644 --- a/src/Symfony/Component/Form/FormRenderer.php +++ b/src/Symfony/Component/Form/FormRenderer.php @@ -22,7 +22,7 @@ use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface; */ class FormRenderer implements FormRendererInterface { - const CACHE_KEY_VAR = 'full_block_name'; + const CACHE_KEY_VAR = 'unique_block_prefix'; /** * @var FormRendererEngineInterface @@ -173,10 +173,9 @@ class FormRenderer implements FormRendererInterface // Calculate the hierarchy of template blocks and start on // the bottom level of the hierarchy (= "__
" block) $blockNameHierarchy = array(); - foreach ($view->vars['types'] as $type) { - $blockNameHierarchy[] = $type . '_' . $blockNameSuffix; + foreach ($view->vars['block_prefixes'] as $blockNamePrefix) { + $blockNameHierarchy[] = $blockNamePrefix . '_' . $blockNameSuffix; } - $blockNameHierarchy[] = $view->vars['full_block_name'] . '_' . $blockNameSuffix; $hierarchyLevel = count($blockNameHierarchy) - 1; $hierarchyInit = true;