Merge pull request #15691 from Automattic/vkarpov15/gh-15672-3

perf(setDefaultsOnInsert): avoid computing all modified paths when running setDefaultsOnInsert and update validators, only calculate if there are defaults to set

Author: vkarpov15

lib/helpers/setDefaultsOnInsert.js

@@ -1,5 +1,4 @@
 'use strict';
-const modifiedPaths = require('./common').modifiedPaths;
 const get = require('./get');
 
 /**
@@ -29,21 +28,16 @@ module.exports = function(filter, schema, castedDoc, options) {
   const updatedKeys = {};
   const updatedValues = {};
   const numKeys = keys.length;
-  const modified = {};
 
   let hasDollarUpdate = false;
 
   for (let i = 0; i < numKeys; ++i) {
-    if (keys[i].startsWith('$')) {
-      modifiedPaths(castedDoc[keys[i]], '', modified);
+    if (keys[i].charAt(0) === '$') {
       hasDollarUpdate = true;
+      break;
     }
   }
 
-  if (!hasDollarUpdate) {
-    modifiedPaths(castedDoc, '', modified);
-  }
-
   const paths = Object.keys(filter);
   const numPaths = paths.length;
   for (let i = 0; i < numPaths; ++i) {
@@ -54,7 +48,7 @@ module.exports = function(filter, schema, castedDoc, options) {
       const numConditionKeys = conditionKeys.length;
       let hasDollarKey = false;
       for (let j = 0; j < numConditionKeys; ++j) {
-        if (conditionKeys[j].startsWith('$')) {
+        if (conditionKeys[j].charAt(0) === '$') {
           hasDollarKey = true;
           break;
         }
@@ -64,7 +58,6 @@ module.exports = function(filter, schema, castedDoc, options) {
       }
     }
     updatedKeys[path] = true;
-    modified[path] = true;
   }
 
   if (options && options.overwrite && !hasDollarUpdate) {
@@ -79,16 +72,17 @@ module.exports = function(filter, schema, castedDoc, options) {
       return;
     }
     const def = schemaType.getDefault(null, true);
-    if (isModified(modified, path)) {
-      return;
-    }
     if (typeof def === 'undefined') {
       return;
     }
-    if (schemaType.splitPath().includes('$*')) {
+    const pathPieces = schemaType.splitPath();
+    if (pathPieces.includes('$*')) {
       // Skip defaults underneath maps. We should never do `$setOnInsert` on a path with `$*`
       return;
     }
+    if (isModified(castedDoc, updatedKeys, path, pathPieces, hasDollarUpdate)) {
+      return;
+    }
 
     castedDoc = castedDoc || {};
     castedDoc.$setOnInsert = castedDoc.$setOnInsert || {};
@@ -101,31 +95,66 @@ module.exports = function(filter, schema, castedDoc, options) {
   return castedDoc;
 };
 
-function isModified(modified, path) {
-  if (modified[path]) {
+function isModified(castedDoc, updatedKeys, path, pathPieces, hasDollarUpdate) {
+  // Check if path is in filter (updatedKeys)
+  if (updatedKeys[path]) {
     return true;
   }
 
-  // Is any parent path of `path` modified?
-  const sp = path.split('.');
-  let cur = sp[0];
-  for (let i = 1; i < sp.length; ++i) {
-    if (modified[cur]) {
+  // Check if any parent path is in filter
+  let cur = pathPieces[0];
+  for (let i = 1; i < pathPieces.length; ++i) {
+    if (updatedKeys[cur]) {
       return true;
     }
-    cur += '.' + sp[i];
+    cur += '.' + pathPieces[i];
   }
 
-  // Is any child of `path` modified?
-  const modifiedKeys = Object.keys(modified);
-  if (modifiedKeys.length) {
-    const parentPath = path + '.';
-
-    for (const modifiedPath of modifiedKeys) {
-      if (modifiedPath.slice(0, path.length + 1) === parentPath) {
-        return true;
+  // Check if path is modified in the update
+  if (hasDollarUpdate) {
+    // Check each update operator
+    for (const key in castedDoc) {
+      if (key.charAt(0) === '$') {
+        if (pathExistsInUpdate(castedDoc[key], path, pathPieces)) {
+          return true;
+        }
       }
     }
+  } else {
+    // No dollar operators, check the castedDoc directly
+    if (pathExistsInUpdate(castedDoc, path, pathPieces)) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
+function pathExistsInUpdate(update, targetPath, pathPieces) {
+  if (update == null || typeof update !== 'object') {
+    return false;
+  }
+
+  // Check exact match
+  if (update.hasOwnProperty(targetPath)) {
+    return true;
+  }
+
+  // Check if any parent path exists
+  let cur = pathPieces[0];
+  for (let i = 1; i < pathPieces.length; ++i) {
+    if (update.hasOwnProperty(cur)) {
+      return true;
+    }
+    cur += '.' + pathPieces[i];
+  }
+
+  // Check if any child path exists (e.g., path is "a" and update has "a.b")
+  const prefix = targetPath + '.';
+  for (const key in update) {
+    if (key.startsWith(prefix)) {
+      return true;
+    }
   }
 
   return false;

lib/helpers/updateValidators.js

@@ -7,7 +7,6 @@
 const ValidationError = require('../error/validation');
 const cleanPositionalOperators = require('./schema/cleanPositionalOperators');
 const flatten = require('./common').flatten;
-const modifiedPaths = require('./common').modifiedPaths;
 
 /**
  * Applies validators and defaults to update and findOneAndUpdate operations,
@@ -29,7 +28,6 @@ module.exports = function(query, schema, castedDoc, options, callback) {
   const arrayAtomicUpdates = {};
   const numKeys = keys.length;
   let hasDollarUpdate = false;
-  const modified = {};
   let currentUpdate;
   let key;
   let i;
@@ -51,7 +49,6 @@ module.exports = function(query, schema, castedDoc, options, callback) {
         }
         continue;
       }
-      modifiedPaths(castedDoc[keys[i]], '', modified);
       const flat = flatten(castedDoc[keys[i]], null, null, schema);
       const paths = Object.keys(flat);
       const numPaths = paths.length;
@@ -76,7 +73,6 @@ module.exports = function(query, schema, castedDoc, options, callback) {
   }
 
   if (!hasDollarUpdate) {
-    modifiedPaths(castedDoc, '', modified);
     updatedValues = flatten(castedDoc, null, null, schema);
     updatedKeys = Object.keys(updatedValues);
   }

lib/schemaType.js

@@ -1174,6 +1174,9 @@ SchemaType.prototype.ref = function(ref) {
 
 SchemaType.prototype.getDefault = function(scope, init, options) {
   let ret;
+  if (this.defaultValue == null) {
+    return this.defaultValue;
+  }
   if (typeof this.defaultValue === 'function') {
     if (
       this.defaultValue === Date.now ||