Compare commits
8 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 4058e51dfb | |||
| d30ce5ccc7 | |||
| 97d9302e71 | |||
| fa60f625e9 | |||
| afd79cfabc | |||
| f3a4a3bbba | |||
| 78207ffad6 | |||
| abf84359b4 |
29
changelog.md
29
changelog.md
@@ -1,5 +1,34 @@
|
||||
# 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
|
||||
|
||||
- Wrap insertOne() in error handling to detect MongoDB duplicate key conflicts
|
||||
- Log a clearer message with the collection name and identifiable object when unique indexes are involved
|
||||
- Guide callers to use getInstance() or save() on a db-retrieved instance when a duplicate already exists
|
||||
|
||||
## 2026-04-05 - 7.1.5 - fix(collection)
|
||||
ensure unique indexes are marked before upsert operations
|
||||
|
||||
- calls unique index marking during collection updates before executing upsert logic
|
||||
- helps keep update behavior aligned with index handling already applied on inserts
|
||||
|
||||
## 2026-04-05 - 7.1.4 - fix(collection)
|
||||
improve index creation resilience and add collection integrity checks
|
||||
|
||||
- Handle MongoDB index creation failures with structured logging instead of failing silently or racing on repeated attempts
|
||||
- Log duplicate field values when unique index creation fails due to existing duplicate data
|
||||
- Await unique and regular index creation during insert operations to ensure index setup completes predictably
|
||||
- Add collection integrity checks for estimated vs actual document counts and duplicate values on tracked unique fields
|
||||
- Expose collection integrity checks through the document class API
|
||||
|
||||
## 2026-03-26 - 7.1.3 - fix(deps)
|
||||
bump development dependencies for tooling and Node types
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "@push.rocks/smartdata",
|
||||
"version": "7.1.3",
|
||||
"version": "7.1.7",
|
||||
"private": false,
|
||||
"description": "An advanced library for NoSQL data organization and manipulation using TypeScript with support for MongoDB, data validation, collections, and custom data types.",
|
||||
"exports": {
|
||||
|
||||
70
test/test.collectionfactory.ts
Normal file
70
test/test.collectionfactory.ts
Normal file
@@ -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();
|
||||
@@ -3,6 +3,6 @@
|
||||
*/
|
||||
export const commitinfo = {
|
||||
name: '@push.rocks/smartdata',
|
||||
version: '7.1.3',
|
||||
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.'
|
||||
}
|
||||
|
||||
@@ -216,8 +216,15 @@ export class SmartdataCollection<T> {
|
||||
const indexSpec: Record<string, 'text'> = {};
|
||||
searchableFields.forEach(f => { indexSpec[f] = 'text'; });
|
||||
// Cast to any to satisfy TypeScript IndexSpecification typing
|
||||
await this.mongoDbCollection.createIndex(indexSpec as any, { name: 'smartdata_text_index' });
|
||||
this.textIndexCreated = true;
|
||||
try {
|
||||
await this.mongoDbCollection.createIndex(indexSpec as any, { name: 'smartdata_text_index' });
|
||||
this.textIndexCreated = true;
|
||||
} catch (err: any) {
|
||||
logger.log(
|
||||
'warn',
|
||||
`Failed to create text index on fields [${searchableFields.join(', ')}] in collection "${this.collectionName}": ${err?.message || String(err)}`
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -228,11 +235,25 @@ export class SmartdataCollection<T> {
|
||||
public async markUniqueIndexes(keyArrayArg: string[] = []) {
|
||||
for (const key of keyArrayArg) {
|
||||
if (!this.uniqueIndexes.includes(key)) {
|
||||
await this.mongoDbCollection.createIndex({ [key]: 1 }, {
|
||||
unique: true,
|
||||
});
|
||||
// make sure we only call this once and not for every doc we create
|
||||
// Claim the slot immediately to prevent concurrent inserts from retrying
|
||||
this.uniqueIndexes.push(key);
|
||||
try {
|
||||
await this.mongoDbCollection.createIndex({ [key]: 1 }, {
|
||||
unique: true,
|
||||
});
|
||||
} catch (err: any) {
|
||||
const errorCode = err?.code || err?.codeName || 'unknown';
|
||||
const errorMessage = err?.message || String(err);
|
||||
logger.log(
|
||||
'error',
|
||||
`Failed to create unique index on field "${key}" in collection "${this.collectionName}". ` +
|
||||
`MongoDB error [${errorCode}]: ${errorMessage}. ` +
|
||||
`Uniqueness constraint on "${key}" is NOT enforced.`
|
||||
);
|
||||
if (errorCode === 11000 || errorCode === 'DuplicateKey' || String(errorMessage).includes('E11000')) {
|
||||
await this.logDuplicatesForField(key);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -245,16 +266,66 @@ export class SmartdataCollection<T> {
|
||||
// Check if we've already created this index
|
||||
const indexKey = indexDef.field;
|
||||
if (!this.regularIndexes.some(i => i.field === indexKey)) {
|
||||
await this.mongoDbCollection.createIndex(
|
||||
{ [indexDef.field]: 1 }, // Simple single-field index
|
||||
indexDef.options
|
||||
);
|
||||
// Track that we've created this index
|
||||
// Claim the slot immediately to prevent concurrent retries
|
||||
this.regularIndexes.push(indexDef);
|
||||
try {
|
||||
await this.mongoDbCollection.createIndex(
|
||||
{ [indexDef.field]: 1 }, // Simple single-field index
|
||||
indexDef.options
|
||||
);
|
||||
} catch (err: any) {
|
||||
const errorCode = err?.code || err?.codeName || 'unknown';
|
||||
const errorMessage = err?.message || String(err);
|
||||
logger.log(
|
||||
'warn',
|
||||
`Failed to create index on field "${indexKey}" in collection "${this.collectionName}". ` +
|
||||
`MongoDB error [${errorCode}]: ${errorMessage}.`
|
||||
);
|
||||
if (
|
||||
indexDef.options?.unique &&
|
||||
(errorCode === 11000 || errorCode === 'DuplicateKey' || String(errorMessage).includes('E11000'))
|
||||
) {
|
||||
await this.logDuplicatesForField(indexKey);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Logs duplicate values for a field to help diagnose unique index creation failures.
|
||||
*/
|
||||
private async logDuplicatesForField(field: string): Promise<void> {
|
||||
try {
|
||||
const pipeline = [
|
||||
{ $group: { _id: `$${field}`, count: { $sum: 1 }, ids: { $push: '$_id' } } },
|
||||
{ $match: { count: { $gt: 1 } } },
|
||||
{ $limit: 5 },
|
||||
];
|
||||
const duplicates = await this.mongoDbCollection.aggregate(pipeline).toArray();
|
||||
if (duplicates.length > 0) {
|
||||
for (const dup of duplicates) {
|
||||
logger.log(
|
||||
'warn',
|
||||
`Duplicate values for "${field}" in "${this.collectionName}": ` +
|
||||
`value=${JSON.stringify(dup._id)} appears ${dup.count} times ` +
|
||||
`(document _ids: ${JSON.stringify(dup.ids.slice(0, 5))})`
|
||||
);
|
||||
}
|
||||
logger.log(
|
||||
'warn',
|
||||
`Unique index on "${field}" in "${this.collectionName}" was NOT created. ` +
|
||||
`Resolve duplicates and restart to enforce uniqueness.`
|
||||
);
|
||||
}
|
||||
} catch (aggErr: any) {
|
||||
logger.log(
|
||||
'warn',
|
||||
`Could not identify duplicate documents for field "${field}" in "${this.collectionName}": ${aggErr?.message || String(aggErr)}`
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* adds a validation function that all newly inserted and updated objects have to pass
|
||||
*/
|
||||
@@ -295,6 +366,28 @@ export class SmartdataCollection<T> {
|
||||
const cursor = this.mongoDbCollection.find(filterObject, { session: opts?.session });
|
||||
const result = await cursor.toArray();
|
||||
cursor.close();
|
||||
|
||||
// In-memory check for duplicate _id values (should never happen)
|
||||
if (result.length > 0) {
|
||||
const idSet = new Set<string>();
|
||||
const duplicateIds: string[] = [];
|
||||
for (const doc of result) {
|
||||
const idStr = String(doc._id);
|
||||
if (idSet.has(idStr)) {
|
||||
duplicateIds.push(idStr);
|
||||
} else {
|
||||
idSet.add(idStr);
|
||||
}
|
||||
}
|
||||
if (duplicateIds.length > 0) {
|
||||
logger.log(
|
||||
'error',
|
||||
`Integrity issue in "${this.collectionName}": found ${duplicateIds.length} duplicate _id values ` +
|
||||
`in findAll results: [${duplicateIds.slice(0, 5).join(', ')}]. This should never happen.`
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
@@ -346,16 +439,30 @@ export class SmartdataCollection<T> {
|
||||
): Promise<any> {
|
||||
await this.init();
|
||||
await this.checkDoc(dbDocArg);
|
||||
this.markUniqueIndexes(dbDocArg.uniqueIndexes);
|
||||
|
||||
await this.markUniqueIndexes(dbDocArg.uniqueIndexes);
|
||||
|
||||
// Create regular indexes if available
|
||||
if (dbDocArg.regularIndexes && dbDocArg.regularIndexes.length > 0) {
|
||||
this.createRegularIndexes(dbDocArg.regularIndexes);
|
||||
await this.createRegularIndexes(dbDocArg.regularIndexes);
|
||||
}
|
||||
|
||||
const saveableObject = await dbDocArg.createSavableObject() as any;
|
||||
const result = await this.mongoDbCollection.insertOne(saveableObject, { session: opts?.session });
|
||||
return result;
|
||||
try {
|
||||
const result = await this.mongoDbCollection.insertOne(saveableObject, { session: opts?.session });
|
||||
return result;
|
||||
} catch (err: any) {
|
||||
const isDuplicateKey = err?.code === 11000 || err?.codeName === 'DuplicateKey';
|
||||
if (isDuplicateKey && dbDocArg.uniqueIndexes && dbDocArg.uniqueIndexes.length > 0) {
|
||||
const identifiableObject = await dbDocArg.createIdentifiableObject();
|
||||
logger.log(
|
||||
'error',
|
||||
`Duplicate key conflict in "${this.collectionName}" on insert. ` +
|
||||
`A document with ${JSON.stringify(identifiableObject)} already exists. ` +
|
||||
`Use getInstance() to retrieve the existing document, or update it via save() on a db-retrieved instance.`
|
||||
);
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -367,6 +474,7 @@ export class SmartdataCollection<T> {
|
||||
): Promise<any> {
|
||||
await this.init();
|
||||
await this.checkDoc(dbDocArg);
|
||||
await this.markUniqueIndexes(dbDocArg.uniqueIndexes);
|
||||
const identifiableObject = await dbDocArg.createIdentifiableObject();
|
||||
const saveableObject = await dbDocArg.createSavableObject() as any;
|
||||
const updateableObject: any = {};
|
||||
@@ -402,6 +510,74 @@ export class SmartdataCollection<T> {
|
||||
return this.mongoDbCollection.countDocuments(filterObject, { session: opts?.session });
|
||||
}
|
||||
|
||||
/**
|
||||
* Runs an integrity check on the collection.
|
||||
* Compares estimated vs actual document count and checks for duplicates on unique index fields.
|
||||
*/
|
||||
public async checkCollectionIntegrity(): Promise<{
|
||||
ok: boolean;
|
||||
estimatedCount: number;
|
||||
actualCount: number;
|
||||
duplicateFields: Array<{ field: string; duplicateValues: number }>;
|
||||
}> {
|
||||
await this.init();
|
||||
const result = {
|
||||
ok: true,
|
||||
estimatedCount: 0,
|
||||
actualCount: 0,
|
||||
duplicateFields: [] as Array<{ field: string; duplicateValues: number }>,
|
||||
};
|
||||
|
||||
try {
|
||||
result.estimatedCount = await this.mongoDbCollection.estimatedDocumentCount();
|
||||
result.actualCount = await this.mongoDbCollection.countDocuments({});
|
||||
|
||||
if (result.estimatedCount !== result.actualCount) {
|
||||
result.ok = false;
|
||||
logger.log(
|
||||
'warn',
|
||||
`Integrity check on "${this.collectionName}": estimatedDocumentCount=${result.estimatedCount} ` +
|
||||
`but countDocuments=${result.actualCount}. Possible data inconsistency.`
|
||||
);
|
||||
}
|
||||
|
||||
// Check for duplicates on each tracked unique index field
|
||||
for (const field of this.uniqueIndexes) {
|
||||
try {
|
||||
const pipeline = [
|
||||
{ $group: { _id: `$${field}`, count: { $sum: 1 } } },
|
||||
{ $match: { count: { $gt: 1 } } },
|
||||
{ $count: 'total' },
|
||||
];
|
||||
const countResult = await this.mongoDbCollection.aggregate(pipeline).toArray();
|
||||
const dupCount = countResult[0]?.total || 0;
|
||||
if (dupCount > 0) {
|
||||
result.ok = false;
|
||||
result.duplicateFields.push({ field, duplicateValues: dupCount });
|
||||
logger.log(
|
||||
'warn',
|
||||
`Integrity check on "${this.collectionName}": field "${field}" has ${dupCount} values with duplicates ` +
|
||||
`despite being marked as unique.`
|
||||
);
|
||||
}
|
||||
} catch (fieldErr: any) {
|
||||
logger.log(
|
||||
'warn',
|
||||
`Integrity check: could not verify uniqueness of "${field}" in "${this.collectionName}": ${fieldErr?.message || String(fieldErr)}`
|
||||
);
|
||||
}
|
||||
}
|
||||
} catch (err: any) {
|
||||
result.ok = false;
|
||||
logger.log(
|
||||
'error',
|
||||
`Integrity check failed for "${this.collectionName}": ${err?.message || String(err)}`
|
||||
);
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
* checks a Doc for constraints
|
||||
* if this.objectValidation is not set it passes.
|
||||
|
||||
@@ -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<className, SmartdataCollection>. Entries are GC'd
|
||||
* with their parent db automatically.
|
||||
*/
|
||||
export class CollectionFactory {
|
||||
public collections: { [key: string]: SmartdataCollection<any> } = {};
|
||||
private perDbCollections: WeakMap<SmartdataDb, Map<string, SmartdataCollection<any>>> = new WeakMap();
|
||||
|
||||
public getCollection = (nameArg: string, dbArg: SmartdataDb): SmartdataCollection<any> => {
|
||||
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<any>;
|
||||
}
|
||||
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<className, SmartdataCollection> 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<any> } {
|
||||
return {};
|
||||
}
|
||||
}
|
||||
|
||||
@@ -597,6 +597,17 @@ export class SmartDataDbDoc<T extends TImplements, TImplements, TManager extends
|
||||
return await collection.getCount(filterArg);
|
||||
}
|
||||
|
||||
/**
|
||||
* Runs an integrity check on this collection.
|
||||
* Returns a summary with estimated vs actual counts and any duplicate unique fields.
|
||||
*/
|
||||
public static async checkCollectionIntegrity<T>(
|
||||
this: plugins.tsclass.typeFest.Class<T>,
|
||||
) {
|
||||
const collection: SmartdataCollection<T> = (this as any).collection;
|
||||
return await collection.checkCollectionIntegrity();
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a MongoDB filter from a Lucene query string
|
||||
* @param luceneQuery Lucene query string
|
||||
|
||||
@@ -93,7 +93,9 @@ export class EasyStore<T> {
|
||||
}
|
||||
|
||||
/**
|
||||
* 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<T>) {
|
||||
const easyStore = await this.getEasyStore();
|
||||
@@ -101,6 +103,17 @@ export class EasyStore<T> {
|
||||
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<T>) {
|
||||
const easyStore = await this.getEasyStore();
|
||||
easyStore.data = { ...keyValueObject };
|
||||
await easyStore.save();
|
||||
}
|
||||
|
||||
/**
|
||||
* wipes a key value store from disk
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user