Skip to content

Commit 45093e1

Browse files
perf(core): Optimize collection variants N+1 queries with batch loading
1 parent a8ea074 commit 45093e1

9 files changed

Lines changed: 772 additions & 24 deletions

File tree

e2e-common/benchmarks/benchmark-list-query.e2e-spec.ts

Lines changed: 606 additions & 0 deletions
Large diffs are not rendered by default.

packages/core/src/api/resolvers/admin/collection.resolver.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { PaginatedList } from '@vendure/common/lib/shared-types';
1818
import { GraphQLResolveInfo } from 'graphql';
1919

2020
import { RequestContextCacheService } from '../../../cache/request-context-cache.service';
21-
import { CacheKey } from '../../../common/constants';
21+
import { CacheKey, COLLECTION_VARIANTS_CACHE_RELATIONS } from '../../../common/constants';
2222
import { UserInputError } from '../../../common/error/errors';
2323
import { Translated } from '../../../common/types/locale-types';
2424
import { CollectionFilter } from '../../../config/catalog/collection-filter';
@@ -66,11 +66,20 @@ export class CollectionResolver {
6666
const collections = await this.collectionService.findAll(ctx, args.options || undefined, relations);
6767
// Cache the variant counts query promise if productVariantCount is requested,
6868
// allowing the DB query to start before the field resolvers are called
69+
const collectionIds = collections.items.map(c => c.id);
6970
if (isFieldInSelection(info, 'productVariantCount')) {
70-
const collectionIds = collections.items.map(c => c.id);
7171
const countsPromise = this.collectionService.getProductVariantCounts(ctx, collectionIds);
7272
this.requestContextCache.set(ctx, CacheKey.CollectionVariantCounts, countsPromise);
7373
}
74+
if (isFieldInSelection(info, 'productVariants')) {
75+
const variantsPromise = this.collectionService.getProductVariantsForCollections(
76+
ctx,
77+
collectionIds,
78+
undefined,
79+
[...COLLECTION_VARIANTS_CACHE_RELATIONS],
80+
);
81+
this.requestContextCache.set(ctx, CacheKey.CollectionVariants, variantsPromise);
82+
}
7483
return collections;
7584
}
7685

