Skip to content

Commit 65f831a

Browse files
fix(core): Optimize collection variants N+1 and persist catalog filters
1 parent a8ea074 commit 65f831a

19 files changed

Lines changed: 505 additions & 36 deletions

File tree

.gemini/settings.json

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
{
2+
"mcpServers": {
3+
"fetch": {
4+
"command": "https://raw.githubusercontent.com/modelcontextprotocol/servers/main/src/fetch/index.ts",
5+
"args": []
6+
},
7+
"sqlite": {
8+
"command": "https://raw.githubusercontent.com/modelcontextprotocol/servers/main/src/sqlite/index.ts",
9+
"args": []
10+
},
11+
"elasticsearch": {
12+
"command": "https://raw.githubusercontent.com/modelcontextprotocol/servers/main/src/elasticsearch/index.ts",
13+
"args": []
14+
},
15+
"puppeteer": {
16+
"command": "npx",
17+
"args": [
18+
"-y",
19+
"@modelcontextprotocol/server-puppeteer"
20+
]
21+
},
22+
"docker": {
23+
"command": "npx",
24+
"args": [
25+
"-y",
26+
"@modelcontextprotocol/server-docker"
27+
]
28+
}
29+
}
30+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
{
2+
"mcpServers": {
3+
"fetch": {
4+
"command": "https://raw.githubusercontent.com/modelcontextprotocol/servers/main/src/fetch/index.ts",
5+
"args": []
6+
},
7+
"sqlite": {
8+
"command": "https://raw.githubusercontent.com/modelcontextprotocol/servers/main/src/sqlite/index.ts",
9+
"args": []
10+
},
11+
"elasticsearch": {
12+
"command": "https://raw.githubusercontent.com/modelcontextprotocol/servers/main/src/elasticsearch/index.ts",
13+
"args": []
14+
},
15+
"puppeteer": {
16+
"command": "npx",
17+
"args": [
18+
"-y",
19+
"@modelcontextprotocol/server-puppeteer"
20+
]
21+
},
22+
"docker": {
23+
"command": "npx",
24+
"args": [
25+
"-y",
26+
"@modelcontextprotocol/server-docker"
27+
]
28+
}
29+
}
30+
}

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
.env
1212
lerna-debug.log
1313
dist
14+
lib/
15+
package/
1416
e2e/__data__/*
1517
docs/resources/_gen/*
1618
docs/static/main.js*
Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
/* eslint-disable no-console */
2+
import { ID, FacetValue, VendureConfig } from '@vendure/core';
3+
import { createTestEnvironment } from '@vendure/testing';
4+
import { gql } from 'graphql-tag';
5+
import path from 'path';
6+
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
7+
8+
import { initialData } from '../e2e-initial-data';
9+
import { testConfig } from '../test-config';
10+
11+
describe('ListQueryBuilder Optimization Benchmark', () => {
12+
let capturedQueries: string[] = [];
13+
const baseConfig = testConfig();
14+
15+
const benchmarkConfig: VendureConfig = {
16+
...baseConfig,
17+
customFields: {
18+
Product: [
19+
{
20+
name: 'testManyToMany',
21+
type: 'relation',
22+
entity: FacetValue,
23+
graphQLType: 'FacetValue',
24+
list: true,
25+
},
26+
{
27+
name: 'testManyToOne',
28+
type: 'relation',
29+
entity: FacetValue,
30+
graphQLType: 'FacetValue',
31+
list: false,
32+
},
33+
],
34+
},
35+
dbConnectionOptions: {
36+
...baseConfig.dbConnectionOptions,
37+
logging: ['query'],
38+
logger: {
39+
logQuery(query: string) {
40+
if (
41+
query.includes('SELECT') &&
42+
(query.includes('"product"') || query.includes('`product`'))
43+
) {
44+
capturedQueries.push(query);
45+
}
46+
},
47+
logQueryError: (error: string) => console.error(error),
48+
logQuerySlow: (time: number, query: string) => console.warn(query, time),
49+
logSchemaBuild: () => {
50+
/* no-op */
51+
},
52+
logMigration: () => {
53+
/* no-op */
54+
},
55+
log: () => {
56+
/* no-op */
57+
},
58+
} as any,
59+
},
60+
};
61+
62+
const { server, adminClient } = createTestEnvironment(benchmarkConfig);
63+
64+
beforeAll(async () => {
65+
await server.init({
66+
initialData,
67+
productsCsvPath: path.join(__dirname, '../../packages/core/e2e/fixtures/e2e-products-minimal.csv'),
68+
customerCount: 1,
69+
});
70+
await adminClient.asSuperAdmin();
71+
}, 240000);
72+
73+
afterAll(async () => {
74+
await server.destroy();
75+
});
76+
77+
it('uses multiple EXISTS for ManyToMany custom field relation AND filter', async () => {
78+
const GET_PRODUCTS = gql`
79+
query GetProducts($options: ProductListOptions) {
80+
products(options: $options) {
81+
items {
82+
id
83+
}
84+
totalItems
85+
}
86+
}
87+
`;
88+
89+
capturedQueries = [];
90+
91+
await adminClient.query(GET_PRODUCTS, {
92+
options: {
93+
filter: {
94+
_and: [
95+
{ testManyToManyId: { eq: '1' } },
96+
{ testManyToManyId: { eq: '3' } }
97+
],
98+
},
99+
},
100+
});
101+
102+
const lastQuery = capturedQueries.slice().reverse().find(
103+
q => q.includes('WHERE') && q.includes('testManyToMany') && !/SELECT\s+COUNT/i.test(q),
104+
);
105+
expect(lastQuery, 'Should have a query with WHERE and testManyToMany').toBeDefined();
106+
if (lastQuery) {
107+
const existsCount = (lastQuery.match(/EXISTS/g) || []).length;
108+
expect(existsCount).toBe(2);
109+
// Verify no JOIN was added for the filter
110+
expect(lastQuery).not.toContain('LEFT JOIN');
111+
}
112+
113+
// Verify that the query executes and returns a valid paginated result
114+
// Even if empty, totalItems should be a number.
115+
const { products } = await adminClient.query(GET_PRODUCTS, {
116+
options: { filter: { testManyToManyId: { eq: '1' } } }
117+
});
118+
expect(products.totalItems).toBeDefined();
119+
expect(typeof products.totalItems).toBe('number');
120+
});
121+
122+
it('uses EXISTS for ManyToOne custom field relation when filtering (optimized)', async () => {
123+
const GET_PRODUCTS = gql`
124+
query GetProducts($options: ProductListOptions) {
125+
products(options: $options) {
126+
items {
127+
id
128+
}
129+
totalItems
130+
}
131+
}
132+
`;
133+
134+
capturedQueries = [];
135+
136+
await adminClient.query(GET_PRODUCTS, {
137+
options: {
138+
filter: {
139+
testManyToOneId: { eq: '1' },
140+
},
141+
},
142+
});
143+
144+
const lastQuery = capturedQueries.slice().reverse().find(
145+
q => q.includes('WHERE') && q.includes('testManyToOne') && !/SELECT\s+COUNT/i.test(q),
146+
);
147+
expect(lastQuery, 'Should have a query with WHERE and testManyToOne').toBeDefined();
148+
if (lastQuery) {
149+
const existsCount = (lastQuery.match(/EXISTS/g) || []).length;
150+
expect(existsCount).toBe(1);
151+
// Verify no JOIN was added for the ManyToOne filter (optimization)
152+
expect(lastQuery).not.toContain('LEFT JOIN');
153+
}
154+
});
155+
156+
it('uses JOIN for ManyToOne custom field relation when sorting', async () => {
157+
const GET_PRODUCTS = gql`
158+
query GetProducts($options: ProductListOptions) {
159+
products(options: $options) {
160+
items {
161+
id
162+
}
163+
}
164+
}
165+
`;
166+
167+
capturedQueries = [];
168+
169+
await adminClient.query(GET_PRODUCTS, {
170+
options: {
171+
sort: {
172+
testManyToOneId: 'ASC',
173+
},
174+
},
175+
});
176+
177+
const lastQuery = capturedQueries.slice().reverse().find(
178+
q => q.includes('testManyToOne') && !/SELECT\s+COUNT/i.test(q),
179+
);
180+
expect(lastQuery, 'Should have a query with testManyToOne').toBeDefined();
181+
if (lastQuery) {
182+
// Verify JOIN was added for sorting
183+
expect(lastQuery).toContain('LEFT JOIN');
184+
// EXISTS is not used for sorting
185+
const existsCount = (lastQuery.match(/EXISTS/g) || []).length;
186+
expect(existsCount).toBe(0);
187+
}
188+
});
189+
});

packages/core/src/api/middleware/auth-guard.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { Reflector } from '@nestjs/core';
33
import { Permission } from '@vendure/common/lib/generated-types';
44
import { Request, Response } from 'express';
55
import { GraphQLResolveInfo } from 'graphql';
6-
import ms, { type StringValue } from 'ms';
6+
import ms from 'ms';
77

88
import { ForbiddenError } from '../../common/error/errors';
99
import { API_KEY_AUTH_STRATEGY_NAME } from '../../config';
@@ -212,7 +212,7 @@ export class AuthGuard implements CanActivate {
212212
const lastUsedThreshold = new Date(
213213
Date.now() -
214214
(typeof strategy.lastUsedAtUpdateInterval === 'string'
215-
? ms(strategy.lastUsedAtUpdateInterval as StringValue)
215+
? ms(strategy.lastUsedAtUpdateInterval)
216216
: strategy.lastUsedAtUpdateInterval),
217217
);
218218
if (!apiKey.lastUsedAt || apiKey.lastUsedAt < lastUsedThreshold) {

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;

0 commit comments

Comments
 (0)