Skip to content

Commit ce77f59

Browse files
committed
fix(document): handle nested paths in optimisticConcurrency include/exclude
When using `optimisticConcurrency` with an include array or an exclude object, nested paths were not handled properly. Excluding `profile` did not exclude `profile.firstName`, and including `profile.address.country` did not trigger versioning when setting `profile.address`. Added two helper functions that check parent-child path relationships: - `_isExcludedByOptCon`: a path is excluded if it or any parent prefix is in the exclude set - `_isIncludedByOptCon`: a path is included if it or any parent prefix is in the include set, or if any included path is a child of the modified path Fixes #16054
1 parent 37ff792 commit ce77f59

File tree

2 files changed

+141
-4
lines changed

2 files changed

+141
-4
lines changed

lib/document.js

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5055,17 +5055,17 @@ Document.prototype.$__delta = function $__delta(pathsToSave, pathsToSaveSet) {
50555055
const optCon = new Set(optimisticConcurrency);
50565056
const modPaths = this.modifiedPaths();
50575057
const hasRelevantModPaths = pathsToSave == null ?
5058-
modPaths.find(path => optCon.has(path)) :
5059-
modPaths.find(path => optCon.has(path) && isInPathsToSave(path, pathsToSaveSet, pathsToSave));
5058+
modPaths.find(path => _isIncludedByOptCon(path, optCon)) :
5059+
modPaths.find(path => _isIncludedByOptCon(path, optCon) && isInPathsToSave(path, pathsToSaveSet, pathsToSave));
50605060
if (hasRelevantModPaths) {
50615061
this.$__.version = dirty.length ? VERSION_ALL : VERSION_WHERE;
50625062
}
50635063
} else if (Array.isArray(optimisticConcurrency?.exclude)) {
50645064
const excluded = new Set(optimisticConcurrency.exclude);
50655065
const modPaths = this.modifiedPaths();
50665066
const hasRelevantModPaths = pathsToSave == null ?
5067-
modPaths.find(path => !excluded.has(path)) :
5068-
modPaths.find(path => !excluded.has(path) && isInPathsToSave(path, pathsToSaveSet, pathsToSave));
5067+
modPaths.find(path => !_isExcludedByOptCon(path, excluded)) :
5068+
modPaths.find(path => !_isExcludedByOptCon(path, excluded) && isInPathsToSave(path, pathsToSaveSet, pathsToSave));
50695069
if (hasRelevantModPaths) {
50705070
this.$__.version = dirty.length ? VERSION_ALL : VERSION_WHERE;
50715071
}
@@ -5588,6 +5588,56 @@ Document.prototype._applyVersionIncrement = function _applyVersionIncrement() {
55885588
}
55895589
};
55905590

5591+
/*!
5592+
* Check if a path is included by an optimisticConcurrency include list,
5593+
* handling nested paths in both directions.
5594+
*/
5595+
5596+
function _isIncludedByOptCon(path, includeSet) {
5597+
if (includeSet.has(path)) {
5598+
return true;
5599+
}
5600+
// Check if any parent prefix is included (e.g. 'profile' includes 'profile.name')
5601+
const pieces = path.split('.');
5602+
let cur = pieces[0];
5603+
for (let i = 1; i < pieces.length; ++i) {
5604+
if (includeSet.has(cur)) {
5605+
return true;
5606+
}
5607+
cur += '.' + pieces[i];
5608+
}
5609+
// Check if any included path is a child (e.g. modifying 'profile.address' is
5610+
// relevant when 'profile.address.country' is included)
5611+
const prefix = path + '.';
5612+
for (const p of includeSet) {
5613+
if (p.startsWith(prefix)) {
5614+
return true;
5615+
}
5616+
}
5617+
return false;
5618+
}
5619+
5620+
/*!
5621+
* Check if a path is excluded by an optimisticConcurrency exclude list,
5622+
* handling nested paths so that excluding a parent also excludes children.
5623+
*/
5624+
5625+
function _isExcludedByOptCon(path, excludeSet) {
5626+
if (excludeSet.has(path)) {
5627+
return true;
5628+
}
5629+
// Check if any parent prefix is excluded (e.g. 'profile' excludes 'profile.firstName')
5630+
const pieces = path.split('.');
5631+
let cur = pieces[0];
5632+
for (let i = 1; i < pieces.length; ++i) {
5633+
if (excludeSet.has(cur)) {
5634+
return true;
5635+
}
5636+
cur += '.' + pieces[i];
5637+
}
5638+
return false;
5639+
}
5640+
55915641
/*!
55925642
* Module exports.
55935643
*/

