Misc refactorings (#3)

* EntityPreloader: extract getCommonAncestor()

* EntityPreloader: refactor loadProxies() to be usable in preloadToOne()

* EntityPreloader: disallow ordered associations

* QueryLogger: improve getAggregatedQueries()

Author: JanTvrdik

src/EntityPreloader.php

@@ -44,42 +44,28 @@ public function preload(
         ?int $maxFetchJoinSameFieldCount = null,
     ): array
     {
-        $sourceEntitiesCommonAncestor = null;
-
-        foreach ($sourceEntities as $sourceEntity) {
-            $sourceEntityClassName = $sourceEntity::class;
-
-            if ($sourceEntitiesCommonAncestor === null) {
-                $sourceEntitiesCommonAncestor = $sourceEntityClassName;
-                continue;
-            }
-
-            while (!is_a($sourceEntityClassName, $sourceEntitiesCommonAncestor, true)) {
-                $sourceEntitiesCommonAncestor = get_parent_class($sourceEntitiesCommonAncestor);
-
-                if ($sourceEntitiesCommonAncestor === false) {
-                    throw new LogicException('Source entities must have a common ancestor');
-                }
-            }
-        }
+        $sourceEntitiesCommonAncestor = $this->getCommonAncestor($sourceEntities);
 
         if ($sourceEntitiesCommonAncestor === null) {
             return [];
         }
 
-        /** @var ClassMetadata<E> $sourceClassMetadata */
         $sourceClassMetadata = $this->entityManager->getClassMetadata($sourceEntitiesCommonAncestor);
         $associationMapping = $sourceClassMetadata->getAssociationMapping($sourcePropertyName);
+
         /** @var ClassMetadata<E> $targetClassMetadata */
         $targetClassMetadata = $this->entityManager->getClassMetadata($associationMapping->targetEntity);
 
         if ($associationMapping->isIndexed()) {
             throw new LogicException('Preloading of indexed associations is not supported');
         }
 
-        $maxFetchJoinSameFieldCount ??= 1;
+        if ($associationMapping->isOrdered()) {
+            throw new LogicException('Preloading of ordered associations is not supported');
+        }
 
-        $this->loadProxies($sourceClassMetadata, $sourceEntities, $batchSize, $maxFetchJoinSameFieldCount);
+        $maxFetchJoinSameFieldCount ??= 1;
+        $sourceEntities = $this->loadProxies($sourceClassMetadata, $sourceEntities, $batchSize ?? self::BATCH_SIZE, $maxFetchJoinSameFieldCount);
 
         return match ($associationMapping->type()) {
             ClassMetadata::ONE_TO_MANY => $this->preloadOneToMany($sourceEntities, $sourceClassMetadata, $sourcePropertyName, $targetClassMetadata, $batchSize, $maxFetchJoinSameFieldCount),
@@ -90,38 +76,75 @@ public function preload(
     }
 
     /**
-     * @param ClassMetadata<T> $sourceClassMetadata
-     * @param list<T> $sourceEntities
-     * @param positive-int|null $batchSize
+     * @param list<S> $entities
+     * @return class-string<S>|null
+     * @template S of E
+     */
+    private function getCommonAncestor(array $entities): ?string
+    {
+        $commonAncestor = null;
+
+        foreach ($entities as $entity) {
+            $entityClassName = $entity::class;
+
+            if ($commonAncestor === null) {
+                $commonAncestor = $entityClassName;
+                continue;
+            }
+
+            while (!is_a($entityClassName, $commonAncestor, true)) {
+                /** @var class-string<S>|false $commonAncestor */
+                $commonAncestor = get_parent_class($commonAncestor);
+
+                if ($commonAncestor === false) {
+                    throw new LogicException('Given entities must have a common ancestor');
+                }
+            }
+        }
+
+        return $commonAncestor;
+    }
+
+    /**
+     * @param ClassMetadata<T> $classMetadata
+     * @param list<T> $entities
+     * @param positive-int $batchSize
      * @param non-negative-int $maxFetchJoinSameFieldCount
+     * @return list<T>
      * @template T of E
      */
     private function loadProxies(
-        ClassMetadata $sourceClassMetadata,
-        array $sourceEntities,
-        ?int $batchSize,
+        ClassMetadata $classMetadata,
+        array $entities,
+        int $batchSize,
         int $maxFetchJoinSameFieldCount,
-    ): void
+    ): array
     {
-        $sourceIdentifierReflection = $sourceClassMetadata->getSingleIdReflectionProperty();
+        $identifierReflection = $classMetadata->getSingleIdReflectionProperty(); // e.g. Order::$id reflection
+        $identifierName = $classMetadata->getSingleIdentifierFieldName(); // e.g. 'id'
 
-        if ($sourceIdentifierReflection === null) {
+        if ($identifierReflection === null) {
             throw new LogicException('Doctrine should use RuntimeReflectionService which never returns null.');
         }
 
-        $proxyIds = [];
+        $uniqueEntities = [];
+        $uninitializedIds = [];
 
-        foreach ($sourceEntities as $sourceEntity) {
-            if ($sourceEntity instanceof Proxy && !$sourceEntity->__isInitialized()) {
-                $proxyIds[] = $sourceIdentifierReflection->getValue($sourceEntity);
+        foreach ($entities as $entity) {
+            $entityId = $identifierReflection->getValue($entity);
+            $entityKey = (string) $entityId;
+            $uniqueEntities[$entityKey] = $entity;
+
+            if ($entity instanceof Proxy && !$entity->__isInitialized()) {
+                $uninitializedIds[$entityKey] = $entityId;
             }
         }
 
-        $batchSize ??= self::PRELOAD_COLLECTION_DEFAULT_BATCH_SIZE;
-
-        foreach (array_chunk($proxyIds, $batchSize) as $idsChunk) {
-            $this->loadEntitiesBy($sourceClassMetadata, $sourceIdentifierReflection->getName(), $idsChunk, $maxFetchJoinSameFieldCount);
+        foreach (array_chunk($uninitializedIds, $batchSize) as $idsChunk) {
+            $this->loadEntitiesBy($classMetadata, $identifierName, $idsChunk, $maxFetchJoinSameFieldCount);
         }
+
+        return array_values($uniqueEntities);
     }
 
     /**
@@ -214,39 +237,23 @@ private function preloadToOne(
     ): array
     {
         $sourcePropertyReflection = $sourceClassMetadata->getReflectionProperty($sourcePropertyName); // e.g. Item::$order reflection
-        $targetIdentifierReflection = $targetClassMetadata->getSingleIdReflectionProperty(); // e.g. Order::$id reflection
+        $targetEntities = [];
 
-        if ($sourcePropertyReflection === null || $targetIdentifierReflection === null) {
+        if ($sourcePropertyReflection === null) {
             throw new LogicException('Doctrine should use RuntimeReflectionService which never returns null.');
         }
 
-        $targetIdentifierName = $targetClassMetadata->getSingleIdentifierFieldName(); // e.g. 'id'
-
-        $batchSize ??= self::BATCH_SIZE;
-
-        $targetEntities = [];
-        $uninitializedIds = [];
-
         foreach ($sourceEntities as $sourceEntity) {
             $targetEntity = $sourcePropertyReflection->getValue($sourceEntity);
 
             if ($targetEntity === null) {
                 continue;
             }
 
-            $targetEntityId = (string) $targetIdentifierReflection->getValue($targetEntity);
-            $targetEntities[$targetEntityId] = $targetEntity;
-
-            if ($targetEntity instanceof Proxy && !$targetEntity->__isInitialized()) {
-                $uninitializedIds[$targetEntityId] = true;
-            }
-        }
-
-        foreach (array_chunk(array_keys($uninitializedIds), $batchSize) as $idsChunk) {
-            $this->loadEntitiesBy($targetClassMetadata, $targetIdentifierName, $idsChunk, $maxFetchJoinSameFieldCount);
+            $targetEntities[] = $targetEntity;
         }
 
-        return array_values($targetEntities);
+        return $this->loadProxies($targetClassMetadata, $targetEntities, $batchSize ?? self::BATCH_SIZE, $maxFetchJoinSameFieldCount);
     }
 
     /**

tests/Lib/QueryLogger.php

@@ -84,29 +84,32 @@ static function (string $query) use (
     /**
      * @return list<array{count: int, query: string}>
      */
-    public function getAggregatedQueries(): array
+    public function getAggregatedQueries(
+        bool $omitSelectedColumns = true,
+        bool $omitDiscriminatorConditions = true,
+        bool $multiline = false,
+    ): array
     {
-        $queries = $this->getQueries();
+        $queries = $this->getQueries(
+            omitSelectedColumns: $omitSelectedColumns,
+            omitDiscriminatorConditions: $omitDiscriminatorConditions,
+            multiline: $multiline,
+        );
 
         $aggregatedQueries = [];
 
         foreach ($queries as $query) {
-            $found = false;
-
             foreach ($aggregatedQueries as &$aggregatedQuery) {
                 if ($aggregatedQuery['query'] === $query) {
                     $aggregatedQuery['count']++;
-                    $found = true;
-                    break;
+                    continue 2;
                 }
             }
 
-            if (!$found) {
-                $aggregatedQueries[] = [
-                    'count' => 1,
-                    'query' => $query,
-                ];
-            }
+            $aggregatedQueries[] = [
+                'count' => 1,
+                'query' => $query,
+            ];
         }
 
         return $aggregatedQueries;