Browse files

Fix PropelObjectCollection to use the right approach during search()/…

…contains()

Currently; we try to determine if an object is part of a collection by using a
strict comparison, objects have to have the same reference to be considered
identical. It could make sense but in real life, you can get the same
reference for two objects. By using a hash code, we better handle this special
case, only for ActiveRecord objects.
  • Loading branch information...
1 parent 88e21e6 commit 945efa3ec75f29a120a3f7325769e92ea50c96c2 @willdurand willdurand committed Aug 1, 2012
View
2 generator/lib/behavior/nestedset/NestedSetBehaviorObjectBuilderModifier.php
@@ -716,7 +716,7 @@ public function addNestedSetChild($objectName)
if (\$this->collNestedSetChildren === null) {
\$this->initNestedSetChildren();
}
- if (!\$this->collNestedSetChildren->contains($objectName)) { // only add it if the **same** object is not already associated
+ if (!in_array($objectName, \$this->collNestedSetChildren->getArrayCopy(), true)) { // only add it if the **same** object is not already associated
\$this->collNestedSetChildren[]= $objectName;
{$objectName}->setParent(\$this);
}
View
2 generator/lib/builder/om/PHP5ObjectBuilder.php
@@ -3628,7 +3628,7 @@ public function add".$this->getRefFKPhpNameAffix($refFK, $plural = false)."($cla
\$this->init".$this->getRefFKPhpNameAffix($refFK, $plural = true)."();
\$this->{$collName}Partial = true;
}
- if (!\$this->{$collName}->contains(\$l)) { // only add it if the **same** object is not already associated
+ if (!in_array(\$l, \$this->{$collName}->getArrayCopy(), true)) { // only add it if the **same** object is not already associated
\$this->doAdd" . $this->getRefFKPhpNameAffix($refFK, $plural = false) . "(\$l);
}
View
40 runtime/lib/collection/PropelObjectCollection.php
@@ -16,7 +16,6 @@
*/
class PropelObjectCollection extends PropelCollection
{
-
/**
* Save all the elements in the collection
*
@@ -317,4 +316,43 @@ public function populateRelation($relation, $criteria = null, $con = null)
return $relatedObjects;
}
+
+ /**
+ * {@inheritdoc}
+ */
+ public function search($element)
+ {
+ if ($element instanceof BaseObject) {
+ if (null !== $elt = $this->getIdenticalObject($element)) {
+ $element = $elt;
+ }
+ }
+
+ return parent::search($element);
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function contains($element)
+ {
+ if ($element instanceof BaseObject) {
+ if (null !== $elt = $this->getIdenticalObject($element)) {
+ $element = $elt;
+ }
+ }
+
+ return parent::contains($element);
+ }
+
+ private function getIdenticalObject(BaseObject $object)
+ {
+ foreach ($this as $obj) {
+ if ($obj instanceof BaseObject && $obj->hashCode() === $object->hashCode()) {
+ return $obj;
+ }
+ }
+
+ return null;
+ }
}
View
111 runtime/lib/om/BaseObject.php
@@ -149,71 +149,79 @@ public function preSave(PropelPDO $con = null)
* Code to be run after persisting the object
* @param PropelPDO $con
*/
- public function postSave(PropelPDO $con = null) { }
-
- /**
- * Code to be run before inserting to database
- * @param PropelPDO $con
- * @return boolean
- */
- public function preInsert(PropelPDO $con = null)
- {
- return true;
- }
+ public function postSave(PropelPDO $con = null)
+ {
+ }
+
+ /**
+ * Code to be run before inserting to database
+ * @param PropelPDO $con
+ * @return boolean
+ */
+ public function preInsert(PropelPDO $con = null)
+ {
+ return true;
+ }
/**
* Code to be run after inserting to database
* @param PropelPDO $con
*/
- public function postInsert(PropelPDO $con = null) { }
+ public function postInsert(PropelPDO $con = null)
+ {
+ }
- /**
- * Code to be run before updating the object in database
- * @param PropelPDO $con
- * @return boolean
- */
- public function preUpdate(PropelPDO $con = null)
- {
- return true;
- }
+ /**
+ * Code to be run before updating the object in database
+ * @param PropelPDO $con
+ * @return boolean
+ */
+ public function preUpdate(PropelPDO $con = null)
+ {
+ return true;
+ }
/**
* Code to be run after updating the object in database
* @param PropelPDO $con
*/
- public function postUpdate(PropelPDO $con = null) { }
+ public function postUpdate(PropelPDO $con = null)
+ {
+ }
- /**
- * Code to be run before deleting the object in database
- * @param PropelPDO $con
- * @return boolean
- */
- public function preDelete(PropelPDO $con = null)
- {
- return true;
- }
+ /**
+ * Code to be run before deleting the object in database
+ * @param PropelPDO $con
+ * @return boolean
+ */
+ public function preDelete(PropelPDO $con = null)
+ {
+ return true;
+ }
/**
* Code to be run after deleting the object in database
* @param PropelPDO $con
*/
- public function postDelete(PropelPDO $con = null) { }
-
- /**
- * Sets the modified state for the object to be false.
- * @param string $col If supplied, only the specified column is reset.
- * @return void
- */
- public function resetModified($col = null)
- {
- if ($col !== null) {
- while (($offset = array_search($col, $this->modifiedColumns)) !== false) {
- array_splice($this->modifiedColumns, $offset, 1);
- }
- } else {
- $this->modifiedColumns = array();
+ public function postDelete(PropelPDO $con = null)
+ {
+ }
+
+ /**
+ * Sets the modified state for the object to be false.
+ * @param string $col If supplied, only the specified column is reset.
+ * @return void
+ */
+ public function resetModified($col = null)
+ {
+ if ($col !== null) {
+ while (($offset = array_search($col, $this->modifiedColumns)) !== false) {
+ array_splice($this->modifiedColumns, $offset, 1);
}
+ } else {
+ $this->modifiedColumns = array();
}
+ }
/**
* Compares this with another <code>BaseObject</code> instance. If
@@ -240,19 +248,18 @@ public function equals($obj)
}
/**
- * If the primary key is not <code>null</code>, return the hashcode of the
- * primary key. Otherwise calls <code>Object.hashCode()</code>.
+ * If the primary key is not null, return the hashcode of the
+ * primary key. Otherwise, return the hash code of the object.
*
* @return int Hashcode
*/
public function hashCode()
{
- $ok = $this->getPrimaryKey();
- if ($ok === null) {
- return crc32(serialize($this));
+ if (null !== $this->getPrimaryKey()) {
+ return crc32(serialize($this->getPrimaryKey()));
}
- return crc32(serialize($ok)); // serialize because it could be an array ("ComboKey")
+ return crc32(serialize($this));
}
/**
View
2 test/testsuite/runtime/collection/PropelCollectionTest.php
@@ -405,7 +405,7 @@ public function testDiffWithNonEmptyCollectionReturnsObjectsInTheFirstCollection
$result = $col1->diff($col2);
$this->assertInstanceOf('PropelCollection', $result);
- $this->assertEquals(1, count($result));
+ $this->assertCount(1, $result);
$this->assertSame($b1, $result[0]);
}
View
113 test/testsuite/runtime/collection/PropelObjectCollectionTest.php
@@ -170,4 +170,117 @@ public function testToKeyValue()
9012 => 'Don Juan',
), $coll->toKeyValue(array('Author', 'Books', 'First', 'Id'), 'Title'));
}
+
+ public function testContainsWithNoPersistentElements()
+ {
+ $col = new PropelObjectCollection();
+ $this->assertFalse($col->contains('foo_1'), 'contains() returns false on an empty collection');
+ $data = array('bar1', 'bar2', 'bar3');
+ $col = new PropelObjectCollection($data);
+ $this->assertTrue($col->contains('bar1'), 'contains() returns true when the key exists');
+ $this->assertFalse($col->contains('bar4'), 'contains() returns false when the key does not exist');
+ }
+
+ public function testSearchWithNoPersistentElements()
+ {
+ $col = new PropelObjectCollection();
+ $this->assertFalse($col->search('bar1'), 'search() returns false on an empty collection');
+ $data = array('bar1', 'bar2', 'bar3');
+ $col = new PropelObjectCollection($data);
+ $this->assertEquals(1, $col->search('bar2'), 'search() returns the key when the element exists');
+ $this->assertFalse($col->search('bar4'), 'search() returns false when the element does not exist');
+ }
+
+ public function testContainsWithClassicBehavior()
+ {
+ $col = new PropelObjectCollection();
+ $b1 = new Book();
+ $b1->setTitle('Bar');
+ $b2 = new Book();
+ $b2->setTitle('Foo');
+
+ $this->assertFalse($col->contains($b1), 'contains() returns false on an empty collection');
+
+ $col = new PropelObjectCollection(array($b1));
+
+ $this->assertTrue($col->contains($b1), 'contains() returns true when the key exists');
+ $this->assertFalse($col->contains($b2), 'contains() returns false when the key does not exist');
+ }
+
+ public function testSearchWithClassicBehavior()
+ {
+ $col = new PropelObjectCollection();
+ $b1 = new Book();
+ $b1->setTitle('Bar');
+ $b2 = new Book();
+ $b2->setTitle('Foo');
+
+ $this->assertFalse($col->search($b1), 'search() returns false on an empty collection');
+
+ $col = new PropelObjectCollection(array($b1));
+ $this->assertEquals(0, $col->search($b1), 'search() returns the key when the element exists');
+ $this->assertFalse($col->search($b2), 'search() returns false when the element does not exist');
+ }
+
+ public function testContainsMatchesSimilarObjects()
+ {
+ $col = new PropelObjectCollection();
+ $b1 = new Book();
+ $b1->setTitle('Bar');
+ $b1->save();
+
+ $b2 = clone $b1;
+
+ $this->assertFalse($col->contains($b1), 'contains() returns false on an empty collection');
+
+ $col = new PropelObjectCollection(array($b1));
+
+ $this->assertTrue($col->contains($b1));
+ $this->assertTrue($col->contains($b2));
+ }
+
+ public function testSearchMatchesSimilarObjects()
+ {
+ $col = new PropelObjectCollection();
+ $b1 = new Book();
+ $b1->setTitle('Bar');
+ $b1->save();
+
+ $b2 = clone $b1;
+
+ $this->assertFalse($col->search($b1), 'search() returns false on an empty collection');
+
+ $col = new PropelObjectCollection(array($b1));
+ $this->assertTrue(0 === $col->search($b1));
+ $this->assertTrue(0 === $col->search($b2));
+ }
+
+ public function testContainsMatchesSimilarNewObjects()
+ {
+ $col = new PropelObjectCollection();
+ $b1 = new Book();
+ $b1->setTitle('Bar');
+ $b2 = clone $b1;
+
+ $this->assertFalse($col->contains($b1), 'contains() returns false on an empty collection');
+
+ $col = new PropelObjectCollection(array($b1));
+
+ $this->assertTrue($col->contains($b1));
+ $this->assertTrue($col->contains($b2));
+ }
+
+ public function testSearchMatchesSimilarNewObjects()
+ {
+ $col = new PropelObjectCollection();
+ $b1 = new Book();
+ $b1->setTitle('Bar');
+ $b2 = clone $b1;
+
+ $this->assertFalse($col->search($b1), 'search() returns false on an empty collection');
+
+ $col = new PropelObjectCollection(array($b1));
+ $this->assertTrue(0 === $col->search($b1));
+ $this->assertTrue(0 === $col->search($b2));
+ }
}

0 comments on commit 945efa3

Please sign in to comment.