Skip to content

fix(core): Invalidate stock location channel id cache correctly #5087

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ryandiginomad wants to merge 1 commit into
base: master
Choose a base branch
from
+276 −19
Open

fix(core): Invalidate stock location channel id cache correctly #5087

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 88 additions & 15 deletions packages/core/e2e/stock-location.e2e-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@ import {
createProductDocument,
createProductVariantsDocument,
} from './graphql/shared-definitions';
import { localUpdatedOrderFragment, testOrderFragment } from './graphql/shop-definitions';
import {
getProductWithStockLevelDocument,
localUpdatedOrderFragment,
testOrderFragment,
} from './graphql/shop-definitions';

describe('Stock location', () => {
const defaultStockLocationId = 'T_1';
Expand Down Expand Up @@ -390,15 +394,12 @@ describe('Stock location', () => {
}
// Create a StockLocation in Channel A
adminClient.setChannelToken(CHANNEL_A_TOKEN);
const { createStockLocation } = await adminClient.query(
testCreateStockLocationDocument,
{
input: {
name: 'Channel-A Location',
description: 'Belongs to Channel A only',
},
const { createStockLocation } = await adminClient.query(testCreateStockLocationDocument, {
input: {
name: 'Channel-A Location',
description: 'Belongs to Channel A only',
},
);
});
targetLocationId = createStockLocation.id;
});

Expand All @@ -421,15 +422,87 @@ describe('Stock location', () => {

// Verify the original entity in Channel A is completely unchanged.
adminClient.setChannelToken(CHANNEL_A_TOKEN);
const { stockLocations } = await adminClient.query(
testGetStockLocationsListDocument,
);
const target = stockLocations.items.find(
(sl: any) => sl.id === targetLocationId,
);
const { stockLocations } = await adminClient.query(testGetStockLocationsListDocument);
const target = stockLocations.items.find((sl: any) => sl.id === targetLocationId);
expect(target?.name).toBe('Channel-A Location');
expect(target?.description).toBe('Belongs to Channel A only');
});
});

