Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix root document entityMap #2726

Open
wants to merge 2 commits into
base: 2.10.x
Choose a base branch
from
Open

Fix root document entityMap #2726

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9bdde11
Fix root document entityMap
Feb 4, 2025
f373ffa
Fix root document entityMap
Feb 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadata.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2035,7 +2035,7 @@ public function setSubclasses(array $subclasses): void

/**
* Sets the parent class names.
* Assumes that the class names in the passed array are in the order:
* Assumes that the class names passed array are in the order:
* directParent -> directParentParent -> directParentParentParent ... -> root.
*
* @param list<class-string> $classNames
Expand All @@ -2044,7 +2044,7 @@ public function setParentClasses(array $classNames): void
{
$this->parentClasses = $classNames;

if (count($classNames) <= 0) {
if ($this->isEmbeddedDocument || count($classNames) <= 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need to add this condition?

Copy link
Member

Choose a reason for hiding this comment

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

Found the response here: #2725 (comment)

return;
}

Expand Down
24 changes: 12 additions & 12 deletions lib/Doctrine/ODM/MongoDB/UnitOfWork.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1505,11 +1505,11 @@ public function addToIdentityMap(object $document): bool
$class = $this->dm->getClassMetadata($document::class);
$id = $this->getIdForIdentityMap($document);

if (isset($this->identityMap[$class->name][$id])) {
if (isset($this->identityMap[$class->rootDocumentName][$id])) {
Copy link
Member

@GromNaN GromNaN Mar 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This need to be addressed and covered by tests: #2725 (comment)

I don't remember details about what is stored in rootDocumentName but at a glance now there can be a collision for COLLECTION_PER_CLASS inheritance type as it's perfectly possible for two separate documents to have same id

We can update the value of ClassMetadata::$rootDocumentName to contain $name when the strategy is COLLECTION_PER_CLASS. That would solve the issue. (Or an new internal property $identityMapName to keep backward compatibility).

@malarzm any idea?

I don't understand the purpose of COLLECTION_PER_CLASS, it looks the same as just extending the class as you cannot query the root repository. Having duplicate ids make it even more useless.

Copy link
Member

Choose a reason for hiding this comment

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

I think I was using COLLECTION_PER_CLASS years ago to work around some mapped superclass limitations, but it's been so long I can't be sure.

return false;
}

$this->identityMap[$class->name][$id] = $document;
$this->identityMap[$class->rootDocumentName][$id] = $document;

if ($document instanceof NotifyPropertyChanged && ! $this->isUninitializedObject($document)) {
$document->addPropertyChangedListener($this);
Expand Down Expand Up @@ -1597,8 +1597,8 @@ public function removeFromIdentityMap(object $document): bool
$class = $this->dm->getClassMetadata($document::class);
$id = $this->getIdForIdentityMap($document);

if (isset($this->identityMap[$class->name][$id])) {
unset($this->identityMap[$class->name][$id]);
if (isset($this->identityMap[$class->rootDocumentName][$id])) {
unset($this->identityMap[$class->rootDocumentName][$id]);
$this->documentStates[$oid] = self::STATE_DETACHED;

return true;
Expand Down Expand Up @@ -1629,7 +1629,7 @@ public function getById($id, ClassMetadata $class): object

$serializedId = serialize($class->getDatabaseIdentifierValue($id));

return $this->identityMap[$class->name][$serializedId];
return $this->identityMap[$class->rootDocumentName][$serializedId];
}

/**
Expand Down Expand Up @@ -1658,7 +1658,7 @@ public function tryGetById($id, ClassMetadata $class)

$serializedId = serialize($class->getDatabaseIdentifierValue($id));

return $this->identityMap[$class->name][$serializedId] ?? false;
return $this->identityMap[$class->rootDocumentName][$serializedId] ?? false;
}

/**
Expand Down Expand Up @@ -1688,7 +1688,7 @@ public function isInIdentityMap(object $document): bool
$class = $this->dm->getClassMetadata($document::class);
$id = $this->getIdForIdentityMap($document);

return isset($this->identityMap[$class->name][$id]);
return isset($this->identityMap[$class->rootDocumentName][$id]);
}

private function getIdForIdentityMap(object $document): string
Expand Down Expand Up @@ -2768,13 +2768,13 @@ public function getOrCreateDocument(string $className, array $data, array &$hint
if (! $class->isQueryResultDocument) {
$id = $class->getDatabaseIdentifierValue($data['_id']);
$serializedId = serialize($id);
$isManagedObject = isset($this->identityMap[$class->name][$serializedId]);
$isManagedObject = isset($this->identityMap[$class->rootDocumentName][$serializedId]);
}

$oid = null;
if ($isManagedObject) {
/** @phpstan-var T $document */
$document = $this->identityMap[$class->name][$serializedId];
$document = $this->identityMap[$class->rootDocumentName][$serializedId];
$oid = spl_object_hash($document);
if ($this->isUninitializedObject($document)) {
$document->setProxyInitializer(null);
Expand All @@ -2798,9 +2798,9 @@ public function getOrCreateDocument(string $className, array $data, array &$hint

if (! $class->isQueryResultDocument) {
$this->registerManaged($document, $id, $data);
$oid = spl_object_hash($document);
$this->documentStates[$oid] = self::STATE_MANAGED;
$this->identityMap[$class->name][$serializedId] = $document;
$oid = spl_object_hash($document);
$this->documentStates[$oid] = self::STATE_MANAGED;
$this->identityMap[$class->rootDocumentName][$serializedId] = $document;
}

$data = $this->hydratorFactory->hydrate($document, $data, $hints);
Expand Down