Skip to content

Commit 60ced9d

Browse files
feat(elasticsearch-plugin): avoid redundant reindex on stock movements
Adds two opt-in options that stop stock movements from needlessly re-indexing a product. Both default to the current behavior, so existing installs are unaffected. reindexOnStockMovement: 'always' | 'onStockStatusChange' (default 'always'). In 'onStockStatusChange' the StockMovementEvent subscriber only enqueues an update when the movement flips a variant's inStock or its product's productInStock, so a 50 to 49 change never creates a job. It inspects only the built-in stock booleans, so it is meant for setups without a stock-derived custom mapping. skipUnchangedIndexUpdates: boolean (default false). Before the delete-then-recreate, the indexer builds the product's documents, compares them against what is currently indexed, and skips the write when they are identical. It compares the whole document so it stays correct for any mapping configuration, and it removes the delete-then- recreate window during which a product drops out of search. A full reindex is never skipped. The document comparison reads through the existing SearchClientAdapter.search and the stock check uses ProductVariantService.getSaleableStockLevel, so both features behave the same on Elasticsearch and OpenSearch. No adapter interface or index mapping change. Refs #50.
1 parent d0847a1 commit 60ced9d

7 files changed

Lines changed: 489 additions & 30 deletions

File tree

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
/* eslint-disable @typescript-eslint/no-non-null-assertion */
2+
import { GlobalFlag } from '@vendure/common/lib/generated-types';
3+
import { DefaultJobQueuePlugin, mergeConfig } from '@vendure/core';
4+
import { createTestEnvironment } from '@vendure/testing';
5+
import path from 'path';
6+
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
7+
8+
import { initialData } from '../../../e2e-common/e2e-initial-data';
9+
import { TEST_SETUP_TIMEOUT_MS, testConfig } from '../../../e2e-common/test-config';
10+
import { ElasticsearchPlugin } from '../src/plugin';
11+
12+
import { awaitRunningJobs } from './await-running-jobs';
13+
import { buildAdapterForBackend } from './build-adapter-for-backend';
14+
import { dropElasticIndices } from './e2e-helpers';
15+
import { graphql } from './graphql/graphql-admin';
16+
import { updateProductVariantsDocument } from './graphql/shared-definitions';
17+
import { searchProductsShopDocument } from './graphql/shop-definitions';
18+
19+
const { searchBackend } = require('./constants');
20+
21+
const INDEX_PREFIX = `e2e-skip-unchanged-${searchBackend as string}-`;
22+
23+
const reindexDocument = graphql(`
24+
mutation Reindex {
25+
reindex {
26+
id
27+
}
28+
}
29+
`);
30+
31+
describe(`Elasticsearch plugin skip-unchanged reindex [${searchBackend as string}]`, () => {
32+
const { server, adminClient, shopClient } = createTestEnvironment(
33+
mergeConfig(testConfig(), {
34+
plugins: [
35+
ElasticsearchPlugin.init({
36+
indexPrefix: INDEX_PREFIX,
37+
adapter: buildAdapterForBackend(),
38+
// The two guards under test.
39+
reindexOnStockMovement: 'onStockStatusChange',
40+
skipUnchangedIndexUpdates: true,
41+
}),
42+
DefaultJobQueuePlugin,
43+
],
44+
}),
45+
);
46+
47+
beforeAll(async () => {
48+
await dropElasticIndices(INDEX_PREFIX);
49+
await server.init({
50+
initialData,
51+
productsCsvPath: path.join(__dirname, 'fixtures/e2e-products-full.csv'),
52+
customerCount: 1,
53+
});
54+
await adminClient.asSuperAdmin();
55+
await awaitRunningJobs(adminClient, 20_000, 1000);
56+
await adminClient.query(reindexDocument);
57+
await awaitRunningJobs(adminClient);
58+
}, TEST_SETUP_TIMEOUT_MS);
59+
60+
afterAll(async () => {
61+
await awaitRunningJobs(adminClient);
62+
await server.destroy();
63+
}, TEST_SETUP_TIMEOUT_MS);
64+
65+
async function inStockVariantIds(inStock: boolean): Promise<string[]> {
66+
const result = await shopClient.query(searchProductsShopDocument, {
67+
input: { groupByProduct: false, inStock, take: 200 },
68+
});
69+
return result.search.items.map(i => i.productVariantId);
70+
}
71+
72+
async function setStock(id: string, stockOnHand: number): Promise<void> {
73+
await adminClient.query(updateProductVariantsDocument, {
74+
input: [{ id, trackInventory: GlobalFlag.TRUE, stockOnHand }],
75+
});
76+
await awaitRunningJobs(adminClient);
77+
}
78+
79+
// A variant that is in stock in the seeded data; captured in the first test.
80+
let variantId: string;
81+
82+
it('has in-stock variants after reindex', async () => {
83+
const inStock = await inStockVariantIds(true);
84+
expect(inStock.length).toBeGreaterThan(0);
85+
variantId = inStock[0];
86+
});
87+
88+
it('reflects a stock movement that flips the variant out of stock', async () => {
89+
await setStock(variantId, 0);
90+
expect(await inStockVariantIds(true)).not.toContain(variantId);
91+
expect(await inStockVariantIds(false)).toContain(variantId);
92+
});
93+
94+
it('reflects a stock movement that flips the variant back in stock', async () => {
95+
await setStock(variantId, 100);
96+
expect(await inStockVariantIds(true)).toContain(variantId);
97+
expect(await inStockVariantIds(false)).not.toContain(variantId);
98+
});
99+
100+
it('keeps search correct after a non-boundary stock movement (still in stock)', async () => {
101+
await setStock(variantId, 99);
102+
expect(await inStockVariantIds(true)).toContain(variantId);
103+
expect(await inStockVariantIds(false)).not.toContain(variantId);
104+
});
105+
106+
it('still applies an ordinary variant edit (disabling removes it from search)', async () => {
107+
await adminClient.query(updateProductVariantsDocument, {
108+
input: [{ id: variantId, enabled: false }],
109+
});
110+
await awaitRunningJobs(adminClient);
111+
const allVariantIds = (
112+
await shopClient.query(searchProductsShopDocument, {
113+
input: { groupByProduct: false, take: 200 },
114+
})
115+
).search.items.map(i => i.productVariantId);
116+
expect(allVariantIds).not.toContain(variantId);
117+
118+
// Re-enable so the state is restored for any following assertions.
119+
await adminClient.query(updateProductVariantsDocument, {
120+
input: [{ id: variantId, enabled: true }],
121+
});
122+
await awaitRunningJobs(adminClient);
123+
});
124+
125+
it('rebuilds the whole index on a full reindex (guard is exempt)', async () => {
126+
await adminClient.query(reindexDocument);
127+
await awaitRunningJobs(adminClient);
128+
expect(await inStockVariantIds(true)).toContain(variantId);
129+
});
130+
});

