diff --git a/lib/document.js b/lib/document.js index eda744170c..9e01f3786d 100644 --- a/lib/document.js +++ b/lib/document.js @@ -5055,8 +5055,8 @@ 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; } @@ -5064,8 +5064,8 @@ Document.prototype.$__delta = function $__delta(pathsToSave, pathsToSaveSet) { 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; } @@ -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) { + if (includeSet.has(path)) { + return true; + } + // Check if any parent prefix is included (e.g. 'profile' includes 'profile.name') + const pieces = path.split('.'); + 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; + } + } + 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. */ diff --git a/test/versioning.test.js b/test/versioning.test.js index e885d01c09..e1e1c9f8d0 100644 --- a/test/versioning.test.js +++ b/test/versioning.test.js @@ -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); + }); + }); }); });