Commit 31892fb4 authored by romanb's avatar romanb

[2.0] Fixed cascading issue (#2307). Fixed many-many object hydration issue.

parent 3f4cd46b
...@@ -123,7 +123,6 @@ class ObjectHydrator extends AbstractHydrator ...@@ -123,7 +123,6 @@ class ObjectHydrator extends AbstractHydrator
// Take snapshots from all initialized collections // Take snapshots from all initialized collections
foreach ($this->_collections as $coll) { foreach ($this->_collections as $coll) {
$coll->takeSnapshot(); $coll->takeSnapshot();
$coll->setHydrationFlag(false);
} }
// Clean up // Clean up
...@@ -344,7 +343,7 @@ class ObjectHydrator extends AbstractHydrator ...@@ -344,7 +343,7 @@ class ObjectHydrator extends AbstractHydrator
if ( ! $relation->isOneToOne()) { if ( ! $relation->isOneToOne()) {
if (isset($nonemptyComponents[$dqlAlias])) { if (isset($nonemptyComponents[$dqlAlias])) {
if ( ! isset($this->_initializedRelations[spl_object_hash($baseElement)][$relationAlias])) { if ( ! isset($this->_initializedRelations[spl_object_hash($baseElement)][$relationAlias])) {
$this->initRelatedCollection($baseElement, $relationAlias)->setHydrationFlag(true); $this->initRelatedCollection($baseElement, $relationAlias);
} }
$path = $parent . '.' . $dqlAlias; $path = $parent . '.' . $dqlAlias;
...@@ -357,12 +356,19 @@ class ObjectHydrator extends AbstractHydrator ...@@ -357,12 +356,19 @@ class ObjectHydrator extends AbstractHydrator
// If it's a bi-directional many-to-many, also initialize the reverse collection. // If it's a bi-directional many-to-many, also initialize the reverse collection.
if ($relation->isManyToMany()) { if ($relation->isManyToMany()) {
if ($relation->isOwningSide && isset($this->_ce[$entityName]->inverseMappings[$relationAlias])) { if ($relation->isOwningSide && isset($this->_ce[$entityName]->inverseMappings[$relationAlias])) {
$inverseFieldName = $this->_ce[$entityName]->inverseMappings[$relationAlias]->sourceFieldName;
// Only initialize reverse collection if it is not yet initialized.
if ( ! isset($this->_initializedRelations[spl_object_hash($element)][$inverseFieldName])) {
$this->initRelatedCollection($element, $this->_ce[$entityName] $this->initRelatedCollection($element, $this->_ce[$entityName]
->inverseMappings[$relationAlias]->sourceFieldName); ->inverseMappings[$relationAlias]->sourceFieldName);
}
} else if ($relation->mappedByFieldName) { } else if ($relation->mappedByFieldName) {
// Only initialize reverse collection if it is not yet initialized.
if ( ! isset($this->_initializedRelations[spl_object_hash($element)][$relation->mappedByFieldName])) {
$this->initRelatedCollection($element, $relation->mappedByFieldName); $this->initRelatedCollection($element, $relation->mappedByFieldName);
} }
} }
}
if ($field = $this->_getCustomIndexField($dqlAlias)) { if ($field = $this->_getCustomIndexField($dqlAlias)) {
$indexValue = $this->_ce[$entityName] $indexValue = $this->_ce[$entityName]
...@@ -371,12 +377,12 @@ class ObjectHydrator extends AbstractHydrator ...@@ -371,12 +377,12 @@ class ObjectHydrator extends AbstractHydrator
$this->_ce[$parentClass] $this->_ce[$parentClass]
->reflFields[$relationAlias] ->reflFields[$relationAlias]
->getValue($baseElement) ->getValue($baseElement)
->set($indexValue, $element); ->hydrateSet($indexValue, $element);
} else { } else {
$this->_ce[$parentClass] $this->_ce[$parentClass]
->reflFields[$relationAlias] ->reflFields[$relationAlias]
->getValue($baseElement) ->getValue($baseElement)
->add($element); ->hydrateAdd($element);
} }
$this->_identifierMap[$path][$id[$parent]][$id[$dqlAlias]] = $this->getLastKey( $this->_identifierMap[$path][$id[$parent]][$id[$dqlAlias]] = $this->getLastKey(
$this->_ce[$parentClass] $this->_ce[$parentClass]
......
...@@ -96,14 +96,6 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection ...@@ -96,14 +96,6 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection
*/ */
private $_backRefFieldName; private $_backRefFieldName;
/**
* Hydration flag.
*
* @var boolean
* @see setHydrationFlag()
*/
private $_hydrationFlag = false;
/** /**
* The class descriptor of the owning entity. * The class descriptor of the owning entity.
*/ */
...@@ -170,8 +162,7 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection ...@@ -170,8 +162,7 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection
$targetClass = $this->_em->getClassMetadata($assoc->targetEntityName); $targetClass = $this->_em->getClassMetadata($assoc->targetEntityName);
if (isset($targetClass->inverseMappings[$assoc->sourceFieldName])) { if (isset($targetClass->inverseMappings[$assoc->sourceFieldName])) {
// Bi-directional // Bi-directional
$this->_backRefFieldName = $targetClass->inverseMappings[$assoc->sourceFieldName] $this->_backRefFieldName = $targetClass->inverseMappings[$assoc->sourceFieldName]->sourceFieldName;
->sourceFieldName;
} }
} }
} }
...@@ -228,10 +219,8 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection ...@@ -228,10 +219,8 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection
public function set($key, $value) public function set($key, $value)
{ {
parent::set($key, $value); parent::set($key, $value);
if ( ! $this->_hydrationFlag) {
$this->_changed(); $this->_changed();
} }
}
/** /**
* Adds an element to the collection. * Adds an element to the collection.
...@@ -244,11 +233,23 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection ...@@ -244,11 +233,23 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection
public function add($value) public function add($value)
{ {
parent::add($value); parent::add($value);
$this->_changed();
return true;
}
if ($this->_hydrationFlag) { /**
* INTERNAL:
* Adds an element to a collection during hydration.
*
* @param mixed $value The element to add.
*/
public function hydrateAdd($value)
{
parent::add($value);
if ($this->_backRefFieldName) { if ($this->_backRefFieldName) {
// Set back reference to owner // Set back reference to owner
if ($this->_association->isOneToMany()) { if ($this->_association->isOneToMany()) {
// OneToMany
$this->_typeClass->getReflectionProperty($this->_backRefFieldName) $this->_typeClass->getReflectionProperty($this->_backRefFieldName)
->setValue($value, $this->_owner); ->setValue($value, $this->_owner);
} else { } else {
...@@ -257,26 +258,18 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection ...@@ -257,26 +258,18 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection
->getValue($value)->add($this->_owner); ->getValue($value)->add($this->_owner);
} }
} }
} else {
$this->_changed();
}
return true;
} }
/** /**
* Adds all elements of the other collection to this collection. * INTERNAL:
* Sets a keyed element in the collection during hydration.
* *
* @param object $otherCollection * @param mixed $key The key to set.
* @todo Impl * $param mixed $value The element to set.
* @override
*/ */
public function addAll($otherCollection) public function hydrateSet($key, $value)
{ {
parent::addAll($otherCollection); parent::set($key, $value);
//...
//TODO: Register collection as dirty with the UoW if necessary
//$this->_changed();
} }
/** /**
...@@ -314,25 +307,6 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection ...@@ -314,25 +307,6 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection
} }
/**
* INTERNAL:
* Sets a flag that indicates whether the collection is currently being hydrated.
*
* If the flag is set to TRUE, this has the following consequences:
*
* 1) During hydration, bidirectional associations are completed automatically
* by setting the back reference.
* 2) During hydration no change notifications are reported to the UnitOfWork.
* That means add() etc. do not cause the collection to be scheduled
* for an update.
*
* @param boolean $bool
*/
public function setHydrationFlag($bool)
{
$this->_hydrationFlag = $bool;
}
/** /**
* INTERNAL: * INTERNAL:
* Tells this collection to take a snapshot of its current state. * Tells this collection to take a snapshot of its current state.
......
...@@ -1148,11 +1148,11 @@ class UnitOfWork implements PropertyChangedListener ...@@ -1148,11 +1148,11 @@ class UnitOfWork implements PropertyChangedListener
} }
$relatedEntities = $class->reflFields[$assocMapping->getSourceFieldName()] $relatedEntities = $class->reflFields[$assocMapping->getSourceFieldName()]
->getValue($entity); ->getValue($entity);
if (($relatedEntities instanceof Collection) && count($relatedEntities) > 0) { if ($relatedEntities instanceof Collection) {
foreach ($relatedEntities as $relatedEntity) { foreach ($relatedEntities as $relatedEntity) {
$this->_doMerge($relatedEntity, $visited, $managedCopy, $assocMapping); $this->_doMerge($relatedEntity, $visited, $managedCopy, $assocMapping);
} }
} else if (is_object($relatedEntities)) { } else if ($relatedEntities !== null) {
$this->_doMerge($relatedEntities, $visited, $managedCopy, $assocMapping); $this->_doMerge($relatedEntities, $visited, $managedCopy, $assocMapping);
} }
} }
...@@ -1173,12 +1173,11 @@ class UnitOfWork implements PropertyChangedListener ...@@ -1173,12 +1173,11 @@ class UnitOfWork implements PropertyChangedListener
continue; continue;
} }
$relatedEntities = $class->reflFields[$assocMapping->sourceFieldName]->getValue($entity); $relatedEntities = $class->reflFields[$assocMapping->sourceFieldName]->getValue($entity);
if (($relatedEntities instanceof Collection || is_array($relatedEntities)) if (($relatedEntities instanceof Collection || is_array($relatedEntities))) {
&& count($relatedEntities) > 0) {
foreach ($relatedEntities as $relatedEntity) { foreach ($relatedEntities as $relatedEntity) {
$this->_doSave($relatedEntity, $visited, $insertNow); $this->_doSave($relatedEntity, $visited, $insertNow);
} }
} else if (is_object($relatedEntities)) { } else if ($relatedEntities !== null) {
$this->_doSave($relatedEntities, $visited, $insertNow); $this->_doSave($relatedEntities, $visited, $insertNow);
} }
} }
...@@ -1199,12 +1198,11 @@ class UnitOfWork implements PropertyChangedListener ...@@ -1199,12 +1198,11 @@ class UnitOfWork implements PropertyChangedListener
} }
$relatedEntities = $class->reflFields[$assocMapping->sourceFieldName] $relatedEntities = $class->reflFields[$assocMapping->sourceFieldName]
->getValue($entity); ->getValue($entity);
if ($relatedEntities instanceof Collection || is_array($relatedEntities) if ($relatedEntities instanceof Collection || is_array($relatedEntities)) {
&& count($relatedEntities) > 0) {
foreach ($relatedEntities as $relatedEntity) { foreach ($relatedEntities as $relatedEntity) {
$this->_doDelete($relatedEntity, $visited); $this->_doDelete($relatedEntity, $visited);
} }
} else if (is_object($relatedEntities)) { } else if ($relatedEntities !== null) {
$this->_doDelete($relatedEntities, $visited); $this->_doDelete($relatedEntities, $visited);
} }
} }
......
...@@ -31,4 +31,8 @@ class CmsArticle ...@@ -31,4 +31,8 @@ class CmsArticle
* @OneToMany(targetEntity="CmsComment", mappedBy="article") * @OneToMany(targetEntity="CmsComment", mappedBy="article")
*/ */
public $comments; public $comments;
public function setAuthor(CmsUser $author) {
$this->user = $author;
}
} }
...@@ -9,8 +9,7 @@ namespace Doctrine\Tests\Models\CMS; ...@@ -9,8 +9,7 @@ namespace Doctrine\Tests\Models\CMS;
class CmsPhonenumber class CmsPhonenumber
{ {
/** /**
* @Column(type="string", length=50) * @Id @Column(type="string", length=50)
* @Id
*/ */
public $phonenumber; public $phonenumber;
/** /**
...@@ -21,6 +20,9 @@ class CmsPhonenumber ...@@ -21,6 +20,9 @@ class CmsPhonenumber
public function setUser(CmsUser $user) { public function setUser(CmsUser $user) {
$this->user = $user; $this->user = $user;
$user->addPhonenumber($this); }
public function getUser() {
return $this->user;
} }
} }
...@@ -68,9 +68,7 @@ class CmsUser ...@@ -68,9 +68,7 @@ class CmsUser
*/ */
public function addPhonenumber(CmsPhonenumber $phone) { public function addPhonenumber(CmsPhonenumber $phone) {
$this->phonenumbers[] = $phone; $this->phonenumbers[] = $phone;
if ($phone->user !== $this) { $phone->setUser($this);
$phone->user = $this;
}
} }
public function getPhonenumbers() { public function getPhonenumbers() {
...@@ -79,9 +77,7 @@ class CmsUser ...@@ -79,9 +77,7 @@ class CmsUser
public function addArticle(CmsArticle $article) { public function addArticle(CmsArticle $article) {
$this->articles[] = $article; $this->articles[] = $article;
if ($article->user !== $this) { $article->setAuthor($this);
$article->user = $this;
}
} }
public function addGroup(CmsGroup $group) { public function addGroup(CmsGroup $group) {
......
...@@ -42,10 +42,7 @@ class ECommerceFeature ...@@ -42,10 +42,7 @@ class ECommerceFeature
} }
public function setProduct(ECommerceProduct $product) { public function setProduct(ECommerceProduct $product) {
if ($this->product !== $product) {
$this->product = $product; $this->product = $product;
$product->addFeature($this);
}
} }
public function removeProduct() { public function removeProduct() {
......
...@@ -32,7 +32,8 @@ class AbstractManyToManyAssociationTestCase extends \Doctrine\Tests\OrmFunctiona ...@@ -32,7 +32,8 @@ class AbstractManyToManyAssociationTestCase extends \Doctrine\Tests\OrmFunctiona
FROM {$this->_table} FROM {$this->_table}
WHERE {$this->_firstField}=? WHERE {$this->_firstField}=?
AND {$this->_secondField}=?", AND {$this->_secondField}=?",
array($firstId, $secondId))); array($firstId, $secondId))
->fetchAll());
} }
public function assertCollectionEquals(Collection $first, Collection $second) public function assertCollectionEquals(Collection $first, Collection $second)
......
...@@ -14,9 +14,9 @@ require_once __DIR__ . '/../../TestInit.php'; ...@@ -14,9 +14,9 @@ require_once __DIR__ . '/../../TestInit.php';
*/ */
class ManyToManyBidirectionalAssociationTest extends AbstractManyToManyAssociationTestCase class ManyToManyBidirectionalAssociationTest extends AbstractManyToManyAssociationTestCase
{ {
protected $_firstField = 'cart_id'; protected $_firstField = 'product_id';
protected $_secondField = 'product_id'; protected $_secondField = 'category_id';
protected $_table = 'ecommerce_carts_products'; protected $_table = 'ecommerce_products_categories';
private $firstProduct; private $firstProduct;
private $secondProduct; private $secondProduct;
private $firstCategory; private $firstCategory;
...@@ -77,12 +77,23 @@ class ManyToManyBidirectionalAssociationTest extends AbstractManyToManyAssociati ...@@ -77,12 +77,23 @@ class ManyToManyBidirectionalAssociationTest extends AbstractManyToManyAssociati
{ {
$this->_createLoadingFixture(); $this->_createLoadingFixture();
list ($firstProduct, $secondProduct) = $this->_findProducts(); list ($firstProduct, $secondProduct) = $this->_findProducts();
$this->assertEquals(2, count($firstProduct->getCategories()));
$this->assertEquals(2, count($secondProduct->getCategories()));
$categories = $firstProduct->getCategories(); $categories = $firstProduct->getCategories();
$products = $categories[0]->getProducts(); $firstCategoryProducts = $categories[0]->getProducts();
$secondCategoryProducts = $categories[1]->getProducts();
$this->assertEquals(2, count($firstCategoryProducts));
$this->assertEquals(2, count($secondCategoryProducts));
$this->assertTrue($firstCategoryProducts[0] instanceof ECommerceProduct);
$this->assertTrue($firstCategoryProducts[1] instanceof ECommerceProduct);
$this->assertTrue($secondCategoryProducts[0] instanceof ECommerceProduct);
$this->assertTrue($secondCategoryProducts[1] instanceof ECommerceProduct);
$this->assertTrue($products[0] instanceof ECommerceProduct); $this->assertCollectionEquals($firstCategoryProducts, $secondCategoryProducts);
$this->assertTrue($products[1] instanceof ECommerceProduct);
$this->assertCollectionEquals($products, $categories[1]->getProducts());
} }
protected function _createLoadingFixture() protected function _createLoadingFixture()
......
...@@ -41,10 +41,8 @@ class ManyToManyUnidirectionalAssociationTest extends AbstractManyToManyAssociat ...@@ -41,10 +41,8 @@ class ManyToManyUnidirectionalAssociationTest extends AbstractManyToManyAssociat
$this->_em->save($this->firstCart); $this->_em->save($this->firstCart);
$this->_em->flush(); $this->_em->flush();
$this->assertForeignKeysContain($this->firstCart->getId(), $this->assertForeignKeysContain($this->firstCart->getId(), $this->firstProduct->getId());
$this->firstProduct->getId()); $this->assertForeignKeysContain($this->firstCart->getId(), $this->secondProduct->getId());
$this->assertForeignKeysContain($this->firstCart->getId(),
$this->secondProduct->getId());
} }
public function testRemovesAManyToManyAssociation() public function testRemovesAManyToManyAssociation()
...@@ -56,10 +54,8 @@ class ManyToManyUnidirectionalAssociationTest extends AbstractManyToManyAssociat ...@@ -56,10 +54,8 @@ class ManyToManyUnidirectionalAssociationTest extends AbstractManyToManyAssociat
$this->_em->flush(); $this->_em->flush();
$this->assertForeignKeysNotContain($this->firstCart->getId(), $this->assertForeignKeysNotContain($this->firstCart->getId(), $this->firstProduct->getId());
$this->firstProduct->getId()); $this->assertForeignKeysContain($this->firstCart->getId(), $this->secondProduct->getId());
$this->assertForeignKeysContain($this->firstCart->getId(),
$this->secondProduct->getId());
} }
public function testEagerLoad() public function testEagerLoad()
......
...@@ -83,6 +83,7 @@ class OrmFunctionalTestCase extends OrmTestCase ...@@ -83,6 +83,7 @@ class OrmFunctionalTestCase extends OrmTestCase
$conn->exec('DELETE FROM ecommerce_shippings'); $conn->exec('DELETE FROM ecommerce_shippings');
$conn->exec('DELETE FROM ecommerce_features'); $conn->exec('DELETE FROM ecommerce_features');
$conn->exec('DELETE FROM ecommerce_categories'); $conn->exec('DELETE FROM ecommerce_categories');
$conn->exec('DELETE FROM ecommerce_products_categories');
} }
if (isset($this->_usedModelSets['company'])) { if (isset($this->_usedModelSets['company'])) {
$conn->exec('DELETE FROM company_persons_friends'); $conn->exec('DELETE FROM company_persons_friends');
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment