Merge pull request #522 from strongloop/feature/enhance-id-comparison-for-updateAttributes

Enhance id comparision for updateAttributes

lib/dao.js

@@ -1886,6 +1886,27 @@ DataAccessObject.prototype.unsetAttribute = function unsetAttribute(name, nullif
   }
 };
 
+// Compare two id values to decide if updateAttributes is trying to change
+// the id value for a given instance
+function idEquals(id1, id2) {
+  if (id1 === id2) {
+    return true;
+  }
+  // Allows number/string conversions
+  if ((typeof id1 === 'number' && typeof id2 === 'string') ||
+    (typeof id1 === 'string' && typeof id2 === 'number')) {
+    return id1 == id2;
+  }
+  // For complex id types such as MongoDB ObjectID
+  id1 = JSON.stringify(id1);
+  id2 = JSON.stringify(id2);
+  if (id1 === id2) {
+    return true;
+  }
+
+  return false;
+}
+
 /**
  * Update set of attributes.
  * Performs validation before updating.
@@ -1938,8 +1959,9 @@ DataAccessObject.prototype.updateAttributes = function updateAttributes(data, op
   var idNames = Model.definition.idNames();
   for (var i = 0, n = idNames.length; i < n; i++) {
     var idName = idNames[i];
-    if (data[idName] !== undefined && data[idName] !== inst[idName]) {
+    if (data[idName] !== undefined && !idEquals(data[idName], inst[idName])) {
-      var err = new Error('id property value cannot be updated: ' + idName);
+      var err = new Error('id property (' + idName + ') ' +
+        'cannot be updated from ' + inst[idName] + ' to ' + data[idName]);
       err.statusCode = 400;
       return process.nextTick(function() {
         cb(err);

test/manipulation.test.js

@@ -446,7 +446,7 @@ describe('manipulation', function () {
     before(function (done) {
       Person.destroyAll(function () {
         Person.create({name: 'Mary', age: 15}, function(err, p) {
-          should.not.exist(err);
+          if (err) return done(err);
           person = p;
           done();
         });
@@ -455,9 +455,9 @@ describe('manipulation', function () {
 
     it('should update one attribute', function (done) {
       person.updateAttribute('name', 'Paul Graham', function (err, p) {
-        should.not.exist(err);
+        if (err) return done(err);
         Person.all(function (e, ps) {
-          should.not.exist(e);
+          if (e) return done(e);
           ps.should.have.lengthOf(1);
           ps.pop().name.should.equal('Paul Graham');
           done();
@@ -480,9 +480,9 @@ describe('manipulation', function () {
     it('should ignore undefined values on updateAttributes', function(done) {
       person.updateAttributes({'name': 'John', age: undefined},
         function(err, p) {
-          should.not.exist(err);
+          if (err) return done(err);
           Person.findById(p.id, function(e, p) {
-            should.not.exist(e);
+            if (e) return done(e);
             p.name.should.equal('John');
             p.age.should.equal(15);
             done();
@@ -493,9 +493,9 @@ describe('manipulation', function () {
     it('should allow same id value on updateAttributes', function(done) {
       person.updateAttributes({id: person.id, name: 'John'},
         function(err, p) {
-          should.not.exist(err);
+          if (err) return done(err);
           Person.findById(p.id, function(e, p) {
-            should.not.exist(e);
+            if (e) return done(e);
             p.name.should.equal('John');
             p.age.should.equal(15);
             done();
@@ -503,6 +503,25 @@ describe('manipulation', function () {
         });
     });
 
+    it('should allow same stringified id value on updateAttributes',
+      function(done) {
+        var pid = person.id;
+        if (typeof person.id === 'object' || typeof person.id === 'number') {
+          // For example MongoDB ObjectId
+          pid = person.id.toString();
+        }
+        person.updateAttributes({id: pid, name: 'John'},
+          function(err, p) {
+            if (err) return done(err);
+            Person.findById(p.id, function(e, p) {
+              if (e) return done(e);
+              p.name.should.equal('John');
+              p.age.should.equal(15);
+              done();
+            });
+          });
+      });
+
     it('should fail if an id value is to be changed on updateAttributes',
       function(done) {
       person.updateAttributes({id: person.id + 1, name: 'John'},
@@ -515,9 +534,9 @@ describe('manipulation', function () {
     it('should allows model instance on updateAttributes', function(done) {
       person.updateAttributes(new Person({'name': 'John', age: undefined}),
         function(err, p) {
-          should.not.exist(err);
+          if (err) return done(err);
           Person.findById(p.id, function(e, p) {
-            should.not.exist(e);
+            if (e) return done(e);
             p.name.should.equal('John');
             p.age.should.equal(15);
             done();