Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 54 additions & 4 deletions lib/document.js
Original file line number Diff line number Diff line change
Expand Up @@ -5055,17 +5055,17 @@ Document.prototype.$__delta = function $__delta(pathsToSave, pathsToSaveSet) {
const optCon = new Set(optimisticConcurrency);
const modPaths = this.modifiedPaths();
const hasRelevantModPaths = pathsToSave == null ?
modPaths.find(path => optCon.has(path)) :
modPaths.find(path => optCon.has(path) && isInPathsToSave(path, pathsToSaveSet, pathsToSave));
modPaths.find(path => _isIncludedByOptCon(path, optCon)) :
modPaths.find(path => _isIncludedByOptCon(path, optCon) && isInPathsToSave(path, pathsToSaveSet, pathsToSave));
if (hasRelevantModPaths) {
this.$__.version = dirty.length ? VERSION_ALL : VERSION_WHERE;
}
} else if (Array.isArray(optimisticConcurrency?.exclude)) {
const excluded = new Set(optimisticConcurrency.exclude);
const modPaths = this.modifiedPaths();
const hasRelevantModPaths = pathsToSave == null ?
modPaths.find(path => !excluded.has(path)) :
modPaths.find(path => !excluded.has(path) && isInPathsToSave(path, pathsToSaveSet, pathsToSave));
modPaths.find(path => !_isExcludedByOptCon(path, excluded)) :
modPaths.find(path => !_isExcludedByOptCon(path, excluded) && isInPathsToSave(path, pathsToSaveSet, pathsToSave));
if (hasRelevantModPaths) {
this.$__.version = dirty.length ? VERSION_ALL : VERSION_WHERE;
}
Expand Down Expand Up @@ -5588,6 +5588,56 @@ Document.prototype._applyVersionIncrement = function _applyVersionIncrement() {
}
};

/*!
* Check if a path is included by an optimisticConcurrency include list,
* handling nested paths in both directions.
*/

function _isIncludedByOptCon(path, includeSet) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to see if there is a way to reuse split values in this function similar to hasIncludedChildren for projections in lib/document.js. This check is very expensive - lots of split() calls, loops. It would be ideal to avoid this by precomputing the split values from this.$__schema.options.optimisticConcurrency

if (includeSet.has(path)) {
return true;
}
// Check if any parent prefix is included (e.g. 'profile' includes 'profile.name')
const pieces = path.split('.');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to make sure to check if path contains '.' before splitting for performance

let cur = pieces[0];
for (let i = 1; i < pieces.length; ++i) {
if (includeSet.has(cur)) {
return true;
}
cur += '.' + pieces[i];
}
// Check if any included path is a child (e.g. modifying 'profile.address' is
// relevant when 'profile.address.country' is included)
const prefix = path + '.';
for (const p of includeSet) {
if (p.startsWith(prefix)) {
return true;
}
}
Comment on lines +5609 to +5616
return false;
}

/*!
* Check if a path is excluded by an optimisticConcurrency exclude list,
* handling nested paths so that excluding a parent also excludes children.
*/

function _isExcludedByOptCon(path, excludeSet) {
if (excludeSet.has(path)) {
return true;
}
// Check if any parent prefix is excluded (e.g. 'profile' excludes 'profile.firstName')
const pieces = path.split('.');
let cur = pieces[0];
for (let i = 1; i < pieces.length; ++i) {
if (excludeSet.has(cur)) {
return true;
}
cur += '.' + pieces[i];
}
return false;
}

/*!
* Module exports.
*/
Expand Down
87 changes: 87 additions & 0 deletions test/versioning.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -892,5 +892,92 @@ describe('versioning', function() {

return { user };
}

describe('nested paths (gh-16054)', function() {
it('exclude: excluding parent also excludes child paths', async function() {
const profileSchema = new Schema({ firstName: String, lastName: String }, { _id: false });
const schema = new Schema({
profile: profileSchema,
balance: Number
}, { optimisticConcurrency: { exclude: ['profile'] } });

const User = db.model('TestOCNested1', schema);
const user = await User.create({ profile: { firstName: 'Alice', lastName: 'Smith' }, balance: 100 });

user.profile.firstName = 'Bob';
user.$__delta();

assert.strictEqual(user.$__.version, undefined);
});

it('exclude: modifying non-excluded path still triggers version when excluded parent has modified children', async function() {
const profileSchema = new Schema({ firstName: String, lastName: String }, { _id: false });
const schema = new Schema({
profile: profileSchema,
balance: Number
}, { optimisticConcurrency: { exclude: ['profile'] } });

const User = db.model('TestOCNested2', schema);
const user = await User.create({ profile: { firstName: 'Alice', lastName: 'Smith' }, balance: 100 });

user.profile.firstName = 'Bob';
user.balance = 200;
user.$__delta();

assert.strictEqual(user.$__.version, VERSION_ALL);
});

it('include: including parent also includes child paths', async function() {
const profileSchema = new Schema({ firstName: String, lastName: String }, { _id: false });
const schema = new Schema({
profile: profileSchema,
balance: Number
}, { optimisticConcurrency: ['profile'] });

const User = db.model('TestOCNested3', schema);
const user = await User.create({ profile: { firstName: 'Alice', lastName: 'Smith' }, balance: 100 });

user.profile.firstName = 'Bob';
user.$__delta();

assert.strictEqual(user.$__.version, VERSION_ALL);
});

it('include: modifying parent triggers version when child is included', async function() {
const addressSchema = new Schema({ street: String, country: String }, { _id: false });
const profileSchema = new Schema({ firstName: String, address: addressSchema }, { _id: false });
const schema = new Schema({
profile: profileSchema,
balance: Number
}, { optimisticConcurrency: ['profile.address.country'] });

const User = db.model('TestOCNested4', schema);
const user = await User.create({
profile: { firstName: 'Alice', address: { street: '123 Main', country: 'US' } },
balance: 100
});

user.set('profile.address', { street: '456 Oak', country: 'UK' });
user.$__delta();

assert.strictEqual(user.$__.version, VERSION_ALL);
});

it('include: modifying non-included nested path does not trigger version', async function() {
const profileSchema = new Schema({ firstName: String, lastName: String }, { _id: false });
const schema = new Schema({
profile: profileSchema,
balance: Number
}, { optimisticConcurrency: ['balance'] });

const User = db.model('TestOCNested5', schema);
const user = await User.create({ profile: { firstName: 'Alice', lastName: 'Smith' }, balance: 100 });

user.profile.firstName = 'Bob';
user.$__delta();

assert.strictEqual(user.$__.version, undefined);
});
});
});
});
Loading