cacheable - fix: adding in a unique cacheId to handle wrap async conf…

…licts for each cache instance (#974)
jaredwray · Jan 11, 2025 · 9eae729 · 9eae729
1 parent 8c6ec41
commit 9eae729
Show file tree

Hide file tree

Showing 6 changed files with 44 additions and 5 deletions.
diff --git 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
@@ -253,7 +253,7 @@ export const createCache = (options?: CreateCacheOptions): Cache => {
 		fnc: () => T | Promise<T>,
 		ttl?: number | ((value: T) => number),
 		refreshThreshold?: number,
-	): Promise<T> => coalesceAsync(`${_cacheId}__${key}`, async () => {
+	): Promise<T> => 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
@@ -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
@@ -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<T>(function_, wrapOptions);

diff --git 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<T>(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
@@ -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 () => {