From 62446f0f3c1d935049762496650e0966c2a72ea2 Mon Sep 17 00:00:00 2001 From: romanb Date: Tue, 28 Jul 2009 16:36:24 +0000 Subject: [PATCH] [2.0] Cleanup for changeset #6172. --- .../ORM/Internal/Hydration/ObjectHydrator.php | 79 ++++--- .../ORM/Mapping/AssociationMapping.php | 3 +- lib/Doctrine/ORM/Mapping/ClassMetadata.php | 6 +- .../ORM/Mapping/ManyToManyMapping.php | 26 +- lib/Doctrine/ORM/Mapping/OneToManyMapping.php | 15 +- lib/Doctrine/ORM/PersistentCollection.php | 223 +++++++----------- .../Persisters/JoinedSubclassPersister.php | 2 +- .../Persisters/StandardEntityPersister.php | 89 ++++++- lib/Doctrine/ORM/Query/SqlWalker.php | 7 +- tests/Doctrine/Tests/Models/CMS/CmsUser.php | 13 +- ...ManyToManyBidirectionalAssociationTest.php | 26 +- .../ORM/Query/SelectSqlGenerationTest.php | 6 +- 12 files changed, 278 insertions(+), 217 deletions(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php index 61e684a55..7cddfd258 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php @@ -37,7 +37,7 @@ class ObjectHydrator extends AbstractHydrator /* * These two properties maintain their values between hydration runs. */ - /* Class entries */ + /* Local ClassMetadata cache to avoid going to the EntityManager all the time. */ private $_ce = array(); private $_discriminatorMap = array(); /* @@ -48,9 +48,9 @@ class ObjectHydrator extends AbstractHydrator private $_identifierMap = array(); private $_resultPointers = array(); private $_idTemplate = array(); - private $_resultCounter = 0; + private $_resultCounter; private $_rootAliases = array(); - private $_fetchedAssociations = array(); + private $_fetchedAssociations; /* TODO: Consider unifying _collections and _initializedRelations */ /** Collections initialized by the hydrator */ private $_collections = array(); @@ -63,11 +63,12 @@ class ObjectHydrator extends AbstractHydrator $this->_isSimpleQuery = count($this->_rsm->aliasMap) <= 1; $this->_allowPartialObjects = $this->_em->getConfiguration()->getAllowPartialObjects() || isset($this->_hints[Query::HINT_FORCE_PARTIAL_LOAD]); - $this->_identifierMap = array(); - $this->_resultPointers = array(); - $this->_idTemplate = array(); - $this->_resultCounter = 0; + + $this->_identifierMap = + $this->_resultPointers = + $this->_idTemplate = $this->_fetchedAssociations = array(); + $this->_resultCounter = 0; foreach ($this->_rsm->aliasMap as $dqlAlias => $className) { $this->_identifierMap[$dqlAlias] = array(); @@ -172,25 +173,26 @@ class ObjectHydrator extends AbstractHydrator * * @param object $entity The entity to which the collection belongs. * @param string $name The name of the field on the entity that holds the collection. - * @return PersistentCollection */ private function initRelatedCollection($entity, $name) { $oid = spl_object_hash($entity); - $classMetadata = $this->_ce[get_class($entity)]; + $class = $this->_ce[get_class($entity)]; - $relation = $classMetadata->associationMappings[$name]; - if ( ! isset($this->_ce[$relation->targetEntityName])) { - $this->_ce[$relation->targetEntityName] = $this->_em->getClassMetadata($relation->targetEntityName); - } - $coll = new PersistentCollection($this->_em, $this->_ce[$relation->targetEntityName]); - $this->_collections[] = $coll; - $coll->setOwner($entity, $relation); + $relation = $class->associationMappings[$name]; + + //$coll = $class->reflFields[$name]->getValue($entity); + //if ( ! $coll) { + // $coll = new Collection; + //} + + $pColl = new PersistentCollection($this->_em, $this->_getClassMetadata($relation->targetEntityName)); + $this->_collections[] = $pColl; + $pColl->setOwner($entity, $relation); - $classMetadata->reflFields[$name]->setValue($entity, $coll); - $this->_uow->setOriginalEntityProperty($oid, $name, $coll); + $class->reflFields[$name]->setValue($entity, $pColl); + $this->_uow->setOriginalEntityProperty($oid, $name, $pColl); $this->_initializedRelations[$oid][$name] = true; - return $coll; } /** @@ -210,10 +212,10 @@ class ObjectHydrator extends AbstractHydrator } /** + * Gets the last key of a collection/array. * - * @param $coll - * @return - * @todo Consider inlining this method, introducing $coll->lastKey(). + * @param Collection|array $coll + * @return string|integer */ private function getLastKey($coll) { @@ -248,6 +250,7 @@ class ObjectHydrator extends AbstractHydrator // Properly initialize any unfetched associations, if partial objects are not allowed. if ( ! $this->_allowPartialObjects) { foreach ($this->_ce[$className]->associationMappings as $field => $assoc) { + // Check if the association is not among the fetch-joined associatons already. if ( ! isset($this->_fetchedAssociations[$className][$field])) { if ($assoc->isOneToOne()) { $joinColumns = array(); @@ -257,7 +260,8 @@ class ObjectHydrator extends AbstractHydrator if ($assoc->isLazilyFetched()) { // Inject proxy $this->_ce[$className]->reflFields[$field]->setValue($entity, - $this->_em->getProxyFactory()->getAssociationProxy($entity, $assoc, $joinColumns)); + $this->_em->getProxyFactory()->getAssociationProxy($entity, $assoc, $joinColumns) + ); } else { // Eager load //TODO: Allow more efficient and configurable batching of these loads @@ -265,12 +269,14 @@ class ObjectHydrator extends AbstractHydrator } } else { // Inject collection - $collection = $this->initRelatedCollection($entity, $field); - if ($assoc->isLazilyFetched()) { - $collection->setLazyInitialization(); - } else { + $pColl = new PersistentCollection($this->_em, $this->_getClassMetadata($assoc->targetEntityName)); + $pColl->setOwner($entity, $assoc); + $this->_ce[$className]->reflFields[$field]->setValue($entity, $pColl); + if ( ! $assoc->isLazilyFetched()) { //TODO: Allow more efficient and configurable batching of these loads - $assoc->load($entity, $collection, $this->_em); + $assoc->load($entity, $pColl, $this->_em); + } else { + $pColl->setInitialized(false); } } } @@ -279,6 +285,22 @@ class ObjectHydrator extends AbstractHydrator return $entity; } + + /** + * Gets a ClassMetadata instance from the local cache. + * If the instance is not yet in the local cache, it is loaded into the + * local cache. + * + * @param string $className The name of the class. + * @return ClassMetadata + */ + private function _getClassMetadata($className) + { + if ( ! isset($this->_ce[$className])) { + $this->_ce[$className] = $this->_em->getClassMetadata($className); + } + return $this->_ce[$className]; + } /** * Sets a related element. @@ -463,7 +485,6 @@ class ObjectHydrator extends AbstractHydrator $index = $this->_identifierMap[$dqlAlias][$id[$dqlAlias]]; } $this->updateResultPointer($result, $index, $dqlAlias); - //unset($rowData[$dqlAlias]); } } diff --git a/lib/Doctrine/ORM/Mapping/AssociationMapping.php b/lib/Doctrine/ORM/Mapping/AssociationMapping.php index 3aedb7857..bf417e7be 100644 --- a/lib/Doctrine/ORM/Mapping/AssociationMapping.php +++ b/lib/Doctrine/ORM/Mapping/AssociationMapping.php @@ -412,7 +412,6 @@ abstract class AssociationMapping */ protected function _getPrivateValue(ClassMetadata $class, $entity, $column) { - $reflField = $class->getReflectionProperty($class->getFieldName($column)); - return $reflField->getValue($entity); + return $class->reflFields[$class->fieldNames[$column]]->getValue($entity); } } diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadata.php b/lib/Doctrine/ORM/Mapping/ClassMetadata.php index 0cd637404..a8460f5de 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadata.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadata.php @@ -405,10 +405,10 @@ final class ClassMetadata public function __construct($entityName) { $this->name = $entityName; - $this->namespace = substr($entityName, 0, strrpos($entityName, '\\')); - $this->primaryTable['name'] = str_replace($this->namespace . '\\', '', $this->name); - $this->rootEntityName = $entityName; $this->reflClass = new \ReflectionClass($entityName); + $this->namespace = $this->reflClass->getNamespaceName(); + $this->primaryTable['name'] = $this->reflClass->getShortName(); + $this->rootEntityName = $entityName; } /** diff --git a/lib/Doctrine/ORM/Mapping/ManyToManyMapping.php b/lib/Doctrine/ORM/Mapping/ManyToManyMapping.php index f32abc675..b9eccad7a 100644 --- a/lib/Doctrine/ORM/Mapping/ManyToManyMapping.php +++ b/lib/Doctrine/ORM/Mapping/ManyToManyMapping.php @@ -70,7 +70,7 @@ class ManyToManyMapping extends AssociationMapping /** * Initializes a new ManyToManyMapping. * - * @param array $mapping The mapping information. + * @param array $mapping The mapping definition. */ public function __construct(array $mapping) { @@ -86,7 +86,7 @@ class ManyToManyMapping extends AssociationMapping protected function _validateAndCompleteMapping(array $mapping) { parent::_validateAndCompleteMapping($mapping); - if ($this->isOwningSide()) { + if ($this->isOwningSide) { // owning side MUST have a join table if ( ! isset($mapping['joinTable'])) { throw MappingException::joinTableRequired($mapping['fieldName']); @@ -143,15 +143,19 @@ class ManyToManyMapping extends AssociationMapping * Loads entities in $targetCollection using $em. * The data of $sourceEntity are used to restrict the collection * via the join table. + * + * @param object The owner of the collection. + * @param object The collection to populate. + * @param array */ public function load($sourceEntity, $targetCollection, $em, array $joinColumnValues = array()) { $sourceClass = $em->getClassMetadata($this->sourceEntityName); $joinTableConditions = array(); if ($this->isOwningSide()) { - $joinTable = $this->getJoinTable(); - $joinClauses = $this->getTargetToRelationKeyColumns(); - foreach ($this->getSourceToRelationKeyColumns() as $sourceKeyColumn => $relationKeyColumn) { + $joinTable = $this->joinTable; + $joinClauses = $this->targetToRelationKeyColumns; + foreach ($this->sourceToRelationKeyColumns as $sourceKeyColumn => $relationKeyColumn) { // getting id if (isset($sourceClass->reflFields[$sourceKeyColumn])) { $joinTableConditions[$relationKeyColumn] = $this->_getPrivateValue($sourceClass, $sourceEntity, $sourceKeyColumn); @@ -160,11 +164,11 @@ class ManyToManyMapping extends AssociationMapping } } } else { - $owningAssoc = $em->getClassMetadata($this->targetEntityName)->getAssociationMapping($this->mappedByFieldName); - $joinTable = $owningAssoc->getJoinTable(); + $owningAssoc = $em->getClassMetadata($this->targetEntityName)->associationMappings[$this->mappedByFieldName]; + $joinTable = $owningAssoc->joinTable; // TRICKY: since the association is inverted source and target are flipped - $joinClauses = $owningAssoc->getSourceToRelationKeyColumns(); - foreach ($owningAssoc->getTargetToRelationKeyColumns() as $sourceKeyColumn => $relationKeyColumn) { + $joinClauses = $owningAssoc->sourceToRelationKeyColumns; + foreach ($owningAssoc->targetToRelationKeyColumns as $sourceKeyColumn => $relationKeyColumn) { // getting id if (isset($sourceClass->reflFields[$sourceKeyColumn])) { $joinTableConditions[$relationKeyColumn] = $this->_getPrivateValue($sourceClass, $sourceEntity, $sourceKeyColumn); @@ -179,13 +183,11 @@ class ManyToManyMapping extends AssociationMapping 'criteria' => $joinTableConditions ); $persister = $em->getUnitOfWork()->getEntityPersister($this->targetEntityName); - $persister->loadCollection(array($joinTableCriteria), $targetCollection); + $persister->loadManyToManyCollection(array($joinTableCriteria), $targetCollection); } /** * {@inheritdoc} - * - * @override */ public function isManyToMany() { diff --git a/lib/Doctrine/ORM/Mapping/OneToManyMapping.php b/lib/Doctrine/ORM/Mapping/OneToManyMapping.php index 34d30177e..68a769a6a 100644 --- a/lib/Doctrine/ORM/Mapping/OneToManyMapping.php +++ b/lib/Doctrine/ORM/Mapping/OneToManyMapping.php @@ -104,13 +104,22 @@ class OneToManyMapping extends AssociationMapping { return true; } - + + /** + * + * + * @param $sourceEntity + * @param $targetCollection + * @param $em + * @param $joinColumnValues + * @return unknown_type + */ public function load($sourceEntity, $targetCollection, $em, array $joinColumnValues = array()) { $persister = $em->getUnitOfWork()->getEntityPersister($this->targetEntityName); // a one-to-many is always inverse (does not have foreign key) $sourceClass = $em->getClassMetadata($this->sourceEntityName); - $owningAssoc = $em->getClassMetadata($this->targetEntityName)->getAssociationMapping($this->mappedByFieldName); + $owningAssoc = $em->getClassMetadata($this->targetEntityName)->associationMappings[$this->mappedByFieldName]; // TRICKY: since the association is specular source and target are flipped foreach ($owningAssoc->getTargetToSourceKeyColumns() as $sourceKeyColumn => $targetKeyColumn) { // getting id @@ -121,6 +130,6 @@ class OneToManyMapping extends AssociationMapping } } - $persister->loadCollection($conditions, $targetCollection); + $persister->loadOneToManyCollection($conditions, $targetCollection); } } diff --git a/lib/Doctrine/ORM/PersistentCollection.php b/lib/Doctrine/ORM/PersistentCollection.php index 5f22a5316..729837b3c 100644 --- a/lib/Doctrine/ORM/PersistentCollection.php +++ b/lib/Doctrine/ORM/PersistentCollection.php @@ -46,13 +46,6 @@ use \Closure; */ final class PersistentCollection extends \Doctrine\Common\Collections\Collection { - /** - * The base type of the collection. - * - * @var string - */ - private $_type; - /** * A snapshot of the collection at the moment it was fetched from the database. * This is used to create a diff of the collection at commit time. @@ -76,13 +69,6 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection */ private $_association; - /** - * The name of the field that is used for collection indexing. - * - * @var string - */ - private $_keyField; - /** * The EntityManager that manages the persistence of the collection. * @@ -112,7 +98,7 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection private $_isDirty = false; /** Whether the collection has already been initialized. */ - protected $_initialized = true; + private $_initialized = true; /** * Creates a new persistent collection. @@ -120,32 +106,10 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection public function __construct(EntityManager $em, $class, array $data = array()) { parent::__construct($data); - $this->_type = $class->name; $this->_em = $em; $this->_typeClass = $class; } - /** - * INTERNAL: Sets the key column for this collection - * - * @param string $column - * @return Doctrine_Collection - */ - public function setKeyField($fieldName) - { - $this->_keyField = $fieldName; - } - - /** - * INTERNAL: returns the name of the key column - * - * @return string - */ - public function getKeyField() - { - return $this->_keyField; - } - /** * INTERNAL: * Sets the collection owner. Used (only?) during hydration. @@ -157,6 +121,7 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection { $this->_owner = $entity; $this->_association = $assoc; + // Check for bidirectionality if ($assoc->isInverseSide()) { // For sure bi-directional $this->_backRefFieldName = $assoc->mappedByFieldName; @@ -190,23 +155,18 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection return $this->_typeClass; } - public function setLazyInitialization() - { - $this->_initialized = false; - } - /** * INTERNAL: - * Adds an element to a collection during hydration, if it not already set. - * This control is performed to avoid infinite recursion during hydration - * of bidirectional many-to-many collections. + * Adds an element to a collection during hydration. * * @param mixed $element The element to add. */ - public function hydrateAdd($element, $setBackRef = true) + public function hydrateAdd($element) { parent::add($element); - if ($this->_backRefFieldName and $setBackRef) { + // If _backRefFieldName is set, then the association is bidirectional + // and we need to set the back reference. + if ($this->_backRefFieldName) { // Set back reference to owner if ($this->_association->isOneToMany()) { // OneToMany @@ -214,9 +174,8 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection ->setValue($element, $this->_owner); } else { // ManyToMany - $otherCollection = $this->_typeClass->reflFields[$this->_backRefFieldName] - ->getValue($element); - $otherCollection->hydrateAdd($this->_owner, false); + $this->_typeClass->reflFields[$this->_backRefFieldName] + ->getValue($element)->add($this->_owner); } } } @@ -234,12 +193,12 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection } /** - * Initializes the collection by loading its contents from the database. + * Initializes the collection by loading its contents from the database + * if the collection is not yet initialized. */ private function _initialize() { - if (!$this->_initialized) { - parent::clear(); + if ( ! $this->_initialized) { $this->_association->load($this->_owner, $this, $this->_em); $this->_initialized = true; } @@ -345,54 +304,33 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection { $this->_initialized = $bool; } - - /* Serializable implementation */ - + /** - * Called by PHP when this collection is serialized. Ensures that only the - * elements are properly serialized. + * Checks whether this collection has been initialized. * - * @internal Tried to implement Serializable first but that did not work well - * with circular references. This solution seems simpler and works well. + * @return boolean */ - public function __sleep() + public function isInitialized() { - return array('_elements', '_initialized'); - } - - /** - * Decorated Collection methods. - */ - public function unwrap() - { - $this->_initialize(); - return parent::unwrap(); + return $this->_initialized; } + /** {@inheritdoc} */ public function first() { $this->_initialize(); return parent::first(); } + /** {@inheritdoc} */ public function last() { $this->_initialize(); return parent::last(); } - public function key() - { - $this->_initialize(); - return parent::key(); - } - /** - * Removes an element from the collection. - * - * @param mixed $key - * @return boolean - * @override + * {@inheritdoc} */ public function remove($key) { @@ -408,6 +346,9 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection return $removed; } + /** + * {@inheritdoc} + */ public function removeElement($element) { $this->_initialize(); @@ -416,34 +357,9 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection return $result; } - public function offsetExists($offset) - { - $this->_initialize(); - return parent::offsetExists($offset); - } - - public function offsetGet($offset) - { - $this->_initialize(); - return parent::offsetGet($offset); - } - - public function offsetSet($offset, $value) - { - $this->_initialize(); - $result = parent::offsetSet($offset, $value); - $this->_changed(); - return $result; - } - - public function offsetUnset($offset) - { - $this->_initialize(); - $result = parent::offsetUnset($offset); - $this->_changed(); - return $result; - } - + /** + * {@inheritdoc} + */ public function containsKey($key) { $this->_initialize(); @@ -451,8 +367,7 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection } /** - * Checks whether an element is contained in the collection. - * This is an O(n) operation. + * {@inheritdoc} */ public function contains($element) { @@ -460,30 +375,45 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection return parent::contains($element); } + /** + * {@inheritdoc} + */ public function exists(Closure $p) { $this->_initialize(); return parent::exists($p); } + /** + * {@inheritdoc} + */ public function search($element) { $this->_initialize(); return parent::search($element); } + /** + * {@inheritdoc} + */ public function get($key) { $this->_initialize(); return parent::get($key); } + /** + * {@inheritdoc} + */ public function getKeys() { $this->_initialize(); return parent::getKeys(); } + /** + * {@inheritdoc} + */ public function getElements() { $this->_initialize(); @@ -491,7 +421,7 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection } /** - * @override + * {@inheritdoc} */ public function count() { @@ -500,49 +430,45 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection } /** - * When the collection is used as a Map this is like put(key,value)/add(key,value). - * When the collection is used as a List this is like add(position,value). - * - * @param integer $key - * @param mixed $value - * @override + * {@inheritdoc} */ public function set($key, $value) { - $this->_initialize(); - $result = parent::set($key, $value); + parent::set($key, $value); $this->_changed(); - return $result; } /** - * Adds an element to the collection. - * - * @param mixed $value - * @param string $key - * @return boolean Always TRUE. - * @override + * {@inheritdoc} */ public function add($value) { - $this->_initialize(); - $result = parent::add($value); + parent::add($value); $this->_changed(); - return $result; + return true; } + /** + * {@inheritdoc} + */ public function isEmpty() { $this->_initialize(); return parent::isEmpty(); } - + + /** + * {@inheritdoc} + */ public function getIterator() { $this->_initialize(); return parent::getIterator(); } + /** + * {@inheritdoc} + */ public function map(Closure $func) { $this->_initialize(); @@ -551,18 +477,27 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection return $result; } + /** + * {@inheritdoc} + */ public function filter(Closure $p) { $this->_initialize(); return parent::filter($p); } - + + /** + * {@inheritdoc} + */ public function forAll(Closure $p) { $this->_initialize(); return parent::forAll($p); } + /** + * {@inheritdoc} + */ public function partition(Closure $p) { $this->_initialize(); @@ -570,23 +505,29 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection } /** - * Clears the collection. + * {@inheritdoc} */ public function clear() { $this->_initialize(); - //TODO: Register collection as dirty with the UoW if necessary - //TODO: If oneToMany() && shouldDeleteOrphan() delete entities - /*if ($this->_association->isOneToMany() && $this->_association->shouldDeleteOrphans) { - foreach ($this->_data as $entity) { - $this->_em->remove($entity); - } - }*/ $result = parent::clear(); if ($this->_association->isOwningSide) { $this->_changed(); $this->_em->getUnitOfWork()->scheduleCollectionDeletion($this); } + return $result; } + + /** + * Called by PHP when this collection is serialized. Ensures that only the + * elements are properly serialized. + * + * @internal Tried to implement Serializable first but that did not work well + * with circular references. This solution seems simpler and works well. + */ + public function __sleep() + { + return array('_elements'); + } } diff --git a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php index e6d4502f0..f7fb9d359 100644 --- a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php +++ b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php @@ -273,7 +273,7 @@ class JoinedSubclassPersister extends StandardEntityPersister * @todo Quote identifier. * @override */ - protected function _getSelectSingleEntitySql(array &$criteria) + protected function _getSelectEntitiesSql(array &$criteria) { $tableAliases = array(); $aliasIndex = 1; diff --git a/lib/Doctrine/ORM/Persisters/StandardEntityPersister.php b/lib/Doctrine/ORM/Persisters/StandardEntityPersister.php index c9952475b..5664eafce 100644 --- a/lib/Doctrine/ORM/Persisters/StandardEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/StandardEntityPersister.php @@ -395,16 +395,20 @@ class StandardEntityPersister */ public function load(array $criteria, $entity = null) { - $stmt = $this->_conn->prepare($this->_getSelectSingleEntitySql($criteria)); + $stmt = $this->_conn->prepare($this->_getSelectEntitiesSql($criteria)); $stmt->execute(array_values($criteria)); $result = $stmt->fetch(Connection::FETCH_ASSOC); $stmt->closeCursor(); return $this->_createEntity($result, $entity); } - - public function loadCollection(array $criteria, PersistentCollection $collection) + + /** + * + * + */ + public function loadOneToManyCollection(array $criteria, PersistentCollection $collection) { - $sql = $this->_getSelectSingleEntitySql($criteria); + $sql = $this->_getSelectEntitiesSql($criteria); $stmt = $this->_conn->prepare($sql); $stmt->execute(array_values($criteria)); while ($result = $stmt->fetch(Connection::FETCH_ASSOC)) { @@ -412,7 +416,28 @@ class StandardEntityPersister } $stmt->closeCursor(); } - + + /** + * + * + */ + public function loadManyToManyCollection(array $criteria, PersistentCollection $collection) + { + $sql = $this->_getSelectManyToManyEntityCollectionSql($criteria); + $stmt = $this->_conn->prepare($sql); + $stmt->execute(array_values($criteria)); + while ($result = $stmt->fetch(Connection::FETCH_ASSOC)) { + $collection->add($this->_createEntity($result)); + } + $stmt->closeCursor(); + } + + /** + * + * @param $result + * @param $entity + * @return object + */ private function _createEntity($result, $entity = null) { $data = array(); @@ -469,7 +494,7 @@ class StandardEntityPersister $coll->setOwner($entity, $assoc); $this->_class->reflFields[$field]->setValue($entity, $coll); if ($assoc->isLazilyFetched()) { - $coll->setLazyInitialization(); + $coll->setInitialized(false); } else { //TODO: Allow more efficient and configurable batching of these loads $assoc->load($entity, $coll, $this->_em); @@ -490,7 +515,7 @@ class StandardEntityPersister * JoinClauses are used to restrict the result returned but only columns of this entity are selected (@see _getJoinSql()). * @return string The SQL. */ - protected function _getSelectSingleEntitySql(array &$criteria) + protected function _getSelectEntitiesSql(array &$criteria) { $columnList = ''; foreach ($this->_class->columnNames as $column) { @@ -527,6 +552,7 @@ class StandardEntityPersister unset($criteria[$field]); continue; } + if (isset($this->_class->columnNames[$field])) { $columnName = $this->_class->columnNames[$field]; } else if (in_array($field, $joinColumnNames)) { @@ -542,9 +568,54 @@ class StandardEntityPersister . $joinSql . ' WHERE ' . $conditionSql; } + + /** + * Gets the SQL to select a collection of entities in a many-many association. + * + * @return string + */ + protected function _getSelectManyToManyEntityCollectionSql(array &$criteria) + { + $columnList = ''; + foreach ($this->_class->columnNames as $column) { + if ($columnList != '') $columnList .= ', '; + $columnList .= $this->_conn->quoteIdentifier($column); + } + + if ( ! $this->_em->getConfiguration()->getAllowPartialObjects()) { + foreach ($this->_class->associationMappings as $assoc) { + if ($assoc->isOwningSide && $assoc->isOneToOne()) { + foreach ($assoc->targetToSourceKeyColumns as $srcColumn) { + $columnList .= ', ' . $this->_conn->quoteIdentifier($srcColumn); + } + } + } + } + + $joinSql = ''; + $conditionSql = ''; + foreach ($criteria as $field => $value) { + if ($conditionSql != '') { + $conditionSql .= ' AND '; + } + $joinSql .= $this->_getJoinSql($value); + foreach ($value['criteria'] as $nestedField => $nestedValue) { + $columnName = "{$value['table']}.{$nestedField}"; + $conditionSql .= $this->_conn->quoteIdentifier($columnName) . ' = ?'; + $criteria[$columnName] = $nestedValue; + } + unset($criteria[$field]); + } + + return 'SELECT ' . $columnList + . ' FROM ' . $this->_class->primaryTable['name'] + . $joinSql + . ' WHERE ' . $conditionSql; + } /** - * Builds a LEFT JOIN sql query part using $joinClause + * Builds an INNER JOIN sql query part using $joinClause. + * * @param array $joinClause keys are: * 'table' => table to join * 'join' => array of [sourceField => joinTableField] @@ -558,6 +629,6 @@ class StandardEntityPersister $clauses[] = $this->_class->getTableName() . ".{$sourceField} = " . "{$joinClause['table']}.{$joinTableField}"; } - return " LEFT JOIN {$joinClause['table']} ON " . implode(' AND ', $clauses); + return ' INNER JOIN ' . $joinClause['table'] . ' ON ' . implode(' AND ', $clauses); } } diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 13c7bfae1..f73ec637a 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -248,11 +248,8 @@ class SqlWalker implements TreeWalker $qComp = $this->_queryComponents[$dqlAlias]; $columnName = $qComp['metadata']->getColumnName($parts[0]); $sql = $this->getSqlTableAlias($qComp['metadata']->getTableName(), $dqlAlias) . '.' . $columnName; - if ($orderByItem->isAsc()) { - $sql .= ' ASC '; - } else if ($orderByItem->isDesc()) { - $sql .= ' DESC '; - } + $sql .= $orderByItem->isDesc() ? ' DESC' : ' ASC'; + return $sql; } diff --git a/tests/Doctrine/Tests/Models/CMS/CmsUser.php b/tests/Doctrine/Tests/Models/CMS/CmsUser.php index 9f3f225e5..a8f2b15e6 100644 --- a/tests/Doctrine/Tests/Models/CMS/CmsUser.php +++ b/tests/Doctrine/Tests/Models/CMS/CmsUser.php @@ -2,6 +2,8 @@ namespace Doctrine\Tests\Models\CMS; +use Doctrine\Common\Collections\Collection; + /** * @Entity * @Table(name="cms_users") @@ -40,10 +42,17 @@ class CmsUser /** * @ManyToMany(targetEntity="CmsGroup", cascade={"persist"}) * @JoinTable(name="cms_users_groups", - joinColumns={@JoinColumn(name="user_id", referencedColumnName="id")}, - inverseJoinColumns={@JoinColumn(name="group_id", referencedColumnName="id")}) + * joinColumns={@JoinColumn(name="user_id", referencedColumnName="id")}, + * inverseJoinColumns={@JoinColumn(name="group_id", referencedColumnName="id")} + * ) */ public $groups; + + public function __construct() { + $this->phonenumbers = new Collection; + $this->articles = new Collection; + $this->groups = new Collection; + } public function getId() { return $this->id; diff --git a/tests/Doctrine/Tests/ORM/Functional/ManyToManyBidirectionalAssociationTest.php b/tests/Doctrine/Tests/ORM/Functional/ManyToManyBidirectionalAssociationTest.php index 0122e4596..951bf5aa0 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ManyToManyBidirectionalAssociationTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ManyToManyBidirectionalAssociationTest.php @@ -28,7 +28,9 @@ class ManyToManyBidirectionalAssociationTest extends AbstractManyToManyAssociati $this->useModelSet('ecommerce'); parent::setUp(); $this->firstProduct = new ECommerceProduct(); + $this->firstProduct->setName("First Product"); $this->secondProduct = new ECommerceProduct(); + $this->secondProduct->setName("Second Product"); $this->firstCategory = new ECommerceCategory(); $this->firstCategory->setName("Business"); $this->secondCategory = new ECommerceCategory(); @@ -102,6 +104,11 @@ class ManyToManyBidirectionalAssociationTest extends AbstractManyToManyAssociati $query = $this->_em->createQuery('SELECT p FROM Doctrine\Tests\Models\ECommerce\ECommerceProduct p order by p.id'); $products = $query->getResultList(); + + $this->assertEquals(2, count($products)); + $this->assertEquals(0, count($products[0]->getCategories()->unwrap())); + $this->assertEquals(0, count($products[1]->getCategories()->unwrap())); + $this->assertLoadingOfOwningSide($products); } @@ -128,16 +135,21 @@ class ManyToManyBidirectionalAssociationTest extends AbstractManyToManyAssociati public function assertLoadingOfOwningSide($products) { list ($firstProduct, $secondProduct) = $products; - $this->assertEquals(2, count($firstProduct->getCategories())); - $this->assertEquals(2, count($secondProduct->getCategories())); - - $categories = $firstProduct->getCategories(); - $firstCategoryProducts = $categories[0]->getProducts(); - $secondCategoryProducts = $categories[1]->getProducts(); + $firstProductCategories = $firstProduct->getCategories(); + $secondProductCategories = $secondProduct->getCategories(); + + $this->assertEquals(2, count($firstProductCategories)); + $this->assertEquals(2, count($secondProductCategories)); + $this->assertTrue($firstProductCategories[0] === $secondProductCategories[0]); + $this->assertTrue($firstProductCategories[1] === $secondProductCategories[1]); + + $firstCategoryProducts = $firstProductCategories[0]->getProducts(); + $secondCategoryProducts = $firstProductCategories[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); diff --git a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php index 3700e1e95..79babd4f5 100644 --- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php @@ -70,7 +70,7 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase { $this->assertSqlGeneration( 'SELECT u FROM Doctrine\Tests\Models\Forum\ForumUser u ORDER BY u.id', - 'SELECT f0_.id AS id0, f0_.username AS username1 FROM forum_users f0_ ORDER BY f0_.id ASC ' + 'SELECT f0_.id AS id0, f0_.username AS username1 FROM forum_users f0_ ORDER BY f0_.id ASC' ); } @@ -78,14 +78,14 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase { $this->assertSqlGeneration( 'SELECT u FROM Doctrine\Tests\Models\Forum\ForumUser u ORDER BY u.id asc', - 'SELECT f0_.id AS id0, f0_.username AS username1 FROM forum_users f0_ ORDER BY f0_.id ASC ' + 'SELECT f0_.id AS id0, f0_.username AS username1 FROM forum_users f0_ ORDER BY f0_.id ASC' ); } public function testSupportsOrderByDesc() { $this->assertSqlGeneration( 'SELECT u FROM Doctrine\Tests\Models\Forum\ForumUser u ORDER BY u.id desc', - 'SELECT f0_.id AS id0, f0_.username AS username1 FROM forum_users f0_ ORDER BY f0_.id DESC ' + 'SELECT f0_.id AS id0, f0_.username AS username1 FROM forum_users f0_ ORDER BY f0_.id DESC' ); }