From 9eae72913f5d105103ab90980e7aefed406aa5af Mon Sep 17 00:00:00 2001 From: Jared Wray Date: Sat, 11 Jan 2025 09:06:19 -0800 Subject: [PATCH] cacheable - fix: adding in a unique cacheId to handle wrap async conflicts for each cache instance (#974) --- packages/cache-manager/README.md | 2 +- packages/cache-manager/src/index.ts | 2 +- packages/cache-manager/test/wrap.test.ts | 4 ++-- packages/cacheable/src/index.ts | 29 ++++++++++++++++++++++++ packages/cacheable/src/wrap.ts | 5 +++- packages/cacheable/test/index.test.ts | 7 ++++++ 6 files changed, 44 insertions(+), 5 deletions(-) diff --git a/packages/cache-manager/README.md b/packages/cache-manager/README.md index d51d1ce9..69549a34 100644 --- a/packages/cache-manager/README.md +++ b/packages/cache-manager/README.md @@ -183,7 +183,7 @@ const cache = createCache({ stores: [keyv] }); - **cacheId**?: string - Defaults to random string - Unique identifier for the cache instance. + Unique identifier for the cache instance. This is primarily used to not have conflicts when using `wrap` with multiple cache instances. # Methods ## set diff --git a/packages/cache-manager/src/index.ts b/packages/cache-manager/src/index.ts index 99a3be62..8a136cc2 100644 --- a/packages/cache-manager/src/index.ts +++ b/packages/cache-manager/src/index.ts @@ -253,7 +253,7 @@ export const createCache = (options?: CreateCacheOptions): Cache => { fnc: () => T | Promise, ttl?: number | ((value: T) => number), refreshThreshold?: number, - ): Promise => coalesceAsync(`${_cacheId}__${key}`, async () => { + ): Promise => coalesceAsync(`${_cacheId}::${key}`, async () => { let value: T | undefined; let i = 0; let remainingTtl: number | undefined; diff --git a/packages/cache-manager/test/wrap.test.ts b/packages/cache-manager/test/wrap.test.ts index 78e7340b..733bd4c8 100644 --- a/packages/cache-manager/test/wrap.test.ts +++ b/packages/cache-manager/test/wrap.test.ts @@ -89,7 +89,7 @@ describe('wrap', () => { const getValueA = vi.fn(() => 'A'); const getValueB = vi.fn(() => 'B'); const anotherCache = createCache({stores: [new Keyv()]}); - expect(await cache.wrap(data.key, async () => anotherCache.wrap(data.key, getValueB).then((v) => v + getValueA()))).toEqual('BA'); + expect(await cache.wrap(data.key, async () => anotherCache.wrap(data.key, getValueB).then(v => v + getValueA()))).toEqual('BA'); expect(getValueA).toHaveBeenCalledOnce(); expect(getValueB).toHaveBeenCalledOnce(); }); @@ -110,7 +110,7 @@ describe('wrap', () => { const getValueA = vi.fn(() => 'A'); const getValueB = vi.fn(() => 'B'); const anotherCache = createCache({stores: [new Keyv()]}); - expect(await cache.wrap(data.key, async () => anotherCache.wrap(data.key, getValueB).then((v) => v + getValueA()))).toEqual('BA'); + expect(await cache.wrap(data.key, async () => anotherCache.wrap(data.key, getValueB).then(v => v + getValueA()))).toEqual('BA'); expect(getValueA).toHaveBeenCalledOnce(); expect(getValueB).toHaveBeenCalledOnce(); }); diff --git a/packages/cacheable/src/index.ts b/packages/cacheable/src/index.ts index 8d127e0e..1bb3e18f 100644 --- a/packages/cacheable/src/index.ts +++ b/packages/cacheable/src/index.ts @@ -50,6 +50,11 @@ export type CacheableOptions = { * The namespace for the cacheable instance. It can be a string or a function that returns a string. */ namespace?: string | (() => string); + /** + * The cacheId for the cacheable instance. This is primarily used for the wrap function to not have conflicts. + * If it is not set then it will be a random string that is generated + */ + cacheId?: string; }; export class Cacheable extends Hookified { @@ -59,6 +64,7 @@ export class Cacheable extends Hookified { private _ttl?: number | string; private readonly _stats = new CacheableStats({enabled: false}); private _namespace?: string | (() => string); + private _cacheId: string = Math.random().toString(36).slice(2); /** * Creates a new cacheable instance @@ -87,6 +93,10 @@ export class Cacheable extends Hookified { this.setTtl(options.ttl); } + if (options?.cacheId) { + this._cacheId = options.cacheId; + } + if (options?.namespace) { this._namespace = options.namespace; this._primary.namespace = this.getNameSpace(); @@ -225,6 +235,24 @@ export class Cacheable extends Hookified { this.setTtl(ttl); } + /** + * The cacheId for the cacheable instance. This is primarily used for the wrap function to not have conflicts. + * If it is not set then it will be a random string that is generated + * @returns {string} The cacheId for the cacheable instance + */ + public get cacheId(): string { + return this._cacheId; + } + + /** + * Sets the cacheId for the cacheable instance. This is primarily used for the wrap function to not have conflicts. + * If it is not set then it will be a random string that is generated + * @param {string} cacheId The cacheId for the cacheable instance + */ + public set cacheId(cacheId: string) { + this._cacheId = cacheId; + } + /** * Sets the primary store for the cacheable instance * @param {Keyv | KeyvStoreAdapter} primary The primary store for the cacheable instance @@ -603,6 +631,7 @@ export class Cacheable extends Hookified { ttl: options?.ttl ?? this._ttl, keyPrefix: options?.keyPrefix, cache: this, + cacheId: this._cacheId, }; return wrap(function_, wrapOptions); diff --git a/packages/cacheable/src/wrap.ts b/packages/cacheable/src/wrap.ts index 6bf6cd30..9de5cac7 100644 --- a/packages/cacheable/src/wrap.ts +++ b/packages/cacheable/src/wrap.ts @@ -6,6 +6,7 @@ export type WrapFunctionOptions = { ttl?: number | string; keyPrefix?: string; cacheErrors?: boolean; + cacheId?: string; }; export type WrapOptions = WrapFunctionOptions & { @@ -53,7 +54,9 @@ export function wrap(function_: AnyFunction, options: WrapOptions): AnyFuncti value = await cache.get(cacheKey) as T | undefined; if (value === undefined) { - value = await coalesceAsync(cacheKey, async () => { + const cacheId = options.cacheId ?? 'default'; + const coalesceKey = `${cacheId}::${cacheKey}`; + value = await coalesceAsync(coalesceKey, async () => { try { // eslint-disable-next-line @typescript-eslint/no-unsafe-argument const result = await function_(...arguments_) as T; diff --git a/packages/cacheable/test/index.test.ts b/packages/cacheable/test/index.test.ts index a041583f..a6e5d15b 100644 --- a/packages/cacheable/test/index.test.ts +++ b/packages/cacheable/test/index.test.ts @@ -88,6 +88,13 @@ describe('cacheable options and properties', async () => { cacheable.ttl = 0; expect(cacheable.ttl).toEqual(undefined); }); + + test('should be able to set the cacheId', async () => { + const cacheable = new Cacheable({cacheId: 'test'}); + expect(cacheable.cacheId).toEqual('test'); + cacheable.cacheId = 'test2'; + expect(cacheable.cacheId).toEqual('test2'); + }); }); describe('cacheable stats', async () => {