test/versioning.test.js

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -892,5 +892,92 @@ describe('versioning', function() {
892892

893893
return { user };
894894
}
895+
896+
describe('nested paths (gh-16054)', function() {
897+
it('exclude: excluding parent also excludes child paths', async function() {
898+
const profileSchema = new Schema({ firstName: String, lastName: String }, { _id: false });
899+
const schema = new Schema({
900+
profile: profileSchema,
901+
balance: Number
902+
}, { optimisticConcurrency: { exclude: ['profile'] } });
903+
904+
const User = db.model('TestOCNested1', schema);
905+
const user = await User.create({ profile: { firstName: 'Alice', lastName: 'Smith' }, balance: 100 });
906+
907+
user.profile.firstName = 'Bob';
908+
user.$__delta();
909+
910+
assert.strictEqual(user.$__.version, undefined);
911+
});
912+
913+
it('exclude: modifying non-excluded path still triggers version when excluded parent has modified children', async function() {
914+
const profileSchema = new Schema({ firstName: String, lastName: String }, { _id: false });
915+
const schema = new Schema({
916+
profile: profileSchema,
917+
balance: Number
918+
}, { optimisticConcurrency: { exclude: ['profile'] } });
919+
920+
const User = db.model('TestOCNested2', schema);
921+
const user = await User.create({ profile: { firstName: 'Alice', lastName: 'Smith' }, balance: 100 });
922+
923+
user.profile.firstName = 'Bob';
924+
user.balance = 200;
925+
user.$__delta();
926+
927+
assert.strictEqual(user.$__.version, VERSION_ALL);
928+
});
929+
930+
it('include: including parent also includes child paths', async function() {
931+
const profileSchema = new Schema({ firstName: String, lastName: String }, { _id: false });
932+
const schema = new Schema({
933+
profile: profileSchema,
934+
balance: Number
935+
}, { optimisticConcurrency: ['profile'] });
936+
937+
const User = db.model('TestOCNested3', schema);
938+
const user = await User.create({ profile: { firstName: 'Alice', lastName: 'Smith' }, balance: 100 });
939+
940+
user.profile.firstName = 'Bob';
941+
user.$__delta();
942+
943+
assert.strictEqual(user.$__.version, VERSION_ALL);
944+
});
945+
946+
it('include: modifying parent triggers version when child is included', async function() {
947+
const addressSchema = new Schema({ street: String, country: String }, { _id: false });
948+
const profileSchema = new Schema({ firstName: String, address: addressSchema }, { _id: false });
949+
const schema = new Schema({
950+
profile: profileSchema,
951+
balance: Number
952+
}, { optimisticConcurrency: ['profile.address.country'] });
953+
954+
const User = db.model('TestOCNested4', schema);
955+
const user = await User.create({
956+
profile: { firstName: 'Alice', address: { street: '123 Main', country: 'US' } },
957+
balance: 100
958+
});
959+
960+
user.set('profile.address', { street: '456 Oak', country: 'UK' });
961+
user.$__delta();
962+
963+
assert.strictEqual(user.$__.version, VERSION_ALL);
964+
});
965+
966+
it('include: modifying non-included nested path does not trigger version', async function() {
967+
const profileSchema = new Schema({ firstName: String, lastName: String }, { _id: false });
968+
const schema = new Schema({
969+
profile: profileSchema,
970+
balance: Number
971+
}, { optimisticConcurrency: ['balance'] });
972+
973+
const User = db.model('TestOCNested5', schema);
974+
const user = await User.create({ profile: { firstName: 'Alice', lastName: 'Smith' }, balance: 100 });
975+
976+
user.profile.firstName = 'Bob';
977+
user.$__delta();
978+
979+
assert.strictEqual(user.$__.version, undefined);
980+
});
981+
});
895982
});
896983
});

0 commit comments

Comments
 (0)