Skip to content
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.

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

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion packages/cache-manager/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/cache-manager/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions packages/cache-manager/test/wrap.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Expand All @@ -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();
});
Expand Down
29 changes: 29 additions & 0 deletions packages/cacheable/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion packages/cacheable/src/wrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export type WrapFunctionOptions = {
ttl?: number | string;
keyPrefix?: string;
cacheErrors?: boolean;
cacheId?: string;
};

export type WrapOptions = WrapFunctionOptions & {
Expand Down Expand Up @@ -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;
Expand Down
7 changes: 7 additions & 0 deletions packages/cacheable/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
Loading