Merge pull request #696 from WyzeLink/fix/embedded-relations-id-comparison

Fix id comparison by using strings

lib/relation-definition.js

@@ -8,6 +8,7 @@ var utils = require('./utils');
 var i8n = require('inflection');
 var defineScope = require('./scope.js').defineScope;
 var mergeQuery = utils.mergeQuery;
+var idEquals = utils.idEquals;
 var ModelBaseClass = require('./model.js');
 var applyFilter = require('./connectors/memory').applyFilter;
 var ValidationError = require('./validations.js').ValidationError;
@@ -332,7 +333,7 @@ HasMany.prototype.removeFromCache = function (id) {
   var idName = this.definition.modelTo.definition.idName();
   if (Array.isArray(cache)) {
     for (var i = 0, n = cache.length; i < n; i++) {
-      if (cache[i][idName] === id) {
+      if (idEquals(cache[i][idName],id)) {
         return cache.splice(i, 1);
       }
     }
@@ -351,7 +352,7 @@ HasMany.prototype.addToCache = function (inst) {
   var idName = this.definition.modelTo.definition.idName();
   if (Array.isArray(cache)) {
     for (var i = 0, n = cache.length; i < n; i++) {
-      if (cache[i][idName] === inst[idName]) {
+      if (idEquals(cache[i][idName],inst[idName])) {
         cache[i] = inst;
         return;
       }
@@ -783,7 +784,7 @@ HasMany.prototype.findById = function (fkId, options, cb) {
       return cb(err);
     }
     // Check if the foreign key matches the primary key
-    if (inst[fk] != null && inst[fk].toString() === modelInstance[pk].toString()) {
+    if (inst[fk] != null && idEquals(inst[fk], modelInstance[pk])) {
       cb(null, inst);
     } else {
       err = new Error('Key mismatch: ' + modelFrom.modelName + '.' + pk
@@ -2268,9 +2269,9 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params)
   if (!params.polymorphic) {
     modelFrom.validate(propertyName, function(err) {
       var embeddedList = this[propertyName] || [];
-      var ids = embeddedList.map(function(m) { return m[idName]; });
+      var ids = embeddedList.map(function(m) { return m[idName] && m[idName].toString(); }); // mongodb
       var uniqueIds = ids.filter(function(id, pos) {
-          return ids.indexOf(id) === pos;
+          return utils.findIndexOf(ids, id, idEquals) === pos;
       });
       if (ids.length !== uniqueIds.length) {
         this.errors.add(propertyName, 'contains duplicate `' + idName + '`', 'uniqueness');
@@ -2475,7 +2476,7 @@ EmbedsMany.prototype.findById = function (fkId, options, cb) {
   var find = function(id) {
     for (var i = 0; i < embeddedList.length; i++) {
       var item = embeddedList[i];
-      if (item[pk].toString() === id) return item;
+      if (idEquals(item[pk],id)) return item;
     }
     return null;
   };
@@ -2871,7 +2872,7 @@ RelationDefinition.referencesMany = function referencesMany(modelFrom, modelTo,
   modelFrom.validate(relationName, function(err) {
     var ids = this[fk] || [];
     var uniqueIds = ids.filter(function(id, pos) {
-        return ids.indexOf(id) === pos;
+        return utils.findIndexOf(ids, id, idEquals) === pos;
     });
     if (ids.length !== uniqueIds.length) {
       var msg = 'contains duplicate `' + modelTo.modelName + '` instance';
@@ -3004,14 +3005,8 @@ ReferencesMany.prototype.findById = function (fkId, options, cb) {
       return cb(err);
     }
 
-    var currentIds = ids.map(function(id) { return id.toString(); });
-    var id = '';
-    if (inst[pk] != null) {
-      id = inst[pk].toString();
-    }
-
     // Check if the foreign key is amongst the ids
-    if (currentIds.indexOf(id) > -1) {
+    if (utils.findIndexOf(ids, inst[pk], idEquals) > -1) {
       cb(null, inst);
     } else {
       err = new Error('Key mismatch: ' + modelFrom.modelName + '.' + fk
@@ -3032,11 +3027,9 @@ ReferencesMany.prototype.exists = function (fkId, options, cb) {
   }
   var fk = this.definition.keyFrom;
   var ids = this.modelInstance[fk] || [];
-  var currentIds = ids.map(function(id) { return id.toString(); });
-  var fkId = (fkId || '').toString(); // mongodb
 
   cb = cb || utils.createPromiseCallback();
-  process.nextTick(function() { cb(null, currentIds.indexOf(fkId) > -1) });
+  process.nextTick(function() { cb(null, utils.findIndexOf(ids, fkId, idEquals) > -1) });
   return cb.promise;
 };
 
@@ -3221,14 +3214,11 @@ ReferencesMany.prototype.remove = function (acInst, options, cb) {
 
   var ids = modelInstance[fk] || [];
 
-  var currentIds = ids.map(function(id) { return id.toString(); });
-
   var id = (acInst instanceof definition.modelTo) ? acInst[pk] : acInst;
-  id = id.toString();
 
   cb = cb || utils.createPromiseCallback();
 
-  var index = currentIds.indexOf(id);
+  var index = utils.findIndexOf(ids, id, idEquals);
   if (index > -1) {
     ids.splice(index, 1);
     modelInstance.updateAttribute(fk, ids, options, function(err, inst) {

lib/utils.js

@@ -15,6 +15,7 @@ exports.uniq = uniq;
 exports.toRegExp = toRegExp;
 exports.hasRegExpFlags = hasRegExpFlags;
 exports.idEquals = idEquals;
+exports.findIndexOf = findIndexOf;
 
 var traverse = require('traverse');
 var assert = require('assert');
@@ -573,3 +574,17 @@ function idEquals(id1, id2) {
 
   return false;
 }
+
+// Defaults to native Array.prototype.indexOf when no idEqual is present
+// Otherwise, returns the lowest index for which isEqual(arr[]index, target) is true
+function findIndexOf(arr, target, isEqual) {
+  if (!isEqual) {
+    return arr.indexOf(target);
+  }
+
+  for (var i = 0; i < arr.length; i++) {
+    if (isEqual(arr[i], target)) { return i; }
+  };
+
+  return -1;
+}