Browse Source

fix(core): Fix `getMany()` method with ListQueryBuilder

Fixes #1586
Michael Bromley 3 years ago
parent
commit
6be93b8851

+ 16 - 0
packages/core/e2e/fixtures/test-plugins/list-query-plugin.ts

@@ -134,6 +134,21 @@ export class ListQueryResolver {
                 };
             });
     }
+
+    @Query()
+    testEntitiesGetMany(@Ctx() ctx: RequestContext, @Args() args: any) {
+        return this.listQueryBuilder
+            .build(TestEntity, args.options, { ctx, relations: ['prices'] })
+            .getMany()
+            .then(items => {
+                for (const item of items) {
+                    if (item.prices && item.prices.length) {
+                        item.activePrice = item.prices[0].price;
+                    }
+                }
+                return items.map(i => translateDeep(i, ctx.languageCode));
+            });
+    }
 }
 
 const apiExtensions = gql`
@@ -159,6 +174,7 @@ const apiExtensions = gql`
 
     extend type Query {
         testEntities(options: TestEntityListOptions): TestEntityList!
+        testEntitiesGetMany(options: TestEntityListOptions): [TestEntity!]!
     }
 
     input TestEntityListOptions

+ 20 - 0
packages/core/e2e/list-query-builder.e2e-spec.ts

@@ -954,6 +954,14 @@ describe('ListQueryBuilder', () => {
             expect(testEntities.items.map((x: any) => x.name)).toEqual(['egg', 'fahrrad']);
         });
     });
