[MODM-48] Fixed issue with inserting and updating documents with empt…

…y instances of embedded documents.
doctrine · Aug 18, 2010 · c57cc9f · c57cc9f
1 parent 6e7610a
commit c57cc9f
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 32 deletions.
diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/BasicDocumentPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/BasicDocumentPersister.php
@@ -246,6 +246,8 @@ public function update($document, array $options = array())
                     $this->collection->update(array('_id' => $id), $tempUpdate, $options);
                 }
             }
+
+            //print_r($update);
             $this->collection->update(array('_id' => $id), $update, $options);
         }
     }
@@ -400,27 +402,6 @@ public function prepareInsertData($document)
                     $this->fieldsToUpdate[$id][$mapping['fieldName']] = array($mapping, $new);
                 }
             }
-            if (isset($mapping['embedded'])) {
-                $scheduleForUpdate = false;
-                if ($mapping['type'] === 'one') {
-                    if ( ! isset($insertData[$mapping['fieldName']]['_id'])) {
-                        $scheduleForUpdate = true;
-                    }
-                } elseif ($mapping['type'] === 'many') {
-                    foreach ($insertData[$mapping['fieldName']] as $ref) {
-                        if ( ! isset($ref['_id'])) {
-                            $scheduleForUpdate = true;
-                            break;
-                        }
-                    }
-                }
-                if ($scheduleForUpdate) {
-                    unset($insertData[$mapping['fieldName']]);
-                    $id = spl_object_hash($document);
-                    $this->documentsToUpdate[$id] = $document;
-                    $this->fieldsToUpdate[$id][$mapping['fieldName']] = array($mapping, $new);
-                }
-            }
         }
         // add discriminator if the class has one
         if ($this->class->hasDiscriminator()) {
@@ -495,14 +476,25 @@ public function prepareUpdateData($document)
                             $result[$this->cmd . 'inc'][$mapping['fieldName']] = ($old - $new) * -1;
                         }
                     } else {
+                        // Single embedded
                         if (isset($mapping['embedded']) && $mapping['type'] === 'one') {
-                            $embeddedDocument = $class->getFieldValue($document, $mapping['fieldName']);
-                            $update = $this->prepareUpdateData($embeddedDocument);
-                            foreach ($update as $cmd => $values) {
-                                foreach ($values as $key => $value) {
-                                    $result[$cmd][$mapping['fieldName'] . '.' . $key] = $value;
+                            // If we didn't have a value before and now we do
+                            if ( ! $old && $new) {
+                                $new = $this->prepareValue($mapping, $new);
+                                if (isset($new) || $mapping['nullable'] === true) {
+                                    $result[$this->cmd . 'set'][$mapping['fieldName']] = $new;
+                                }
+                            // If we had an old value before and it has changed
+                            } elseif ($old && $new) {
+                                $embeddedDocument = $class->getFieldValue($document, $mapping['fieldName']);
+                                $update = $this->prepareUpdateData($embeddedDocument);
+                                foreach ($update as $cmd => $values) {
+                                    foreach ($values as $key => $value) {
+                                        $result[$cmd][$mapping['fieldName'] . '.' . $key] = $value;
+                                    }
                                 }
                             }
+                        // $set all other fields
                         } else {
                             $new = $this->prepareValue($mapping, $new);
                             if (isset($new) || $mapping['nullable'] === true) {

diff --git a/lib/Doctrine/ODM/MongoDB/UnitOfWork.php b/lib/Doctrine/ODM/MongoDB/UnitOfWork.php
@@ -380,8 +380,7 @@ public function computeChangeSet($parentDocument, Mapping\ClassMetadata $class,
                 $coll->setDirty( ! $coll->isEmpty());
                 $class->reflFields[$name]->setValue($document, $coll);
                 $actualData[$name] = $coll;
-            }
-            if ($class->isSingleValuedEmbed($name) && is_object($actualData[$name])) {
+            } elseif ($class->isSingleValuedEmbed($name) && is_object($actualData[$name])) {
                 $embeddedDocument = $actualData[$name];
                 $embeddedMetadata = $this->dm->getClassMetadata(get_class($embeddedDocument));
                 $actualData[$name] = array();
@@ -416,11 +415,18 @@ public function computeChangeSet($parentDocument, Mapping\ClassMetadata $class,
                 if ($actualValue instanceof PersistentCollection) {
                     $actualValue = $actualValue->toArray();
                 }
-                if ((isset($class->fieldMappings[$propName]['embedded']) && $class->fieldMappings[$propName]['type'] === 'one')
-                        || (isset($class->fieldMappings[$propName]['reference']) && $class->fieldMappings[$propName]['type'] === 'one')) {
-                    if ($orgValue !== $actualValue) {
-                        $changeSet[$propName] = array($orgValue, $actualValue);
+                if (isset($class->fieldMappings[$propName]['embedded']) && $class->fieldMappings[$propName]['type'] === 'one' && $orgValue !== $actualValue) {
+                    if (is_object($orgValue)) {
+                        $embeddedOid = spl_object_hash($orgValue);
+                        $orgValue = isset($this->originalDocumentData[$embeddedOid]) ? $this->originalDocumentData[$embeddedOid] : $orgValue;
+                    }
+                    $changeSet[$propName] = array($orgValue, $actualValue);
+                } else if (isset($class->fieldMappings[$propName]['reference']) && $class->fieldMappings[$propName]['type'] === 'one' && $orgValue !== $actualValue) {
+                    if (is_object($orgValue)) {
+                        $referenceOid = spl_object_hash($orgValue);
+                        $orgValue = isset($this->originalDocumentData[$referenceOid]) ? $this->originalDocumentData[$referenceOid] : $orgValue;
                     }
+                    $changeSet[$propName] = array($orgValue, $actualValue);
                 } else if ($isChangeTrackingNotify) {
                     continue;
                 } else if (isset($class->fieldMappings[$propName]['type']) && $class->fieldMappings[$propName]['type'] === 'many') {

diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/MODM48Test.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/MODM48Test.php
@@ -0,0 +1,52 @@
+<?php
+
+namespace Doctrine\ODM\MongoDB\Tests\Functional\Ticket;
+
+require_once __DIR__ . '/../../../../../../TestInit.php';
+
+class MODM48Test extends \Doctrine\ODM\MongoDB\Tests\BaseTest
+{
+    public function testTest()
+    {
+        $a = new MODM48A();
+        $a->b = new MODM48B();
+        $this->dm->persist($a);
+        $this->dm->flush();
+        $this->dm->clear();
+
+        $a = $this->dm->findOne(__NAMESPACE__.'\MODM48A');
+        $this->assertNotNull($a);
+
+        $a->getB()->setVal('test');
+
+        $this->dm->flush(array('safe' => true));
+        $this->dm->clear();
+
+        $a = $this->dm->findOne(__NAMESPACE__.'\MODM48A');
+        $this->assertEquals('test', $a->getB()->getVal());
+    }
+}
+
+/** @Document(db="modm48_tests", collection="a") */
+class MODM48A
+{
+    /** @Id */
+    public $id;
+
+    /** @EmbedOne(targetDocument="MODM48B", cascade="all") */
+    public $b;
+
+    function getId()  {return $this->id;}
+    function getB()   {return $this->b;}
+    function setB($b) {$this->b = $b;}
+}
+
+/** @EmbeddedDocument */
+class MODM48B
+{
+    /** @String */
+    public $val;
+
+    function setVal($val) {$this->val = $val;}
+    function getVal() {return $this->val;}
+}