Skip to content

Commit

Permalink
Refactoring to remove need for originalObject and fixing issue with i…
Browse files Browse the repository at this point in the history 
…ncorrect changesets being computed because of the original document data being incorrect in certain caises.
  • Loading branch information
@jwage
jwage committed Oct 4, 2010
1 parent 07346d6 commit 8bd3aef
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 30 deletions.
8 changes: 5 additions & 3 deletions lib/Doctrine/ODM/MongoDB/Hydrator.php
View file
Expand Up @@ -125,15 +125,17 @@ public function hydrate($document, &$data)
$embeddedMetadata = $this->dm->getClassMetadata($className);
$value = $embeddedMetadata->newInstance();
$this->hydrate($value, $embeddedDocument);
$data[$mapping['name']] = $embeddedDocument;
$this->dm->getUnitOfWork()->registerManaged($value, null, $embeddedDocument);
} elseif ($mapping['type'] === 'many') {
$embeddedDocuments = $rawValue;
$coll = new PersistentCollection(new ArrayCollection());
foreach ($embeddedDocuments as $embeddedDocument) {
foreach ($embeddedDocuments as $key => $embeddedDocument) {
$className = $this->dm->getClassNameFromDiscriminatorValue($mapping, $embeddedDocument);
$embeddedMetadata = $this->dm->getClassMetadata($className);
$embeddedDocumentObject = $embeddedMetadata->newInstance();
$this->hydrate($embeddedDocumentObject, $embeddedDocument);
$data[$mapping['name']][$key] = $embeddedDocument;
$this->dm->getUnitOfWork()->registerManaged($embeddedDocumentObject, null, $embeddedDocument);
$coll->add($embeddedDocumentObject);
}
Expand All @@ -159,15 +161,15 @@ public function hydrate($document, &$data)
// accessed and initialized for the first ime
$value->setReferences($references);
}

$data[$mapping['name']] = $value;
// Hydrate regular field
} else {
$value = Type::getType($mapping['type'])->convertToPHPValue($rawValue);
$data[$mapping['name']] = $value;
}

// Set hydrated field value to document
if ($value !== null) {
$data[$mapping['name']] = $value;
$metadata->setFieldValue($document, $mapping['fieldName'], $value);
}
}
Expand Down
42 changes: 20 additions & 22 deletions lib/Doctrine/ODM/MongoDB/Persisters/BasicDocumentPersister.php
View file
Expand Up @@ -375,7 +375,8 @@ public function prepareInsertData($document)
$insertData['_id'] = $this->prepareValue($mapping, $new);
continue;
}
$value = $this->prepareValue($mapping, $new);
$current = $this->class->getFieldValue($document, $mapping['fieldName']);
$value = $this->prepareValue($mapping, $current);
if ($value === null && $mapping['nullable'] === false) {
continue;
}
Expand Down Expand Up @@ -418,9 +419,6 @@ public function prepareInsertData($document)
*/
public function prepareUpdateData($document)
{
if (is_array($document) && isset($document['originalObject'])) {
$document = $document['originalObject'];
}
$oid = spl_object_hash($document);
$class = $this->dm->getClassMetadata(get_class($document));
$changeset = $this->uow->getDocumentChangeSet($document);
Expand All @@ -431,6 +429,7 @@ public function prepareUpdateData($document)
}
$old = isset($changeset[$mapping['fieldName']][0]) ? $changeset[$mapping['fieldName']][0] : null;
$new = isset($changeset[$mapping['fieldName']][1]) ? $changeset[$mapping['fieldName']][1] : null;
$current = $class->getFieldValue($document, $mapping['fieldName']);