+
+    // https://github.com/vendure-ecommerce/vendure/issues/1586
+    it('using the getMany() of the resulting QueryBuilder', async () => {
+        const { testEntitiesGetMany } = await adminClient.query(GET_ARRAY_LIST, {});
+        expect(testEntitiesGetMany.sort((a: any, b: any) => a.id - b.id).map((x: any) => x.price)).toEqual([
+            11, 9, 22, 14, 13, 33,
+        ]);
+    });
 });
 
 const GET_LIST = gql`
@@ -969,3 +977,15 @@ const GET_LIST = gql`
         }
     }
 `;
+
+const GET_ARRAY_LIST = gql`
+    query GetTestEntitiesArray($options: TestEntityListOptions) {
+        testEntitiesGetMany(options: $options) {
+            id
+            label
+            name
+            date
+            price
+        }
+    }
+`;

+ 76 - 38
packages/core/src/service/helpers/list-query-builder/list-query-builder.ts

@@ -262,6 +262,7 @@ export class ListQueryBuilder implements OnApplicationBootstrap {
 
         qb.orderBy(sort);
         this.optimizeGetManyAndCountMethod(qb, repo, extendedOptions, minimumRequiredRelations);
+        this.optimizeGetManyMethod(qb, repo, extendedOptions, minimumRequiredRelations);
         return qb;
     }
 
@@ -351,49 +352,86 @@ export class ListQueryBuilder implements OnApplicationBootstrap {
                 // return the regular result.
                 return [entities, count];
             }
-            const entityMap = new Map(entities.map(e => [e.id, e]));
-            const entitiesIds = entities.map(({ id }) => id);
-
-            const splitRelations = relations.map(r => r.split('.'));
-            const groupedRelationsMap = new Map<string, string[]>();
-            for (const relationParts of splitRelations) {
-                const group = groupedRelationsMap.get(relationParts[0]);
-                if (group) {
-                    group.push(relationParts.join('.'));
-                } else {
-                    groupedRelationsMap.set(relationParts[0], [relationParts.join('.')]);
-                }
+            const result = await this.parallelLoadRelations(entities, relations, alreadyJoined, repo);
+            return [result, count];
+        };
+    }
+    /**
+     * @description
+     * This will monkey-patch the `getMany()` method in order to implement a more efficient
+     * parallel-query based approach to joining multiple relations. This is loosely based on the
+     * solution outlined here: https://github.com/typeorm/typeorm/issues/3857#issuecomment-633006643
+     *
+     * TODO: When upgrading to TypeORM v0.3+, this will likely become redundant due to the new
+     * `relationLoadStrategy` feature.
+     */
+    private optimizeGetManyMethod<T extends VendureEntity>(
+        qb: SelectQueryBuilder<T>,
+        repo: Repository<T>,
+        extendedOptions: ExtendedListQueryOptions<T>,
+        alreadyJoined: string[],
+    ) {
+        const originalGetMany = qb.getMany.bind(qb);
+        qb.getMany = async () => {
+            const relations = unique(extendedOptions.relations ?? []);
+            const entities = await originalGetMany();
+            if (relations == null || alreadyJoined.sort().join() === relations?.sort().join()) {
+                // No further relations need to be joined, so we just
+                // return the regular result.
+                return entities;
             }
+            return this.parallelLoadRelations(entities, relations, alreadyJoined, repo);
+        };
+    }
 
-            // If the extendedOptions includes relations that were already joined, then
-            // we ignore those now so as not to do the work of joining twice.
-            for (const tableName of alreadyJoined) {
-                if (groupedRelationsMap.get(tableName)?.length === 1) {
-                    groupedRelationsMap.delete(tableName);
-                }
+    private async parallelLoadRelations<T extends VendureEntity>(
+        entities: T[],
+        relations: string[],
+        alreadyJoined: string[],
+        repo: Repository<T>,
+    ): Promise<T[]> {
+        const entityMap = new Map(entities.map(e => [e.id, e]));
+        const entitiesIds = entities.map(({ id }) => id);
+
+        const splitRelations = relations.map(r => r.split('.'));
+        const groupedRelationsMap = new Map<string, string[]>();
+        for (const relationParts of splitRelations) {
+            const group = groupedRelationsMap.get(relationParts[0]);
+            if (group) {
+                group.push(relationParts.join('.'));
+            } else {
+                groupedRelationsMap.set(relationParts[0], [relationParts.join('.')]);
+            }
+        }
+
+        // If the extendedOptions includes relations that were already joined, then
+        // we ignore those now so as not to do the work of joining twice.
+        for (const tableName of alreadyJoined) {
+            if (groupedRelationsMap.get(tableName)?.length === 1) {
+                groupedRelationsMap.delete(tableName);
             }
+        }
 
-            const entitiesIdsWithRelations = await Promise.all(
-                Array.from(groupedRelationsMap.values())?.map(relationPaths => {
-                    return repo
-                        .findByIds(entitiesIds, {
-                            select: ['id'],
-                            relations: relationPaths,
-                            loadEagerRelations: false,
-                        })
-                        .then(results =>
-                            results.map(r => ({ relation: relationPaths[0] as keyof T, entity: r })),
-                        );
-                }),
-            ).then(all => all.flat());
-            for (const entry of entitiesIdsWithRelations) {
-                const finalEntity = entityMap.get(entry.entity.id);
-                if (finalEntity) {
-                    finalEntity[entry.relation] = entry.entity[entry.relation];
-                }
+        const entitiesIdsWithRelations = await Promise.all(
+            Array.from(groupedRelationsMap.values())?.map(relationPaths => {
+                return repo
+                    .findByIds(entitiesIds, {
+                        select: ['id'],
+                        relations: relationPaths,
+                        loadEagerRelations: false,
+                    })
+                    .then(results =>
+                        results.map(r => ({ relation: relationPaths[0] as keyof T, entity: r })),
+                    );
+            }),
+        ).then(all => all.flat());
+        for (const entry of entitiesIdsWithRelations) {
+            const finalEntity = entityMap.get(entry.entity.id);
+            if (finalEntity) {
+                finalEntity[entry.relation] = entry.entity[entry.relation];
             }
-            return [Array.from(entityMap.values()), count];
-        };
+        }
+        return Array.from(entityMap.values());
     }
 
     /**