From bb873826ca4f53e02086cdb7f49913592d6afec3 Mon Sep 17 00:00:00 2001 From: Miha Vrhovnik Date: Fri, 27 May 2011 08:41:31 +0200 Subject: [PATCH 01/14] Fixing Notice: Undefined index: orderBy in ...Doctrine/ORM/Tools/Export/Driver/YamlExporter --- .../ORM/Tools/Export/Driver/YamlExporter.php | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/Export/Driver/YamlExporter.php b/lib/Doctrine/ORM/Tools/Export/Driver/YamlExporter.php index 1cc699610..d5c1efd8a 100644 --- a/lib/Doctrine/ORM/Tools/Export/Driver/YamlExporter.php +++ b/lib/Doctrine/ORM/Tools/Export/Driver/YamlExporter.php @@ -43,7 +43,7 @@ class YamlExporter extends AbstractExporter * * TODO: Should this code be pulled out in to a toArray() method in ClassMetadata * - * @param ClassMetadataInfo $metadata + * @param ClassMetadataInfo $metadata * @return mixed $exported */ public function exportClassMetadata(ClassMetadataInfo $metadata) @@ -84,9 +84,9 @@ class YamlExporter extends AbstractExporter if (isset($metadata->table['uniqueConstraints'])) { $array['uniqueConstraints'] = $metadata->table['uniqueConstraints']; } - + $fieldMappings = $metadata->fieldMappings; - + $ids = array(); foreach ($fieldMappings as $name => $fieldMapping) { $fieldMapping['column'] = $fieldMapping['columnName']; @@ -94,7 +94,7 @@ class YamlExporter extends AbstractExporter $fieldMapping['columnName'], $fieldMapping['fieldName'] ); - + if ($fieldMapping['column'] == $name) { unset($fieldMapping['column']); } @@ -111,7 +111,7 @@ class YamlExporter extends AbstractExporter if ($idGeneratorType = $this->_getIdGeneratorTypeString($metadata->generatorType)) { $ids[$metadata->getSingleIdentifierFieldName()]['generator']['strategy'] = $this->_getIdGeneratorTypeString($metadata->generatorType); } - + if ($ids) { $array['fields'] = $ids; } @@ -145,7 +145,7 @@ class YamlExporter extends AbstractExporter 'targetEntity' => $associationMapping['targetEntity'], 'cascade' => $cascade, ); - + if ($associationMapping['type'] & ClassMetadataInfo::TO_ONE) { $joinColumns = $associationMapping['joinColumns']; $newJoinColumns = array(); @@ -164,7 +164,7 @@ class YamlExporter extends AbstractExporter 'joinColumns' => $newJoinColumns, 'orphanRemoval' => $associationMapping['orphanRemoval'], ); - + $associationMappingArray = array_merge($associationMappingArray, $oneToOneMappingArray); $array['oneToOne'][$name] = $associationMappingArray; } else if ($associationMapping['type'] == ClassMetadataInfo::ONE_TO_MANY) { @@ -172,7 +172,7 @@ class YamlExporter extends AbstractExporter 'mappedBy' => $associationMapping['mappedBy'], 'inversedBy' => $associationMapping['inversedBy'], 'orphanRemoval' => $associationMapping['orphanRemoval'], - 'orderBy' => $associationMapping['orderBy'] + 'orderBy' => isset($associationMapping['orderBy']) ? $associationMapping['orderBy'] : null ); $associationMappingArray = array_merge($associationMappingArray, $oneToManyMappingArray); @@ -182,9 +182,9 @@ class YamlExporter extends AbstractExporter 'mappedBy' => $associationMapping['mappedBy'], 'inversedBy' => $associationMapping['inversedBy'], 'joinTable' => $associationMapping['joinTable'], - 'orderBy' => isset($associationMapping['orderBy']) ? $associationMapping['orderBy'] : null + 'orderBy' => isset($associationMapping['orderBy']) ? $associationMapping['orderBy'] : null ); - + $associationMappingArray = array_merge($associationMappingArray, $manyToManyMappingArray); $array['manyToMany'][$name] = $associationMappingArray; } From 23540c17f182143f391ef6c1a4d495a81bb26a67 Mon Sep 17 00:00:00 2001 From: chesteroni Date: Sat, 28 May 2011 20:57:19 -0700 Subject: [PATCH 02/14] Added checking for existing indexes in associatation mapping array. --- .../ORM/Tools/Export/Driver/PhpExporter.php | 32 ++++++++++++------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/Export/Driver/PhpExporter.php b/lib/Doctrine/ORM/Tools/Export/Driver/PhpExporter.php index 005b18a7a..900fb5bda 100644 --- a/lib/Doctrine/ORM/Tools/Export/Driver/PhpExporter.php +++ b/lib/Doctrine/ORM/Tools/Export/Driver/PhpExporter.php @@ -120,22 +120,30 @@ class PhpExporter extends AbstractExporter $associationMappingArray = array_merge($associationMappingArray, $oneToOneMappingArray); } else if ($associationMapping['type'] == ClassMetadataInfo::ONE_TO_MANY) { - $method = 'mapOneToMany'; - $oneToManyMappingArray = array( - 'mappedBy' => $associationMapping['mappedBy'], - 'orphanRemoval' => $associationMapping['orphanRemoval'], - 'orderBy' => $associationMapping['orderBy'] + $method = 'mapOneToMany'; + $potentialAssociationMappingIndexes = array( + 'mappedBy', + 'orphanRemoval', + 'orderBy', ); - + foreach ($potentialAssociationMappingIndexes as $index) { + if (isset($associationMapping[$index])) { + $oneToManyMappingArray[$index] = $associationMapping[$index]; + } + } $associationMappingArray = array_merge($associationMappingArray, $oneToManyMappingArray); } else if ($associationMapping['type'] == ClassMetadataInfo::MANY_TO_MANY) { - $method = 'mapManyToMany'; - $manyToManyMappingArray = array( - 'mappedBy' => $associationMapping['mappedBy'], - 'joinTable' => $associationMapping['joinTable'], - 'orderBy' => $associationMapping['orderBy'] + $method = 'mapManyToMany'; + $potentialAssociationMappingIndexes = array( + 'mappedBy', + 'joinTable', + 'orderBy', ); - + foreach ($potentialAssociationMappingIndexes as $index) { + if (isset($associationMapping[$index])) { + $manyToManyMappingArray[$index] = $associationMapping[$index]; + } + } $associationMappingArray = array_merge($associationMappingArray, $manyToManyMappingArray); } From 86c3744b8c2b2a85d7c33000ab200e7d99adb577 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sun, 5 Jun 2011 08:03:41 +0200 Subject: [PATCH 03/14] Made orm:convert-mapping command more configurable (allow to change the extension of the generated files for instance) --- .../Tools/Console/Command/ConvertMappingCommand.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/Console/Command/ConvertMappingCommand.php b/lib/Doctrine/ORM/Tools/Console/Command/ConvertMappingCommand.php index 8c1a8fea1..1522277fc 100644 --- a/lib/Doctrine/ORM/Tools/Console/Command/ConvertMappingCommand.php +++ b/lib/Doctrine/ORM/Tools/Console/Command/ConvertMappingCommand.php @@ -137,8 +137,7 @@ EOT $toType = strtolower($input->getArgument('to-type')); - $cme = new ClassMetadataExporter(); - $exporter = $cme->getExporter($toType, $destPath); + $exporter = $this->getExporter($toType, $destPath); $exporter->setOverwriteExistingFiles( ($input->getOption('force') !== false) ); if ($toType == 'annotation') { @@ -167,4 +166,11 @@ EOT $output->write('No Metadata Classes to process.' . PHP_EOL); } } + + protected function getExporter($toType, $destPath) + { + $cme = new ClassMetadataExporter(); + + return $cme->getExporter($toType, $destPath); + } } From 875912bffda24fc909bd8d8ecafede992c5ff9d9 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 5 Jun 2011 08:44:38 +0200 Subject: [PATCH 04/14] DDC-733 - Add UnitOfWork::initializeObject() method. --- lib/Doctrine/ORM/Proxy/ProxyFactory.php | 5 ++-- lib/Doctrine/ORM/UnitOfWork.php | 23 ++++++++++++++++++- .../ManyToManyBasicAssociationTest.php | 15 ++++++++++++ .../ORM/Functional/ReferenceProxyTest.php | 15 ++++++++++++ 4 files changed, 55 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ORM/Proxy/ProxyFactory.php b/lib/Doctrine/ORM/Proxy/ProxyFactory.php index 23186d7aa..52a4791a9 100644 --- a/lib/Doctrine/ORM/Proxy/ProxyFactory.php +++ b/lib/Doctrine/ORM/Proxy/ProxyFactory.php @@ -269,7 +269,8 @@ class extends \ implements \Doctrine\ORM\Proxy\Proxy $this->_entityPersister = $entityPersister; $this->_identifier = $identifier; } - private function __load() + /** @private */ + public function __load() { if (!$this->__isInitialized__ && $this->_entityPersister) { $this->__isInitialized__ = true; @@ -279,7 +280,7 @@ class extends \ implements \Doctrine\ORM\Proxy\Proxy unset($this->_entityPersister, $this->_identifier); } } - + public function __sleep() diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 90d3117e3..aa5309a7c 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -2335,7 +2335,28 @@ class UnitOfWork implements PropertyChangedListener { return $this->collectionUpdates; } - + + /** + * Helper method to initialize a lazy loading proxy or persistent collection. + * + * @param object + * @return void + */ + public function initializeObject($obj) + { + if ($obj instanceof Proxy) { + $obj->__load(); + } else if ($obj instanceof PersistentCollection) { + $obj->initialize(); + } + } + + /** + * Helper method to show an object as string. + * + * @param object $obj + * @return string + */ private static function objToStr($obj) { return method_exists($obj, '__toString') ? (string)$obj : get_class($obj).'@'.spl_object_hash($obj); diff --git a/tests/Doctrine/Tests/ORM/Functional/ManyToManyBasicAssociationTest.php b/tests/Doctrine/Tests/ORM/Functional/ManyToManyBasicAssociationTest.php index 62ba0c08a..c537b4c5f 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ManyToManyBasicAssociationTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ManyToManyBasicAssociationTest.php @@ -342,4 +342,19 @@ class ManyToManyBasicAssociationTest extends \Doctrine\Tests\OrmFunctionalTestCa $this->assertEquals('Developers_New1', $user->groups[0]->name); $this->assertEquals('Developers_New2', $user->groups[1]->name); } + + /** + * @group DDC-733 + */ + public function testInitializePersistentCollection() + { + $user = $this->addCmsUserGblancoWithGroups(2); + $this->_em->clear(); + + $user = $this->_em->find(get_class($user), $user->id); + + $this->assertFalse($user->groups->isInitialized(), "Pre-condition: lazy collection"); + $this->_em->getUnitOfWork()->initializeObject($user->groups); + $this->assertTrue($user->groups->isInitialized(), "Collection should be initialized after calling UnitOfWork::initializeObject()"); + } } diff --git a/tests/Doctrine/Tests/ORM/Functional/ReferenceProxyTest.php b/tests/Doctrine/Tests/ORM/Functional/ReferenceProxyTest.php index 3e66e0b12..9fcb90a5e 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ReferenceProxyTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ReferenceProxyTest.php @@ -97,4 +97,19 @@ class ReferenceProxyTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertTrue($clone->isCloned); $this->assertFalse($entity->isCloned); } + + /** + * @group DDC-733 + */ + public function testInitializeProxy() + { + $id = $this->createProduct(); + + /* @var $entity Doctrine\Tests\Models\ECommerce\ECommerceProduct */ + $entity = $this->_em->getReference('Doctrine\Tests\Models\ECommerce\ECommerceProduct' , $id); + + $this->assertFalse($entity->__isInitialized__, "Pre-Condition: Object is unitialized proxy."); + $this->_em->getUnitOfWork()->initializeObject($entity); + $this->assertTrue($entity->__isInitialized__, "Should be initialized after called UnitOfWork::initializeObject()"); + } } From acaf08d4b751f1fc9772de5c0e0c5d8371722f17 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 5 Jun 2011 09:59:16 +0200 Subject: [PATCH 05/14] DDC-1193 - Fix bug with cascade remove and proxy classes. --- .../ORM/Persisters/BasicEntityPersister.php | 4 +- lib/Doctrine/ORM/UnitOfWork.php | 20 +++- .../ORM/Functional/Ticket/DDC1193Test.php | 93 +++++++++++++++++++ 3 files changed, 111 insertions(+), 6 deletions(-) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1193Test.php diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 5ce7080c2..a14528ecd 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -563,7 +563,7 @@ class BasicEntityPersister * @todo Check identity map? loadById method? Try to guess whether $criteria is the id? */ public function load(array $criteria, $entity = null, $assoc = null, array $hints = array(), $lockMode = 0) - { + { $sql = $this->_getSelectEntitiesSQL($criteria, $assoc, $lockMode); list($params, $types) = $this->expandParameters($criteria); $stmt = $this->_conn->executeQuery($sql, $params, $types); @@ -577,7 +577,7 @@ class BasicEntityPersister } else { $hydrator = $this->_em->newHydrator(Query::HYDRATE_SIMPLEOBJECT); } - $entities = $hydrator->hydrateAll($stmt, $this->_rsm, $hints); + $entities = $hydrator->hydrateAll($stmt, $this->_rsm, $hints); return $entities ? $entities[0] : null; } diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index aa5309a7c..99152a652 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -1290,6 +1290,10 @@ class UnitOfWork implements PropertyChangedListener } $visited[$oid] = $entity; // mark visited + + // Cascade first, because scheduleForDelete() removes the entity from the identity map, which + // can cause problems when a lazy proxy has to be initialized for the cascade operation. + $this->cascadeRemove($entity, $visited); $class = $this->em->getClassMetadata(get_class($entity)); $entityState = $this->getEntityState($entity); @@ -1313,7 +1317,6 @@ class UnitOfWork implements PropertyChangedListener throw new UnexpectedValueException("Unexpected entity state: $entityState."); } - $this->cascadeRemove($entity, $visited); } /** @@ -1674,6 +1677,11 @@ class UnitOfWork implements PropertyChangedListener if ( ! $assoc['isCascadePersist']) { continue; } + + if ($entity instanceof Proxy && !$entity->__isInitialized__) { + $entity->__load(); + } + $relatedEntities = $class->reflFields[$assoc['fieldName']]->getValue($entity); if (($relatedEntities instanceof Collection || is_array($relatedEntities))) { if ($relatedEntities instanceof PersistentCollection) { @@ -1702,7 +1710,11 @@ class UnitOfWork implements PropertyChangedListener if ( ! $assoc['isCascadeRemove']) { continue; } - //TODO: If $entity instanceof Proxy => Initialize ? + + if ($entity instanceof Proxy) { + $entity->__load(); + } + $relatedEntities = $class->reflFields[$assoc['fieldName']]->getValue($entity); if ($relatedEntities instanceof Collection || is_array($relatedEntities)) { // If its a PersistentCollection initialization is intended! No unwrap! @@ -1865,8 +1877,8 @@ class UnitOfWork implements PropertyChangedListener } $id = array($class->identifier[0] => $idHash); } - - if (isset($this->identityMap[$class->rootEntityName][$idHash])) { + + if (isset($this->identityMap[$class->rootEntityName][$idHash])) { $entity = $this->identityMap[$class->rootEntityName][$idHash]; $oid = spl_object_hash($entity); if ($entity instanceof Proxy && ! $entity->__isInitialized__) { diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1193Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1193Test.php new file mode 100644 index 000000000..3ddf37e70 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1193Test.php @@ -0,0 +1,93 @@ +_em->getConnection()->getConfiguration()->setSQLLogger(new \Doctrine\DBAL\Logging\EchoSQLLogger); + $this->_schemaTool->createSchema(array( + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC1193Company'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC1193Person'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC1193Account') + )); + } + + /** + * @group DDC-1193 + */ + public function testIssue() + { + $company = new DDC1193Company(); + $person = new DDC1193Person(); + $account = new DDC1193Account(); + + $person->account = $account; + $person->company = $company; + + $company->member = $person; + + $this->_em->persist($company); + + $this->_em->flush(); + + $companyId = $company->id; + $accountId = $account->id; + $this->_em->clear(); + + $company = $this->_em->find(get_class($company), $companyId); + + $this->assertTrue($this->_em->getUnitOfWork()->isInIdentityMap($company), "Company is in identity map."); + $this->assertFalse($company->member->__isInitialized__, "Pre-Condition"); + $this->assertTrue($this->_em->getUnitOfWork()->isInIdentityMap($company->member), "Member is in identity map."); + + $this->_em->remove($company); + $this->_em->flush(); + + $this->assertEquals(count($this->_em->getRepository(get_class($account))->findAll()), 0); + } +} + +/** @Entity */ +class DDC1193Company { + /** + * @Id @Column(type="integer") + * @GeneratedValue + */ + public $id; + + /** @OneToOne(targetEntity="DDC1193Person", cascade={"persist", "remove"}) */ + public $member; + +} + +/** @Entity */ +class DDC1193Person { + /** + * @Id @Column(type="integer") + * @GeneratedValue + */ + public $id; + + /** + * @OneToOne(targetEntity="DDC1193Account", cascade={"persist", "remove"}) + */ + public $account; +} + +/** @Entity */ +class DDC1193Account { + /** + * @Id @Column(type="integer") + * @GeneratedValue + */ + public $id; + +} + + From bda4165bf82073b13754375c5b3932d4c9878b12 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 5 Jun 2011 10:02:57 +0200 Subject: [PATCH 06/14] DDC-1193 - Fix previous commit. --- lib/Doctrine/ORM/Persisters/BasicEntityPersister.php | 4 ++-- lib/Doctrine/ORM/UnitOfWork.php | 6 +----- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index a14528ecd..5ce7080c2 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -563,7 +563,7 @@ class BasicEntityPersister * @todo Check identity map? loadById method? Try to guess whether $criteria is the id? */ public function load(array $criteria, $entity = null, $assoc = null, array $hints = array(), $lockMode = 0) - { + { $sql = $this->_getSelectEntitiesSQL($criteria, $assoc, $lockMode); list($params, $types) = $this->expandParameters($criteria); $stmt = $this->_conn->executeQuery($sql, $params, $types); @@ -577,7 +577,7 @@ class BasicEntityPersister } else { $hydrator = $this->_em->newHydrator(Query::HYDRATE_SIMPLEOBJECT); } - $entities = $hydrator->hydrateAll($stmt, $this->_rsm, $hints); + $entities = $hydrator->hydrateAll($stmt, $this->_rsm, $hints); return $entities ? $entities[0] : null; } diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 99152a652..6d9c756b4 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -1678,10 +1678,6 @@ class UnitOfWork implements PropertyChangedListener continue; } - if ($entity instanceof Proxy && !$entity->__isInitialized__) { - $entity->__load(); - } - $relatedEntities = $class->reflFields[$assoc['fieldName']]->getValue($entity); if (($relatedEntities instanceof Collection || is_array($relatedEntities))) { if ($relatedEntities instanceof PersistentCollection) { @@ -1711,7 +1707,7 @@ class UnitOfWork implements PropertyChangedListener continue; } - if ($entity instanceof Proxy) { + if ($entity instanceof Proxy && !$entity->__isInitialized__) { $entity->__load(); } From 1038a866a49e6d140d1e1670d8836dd6ceb0f3e0 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 5 Jun 2011 10:48:21 +0200 Subject: [PATCH 07/14] DDC-1194 - Improve error handling for DQL INSTANCE OF --- lib/Doctrine/ORM/Query/QueryException.php | 6 +++++ lib/Doctrine/ORM/Query/SqlWalker.php | 4 ++++ .../ORM/Query/SelectSqlGenerationTest.php | 22 +++++++++++++++++++ 3 files changed, 32 insertions(+) diff --git a/lib/Doctrine/ORM/Query/QueryException.php b/lib/Doctrine/ORM/Query/QueryException.php index aafe1e9d7..39dc42505 100644 --- a/lib/Doctrine/ORM/Query/QueryException.php +++ b/lib/Doctrine/ORM/Query/QueryException.php @@ -135,4 +135,10 @@ class QueryException extends \Doctrine\ORM\ORMException "in the query." ); } + + public static function instanceOfUnrelatedClass($className, $rootClass) + { + return new self("Cannot check if a child of '" . $rootClass . "' is instanceof '" . $className . "', " . + "inheritance hierachy exists between these two classes."); + } } \ No newline at end of file diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 611cd0a1d..270131ea2 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -1657,6 +1657,10 @@ class SqlWalker implements TreeWalker $sql .= $this->_conn->quote($class->discriminatorValue); } else { $discrMap = array_flip($class->discriminatorMap); + if (!isset($discrMap[$entityClassName])) { + throw QueryException::instanceOfUnrelatedClass($entityClassName, $class->rootEntityName); + } + $sql .= $this->_conn->quote($discrMap[$entityClassName]); } diff --git a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php index 2d0101e03..6652ad67a 100644 --- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php @@ -383,6 +383,28 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase "SELECT c0_.id AS id0, c0_.name AS name1, c0_.discr AS discr2 FROM company_persons c0_ WHERE c0_.discr = 'employee'" ); } + + /** + * @group DDC-1194 + */ + public function testSupportsInstanceOfExpressionsInWherePartPrefixedSlash() + { + $this->assertSqlGeneration( + "SELECT u FROM Doctrine\Tests\Models\Company\CompanyPerson u WHERE u INSTANCE OF \Doctrine\Tests\Models\Company\CompanyEmployee", + "SELECT c0_.id AS id0, c0_.name AS name1, c0_.discr AS discr2 FROM company_persons c0_ WHERE c0_.discr = 'employee'" + ); + } + + /** + * @group DDC-1194 + */ + public function testSupportsInstanceOfExpressionsInWherePartWithUnrelatedClass() + { + $this->assertInvalidSqlGeneration( + "SELECT u FROM Doctrine\Tests\Models\Company\CompanyPerson u WHERE u INSTANCE OF \Doctrine\Tests\Models\CMS\CmsUser", + "Doctrine\ORM\Query\QueryException" + ); + } public function testSupportsInstanceOfExpressionsInWherePartInDeeperLevel() { From 70d756d59c209b2e447b80aed1584d0921d5b65c Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 5 Jun 2011 12:54:29 +0200 Subject: [PATCH 08/14] DDC-1184 - Improve error handling in AssignedIdGenerator --- lib/Doctrine/ORM/Id/AssignedGenerator.php | 10 +++++++++- lib/Doctrine/ORM/ORMException.php | 17 ++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Id/AssignedGenerator.php b/lib/Doctrine/ORM/Id/AssignedGenerator.php index 00aeaaa03..63c6e5418 100644 --- a/lib/Doctrine/ORM/Id/AssignedGenerator.php +++ b/lib/Doctrine/ORM/Id/AssignedGenerator.php @@ -47,9 +47,13 @@ class AssignedGenerator extends AbstractIdGenerator if ($class->isIdentifierComposite) { $idFields = $class->getIdentifierFieldNames(); foreach ($idFields as $idField) { - $value = $class->getReflectionProperty($idField)->getValue($entity); + $value = $class->reflFields[$idField]->getValue($entity); if (isset($value)) { if (is_object($value)) { + if (!$em->getUnitOfWork()->isInIdentityMap($value)) { + throw ORMException::entityMissingForeignAssignedId($entity, $value); + } + // NOTE: Single Columns as associated identifiers only allowed - this constraint it is enforced. $identifier[$idField] = current($em->getUnitOfWork()->getEntityIdentifier($value)); } else { @@ -64,6 +68,10 @@ class AssignedGenerator extends AbstractIdGenerator $value = $class->reflFields[$idField]->getValue($entity); if (isset($value)) { if (is_object($value)) { + if (!$em->getUnitOfWork()->isInIdentityMap($value)) { + throw ORMException::entityMissingForeignAssignedId($entity, $value); + } + // NOTE: Single Columns as associated identifiers only allowed - this constraint it is enforced. $identifier[$idField] = current($em->getUnitOfWork()->getEntityIdentifier($value)); } else { diff --git a/lib/Doctrine/ORM/ORMException.php b/lib/Doctrine/ORM/ORMException.php index c84dec41e..0825ae87d 100644 --- a/lib/Doctrine/ORM/ORMException.php +++ b/lib/Doctrine/ORM/ORMException.php @@ -34,10 +34,25 @@ class ORMException extends Exception return new self("It's a requirement to specify a Metadata Driver and pass it ". "to Doctrine\ORM\Configuration::setMetadataDriverImpl()."); } + + public static function entityMissingForeignAssignedId($entity, $relatedEntity) + { + return new self( + "Entity of type " . get_class($entity) . " has identity through a foreign entity " . get_class($relatedEntityClass) . ", " . + "however this entity has no ientity itself. You have to call EntityManager#persist() on the related entity " . + "and make sure it an identifier was generated before trying to persist '" . get_class($entity) . "'. In case " . + "of Post Insert ID Generation (such as MySQL Auto-Increment or PostgreSQL SERIAL) this means you have to call " . + "EntityManager#flush() between both persist operations." + ); + } public static function entityMissingAssignedId($entity) { - return new self("Entity of type " . get_class($entity) . " is missing an assigned ID."); + return new self("Entity of type " . get_class($entity) . " is missing an assigned ID. " . + "The identifier generation strategy for this entity requires the ID field to be populated before ". + "EntityManager#persist() is called. If you want automatically generated identifiers instead " . + "you need to adjust the metadata mapping accordingly." + ); } public static function unrecognizedField($field) From ddb647f39fb2dca09bd9a299bc2357ac5a730c79 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 5 Jun 2011 13:34:07 +0200 Subject: [PATCH 09/14] DDC-1173 - Fix bug when calling UnitOfWork::clearEntityChangeSet() in listener --- lib/Doctrine/ORM/UnitOfWork.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 6d9c756b4..8a3d582f7 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -759,7 +759,9 @@ class UnitOfWork implements PropertyChangedListener ); } - $persister->update($entity); + if ($this->entityChangeSets[$oid]) { + $persister->update($entity); + } unset($this->entityUpdates[$oid]); if ($hasPostUpdateLifecycleCallbacks) { @@ -2263,7 +2265,7 @@ class UnitOfWork implements PropertyChangedListener */ public function clearEntityChangeSet($oid) { - unset($this->entityChangeSets[$oid]); + $this->entityChangeSets[$oid] = array(); } /* PropertyChangedListener implementation */ From d3ab9b51fad3b99b13c257d43a513a1c02b62b95 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 5 Jun 2011 13:57:44 +0200 Subject: [PATCH 10/14] DDC-1181 - Add test that verifies cascade remove works for entities with foreign identifiers --- .../ORM/Functional/Ticket/DDC1181Test.php | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1181Test.php diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1181Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1181Test.php new file mode 100644 index 000000000..225353034 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1181Test.php @@ -0,0 +1,101 @@ +_schemaTool->createSchema(array( + $this->_em->getClassMetadata(__NAMESPACE__ . '\\DDC1181Hotel'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\\DDC1181Booking'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\\DDC1181Room'), + )); + } + + /** + * @group DDC-1181 + */ + public function testIssue() + { + $hotel = new DDC1181Hotel(); + $room1 = new DDC1181Room(); + $room2 = new DDC1181Room(); + + $this->_em->persist($hotel); + $this->_em->persist($room1); + $this->_em->persist($room2); + $this->_em->flush(); + + $booking1 = new DDC1181Booking; + $booking1->hotel = $hotel; + $booking1->room = $room1; + $booking2 = new DDC1181Booking; + $booking2->hotel = $hotel; + $booking2->room = $room2; + $hotel->bookings[] = $booking1; + $hotel->bookings[] = $booking2; + + $this->_em->persist($booking1); + $this->_em->persist($booking2); + $this->_em->flush(); + + $this->_em->remove($hotel); + $this->_em->flush(); + } +} + +/** + * @Entity + */ +class DDC1181Hotel +{ + /** @Id @Column(type="integer") @GeneratedValue */ + public $id; + + /** + * @oneToMany(targetEntity="DDC1181Booking", mappedBy="hotel", cascade={"remove"}) + * @var Booking[] + */ + public $bookings; + +} + +/** + * @Entity + */ +class DDC1181Booking +{ + /** + * @var Hotel + * + * @Id + * @ManyToOne(targetEntity="DDC1181Hotel", inversedBy="bookings") + * @JoinColumns({ + * @JoinColumn(name="hotel_id", referencedColumnName="id") + * }) + */ + public $hotel; + /** + * @var Room + * + * @Id + * @ManyToOne(targetEntity="DDC1181Room") + * @JoinColumns({ + * @JoinColumn(name="room_id", referencedColumnName="id") + * }) + */ + public $room; +} + +/** + * @Entity + */ +class DDC1181Room +{ + /** @Id @Column(type="integer") @GeneratedValue */ + public $id; +} \ No newline at end of file From d17d0f5452b527ed8027c339f2d99214032272b0 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 5 Jun 2011 14:49:54 +0200 Subject: [PATCH 11/14] DDC-1192 - Fix notice in XmlDriver, removed unnecessary code. --- lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php index a42d449e6..b41710b4b 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php +++ b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php @@ -356,9 +356,6 @@ class XmlDriver extends AbstractFileDriver $joinColumns[] = $this->_getJoinColumnMapping($manyToOneElement->{'join-column'}); } else if (isset($manyToOneElement->{'join-columns'})) { foreach ($manyToOneElement->{'join-columns'}->{'join-column'} as $joinColumnElement) { - if (!isset($joinColumnElement['name'])) { - $joinColumnElement['name'] = $name; - } $joinColumns[] = $this->_getJoinColumnMapping($joinColumnElement); } } From 22826ac10db1eaf1422c306edd11898d08cb9085 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 5 Jun 2011 15:00:49 +0200 Subject: [PATCH 12/14] DDC-1156 - Do not throw exception for mapped superclass in middle of inheritance hierachy anymore. --- .../ORM/Mapping/ClassMetadataFactory.php | 3 +- .../ORM/Mapping/AnnotationDriverTest.php | 46 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php index cd7a6da68..506f99763 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php @@ -325,7 +325,8 @@ class ClassMetadataFactory implements ClassMetadataFactoryInterface if (!$class->discriminatorColumn) { throw MappingException::missingDiscriminatorColumn($class->name); } - } else if ($class->isMappedSuperclass && (count($class->discriminatorMap) || $class->discriminatorColumn)) { + } else if ($class->isMappedSuperclass && $class->name == $class->rootEntityName && (count($class->discriminatorMap) || $class->discriminatorColumn)) { + // second condition is necessary for mapped superclasses in the middle of an inheritance hierachy throw MappingException::noInheritanceOnMappedSuperClass($class->name); } diff --git a/tests/Doctrine/Tests/ORM/Mapping/AnnotationDriverTest.php b/tests/Doctrine/Tests/ORM/Mapping/AnnotationDriverTest.php index be8a9bc43..e946a2628 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/AnnotationDriverTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/AnnotationDriverTest.php @@ -184,6 +184,21 @@ class AnnotationDriverTest extends AbstractMappingDriverTest $cm = $factory->getMetadataFor('Doctrine\Tests\ORM\Mapping\AnnotationParent'); $this->assertEquals(array("postLoad" => array("postLoad"), "preUpdate" => array("preUpdate")), $cm->lifecycleCallbacks); } + + /** + * @group DDC-1156 + */ + public function testMappedSuperclassInMiddleOfInheritanceHierachy() + { + $annotationDriver = $this->_loadDriver(); + + $em = $this->_getTestEntityManager(); + $em->getConfiguration()->setMetadataDriverImpl($annotationDriver); + $factory = new \Doctrine\ORM\Mapping\ClassMetadataFactory(); + $factory->setEntityManager($em); + + $cm = $factory->getMetadataFor('Doctrine\Tests\ORM\Mapping\ChildEntity'); + } } /** @@ -264,4 +279,35 @@ class AnnotationParent class AnnotationChild extends AnnotationParent { +} + +/** + * @Entity + * @InheritanceType("SINGLE_TABLE") + * @DiscriminatorMap({"s"="SuperEntity", "c"="ChildEntity"}) + */ +class SuperEntity +{ + /** @Id @Column(type="string") */ + private $id; +} + +/** + * @MappedSuperclass + */ +class MiddleMappedSuperclass extends SuperEntity +{ + /** @Column(type="string") */ + private $name; +} + +/** + * @Entity + */ +class ChildEntity extends MiddleMappedSuperclass +{ + /** + * @Column(type="string") + */ + private $text; } \ No newline at end of file From 4371e8fab0f56cff469e57f6bbf44ef48dce5778 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 5 Jun 2011 16:20:41 +0200 Subject: [PATCH 13/14] DDC-1163 - Fix nasty bug with inheritance in UnitOfWork::executeUpdates() and executeRemovals() --- lib/Doctrine/ORM/UnitOfWork.php | 4 +- .../ORM/Functional/ReferenceProxyTest.php | 18 ++ .../ORM/Functional/Ticket/DDC1163Test.php | 215 ++++++++++++++++++ 3 files changed, 235 insertions(+), 2 deletions(-) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1163Test.php diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 8a3d582f7..e53f38c52 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -746,7 +746,7 @@ class UnitOfWork implements PropertyChangedListener $hasPostUpdateListeners = $this->evm->hasListeners(Events::postUpdate); foreach ($this->entityUpdates as $oid => $entity) { - if (get_class($entity) == $className || $entity instanceof Proxy && $entity instanceof $className) { + if (get_class($entity) == $className || $entity instanceof Proxy && get_parent_class($entity) == $className) { if ($hasPreUpdateLifecycleCallbacks) { $class->invokeLifecycleCallbacks(Events::preUpdate, $entity); @@ -788,7 +788,7 @@ class UnitOfWork implements PropertyChangedListener $hasListeners = $this->evm->hasListeners(Events::postRemove); foreach ($this->entityDeletions as $oid => $entity) { - if (get_class($entity) == $className || $entity instanceof Proxy && $entity instanceof $className) { + if (get_class($entity) == $className || $entity instanceof Proxy && get_parent_class($entity) == $className) { $persister->delete($entity); unset( $this->entityDeletions[$oid], diff --git a/tests/Doctrine/Tests/ORM/Functional/ReferenceProxyTest.php b/tests/Doctrine/Tests/ORM/Functional/ReferenceProxyTest.php index 9fcb90a5e..3b6e1f546 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ReferenceProxyTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ReferenceProxyTest.php @@ -112,4 +112,22 @@ class ReferenceProxyTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->_em->getUnitOfWork()->initializeObject($entity); $this->assertTrue($entity->__isInitialized__, "Should be initialized after called UnitOfWork::initializeObject()"); } + + /** + * @group DDC-1163 + */ + public function testInitializeChangeAndFlushProxy() + { + $id = $this->createProduct(); + + /* @var $entity Doctrine\Tests\Models\ECommerce\ECommerceProduct */ + $entity = $this->_em->getReference('Doctrine\Tests\Models\ECommerce\ECommerceProduct' , $id); + $entity->setName('Doctrine 2 Cookbook'); + + $this->_em->flush(); + $this->_em->clear(); + + $entity = $this->_em->getReference('Doctrine\Tests\Models\ECommerce\ECommerceProduct' , $id); + $this->assertEquals('Doctrine 2 Cookbook', $entity->getName()); + } } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1163Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1163Test.php new file mode 100644 index 000000000..7142ff211 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1163Test.php @@ -0,0 +1,215 @@ +_em->getConnection()->getConfiguration()->setSQLLogger(new \Doctrine\DBAL\Logging\EchoSQLLogger); + $this->_schemaTool->createSchema(array( + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC1163Product'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC1163SpecialProduct'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC1163ProxyHolder'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC1163Tag'), + )); + } + + public function testIssue() + { + $this->createSpecialProductAndProxyHolderReferencingIt(); + $this->_em->clear(); + + $this->createProxyForSpecialProduct(); + + $this->setPropertyAndAssignTagToSpecialProduct(); + + // fails + $this->_em->flush(); + } + + private function createSpecialProductAndProxyHolderReferencingIt() + { + $specialProduct = new DDC1163SpecialProduct(); + $this->_em->persist($specialProduct); + + $proxyHolder = new DDC1163ProxyHolder(); + $this->_em->persist($proxyHolder); + + $proxyHolder->setSpecialProduct($specialProduct); + + $this->_em->flush(); + + $this->productId = $specialProduct->getId(); + $this->proxyHolderId = $proxyHolder->getId(); + } + + /** + * We want Doctrine to instantiate a lazy-load proxy for the previously created + * 'SpecialProduct' and register it. + * + * When Doctrine loads the 'ProxyHolder', it will do just that because the 'ProxyHolder' + * references the 'SpecialProduct'. + */ + private function createProxyForSpecialProduct() + { + /* @var $proxyHolder ProxyHolder */ + $proxyHolder = $this->_em->find(__NAMESPACE__ . '\\DDC1163ProxyHolder', $this->proxyHolderId); + + $this->assertInstanceOf(__NAMESPACE__.'\\DDC1163SpecialProduct', $proxyHolder->getSpecialProduct()); + } + + private function setPropertyAndAssignTagToSpecialProduct() + { + /* @var $specialProduct SpecialProduct */ + $specialProduct = $this->_em->find(__NAMESPACE__ . '\\DDC1163SpecialProduct', $this->productId); + + $this->assertInstanceOf(__NAMESPACE__.'\\DDC1163SpecialProduct', $specialProduct); + $this->assertInstanceOf('Doctrine\ORM\Proxy\Proxy', $specialProduct); + + $specialProduct->setSubclassProperty('foobar'); + + // this screams violation of law of demeter ;) + $this->assertEquals( + __NAMESPACE__.'\\DDC1163SpecialProduct', + $this->_em->getUnitOfWork()->getEntityPersister(get_class($specialProduct))->getClassMetadata()->name + ); + + $tag = new DDC1163Tag('Foo'); + $this->_em->persist($tag); + $tag->setProduct($specialProduct); + } +} + +/** + * @Entity + */ +class DDC1163ProxyHolder +{ + + /** + * @var int + * @Column(name="id", type="integer") + * @Id + * @GeneratedValue(strategy="AUTO") + */ + private $id; + /** + * @var SpecialProduct + * @OneToOne(targetEntity="DDC1163SpecialProduct") + */ + private $specialProduct; + + public function getId() + { + return $this->id; + } + + public function setSpecialProduct(DDC1163SpecialProduct $specialProduct) + { + $this->specialProduct = $specialProduct; + } + + public function getSpecialProduct() + { + return $this->specialProduct; + } + +} + +/** + * @Entity + * @InheritanceType("JOINED") + * @DiscriminatorColumn(name="type", type="string") + * @DiscriminatorMap({"special" = "DDC1163SpecialProduct"}) + */ +abstract class DDC1163Product +{ + + /** + * @var int + * @Column(name="id", type="integer") + * @Id + * @GeneratedValue(strategy="AUTO") + */ + protected $id; + + public function getId() + { + return $this->id; + } + +} + +/** + * @Entity + */ +class DDC1163SpecialProduct extends DDC1163Product +{ + + /** + * @var string + * @Column(name="subclass_property", type="string", nullable=true) + */ + private $subclassProperty; + + /** + * @param string $value + */ + public function setSubclassProperty($value) + { + $this->subclassProperty = $value; + } + +} + +/** + * @Entity + */ +class DDC1163Tag +{ + + /** + * @var int + * @Column(name="id", type="integer") + * @Id + * @GeneratedValue(strategy="AUTO") + */ + private $id; + /** + * @var string + * @Column(name="name", type="string") + */ + private $name; + /** + * @var Product + * @ManyToOne(targetEntity="DDC1163Product", inversedBy="tags") + * @JoinColumns({ + * @JoinColumn(name="product_id", referencedColumnName="id") + * }) + */ + private $product; + + /** + * @param string $name + */ + public function __construct($name) + { + $this->name = $name; + } + + /** + * @param Product $product + */ + public function setProduct(DDC1163Product $product) + { + $this->product = $product; + } + +} \ No newline at end of file From a4cbb23fc8612587d1886e4c3e7d62d72457a297 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 5 Jun 2011 17:21:37 +0200 Subject: [PATCH 14/14] Slight adjustment to build.xml --- build.xml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/build.xml b/build.xml index 320b593ed..e4c138ef6 100644 --- a/build.xml +++ b/build.xml @@ -232,6 +232,7 @@ + bin Doctrine/Common/ Doctrine/DBAL/ Doctrine/ORM/ @@ -254,6 +255,7 @@ + bin Doctrine/Common/ Doctrine/DBAL/ Doctrine/ORM/