From 66c0122310747935c09bb23c026006cef409242b Mon Sep 17 00:00:00 2001 From: Forbes Lindesay Date: Tue, 20 Feb 2024 13:38:55 +0000 Subject: [PATCH] feat: pass deletePrefix and delete spread through dedupe (#319) --- docs/dataloader.md | 39 +++ packages/cache/src/index.ts | 2 +- .../dataloader/src/CacheMapImplementation.ts | 112 +++++++++ packages/dataloader/src/MultiKeyMap.ts | 17 +- .../src/__tests__/dedupeAsync.test.ts | 223 ++++++++++++++++++ packages/dataloader/src/dedupeAsync.ts | 39 +-- packages/dataloader/src/dedupeSync.ts | 39 +-- packages/dataloader/src/index.ts | 9 +- packages/dataloader/src/types.ts | 28 ++- 9 files changed, 418 insertions(+), 90 deletions(-) create mode 100644 packages/dataloader/src/CacheMapImplementation.ts diff --git a/docs/dataloader.md b/docs/dataloader.md index daa4c9b0..91078374 100644 --- a/docs/dataloader.md +++ b/docs/dataloader.md @@ -349,6 +349,45 @@ function onUserChanged(id: DbUser['id']) { } ``` +One downside of this approach is that you can only use this outside of a transaction. The simplest way to resolve this is to separate the implementation from the caching: + +```typescript +import {dedupeAsync} from '@databases/dataloader'; +import database, {tables, DbUser} from './database'; + +async function getUserBase( + database: Queryable, + userId: DbUser['id'], +): Promise { + return await tables.users(database).findOneRequired({id: userId}); +} + +// (userId: DbUser['id']) => Promise +const getUserCached = dedupeAsync( + async (userId) => await getUserBase(userId, database), + {cache: createCache({name: 'Users'})}, +); + +export async function getUser( + db: Queryable, + userId: DbUser['id'], +): Promise { + if (db === database) { + // If we're using the default connection, + // it's safe to read from the cache + return await getUserCached(userId); + } else { + // If we're inside a transaction, we may + // need to bypass the cache + return await getUserBase(db, userId); + } +} + +function onUserChanged(id: DbUser['id']) { + getUserCached.cache.delete(id); +} +``` + #### Caching fetch requests The following example caches requests to load a user from some imaginary API. diff --git a/packages/cache/src/index.ts b/packages/cache/src/index.ts index d0184941..77965ebd 100644 --- a/packages/cache/src/index.ts +++ b/packages/cache/src/index.ts @@ -442,7 +442,7 @@ export default function createCacheRealm( } this._delete(k); } - if (onReplicationEvent) { + if (onReplicationEvent && serializedKeys.size) { onReplicationEvent({ kind: 'DELETE_MULTIPLE', name: this.name, diff --git a/packages/dataloader/src/CacheMapImplementation.ts b/packages/dataloader/src/CacheMapImplementation.ts new file mode 100644 index 00000000..1e7cdc12 --- /dev/null +++ b/packages/dataloader/src/CacheMapImplementation.ts @@ -0,0 +1,112 @@ +import {AsyncCacheMap, CacheMap, CacheMapInput, KeyPrefix} from './types'; + +const supportsDeleteSpreadCache = new WeakMap(); + +function supportsDeleteSpreadUncached( + cacheMap: CacheMapInput, +): boolean { + return /^[^={]*\.\.\./.test(cacheMap.delete.toString()); +} + +function supportsDeleteSpread( + cacheMap: CacheMapInput, +): boolean { + if (cacheMap.constructor === Map || cacheMap.constructor === WeakMap) { + return false; + } + if (cacheMap.constructor === Object || cacheMap.constructor === Function) { + return supportsDeleteSpreadUncached(cacheMap); + } + + const cached = supportsDeleteSpreadCache.get(cacheMap.constructor); + if (cached !== undefined) return cached; + + const freshValue = supportsDeleteSpreadUncached(cacheMap); + supportsDeleteSpreadCache.set(cacheMap.constructor, freshValue); + return freshValue; +} + +class CacheMapImplementation + implements CacheMap +{ + private readonly _map: CacheMapInput; + private readonly _mapKey: (key: TKey) => TMappedKey; + private readonly _supportsDeleteSpread: boolean; + constructor( + map: CacheMapInput, + mapKey: (key: TKey) => TMappedKey, + ) { + this._map = map; + this._mapKey = mapKey; + this._supportsDeleteSpread = supportsDeleteSpread(map); + } + + get size() { + return this._map.size; + } + get(key: TKey): TResult | undefined { + const cacheKey = this._mapKey(key); + return this._map.get(cacheKey); + } + set(key: TKey, value: TResult): void { + const cacheKey = this._mapKey(key); + this._map.set(cacheKey, value); + } + deletePrefix(prefix: KeyPrefix): void { + if (this._map.deletePrefix) { + this._map.deletePrefix(prefix as any); + } else if (this._map.keys && typeof prefix === 'string') { + for (const key of this._map.keys()) { + const k: unknown = key; + if (typeof k !== 'string') { + throw new Error( + `This cache contains non-string keys so you cannot use deletePrefix.`, + ); + } + if (k.startsWith(prefix)) { + this._map.delete(key); + } + } + } else { + throw new Error(`This cache does not support deletePrefix.`); + } + } + delete(...keys: TKey[]): void { + if (!this._supportsDeleteSpread || keys.length < 2) { + for (const key of keys) { + const cacheKey = this._mapKey(key); + this._map.delete(cacheKey); + } + } else { + const cacheKeys = keys.map(this._mapKey); + this._map.delete(...cacheKeys); + } + } + clear(): void { + if (!this._map.clear) { + throw new Error(`This cache does not support clearing`); + } + this._map.clear(); + } +} +export function createCacheMap( + map: CacheMapInput, + mapKey: (key: TKey) => TMappedKey, +): CacheMap { + return new CacheMapImplementation(map, mapKey); +} + +class AsyncCacheMapImplementation + extends CacheMapImplementation, TMappedKey> + implements AsyncCacheMap +{ + set(key: TKey, value: TResult | Promise): void { + super.set(key, Promise.resolve(value)); + } +} +export function createAsyncCacheMap( + map: CacheMapInput>, + mapKey: (key: TKey) => TMappedKey, +): AsyncCacheMap { + return new AsyncCacheMapImplementation(map, mapKey); +} diff --git a/packages/dataloader/src/MultiKeyMap.ts b/packages/dataloader/src/MultiKeyMap.ts index 2dd3f337..ccbe81ee 100644 --- a/packages/dataloader/src/MultiKeyMap.ts +++ b/packages/dataloader/src/MultiKeyMap.ts @@ -1,18 +1,10 @@ -import {CacheMap, CacheMapInput} from './types'; +import {CacheMapInput, Path, SubPath} from './types'; -type Path = readonly [unknown, ...(readonly unknown[])]; -type SubPath = TKeys extends readonly [ - ...infer THead, - infer TTail, -] - ? {readonly [i in keyof TKeys]: TKeys[i]} | SubPath - : never; - -export interface MultiKeyMap - extends CacheMap { +export interface MultiKeyMap { readonly size: number; get: (key: TKeys) => TValue | undefined; set: (key: TKeys, value: TValue) => void; + deletePrefix: (key: SubPath) => void; delete: (key: TKeys | SubPath) => void; clear: () => void; } @@ -136,6 +128,9 @@ class MultiKeyMapImplementation set(key: TKeys, value: TValue): void { this._root.set(key, value); } + deletePrefix(key: SubPath): void { + this._root.delete(key); + } delete(key: TKeys | SubPath): void { this._root.delete(key); } diff --git a/packages/dataloader/src/__tests__/dedupeAsync.test.ts b/packages/dataloader/src/__tests__/dedupeAsync.test.ts index b32cfacb..3fc659c4 100644 --- a/packages/dataloader/src/__tests__/dedupeAsync.test.ts +++ b/packages/dataloader/src/__tests__/dedupeAsync.test.ts @@ -1,3 +1,4 @@ +import createMultiKeyMap from '../MultiKeyMap'; import dedupeAsync from '../dedupeAsync'; import requestsTester from './requestsTester'; @@ -248,3 +249,225 @@ test('dedupeAsync - WeakMap', async () => { ).toEqual([{source: 'hello'}, {source: 'from set'}]); }); }); + +test('dedupeAsync - MultiKeyMap', async () => { + const requests = requestsTester<`${string}:${number}`>(); + const load = dedupeAsync( + async ([{id}, value]: [{id: string}, number]) => { + requests.add(`${id}:${value}`); + if (id === 'ERROR_THIS') { + throw new Error('Errored'); + } + return {source: id, value}; + }, + { + cache: createMultiKeyMap< + [{id: string}, number], + Promise<{source: string; value: number}> + >([{getCache: () => new WeakMap()}, {}]), + }, + ); + + const HELLO_REQUEST = {id: 'hello'}; + const WORLD_REQUEST = {id: 'world'}; + + await requests.expect([`hello:1`, `hello:2`, `world:3`], async () => { + expect( + await Promise.all([ + load([HELLO_REQUEST, 1]), + load([HELLO_REQUEST, 2]), + load([WORLD_REQUEST, 3]), + ]), + ).toEqual([ + {source: 'hello', value: 1}, + {source: 'hello', value: 2}, + {source: 'world', value: 3}, + ]); + }); + + await expect(load.cache.get([HELLO_REQUEST, 1])).resolves.toEqual({ + source: 'hello', + value: 1, + }); + await expect(load.cache.get([HELLO_REQUEST, 2])).resolves.toEqual({ + source: 'hello', + value: 2, + }); + await expect(load.cache.get([WORLD_REQUEST, 3])).resolves.toEqual({ + source: 'world', + value: 3, + }); + + load.cache.deletePrefix([HELLO_REQUEST]); + load.cache.set([WORLD_REQUEST, 3], {source: 'from set', value: 3}); + + await requests.expect([`hello:1`, `hello:2`], async () => { + expect( + await Promise.all([ + load([HELLO_REQUEST, 1]), + load([HELLO_REQUEST, 2]), + load([WORLD_REQUEST, 3]), + ]), + ).toEqual([ + {source: 'hello', value: 1}, + {source: 'hello', value: 2}, + {source: 'from set', value: 3}, + ]); + }); + + await requests.expect([], async () => { + expect( + await Promise.all([ + load([HELLO_REQUEST, 1]), + load([HELLO_REQUEST, 2]), + load([WORLD_REQUEST, 3]), + ]), + ).toEqual([ + {source: 'hello', value: 1}, + {source: 'hello', value: 2}, + {source: 'from set', value: 3}, + ]); + }); +}); + +test('dedupeAsync - delete spread', async () => { + let deleteCalls = 0; + class MapWithDeleteSpread extends Map { + delete(...keys: TKey[]) { + deleteCalls++; + for (const key of keys) { + super.delete(key); + } + return true; + } + } + class MapWithoutDeleteSpread extends Map { + delete(key: TKey) { + deleteCalls++; + return super.delete(key); + } + } + const requests = requestsTester(); + const load = dedupeAsync( + async (source: string) => { + requests.add(source); + return {source}; + }, + {cache: new MapWithDeleteSpread()}, + ); + + await requests.expect(['hello', 'world', 'other'], async () => { + expect( + await Promise.all([ + load('hello'), + load('hello'), + load('world'), + load('other'), + ]), + ).toEqual([ + {source: 'hello'}, + {source: 'hello'}, + {source: 'world'}, + {source: 'other'}, + ]); + }); + load.cache.delete('hello', 'world'); + expect(deleteCalls).toBe(1); + await requests.expect(['hello', 'world'], async () => { + expect( + await Promise.all([ + load('hello'), + load('hello'), + load('world'), + load('other'), + ]), + ).toEqual([ + {source: 'hello'}, + {source: 'hello'}, + {source: 'world'}, + {source: 'other'}, + ]); + }); + + deleteCalls = 0; + const load2 = dedupeAsync( + async (source: string) => { + requests.add(source); + return {source}; + }, + {cache: new MapWithoutDeleteSpread()}, + ); + + await requests.expect(['hello', 'world', 'other'], async () => { + expect( + await Promise.all([ + load2('hello'), + load2('hello'), + load2('world'), + load2('other'), + ]), + ).toEqual([ + {source: 'hello'}, + {source: 'hello'}, + {source: 'world'}, + {source: 'other'}, + ]); + }); + load2.cache.delete('hello', 'world'); + expect(deleteCalls).toBe(2); + await requests.expect(['hello', 'world'], async () => { + expect( + await Promise.all([ + load2('hello'), + load2('hello'), + load2('world'), + load2('other'), + ]), + ).toEqual([ + {source: 'hello'}, + {source: 'hello'}, + {source: 'world'}, + {source: 'other'}, + ]); + }); +}); + +test('dedupeAsync - delete prefix', async () => { + const requests = requestsTester(); + const load = dedupeAsync(async (source: string) => { + requests.add(source); + return {source}; + }); + + await requests.expect(['hello:1', 'hello:2', 'world:3'], async () => { + expect( + await Promise.all([ + load('hello:1'), + load('hello:2'), + load('world:3'), + load('hello:1'), + ]), + ).toEqual([ + {source: 'hello:1'}, + {source: 'hello:2'}, + {source: 'world:3'}, + {source: 'hello:1'}, + ]); + }); + load.cache.deletePrefix('hello:'); + await requests.expect(['hello:1', 'hello:2'], async () => { + expect( + await Promise.all([ + load('hello:1'), + load('hello:2'), + load('world:3'), + load('hello:1'), + ]), + ).toEqual([ + {source: 'hello:1'}, + {source: 'hello:2'}, + {source: 'world:3'}, + {source: 'hello:1'}, + ]); + }); +}); diff --git a/packages/dataloader/src/dedupeAsync.ts b/packages/dataloader/src/dedupeAsync.ts index 5c01d20a..fee1c1c1 100644 --- a/packages/dataloader/src/dedupeAsync.ts +++ b/packages/dataloader/src/dedupeAsync.ts @@ -1,3 +1,4 @@ +import {createAsyncCacheMap} from './CacheMapImplementation'; import {AsyncCacheMap, CacheMapInput} from './types'; export interface DedupedAsyncFunction { @@ -56,7 +57,7 @@ export default function dedupeAsync( cache.set(cacheKey, fresh); return fresh; }, - {cache: new AsyncCacheMapImplementation(cache, mapKey)}, + {cache: createAsyncCacheMap(cache, mapKey)}, ); } @@ -78,39 +79,3 @@ function normalizeDedupeAsyncOptions( shouldCache: options?.shouldCache ?? trueFn, }; } - -class AsyncCacheMapImplementation - implements AsyncCacheMap -{ - private readonly _map: CacheMapInput>; - private readonly _mapKey: (key: TKey) => TMappedKey; - constructor( - map: CacheMapInput>, - mapKey: (key: TKey) => TMappedKey, - ) { - this._map = map; - this._mapKey = mapKey; - } - - get size() { - return this._map.size; - } - get(key: TKey): Promise | undefined { - const cacheKey = this._mapKey(key); - return this._map.get(cacheKey); - } - set(key: TKey, value: TResult | Promise): void { - const cacheKey = this._mapKey(key); - this._map.set(cacheKey, Promise.resolve(value)); - } - delete(key: TKey): void { - const cacheKey = this._mapKey(key); - this._map.delete(cacheKey); - } - clear(): void { - if (!this._map.clear) { - throw new Error(`This cache does not support clearing`); - } - this._map.clear(); - } -} diff --git a/packages/dataloader/src/dedupeSync.ts b/packages/dataloader/src/dedupeSync.ts index 0ffd4cf7..f7c380f4 100644 --- a/packages/dataloader/src/dedupeSync.ts +++ b/packages/dataloader/src/dedupeSync.ts @@ -1,3 +1,4 @@ +import {createCacheMap} from './CacheMapImplementation'; import {CacheMap, CacheMapInput} from './types'; export interface DedupedSyncFunction { @@ -41,7 +42,7 @@ export default function dedupeSync( return fresh; }, { - cache: new CacheMapImplementation(cache, mapKey), + cache: createCacheMap(cache, mapKey), }, ); } @@ -88,39 +89,3 @@ function addErrorHandler( } }; } - -class CacheMapImplementation - implements CacheMap -{ - private readonly _map: CacheMapInput; - private readonly _mapKey: (key: TKey) => TMappedKey; - constructor( - map: CacheMapInput, - mapKey: (key: TKey) => TMappedKey, - ) { - this._map = map; - this._mapKey = mapKey; - } - - get size() { - return this._map.size; - } - get(key: TKey): TResult | undefined { - const cacheKey = this._mapKey(key); - return this._map.get(cacheKey); - } - set(key: TKey, value: TResult): void { - const cacheKey = this._mapKey(key); - this._map.set(cacheKey, value); - } - delete(key: TKey): void { - const cacheKey = this._mapKey(key); - this._map.delete(cacheKey); - } - clear(): void { - if (!this._map.clear) { - throw new Error(`This cache does not support clearing`); - } - this._map.clear(); - } -} diff --git a/packages/dataloader/src/index.ts b/packages/dataloader/src/index.ts index ddad52e0..99731ea3 100644 --- a/packages/dataloader/src/index.ts +++ b/packages/dataloader/src/index.ts @@ -27,7 +27,14 @@ export type { DedupeSyncOptionsWithMapKey, DedupeSyncOptionsWithoutMapKey, } from './dedupeSync'; -export type {CacheMapInput, CacheMap} from './types'; +export type { + AsyncCacheMap, + CacheMap, + CacheMapInput, + KeyPrefix, + Path, + SubPath, +} from './types'; export {default as batch, batchGroups} from './batch'; export {default as createMultiKeyMap} from './MultiKeyMap'; diff --git a/packages/dataloader/src/types.ts b/packages/dataloader/src/types.ts index 1ca46942..ecbd2268 100644 --- a/packages/dataloader/src/types.ts +++ b/packages/dataloader/src/types.ts @@ -1,21 +1,43 @@ +export type Path = readonly [unknown, ...(readonly unknown[])]; +export type SubPath = TKeys extends readonly [ + ...infer THead, + infer TTail, +] + ? {readonly [i in keyof TKeys]: TKeys[i]} | SubPath + : never; + +export type KeyPrefix = TKey extends readonly unknown[] + ? SubPath | string + : string; export interface CacheMapInput { readonly size?: number; get: (key: TKey) => TValue | undefined; set: (key: TKey, value: TValue) => unknown; - delete: (key: TKey) => unknown; + deletePrefix?: TKey extends readonly unknown[] + ? ((prefix: string) => unknown) | ((prefix: SubPath) => unknown) + : (prefix: string) => unknown; + delete: (...keys: TKey[]) => unknown; clear?: () => unknown; + keys?: () => IterableIterator; } + export interface CacheMap { readonly size?: number; get: (key: TKey) => TValue | undefined; set: (key: TKey, value: TValue) => void; - delete: (key: TKey) => void; + deletePrefix: (prefix: KeyPrefix) => void; + delete: (...keys: TKey[]) => void; clear: () => void; } + export interface AsyncCacheMap { readonly size?: number; get: (key: TKey) => Promise | undefined; set: (key: TKey, value: Promise | TValue) => void; - delete: (key: TKey) => void; + deletePrefix: (prefix: KeyPrefix) => void; + delete: (...keys: TKey[]) => void; clear: () => void; } + +// const x: CacheMapInput = new Map(); +// const y: CacheMapInput<{}, number> = new WeakMap<{}, number>();