Skip to content

Commit

Permalink
Fix errors with nullable typed associations (#2302)
Browse files Browse the repository at this point in the history 
* Fix initialising nullable associations

* Fix error when merging documents with uninitialised typed properties
  • Loading branch information
@alcaeus
alcaeus authored Apr 22, 2021
1 parent 18d9ac7 commit 5ca00af
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 75 deletions.
49 changes: 27 additions & 22 deletions lib/Doctrine/ODM/MongoDB/Hydrator/HydratorFactory.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,18 +250,20 @@ private function generateHydratorClass(ClassMetadata $class, string $hydratorCla
<<<EOF
/** @ReferenceOne */
if (isset(\$data['%1\$s'])) {
\$reference = \$data['%1\$s'];
if (isset(\$data['%1\$s']) || (! empty(\$this->class->fieldMappings['%2\$s']['nullable']) && array_key_exists('%1\$s', \$data))) {
\$return = \$data['%1\$s'];
if (\$return !== null) {
if (\$this->class->fieldMappings['%2\$s']['storeAs'] !== ClassMetadata::REFERENCE_STORE_AS_ID && ! is_array(\$return)) {
throw HydratorException::associationTypeMismatch('%3\$s', '%1\$s', 'array', gettype(\$return));
}
if (\$this->class->fieldMappings['%2\$s']['storeAs'] !== ClassMetadata::REFERENCE_STORE_AS_ID && ! is_array(\$reference)) {
throw HydratorException::associationTypeMismatch('%3\$s', '%1\$s', 'array', gettype(\$reference));
\$className = \$this->unitOfWork->getClassNameForAssociation(\$this->class->fieldMappings['%2\$s'], \$return);
\$identifier = ClassMetadata::getReferenceId(\$return, \$this->class->fieldMappings['%2\$s']['storeAs']);
\$targetMetadata = \$this->dm->getClassMetadata(\$className);
\$id = \$targetMetadata->getPHPIdentifierValue(\$identifier);
\$return = \$this->dm->getReference(\$className, \$id);
}
\$className = \$this->unitOfWork->getClassNameForAssociation(\$this->class->fieldMappings['%2\$s'], \$reference);
\$identifier = ClassMetadata::getReferenceId(\$reference, \$this->class->fieldMappings['%2\$s']['storeAs']);
\$targetMetadata = \$this->dm->getClassMetadata(\$className);
\$id = \$targetMetadata->getPHPIdentifierValue(\$identifier);
\$return = \$this->dm->getReference(\$className, \$id);
\$this->class->reflFields['%2\$s']->setValue(\$document, \$return);
\$hydratedData['%2\$s'] = \$return;
}
Expand Down Expand Up @@ -344,24 +346,27 @@ private function generateHydratorClass(ClassMetadata $class, string $hydratorCla
<<<EOF
/** @EmbedOne */
if (isset(\$data['%1\$s'])) {
\$embeddedDocument = \$data['%1\$s'];
if (isset(\$data['%1\$s']) || (! empty(\$this->class->fieldMappings['%2\$s']['nullable']) && array_key_exists('%1\$s', \$data))) {
\$return = \$data['%1\$s'];
if (\$return !== null) {
\$embeddedDocument = \$return;
if (! is_array(\$embeddedDocument)) {
throw HydratorException::associationTypeMismatch('%3\$s', '%1\$s', 'array', gettype(\$embeddedDocument));
}
if (! is_array(\$embeddedDocument)) {
throw HydratorException::associationTypeMismatch('%3\$s', '%1\$s', 'array', gettype(\$embeddedDocument));
}
\$className = \$this->unitOfWork->getClassNameForAssociation(\$this->class->fieldMappings['%2\$s'], \$embeddedDocument);
\$embeddedMetadata = \$this->dm->getClassMetadata(\$className);
\$return = \$embeddedMetadata->newInstance();
\$className = \$this->unitOfWork->getClassNameForAssociation(\$this->class->fieldMappings['%2\$s'], \$embeddedDocument);
\$embeddedMetadata = \$this->dm->getClassMetadata(\$className);
\$return = \$embeddedMetadata->newInstance();
\$this->unitOfWork->setParentAssociation(\$return, \$this->class->fieldMappings['%2\$s'], \$document, '%1\$s');
\$this->unitOfWork->setParentAssociation(\$return, \$this->class->fieldMappings['%2\$s'], \$document, '%1\$s');
\$embeddedData = \$this->dm->getHydratorFactory()->hydrate(\$return, \$embeddedDocument, \$hints);
\$embeddedId = \$embeddedMetadata->identifier && isset(\$embeddedData[\$embeddedMetadata->identifier]) ? \$embeddedData[\$embeddedMetadata->identifier] : null;
\$embeddedData = \$this->dm->getHydratorFactory()->hydrate(\$return, \$embeddedDocument, \$hints);
\$embeddedId = \$embeddedMetadata->identifier && isset(\$embeddedData[\$embeddedMetadata->identifier]) ? \$embeddedData[\$embeddedMetadata->identifier] : null;
if (empty(\$hints[Query::HINT_READ_ONLY])) {
\$this->unitOfWork->registerManaged(\$return, \$embeddedId, \$embeddedData);
if (empty(\$hints[Query::HINT_READ_ONLY])) {
\$this->unitOfWork->registerManaged(\$return, \$embeddedId, \$embeddedData);
}
}
\$this->class->reflFields['%2\$s']->setValue(\$document, \$return);
Expand Down
5 changes: 5 additions & 0 deletions lib/Doctrine/ODM/MongoDB/UnitOfWork.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1830,6 +1830,11 @@ private function doMerge(object $document, array &$visited, ?object $prevManaged
$name = $nativeReflection->name;
$prop = $this->reflectionService->getAccessibleProperty($class->name, $name);
assert($prop instanceof ReflectionProperty);

if (method_exists($prop, 'isInitialized') && ! $prop->isInitialized($document)) {
continue;
}

if (! isset($class->associationMappings[$name])) {
if (! $class->isIdentifier($name)) {
$prop->setValue($managedCopy, $prop->getValue($document));
Expand Down
86 changes: 65 additions & 21 deletions tests/Doctrine/ODM/MongoDB/Tests/Functional/TypedPropertiesTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,52 +26,96 @@ public function setUp(): void

public function testPersistNew(): void
{
$doc = new TypedDocument();
$doc->setName('Maciej');
$doc->setEmbedOne(new TypedEmbeddedDocument('The answer', 42));
$ref = new TypedDocument();
$ref->name = 'alcaeus';
$this->dm->persist($ref);

$doc = new TypedDocument();
$doc->name = 'Maciej';
$doc->embedOne = new TypedEmbeddedDocument('The answer', 42);
$doc->referenceOne = $ref;
$doc->getEmbedMany()->add(new TypedEmbeddedDocument('Lucky number', 7));
$this->dm->persist($doc);
$this->dm->flush();
$this->dm->clear();

$saved = $this->dm->find(TypedDocument::class, $doc->getId());
$ref = $this->dm->find(TypedDocument::class, $ref->id);
$saved = $this->dm->find(TypedDocument::class, $doc->id);
assert($saved instanceof TypedDocument);
$this->assertEquals($doc->getId(), $saved->getId());
$this->assertSame($doc->getName(), $saved->getName());
$this->assertEquals($doc->getEmbedOne(), $saved->getEmbedOne());
$this->assertEquals($doc->id, $saved->id);
$this->assertSame($doc->name, $saved->name);
$this->assertEquals($doc->embedOne, $saved->embedOne);
$this->assertSame($ref, $saved->referenceOne);
$this->assertEquals($doc->getEmbedMany()->getValues(), $saved->getEmbedMany()->getValues());
}

public function testMerge(): void
{
$doc = new TypedDocument();
$doc->setId((string) new ObjectId());
$doc->setName('Maciej');
$doc->setEmbedOne(new TypedEmbeddedDocument('The answer', 42));
$ref = new TypedDocument();
$ref->name = 'alcaeus';
$this->dm->persist($ref);

$doc = new TypedDocument();
$doc->id = (string) new ObjectId();
$doc->name = 'Maciej';
$doc->embedOne = new TypedEmbeddedDocument('The answer', 42);
$doc->referenceOne = $ref;
$doc->getEmbedMany()->add(new TypedEmbeddedDocument('Lucky number', 7));

$merged = $this->dm->merge($doc);
assert($merged instanceof TypedDocument);
$this->assertEquals($doc->id, $merged->id);
$this->assertSame($doc->name, $merged->name);
$this->assertEquals($doc->embedOne, $merged->embedOne);
$this->assertEquals($doc->referenceOne, $merged->referenceOne);
$this->assertEquals($doc->getEmbedMany()->getValues(), $merged->getEmbedMany()->getValues());
}

public function testMergeWithUninitializedAssociations(): void
{
$doc = new TypedDocument();
$doc->id = (string) new ObjectId();
$doc->name = 'Maciej';
$doc->getEmbedMany()->add(new TypedEmbeddedDocument('Lucky number', 7));

$merged = $this->dm->merge($doc);
$this->assertEquals($doc->getId(), $merged->getId());
$this->assertSame($doc->getName(), $merged->getName());
$this->assertEquals($doc->getEmbedOne(), $merged->getEmbedOne());
assert($merged instanceof TypedDocument);
$this->assertEquals($doc->id, $merged->id);
$this->assertSame($doc->name, $merged->name);
$this->assertEquals($doc->getEmbedMany()->getValues(), $merged->getEmbedMany()->getValues());
}

public function testProxying(): void
{
$doc = new TypedDocument();
$doc->setName('Maciej');
$doc->setEmbedOne(new TypedEmbeddedDocument('The answer', 42));
$doc = new TypedDocument();
$doc->name = 'Maciej';
$doc->embedOne = new TypedEmbeddedDocument('The answer', 42);
$doc->getEmbedMany()->add(new TypedEmbeddedDocument('Lucky number', 7));
$this->dm->persist($doc);
$this->dm->flush();
$this->dm->clear();

$proxy = $this->dm->getReference(TypedDocument::class, $doc->getId());
$proxy = $this->dm->getReference(TypedDocument::class, $doc->id);
assert($proxy instanceof TypedDocument);
$this->assertEquals($doc->getId(), $proxy->getId());
$this->assertSame($doc->getName(), $proxy->getName());
$this->assertEquals($doc->getEmbedOne(), $proxy->getEmbedOne());
$this->assertEquals($doc->id, $proxy->id);
$this->assertSame($doc->name, $proxy->name);
$this->assertEquals($doc->embedOne, $proxy->embedOne);
$this->assertEquals($doc->getEmbedMany()->getValues(), $proxy->getEmbedMany()->getValues());
}

public function testNullableProperties(): void
{
$doc = new TypedDocument();
$doc->name = 'webmozart';
$this->dm->persist($doc);
$this->dm->flush();
$this->dm->clear();

$saved = $this->dm->find(TypedDocument::class, $doc->id);
assert($saved instanceof TypedDocument);
$this->assertNull($saved->nullableEmbedOne);
$this->assertNull($saved->initializedNullableEmbedOne);
$this->assertNull($saved->nullableReferenceOne);
$this->assertNull($saved->initializedNullableReferenceOne);
}
}
49 changes: 17 additions & 32 deletions tests/Documents74/TypedDocument.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,50 +14,35 @@
class TypedDocument
{
/** @ODM\Id() */
private string $id;
public string $id;

/** @ODM\Field(type="string") */
private string $name;
public string $name;

/** @ODM\EmbedOne(targetDocument=TypedEmbeddedDocument::class) */
private TypedEmbeddedDocument $embedOne;
public TypedEmbeddedDocument $embedOne;

/** @ODM\EmbedMany(targetDocument=TypedEmbeddedDocument::class) */
private Collection $embedMany;
/** @ODM\EmbedOne(targetDocument=TypedEmbeddedDocument::class, nullable=true) */
public ?TypedEmbeddedDocument $nullableEmbedOne;

public function __construct()
{
$this->embedMany = new ArrayCollection();
}
/** @ODM\EmbedOne(targetDocument=TypedEmbeddedDocument::class, nullable=true) */
public ?TypedEmbeddedDocument $initializedNullableEmbedOne = null;

public function getId(): string
{
return $this->id;
}
/** @ODM\ReferenceOne(targetDocument=TypedDocument::class) */
public TypedDocument $referenceOne;

public function setId(string $id): void
{
$this->id = $id;
}
/** @ODM\ReferenceOne(targetDocument=TypedDocument::class, nullable=true) */
public ?TypedDocument $nullableReferenceOne;

public function getName(): string
{
return $this->name;
}
/** @ODM\ReferenceOne(targetDocument=TypedDocument::class, nullable=true) */
public ?TypedDocument $initializedNullableReferenceOne = null;

public function setName(string $name): void
{
$this->name = $name;
}

public function getEmbedOne(): TypedEmbeddedDocument
{
return $this->embedOne;
}
/** @ODM\EmbedMany(targetDocument=TypedEmbeddedDocument::class) */
private Collection $embedMany;

public function setEmbedOne(TypedEmbeddedDocument $embedOne): void
public function __construct()
{
$this->embedOne = $embedOne;
$this->embedMany = new ArrayCollection();
}

public function getEmbedMany(): Collection
Expand Down

0 comments on commit 5ca00af

Please sign in to comment.