fix(document): handle including and excluding nested paths with optimistic concurrency

[vkarpov15]

Fix #16054

Summary

I know @AbdelrahmanHafez assigned himself but we were getting too much AI-generated PR spam related to that issue so I figured I'd take care of it, along with making it so that we don't run new Set() every time we check optimistic concurrency, because creating a new set() is more expensive than just doing includes() on the array:

Also came up with a faster way to check for subpaths in an array using substring() in _pathOrAncestorInSet() - this approach is slightly faster because it avoids creating a new array.

Examples

[Copilot]

Pull request overview

Fixes optimistic concurrency path matching for nested paths (gh-16054) and reduces per-save overhead by caching optimisticConcurrency include/exclude sets on the schema.

Changes:

• Add regression tests ensuring excluded parent paths also exclude nested subpath modifications.
• Precompute/caches optimisticConcurrency include/exclude sets during schema pre-compilation.
• Update Document#$__delta() to use direct modified paths plus ancestor matching for faster/accurate checks.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
test/model.test.js	Adds regression tests for optimisticConcurrency.exclude with nested paths (including non-strict subdocs).
lib/schema.js	Computes and stores cached include/exclude sets on schema.s during _preCompile().
lib/document.js	Uses cached sets + directModifiedPaths() and new _pathOrAncestorInSet() helper when deciding whether to apply optimistic concurrency versioning.

[hasezoey]

Looks good to me, though the Copilot comment should be investigated, like the misleading function doc.

[AbdelrahmanHafez]

A couple of suggestions, otherwise LGTM

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

[Copilot]

Using _pathOverlapsSet() for the optimisticConcurrency.exclude logic makes exclusion symmetric (ancestors and descendants). That can incorrectly skip version checks when the excluded set contains only a nested path but the document directly modifies its parent path (e.g. schema has profile.firstName + profile.lastName, exclude: ['profile.firstName'], and doc.profile = { lastName: 'changed' } => direct modified path is profile, _pathOverlapsSet() returns true because of the descendant, and versioning is skipped even though a non-excluded subpath changed). For exclude, the check should treat a path as excluded only if the path itself or one of its ancestors is in the exclude set (not if any descendant is). Consider using a separate helper for the exclude case (path-or-ancestor-in-set) and add a regression test for this scenario.

[Copilot]

_pathOverlapsSet() checks descendant paths by iterating for (const p of pathSet), which makes each lookup O(|pathSet|) in the worst case. Since this function is called for each directly modified path during $__delta(), this can become noticeably more expensive than the prior Set.has() check when users configure many optimisticConcurrency paths. If the goal is to speed up optimistic concurrency checks, consider precomputing additional entries (for example, adding all ancestors of each included path to the include set at schema compile time) so the runtime check can avoid scanning the whole set.

Suggested change

	function _pathOverlapsSet(path, pathSet) {
	if (pathSet.has(path)) {
	return true;
	}
	let idx = path.indexOf('.');
	while (idx !== -1) {
	if (pathSet.has(path.substring(0, idx))) {
	return true;
	}
	idx = path.indexOf('.', idx + 1);
	}
	for (const p of pathSet) {
	if (p.length > path.length + 1 && p[path.length] === '.' && p.slice(0, path.length) === path) {
	return true;
	}
	}
	const _augmentedPathSets = new WeakMap();
	function _getAugmentedPathSet(pathSet) {
	// Cache an augmented version of `pathSet` that also includes all ancestor
	// paths of each entry. This lets `_pathOverlapsSet()` rely solely on
	// `Set.has()` checks without scanning the entire set on every call.
	if (_augmentedPathSets.has(pathSet)) {
	return _augmentedPathSets.get(pathSet);
	}
	const augmented = new Set(pathSet);
	for (const p of pathSet) {
	let idx = p.indexOf('.');
	while (idx !== -1) {
	augmented.add(p.substring(0, idx));
	idx = p.indexOf('.', idx + 1);
	}
	}
	_augmentedPathSets.set(pathSet, augmented);
	return augmented;
	}
	function _pathOverlapsSet(path, pathSet) {
	const augmentedPathSet = _getAugmentedPathSet(pathSet);
	if (augmentedPathSet.has(path)) {
	return true;
	}
	let idx = path.indexOf('.');
	while (idx !== -1) {
	if (augmentedPathSet.has(path.substring(0, idx))) {
	return true;
	}
	idx = path.indexOf('.', idx + 1);
	}

[AbdelrahmanHafez]

LGTM 👍

[hasezoey]

LGTM