From 954a8c3935312d11de566e693267ed603c60b2ac Mon Sep 17 00:00:00 2001 From: "Roman S. Borschel" Date: Thu, 29 Jul 2010 14:08:36 +0200 Subject: [PATCH] Updated tests. --- lib/Doctrine/ORM/UnitOfWork.php | 80 ++++++++++++------- .../ORM/Functional/BasicFunctionalTest.php | 60 +++++++------- .../ORM/Functional/Ticket/DDC518Test.php | 36 +++++++++ 3 files changed, 117 insertions(+), 59 deletions(-) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC518Test.php diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index f1b5a338e..3f1b3e332 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -1344,34 +1344,51 @@ class UnitOfWork implements PropertyChangedListener */ private function doMerge($entity, array &$visited, $prevManagedCopy = null, $assoc = null) { - $class = $this->em->getClassMetadata(get_class($entity)); - $id = $class->getIdentifierValues($entity); - - if ( ! $id) { - throw new InvalidArgumentException('New entity detected during merge.' - . ' Persist the new entity before merging.'); + $oid = spl_object_hash($entity); + if (isset($visited[$oid])) { + return; // Prevent infinite recursion } - // MANAGED entities are ignored by the merge operation + $class = $this->em->getClassMetadata(get_class($entity)); + + // First we assume DETACHED, although it can still be NEW but we can avoid + // an extra db-roundtrip this way. If it is DETACHED or NEW, we need to fetch + // it from the db anyway in order to merge. + // MANAGED entities are ignored by the merge operation. if ($this->getEntityState($entity, self::STATE_DETACHED) == self::STATE_MANAGED) { $managedCopy = $entity; } else { // Try to look the entity up in the identity map. - $managedCopy = $this->tryGetById($id, $class->rootEntityName); - if ($managedCopy) { - // We have the entity in-memory already, just make sure its not removed. - if ($this->getEntityState($managedCopy) == self::STATE_REMOVED) { - throw new InvalidArgumentException('Removed entity detected during merge.' - . ' Can not merge with a removed entity.'); - } - } else { - // We need to fetch the managed copy in order to merge. - $managedCopy = $this->em->find($class->name, $id); - } + $id = $class->getIdentifierValues($entity); - if ($managedCopy === null) { - throw new InvalidArgumentException('New entity detected during merge.' - . ' Persist the new entity before merging.'); + // If there is no ID, it is actually NEW. + if ( ! $id) { + $managedCopy = $class->newInstance(); + $this->persistNew($class, $managedCopy); + } else { + $managedCopy = $this->tryGetById($id, $class->rootEntityName); + if ($managedCopy) { + // We have the entity in-memory already, just make sure its not removed. + if ($this->getEntityState($managedCopy) == self::STATE_REMOVED) { + throw new InvalidArgumentException('Removed entity detected during merge.' + . ' Can not merge with a removed entity.'); + } + } else { + // We need to fetch the managed copy in order to merge. + $managedCopy = $this->em->find($class->name, $id); + } + + if ($managedCopy === null) { + // If the identifier is ASSIGNED, it is NEW, otherwise an error + // since the managed entity was not found. + if ($class->isIdentifierNatural()) { + $managedCopy = $class->newInstance(); + $class->setIdentifierValues($managedCopy, $id); + $this->persistNew($class, $managedCopy); + } else { + throw new EntityNotFoundException; + } + } } if ($class->isVersioned) { @@ -1388,17 +1405,20 @@ class UnitOfWork implements PropertyChangedListener if ( ! isset($class->associationMappings[$name])) { $prop->setValue($managedCopy, $prop->getValue($entity)); } else { - // why $assoc2? See the method signature, there is $assoc already! $assoc2 = $class->associationMappings[$name]; if ($assoc2->isOneToOne()) { if ( ! $assoc2->isCascadeMerge) { - $other = $class->reflFields[$name]->getValue($entity); //TODO: Just $prop->getValue($entity)? + $other = $prop->getValue($entity); if ($other !== null) { - $targetClass = $this->em->getClassMetadata($assoc2->targetEntityName); - $id = $targetClass->getIdentifierValues($other); - $proxy = $this->em->getProxyFactory()->getProxy($assoc2->targetEntityName, $id); - $prop->setValue($managedCopy, $proxy); - $this->registerManaged($proxy, $id, array()); + if ($this->getEntityState($other, self::STATE_DETACHED) == self::STATE_MANAGED) { + $prop->setValue($managedCopy, $other); + } else { + $targetClass = $this->em->getClassMetadata($assoc2->targetEntityName); + $id = $targetClass->getIdentifierValues($other); + $proxy = $this->em->getProxyFactory()->getProxy($assoc2->targetEntityName, $id); + $prop->setValue($managedCopy, $proxy); + $this->registerManaged($proxy, $id, array()); + } } } } else { @@ -1421,8 +1441,8 @@ class UnitOfWork implements PropertyChangedListener //TODO: put changed fields in changeset...? } } - if ($class->isChangeTrackingDeferredExplicit()) { - //TODO: Mark $managedCopy for dirty check...? ($this->scheduledForDirtyCheck) + if ( ! $class->isChangeTrackingDeferredImplicit()) { + $this->scheduleForDirtyCheck($entity); } } diff --git a/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php b/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php index a918bebe5..d265dc336 100644 --- a/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php @@ -768,37 +768,39 @@ class BasicFunctionalTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertEquals('Stephan', $this->_em->find(get_class($user), $userId)->name); } - - //DRAFT OF EXPECTED/DESIRED BEHAVIOR - /*public function testPersistentCollectionContainsDoesNeverInitialize() + + public function testMergePersistsNewEntities() { - $user = new CmsUser; - $user->name = 'Guilherme'; - $user->username = 'gblanco'; - $user->status = 'developer'; - - $group = new CmsGroup; - $group->name = 'Developers'; - - $user->addGroup($group); - - $this->_em->persist($user); + $user = new CmsUser(); + $user->username = "beberlei"; + $user->name = "Benjamin E."; + $user->status = 'active'; + + $managedUser = $this->_em->merge($user); + $this->assertEquals('beberlei', $managedUser->username); + $this->assertEquals('Benjamin E.', $managedUser->name); + $this->assertEquals('active', $managedUser->status); + + $this->assertTrue($user !== $managedUser); + $this->assertTrue($this->_em->contains($managedUser)); + $this->_em->flush(); + $userId = $managedUser->id; $this->_em->clear(); - - $group = $this->_em->find(get_class($group), $group->getId()); - - - - $user2 = new CmsUser; - $user2->id = $user->getId(); - $this->assertFalse($group->getUsers()->contains($user2)); - $this->assertFalse($group->getUsers()->isInitialized()); - - $user2 = $this->_em->getReference(get_class($user), $user->getId()); - $this->assertTrue($group->getUsers()->contains($user2)); - $this->assertFalse($group->getUsers()->isInitialized()); - + + $this->assertTrue($this->_em->find(get_class($managedUser), $userId) instanceof CmsUser); + } + + public function testMergeThrowsExceptionIfEntityWithGeneratedIdentifierDoesNotExist() + { + $user = new CmsUser(); + $user->username = "beberlei"; + $user->name = "Benjamin E."; + $user->status = 'active'; + $user->id = 42; + try { + $this->_em->merge($user); + $this->fail(); + } catch (\Doctrine\ORM\EntityNotFoundException $enfe) {} } - */ } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC518Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC518Test.php new file mode 100644 index 000000000..2a3450666 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC518Test.php @@ -0,0 +1,36 @@ +useModelSet('cms'); + parent::setUp(); + } + + public function testMergeWithRelatedNew() + { + $article = new \Doctrine\Tests\Models\CMS\CmsArticle(); + $article->text = "foo"; + $article->topic = "bar"; + + $this->_em->persist($article); + $this->_em->flush(); + $this->_em->detach($article); + $this->_em->clear(); + + $user = new \Doctrine\Tests\Models\CMS\CmsUser(); + $user->username = "beberlei"; + $user->name = "Benjamin Eberlei"; + $user->status = "active"; + $article->user = $user; + + $this->_em->persist($user); + $managedArticle = $this->_em->merge($article); + + $this->assertSame($article->user, $managedArticle->user); + } +} \ No newline at end of file