Merge pull request #290 from fabien/feature/enforce-id

Enforce id (prevent user-set value), fix isNewRecord

lib/dao.js

@@ -137,14 +137,16 @@ DataAccessObject.create = function (data, callback) {
   }
 
   var obj;
+  var idValue = getIdValue(this, data);
+  
   // if we come from save
-  if (data instanceof Model && !getIdValue(this, data)) {
+  if (data instanceof Model && !idValue) {
     obj = data;
   } else {
     obj = new Model(data);
   }
   data = obj.toObject(true);
-
+  
   // validation required
   obj.isValid(function (valid) {
     if (valid) {
@@ -170,6 +172,7 @@ DataAccessObject.create = function (data, callback) {
           if (err) {
             return callback(err, obj);
           }
+          obj.__persisted = true;
           saveDone.call(obj, function () {
             createDone.call(obj, function () {
               callback(err, obj);
@@ -315,7 +318,7 @@ DataAccessObject.findById = function find(id, cb) {
       if (!getIdValue(this, data)) {
         setIdValue(this, data, id);
       }
-      obj = new this(data, {applySetters: false});
+      obj = new this(data, {applySetters: false, persisted: true});
     }
     cb(err, obj);
   }.bind(this));
@@ -732,7 +735,7 @@ DataAccessObject.find = function find(query, cb) {
   this.getDataSource().connector.all(this.modelName, query, function (err, data) {
     if (data && data.forEach) {
       data.forEach(function (d, i) {
-        var obj = new self(d, {fields: query.fields, applySetters: false});
+        var obj = new self(d, {fields: query.fields, applySetters: false, persisted: true});
 
         if (query && query.include) {
           if (query.collect) {
@@ -928,7 +931,7 @@ DataAccessObject.prototype.save = function (options, callback) {
   var data = inst.toObject(true);
   var modelName = Model.modelName;
 
-  if (!getIdValue(Model, this)) {
+  if (this.isNewRecord()) {
     return Model.create(this, callback);
   }
 
@@ -959,7 +962,7 @@ DataAccessObject.prototype.save = function (options, callback) {
           if (err) {
             return callback(err, inst);
           }
-          inst._initProperties(data);
+          inst._initProperties(data, { persisted: true });
           updateDone.call(inst, function () {
             saveDone.call(inst, function () {
               callback(err, inst);
@@ -1027,7 +1030,7 @@ DataAccessObject.updateAll = function (where, data, cb) {
 };
 
 DataAccessObject.prototype.isNewRecord = function () {
-  return !getIdValue(this.constructor, this);
+  return !this.__persisted;
 };
 
 /**
@@ -1168,6 +1171,7 @@ DataAccessObject.prototype.updateAttributes = function updateAttributes(data, cb
 
           inst._adapter().updateAttributes(model, getIdValue(inst.constructor, inst),
             inst.constructor._forDB(typedData), function (err) {
+            if (!err) inst.__persisted = true;
             done.call(inst, function () {
               saveDone.call(inst, function () {
                 if(cb) cb(err, inst);

lib/datasource.js

@@ -489,6 +489,9 @@ DataSource.prototype.setupDataAccess = function (modelClass, settings) {
       var idType = this.connector.getDefaultIdType() || String;
       idProp.type = idType;
       modelClass.definition.properties[idName].type = idType;
+      if (settings.forceId) {
+        modelClass.validatesAbsenceOf(idName, { if: 'isNewRecord' });
+      }
     }
     if (this.connector.define) {
       // pass control to connector

lib/model.js

@@ -47,6 +47,7 @@ function ModelBaseClass(data, options) {
  * @param {Object} options An object to control the instantiation
  * @property {Boolean} applySetters Controls if the setters will be applied
  * @property {Boolean} strict Set the instance level strict mode
+ * @property {Boolean} persisted Whether the instance has been persisted
  * @private
  */
 ModelBaseClass.prototype._initProperties = function (data, options) {
@@ -67,7 +68,11 @@ ModelBaseClass.prototype._initProperties = function (data, options) {
   if(strict === undefined) {
     strict = ctor.definition.settings.strict;
   }
-
+  
+  if (options.persisted !== undefined) {
+    this.__persisted = options.persisted === true;
+  }
+  
   if (ctor.hideInternalProperties) {
     // Object.defineProperty() is expensive. We only try to make the internal
     // properties hidden (non-enumerable) if the model class has the

lib/relation-definition.js

@@ -1752,6 +1752,8 @@ EmbedsOne.prototype.related = function (refresh, params) {
   }
   
   var embeddedInstance = modelInstance[propertyName];
+  embeddedInstance.__persisted = true;
+  
   if (typeof params === 'function') { // acts as async getter
     var cb = params;
     process.nextTick(function() { 
@@ -1977,6 +1979,7 @@ EmbedsMany.prototype.prepareEmbeddedInstance = function(inst) {
     var self = this;
     var propertyName = this.definition.keyFrom;
     var modelInstance = this.modelInstance;
+    inst.__persisted = true;
     inst.triggerParent = function(actionName, callback) {
       if (actionName === 'save' || actionName === 'destroy') {
         var embeddedList = self.embeddedList();

test/manipulation.test.js

@@ -16,7 +16,7 @@ describe('manipulation', function () {
       age: {type: Number, index: true},
       dob: Date,
       createdAt: {type: Number, default: Date.now}
-    });
+    }, { forceId: true });
 
     db.automigrate(done);
 
@@ -40,6 +40,18 @@ describe('manipulation', function () {
         });
       });
     });
+    
+    it('should instantiate an object', function (done) {
+      var p = new Person({name: 'Anatoliy'});
+      p.name.should.equal('Anatoliy');
+      p.isNewRecord().should.be.true;
+      p.save(function(err, inst) {
+        should.not.exist(err);
+        inst.isNewRecord().should.be.false;
+        inst.should.equal(p);
+        done();
+      });
+    });
 
     it('should return instance of object', function (done) {
       var person = Person.create(function (err, p) {
@@ -50,6 +62,31 @@ describe('manipulation', function () {
       person.should.be.an.instanceOf(Person);
       should.not.exist(person.id);
     });
+    
+    it('should not allow user-defined value for the id of object - create', function (done) {
+      Person.create({id: 123456}, function (err, p) {
+        err.should.be.instanceof(ValidationError);
+        err.message.should.equal('The `Person` instance is not valid. Details: `id` can\'t be set.');
+        err.statusCode.should.equal(422);
+        p.should.be.instanceof(Person);
+        p.id.should.equal(123456);
+        p.isNewRecord().should.be.true;
+        done();
+      });
+    });
+    
+    it('should not allow user-defined value for the id of object - save', function (done) {
+      var p = new Person({id: 123456});
+      p.isNewRecord().should.be.true;
+      p.save(function(err, inst) {
+        err.should.be.instanceof(ValidationError);
+        err.message.should.equal('The `Person` instance is not valid. Details: `id` can\'t be set.');
+        err.statusCode.should.equal(422);
+        inst.id.should.equal(123456);
+        inst.isNewRecord().should.be.true;
+        done();
+      });
+    });
 
     it('should work when called without callback', function (done) {
       Person.afterCreate = function (next) {