merged branch bschussek/issue4213 (PR #4284)

Commits
-------

9215c44 [Form] fix failing tests for remove call on an objectCollection
076a104 [Form] Created failing test for PropertyPath modifying collections while iterating them

Discussion
----------

[Form] Fixes PropertyPath not to remove elements while iterating a collection

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

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue4213)

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

by travisbot at 2012-05-14T18:56:29Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1329584) (merged 9215c447 into 46ffbd52).

src/Symfony/Component/Form/Tests/Util/PropertyPathCollectionTest.php

@@ -15,12 +15,34 @@ use Symfony\Component\Form\Util\PropertyPath;
 
 class PropertyPathCollectionTest_Car
 {
+    private $axes;
+
+    public function __construct($axes = null)
+    {
+        $this->axes = $axes;
+    }
+
     // In the test, use a name that FormUtil can't uniquely singularify
-    public function addAxis($axis) {}
+    public function addAxis($axis)
+    {
+        $this->axes[] = $axis;
+    }
 
-    public function removeAxis($axis) {}
+    public function removeAxis($axis)
+    {
+        foreach ($this->axes as $key => $value) {
+            if ($value === $axis) {
+                unset($this->axes[$key]);
 
-    public function getAxes() {}
+                return;
+            }
+        }
+    }
+
+    public function getAxes()
+    {
+        return $this->axes;
+    }
 }
 
 class PropertyPathCollectionTest_CarCustomSingular
@@ -70,26 +92,19 @@ abstract class PropertyPathCollectionTest extends \PHPUnit_Framework_TestCase
 
     public function testSetValueCallsAdderAndRemoverForCollections()
     {
-        $car = $this->getMock(__CLASS__ . '_Car');
-        $axesBefore = $this->getCollection(array(1 => 'second', 3 => 'fourth'));
-        $axesAfter = $this->getCollection(array(0 => 'first', 1 => 'second', 2 => 'third'));
+        $axesBefore = $this->getCollection(array(1 => 'second', 3 => 'fourth', 4 => 'fifth'));
+        $axesMerged = $this->getCollection(array(1 => 'first', 2 => 'second', 3 => 'third'));
+        $axesAfter = $this->getCollection(array(1 => 'second', 5 => 'first', 6 => 'third'));
+
+        // Don't use a mock in order to test whether the collections are
+        // modified while iterating them
+        $car = new PropertyPathCollectionTest_Car($axesBefore);
 
         $path = new PropertyPath('axes');
 
-        $car->expects($this->at(0))
-            ->method('getAxes')
-            ->will($this->returnValue($axesBefore));
-        $car->expects($this->at(1))
-            ->method('removeAxis')
-            ->with('fourth');
-        $car->expects($this->at(2))
-            ->method('addAxis')
-            ->with('first');
-        $car->expects($this->at(3))
-            ->method('addAxis')
-            ->with('third');
+        $path->setValue($car, $axesMerged);
 
-        $path->setValue($car, $axesAfter);
+        $this->assertEquals($axesAfter, $car->getAxes());
     }
 
     public function testSetValueCallsAdderAndRemoverForNestedCollections()

src/Symfony/Component/Form/Util/PropertyPath.php

@@ -489,6 +489,7 @@ class PropertyPath implements \IteratorAggregate
             // Collection with matching adder/remover in $objectOrArray
             if ($addMethod && $removeMethod) {
                 $itemsToAdd = is_object($value) ? clone $value : $value;
+                $itemToRemove = array();
                 $previousValue = $this->readProperty($objectOrArray, $currentIndex);
 
                 if (is_array($previousValue) || $previousValue instanceof Traversable) {
@@ -503,11 +504,15 @@ class PropertyPath implements \IteratorAggregate
                             }
                         }
 
-                        // Item not found, remove
-                        $objectOrArray->$removeMethod($previousItem);
+                        // Item not found, add to remove list
+                        $itemToRemove[] = $previousItem;
                     }
                 }
 
+                foreach ($itemToRemove as $item) {
+                    $objectOrArray->$removeMethod($item);
+                }
+
                 foreach ($itemsToAdd as $item) {
                     $objectOrArray->$addMethod($item);
                 }