packages/elasticsearch-plugin/src/indexing/elasticsearch-index.service.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,17 @@ export class ElasticsearchIndexService implements OnApplicationBootstrap {
8484
return this.updateIndexQueue.add({ type: 'update-variants', ctx: ctx.serialize(), variantIds }, { ctx });
8585
}
8686

87+
async updateVariantsForStockMovement(ctx: RequestContext, variants: ProductVariant[]) {
88+
if (await this.indexerController.stockMovementWouldChangeIndex(ctx, variants)) {
89+
return this.updateVariants(ctx, variants);
90+
}
91+
Logger.debug(
92+
'Skipping index update for stock movement: indexed stock status unchanged',
93+
loggerCtx,
94+
);
95+
return undefined;
96+
}
97+
8798
deleteProduct(ctx: RequestContext, product: Product) {
8899
return this.updateIndexQueue.add({
89100
type: 'delete-product',
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
import { describe, expect, it } from 'vitest';
2+
3+
import { indexedDocumentsMatch, stableStringify } from './index-diff';
4+
5+
describe('stableStringify', () => {
6+
it('is independent of object key order', () => {
7+
expect(stableStringify({ a: 1, b: 2 })).toBe(stableStringify({ b: 2, a: 1 }));
8+
});
9+
10+
it('is independent of key order in nested objects', () => {
11+
const x = { outer: { a: 1, b: { c: 3, d: 4 } } };
12+
const y = { outer: { b: { d: 4, c: 3 }, a: 1 } };
13+
expect(stableStringify(x)).toBe(stableStringify(y));
14+
});
15+
16+
it('preserves array order (arrays are position-sensitive)', () => {
17+
expect(stableStringify([1, 2, 3])).not.toBe(stableStringify([3, 2, 1]));
18+
expect(stableStringify(['a', 'b'])).toBe(stableStringify(['a', 'b']));
19+
});
20+
21+
it('omits undefined values so they equal an absent key', () => {
22+
expect(stableStringify({ a: 1, b: undefined })).toBe(stableStringify({ a: 1 }));
23+
});
24+
25+
it('distinguishes null from absent/undefined', () => {
26+
expect(stableStringify({ a: null })).not.toBe(stableStringify({}));
27+
expect(stableStringify(null)).toBe('null');
28+
});
29+
30+
it('serializes primitives', () => {
31+
expect(stableStringify('x')).toBe('"x"');
32+
expect(stableStringify(42)).toBe('42');
33+
expect(stableStringify(true)).toBe('true');
34+
});
35+
36+
it('distinguishes number from numeric string', () => {
37+
expect(stableStringify({ n: 2 })).not.toBe(stableStringify({ n: '2' }));
38+
});
39+
});
40+
41+
describe('indexedDocumentsMatch', () => {
42+
const target = new Map<string, unknown>([
43+
['1_10_en', { inStock: true, productInStock: true, sku: 'A', facetIds: ['1', '2'] }],
44+
['1_11_en', { inStock: false, productInStock: true, sku: 'B', facetIds: [] }],
45+
]);
46+
47+
it('returns true when ids and content match (ignoring key order)', () => {
48+
const hits = [
49+
// deliberately reordered keys in _source
50+
{ _id: '1_11_en', _source: { facetIds: [], sku: 'B', productInStock: true, inStock: false } },
51+
{ _id: '1_10_en', _source: { facetIds: ['1', '2'], sku: 'A', inStock: true, productInStock: true } },
52+
];
53+
expect(indexedDocumentsMatch(target, hits)).toBe(true);
54+
});
55+
56+
it('returns false when a document field differs (e.g. inStock flipped)', () => {
57+
const hits = [
58+
{ _id: '1_10_en', _source: { inStock: false, productInStock: true, sku: 'A', facetIds: ['1', '2'] } },
59+
{ _id: '1_11_en', _source: { inStock: false, productInStock: true, sku: 'B', facetIds: [] } },
60+
];
61+
expect(indexedDocumentsMatch(target, hits)).toBe(false);
62+
});
63+
64+
it('returns false when a stock-derived custom field differs (guards the onStockStatusChange caveat)', () => {
65+
const targetWithCustom = new Map<string, unknown>([
66+
['1_10_en', { inStock: true, 'product-stockCount': 5 }],
67+
]);
68+
const hits = [{ _id: '1_10_en', _source: { inStock: true, 'product-stockCount': 4 } }];
69+
expect(indexedDocumentsMatch(targetWithCustom, hits)).toBe(false);
70+
});
71+
72+
it('returns false when the index has an extra document (removal needed)', () => {
73+
const hits = [
74+
{ _id: '1_10_en', _source: { inStock: true, productInStock: true, sku: 'A', facetIds: ['1', '2'] } },
75+
{ _id: '1_11_en', _source: { inStock: false, productInStock: true, sku: 'B', facetIds: [] } },
76+
{ _id: '1_12_en', _source: { inStock: true, productInStock: true, sku: 'C', facetIds: [] } },
77+
];
78+
expect(indexedDocumentsMatch(target, hits)).toBe(false);
79+
});
80+
81+
it('returns false when a target document is missing from the index (needs creating)', () => {
82+
const hits = [
83+
{ _id: '1_10_en', _source: { inStock: true, productInStock: true, sku: 'A', facetIds: ['1', '2'] } },
84+
];
85+
expect(indexedDocumentsMatch(target, hits)).toBe(false);
86+
});
87+
88+
it('returns false when an indexed id is not in the target set', () => {
89+
const hits = [
90+
{ _id: '1_10_en', _source: { inStock: true, productInStock: true, sku: 'A', facetIds: ['1', '2'] } },
91+
{ _id: '9_99_en', _source: { inStock: false, productInStock: true, sku: 'B', facetIds: [] } },
92+
];
93+
expect(indexedDocumentsMatch(target, hits)).toBe(false);
94+
});
95+
});
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/**
2+
* Deterministic JSON serialization (keys sorted, `undefined` omitted, array order preserved).
3+
*/
4+
export function stableStringify(value: any): string {
5+
if (value === null || typeof value !== 'object') {
6+
return JSON.stringify(value) ?? 'null';
7+
}
8+
if (Array.isArray(value)) {
9+
return `[${value.map(v => stableStringify(v)).join(',')}]`;
10+
}
11+
const keys = Object.keys(value)
12+
.filter(k => value[k] !== undefined)
13+
.sort();
14+
return `{${keys.map(k => `${JSON.stringify(k)}:${stableStringify(value[k])}`).join(',')}}`;
15+
}
16+
17+
/**
18+
* True when the freshly-built documents (keyed by `_id`) match what is currently indexed, both in
19+
* the set of ids and in content.
20+
*/
21+
export function indexedDocumentsMatch(
22+
targetById: Map<string, unknown>,
23+
currentHits: Array<{ _id: string; _source: unknown }>,
24+
): boolean {
25+
if (currentHits.length !== targetById.size) {
26+
return false;
27+
}
28+
for (const hit of currentHits) {
29+
if (!targetById.has(hit._id)) {
30+
return false;
31+
}
32+
if (stableStringify(targetById.get(hit._id)) !== stableStringify(hit._source)) {
33+
return false;
34+
}
35+
}
36+
return true;
37+
}

0 commit comments

Comments
 (0)