bug #20762 [Form] Fix FormDataCollector (nicolas-grekas, Padam87)

This PR was merged into the 3.2 branch. Discussion ---------- [Form] Fix FormDataCollector | Q | A | ------------- | --- | Branch? | 3.2 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #20698 | License | MIT | Doc PR | - Alternative to #20707 Commits ------- 50400c4 [Form] Add failing test for data collector bug 164a20c [Form] Fix FormDataCollector
symfony · Dec 6, 2016 · 63087e5 · 63087e5
2 parents 456e68b + 50400c4
commit 63087e5
Show file tree

Hide file tree

Showing 2 changed files with 123 additions and 19 deletions.
diff --git a/src/Symfony/Component/Form/Extension/DataCollector/FormDataCollector.php b/src/Symfony/Component/Form/Extension/DataCollector/FormDataCollector.php
@@ -209,19 +209,15 @@ public function collectViewVariables(FormView $view)
      */
     public function buildPreliminaryFormTree(FormInterface $form)
     {
-        $this->data['forms'][$form->getName()] = array();
-
-        $this->recursiveBuildPreliminaryFormTree($form, $this->data['forms'][$form->getName()], $this->data['forms_by_hash']);
+        $this->data['forms'][$form->getName()] = &$this->recursiveBuildPreliminaryFormTree($form, $this->data['forms_by_hash']);
     }
 
     /**
      * {@inheritdoc}
      */
     public function buildFinalFormTree(FormInterface $form, FormView $view)
     {
-        $this->data['forms'][$form->getName()] = array();
-
-        $this->recursiveBuildFinalFormTree($form, $view, $this->data['forms'][$form->getName()], $this->data['forms_by_hash']);
+        $this->data['forms'][$form->getName()] = &$this->recursiveBuildFinalFormTree($form, $view, $this->data['forms_by_hash']);
     }
 
     /**
@@ -258,7 +254,7 @@ public function serialize()
                     case 'resolved_options':
                     case 'default_data':
                     case 'submitted_data':
-                        if ($v) {
+                        if ($v && is_array($v)) {
                             $form[$k] = array_map($cloneVar, $v);
                         }
                         break;
@@ -354,26 +350,25 @@ protected function cloneVar($var, $isClass = false)
         return $cache = $this->cloner->cloneVar($var);
     }
 
-    private function recursiveBuildPreliminaryFormTree(FormInterface $form, &$output, array &$outputByHash)
+    private function &recursiveBuildPreliminaryFormTree(FormInterface $form, array &$outputByHash)
     {
         $hash = spl_object_hash($form);
 
+        $output = &$outputByHash[$hash];
         $output = isset($this->dataByForm[$hash])
             ? $this->dataByForm[$hash]
             : array();
 
-        $outputByHash[$hash] = &$output;
-
         $output['children'] = array();
 
         foreach ($form as $name => $child) {
-            $output['children'][$name] = array();
-
-            $this->recursiveBuildPreliminaryFormTree($child, $output['children'][$name], $outputByHash);
+            $output['children'][$name] = &$this->recursiveBuildPreliminaryFormTree($child, $outputByHash);
         }
+
+        return $output;
     }
 
-    private function recursiveBuildFinalFormTree(FormInterface $form = null, FormView $view, &$output, array &$outputByHash)
+    private function &recursiveBuildFinalFormTree(FormInterface $form = null, FormView $view, array &$outputByHash)
     {
         $viewHash = spl_object_hash($view);
         $formHash = null;
@@ -386,6 +381,9 @@ private function recursiveBuildFinalFormTree(FormInterface $form = null, FormVie
             // corresponding FormInterface instance for its view in a different way
             $formHash = $this->formsByView[$viewHash];
         }
+        if (null !== $formHash) {
+            $output = &$outputByHash[$formHash];
+        }
 
         $output = isset($this->dataByView[$viewHash])
             ? $this->dataByView[$viewHash]
@@ -398,8 +396,6 @@ private function recursiveBuildFinalFormTree(FormInterface $form = null, FormVie
                     ? $this->dataByForm[$formHash]
                     : array()
             );
-
-            $outputByHash[$formHash] = &$output;
         }
 
         $output['children'] = array();
@@ -411,9 +407,9 @@ private function recursiveBuildFinalFormTree(FormInterface $form = null, FormVie
                 ? $form->get($name)
                 : null;
 
-            $output['children'][$name] = array();
-
-            $this->recursiveBuildFinalFormTree($childForm, $childView, $output['children'][$name], $outputByHash);
+            $output['children'][$name] = &$this->recursiveBuildFinalFormTree($childForm, $childView, $outputByHash);
         }
+
+        return $output;
     }
 }
diff --git a/src/Symfony/Component/Form/Tests/Extension/DataCollector/FormDataCollectorTest.php b/src/Symfony/Component/Form/Tests/Extension/DataCollector/FormDataCollectorTest.php
@@ -342,6 +342,114 @@ public function testBuildFinalFormTree()
         ), $this->dataCollector->getData());
     }
 
+    public function testSerializeWithFormAddedMultipleTimes()
+    {
+        $form1 = $this->createForm('form1');
+        $form2 = $this->createForm('form2');
+        $child1 = $this->createForm('child1');
+
+        $form1View = new FormView();
+        $form2View = new FormView();
+        $child1View = new FormView();
+        $child1View->vars['is_selected'] = function ($choice, array $values) {
+            return in_array($choice, $values, true);
+        };
+
+        $form1->add($child1);
+        $form2->add($child1);
+
+        $form1View->children['child1'] = $child1View;
+        $form2View->children['child1'] = $child1View;
+
+        $this->dataExtractor->expects($this->at(0))
+            ->method('extractConfiguration')
+            ->with($form1)
+            ->will($this->returnValue(array('config' => 'foo')));
+        $this->dataExtractor->expects($this->at(1))
+            ->method('extractConfiguration')
+            ->with($child1)
+            ->will($this->returnValue(array('config' => 'bar')));
+
+        $this->dataExtractor->expects($this->at(2))
+            ->method('extractDefaultData')
+            ->with($form1)
+            ->will($this->returnValue(array('default_data' => 'foo')));
+        $this->dataExtractor->expects($this->at(3))
+            ->method('extractDefaultData')
+            ->with($child1)
+            ->will($this->returnValue(array('default_data' => 'bar')));
+
+        $this->dataExtractor->expects($this->at(4))
+            ->method('extractSubmittedData')
+            ->with($form1)
+            ->will($this->returnValue(array('submitted_data' => 'foo')));
+        $this->dataExtractor->expects($this->at(5))
+            ->method('extractSubmittedData')
+            ->with($child1)
+            ->will($this->returnValue(array('submitted_data' => 'bar')));
+
+        $this->dataExtractor->expects($this->at(6))
+            ->method('extractViewVariables')
+            ->with($form1View)
+            ->will($this->returnValue(array('view_vars' => 'foo')));
+
+        $this->dataExtractor->expects($this->at(7))
+            ->method('extractViewVariables')
+            ->with($child1View)
+            ->will($this->returnValue(array('view_vars' => $child1View->vars)));
+
+        $this->dataExtractor->expects($this->at(8))
+            ->method('extractConfiguration')
+            ->with($form2)
+            ->will($this->returnValue(array('config' => 'foo')));
+        $this->dataExtractor->expects($this->at(9))
+            ->method('extractConfiguration')
+            ->with($child1)
+            ->will($this->returnValue(array('config' => 'bar')));
+
+        $this->dataExtractor->expects($this->at(10))
+            ->method('extractDefaultData')
+            ->with($form2)
+            ->will($this->returnValue(array('default_data' => 'foo')));
+        $this->dataExtractor->expects($this->at(11))
+            ->method('extractDefaultData')
+            ->with($child1)
+            ->will($this->returnValue(array('default_data' => 'bar')));
+
+        $this->dataExtractor->expects($this->at(12))
+            ->method('extractSubmittedData')
+            ->with($form2)
+            ->will($this->returnValue(array('submitted_data' => 'foo')));
+        $this->dataExtractor->expects($this->at(13))
+            ->method('extractSubmittedData')
+            ->with($child1)
+            ->will($this->returnValue(array('submitted_data' => 'bar')));
+
+        $this->dataExtractor->expects($this->at(14))
+            ->method('extractViewVariables')
+            ->with($form2View)
+            ->will($this->returnValue(array('view_vars' => 'foo')));
+
+        $this->dataExtractor->expects($this->at(15))
+            ->method('extractViewVariables')
+            ->with($child1View)
+            ->will($this->returnValue(array('view_vars' => $child1View->vars)));
+
+        $this->dataCollector->collectConfiguration($form1);
+        $this->dataCollector->collectDefaultData($form1);
+        $this->dataCollector->collectSubmittedData($form1);
+        $this->dataCollector->collectViewVariables($form1View);
+        $this->dataCollector->buildFinalFormTree($form1, $form1View);
+
+        $this->dataCollector->collectConfiguration($form2);
+        $this->dataCollector->collectDefaultData($form2);
+        $this->dataCollector->collectSubmittedData($form2);
+        $this->dataCollector->collectViewVariables($form2View);
+        $this->dataCollector->buildFinalFormTree($form2, $form2View);
+
+        $this->dataCollector->serialize();
+    }
+
     public function testFinalFormReliesOnFormViewStructure()
     {
         $this->form->add($child1 = $this->createForm('first'));