From 3b4b38e1841ed529c8ca92e26f46ac29e6ca619a Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 14 Aug 2011 19:20:04 +0200 Subject: [PATCH 1/3] DDC-1306, DDC-1113 - Fix issues with inheritance and commit order --- lib/Doctrine/ORM/UnitOfWork.php | 5 +- .../ORM/Functional/Ticket/DDC1113Test.php | 99 +++++++++++++++++++ .../ORM/Functional/Ticket/DDC1306Test.php | 54 ++++++++++ 3 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1113Test.php create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1306Test.php diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index d62eb62c7..652155d2f 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -834,6 +834,8 @@ class UnitOfWork implements PropertyChangedListener // See if there are any new classes in the changeset, that are not in the // commit order graph yet (dont have a node). + + // TODO: Can we know the know the possible $newNodes based on something more efficient? IdentityMap? $newNodes = array(); foreach ($entityChangeSet as $oid => $entity) { $className = get_class($entity); @@ -845,7 +847,7 @@ class UnitOfWork implements PropertyChangedListener } // Calculate dependencies for new nodes - foreach ($newNodes as $class) { + while ($class = array_pop($newNodes)) { foreach ($class->associationMappings as $assoc) { if ($assoc['isOwningSide'] && $assoc['type'] & ClassMetadata::TO_ONE) { $targetClass = $this->em->getClassMetadata($assoc['targetEntity']); @@ -860,6 +862,7 @@ class UnitOfWork implements PropertyChangedListener $targetSubClass = $this->em->getClassMetadata($subClassName); if ( ! $calc->hasClass($subClassName)) { $calc->addClass($targetSubClass); + $newNodes[] = $targetSubClass; } $calc->addDependency($targetSubClass, $class); } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1113Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1113Test.php new file mode 100644 index 000000000..3e8a2fc0a --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1113Test.php @@ -0,0 +1,99 @@ +_schemaTool->createSchema(array( + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC1113Engine'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC1113Vehicle'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC1113Car'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC1113Bus'), + )); + } catch (\Exception $e) { + + } + } + + public function testIssue() + { + $car = new DDC1113Car(); + $car->engine = new DDC1113Engine(); + + $bus = new DDC1113Bus(); + $bus->engine = new DDC1113Engine(); + + $this->_em->persist($car); + $this->_em->flush(); + + $this->_em->persist($bus); + $this->_em->flush(); + + $this->_em->remove($bus); + $this->_em->remove($car); + $this->_em->flush(); + } + +} + +/** + * @Entity + * @InheritanceType("SINGLE_TABLE") + * @DiscriminatorMap({"car" = "DDC1113Car", "bus" = "DDC1113Bus"}) + */ +class DDC1113Vehicle +{ + + /** @Id @GeneratedValue @Column(type="integer") */ + public $id; + + /** + * @ManyToOne(targetEntity="DDC1113Vehicle") + */ + public $parent; + + /** @OneToOne(targetEntity="DDC1113Engine", cascade={"persist", "remove"}) */ + public $engine; + +} + +/** + * @Entity + */ +class DDC1113Car extends DDC1113Vehicle +{ + +} + +/** + * @Entity + */ +class DDC1113Bus extends DDC1113Vehicle +{ + +} + +/** + * @Entity + */ +class DDC1113Engine +{ + + /** @Id @GeneratedValue @Column(type="integer") */ + public $id; + +} + diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1306Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1306Test.php new file mode 100644 index 000000000..b143b0030 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1306Test.php @@ -0,0 +1,54 @@ +useModelSet('cms'); + parent::setUp(); + } + + public function testIssue() + { + $phone = new CmsPhonenumber(); + $phone->phonenumber = "1234"; + + // puts user and phone into commit order calculator + $this->_em->persist($phone); + $this->_em->flush(); + + $address = new \Doctrine\Tests\Models\CMS\CmsAddress(); + $address->city = "bonn"; + $address->country = "Germany"; + $address->street = "somestreet!"; + $address->zip = 12345; + + $this->_em->persist($address); + + $user = new CmsUser(); + $user->username = "beberlei"; + $user->name = "benjamin"; + $user->status = "active"; + $user->setAddress($address); + + // puts user and address into commit order calculator, but does not calculate user dependencies new + $this->_em->persist($user); + $this->_em->flush(); + + $this->_em->remove($user->getAddress()); + $this->_em->remove($user); + $this->_em->flush(); + } +} \ No newline at end of file From b145f061c915b671ae0874f23f595cdf6fd38081 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 28 Aug 2011 15:57:33 +0200 Subject: [PATCH 2/3] DDC-1348 - Fix bug with UnitOfWork::getEntityState() --- lib/Doctrine/ORM/UnitOfWork.php | 18 +++++++++++++++++- .../ORM/Functional/BasicFunctionalTest.php | 12 ++++++------ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 7394bfec8..830829d14 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -1105,6 +1105,22 @@ class UnitOfWork implements PropertyChangedListener } } } + } else if (!$class->idGenerator->isPostInsertGenerator()) { + // if we have a pre insert generator we can't be sure that having an id + // really means that the entity exists. We have to verify this through + // the last resort: a db lookup + + // Last try before db lookup: check the identity map. + if ($this->tryGetById($id, $class->rootEntityName)) { + return self::STATE_DETACHED; + } else { + // db lookup + if ($this->getEntityPersister(get_class($entity))->exists($entity)) { + return self::STATE_DETACHED; + } else { + return self::STATE_NEW; + } + } } else { return self::STATE_DETACHED; } @@ -1745,7 +1761,7 @@ class UnitOfWork implements PropertyChangedListener */ public function lock($entity, $lockMode, $lockVersion = null) { - if ($this->getEntityState($entity) != self::STATE_MANAGED) { + if ($this->getEntityState($entity, self::STATE_DETACHED) != self::STATE_MANAGED) { throw new InvalidArgumentException("Entity is not MANAGED."); } diff --git a/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php b/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php index d94aa253d..bed8de33d 100644 --- a/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php @@ -150,15 +150,15 @@ class BasicFunctionalTest extends \Doctrine\Tests\OrmFunctionalTestCase $user->username = 'gblanco'; $user->status = 'developer'; - $this->assertEquals(\Doctrine\ORM\UnitOfWork::STATE_NEW, $this->_em->getUnitOfWork()->getEntityState($user)); + $this->assertEquals(\Doctrine\ORM\UnitOfWork::STATE_NEW, $this->_em->getUnitOfWork()->getEntityState($user), "State should be UnitOfWork::STATE_NEW"); $this->_em->persist($user); - $this->assertEquals(\Doctrine\ORM\UnitOfWork::STATE_MANAGED, $this->_em->getUnitOfWork()->getEntityState($user)); + $this->assertEquals(\Doctrine\ORM\UnitOfWork::STATE_MANAGED, $this->_em->getUnitOfWork()->getEntityState($user), "State should be UnitOfWork::STATE_MANAGED"); $this->_em->remove($user); - - $this->assertEquals(\Doctrine\ORM\UnitOfWork::STATE_NEW, $this->_em->getUnitOfWork()->getEntityState($user)); + + $this->assertEquals(\Doctrine\ORM\UnitOfWork::STATE_NEW, $this->_em->getUnitOfWork()->getEntityState($user), "State should be UnitOfWork::STATE_NEW"); $this->_em->persist($user); $this->_em->flush(); @@ -166,10 +166,10 @@ class BasicFunctionalTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->_em->remove($user); - $this->assertEquals(\Doctrine\ORM\UnitOfWork::STATE_REMOVED, $this->_em->getUnitOfWork()->getEntityState($user)); + $this->assertEquals(\Doctrine\ORM\UnitOfWork::STATE_REMOVED, $this->_em->getUnitOfWork()->getEntityState($user), "State should be UnitOfWork::STATE_REMOVED"); $this->_em->flush(); - $this->assertEquals(\Doctrine\ORM\UnitOfWork::STATE_NEW, $this->_em->getUnitOfWork()->getEntityState($user)); + $this->assertEquals(\Doctrine\ORM\UnitOfWork::STATE_NEW, $this->_em->getUnitOfWork()->getEntityState($user), "State should be UnitOfWork::STATE_NEW"); $this->assertNull($this->_em->find('Doctrine\Tests\Models\CMS\CmsUser', $id)); } From 0a6709acba975b1f82d88bfde49bdc8759e8cda2 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 28 Aug 2011 16:00:21 +0200 Subject: [PATCH 3/3] Fix tests with regard to repositoryClass handling and applying the default namespace --- tests/Doctrine/Tests/ORM/Mapping/ClassMetadataTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataTest.php b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataTest.php index 7868fb2fb..d16ec19f0 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataTest.php @@ -45,7 +45,7 @@ class ClassMetadataTest extends \Doctrine\Tests\OrmTestCase $this->assertEquals('UserParent', $cm->rootEntityName); $this->assertEquals(array('Doctrine\Tests\Models\CMS\One', 'Doctrine\Tests\Models\CMS\Two', 'Doctrine\Tests\Models\CMS\Three'), $cm->subClasses); $this->assertEquals(array('UserParent'), $cm->parentClasses); - $this->assertEquals('UserRepository', $cm->customRepositoryClassName); + $this->assertEquals('Doctrine\Tests\Models\CMS\UserRepository', $cm->customRepositoryClassName); $this->assertEquals(array('name' => 'disc', 'type' => 'integer', 'fieldName' => 'disc'), $cm->discriminatorColumn); $this->assertTrue($cm->associationMappings['phonenumbers']['type'] == ClassMetadata::ONE_TO_ONE); $this->assertEquals(1, count($cm->associationMappings));