From 28ca2acb8b8f062da911c51e85a8ddbd659040ac Mon Sep 17 00:00:00 2001 From: romanb Date: Sat, 25 Jul 2009 16:33:29 +0000 Subject: [PATCH] [2.0] Refined implementation and semantics of the merge and detach operations. General cleanups and API improvements. Added a testcase for detaching/serializing->unserializing->modifying->merging to demonstrate the transparent serialization. --- doctrine-mapping.xsd | 4 +- lib/Doctrine/ORM/EntityManager.php | 14 +- .../ORM/Mapping/AssociationMapping.php | 18 +- lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php | 8 +- .../ORM/Mapping/Driver/YamlDriver.php | 8 +- lib/Doctrine/ORM/PersistentCollection.php | 14 +- lib/Doctrine/ORM/UnitOfWork.php | 320 ++++++++++++------ tests/Doctrine/Tests/Models/CMS/CmsUser.php | 6 +- .../Tests/Models/ECommerce/ECommerceCart.php | 2 +- .../Models/ECommerce/ECommerceCategory.php | 2 +- .../Models/ECommerce/ECommerceCustomer.php | 4 +- .../Models/ECommerce/ECommerceProduct.php | 8 +- .../Doctrine/Tests/Models/Forum/ForumUser.php | 2 +- .../ORM/Functional/DetachedEntityTest.php | 44 +++ .../Tests/ORM/Mapping/XmlDriverTest.php | 2 +- .../Tests/ORM/Mapping/YamlDriverTest.php | 2 +- .../Mapping/xml/XmlMappingTest.User.dcm.xml | 2 +- .../Mapping/yaml/YamlMappingTest.User.dcm.yml | 2 +- .../ORM/Performance/InsertPerformanceTest.php | 3 +- 19 files changed, 312 insertions(+), 153 deletions(-) diff --git a/doctrine-mapping.xsd b/doctrine-mapping.xsd index 25976c428..c19e6dac6 100644 --- a/doctrine-mapping.xsd +++ b/doctrine-mapping.xsd @@ -25,9 +25,9 @@ - + - + diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index 65da137ea..7363f580b 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -416,21 +416,25 @@ class EntityManager } /** - * Detaches an entity from the EntityManager. + * Detaches an entity from the EntityManager, causing a managed entity to + * become detached. Unflushed changes made to the entity if any + * (including removal of the entity), will not be synchronized to the database. + * Entities which previously referenced the detached entity will continue to + * reference it. * * @param object $entity The entity to detach. - * @return boolean */ public function detach($entity) { - return $this->_unitOfWork->removeFromIdentityMap($entity); + $this->_unitOfWork->detach($entity); } /** * Merges the state of a detached entity into the persistence context - * of this EntityManager. + * of this EntityManager and returns the managed copy of the entity. + * The entity passed to merge will not become associated/managed with this EntityManager. * - * @param object $entity The entity to merge into the persistence context. + * @param object $entity The detached entity to merge into the persistence context. * @return object The managed copy of the entity. */ public function merge($entity) diff --git a/lib/Doctrine/ORM/Mapping/AssociationMapping.php b/lib/Doctrine/ORM/Mapping/AssociationMapping.php index cd72b09c3..aba3efa0d 100644 --- a/lib/Doctrine/ORM/Mapping/AssociationMapping.php +++ b/lib/Doctrine/ORM/Mapping/AssociationMapping.php @@ -66,8 +66,8 @@ abstract class AssociationMapping ); public $cascades = array(); - public $isCascadeDelete; - public $isCascadeSave; + public $isCascadeRemove; + public $isCascadePersist; public $isCascadeRefresh; public $isCascadeMerge; @@ -184,8 +184,8 @@ abstract class AssociationMapping (bool)$mapping['optional'] : true; $this->cascades = isset($mapping['cascade']) ? (array)$mapping['cascade'] : array(); - $this->isCascadeDelete = in_array('delete', $this->cascades); - $this->isCascadeSave = in_array('save', $this->cascades); + $this->isCascadeRemove = in_array('remove', $this->cascades); + $this->isCascadePersist = in_array('persist', $this->cascades); $this->isCascadeRefresh = in_array('refresh', $this->cascades); $this->isCascadeMerge = in_array('merge', $this->cascades); } @@ -196,9 +196,9 @@ abstract class AssociationMapping * * @return boolean */ - public function isCascadeDelete() + public function isCascadeRemove() { - return $this->isCascadeDelete; + return $this->isCascadeRemove; } /** @@ -207,9 +207,9 @@ abstract class AssociationMapping * * @return boolean */ - public function isCascadeSave() + public function isCascadePersist() { - return $this->isCascadeSave; + return $this->isCascadePersist; } /** @@ -374,7 +374,7 @@ abstract class AssociationMapping */ public function usesJoinTable() { - return (bool)$this->joinTable; + return (bool) $this->joinTable; } /** diff --git a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php index 257eff2eb..ae78c814a 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php +++ b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php @@ -269,11 +269,11 @@ class XmlDriver extends AbstractFileDriver private function _getCascadeMappings($cascadeElement) { $cascades = array(); - if (isset($cascadeElement->{'cascade-save'})) { - $cascades[] = 'save'; + if (isset($cascadeElement->{'cascade-persist'})) { + $cascades[] = 'persist'; } - if (isset($cascadeElement->{'cascade-delete'})) { - $cascades[] = 'delete'; + if (isset($cascadeElement->{'cascade-remove'})) { + $cascades[] = 'remove'; } if (isset($cascadeElement->{'cascade-merge'})) { $cascades[] = 'merge'; diff --git a/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php b/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php index 01e882bf9..9552c2edd 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php +++ b/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php @@ -255,11 +255,11 @@ class YamlDriver extends AbstractFileDriver private function _getCascadeMappings($cascadeElement) { $cascades = array(); - if (isset($cascadeElement['cascadeSave'])) { - $cascades[] = 'save'; + if (isset($cascadeElement['cascadePersist'])) { + $cascades[] = 'persist'; } - if (isset($cascadeElement['cascadeDelete'])) { - $cascades[] = 'delete'; + if (isset($cascadeElement['cascadeRemove'])) { + $cascades[] = 'remove'; } if (isset($cascadeElement['cascadeMerge'])) { $cascades[] = 'merge'; diff --git a/lib/Doctrine/ORM/PersistentCollection.php b/lib/Doctrine/ORM/PersistentCollection.php index caaa9243b..089d609d9 100644 --- a/lib/Doctrine/ORM/PersistentCollection.php +++ b/lib/Doctrine/ORM/PersistentCollection.php @@ -250,11 +250,11 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection // Set back reference to owner if ($this->_association->isOneToMany()) { // OneToMany - $this->_typeClass->getReflectionProperty($this->_backRefFieldName) + $this->_typeClass->reflFields[$this->_backRefFieldName] ->setValue($value, $this->_owner); } else { // ManyToMany - $this->_typeClass->getReflectionProperty($this->_backRefFieldName) + $this->_typeClass->reflFields[$this->_backRefFieldName] ->getValue($value)->add($this->_owner); } } @@ -418,6 +418,16 @@ final class PersistentCollection extends \Doctrine\Common\Collections\Collection { $this->_isDirty = $dirty; } + + /** + * + * @param $bool + * @return unknown_type + */ + public function setInitialized($bool) + { + $this->_initialized = $bool; + } /* Serializable implementation */ diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 892041135..23951f7f1 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -49,16 +49,12 @@ use Doctrine\ORM\EntityManager; class UnitOfWork implements PropertyChangedListener { /** - * An entity is in managed state when it has a primary key/identifier (and - * therefore persistent state) and is managed by an EntityManager - * (registered in the identity map). - * In MANAGED state the entity is associated with an EntityManager that manages - * the persistent state of the Entity. + * An entity is in MANAGED state when its persistence is managed by an EntityManager. */ const STATE_MANAGED = 1; /** - * An entity is new if it has just been instantiated + * An entity is new if it has just been instantiated (i.e. using the "new" operator) * and is not (yet) managed by an EntityManager. */ const STATE_NEW = 2; @@ -74,7 +70,7 @@ class UnitOfWork implements PropertyChangedListener * associated with an EntityManager, whose persistent state has been * deleted (or is scheduled for deletion). */ - const STATE_DELETED = 4; + const STATE_REMOVED = 4; /** * The identity map that holds references to all managed entities that have @@ -87,14 +83,15 @@ class UnitOfWork implements PropertyChangedListener private $_identityMap = array(); /** - * Map of all identifiers. Keys are object ids (spl_object_hash). + * Map of all identifiers of managed entities. + * Keys are object ids (spl_object_hash). * * @var array */ private $_entityIdentifiers = array(); /** - * Map of the original entity data of entities fetched from the database. + * Map of the original entity data of managed entities. * Keys are object ids (spl_object_hash). This is used for calculating changesets * at commit time. * @@ -106,7 +103,7 @@ class UnitOfWork implements PropertyChangedListener private $_originalEntityData = array(); /** - * Map of data changes. Keys are object ids (spl_object_hash). + * Map of entity changes. Keys are object ids (spl_object_hash). * Filled at the beginning of a commit of the UnitOfWork and cleaned at the end. * * @var array @@ -114,7 +111,7 @@ class UnitOfWork implements PropertyChangedListener private $_entityChangeSets = array(); /** - * The states of entities in this UnitOfWork. + * The (cached) states of any known entities. * Keys are object ids (spl_object_hash). * * @var array @@ -123,7 +120,7 @@ class UnitOfWork implements PropertyChangedListener /** * Map of entities that are scheduled for dirty checking at commit time. - * This is only used if automatic dirty checking is disabled. + * This is only used for entities with a change tracking policy of DEFERRED_EXPLICIT. * Keys are object ids (spl_object_hash). * * @var array @@ -145,7 +142,7 @@ class UnitOfWork implements PropertyChangedListener private $_entityUpdates = array(); /** - * Any extra updates that have been scheduled by persisters. + * Any pending extra updates that have been scheduled by persisters. * * @var array */ @@ -173,14 +170,14 @@ class UnitOfWork implements PropertyChangedListener //private $_collectionCreations = array(); /** - * All collection updates. + * All pending collection updates. * * @var array */ private $_collectionUpdates = array(); /** - * List of collections visited during a commit-phase of a UnitOfWork. + * List of collections visited during changeset calculation on a commit-phase of a UnitOfWork. * At the end of the UnitOfWork all these collections will make new snapshots * of their data. * @@ -218,21 +215,21 @@ class UnitOfWork implements PropertyChangedListener private $_collectionPersisters = array(); /** - * Flag for whether or not to use the C extension for hydration + * Flag for whether or not to make use of the C extension. * * @var boolean */ private $_useCExtension = false; /** - * The EventManager. + * The EventManager used for dispatching events. * * @var EventManager */ private $_evm; /** - * Orphaned entities scheduled for removal. + * Orphaned entities that are scheduled for removal. * * @var array */ @@ -253,7 +250,8 @@ class UnitOfWork implements PropertyChangedListener /** * Commits the UnitOfWork, executing all operations that have been postponed - * up to this point. + * up to this point. The state of all managed entities will be synchronized with + * the database. */ public function commit() { @@ -552,8 +550,8 @@ class UnitOfWork implements PropertyChangedListener $this->_visitedCollections[] = $value; } - if ( ! $assoc->isCascadeSave) { - return; // "Persistence by reachability" only if save cascade specified + if ( ! $assoc->isCascadePersist) { + return; // "Persistence by reachability" only if persist cascade specified } // Look through the entities, and in any of their associations, for transient @@ -563,7 +561,7 @@ class UnitOfWork implements PropertyChangedListener } $targetClass = $this->_em->getClassMetadata($assoc->targetEntityName); foreach ($value as $entry) { - $state = $this->getEntityState($entry); + $state = $this->getEntityState($entry, self::STATE_NEW); $oid = spl_object_hash($entry); if ($state == self::STATE_NEW) { // Get identifier, if possible (not post-insert) @@ -596,8 +594,8 @@ class UnitOfWork implements PropertyChangedListener $this->_entityInsertions[$oid] = $entry; $this->_entityChangeSets[$oid] = $changeSet; $this->_originalEntityData[$oid] = $data; - } else if ($state == self::STATE_DELETED) { - throw DoctrineException::updateMe("Deleted entity in collection detected during flush." + } else if ($state == self::STATE_REMOVED) { + throw DoctrineException::updateMe("Removed entity in collection detected during flush." . " Make sure you properly remove deleted entities from collections."); } // MANAGED associated entities are already taken into account @@ -606,7 +604,7 @@ class UnitOfWork implements PropertyChangedListener } /** - * EXPERIMENTAL: + * INTERNAL, EXPERIMENTAL: * Computes the changeset of an individual entity, independently of the * computeChangeSets() routine that is used at the beginning of a UnitOfWork#commit(). * @@ -893,6 +891,7 @@ class UnitOfWork implements PropertyChangedListener } /** + * INTERNAL: * Schedules an extra update that will be executed immediately after the * regular entity updates. * @@ -957,21 +956,6 @@ class UnitOfWork implements PropertyChangedListener return isset($this->_entityDeletions[spl_object_hash($entity)]); } - /** - * Detaches an entity from the persistence management. It's persistence will - * no longer be managed by Doctrine. - * - * @param object $entity The entity to detach. - */ - public function detach($entity) - { - $oid = spl_object_hash($entity); - $this->removeFromIdentityMap($entity); - unset($this->_entityInsertions[$oid], $this->_entityUpdates[$oid], - $this->_entityDeletions[$oid], $this->_entityIdentifiers[$oid], - $this->_entityStates[$oid]); - } - /** * Checks whether an entity is scheduled for insertion, update or deletion. * @@ -987,6 +971,7 @@ class UnitOfWork implements PropertyChangedListener } /** + * INTERNAL: * Registers an entity in the identity map. * Note that entities in a hierarchy are registered with the class name of * the root entity. @@ -1016,31 +1001,41 @@ class UnitOfWork implements PropertyChangedListener /** * Gets the state of an entity within the current unit of work. + * + * NOTE: This method sees entities that are not MANAGED or REMOVED and have a + * populated identifier, whether it is generated or manually assigned, as + * DETACHED. This can be incorrect for manually assigned identifiers. * * @param object $entity + * @param integer $assume The state to assume if the state is not yet known. This is usually + * used to avoid costly state lookups, in the worst case with a database + * lookup. * @return int The entity state. */ - public function getEntityState($entity) + public function getEntityState($entity, $assume = null) { $oid = spl_object_hash($entity); if ( ! isset($this->_entityStates[$oid])) { - /*if (isset($this->_entityInsertions[$oid])) { - $this->_entityStates[$oid] = self::STATE_NEW; - } else if ( ! isset($this->_entityIdentifiers[$oid])) { - // Either NEW (if no ID) or DETACHED (if ID) - } else { - $this->_entityStates[$oid] = self::STATE_DETACHED; - }*/ - if (isset($this->_entityIdentifiers[$oid]) && ! isset($this->_entityInsertions[$oid])) { - $this->_entityStates[$oid] = self::STATE_DETACHED; + // State can only be NEW or DETACHED, because MANAGED/REMOVED states are immediately + // set by the UnitOfWork directly. We treat all entities that have a populated + // identifier as DETACHED and all others as NEW. This is not really correct for + // manually assigned identifiers but in that case we would need to hit the database + // and we would like to avoid that. + if ($assume === null) { + if ($this->_em->getClassMetadata(get_class($entity))->getIdentifierValues($entity)) { + $this->_entityStates[$oid] = self::STATE_DETACHED; + } else { + $this->_entityStates[$oid] = self::STATE_NEW; + } } else { - $this->_entityStates[$oid] = self::STATE_NEW; + $this->_entityStates[$oid] = $assume; } } return $this->_entityStates[$oid]; } /** + * INTERNAL: * Removes an entity from the identity map. This effectively detaches the * entity from the persistence management of Doctrine. * @@ -1067,6 +1062,7 @@ class UnitOfWork implements PropertyChangedListener } /** + * INTERNAL: * Gets an entity in the identity map by its identifier hash. * * @param string $idHash @@ -1079,6 +1075,7 @@ class UnitOfWork implements PropertyChangedListener } /** + * INTERNAL: * Tries to get an entity by its identifier hash. If no entity is found for * the given hash, FALSE is returned. * @@ -1116,6 +1113,7 @@ class UnitOfWork implements PropertyChangedListener } /** + * INTERNAL: * Checks whether an identifier hash exists in the identity map. * * @param string $idHash @@ -1128,9 +1126,9 @@ class UnitOfWork implements PropertyChangedListener } /** - * Saves an entity as part of the current unit of work. + * Persists an entity as part of the current unit of work. * - * @param object $entity The entity to save. + * @param object $entity The entity to persist. */ public function persist($entity) { @@ -1142,11 +1140,11 @@ class UnitOfWork implements PropertyChangedListener * Saves an entity as part of the current unit of work. * This method is internally called during save() cascades as it tracks * the already visited entities to prevent infinite recursions. + * + * NOTE: This method always considers entities with a manually assigned identifier as NEW. * - * @param object $entity The entity to save. + * @param object $entity The entity to persist. * @param array $visited The already visited entities. - * @param array $insertNow The entities that must be immediately inserted because of - * post-insert ID generation. */ private function _doPersist($entity, array &$visited) { @@ -1157,8 +1155,8 @@ class UnitOfWork implements PropertyChangedListener $visited[$oid] = $entity; // Mark visited - $class = $this->_em->getClassMetadata(get_class($entity)); - switch ($this->getEntityState($entity)) { + $class = $this->_em->getClassMetadata(get_class($entity)); + switch ($this->getEntityState($entity, self::STATE_NEW)) { case self::STATE_MANAGED: // Nothing to do, except if policy is "deferred explicit" if ($class->isChangeTrackingDeferredExplicit()) { @@ -1190,7 +1188,7 @@ class UnitOfWork implements PropertyChangedListener case self::STATE_DETACHED: throw DoctrineException::updateMe("Behavior of save() for a detached entity " . "is not yet defined."); - case self::STATE_DELETED: + case self::STATE_REMOVED: // Entity becomes managed again if ($this->isScheduledForDelete($entity)) { unset($this->_entityDeletions[$oid]); @@ -1202,13 +1200,14 @@ class UnitOfWork implements PropertyChangedListener default: throw DoctrineException::updateMe("Encountered invalid entity state."); } + $this->_cascadePersist($entity, $visited); } /** * Deletes an entity as part of the current unit of work. * - * @param object $entity + * @param object $entity The entity to remove. */ public function remove($entity) { @@ -1224,6 +1223,7 @@ class UnitOfWork implements PropertyChangedListener * * @param object $entity The entity to delete. * @param array $visited The map of the already visited entities. + * @throws InvalidArgumentException If the instance is a detached entity. */ private function _doRemove($entity, array &$visited) { @@ -1237,7 +1237,7 @@ class UnitOfWork implements PropertyChangedListener $class = $this->_em->getClassMetadata(get_class($entity)); switch ($this->getEntityState($entity)) { case self::STATE_NEW: - case self::STATE_DELETED: + case self::STATE_REMOVED: // nothing to do break; case self::STATE_MANAGED: @@ -1250,10 +1250,11 @@ class UnitOfWork implements PropertyChangedListener $this->scheduleForDelete($entity); break; case self::STATE_DETACHED: - throw DoctrineException::updateMe("A detached entity can't be deleted."); + throw new \InvalidArgumentException("A detached entity can not be removed."); default: throw DoctrineException::updateMe("Encountered invalid entity state."); } + $this->_cascadeRemove($entity, $visited); } @@ -1275,6 +1276,9 @@ class UnitOfWork implements PropertyChangedListener * @param object $entity * @param array $visited * @return object The managed copy of the entity. + * @throws OptimisticLockException If the entity uses optimistic locking through a version + * attribute and the version check against the managed copy fails. + * @throws InvalidArgumentException If the entity instance is NEW. */ private function _doMerge($entity, array &$visited, $prevManagedCopy = null, $assoc = null) { @@ -1282,39 +1286,70 @@ class UnitOfWork implements PropertyChangedListener $id = $class->getIdentifierValues($entity); if ( ! $id) { - throw new \InvalidArgumentException('New entity passed to merge().'); - } - - $managedCopy = $this->tryGetById($id, $class->rootEntityName); - if ($managedCopy) { - if ($this->getEntityState($managedCopy) == self::STATE_DELETED) { - throw new InvalidArgumentException('Can not merge with a deleted entity.'); - } - } else { - $managedCopy = $this->_em->find($class->name, $id); + throw new \InvalidArgumentException('New entity detected during merge.' + . ' Persist the new entity before merging.'); } - if ($class->isVersioned) { - $managedCopyVersion = $class->reflFields[$class->versionField]->getValue($managedCopy); - $entityVersion = $class->reflFields[$class->versionField]->getValue($entity); - // Throw exception if versions dont match. - if ($managedCopyVersion != $entity) { - throw OptimisticLockException::versionMismatch(); + // 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); } - } - - // Merge state of $entity into existing (managed) entity - foreach ($class->reflFields as $name => $prop) { - if ( ! isset($class->associationMappings[$name])) { - $prop->setValue($managedCopy, $prop->getValue($entity)); + + if ($managedCopy === null) { + throw new \InvalidArgumentException('New entity detected during merge.' + . ' Persist the new entity before merging.'); } - if ($class->isChangeTrackingNotify()) { + + if ($class->isVersioned) { + $managedCopyVersion = $class->reflFields[$class->versionField]->getValue($managedCopy); + $entityVersion = $class->reflFields[$class->versionField]->getValue($entity); + // Throw exception if versions dont match. + if ($managedCopyVersion != $entity) { + throw OptimisticLockException::versionMismatch(); + } + } + + // Merge state of $entity into existing (managed) entity + foreach ($class->reflFields as $name => $prop) { + if ( ! isset($class->associationMappings[$name])) { + $prop->setValue($managedCopy, $prop->getValue($entity)); + } else { + $assoc2 = $class->associationMappings[$name]; + if ($assoc2->isOneToOne() && ! $assoc2->isCascadeMerge) { + //TODO: Only do this when allowPartialObjects == false? + $targetClass = $this->_em->getClassMetadata($assoc2->targetEntityName); + $prop->setValue($managedCopy, $this->_em->getProxyFactory() + ->getReferenceProxy($assoc2->targetEntityName, $targetClass->getIdentifierValues($entity))); + } else { + //TODO: Only do this when allowPartialObjects == false? + $coll = new PersistentCollection($this->_em, + $this->_em->getClassMetadata($assoc2->targetEntityName) + ); + $coll->setOwner($managedCopy, $assoc2); + $coll->setInitialized($assoc2->isCascadeMerge); + $prop->setValue($managedCopy, $coll); + } + } + if ($class->isChangeTrackingNotify()) { + //TODO + } + } + if ($class->isChangeTrackingDeferredExplicit()) { //TODO } } - if ($class->isChangeTrackingDeferredExplicit()) { - //TODO - } if ($prevManagedCopy !== null) { $assocField = $assoc->sourceFieldName; @@ -1322,7 +1357,7 @@ class UnitOfWork implements PropertyChangedListener if ($assoc->isOneToOne()) { $prevClass->reflFields[$assocField]->setValue($prevManagedCopy, $managedCopy); } else { - $prevClass->reflFields[$assocField]->getValue($prevManagedCopy)->add($managedCopy); + $prevClass->reflFields[$assocField]->getValue($prevManagedCopy)->hydrateAdd($managedCopy); } } @@ -1331,11 +1366,55 @@ class UnitOfWork implements PropertyChangedListener return $managedCopy; } + /** + * Detaches an entity from the persistence management. It's persistence will + * no longer be managed by Doctrine. + * + * @param object $entity The entity to detach. + */ + public function detach($entity) + { + $visited = array(); + $this->_doDetach($entity, $visited); + } + + /** + * Executes a detach operation on the given entity. + * + * @param object $entity + * @param array $visited + * @internal This method always considers entities with an assigned identifier as DETACHED. + */ + private function _doDetach($entity, array &$visited) + { + $oid = spl_object_hash($entity); + if (isset($visited[$oid])) { + return; // Prevent infinite recursion + } + + $visited[$oid] = $entity; // mark visited + + switch ($this->getEntityState($entity, self::STATE_DETACHED)) { + case self::STATE_MANAGED: + $this->removeFromIdentityMap($entity); + unset($this->_entityInsertions[$oid], $this->_entityUpdates[$oid], + $this->_entityDeletions[$oid], $this->_entityIdentifiers[$oid], + $this->_entityStates[$oid]); + break; + case self::STATE_NEW: + case self::STATE_DETACHED: + return; + } + + $this->_cascadeDetach($entity, $visited); + } + /** * Refreshes the state of the given entity from the database, overwriting * any local, unpersisted changes. * * @param object $entity The entity to refresh. + * @throws InvalidArgumentException If the entity is not MANAGED. */ public function refresh($entity) { @@ -1348,6 +1427,7 @@ class UnitOfWork implements PropertyChangedListener * * @param object $entity The entity to refresh. * @param array $visited The already visited entities during cascades. + * @throws InvalidArgumentException If the entity is not MANAGED. */ private function _doRefresh($entity, array &$visited) { @@ -1367,8 +1447,9 @@ class UnitOfWork implements PropertyChangedListener ); break; default: - throw DoctrineException::updateMe("NEW, REMOVED or DETACHED entity can not be refreshed."); + throw new \InvalidArgumentException("Entity is not MANAGED."); } + $this->_cascadeRefresh($entity, $visited); } @@ -1395,6 +1476,30 @@ class UnitOfWork implements PropertyChangedListener } } } + + /** + * Cascades a detach operation to associated entities. + * + * @param object $entity + * @param array $visited + */ + private function _cascadeDetach($entity, array &$visited) + { + $class = $this->_em->getClassMetadata(get_class($entity)); + foreach ($class->associationMappings as $assocMapping) { + if ( ! $assocMapping->isCascadeDetach) { + continue; + } + $relatedEntities = $class->reflFields[$assocMapping->sourceFieldName]->getValue($entity); + if ($relatedEntities instanceof Collection) { + foreach ($relatedEntities as $relatedEntity) { + $this->_doDetach($relatedEntity, $visited); + } + } else if ($relatedEntities !== null) { + $this->_doDetach($relatedEntities, $visited); + } + } + } /** * Cascades a merge operation to associated entities. @@ -1410,8 +1515,7 @@ class UnitOfWork implements PropertyChangedListener if ( ! $assocMapping->isCascadeMerge) { continue; } - $relatedEntities = $class->reflFields[$assocMapping->sourceFieldName] - ->getValue($entity); + $relatedEntities = $class->reflFields[$assocMapping->sourceFieldName]->getValue($entity); if ($relatedEntities instanceof Collection) { foreach ($relatedEntities as $relatedEntity) { $this->_doMerge($relatedEntity, $visited, $managedCopy, $assocMapping); @@ -1433,7 +1537,7 @@ class UnitOfWork implements PropertyChangedListener { $class = $this->_em->getClassMetadata(get_class($entity)); foreach ($class->associationMappings as $assocMapping) { - if ( ! $assocMapping->isCascadeSave) { + if ( ! $assocMapping->isCascadePersist) { continue; } $relatedEntities = $class->reflFields[$assocMapping->sourceFieldName]->getValue($entity); @@ -1457,7 +1561,7 @@ class UnitOfWork implements PropertyChangedListener { $class = $this->_em->getClassMetadata(get_class($entity)); foreach ($class->associationMappings as $assocMapping) { - if ( ! $assocMapping->isCascadeDelete) { + if ( ! $assocMapping->isCascadeRemove) { continue; } $relatedEntities = $class->reflFields[$assocMapping->sourceFieldName] @@ -1824,25 +1928,23 @@ class UnitOfWork implements PropertyChangedListener */ public function propertyChanged($entity, $propertyName, $oldValue, $newValue) { - if ($this->getEntityState($entity) == self::STATE_MANAGED) { - $oid = spl_object_hash($entity); - $class = $this->_em->getClassMetadata(get_class($entity)); + $oid = spl_object_hash($entity); + $class = $this->_em->getClassMetadata(get_class($entity)); - $this->_entityChangeSets[$oid][$propertyName] = array($oldValue, $newValue); + $this->_entityChangeSets[$oid][$propertyName] = array($oldValue, $newValue); - if (isset($class->associationMappings[$propertyName])) { - $assoc = $class->associationMappings[$propertyName]; - if ($assoc->isOneToOne() && $assoc->isOwningSide) { - $this->_entityUpdates[$oid] = $entity; - } else if ($oldValue instanceof PersistentCollection) { - // A PersistentCollection was de-referenced, so delete it. - if ( ! in_array($oldValue, $this->_collectionDeletions, true)) { - $this->_collectionDeletions[] = $oldValue; - } - } - } else { + if (isset($class->associationMappings[$propertyName])) { + $assoc = $class->associationMappings[$propertyName]; + if ($assoc->isOneToOne() && $assoc->isOwningSide) { $this->_entityUpdates[$oid] = $entity; + } else if ($oldValue instanceof PersistentCollection) { + // A PersistentCollection was de-referenced, so delete it. + if ( ! in_array($oldValue, $this->_collectionDeletions, true)) { + $this->_collectionDeletions[] = $oldValue; + } } + } else { + $this->_entityUpdates[$oid] = $entity; } } } \ No newline at end of file diff --git a/tests/Doctrine/Tests/Models/CMS/CmsUser.php b/tests/Doctrine/Tests/Models/CMS/CmsUser.php index 6f103740a..9f3f225e5 100644 --- a/tests/Doctrine/Tests/Models/CMS/CmsUser.php +++ b/tests/Doctrine/Tests/Models/CMS/CmsUser.php @@ -26,7 +26,7 @@ class CmsUser */ public $name; /** - * @OneToMany(targetEntity="CmsPhonenumber", mappedBy="user", cascade={"save", "delete"}, orphanRemoval=true) + * @OneToMany(targetEntity="CmsPhonenumber", mappedBy="user", cascade={"persist", "remove", "merge"}, orphanRemoval=true) */ public $phonenumbers; /** @@ -34,11 +34,11 @@ class CmsUser */ public $articles; /** - * @OneToOne(targetEntity="CmsAddress", mappedBy="user", cascade={"save"}) + * @OneToOne(targetEntity="CmsAddress", mappedBy="user", cascade={"persist"}) */ public $address; /** - * @ManyToMany(targetEntity="CmsGroup", cascade={"save"}) + * @ManyToMany(targetEntity="CmsGroup", cascade={"persist"}) * @JoinTable(name="cms_users_groups", joinColumns={@JoinColumn(name="user_id", referencedColumnName="id")}, inverseJoinColumns={@JoinColumn(name="group_id", referencedColumnName="id")}) diff --git a/tests/Doctrine/Tests/Models/ECommerce/ECommerceCart.php b/tests/Doctrine/Tests/Models/ECommerce/ECommerceCart.php index f1471fa30..7c240e8e2 100644 --- a/tests/Doctrine/Tests/Models/ECommerce/ECommerceCart.php +++ b/tests/Doctrine/Tests/Models/ECommerce/ECommerceCart.php @@ -33,7 +33,7 @@ class ECommerceCart private $customer; /** - * @ManyToMany(targetEntity="ECommerceProduct", cascade={"save"}) + * @ManyToMany(targetEntity="ECommerceProduct", cascade={"persist"}) * @JoinTable(name="ecommerce_carts_products", joinColumns={@JoinColumn(name="cart_id", referencedColumnName="id")}, inverseJoinColumns={@JoinColumn(name="product_id", referencedColumnName="id")}) diff --git a/tests/Doctrine/Tests/Models/ECommerce/ECommerceCategory.php b/tests/Doctrine/Tests/Models/ECommerce/ECommerceCategory.php index 70ae1ffd3..5000c051f 100644 --- a/tests/Doctrine/Tests/Models/ECommerce/ECommerceCategory.php +++ b/tests/Doctrine/Tests/Models/ECommerce/ECommerceCategory.php @@ -32,7 +32,7 @@ class ECommerceCategory private $products; /** - * @OneToMany(targetEntity="ECommerceCategory", mappedBy="parent", cascade={"save"}) + * @OneToMany(targetEntity="ECommerceCategory", mappedBy="parent", cascade={"persist"}) */ private $children; diff --git a/tests/Doctrine/Tests/Models/ECommerce/ECommerceCustomer.php b/tests/Doctrine/Tests/Models/ECommerce/ECommerceCustomer.php index 20b43e63e..fff4c9b82 100644 --- a/tests/Doctrine/Tests/Models/ECommerce/ECommerceCustomer.php +++ b/tests/Doctrine/Tests/Models/ECommerce/ECommerceCustomer.php @@ -25,7 +25,7 @@ class ECommerceCustomer private $name; /** - * @OneToOne(targetEntity="ECommerceCart", mappedBy="customer", cascade={"save"}) + * @OneToOne(targetEntity="ECommerceCart", mappedBy="customer", cascade={"persist"}) */ private $cart; @@ -34,7 +34,7 @@ class ECommerceCustomer * only one customer at the time, while a customer can choose only one * mentor. Not properly appropriate but it works. * - * @OneToOne(targetEntity="ECommerceCustomer", cascade={"save"}) + * @OneToOne(targetEntity="ECommerceCustomer", cascade={"persist"}) * @JoinColumn(name="mentor_id", referencedColumnName="id") */ private $mentor; diff --git a/tests/Doctrine/Tests/Models/ECommerce/ECommerceProduct.php b/tests/Doctrine/Tests/Models/ECommerce/ECommerceProduct.php index e820eaf4c..e3abb5c03 100644 --- a/tests/Doctrine/Tests/Models/ECommerce/ECommerceProduct.php +++ b/tests/Doctrine/Tests/Models/ECommerce/ECommerceProduct.php @@ -27,18 +27,18 @@ class ECommerceProduct private $name; /** - * @OneToOne(targetEntity="ECommerceShipping", cascade={"save"}) + * @OneToOne(targetEntity="ECommerceShipping", cascade={"persist"}) * @JoinColumn(name="shipping_id", referencedColumnName="id") */ private $shipping; /** - * @OneToMany(targetEntity="ECommerceFeature", mappedBy="product", cascade={"save"}) + * @OneToMany(targetEntity="ECommerceFeature", mappedBy="product", cascade={"persist"}) */ private $features; /** - * @ManyToMany(targetEntity="ECommerceCategory", cascade={"save"}) + * @ManyToMany(targetEntity="ECommerceCategory", cascade={"persist"}) * @JoinTable(name="ecommerce_products_categories", joinColumns={@JoinColumn(name="product_id", referencedColumnName="id")}, inverseJoinColumns={@JoinColumn(name="category_id", referencedColumnName="id")}) @@ -48,7 +48,7 @@ class ECommerceProduct /** * This relation is saved with two records in the association table for * simplicity. - * @ManyToMany(targetEntity="ECommerceProduct", cascade={"save"}) + * @ManyToMany(targetEntity="ECommerceProduct", cascade={"persist"}) * @JoinTable(name="ecommerce_products_related", joinColumns={@JoinColumn(name="product_id", referencedColumnName="id")}, inverseJoinColumns={@JoinColumn(name="related_id", referencedColumnName="id")}) diff --git a/tests/Doctrine/Tests/Models/Forum/ForumUser.php b/tests/Doctrine/Tests/Models/Forum/ForumUser.php index 1a1e8b087..fedf10955 100644 --- a/tests/Doctrine/Tests/Models/Forum/ForumUser.php +++ b/tests/Doctrine/Tests/Models/Forum/ForumUser.php @@ -19,7 +19,7 @@ class ForumUser */ public $username; /** - * @OneToOne(targetEntity="ForumAvatar", cascade={"save"}) + * @OneToOne(targetEntity="ForumAvatar", cascade={"persist"}) * @JoinColumn(name="avatar_id", referencedColumnName="id") */ public $avatar; diff --git a/tests/Doctrine/Tests/ORM/Functional/DetachedEntityTest.php b/tests/Doctrine/Tests/ORM/Functional/DetachedEntityTest.php index 574329fb3..f5186989a 100644 --- a/tests/Doctrine/Tests/ORM/Functional/DetachedEntityTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/DetachedEntityTest.php @@ -3,6 +3,7 @@ namespace Doctrine\Tests\ORM\Functional; use Doctrine\Tests\Models\CMS\CmsUser; +use Doctrine\Tests\Models\CMS\CmsPhonenumber; use Doctrine\ORM\UnitOfWork; require_once __DIR__ . '/../../TestInit.php'; @@ -42,5 +43,48 @@ class DetachedEntityTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertTrue($this->_em->contains($user2)); $this->assertEquals('Roman B.', $user2->name); } + + public function testSerializeUnserializeModifyMerge() + { + $user = new CmsUser; + $user->name = 'Guilherme'; + $user->username = 'gblanco'; + $user->status = 'developer'; + + $ph1 = new CmsPhonenumber; + $ph1->phonenumber = 1234; + $user->addPhonenumber($ph1); + + $this->_em->persist($user); + $this->_em->flush(); + $this->assertTrue($this->_em->contains($user)); + + $serialized = serialize($user); + $this->_em->clear(); + $this->assertFalse($this->_em->contains($user)); + unset($user); + + $user = unserialize($serialized); + + $ph2 = new CmsPhonenumber; + $ph2->phonenumber = 56789; + $user->addPhonenumber($ph2); + $this->assertEquals(2, count($user->getPhonenumbers())); + $this->assertFalse($this->_em->contains($user)); + + $this->_em->persist($ph2); + + //$removed = $user->removePhonenumber(1); // [romanb] this is currently broken, I'm on it. + + // Merge back in + $user = $this->_em->merge($user); // merge cascaded to phonenumbers + $this->_em->flush(); + + $this->assertTrue($this->_em->contains($user)); + $this->assertEquals(2, count($user->getPhonenumbers())); + $phonenumbers = $user->getPhonenumbers(); + $this->assertTrue($this->_em->contains($phonenumbers[0])); + $this->assertTrue($this->_em->contains($phonenumbers[1])); + } } diff --git a/tests/Doctrine/Tests/ORM/Mapping/XmlDriverTest.php b/tests/Doctrine/Tests/ORM/Mapping/XmlDriverTest.php index bbebce3cb..63c1d7498 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/XmlDriverTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/XmlDriverTest.php @@ -41,7 +41,7 @@ class XmlDriverTest extends \Doctrine\Tests\OrmTestCase $this->assertTrue(isset($class->associationMappings['phonenumbers'])); $this->assertFalse($class->associationMappings['phonenumbers']->isOwningSide); $this->assertTrue($class->associationMappings['phonenumbers']->isInverseSide()); - $this->assertTrue($class->associationMappings['phonenumbers']->isCascadeSave); + $this->assertTrue($class->associationMappings['phonenumbers']->isCascadePersist); $this->assertTrue($class->associationMappings['groups'] instanceof \Doctrine\ORM\Mapping\ManyToManyMapping); $this->assertTrue(isset($class->associationMappings['groups'])); diff --git a/tests/Doctrine/Tests/ORM/Mapping/YamlDriverTest.php b/tests/Doctrine/Tests/ORM/Mapping/YamlDriverTest.php index 061fd5db8..92bb15420 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/YamlDriverTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/YamlDriverTest.php @@ -41,7 +41,7 @@ class YamlDriverTest extends \Doctrine\Tests\OrmTestCase $this->assertTrue(isset($class->associationMappings['phonenumbers'])); $this->assertFalse($class->associationMappings['phonenumbers']->isOwningSide); $this->assertTrue($class->associationMappings['phonenumbers']->isInverseSide()); - $this->assertTrue($class->associationMappings['phonenumbers']->isCascadeSave); + $this->assertTrue($class->associationMappings['phonenumbers']->isCascadePersist); $this->assertTrue($class->associationMappings['groups'] instanceof \Doctrine\ORM\Mapping\ManyToManyMapping); $this->assertTrue(isset($class->associationMappings['groups'])); diff --git a/tests/Doctrine/Tests/ORM/Mapping/xml/XmlMappingTest.User.dcm.xml b/tests/Doctrine/Tests/ORM/Mapping/xml/XmlMappingTest.User.dcm.xml index 0945821a5..55337c2ca 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/xml/XmlMappingTest.User.dcm.xml +++ b/tests/Doctrine/Tests/ORM/Mapping/xml/XmlMappingTest.User.dcm.xml @@ -19,7 +19,7 @@ - + diff --git a/tests/Doctrine/Tests/ORM/Mapping/yaml/YamlMappingTest.User.dcm.yml b/tests/Doctrine/Tests/ORM/Mapping/yaml/YamlMappingTest.User.dcm.yml index 6de3a2f92..738a5e016 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/yaml/YamlMappingTest.User.dcm.yml +++ b/tests/Doctrine/Tests/ORM/Mapping/yaml/YamlMappingTest.User.dcm.yml @@ -20,7 +20,7 @@ YamlMappingTest\User: phonenumbers: targetEntity: Phonenumber mappedBy: user - cascade: cascadeSave + cascade: cascadePersist manyToMany: groups: targetEntity: Group diff --git a/tests/Doctrine/Tests/ORM/Performance/InsertPerformanceTest.php b/tests/Doctrine/Tests/ORM/Performance/InsertPerformanceTest.php index 30bba855b..2189b4620 100644 --- a/tests/Doctrine/Tests/ORM/Performance/InsertPerformanceTest.php +++ b/tests/Doctrine/Tests/ORM/Performance/InsertPerformanceTest.php @@ -32,7 +32,7 @@ class InsertPerformanceTest extends \Doctrine\Tests\OrmPerformanceTestCase //$mem = memory_get_usage(); //echo "Memory usage before: " . ($mem / 1024) . " KB" . PHP_EOL; $batchSize = 20; - for ($i=0; $i<10000; ++$i) { + for ($i=1; $i<=10000; ++$i) { $user = new CmsUser; $user->status = 'user'; $user->username = 'user' . $i; @@ -43,7 +43,6 @@ class InsertPerformanceTest extends \Doctrine\Tests\OrmPerformanceTestCase $this->_em->clear(); } } - //$memAfter = memory_get_usage(); //echo "Memory usage after: " . ($memAfter / 1024) . " KB" . PHP_EOL;