Merge pull request #1629 from mgabeler-lee-6rs/fix/fast-id-unique-check

fix: accelerate unique id checking

lib/relation-definition.js

@@ -17,6 +17,7 @@ var defineScope = require('./scope.js').defineScope;
 var g = require('strong-globalize')();
 var mergeQuery = utils.mergeQuery;
 var idEquals = utils.idEquals;
+var idsHaveDuplicates = utils.idsHaveDuplicates;
 var ModelBaseClass = require('./model.js');
 var applyFilter = require('./connectors/memory').applyFilter;
 var ValidationError = require('./validations.js').ValidationError;
@@ -2502,10 +2503,7 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelToRef, param
     modelFrom.validate(propertyName, function(err) {
       var embeddedList = this[propertyName] || [];
       var ids = embeddedList.map(function(m) { return m[idName] && m[idName].toString(); }); // mongodb
-      var uniqueIds = ids.filter(function(id, pos) {
-        return utils.findIndexOf(ids, id, idEquals) === pos;
-      });
-      if (ids.length !== uniqueIds.length) {
+      if (idsHaveDuplicates(ids)) {
         this.errors.add(propertyName, 'contains duplicate `' + idName + '`', 'uniqueness');
         err(false);
       }
@@ -3155,10 +3153,7 @@ RelationDefinition.referencesMany = function referencesMany(modelFrom, modelToRe
 
   modelFrom.validate(relationName, function(err) {
     var ids = this[fk] || [];
-    var uniqueIds = ids.filter(function(id, pos) {
-      return utils.findIndexOf(ids, id, idEquals) === pos;
-    });
-    if (ids.length !== uniqueIds.length) {
+    if (idsHaveDuplicates(ids)) {
       var msg = 'contains duplicate `' + modelTo.modelName + '` instance';
       this.errors.add(relationName, msg, 'uniqueness');
       err(false);

lib/utils.js

@@ -26,6 +26,7 @@ exports.findIndexOf = findIndexOf;
 exports.collectTargetIds = collectTargetIds;
 exports.idName = idName;
 exports.rankArrayElements = rankArrayElements;
+exports.idsHaveDuplicates = idsHaveDuplicates;
 
 var g = require('strong-globalize')();
 var traverse = require('traverse');
@@ -684,3 +685,48 @@ function collectTargetIds(targetData, idPropertyName) {
 function idName(m) {
   return m.definition.idName() || 'id';
 }
+
+/**
+ * Check a list of IDs to see if there are any duplicates.
+ *
+ * @param {Array} The array of IDs to check
+ * @returns {boolean} If any duplicates were found
+ */
+function idsHaveDuplicates(ids) {
+  // use Set if available and all ids are of string or number type
+  var hasDuplicates = undefined;
+  var i, j;
+  if (typeof Set === 'function') {
+    var uniqueIds = new Set();
+    for (i = 0; i < ids.length; ++i) {
+      var idType = typeof ids[i];
+      if (idType === 'string' || idType === 'number') {
+        if (uniqueIds.has(ids[i])) {
+          hasDuplicates = true;
+          break;
+        } else {
+          uniqueIds.add(ids[i]);
+        }
+      } else {
+        // ids are not all string/number that can be checked via Set, stop and do the slow test
+        break;
+      }
+    }
+    if (hasDuplicates === undefined && uniqueIds.size === ids.length) {
+      hasDuplicates = false;
+    }
+  }
+  if (hasDuplicates === undefined) {
+    // fast check was inconclusive or unavailable, do the slow check
+    // can still optimize this by doing 1/2 N^2 instead of the full N^2
+    for (i = 0; i < ids.length && hasDuplicates === undefined; ++i) {
+      for (j = 0; j < i; ++j) {
+        if (idEquals(ids[i], ids[j])) {
+          hasDuplicates = true;
+          break;
+        }
+      }
+    }
+  }
+  return hasDuplicates === true;
+}

test/util.test.js

@@ -559,3 +559,35 @@ describe('util.hasRegExpFlags', function() {
     });
   });
 });
+
+describe('util.idsHaveDuplicates', function() {
+  context('with string IDs', function() {
+    it('should be true with a duplicate present', function() {
+      utils.idsHaveDuplicates(['a', 'b', 'a']).should.be.ok;
+    });
+
+    it('should be false when no duplicates are present', function() {
+      utils.idsHaveDuplicates(['a', 'b', 'c']).should.not.be.ok;
+    });
+  });
+
+  context('with numeric IDs', function() {
+    it('should be true with a duplicate present', function() {
+      utils.idsHaveDuplicates([1, 2, 1]).should.be.ok;
+    });
+
+    it('should be false when no duplicates are present', function() {
+      utils.idsHaveDuplicates([1, 2, 3]).should.not.be.ok;
+    });
+  });
+
+  context('with complex IDs', function() {
+    it('should be true with a duplicate present', function() {
+      utils.idsHaveDuplicates(['a', 'b', 'a'].map(id => ({id}))).should.be.ok;
+    });
+
+    it('should be false when no duplicates are present', function() {
+      utils.idsHaveDuplicates(['a', 'b', 'c'].map(id => ({id}))).should.not.be.ok;
+    });
+  });
+});