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
doctrine · Jun 10, 2024 · f3af4bf · f3af4bf
1 parent fed919f
commit f3af4bf
Show file tree

Hide file tree

Showing 8 changed files with 88 additions and 107 deletions.
diff --git 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<string, mixed>|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
@@ -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
@@ -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
@@ -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
@@ -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<string, mixed>|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
@@ -28,6 +28,9 @@
     <ImplementedReturnTypeMismatch>
       <code><![CDATA[ClassMetadataFactoryInterface]]></code>
     </ImplementedReturnTypeMismatch>
+    <NullableReturnStatement>
+      <code><![CDATA[$mapping['targetDocument']]]></code>
+    </NullableReturnStatement>
   </file>
   <file src="lib/Doctrine/ODM/MongoDB/Event/OnClearEventArgs.php">
     <TooManyArguments>
@@ -296,9 +299,6 @@
     <InvalidReturnType>
       <code><![CDATA[array<class-string, array{0: ClassMetadata<object>, 1: array<string, object>}>]]></code>
     </InvalidReturnType>
-    <NullableReturnStatement>
-      <code><![CDATA[$mapping['targetDocument']]]></code>
-    </NullableReturnStatement>
   </file>
   <file src="tests/Doctrine/ODM/MongoDB/Tests/Aggregation/Stage/UnsetTest.php">
     <InvalidArgument>

diff --git 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
@@ -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();