perf(core): Improved performance of validateVariantOptionIds (#337)

These changes reduce the amount of data loaded into memory which hopefully prevents some server crashes. #328. Also added the eager option for allowing performance gains in certain queries.
vendure-ecommerce · May 11, 2020 · 7d19b9c · 7d19b9c
1 parent 1fb5fb4
commit 7d19b9c
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 14 deletions.
diff --git a/packages/core/src/service/helpers/utils/channel-aware-orm-utils.ts b/packages/core/src/service/helpers/utils/channel-aware-orm-utils.ts
@@ -14,11 +14,14 @@ export function findByIdsInChannel<T extends ChannelAware | VendureEntity>(
     ids: ID[],
     channelId: ID,
     findOptions?: FindManyOptions<T>,
+    eager = true,
 ) {
     const qb = connection.getRepository(entity).createQueryBuilder('product');
     FindOptionsUtils.applyFindManyOptionsOrConditionsToQueryBuilder(qb, findOptions);
-    // tslint:disable-next-line:no-non-null-assertion
-    FindOptionsUtils.joinEagerRelations(qb, qb.alias, qb.expressionMap.mainAlias!.metadata);
+    if (eager) {
+        // tslint:disable-next-line:no-non-null-assertion
+        FindOptionsUtils.joinEagerRelations(qb, qb.alias, qb.expressionMap.mainAlias!.metadata);
+    }
     return qb
         .leftJoin('product.channels', 'channel')
         .andWhere('channel.id = :channelId', { channelId })
@@ -35,11 +38,14 @@ export function findOneInChannel<T extends ChannelAware | VendureEntity>(
     id: ID,
     channelId: ID,
     findOptions?: FindManyOptions<T>,
+    eager = true,
 ) {
     const qb = connection.getRepository(entity).createQueryBuilder('product');
     FindOptionsUtils.applyFindManyOptionsOrConditionsToQueryBuilder(qb, findOptions);
-    // tslint:disable-next-line:no-non-null-assertion
-    FindOptionsUtils.joinEagerRelations(qb, qb.alias, qb.expressionMap.mainAlias!.metadata);
+    if (eager) {
+        // tslint:disable-next-line:no-non-null-assertion
+        FindOptionsUtils.joinEagerRelations(qb, qb.alias, qb.expressionMap.mainAlias!.metadata);
+    }
     return qb
         .leftJoin('product.channels', 'channel')
         .andWhere('product.id = :id', { id })

diff --git a/packages/core/src/service/helpers/utils/get-entity-or-throw.ts b/packages/core/src/service/helpers/utils/get-entity-or-throw.ts
@@ -24,17 +24,26 @@ export async function getEntityOrThrow<T extends VendureEntity | ChannelAware>(
     id: ID,
     channelId: ID,
     findOptions?: FindOneOptions<T>,
+    eager?: boolean,
 ): Promise<T>;
 export async function getEntityOrThrow<T extends VendureEntity>(
     connection: Connection,
     entityType: Type<T>,
     id: ID,
     findOptionsOrChannelId?: FindOneOptions<T> | ID,
     maybeFindOptions?: FindOneOptions<T>,
+    eager?: boolean,
 ): Promise<T> {
     let entity: T | undefined;
     if (isId(findOptionsOrChannelId)) {
-        entity = await findOneInChannel(connection, entityType, id, findOptionsOrChannelId, maybeFindOptions);
+        entity = await findOneInChannel(
+            connection,
+            entityType,
+            id,
+            findOptionsOrChannelId,
+            maybeFindOptions,
+            eager,
+        );
     } else {
         entity = await connection.getRepository(entityType).findOne(id, findOptionsOrChannelId);
     }

diff --git a/packages/core/src/service/services/product-variant.service.ts b/packages/core/src/service/services/product-variant.service.ts
@@ -362,25 +362,50 @@ export class ProductVariantService {
     }
 
     private async validateVariantOptionIds(ctx: RequestContext, input: CreateProductVariantInput) {
-        const product = await getEntityOrThrow(this.connection, Product, input.productId, ctx.channelId, {
-            relations: ['optionGroups', 'optionGroups.options', 'variants', 'variants.options'],
-        });
-        const optionIds = [...(input.optionIds || [])];
+        // this could be done with less queries but depending on the data, node will crash
+        // https://github.com/vendure-ecommerce/vendure/issues/328
+        const optionGroups = (
+            await getEntityOrThrow(
+                this.connection,
+                Product,
+                input.productId,
+                ctx.channelId,
+                {
+                    relations: ['optionGroups', 'optionGroups.options'],
+                },
+                false,
+            )
+        ).optionGroups;
 
-        if (optionIds.length !== product.optionGroups.length) {
-            this.throwIncompatibleOptionsError(product.optionGroups);
+        const optionIds = input.optionIds || [];
+
+        if (optionIds.length !== optionGroups.length) {
+            this.throwIncompatibleOptionsError(optionGroups);
         }
         if (
             !samplesEach(
                 optionIds,
-                product.optionGroups.map(g => g.options.map(o => o.id)),
+                optionGroups.map((g) => g.options.map((o) => o.id)),
             )
         ) {
-            this.throwIncompatibleOptionsError(product.optionGroups);
+            this.throwIncompatibleOptionsError(optionGroups);
         }
+
+        const product = await getEntityOrThrow(
+            this.connection,
+            Product,
+            input.productId,
+            ctx.channelId,
+            {
+                relations: ['variants', 'variants.options'],
+            },
+            false,
+        );
+
+        const inputOptionIds = this.sortJoin(optionIds, ',');
+
         product.variants.forEach(variant => {
             const variantOptionIds = this.sortJoin(variant.options, ',', 'id');
-            const inputOptionIds = this.sortJoin(input.optionIds || [], ',');
             if (variantOptionIds === inputOptionIds) {
                 throw new UserInputError('error.product-variant-options-combination-already-exists', {
                     optionNames: this.sortJoin(variant.options, ', ', 'code'),