if ($mapping['type'] === 'many' || $mapping['type'] === 'collection') {
$mapping['strategy'] = isset($mapping['strategy']) ? $mapping['strategy'] : 'pushPull';
Expand All @@ -440,7 +439,7 @@ public function prepareUpdateData($document)
if ( ! isset($old[$k])) {
continue;
}
$update = $this->prepareUpdateData($v);
$update = $this->prepareUpdateData($current[$k]);
foreach ($update as $cmd => $values) {
foreach ($values as $key => $value) {
$result[$cmd][$mapping['name'] . '.' . $k . '.' . $key] = $value;
Expand All @@ -449,15 +448,18 @@ public function prepareUpdateData($document)
}
}
if ($old !== $new) {
$old = $old ? $old : array();
$new = $new ? $new : array();
$compare = function($a, $b) {
$a = is_array($a) && isset($a['originalObject']) ? $a['originalObject'] : $a;
$b = is_array($b) && isset($b['originalObject']) ? $b['originalObject'] : $b;
return $a === $b ? 0 : 1;
};
$deleteDiff = array_udiff_assoc($old, $new, $compare);
$insertDiff = array_udiff_assoc($new, $old, $compare);
if ($mapping['type'] === 'collection' || isset($mapping['reference'])) {
$old = $old ? $old : array();
$new = $new ? $new : array();
$compare = function($a, $b) {
return $a === $b ? 0 : 1;
};
$deleteDiff = array_udiff_assoc($old, $new, $compare);
$insertDiff = array_udiff_assoc($new, $old, $compare);
} elseif (isset($mapping['embedded'])) {
$deleteDiff = $current->getDeleteDiff();
$insertDiff = $current->getInsertDiff();
}

// insert diff
if ($insertDiff) {
Expand All @@ -470,7 +472,7 @@ public function prepareUpdateData($document)
}
} elseif ($mapping['strategy'] === 'set') {
if ($old !== $new) {
$new = $this->prepareValue($mapping, $new);
$new = $this->prepareValue($mapping, $current);
$result[$this->cmd . 'set'][$mapping['name']] = $new;
}
}
Expand All @@ -489,14 +491,13 @@ public function prepareUpdateData($document)
if (isset($mapping['embedded']) && $mapping['type'] === 'one') {
// If we didn't have a value before and now we do
if ( ! $old && $new) {
$new = $this->prepareValue($mapping, $new);
$new = $this->prepareValue($mapping, $current);
if (isset($new) || $mapping['nullable'] === true) {
$result[$this->cmd . 'set'][$mapping['name']] = $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);
$update = $this->prepareUpdateData($current);
foreach ($update as $cmd => $values) {
foreach ($values as $key => $value) {
$result[$cmd][$mapping['name'] . '.' . $key] = $value;
Expand All @@ -505,7 +506,7 @@ public function prepareUpdateData($document)
}
// $set all other fields
} else {
$new = $this->prepareValue($mapping, $new);
$new = $this->prepareValue($mapping, $current);
if (isset($new) || $mapping['nullable'] === true) {
$result[$this->cmd . 'set'][$mapping['name']] = $new;
} else {
Expand Down Expand Up @@ -593,9 +594,6 @@ private function prepareReferencedDocValue(array $referenceMapping, $document)
*/
private function prepareEmbeddedDocValue(array $embeddedMapping, $embeddedDocument)
{
if (is_array($embeddedDocument) && isset($embeddedDocument['originalObject'])) {
$embeddedDocument = $embeddedDocument['originalObject'];

This comment has been minimized.

Sign in to view

Copy link
@sebastianhoitz

sebastianhoitz Oct 12, 2010

Contributor

This introduced a bug for me. Now my project-internal unit tests fail, because get_class is called on an array.

This comment has been minimized.

Sign in to view

Copy link
@jwage

jwage Oct 12, 2010

Author Member

Hmm. Can you provide the documents you're using and the code that triggers the error? We must have a situation I missed in the code where I am still passing the array and not the current object.

This comment has been minimized.

Sign in to view

Copy link
@sebastianhoitz

sebastianhoitz Oct 14, 2010

Contributor

Hm... I tried reproducing the issue with new documents but I was not able to. :(
I can't share the whole document with you, but I will try to put it in the unit test and then dumb it down. But it's gonna take a while.
}
$className = get_class($embeddedDocument);
$class = $this->dm->getClassMetadata($className);
$embeddedDocumentValue = array();
Expand Down
4 changes: 0 additions & 4 deletions lib/Doctrine/ODM/MongoDB/UnitOfWork.php
View file
Expand Up @@ -382,14 +382,10 @@ public function getDocumentActualData($document)
$actualData[$name] = array();
foreach ($embeddedDocuments as $key => $embeddedDocument) {
$actualData[$name][$key] = $this->getDocumentActualData($embeddedDocument);
$actualData[$name][$key]['originalObject'] = $embeddedDocument;
$visited = array();
}
} elseif ($class->isSingleValuedEmbed($name) && is_object($value)) {
$embeddedDocument = $value;
$actualData[$name] = $this->getDocumentActualData($embeddedDocument);
$actualData[$name]['originalObject'] = $embeddedDocument;
$visited = array();
} else {
$actualData[$name] = $value;
}
Expand Down
6 changes: 5 additions & 1 deletion tests/Documents/Functional/EmbeddedTestLevel0b.php
View file
Expand Up @@ -3,8 +3,12 @@
namespace Documents\Functional;

/** @Document(collection="embedded_test") */
class EmbeddedTestLevel0b extends EmbeddedTestLevel0
class EmbeddedTestLevel0b
{
/** @Id */
public $id;
/** @String */
public $name;
/** @EmbedOne(targetDocument="EmbeddedTestLevel1") */
public $oneLevel1;
}

0 comments on commit 8bd3aef

Please sign in to comment.