diff --git a/lib/dao.js b/lib/dao.js index 5d7757ab..26bec26c 100644 --- a/lib/dao.js +++ b/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]) { - var err = new Error('id property value cannot be updated: ' + idName); + if (data[idName] !== undefined && !idEquals(data[idName], inst[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); diff --git a/test/manipulation.test.js b/test/manipulation.test.js index 713d8588..b0c9165e 100644 --- a/test/manipulation.test.js +++ b/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();