From bdf703fb45b3bf181f51e30c548c718763305373 Mon Sep 17 00:00:00 2001 From: Laurent Villeneuve Date: Tue, 18 Aug 2015 10:22:36 -0400 Subject: [PATCH] Use idEquals when comparing ids in relation definitions --- lib/relation-definition.js | 32 +++++++++++--------------------- lib/utils.js | 15 +++++++++++++++ 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index edb408ad..c08f12f9 100644 --- a/lib/relation-definition.js +++ b/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 @@ -2270,7 +2271,7 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) 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 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; }; @@ -2869,9 +2870,9 @@ RelationDefinition.referencesMany = function referencesMany(modelFrom, modelTo, }); modelFrom.validate(relationName, function(err) { - var ids = (this[fk] || []).map(function (id) {return id && id.toString(); }); // mongodb + 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) { diff --git a/lib/utils.js b/lib/utils.js index 2a3521b4..f6be3e98 100644 --- a/lib/utils.js +++ b/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; +}