[2.0] Refined implementation and semantics of the merge and detach operations....

[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

@@ -25,9 +25,9 @@
   <xs:complexType name="cascade-type">
     <xs:sequence>
       <xs:element name="cascade-all" type="orm:emptyType" minOccurs="0"/> 
-      <xs:element name="cascade-save" type="orm:emptyType" minOccurs="0"/> 
+      <xs:element name="cascade-persist" type="orm:emptyType" minOccurs="0"/> 
       <xs:element name="cascade-merge" type="orm:emptyType" minOccurs="0"/> 
-      <xs:element name="cascade-delete" type="orm:emptyType" minOccurs="0"/> 
+      <xs:element name="cascade-remove" type="orm:emptyType" minOccurs="0"/> 
       <xs:element name="cascade-refresh" type="orm:emptyType" minOccurs="0"/> 
     </xs:sequence>
   </xs:complexType>

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)

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;
     }
 
     /**

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';

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';

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);
             }
         }
@@ -419,6 +419,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 */
 
     /**

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.
@@ -1017,30 +1002,40 @@ 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])) {
+            // 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] = $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)
     {
@@ -1143,10 +1141,10 @@ class UnitOfWork implements PropertyChangedListener
      * This method is internally called during save() cascades as it tracks
      * the already visited entities to prevent infinite recursions.
      * 
-     * @param object $entity The entity to save.
+     * NOTE: This method always considers entities with a manually assigned identifier as NEW.
+     *
+     * @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)
     {
@@ -1158,7 +1156,7 @@ class UnitOfWork implements PropertyChangedListener
         $visited[$oid] = $entity; // Mark visited
 
         $class = $this->_em->getClassMetadata(get_class($entity));        
-        switch ($this->getEntityState($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,18 +1286,32 @@ class UnitOfWork implements PropertyChangedListener
         $id = $class->getIdentifierValues($entity);
 
         if ( ! $id) {
-            throw new \InvalidArgumentException('New entity passed to merge().');
+            throw new \InvalidArgumentException('New entity detected during merge.'
+                    . ' Persist the new entity before merging.');
         }
         
+        // 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) {
-            if ($this->getEntityState($managedCopy) == self::STATE_DELETED) {
-                throw new InvalidArgumentException('Can not merge with a deleted entity.');
+                // We have the entity in-memory already, just make sure its not removed.
+                if ($this->getEntityState($managedCopy) == self::STATE_REMOVED) {
+                    throw new \InvalidArgumentException('Removed entity detected during merge.'
+                            . ' Can not merge with a removed entity.');
                 }
             } else {
+                // We need to fetch the managed copy in order to merge.
                 $managedCopy = $this->_em->find($class->name, $id);
             }
             
+            if ($managedCopy === null) {
+                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);
@@ -1307,6 +1325,22 @@ class UnitOfWork implements PropertyChangedListener
             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
@@ -1315,6 +1349,7 @@ class UnitOfWork implements PropertyChangedListener
             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);
     }
     
@@ -1396,6 +1477,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,7 +1928,6 @@ 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));
 
@@ -1844,5 +1947,4 @@ class UnitOfWork implements PropertyChangedListener
             $this->_entityUpdates[$oid] = $entity;
         }
     }
-    }
 }
\ No newline at end of file

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")})

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")})

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;
 

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;

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")})

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;

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]));
+    }
 }
 

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']));

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']));

tests/Doctrine/Tests/ORM/Mapping/xml/XmlMappingTest.User.dcm.xml

@@ -19,7 +19,7 @@
         
         <one-to-many field="phonenumbers" targetEntity="Phonenumber" mappedBy="user">
             <cascade>
-                <cascade-save/>
+                <cascade-persist/>
             </cascade>
         </one-to-many>
         

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

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;