Skip to content

Commit

Permalink
Fixing a bug where certain documents are improperly flushed after ret…
Browse files Browse the repository at this point in the history
…rieving the document and making no changes to it.

This occurs when a document has an embedded document that uses a discriminator map. In this case, after retrieving the object, the Hydrator::hydrate() method will register the "original values" for that object (via UnitOfWork::registerManaged()) WITH the discriminator field and value. On the next flush, the presence of the descriminator field and value will make the document appear that it has been modified (i.e. the changeset for the document shows that the original value has the discriminator field and the actual values do not).

The fix is to remove the discriminator field during hydration (after it's used). This is consistent with how discriminator fields are used on non-embedded documents (the discriminator field is removed in UnitOfWorkgetOrCreateDocument()). We don't remove the discriminator field, however, if it's a real, persisted property on the embedded document.
  • Loading branch information
weaverryan committed Oct 20, 2010
1 parent 0794fca commit 01e0240
Show file tree
Hide file tree
Showing 2 changed files with 132 additions and 0 deletions.
16 changes: 16 additions & 0 deletions lib/Doctrine/ODM/MongoDB/Hydrator.php
Expand Up @@ -124,6 +124,14 @@ public function hydrate($document, &$data)
$className = $this->dm->getClassNameFromDiscriminatorValue($mapping, $embeddedDocument);
$embeddedMetadata = $this->dm->getClassMetadata($className);
$value = $embeddedMetadata->newInstance();

// unset a potential discriminator map field (unless it's a persisted property)
$discriminatorField = isset($mapping['discriminatorField']) ? $mapping['discriminatorField'] : '_doctrine_class_name';
if (!isset($embeddedMetadata->fieldMappings[$discriminatorField]))
{
unset($embeddedDocument[$discriminatorField]);
}

$this->hydrate($value, $embeddedDocument);
$data[$mapping['name']] = $embeddedDocument;
$this->dm->getUnitOfWork()->registerManaged($value, null, $embeddedDocument);
Expand All @@ -134,6 +142,14 @@ public function hydrate($document, &$data)
$className = $this->dm->getClassNameFromDiscriminatorValue($mapping, $embeddedDocument);
$embeddedMetadata = $this->dm->getClassMetadata($className);
$embeddedDocumentObject = $embeddedMetadata->newInstance();

// unset a potential discriminator map field (unless it's a persisted property)
$discriminatorField = isset($mapping['discriminatorField']) ? $mapping['discriminatorField'] : '_doctrine_class_name';
if (!isset($embeddedMetadata->fieldMappings[$discriminatorField]))
{
unset($embeddedDocument[$discriminatorField]);
}

$this->hydrate($embeddedDocumentObject, $embeddedDocument);
$data[$mapping['name']][$key] = $embeddedDocument;
$this->dm->getUnitOfWork()->registerManaged($embeddedDocumentObject, null, $embeddedDocument);
Expand Down
116 changes: 116 additions & 0 deletions tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/MODM90Test.php
@@ -0,0 +1,116 @@
<?php

namespace Doctrine\ODM\MongoDB\Tests\Functional\Ticket;

require_once __DIR__ . '/../../../../../../TestInit.php';

use Doctrine\ODM\MongoDB\ODMEvents;

class MODM90Test extends \Doctrine\ODM\MongoDB\Tests\BaseTest
{
private function getDocumentManager()
{
$this->listener = new MODM90EventListener();
$evm = $this->dm->getEventManager();
$events = array(
ODMEvents::preUpdate,
ODMEvents::postUpdate,
);
$evm->addEventListener($events, $this->listener);
return $this->dm;
}

public function testDocumentWithEmbeddedDocumentNotUpdatedOnFlush()
{
$dm = $this->getDocumentManager();

$testDoc = new MODM90TestDocument();
$testDoc->name = 'Parent';
$testDoc->embedded = new MODM90TestEmbeddedDocument();
$testDoc->embedded->name = 'Child';
$dm->persist($testDoc);
$dm->flush();
$dm->clear();

$testDoc = $dm->findOne(__NAMESPACE__.'\MODM90TestDocument');

// run a flush, in theory, nothing should be flushed.
$dm->flush();
$dm->clear();

// no update events should be called
$called = array();
$this->assertEquals($called, $this->listener->called);
}

/*
* Ensures that the descriminator field is not unset if it's a
* real property on the document.
*/
public function testDiscriminatorFieldValuePresentIfRealProperty()
{
$dm = $this->getDocumentManager();

$testDoc = new MODM90TestDocument();
$testDoc->name = 'Parent';
$testDoc->embedded = new MODM90Test2EmbeddedDocument();
$testDoc->embedded->name = 'Child';
$dm->persist($testDoc);
$dm->flush();
$dm->clear();

$testDoc = $dm->findOne(__NAMESPACE__.'\MODM90TestDocument');

$this->assertEquals($testDoc->embedded->type, 'test2');
}
}

class MODM90EventListener
{
public $called = array();
public function __call($method, $args)
{
$document = $args[0]->getDocument();
$className = get_class($document);
$this->called[$method][] = $className;
}
}

/** @Document */
class MODM90TestDocument
{
/** @Id */
public $id;

/** @String */
public $name;

/**
* @EmbedOne
* (
* discriminatorField="type",
* discriminatorMap={
* "test"="MODM90TestEmbeddedDocument",
* "test2"="MODM90Test2EmbeddedDocument"
* }
* )
*/
public $embedded;
}

/** @EmbeddedDocument */
class MODM90TestEmbeddedDocument
{
/** @String */
public $name;
}

/** @EmbeddedDocument */
class MODM90Test2EmbeddedDocument
{
/** @String */
public $name;

/** @String The discriminator field is a real property */
public $type;
}

2 comments on commit 01e0240

@jwage
Copy link
Member

@jwage jwage commented on 01e0240 Oct 21, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, we missed some coding standards. Can you fix the if conditions with the { on the next line?

@weaverryan
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You caught me - pull request issued with the fix.

thx

Please sign in to comment.