From f3af4bffa13dc8a5393715699eba2d2f761745ad Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 10 Jun 2024 08:38:52 +0200 Subject: [PATCH] Decouple UnitOfWork from Hydrators (#2641) * Always use DocumentManager's UnitOfWork in Hydrators * Move getClassNameForAssociation from UnitOfWork to DocumentManager * Remove unnecessary use statement * Fix phpcs * Fix psalm baseline --- lib/Doctrine/ODM/MongoDB/DocumentManager.php | 45 ++++++++++++++++++- .../ODM/MongoDB/Hydrator/HydratorFactory.php | 32 +++---------- .../MongoDB/Persisters/DocumentPersister.php | 4 +- .../ODM/MongoDB/Query/ReferencePrimer.php | 2 +- lib/Doctrine/ODM/MongoDB/UnitOfWork.php | 42 ----------------- psalm-baseline.xml | 6 +-- .../ODM/MongoDB/Tests/DocumentManagerTest.php | 32 +++++++++++++ .../ODM/MongoDB/Tests/UnitOfWorkTest.php | 32 ------------- 8 files changed, 88 insertions(+), 107 deletions(-) diff --git a/lib/Doctrine/ODM/MongoDB/DocumentManager.php b/lib/Doctrine/ODM/MongoDB/DocumentManager.php index 376ef9327f..b98f0dc3e1 100644 --- a/lib/Doctrine/ODM/MongoDB/DocumentManager.php +++ b/lib/Doctrine/ODM/MongoDB/DocumentManager.php @@ -179,8 +179,7 @@ protected function __construct(?Client $client = null, ?Configuration $config = $this->config->getAutoGenerateHydratorClasses(), ); - $this->unitOfWork = new UnitOfWork($this, $this->eventManager, $this->hydratorFactory); - $this->hydratorFactory->setUnitOfWork($this->unitOfWork); + $this->unitOfWork = new UnitOfWork($this, $this->eventManager, $this->hydratorFactory); $this->schemaManager = new SchemaManager($this, $this->metadataFactory); $this->proxyFactory = new StaticProxyFactory($this); $this->repositoryFactory = $this->config->getRepositoryFactory(); @@ -883,6 +882,48 @@ public function getFilterCollection(): FilterCollection return $this->filterCollection; } + /** + * Gets the class name for an association (embed or reference) with respect + * to any discriminator value. + * + * @internal + * + * @param FieldMapping $mapping + * @param array|null $data + * + * @psalm-return class-string + */ + public function getClassNameForAssociation(array $mapping, $data): string + { + $discriminatorField = $mapping['discriminatorField'] ?? null; + + $discriminatorValue = null; + if (isset($discriminatorField, $data[$discriminatorField])) { + $discriminatorValue = $data[$discriminatorField]; + } elseif (isset($mapping['defaultDiscriminatorValue'])) { + $discriminatorValue = $mapping['defaultDiscriminatorValue']; + } + + if ($discriminatorValue !== null) { + return $mapping['discriminatorMap'][$discriminatorValue] + ?? (string) $discriminatorValue; + } + + $class = $this->getClassMetadata($mapping['targetDocument']); + + if (isset($class->discriminatorField, $data[$class->discriminatorField])) { + $discriminatorValue = $data[$class->discriminatorField]; + } elseif ($class->defaultDiscriminatorValue !== null) { + $discriminatorValue = $class->defaultDiscriminatorValue; + } + + if ($discriminatorValue !== null) { + return $class->discriminatorMap[$discriminatorValue] ?? $discriminatorValue; + } + + return $mapping['targetDocument']; + } + private static function getVersion(): string { if (self::$version === null) { diff --git a/lib/Doctrine/ODM/MongoDB/Hydrator/HydratorFactory.php b/lib/Doctrine/ODM/MongoDB/Hydrator/HydratorFactory.php index 861d0f8e5c..8a076cca5d 100644 --- a/lib/Doctrine/ODM/MongoDB/Hydrator/HydratorFactory.php +++ b/lib/Doctrine/ODM/MongoDB/Hydrator/HydratorFactory.php @@ -46,11 +46,6 @@ final class HydratorFactory */ private DocumentManager $dm; - /** - * The UnitOfWork used to coordinate object-level transactions. - */ - private ?UnitOfWork $unitOfWork = null; - /** * The EventManager associated with this Hydrator */ @@ -96,16 +91,6 @@ public function __construct(DocumentManager $dm, EventManager $evm, ?string $hyd $this->autoGenerate = $autoGenerate; } - /** - * Sets the UnitOfWork instance. - * - * @internal - */ - public function setUnitOfWork(UnitOfWork $uow): void - { - $this->unitOfWork = $uow; - } - /** * Gets the hydrator object for the given document class. * @@ -147,7 +132,7 @@ public function getHydratorFor(string $className): HydratorInterface } } - $this->hydrators[$className] = new $fqn($this->dm, $this->unitOfWork, $class); + $this->hydrators[$className] = new $fqn($this->dm, $class); return $this->hydrators[$className]; } @@ -247,7 +232,7 @@ private function generateHydratorClass(ClassMetadata $class, string $hydratorCla throw HydratorException::associationTypeMismatch('%3$s', '%1$s', 'array', gettype($return)); } - $className = $this->unitOfWork->getClassNameForAssociation($this->class->fieldMappings['%2$s'], $return); + $className = $this->dm->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); @@ -294,7 +279,7 @@ private function generateHydratorClass(ClassMetadata $class, string $hydratorCla isset($this->class->fieldMappings['%2$s']['criteria']) ? $this->class->fieldMappings['%2$s']['criteria'] : array() ); $sort = isset($this->class->fieldMappings['%2$s']['sort']) ? $this->class->fieldMappings['%2$s']['sort'] : array(); - $return = $this->unitOfWork->getDocumentPersister($className)->load($criteria, null, array(), 0, $sort); + $return = $this->dm->getUnitOfWork()->getDocumentPersister($className)->load($criteria, null, array(), 0, $sort); $this->class->reflFields['%2$s']->setValue($document, $return); $hydratedData['%2$s'] = $return; @@ -345,17 +330,17 @@ private function generateHydratorClass(ClassMetadata $class, string $hydratorCla throw HydratorException::associationTypeMismatch('%3$s', '%1$s', 'array', gettype($embeddedDocument)); } - $className = $this->unitOfWork->getClassNameForAssociation($this->class->fieldMappings['%2$s'], $embeddedDocument); + $className = $this->dm->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->dm->getUnitOfWork()->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; if (empty($hints[Query::HINT_READ_ONLY])) { - $this->unitOfWork->registerManaged($return, $embeddedId, $embeddedData); + $this->dm->getUnitOfWork()->registerManaged($return, $embeddedId, $embeddedData); } } @@ -383,7 +368,6 @@ private function generateHydratorClass(ClassMetadata $class, string $hydratorCla use Doctrine\ODM\MongoDB\Hydrator\HydratorException; use Doctrine\ODM\MongoDB\Hydrator\HydratorInterface; use Doctrine\ODM\MongoDB\Query\Query; -use Doctrine\ODM\MongoDB\UnitOfWork; use Doctrine\ODM\MongoDB\Mapping\ClassMetadata; /** @@ -392,13 +376,11 @@ private function generateHydratorClass(ClassMetadata $class, string $hydratorCla class $hydratorClassName implements HydratorInterface { private \$dm; - private \$unitOfWork; private \$class; - public function __construct(DocumentManager \$dm, UnitOfWork \$uow, ClassMetadata \$class) + public function __construct(DocumentManager \$dm, ClassMetadata \$class) { \$this->dm = \$dm; - \$this->unitOfWork = \$uow; \$this->class = \$class; } diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php index c2adf2ddd4..780c0c6004 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php @@ -696,7 +696,7 @@ private function loadEmbedManyCollection(PersistentCollectionInterface $collecti } foreach ($embeddedDocuments as $key => $embeddedDocument) { - $className = $this->uow->getClassNameForAssociation($mapping, $embeddedDocument); + $className = $this->dm->getClassNameForAssociation($mapping, $embeddedDocument); $embeddedMetadata = $this->dm->getClassMetadata($className); $embeddedDocumentObject = $embeddedMetadata->newInstance(); @@ -735,7 +735,7 @@ private function loadReferenceManyCollectionOwningSide(PersistentCollectionInter $sorted = isset($mapping['sort']) && $mapping['sort']; foreach ($collection->getMongoData() as $key => $reference) { - $className = $this->uow->getClassNameForAssociation($mapping, $reference); + $className = $this->dm->getClassNameForAssociation($mapping, $reference); if ($mapping['storeAs'] !== ClassMetadata::REFERENCE_STORE_AS_ID && ! is_array($reference)) { throw HydratorException::associationItemTypeMismatch($owner::class, $mapping['name'], $key, 'array', gettype($reference)); diff --git a/lib/Doctrine/ODM/MongoDB/Query/ReferencePrimer.php b/lib/Doctrine/ODM/MongoDB/Query/ReferencePrimer.php index f8e88645d0..b754661dd3 100644 --- a/lib/Doctrine/ODM/MongoDB/Query/ReferencePrimer.php +++ b/lib/Doctrine/ODM/MongoDB/Query/ReferencePrimer.php @@ -258,7 +258,7 @@ private function addManyReferences(PersistentCollectionInterface $persistentColl $id = ClassMetadata::getReferenceId($reference, $mapping['storeAs']); if ($mapping['storeAs'] !== ClassMetadata::REFERENCE_STORE_AS_ID) { - $className = $this->uow->getClassNameForAssociation($mapping, $reference); + $className = $this->dm->getClassNameForAssociation($mapping, $reference); $class = $this->dm->getClassMetadata($className); } diff --git a/lib/Doctrine/ODM/MongoDB/UnitOfWork.php b/lib/Doctrine/ODM/MongoDB/UnitOfWork.php index c3b2af5e45..71f5f925f8 100644 --- a/lib/Doctrine/ODM/MongoDB/UnitOfWork.php +++ b/lib/Doctrine/ODM/MongoDB/UnitOfWork.php @@ -2727,48 +2727,6 @@ public function getOwningDocument(object $document): object return $document; } - /** - * Gets the class name for an association (embed or reference) with respect - * to any discriminator value. - * - * @internal - * - * @param FieldMapping $mapping - * @param array|null $data - * - * @psalm-return class-string - */ - public function getClassNameForAssociation(array $mapping, $data): string - { - $discriminatorField = $mapping['discriminatorField'] ?? null; - - $discriminatorValue = null; - if (isset($discriminatorField, $data[$discriminatorField])) { - $discriminatorValue = $data[$discriminatorField]; - } elseif (isset($mapping['defaultDiscriminatorValue'])) { - $discriminatorValue = $mapping['defaultDiscriminatorValue']; - } - - if ($discriminatorValue !== null) { - return $mapping['discriminatorMap'][$discriminatorValue] - ?? (string) $discriminatorValue; - } - - $class = $this->dm->getClassMetadata($mapping['targetDocument']); - - if (isset($class->discriminatorField, $data[$class->discriminatorField])) { - $discriminatorValue = $data[$class->discriminatorField]; - } elseif ($class->defaultDiscriminatorValue !== null) { - $discriminatorValue = $class->defaultDiscriminatorValue; - } - - if ($discriminatorValue !== null) { - return $class->discriminatorMap[$discriminatorValue] ?? $discriminatorValue; - } - - return $mapping['targetDocument']; - } - /** * Creates a document. Used for reconstitution of documents during hydration. * diff --git a/psalm-baseline.xml b/psalm-baseline.xml index a0a40fd84f..7bd8cfefb5 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -28,6 +28,9 @@ + + + @@ -296,9 +299,6 @@ , 1: array}>]]> - - - diff --git a/tests/Doctrine/ODM/MongoDB/Tests/DocumentManagerTest.php b/tests/Doctrine/ODM/MongoDB/Tests/DocumentManagerTest.php index ba84318f18..832037e53c 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/DocumentManagerTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/DocumentManagerTest.php @@ -26,6 +26,7 @@ use Documents\CmsUser; use Documents\CustomRepository\Document; use Documents\CustomRepository\Repository; +use Documents\ForumUser; use Documents\Tournament\Participant; use Documents\Tournament\ParticipantSolo; use Documents\User; @@ -229,6 +230,37 @@ public function testDifferentStoreAsDbReferences(): void self::assertCount(1, $dbRef); self::assertArrayHasKey('id', $dbRef); } + + public function testGetClassNameForAssociation(): void + { + $mapping = ClassMetadataTestUtil::getFieldMapping([ + 'discriminatorField' => 'type', + 'discriminatorMap' => ['forum_user' => ForumUser::class], + 'targetDocument' => User::class, + ]); + $data = ['type' => 'forum_user']; + + self::assertEquals(ForumUser::class, $this->dm->getClassNameForAssociation($mapping, $data)); + } + + public function testGetClassNameForAssociationWithClassMetadataDiscriminatorMap(): void + { + $mapping = ClassMetadataTestUtil::getFieldMapping(['targetDocument' => User::class]); + $data = ['type' => 'forum_user']; + + $userClassMetadata = new ClassMetadata(ForumUser::class); + $userClassMetadata->discriminatorField = 'type'; + $userClassMetadata->discriminatorMap = ['forum_user' => ForumUser::class]; + $this->dm->getMetadataFactory()->setMetadataFor(User::class, $userClassMetadata); + + self::assertEquals(ForumUser::class, $this->dm->getClassNameForAssociation($mapping, $data)); + } + + public function testGetClassNameForAssociationReturnsTargetDocumentWithNullData(): void + { + $mapping = ClassMetadataTestUtil::getFieldMapping(['targetDocument' => User::class]); + self::assertEquals(User::class, $this->dm->getClassNameForAssociation($mapping, null)); + } } #[ODM\Document] diff --git a/tests/Doctrine/ODM/MongoDB/Tests/UnitOfWorkTest.php b/tests/Doctrine/ODM/MongoDB/Tests/UnitOfWorkTest.php index 1ec3e632ca..81fac9bea7 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/UnitOfWorkTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/UnitOfWorkTest.php @@ -10,7 +10,6 @@ use Doctrine\Common\Collections\Collection; use Doctrine\ODM\MongoDB\APM\CommandLogger; use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM; -use Doctrine\ODM\MongoDB\Mapping\ClassMetadata; use Doctrine\ODM\MongoDB\MongoDBException; use Doctrine\ODM\MongoDB\Tests\Mocks\ExceptionThrowingListenerMock; use Doctrine\ODM\MongoDB\Tests\Mocks\PreUpdateListenerMock; @@ -475,37 +474,6 @@ public function testEmbeddedDocumentChangeSets(): void self::assertEquals('Nashville', $changeSet['city'][1]); } - public function testGetClassNameForAssociation(): void - { - $mapping = ClassMetadataTestUtil::getFieldMapping([ - 'discriminatorField' => 'type', - 'discriminatorMap' => ['forum_user' => ForumUser::class], - 'targetDocument' => User::class, - ]); - $data = ['type' => 'forum_user']; - - self::assertEquals(ForumUser::class, $this->uow->getClassNameForAssociation($mapping, $data)); - } - - public function testGetClassNameForAssociationWithClassMetadataDiscriminatorMap(): void - { - $mapping = ClassMetadataTestUtil::getFieldMapping(['targetDocument' => User::class]); - $data = ['type' => 'forum_user']; - - $userClassMetadata = new ClassMetadata(ForumUser::class); - $userClassMetadata->discriminatorField = 'type'; - $userClassMetadata->discriminatorMap = ['forum_user' => ForumUser::class]; - $this->dm->getMetadataFactory()->setMetadataFor(User::class, $userClassMetadata); - - self::assertEquals(ForumUser::class, $this->uow->getClassNameForAssociation($mapping, $data)); - } - - public function testGetClassNameForAssociationReturnsTargetDocumentWithNullData(): void - { - $mapping = ClassMetadataTestUtil::getFieldMapping(['targetDocument' => User::class]); - self::assertEquals(User::class, $this->uow->getClassNameForAssociation($mapping, null)); - } - public function testRecomputeChangesetForUninitializedProxyDoesNotCreateChangeset(): void { $user = new ForumUser();