From 00179a6e01a1e0609a91e44c57c71e22df57eda2 Mon Sep 17 00:00:00 2001 From: cnoelle <19640096+cnoelle@users.noreply.github.com> Date: Mon, 23 Sep 2024 23:21:07 +0200 Subject: [PATCH] added option to clone items on insert or return --- package.json | 2 +- src/cache.ts | 49 +++++++++++++++++-------- src/impl/CacheImpl.ts | 45 ++++++++++++++--------- src/impl/Table.ts | 25 ++++++++----- src/impl/Utils.ts | 10 +++++ test/testCacheWithMemory.js | 72 ++++++++++++++++++++++++++++++++++++ test/testDefaultCache.js | 73 +++++++++++++++++++++++++++++++++++++ 7 files changed, 233 insertions(+), 43 deletions(-) diff --git a/package.json b/package.json index 2fd5b74..2ed947b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "lru-cache-idb", - "version": "0.4.0", + "version": "0.5.0", "description": "A least-recently-used (LRU) cache for web applications based on IndexedDB.", "type": "module", "types": "./dist/cache.d.ts", diff --git a/src/cache.ts b/src/cache.ts index 3cd0e47..585885a 100644 --- a/src/cache.ts +++ b/src/cache.ts @@ -65,12 +65,6 @@ export interface LruIdbConfig { */ cleanUpOrphanedPeriod?: Milliseconds; - /** - * When the page visibility changes to false (e.g. the user switching to another tab), persist current lru state? - * Default: true - */ -// persistOnVisibilityChange?: boolean; - /** * Mostly for testing. * Default: globalThis.indexedDB @@ -79,7 +73,6 @@ export interface LruIdbConfig { databaseFactory: IDBFactory; keyRange: /* Class*/ any; // XXX can we really not avoid this? } - //database?: IDBFactory; /** Keep items in memory? */ memoryConfig?: boolean|{ @@ -93,14 +86,40 @@ export interface LruIdbConfig { */ numMemoryItemsToPurge?: number; - /** - * TODO not used yet - * By default uses structuredClone - * @param object - * @returns - */ - deepCopy?: (object: any) => any; - } + }, + + /** + * Return copies of the stored values if they come from memory, so that the caller + * can modify them without impacting the stored values + * (there is no need to copy values restored from persistence). + * The function used to copy values is [`structuredClone`](https://developer.mozilla.org/en-US/docs/Web/API/structuredClone), + * unless a custom {@link deepCopy} function is provided. + * + * If this is false and items are kept in memory (which can happen due to periodic persistence, see {@link persistencePeriod}, and/or + * due to {@link memoryConfig}), then the caller should avoid modifying objects returned from the cache. + * + * Default is false. + */ + copyOnReturn?: boolean; + /** + * Copy values upon insertion. + * The function used to copy values is [`structuredClone`](https://developer.mozilla.org/en-US/docs/Web/API/structuredClone), + * unless a custom {@link deepCopy} function is provided. + * + * If this is false and items are kept in memory (which can happen due to periodic persistence, see {@link persistencePeriod}, and/or + * due to {@link memoryConfig}), then the caller should avoid modifying objects that have been passed to the cache. + * + * Default is false. + */ + copyOnInsert?: boolean; + + /** + * Only relevant if {@link copyOnReturn} is true or {@link copyOnInsert} is true. + * By default it uses the global [`structuredClone`](https://developer.mozilla.org/en-US/docs/Web/API/structuredClone) function. + * @param object + * @returns + */ + deepCopy?: (object: any) => any; } diff --git a/src/impl/CacheImpl.ts b/src/impl/CacheImpl.ts index 3f8e726..0f75cfa 100644 --- a/src/impl/CacheImpl.ts +++ b/src/impl/CacheImpl.ts @@ -25,6 +25,15 @@ export class LruCacheIndexedDBImpl implements LruCacheIndexedDB { readonly #accessTimes: Table<{t: MillisecondsSinceEpoch}>; readonly #persistenceOrchestrator: PersistenceOrchestrator|undefined; readonly #dbLoader: (options?: CacheRequestOptions) => Promise; + + readonly #initTimer: number; + // memory cache + readonly #memory: Map>|undefined; + readonly #maxMemorySize: number|undefined; + readonly #numMemoryItemsToPurge: number|undefined; + readonly #copyOnInsert: boolean; + readonly #copyOnReturn: boolean; + readonly #deepCopy: ((obj: T) => T)|undefined; #dbInitialized: boolean = false; readonly #eviction: PeriodicTask|undefined; @@ -76,12 +85,6 @@ export class LruCacheIndexedDBImpl implements LruCacheIndexedDB { } }; - readonly #initTimer: number; - // memory cache - readonly #memory: Map>|undefined; - readonly #maxMemorySize: number|undefined; - readonly #numMemoryItemsToPurge: number|undefined; - constructor(config?: LruIdbConfig) { this.#config = validateConfig(config); this.#maxMemorySize = (typeof(this.#config.memoryConfig) === "object") ? this.#config.memoryConfig.maxItemsInMemory || undefined : undefined; @@ -89,6 +92,9 @@ export class LruCacheIndexedDBImpl implements LruCacheIndexedDB { this.#numMemoryItemsToPurge = this.#maxMemorySize! > 0 ? this.#config.numItemsToPurge || Math.max(1, Math.round(this.#maxMemorySize!/4)) : undefined; this.#indexedDB = this.#config.indexedDB?.databaseFactory!; this.#IDBKeyRange = this.#config.indexedDB?.keyRange!; + this.#deepCopy = this.#config.deepCopy; + this.#copyOnReturn = this.#config.copyOnReturn!; + this.#copyOnInsert = this.#config.copyOnInsert!; this.#database = this.#config.databaseName!; this.#itemsStorage = this.#config.itemsStorage; this.#accessTimesStorage = this.#config.accessTimesStorage; @@ -136,14 +142,17 @@ export class LruCacheIndexedDBImpl implements LruCacheIndexedDB { item = entry!.value; if (!noUpdate) entry!.lastAccessed = now; + if (this.#copyOnReturn) + item = this.#deepCopy!(item); } - item = item || await this.#items.get(key, options); + item = item || await this.#items.get(key, {...options, deepCopy: this.#copyOnReturn ? this.#deepCopy! : undefined}); if (item && !noUpdate) { // do not wait this.#accessTimes.set(key, {t: now}, options).catch(e => console.log("Failed to set access time for", key, e)); - if (!inMemory) - this.#memory?.set(key, {key: key, value: item, lastAccessed: now}); + if (!inMemory) + this.#memory?.set(key, {key: key, value: this.#copyOnReturn ? this.#deepCopy!(item) : item, lastAccessed: now}); } + return item; } @@ -157,14 +166,14 @@ export class LruCacheIndexedDBImpl implements LruCacheIndexedDB { .filter(key => this.#memory?.has(key)) .map(key => [key, this.#memory?.get(key)!] as [string, CachedItem]) if (entries.length > 0) { - result = new Map(entries.map(([key, value]) => [key, value.value])); + result = new Map(entries.map(([key, value]) => [key, this.#copyOnReturn ? this.#deepCopy!(value.value) : value.value])); entries.map(([key, value]) => value).forEach(val => val.lastAccessed = now); keys = keys.filter(key => !result!.has(key)); if (keys.length === 0) return result; } } - const persistent = await this.#items.getAll(keys, options); + const persistent = await this.#items.getAll(keys, {...options, deepCopy: this.#copyOnReturn ? this.#deepCopy! : undefined}); if (result) persistent.forEach((value, key) => result!.set(key, value)); else @@ -175,8 +184,9 @@ export class LruCacheIndexedDBImpl implements LruCacheIndexedDB { this.#accessTimes.setAll(accessTimes, options).catch(e => console.log("Error setting lru-idb access times", e)); // do not wait if (this.#memory) { persistent.forEach((value, key) => { - if (value) - this.#memory?.set(key, {key: key, value: value, lastAccessed: now}); + if (value) { + this.#memory?.set(key, {key: key, value: this.#copyOnReturn ? this.#deepCopy!(value) : value, lastAccessed: now}); + } }); } return result; @@ -188,12 +198,13 @@ export class LruCacheIndexedDBImpl implements LruCacheIndexedDB { if (this.#persistenceOrchestrator && !options?.persistImmediately) { const options2 = {persistence: this.#persistenceOrchestrator}; options = options ? {...options, ...options2} : options2 as any; - await Promise.all([this.#items.set(key, value, options), this.#accessTimes.set(key, {t: now}, options)]); + await Promise.all([this.#items.set(key, value, this.#copyOnInsert ? {...options, deepCopy: this.#deepCopy} : options), this.#accessTimes.set(key, {t: now}, options)]); } else { // case 2: immediate persistence const entries = new Map, Map>([[this.#items, new Map([[key, value]])], [this.#accessTimes, new Map([[key, {t: now}]])]]); await PersistenceOrchestrator.persistImmediately(entries, await this.#dbLoader(), options); } - this.#memory?.set(key, {key: key, value: value, lastAccessed: now}); + if (this.#memory) + this.#memory?.set(key, {key: key, value: this.#copyOnInsert ? this.#deepCopy!(value) : value, lastAccessed: now}); this.#cleanUpAfterSet(); return undefined; } @@ -208,13 +219,13 @@ export class LruCacheIndexedDBImpl implements LruCacheIndexedDB { if (this.#persistenceOrchestrator && !options?.persistImmediately) { const options2 = {persistence: this.#persistenceOrchestrator}; options = options ? {...options, ...options2} : options2 as any; - await Promise.all([this.#items.setAll(entries, options), this.#accessTimes.setAll(accessTimes, options)]); + await Promise.all([this.#items.setAll(entries, this.#copyOnInsert ? {...options, deepCopy: this.#deepCopy} : options), this.#accessTimes.setAll(accessTimes, options)]); } else { // case 2: immediate persistence const fullEntries = new Map, Map>([[this.#items, entries], [this.#accessTimes, accessTimes]]); await PersistenceOrchestrator.persistImmediately(fullEntries, await this.#dbLoader(), options); } if (this.#memory) - entries.forEach((value, key) => this.#memory?.set(key, {key: key, value: value, lastAccessed: now})); + entries.forEach((value, key) => this.#memory?.set(key, {key: key, value: this.#copyOnInsert ? this.#deepCopy!(value) : value, lastAccessed: now})); this.#cleanUpAfterSet(); return undefined; } diff --git a/src/impl/Table.ts b/src/impl/Table.ts index e9dcd0b..2c4cd52 100644 --- a/src/impl/Table.ts +++ b/src/impl/Table.ts @@ -154,23 +154,28 @@ export class Table /*implements LruCacheIndexedDB*/ { return this.get(key, options); } - async get(key: string, options?: CacheRequestOptions): Promise { - if (this.#itemsForPersistence?.has(key)) - return Promise.resolve(this.#itemsForPersistence.get(key)!); + async get(key: string, options?: CacheRequestOptions&{deepCopy?: Function;}): Promise { + if (this.#itemsForPersistence?.has(key)) { + const item = this.#itemsForPersistence.get(key)!; + return Promise.resolve(options?.deepCopy ? options.deepCopy(item) : item); + } const item = await this.#getInternal(key, options); return item; } - async getAll(keys: Array, options?: CacheRequestOptions&{includeAbsent?: boolean}): Promise> { + async getAll(keys: Array, options?: CacheRequestOptions&{includeAbsent?: boolean; deepCopy?: Function;}): Promise> { if (keys.length === 0) return Promise.resolve(new Map()); let result: Map|undefined; if (this.#itemsForPersistence) { - const entries = keys + let entries = keys .filter(key => this.#itemsForPersistence?.has(key)) .map(key => [key, this.#itemsForPersistence?.get(key)!] as [string, T]) - if (entries.length > 0) + if (entries.length > 0) { + if (options?.deepCopy) + entries = entries.map(([key, value]) => [key, options.deepCopy!(value)]); result = new Map(entries); + } } if (result) { keys = keys.filter(key => !result.has(key)); @@ -297,9 +302,9 @@ export class Table /*implements LruCacheIndexedDB*/ { return donePromise; } - set(key: string, value: T, options?: CacheRequestOptions&CacheWriteOptions&{persistence?: PersistenceOrchestrator}): Promise { + set(key: string, value: T, options?: CacheRequestOptions&CacheWriteOptions&{persistence?: PersistenceOrchestrator; deepCopy?: Function;}): Promise { if (this.#itemsForPersistence && !options?.persistImmediately) { - this.#itemsForPersistence.set(key, value); + this.#itemsForPersistence.set(key, options?.deepCopy ? options.deepCopy(value) : value); options?.persistence?.trigger(); return Promise.resolve(); } else { @@ -307,9 +312,9 @@ export class Table /*implements LruCacheIndexedDB*/ { } } - setAll(entries: Map, options?: CacheRequestOptions&CacheWriteOptions&{persistence?: PersistenceOrchestrator}): Promise { + setAll(entries: Map, options?: CacheRequestOptions&CacheWriteOptions&{persistence?: PersistenceOrchestrator; deepCopy?: Function;}): Promise { if (this.#itemsForPersistence && !options?.persistImmediately) { - entries.forEach((value, key) => this.#itemsForPersistence!.set(key, value)); + entries.forEach((value, key) => this.#itemsForPersistence!.set(key, options?.deepCopy ? options.deepCopy(value) : value)); options?.persistence?.trigger(); return Promise.resolve(); } else { diff --git a/src/impl/Utils.ts b/src/impl/Utils.ts index 3dfb947..bcd99cf 100644 --- a/src/impl/Utils.ts +++ b/src/impl/Utils.ts @@ -58,6 +58,16 @@ export function validateConfig(config?: LruIdbConfig): ValidatedLruIdbConfig { throw new Error("numMemoryItemsToPurge must be a positive integer, got " + memory.numMemoryItemsToPurge); } } + const needCopy: boolean = (cfg.copyOnReturn === true || cfg.copyOnInsert === true) && (!!cfg.memoryConfig || cfg.persistencePeriod! > 0); + if (!needCopy) { + cfg.deepCopy = undefined; + } else { + cfg.deepCopy = cfg.deepCopy || globalThis.structuredClone; + if (cfg.deepCopy === undefined) + throw new Error("structuredClone not available, setting memoryConfig.copyOnReturn = true requires a custom deepCopy function.") + } + cfg.copyOnReturn = cfg.copyOnReturn ?? false; + cfg.copyOnInsert = cfg.copyOnInsert ?? false; const prefix = cfg.tablePrefix || ""; cfg.itemsStorage = prefix + "Items"; cfg.accessTimesStorage = prefix + "AccessTimes"; diff --git a/test/testCacheWithMemory.js b/test/testCacheWithMemory.js index 5ef2c0f..d545433 100644 --- a/test/testCacheWithMemory.js +++ b/test/testCacheWithMemory.js @@ -402,3 +402,75 @@ test("Order of items works with in-memory updates in memory cache", async t => { } await defaultCache.close(); }); + + +test("copyOnInsert works with in-memory cache", async t => { + const cache = createFakeIdb({persistencePeriod: 5_000, copyOnInsert: true, memoryConfig: { maxItemsInMemory: 10 }}); + const obj1 = {a: "test1", b: 1}; + const copy = {...obj1}; + await cache.set(obj1.a, obj1); + obj1.b = 2; + t.deepEqual(await cache.get(obj1.a), copy); + await cache.close(); +}); + +test("copyOnInsert works with in-memory cache with setAll", async t => { + const cache = createFakeIdb({persistencePeriod: 5_000, copyOnInsert: true, memoryConfig: { maxItemsInMemory: 10 }}); + const obj1 = {a: "test1", b: 1}; + const obj2 = {a: "test2", b: 2}; + const copy1 = {...obj1}; + const copy2 = {...obj2}; + await cache.setAll(new Map([[obj1.a, obj1], [obj2.a, obj2]])); + obj1.b = -1; + obj2.b = -2; + t.deepEqual(await cache.get(obj1.a), copy1); + t.deepEqual(await cache.get(obj2.a), copy2); + await cache.close(); +}); + +test("copyOnInsert works with in-memory cache with immediate persistence", async t => { + const cache = createFakeIdb({persistencePeriod: 0, copyOnInsert: true, memoryConfig: { maxItemsInMemory: 10 }}); + const obj1 = {a: "test1", b: 1}; + const copy = {...obj1}; + await cache.set(obj1.a, obj1); + obj1.b = 2; + t.deepEqual(await cache.get(obj1.a), copy); + await cache.close(); +}); + + +test("copyOnReturn works with in-memory cache", async t => { + const cache = createFakeIdb({persistencePeriod: 5_000, copyOnReturn: true, memoryConfig: { maxItemsInMemory: 10 }}); + const obj1 = {a: "test1", b: 1}; + const copy = {...obj1}; + await cache.set(obj1.a, obj1); + (await cache.get(obj1.a)).b = 2; + t.deepEqual(await cache.get(obj1.a), copy); + await cache.close(); +}); + +test("copyOnReturn works with in-memory cache with getAll", async t => { + const cache = createFakeIdb({persistencePeriod: 5_000, copyOnReturn: true, memoryConfig: { maxItemsInMemory: 10 }}); + const obj1 = {a: "test1", b: 1}; + const obj2 = {a: "test2", b: 2}; + const copy1 = {...obj1}; + const copy2 = {...obj2}; + await cache.set(obj1.a, obj1); + await cache.set(obj2.a, obj2); + (await cache.get(obj1.a)).b = -1; + (await cache.get(obj2.a)).b = -2; + t.deepEqual(await cache.get(obj1.a), copy1); + t.deepEqual(await cache.get(obj2.a), copy2); + await cache.close(); +}); + +test("copyOnReturn works with in-memory cache with immediate persistence", async t => { + const cache = createFakeIdb({persistencePeriod: 0, copyOnReturn: true, memoryConfig: { maxItemsInMemory: 10 }}); + const obj1 = {a: "test1", b: 1}; + const copy = {...obj1}; + await cache.set(obj1.a, obj1); + (await cache.get(obj1.a)).b = 2; + t.deepEqual(await cache.get(obj1.a), copy); + await cache.close(); +}); + diff --git a/test/testDefaultCache.js b/test/testDefaultCache.js index a169c97..19d1e89 100644 --- a/test/testDefaultCache.js +++ b/test/testDefaultCache.js @@ -582,3 +582,76 @@ test("Cache persistence works with explicit close", async t => { await cache2.close(); }); +test("copyOnInsert works", async t => { + const cache = createFakeIdb({persistencePeriod: 5_000, copyOnInsert: true}); + const obj1 = {a: "test1", b: 1}; + const copy = {...obj1}; + await cache.set(obj1.a, obj1); + obj1.b = 2; + t.deepEqual(await cache.get(obj1.a), copy); + await cache.close(); +}); + +test("copyOnInsert works with setAll", async t => { + const cache = createFakeIdb({persistencePeriod: 5_000, copyOnInsert: true}); + const obj1 = {a: "test1", b: 1}; + const obj2 = {a: "test2", b: 2}; + const copy1 = {...obj1}; + const copy2 = {...obj2}; + await cache.setAll(new Map([[obj1.a, obj1], [obj2.a, obj2]])); + obj1.b = -1; + obj2.b = -2; + t.deepEqual(await cache.get(obj1.a), copy1); + t.deepEqual(await cache.get(obj2.a), copy2); + await cache.close(); +}); + +// copyOnInsert actually should be irrelevant in this case +test("copyOnInsert works with immediate persistence", async t => { + const cache = createFakeIdb({persistencePeriod: 0, copyOnInsert: true}); + const obj1 = {a: "test1", b: 1}; + const copy = {...obj1}; + await cache.set(obj1.a, obj1); + obj1.b = 2; + t.deepEqual(await cache.get(obj1.a), copy); + await cache.close(); +}); + + +test("copyOnReturn works", async t => { + const cache = createFakeIdb({persistencePeriod: 5_000, copyOnReturn: true}); + const obj1 = {a: "test1", b: 1}; + const copy = {...obj1}; + await cache.set(obj1.a, obj1); + (await cache.get(obj1.a)).b = 2; + t.deepEqual(await cache.get(obj1.a), copy); + await cache.close(); +}); + +test("copyOnReturn works with getAll", async t => { + const cache = createFakeIdb({persistencePeriod: 5_000, copyOnReturn: true}); + const obj1 = {a: "test1", b: 1}; + const obj2 = {a: "test2", b: 2}; + const copy1 = {...obj1}; + const copy2 = {...obj2}; + await cache.set(obj1.a, obj1); + await cache.set(obj2.a, obj2); + (await cache.get(obj1.a)).b = -1; + (await cache.get(obj2.a)).b = -2; + t.deepEqual(await cache.get(obj1.a), copy1); + t.deepEqual(await cache.get(obj2.a), copy2); + await cache.close(); +}); + +// copyOnReturn actually should be irrelevant in this case +test("copyOnReturn works with immediate persistence", async t => { + const cache = createFakeIdb({persistencePeriod: 0, copyOnReturn: true}); + const obj1 = {a: "test1", b: 1}; + const copy = {...obj1}; + await cache.set(obj1.a, obj1); + (await cache.get(obj1.a)).b = 2; + t.deepEqual(await cache.get(obj1.a), copy); + await cache.close(); +}); + +