packages/core/src/api/resolvers/entity/collection-entity.resolver.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,11 @@ import {
88
import { ID, PaginatedList } from '@vendure/common/lib/shared-types';
99

1010
import { RequestContextCacheService } from '../../../cache/request-context-cache.service';
11-
import { CacheKey } from '../../../common/constants';
11+
import { CacheKey, COLLECTION_VARIANTS_CACHE_RELATIONS } from '../../../common/constants';
1212
import { ListQueryOptions } from '../../../common/types/common-types';
1313
import { Translated } from '../../../common/types/locale-types';
1414
import { CollectionFilter } from '../../../config/catalog/collection-filter';
15+
import { ConfigService } from '../../../config/config.service';
1516
import { Asset, Collection, Product, ProductVariant } from '../../../entity';
1617
import { LocaleStringHydrator } from '../../../service/helpers/locale-string-hydrator/locale-string-hydrator';
1718
import { AssetService } from '../../../service/services/asset.service';
@@ -33,6 +34,7 @@ export class CollectionEntityResolver {
3334
private localeStringHydrator: LocaleStringHydrator,
3435
private configurableOperationCodec: ConfigurableOperationCodec,
3536
private requestContextCache: RequestContextCacheService,
37+
private configService: ConfigService,
3638
) {}
3739

3840
@ResolveField()
@@ -63,6 +65,40 @@ export class CollectionEntityResolver {
6365
@Api() apiType: ApiType,
6466
@Relations({ entity: ProductVariant, omit: ['assets'] }) relations: RelationPaths<ProductVariant>,
6567
): Promise<PaginatedList<Translated<ProductVariant>>> {
68+
const isDefaultOptions = !args.options || Object.keys(args.options).length === 0;
69+
if (isDefaultOptions && apiType === 'admin') {
70+
const cachedVariantsPromise = this.requestContextCache.get<
71+
Promise<Map<string, ProductVariant[]>>
72+
>(ctx, CacheKey.CollectionVariants);
73+
if (cachedVariantsPromise) {
74+
const variantsMap = await cachedVariantsPromise;
75+
const variants = variantsMap.get(String(collection.id));
76+
if (variants) {
77+
// Check if the requested relations are compatible with the cached data.
78+
// The cache was populated with default relations defined by COLLECTION_VARIANTS_CACHE_RELATIONS.
79+
// We can use the cache ONLY if the requested relations are a subset of or equal to the default relations.
80+
const isCacheCompatible = relations.every(rel =>
81+
(COLLECTION_VARIANTS_CACHE_RELATIONS as readonly string[]).includes(rel),
82+
);
83+
84+
if (isCacheCompatible) {
85+
// Cache is compatible, use it.
86+
const { adminListQueryLimit } = this.configService.apiOptions;
87+
const skip = args.options?.skip ?? 0;
88+
const take = args.options?.take ?? adminListQueryLimit;
89+
const items = await this.productVariantService.applyPricesAndTranslateVariants(
90+
ctx,
91+
variants.slice(skip, skip + take),
92+
);
93+
return {
94+
items,
95+
totalItems: variants.length,
96+
};
97+
}
98+
}
99+
}
100+
}
101+
66102
let options: ListQueryOptions<Product> = args.options;
67103
if (apiType === 'shop') {
68104
options = {

packages/core/src/common/constants.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,12 @@ export const CacheKey = {
8585
ActiveTaxZone: (channelId: ID) => `ActiveTaxZone:${channelId}`,
8686
ActiveTaxZone_PPA: (channelId: ID) => `ActiveTaxZone_PPA:${channelId}`,
8787
CollectionVariantCounts: 'CollectionService.getProductVariantCounts',
88+
CollectionVariants: 'CollectionService.getProductVariantsForCollections',
8889
ExhaustedPromotions: (channelId: ID, customerId: ID | undefined) =>
8990
`ExhaustedPromotions:${channelId}:${customerId ?? 'guest'}`,
9091
};
92+
93+
/**
94+
* The default relations used when pre-caching product variants for collections.
95+
*/
96+
export const COLLECTION_VARIANTS_CACHE_RELATIONS = ['taxCategory'] as const;

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,11 @@ export class ListQueryBuilder implements OnApplicationBootstrap {
437437
const { customPropertyPath } = condition.isExistsCondition;
438438
const pathParts = customPropertyPath.split('.');
439439

440-
if (pathParts.length < 2) {
440+
// Only handle single-hop paths (e.g., 'facetValues.id'). Multi-hop paths like
441+
// 'facetValues.term.id' cannot be expressed as a simple EXISTS subquery
442+
// because the column being filtered is on a table reachable only through
443+
// the related entity, not on the related entity itself.
444+
if (pathParts.length !== 2) {
441445
return null;
442446
}
443447

@@ -480,7 +484,10 @@ export class ListQueryBuilder implements OnApplicationBootstrap {
480484
// Helper to escape identifiers for the current database driver (handles PostgreSQL quoting)
481485
const escapeId = (name: string) => mainQb.connection.driver.escape(name);
482486
const escapeTablePath = (path: string) =>
483-
path.split('.').map(segment => mainQb.connection.driver.escape(segment)).join('.');
487+
path
488+
.split('.')
489+
.map(segment => mainQb.connection.driver.escape(segment))
490+
.join('.');
484491

485492
let existsQuery: string;
486493

packages/core/src/service/helpers/list-query-builder/parse-filter-params.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ function getToManyRelationCustomProperties<T extends VendureEntity>(
207207

208208
// Parse the path to get the relation name (e.g., 'facetValues.id' -> 'facetValues')
209209
const pathParts = path.split('.');
210-
if (pathParts.length < 2) {
210+
if (pathParts.length !== 2) {
211211
continue;
212212
}
213213

packages/core/src/service/services/collection.service.ts

Lines changed: 95 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ export class CollectionService implements OnModuleInit {
323323
* This performs a single bulk query to get counts for all provided collection IDs,
324324
* avoiding N+1 query issues when resolving productVariantCount on multiple collections.
325325
*/
326-
async getProductVariantCounts(ctx: RequestContext, collectionIds: ID[]): Promise<Map<ID, number>> {
326+
async getProductVariantCounts(ctx: RequestContext, collectionIds: ID[]): Promise<Map<string, number>> {
327327
if (collectionIds.length === 0) {
328328
return new Map();
329329
}
@@ -344,9 +344,7 @@ export class CollectionService implements OnModuleInit {
344344
.groupBy('collection.id')
345345
.getRawMany<{ collectionId: string; count: string }>();
346346

347-
const countMap = new Map<ID, number>();
348-
// Normalize IDs to strings to ensure consistent Map key types,
349-
// since raw query results return collectionId as string
347+
const countMap = new Map<string, number>();
350348
for (const id of collectionIds) {
351349
countMap.set(String(id), 0);
352350
}
@@ -490,7 +488,7 @@ export class CollectionService implements OnModuleInit {
490488
ctx: RequestContext,
491489
input: PreviewCollectionVariantsInput,
492490
options?: ListQueryOptions<ProductVariant>,
493-
relations?: RelationPaths<Collection>,
491+
relations?: RelationPaths<ProductVariant>,
494492
): Promise<PaginatedList<ProductVariant>> {
495493
const applicableFilters = this.getCollectionFiltersFromInput(input);
496494
if (input.parentId && input.inheritFilters) {
@@ -503,13 +501,18 @@ export class CollectionService implements OnModuleInit {
503501
);
504502
applicableFilters.push(...parentFilters, ...ancestorFilters);
505503
}
506-
let qb = this.listQueryBuilder.build(ProductVariant, options, {
507-
relations: relations ?? ['taxCategory'],
508-
channelId: ctx.channelId,
509-
where: { deletedAt: IsNull() },
510-
ctx,
511-
entityAlias: 'productVariant',
512-
});
504+
505+
let qb = this.listQueryBuilder.build(
506+
ProductVariant,
507+
options || { take: undefined, skip: undefined },
508+
{
509+
relations: relations ?? ['taxCategory'],
510+
channelId: ctx.channelId,
511+
where: { deletedAt: IsNull() },
512+
ctx,
513+
entityAlias: 'productVariant',
514+
},
515+
);
513516

514517
const { collectionFilters } = this.configService.catalogOptions;
515518
for (const filterType of collectionFilters) {
@@ -713,6 +716,86 @@ export class CollectionService implements OnModuleInit {
713716
return filters;
714717
}
715718

719+
/**
720+
* Returns a Map of collection IDs to their associated product variants.
721+
* This performs a single bulk query to get all variants for all provided collection IDs,
722+
* avoiding N+1 query issues when resolving variants on multiple collections.
723+
*/
724+
async getProductVariantsForCollections(
725+
ctx: RequestContext,
726+
collectionIds: ID[],
727+
options?: ListQueryOptions<ProductVariant>,
728+
relations?: RelationPaths<ProductVariant>,
729+
): Promise<Map<string, ProductVariant[]>> {
730+
if (collectionIds.length === 0) {
731+
return new Map();
732+
}
733+
734+
// Note: This method intentionally returns ALL matching variants without applying
735+
// the default admin/shop list query limit. This is safe because it is used only
736+
// for batch pre-caching in the admin collection list, where the number of requested
737+
// collections is small and controlled by the UI page size.
738+
//
739+
// Risk: For catalogs with a very large number of product variants per collection,
740+
// this query can become expensive. If that becomes a real issue, per-collection
741+
// pagination (Option B) should be implemented in a follow-up PR.
742+
const qb = this.listQueryBuilder.build(ProductVariant, options ?? {}, {
743+
relations: relations ?? ['taxCategory'],
744+
channelId: ctx.channelId,
745+
where: { deletedAt: IsNull() },
746+
ctx,
747+
entityAlias: 'productVariant',
748+
});
749+
750+
if (options?.take === undefined) {
751+
qb.take(undefined);
752+
}
753+
if (options?.skip === undefined) {
754+
qb.skip(undefined);
755+
}
756+
757+
// We explicitly join with the product to ensure we filter out soft-deleted products,
758+
// matching the behavior of other collection-related variant queries.
759+
qb.innerJoin('productvariant.collections', 'collection', 'collection.id IN (:...collectionIds)', {
760+
collectionIds,
761+
})
762+
.andWhere('product.deletedAt IS NULL')
763+
.addSelect('collection.id', 'collectionId')
764+
// Explicitly select `productvariant.id` so we can reliably read the join result key
765+
// without depending on TypeORM's auto-generated aliases for raw queries.
766+
.addSelect('productvariant.id', 'variantId');
767+
768+
const { entities: allVariants, raw: rawResults } = await qb.getRawAndEntities();
769+
770+
const variantsById = new Map<string, ProductVariant>(allVariants.map(v => [String(v.id), v]));
771+
772+
const variantsByCollectionId = new Map<string, ProductVariant[]>();
773+
const seenInCollection = new Map<string, Set<string>>();
774+
775+
for (const id of collectionIds) {
776+
const idStr = String(id);
777+
variantsByCollectionId.set(idStr, []);
778+
seenInCollection.set(idStr, new Set());
779+
}
780+
781+
for (const raw of rawResults) {
782+
const variantId = String(raw.variantId);
783+
const collectionId = String(raw.collectionId);
784+
const variant = variantsById.get(variantId);
785+
786+
if (variant) {
787+
const collectionVariants = variantsByCollectionId.get(collectionId);
788+
const seenSet = seenInCollection.get(collectionId);
789+
if (collectionVariants && seenSet && !seenSet.has(variantId)) {
790+
seenSet.add(variantId);
791+
collectionVariants.push(variant);
792+
}
793+
}
794+
}
795+
796+
return variantsByCollectionId;
797+
}
798+
716799
private chunkArray = <T>(array: T[], chunkSize: number): T[][] => {
717800
const results = [];
718801
for (let i = 0; i < array.length; i += chunkSize) {

packages/core/src/service/services/product-variant.service.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -766,11 +766,12 @@ export class ProductVariantService {
766766
}
767767

768768
/**
769-
* @description
770-
* Given an array of ProductVariants from the database, this method will apply the correct price and tax
771-
* and translate each item.
772-
*/
773-
private async applyPricesAndTranslateVariants(
769+
/**
770+
* @description
771+
* Given an array of ProductVariants from the database, this method will apply the correct price and tax
772+
* and translate each item.
773+
*/
774+
async applyPricesAndTranslateVariants(
774775
ctx: RequestContext,
775776
variants: ProductVariant[],
776777
): Promise<Array<Translated<ProductVariant>>> {

packages/core/src/service/services/tax-rate.service.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ export class TaxRateService {
8282
}
8383
if (hasCategoryIdFilter) {
8484
effectiveRelations.push('category');
85-
customPropertyMap.zoneId = 'category.id';
85+
customPropertyMap.categoryId = 'category.id';
8686
}
8787
return this.listQueryBuilder
8888
.build(TaxRate, options, {

0 commit comments

Comments
 (0)