// Repro for #3324: the MultiChannelStockLocationStrategy caches each StockLocation's
// channel ids for 7 days. Assigning a StockLocation to a Channel must invalidate that
// cache entry, otherwise the new channel sees no saleable stock until the entry expires
// or the server restarts.
describe('channelId cache invalidation (#3324)', () => {
const CACHE_TEST_CHANNEL_TOKEN = 'stock-loc-cache-test';
let cacheTestChannelId: string;
let cacheTestLocationId: string;

beforeAll(async () => {
adminClient.setChannelToken(E2E_DEFAULT_CHANNEL_TOKEN);
const { createStockLocation } = await adminClient.query(testCreateStockLocationDocument, {
input: {
name: 'Cache test location',
},
});
cacheTestLocationId = createStockLocation.id;
await adminClient.query(testSetStockLevelInLocationDocument, {
input: {
id: 'T_1',
stockLevels: [
{
stockLocationId: cacheTestLocationId,
stockOnHand: 10,
},
],
},
});
const { createChannel } = await adminClient.query(createChannelDocument, {
input: {
code: 'cache-test-channel',
token: CACHE_TEST_CHANNEL_TOKEN,
defaultLanguageCode: LanguageCode.en,
currencyCode: CurrencyCode.GBP,
pricesIncludeTax: true,
defaultShippingZoneId: 'T_1',
defaultTaxZoneId: 'T_1',
},
});
channelGuard.assertSuccess(createChannel);
cacheTestChannelId = createChannel.id;
await adminClient.query(assignProductToChannelDocument, {
input: {
channelId: cacheTestChannelId,
productIds: ['T_1'],
},
});
});

afterAll(() => {
adminClient.setChannelToken(E2E_DEFAULT_CHANNEL_TOKEN);
shopClient.setChannelToken(E2E_DEFAULT_CHANNEL_TOKEN);
});

it('saleable stock becomes visible once the StockLocation is assigned to the channel', async () => {

@Refaat-alktifan Refaat-alktifan Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This proves the assignment direction only. I think the removal direction deserves an equivalent e2e test, since stale positive membership can keep a removed location contributing stock in the channel it was removed from, which is the higher risk case. The unit spec constructs a removed event, but it does not drive the real removeStockLocationsFromChannel path through the Shop projection.

A sibling case could warm IN_STOCK in the channel, remove the location, assert the relation is gone while the source stock stays positive, then poll the Shop query with a deadline until it returns OUT_OF_STOCK.

// Populate the strategy's channelId cache for the location while it is
// not yet assigned to the new channel
shopClient.setChannelToken(CACHE_TEST_CHANNEL_TOKEN);
const before = await shopClient.query(getProductWithStockLevelDocument, { id: 'T_1' });
expect(before.product?.variants[0].stockLevel).toBe('OUT_OF_STOCK');

adminClient.setChannelToken(E2E_DEFAULT_CHANNEL_TOKEN);
await adminClient.query(testAssignStockLocationToChannelDocument, {
input: {
stockLocationIds: [cacheTestLocationId],
channelId: cacheTestChannelId,
},
});
// Wait a bit for the ChangeChannelEvent subscriber to invalidate the cache
await new Promise(resolve => setTimeout(resolve, 100));
Comment on lines +500 to +501

@coderabbitai coderabbitai Bot Aug 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Replace the fixed delay with bounded polling.

Line 501 assumes that cache invalidation completes within 100 ms. Under CI load, the event handler can complete later and cause an intermittent failure. Poll the shop query until it returns IN_STOCK, with a bounded timeout.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/e2e/stock-location.e2e-spec.ts` around lines 500 - 501, Replace
the fixed 100 ms delay in the stock-location test with bounded polling of the
shop query, repeatedly checking until the result is IN_STOCK. Add a finite
timeout or retry limit, and fail clearly if the expected state is not observed
within that bound.


const after = await shopClient.query(getProductWithStockLevelDocument, { id: 'T_1' });
expect(after.product?.variants[0].stockLevel).toBe('IN_STOCK');
});
});
});
});
174 changes: 174 additions & 0 deletions packages/core/src/config/catalog/multi-channel-stock-location-strategy.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
import { Subject } from 'rxjs';
import { filter } from 'rxjs/operators';
import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';

import { RequestContext } from '../../api/common/request-context';
import { Cache } from '../../cache/cache';
import { Channel } from '../../entity/channel/channel.entity';
import { Product } from '../../entity/product/product.entity';
import { StockLevel } from '../../entity/stock-level/stock-level.entity';
import { StockLocation } from '../../entity/stock-location/stock-location.entity';
import { ChangeChannelEvent } from '../../event-bus/events/change-channel-event';
import { StockLocationEvent } from '../../event-bus/events/stock-location-event';
import { ensureConfigLoaded } from '../config-helpers';

/**
* Unit tests for the MultiChannelStockLocationStrategy channelId cache invalidation.
*
* The real `Cache` wrapper is used (backed by an in-memory Map) so that the
* key-prefixing behaviour of `Cache.get()`/`Cache.delete()` matches production —
* the invalidation bugs under test live exactly in that interaction.
*/

const cacheStore = new Map<string, any>();
const mockCacheService = {
createCache: (config: any) =>
new Cache(config, {
get: (key: string) => Promise.resolve(cacheStore.get(key)),
set: (key: string, value: any) => {
cacheStore.set(key, value);
return Promise.resolve();
},
delete: (key: string) => {
cacheStore.delete(key);
return Promise.resolve();
},
} as any),
} as any;

const eventStream = new Subject<any>();
const mockEventBus = {
ofType: (type: any) => eventStream.asObservable().pipe(filter((e: any) => e.constructor === type)),
} as any;

let channelsInDb: Channel[];
const getEntityOrThrow = vi.fn(() => Promise.resolve(new StockLocation({ id: 1, channels: channelsInDb })));

const mockInjector = {
get: (token: unknown) => {
const name = (token as { name?: string })?.name;
if (name === 'EventBus') return mockEventBus;
if (name === 'CacheService') return mockCacheService;
if (name === 'TransactionalConnection') return { getEntityOrThrow };
return {};
},
} as any;

const channel1 = new Channel({ id: 1 });
const channel2 = new Channel({ id: 2 });

function contextForChannel(channel: Channel): RequestContext {
return new RequestContext({
apiType: 'shop',
channel,
authorizedAsOwnerOnly: false,
isAuthorized: true,
session: {} as any,
} as any);
}

const ctxChannel1 = contextForChannel(channel1);
const ctxChannel2 = contextForChannel(channel2);

const stockLevel = new StockLevel({
stockLocationId: 1,
stockOnHand: 100,
stockAllocated: 0,
productVariantId: 1,
});

const stockLocation = new StockLocation({ id: 1 });

/** Lets the async `subscribe` handlers (cache deletes) settle. */
function flushEventHandlers() {
return new Promise(resolve => setTimeout(resolve, 0));
}

describe('MultiChannelStockLocationStrategy', () => {
let strategy: import('./multi-channel-stock-location-strategy').MultiChannelStockLocationStrategy;

beforeAll(async () => {
await ensureConfigLoaded();
});

beforeEach(async () => {
cacheStore.clear();
channelsInDb = [channel1];
getEntityOrThrow.mockClear();
// Dynamic import to avoid vitest circular dependency issue
const { MultiChannelStockLocationStrategy } =
await import('./multi-channel-stock-location-strategy.js');
strategy = new MultiChannelStockLocationStrategy();
await strategy.init(mockInjector);
});

it('caches the channel ids of a StockLocation after the first lookup', async () => {
const first = await strategy.getAvailableStock(ctxChannel1, 1, [stockLevel]);
const second = await strategy.getAvailableStock(ctxChannel1, 1, [stockLevel]);

expect(first.stockOnHand).toBe(100);
expect(second.stockOnHand).toBe(100);
expect(getEntityOrThrow).toHaveBeenCalledTimes(1);
});

it('invalidates the cache when a StockLocation is updated', async () => {
// Cache the channel ids while the location belongs to channel1 only
const stale = await strategy.getAvailableStock(ctxChannel2, 1, [stockLevel]);
expect(stale.stockOnHand).toBe(0);

channelsInDb = [channel1, channel2];
eventStream.next(new StockLocationEvent(ctxChannel1, stockLocation, 'updated'));
await flushEventHandlers();

const fresh = await strategy.getAvailableStock(ctxChannel2, 1, [stockLevel]);
expect(fresh.stockOnHand).toBe(100);
});

it('does not invalidate the cache when a StockLocation is created', async () => {
await strategy.getAvailableStock(ctxChannel1, 1, [stockLevel]);

eventStream.next(new StockLocationEvent(ctxChannel1, stockLocation, 'created'));
await flushEventHandlers();

await strategy.getAvailableStock(ctxChannel1, 1, [stockLevel]);
expect(getEntityOrThrow).toHaveBeenCalledTimes(1);
});

it('invalidates the cache when a StockLocation is assigned to a Channel', async () => {
// Cache the channel ids before the location is assigned to channel2
const stale = await strategy.getAvailableStock(ctxChannel2, 1, [stockLevel]);
expect(stale.stockOnHand).toBe(0);

channelsInDb = [channel1, channel2];
eventStream.next(new ChangeChannelEvent(ctxChannel1, stockLocation, [2], 'assigned', StockLocation));
await flushEventHandlers();

const fresh = await strategy.getAvailableStock(ctxChannel2, 1, [stockLevel]);
expect(fresh.stockOnHand).toBe(100);
});

it('invalidates the cache when a StockLocation is removed from a Channel', async () => {
channelsInDb = [channel1, channel2];
const beforeRemoval = await strategy.getAvailableStock(ctxChannel2, 1, [stockLevel]);
expect(beforeRemoval.stockOnHand).toBe(100);

channelsInDb = [channel1];
eventStream.next(new ChangeChannelEvent(ctxChannel1, stockLocation, [2], 'removed', StockLocation));
await flushEventHandlers();

const fresh = await strategy.getAvailableStock(ctxChannel2, 1, [stockLevel]);
expect(fresh.stockOnHand).toBe(0);
});

it('does not invalidate the cache on ChangeChannelEvents for other entity types', async () => {
await strategy.getAvailableStock(ctxChannel1, 1, [stockLevel]);

eventStream.next(
new ChangeChannelEvent(ctxChannel1, new Product({ id: 1 }) as any, [2], 'assigned', Product),
);
await flushEventHandlers();

await strategy.getAvailableStock(ctxChannel1, 1, [stockLevel]);
expect(getEntityOrThrow).toHaveBeenCalledTimes(1);
});
});
18 changes: 14 additions & 4 deletions packages/core/src/config/catalog/multi-channel-stock-location-strategy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import type { GlobalSettingsService } from '../../service/index';
import { GlobalFlag } from '@vendure/common/lib/generated-types';
import { ID } from '@vendure/common/lib/shared-types';
import ms from 'ms';
import { filter } from 'rxjs/operators';
import type { GlobalSettingsService } from '../../service/index';

import { RequestContext } from '../../api/common/request-context';
import { Cache, CacheService, RequestContextCacheService } from '../../cache/index';
Expand All @@ -11,7 +11,7 @@ import { ProductVariant } from '../../entity/index';
import { OrderLine } from '../../entity/order-line/order-line.entity';
import { StockLevel } from '../../entity/stock-level/stock-level.entity';
import { StockLocation } from '../../entity/stock-location/stock-location.entity';
import { EventBus, StockLocationEvent } from '../../event-bus/index';
import { ChangeChannelEvent, EventBus, StockLocationEvent } from '../../event-bus/index';

import { BaseStockLocationStrategy } from './default-stock-location-strategy';
import { AvailableStock, LocationWithQuantity, StockLocationStrategy } from './stock-location-strategy';
Expand Down Expand Up @@ -59,11 +59,21 @@ export class MultiChannelStockLocationStrategy extends BaseStockLocationStrategy
getKey: id => this.getCacheKey(id),
});

// When a StockLocation is updated, we need to invalidate the cache
// When a StockLocation is updated, we need to invalidate the cache.
// Note: `Cache.delete()` applies the configured `getKey` function itself,
// so it must be passed the raw id, not the result of `getCacheKey()`.
this.eventBus
.ofType(StockLocationEvent)
.pipe(filter(event => event.type !== 'created'))
.subscribe(({ entity }) => this.channelIdCache.delete(this.getCacheKey(entity.id)));
.subscribe(({ entity }) => this.channelIdCache.delete(entity.id));

// Assigning a StockLocation to a Channel (or removing it) does not emit a
// StockLocationEvent, so we also need to invalidate the cache on ChangeChannelEvents
// which relate to StockLocations.
this.eventBus
.ofType(ChangeChannelEvent)
.pipe(filter(event => event.entityType === StockLocation))
.subscribe(({ entity }) => this.channelIdCache.delete(entity.id));
}

/**
Expand Down
Loading