From d30ce5ccc7fc798e3da5424947a466884fc7ef98 Mon Sep 17 00:00:00 2001 From: Juergen Kunz Date: Tue, 7 Apr 2026 16:24:42 +0000 Subject: [PATCH] fix(collectionfactory): isolate collection caching per database and add easy store replace semantics --- changelog.md | 7 ++++ test/test.collectionfactory.ts | 70 +++++++++++++++++++++++++++++++++ ts/00_commitinfo_data.ts | 2 +- ts/classes.collectionfactory.ts | 46 ++++++++++++++++++++-- ts/classes.easystore.ts | 15 ++++++- 5 files changed, 134 insertions(+), 6 deletions(-) create mode 100644 test/test.collectionfactory.ts diff --git a/changelog.md b/changelog.md index b372a33..41673a2 100644 --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,12 @@ # Changelog +## 2026-04-07 - 7.1.7 - fix(collectionfactory) +isolate collection caching per database and add easy store replace semantics + +- Change CollectionFactory to cache collections per SmartdataDb instance instead of by class name globally, preventing cross-database collection reuse. +- Add EasyStore.replace() for full object replacement while keeping writeAll() merge behavior for backward compatibility. +- Add regression tests covering multi-database collection isolation, replace() key removal, and writeAll() merge behavior. + ## 2026-04-05 - 7.1.6 - fix(collection) improve duplicate key error reporting on insert diff --git a/test/test.collectionfactory.ts b/test/test.collectionfactory.ts new file mode 100644 index 0000000..b3a808b --- /dev/null +++ b/test/test.collectionfactory.ts @@ -0,0 +1,70 @@ +import { tap, expect } from '@git.zone/tstest/tapbundle'; +import * as smartmongo from '@push.rocks/smartmongo'; +import * as smartdata from '../ts/index.js'; + +/** + * Regression tests for: + * + * 1. CollectionFactory per-db cache (previously keyed by class name + * alone, which made two SmartdataDb instances share a single + * collection bound to whichever db happened to request it first). + * 2. EasyStore.replace() — new atomic-replace method that clears keys + * not present in the new object. EasyStore.writeAll() still merges + * for backward compatibility. + */ + +let smartmongoInstance: smartmongo.SmartMongo; +let dbA: smartdata.SmartdataDb; +let dbB: smartdata.SmartdataDb; + +tap.test('setup: two dbs against the same replica set', async () => { + smartmongoInstance = await smartmongo.SmartMongo.createAndStart(); + const desc = await smartmongoInstance.getMongoDescriptor(); + dbA = new smartdata.SmartdataDb({ ...desc, mongoDbName: 'cf_test_a' }); + dbB = new smartdata.SmartdataDb({ ...desc, mongoDbName: 'cf_test_b' }); + await dbA.init(); + await dbB.init(); +}); + +tap.test('CollectionFactory: same class name in two dbs yields two collections', async () => { + const easyA = await dbA.createEasyStore<{ marker: string }>('shared'); + const easyB = await dbB.createEasyStore<{ marker: string }>('shared'); + await easyA.writeKey('marker', 'A'); + await easyB.writeKey('marker', 'B'); + const fromA = await easyA.readKey('marker'); + const fromB = await easyB.readKey('marker'); + expect(fromA).toEqual('A'); + // Under the old singleton bug, this would be 'A' because the second + // createEasyStore() call would receive the collection bound to dbA. + expect(fromB).toEqual('B'); +}); + +tap.test('EasyStore.replace: drops keys not present in the new object', async () => { + const store = await dbA.createEasyStore<{ a?: string; b?: string }>('replace_test'); + await store.writeKey('a', '1'); + await store.writeKey('b', '2'); + expect(await store.readKey('a')).toEqual('1'); + expect(await store.readKey('b')).toEqual('2'); + await store.replace({ a: 'only-a' }); + expect(await store.readKey('a')).toEqual('only-a'); + // `b` must be gone — this is the whole point of replace() over writeAll(). + expect(await store.readKey('b')).toBeUndefined(); +}); + +tap.test('EasyStore.writeAll: still merges (back-compat)', async () => { + const store = await dbA.createEasyStore<{ a?: string; b?: string }>('merge_test'); + await store.writeAll({ a: '1' }); + await store.writeAll({ b: '2' }); + expect(await store.readKey('a')).toEqual('1'); + expect(await store.readKey('b')).toEqual('2'); +}); + +tap.test('teardown', async () => { + await dbA.mongoDb.dropDatabase(); + await dbB.mongoDb.dropDatabase(); + await dbA.close(); + await dbB.close(); + await smartmongoInstance.stop(); +}); + +export default tap.start(); diff --git a/ts/00_commitinfo_data.ts b/ts/00_commitinfo_data.ts index 4a0a039..3bb5653 100644 --- a/ts/00_commitinfo_data.ts +++ b/ts/00_commitinfo_data.ts @@ -3,6 +3,6 @@ */ export const commitinfo = { name: '@push.rocks/smartdata', - version: '7.1.6', + version: '7.1.7', description: 'An advanced library for NoSQL data organization and manipulation using TypeScript with support for MongoDB, data validation, collections, and custom data types.' } diff --git a/ts/classes.collectionfactory.ts b/ts/classes.collectionfactory.ts index 7a1d87e..64f75c5 100644 --- a/ts/classes.collectionfactory.ts +++ b/ts/classes.collectionfactory.ts @@ -2,13 +2,51 @@ import * as plugins from './plugins.js'; import { SmartdataCollection } from './classes.collection.js'; import { SmartdataDb } from './classes.db.js'; +/** + * Per-SmartdataDb collection cache. + * + * Historically this class keyed its cache by class name alone, which meant + * the first SmartdataDb to request a collection of a given class name + * "won" — every subsequent call from a different SmartdataDb instance + * received the cached collection bound to the first db. That silently + * broke multi-tenant SaaS apps (one db per tenant), tests instantiating + * multiple SmartdataDbs in sequence, and any in-process db cluster switch. + * + * The cache is now keyed by `(SmartdataDb instance, className)` using a + * WeakMap of db → Map. Entries are GC'd + * with their parent db automatically. + */ export class CollectionFactory { - public collections: { [key: string]: SmartdataCollection } = {}; + private perDbCollections: WeakMap>> = new WeakMap(); public getCollection = (nameArg: string, dbArg: SmartdataDb): SmartdataCollection => { - if (!this.collections[nameArg] && dbArg instanceof SmartdataDb) { - this.collections[nameArg] = new SmartdataCollection(nameArg, dbArg); + if (!(dbArg instanceof SmartdataDb)) { + // Preserve the historical behavior of returning undefined-ish for + // non-db args. All in-repo callers already guard on instanceof + // before using the result (see classes.collection.ts). + return undefined as unknown as SmartdataCollection; } - return this.collections[nameArg]; + let dbMap = this.perDbCollections.get(dbArg); + if (!dbMap) { + dbMap = new Map(); + this.perDbCollections.set(dbArg, dbMap); + } + let coll = dbMap.get(nameArg); + if (!coll) { + coll = new SmartdataCollection(nameArg, dbArg); + dbMap.set(nameArg, coll); + } + return coll; }; + + /** + * @deprecated Internal back-compat shim. The previous field was a public + * Record but was not part of the + * documented public API. WeakMap is not iterable, so this getter returns + * an empty object — anyone actually relying on the old shape would get + * clean nothing rather than wrong-db data. Will be removed in 8.0. + */ + public get collections(): { [key: string]: SmartdataCollection } { + return {}; + } } diff --git a/ts/classes.easystore.ts b/ts/classes.easystore.ts index 52055aa..eb8c3ce 100644 --- a/ts/classes.easystore.ts +++ b/ts/classes.easystore.ts @@ -93,7 +93,9 @@ export class EasyStore { } /** - * writes all keyValue pairs in the object argument + * merges all keyValue pairs from the object argument into the store. + * Existing keys that are not present in `keyValueObject` are preserved. + * To overwrite the entire store and drop missing keys, use `replace()`. */ public async writeAll(keyValueObject: Partial) { const easyStore = await this.getEasyStore(); @@ -101,6 +103,17 @@ export class EasyStore { await easyStore.save(); } + /** + * atomically replaces the entire store with the given object. + * Unlike `writeAll` (which merges), `replace` clears any keys not + * present in `keyValueObject`. Useful when you need to drop a key. + */ + public async replace(keyValueObject: Partial) { + const easyStore = await this.getEasyStore(); + easyStore.data = { ...keyValueObject }; + await easyStore.save(); + } + /** * wipes a key value store from disk */