Follow-up from PR #52
This issue tracks two edge-case bugs identified during review of PR #52 in packages/common/src/utils/injectModel.js that were deferred to keep the merge scope focused.
Fix 1 — Rollback loop may drop pre-existing indexes (Critical)
File: packages/common/src/utils/injectModel.js
createdIndexes.push(indexName) runs unconditionally after createIndex(). Since MongoDB's createIndex is idempotent and succeeds silently on pre-existing indexes, the rollback loop can call dropIndex(indexName) on indexes that already existed before this request, causing potential data integrity loss on live collections.
Proposed fix: Snapshot existing index names before the loop and only push newly created indexes:
// Before the loop
const existingIndexes = await Model.collection.indexes();
const existingIndexNames = new Set(existingIndexes.map((idx) => idx.name));
// Inside the loop, after createIndex:
const createdName = await Model.collection.createIndex({ [normalizedKey]: 1 }, indexOptions);
if (!existingIndexNames.has(createdName)) {
createdIndexes.push(createdName);
}
Fix 2 — getUniqueFieldFilter excludes missing-field docs for required fields (Major)
File: packages/common/src/utils/injectModel.js
getUniqueFieldFilter returns { [fieldKey]: { $exists: true } } for required fields. This excludes documents missing the field from the duplicate precheck. However, MongoDB's unique index still treats multiple missing-field documents as duplicates (they all resolve to null), so createIndex() can still fail with error 11000 even after the precheck passes.
Proposed fix: Use an unrestricted filter for required fields:
function getUniqueFieldFilter(fieldKey, isRequired) {
if (isRequired) {
return {}; // scan ALL docs, including those missing the field
}
return { [fieldKey]: { $exists: true, $ne: null } };
}
Requested by: @yash-pouranik
Reference PR: #52
Review thread: #52 (review)
Follow-up from PR #52
This issue tracks two edge-case bugs identified during review of PR #52 in
packages/common/src/utils/injectModel.jsthat were deferred to keep the merge scope focused.Fix 1 — Rollback loop may drop pre-existing indexes (Critical)
File:
packages/common/src/utils/injectModel.jscreatedIndexes.push(indexName)runs unconditionally aftercreateIndex(). Since MongoDB'screateIndexis idempotent and succeeds silently on pre-existing indexes, the rollback loop can calldropIndex(indexName)on indexes that already existed before this request, causing potential data integrity loss on live collections.Proposed fix: Snapshot existing index names before the loop and only push newly created indexes:
Fix 2 —
getUniqueFieldFilterexcludes missing-field docs for required fields (Major)File:
packages/common/src/utils/injectModel.jsgetUniqueFieldFilterreturns{ [fieldKey]: { $exists: true } }for required fields. This excludes documents missing the field from the duplicate precheck. However, MongoDB's unique index still treats multiple missing-field documents as duplicates (they all resolve tonull), socreateIndex()can still fail with error 11000 even after the precheck passes.Proposed fix: Use an unrestricted filter for required fields:
Requested by: @yash-pouranik
Reference PR: #52
Review thread: #52 (review)