Merge pull request #68 from strongloop/feature/fix-lb-162

Make sure own properties are copied by toObject for non-strict mode

lib/dao.js

@@ -223,7 +223,7 @@ DataAccessObject.upsert = DataAccessObject.updateOrCreate = function upsert(data
     this.getDataSource().connector.updateOrCreate(Model.modelName, inst.toObject(true), function (err, data) {
       var obj;
       if (data) {
-        inst._initProperties(data, false);
+        inst._initProperties(data);
         obj = inst;
       } else {
         obj = null;
@@ -320,7 +320,7 @@ DataAccessObject.findById = function find(id, cb) {
         setIdValue(this, data, id);
       }
       obj = new this();
-      obj._initProperties(data, false);
+      obj._initProperties(data);
     }
     cb(err, obj);
   }.bind(this));
@@ -542,7 +542,7 @@ DataAccessObject.find = function find(params, cb) {
       data.forEach(function (d, i) {
         var obj = new constr();
 
-        obj._initProperties(d, false, params.fields);
+        obj._initProperties(d, {fields: params.fields});
 
         if (params && params.include) {
           if (params.collect) {
@@ -764,7 +764,7 @@ DataAccessObject.prototype.save = function (options, callback) {
           if (err) {
             return callback(err, inst);
           }
-          inst._initProperties(data, false);
+          inst._initProperties(data);
           updateDone.call(inst, function () {
             saveDone.call(inst, function () {
               callback(err, inst);

lib/datasource.js

@@ -431,11 +431,12 @@ DataSource.prototype.setupDataAccess = function (modelClass, settings) {
   if (this.connector) {
     // Check if the id property should be generated
     var idName = modelClass.definition.idName();
-    var idProp = modelClass.definition.properties[idName];
+    var idProp = modelClass.definition.rawProperties[idName];
     if(idProp && idProp.generated && this.connector.getDefaultIdType) {
       // Set the default id type from connector's ability
       var idType = this.connector.getDefaultIdType() || String;
       idProp.type = idType;
+      modelClass.definition.properties[idName].type = idType;
     }
     if (this.connector.define) {
       // pass control to connector

lib/model-builder.js

@@ -156,17 +156,14 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett
   // TODO: [rfeng] We need to decide what names to use for built-in models such as User.
   if (!ModelClass || !ModelClass.settings.unresolved) {
     // every class can receive hash of data as optional param
-    ModelClass = function ModelConstructor(data, dataSource) {
+    ModelClass = function ModelConstructor(data, options) {
       if (!(this instanceof ModelConstructor)) {
-        return new ModelConstructor(data, dataSource);
+        return new ModelConstructor(data, options);
       }
       if (ModelClass.settings.unresolved) {
         throw new Error('Model ' + ModelClass.modelName + ' is not defined.');
       }
       ModelBaseClass.apply(this, arguments);
-      if (dataSource) {
-        hiddenProperty(this, '__dataSource', dataSource);
-      }
     };
     // mix in EventEmitter (don't inherit from)
     var events = new EventEmitter();
@@ -343,16 +340,6 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett
     if (!DataType) {
       throw new Error('Invalid type for property ' + propertyName);
     }
-    if (Array.isArray(DataType) || DataType === Array) {
-      DataType = List;
-    } else if (DataType.name === 'Date') {
-      var OrigDate = Date;
-      DataType = function Date(arg) {
-        return new OrigDate(arg);
-      };
-    } else if (typeof DataType === 'string') {
-      DataType = modelBuilder.resolveType(DataType);
-    }
 
     if (prop.required) {
       var requiredOptions = typeof prop.required === 'object' ? prop.required : undefined;
@@ -368,6 +355,17 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett
         }
       },
       set: function (value) {
+        var DataType = ModelClass.definition.properties[propertyName].type;
+        if (Array.isArray(DataType) || DataType === Array) {
+          DataType = List;
+        } else if (DataType.name === 'Date') {
+          var OrigDate = Date;
+          DataType = function Date(arg) {
+            return new OrigDate(arg);
+          };
+        } else if (typeof DataType === 'string') {
+          DataType = modelBuilder.resolveType(DataType);
+        }
         if (ModelClass.setter[propertyName]) {
           ModelClass.setter[propertyName].call(this, value); // Try setter first
         } else {

lib/model.js

@@ -27,8 +27,13 @@ var BASE_TYPES = ['String', 'Boolean', 'Number', 'Date', 'Text'];
  * @constructor
  * @param {Object} data - initial object data
  */
-function ModelBaseClass(data) {
+function ModelBaseClass(data, options) {
-  this._initProperties(data, true);
+  options = options || {};
+  if(!('applySetters' in options)) {
+    // Default to true
+    options.applySetters = true;
+  }
+  this._initProperties(data, options);
 }
 
 // FIXME: [rfeng] We need to make sure the input data should not be mutated. Disabled cloning for now to get tests passing
@@ -42,19 +47,29 @@ function clone(data) {
    */
   return data;
 }
+
 /**
- * Initialize properties
+ * Initialize the model instance with a list of properties
- * @param data
+ * @param {Object} data The data object
- * @param applySetters
+ * @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
  * @private
  */
-ModelBaseClass.prototype._initProperties = function (data, applySetters) {
+ModelBaseClass.prototype._initProperties = function (data, options) {
   var self = this;
   var ctor = this.constructor;
 
   var properties = ctor.definition.build();
   data = data || {};
 
+  options = options || {};
+  var applySetters = options.applySetters;
+  var strict = options.strict;
+
+  if(strict === undefined) {
+    strict = ctor.definition.settings.strict;
+  }
   Object.defineProperty(this, '__cachedRelations', {
     writable: true,
     enumerable: false,
@@ -76,15 +91,32 @@ ModelBaseClass.prototype._initProperties = function (data, applySetters) {
     value: {}
   });
 
+  /**
+   * Instance level data source
+   */
+  Object.defineProperty(this, '__dataSource', {
+    writable: true,
+    enumerable: false,
+    configurable: true,
+    value: options.dataSource
+  });
+
+  /**
+   * Instance level strict mode
+   */
+  Object.defineProperty(this, '__strict', {
+    writable: true,
+    enumerable: false,
+    configurable: true,
+    value: strict
+  });
+
   if (data.__cachedRelations) {
     this.__cachedRelations = data.__cachedRelations;
   }
 
-  // Check if the strict option is set to false for the model
-  var strict = ctor.definition.settings.strict;
-
   for (var i in data) {
-    if (i in properties) {
+    if (i in properties && typeof data[i] !== 'function') {
       this.__data[i] = this.__dataWas[i] = clone(data[i]);
     } else if (i in ctor.relations) {
       this.__data[ctor.relations[i].keyFrom] = this.__dataWas[i] = data[i][ctor.relations[i].keyTo];
@@ -100,7 +132,7 @@ ModelBaseClass.prototype._initProperties = function (data, applySetters) {
 
   if (applySetters === true) {
     for (var propertyName in data) {
-      if ((propertyName in properties) || (propertyName in ctor.relations)) {
+      if (typeof data[propertyName] !== 'function' && ((propertyName in properties) || (propertyName in ctor.relations))) {
         self[propertyName] = self.__data[propertyName] || data[propertyName];
       }
     }
@@ -109,7 +141,7 @@ ModelBaseClass.prototype._initProperties = function (data, applySetters) {
   // Set the unknown properties as properties to the object
   if (strict === false) {
     for (var propertyName in data) {
-      if (!(propertyName in properties)) {
+      if (typeof data[propertyName] !== 'function' && !(propertyName in properties)) {
         self[propertyName] = self.__data[propertyName] || data[propertyName];
       }
     }
@@ -117,7 +149,7 @@ ModelBaseClass.prototype._initProperties = function (data, applySetters) {
 
   ctor.forEachProperty(function (propertyName) {
 
-    if ('undefined' === typeof self.__data[propertyName]) {
+    if (undefined === self.__data[propertyName]) {
       self.__data[propertyName] = self.__dataWas[propertyName] = getDefault(propertyName);
     } else {
       self.__dataWas[propertyName] = self.__data[propertyName];
@@ -160,7 +192,7 @@ ModelBaseClass.prototype._initProperties = function (data, applySetters) {
   }
 
   this.trigger('initialize');
-}
+};
 
 /**
  * @param {String} prop - property name
@@ -197,18 +229,23 @@ ModelBaseClass.toString = function () {
 };
 
 /**
- * Convert instance to Object
+ * Convert model instance to a plain JSON object
  *
- * @param {Boolean} onlySchema - restrict properties to dataSource only, default false
+ * @param {Boolean} onlySchema - restrict properties to dataSource only,
- * when onlySchema == true, only properties defined in dataSource returned,
+ * default to false. When onlySchema is true, only properties defined in
- * otherwise all enumerable properties returned
+ * the schema are returned, otherwise all enumerable properties returned
  * @returns {Object} - canonical object representation (no getters and setters)
  */
 ModelBaseClass.prototype.toObject = function (onlySchema) {
+  if(onlySchema === undefined) {
+    onlySchema = true;
+  }
   var data = {};
   var self = this;
 
-  var schemaLess = this.constructor.definition.settings.strict === false || !onlySchema;
+  var strict = this.__strict;
+  var schemaLess = (strict === false) || !onlySchema;
+
   this.constructor.forEachProperty(function (propertyName) {
     if (self[propertyName] instanceof List) {
       data[propertyName] = self[propertyName].toObject(!schemaLess);
@@ -223,10 +260,25 @@ ModelBaseClass.prototype.toObject = function (onlySchema) {
     }
   });
 
+  var val = null;
   if (schemaLess) {
-    for (var propertyName in self.__data) {
+    // Find its own properties which can be set via myModel.myProperty = 'myValue'.
+    // If the property is not declared in the model definition, no setter will be
+    // triggered to add it to __data
+    for (var propertyName in self) {
+      if(self.hasOwnProperty(propertyName) && (!data.hasOwnProperty(propertyName))) {
+        val = self[propertyName];
+        if (val !== undefined && val !== null && val.toObject) {
+          data[propertyName] = val.toObject(!schemaLess);
+        } else {
+          data[propertyName] = val;
+        }
+      }
+    }
+    // Now continue to check __data
+    for (propertyName in self.__data) {
       if (!data.hasOwnProperty(propertyName)) {
-        var val = self.hasOwnProperty(propertyName) ? self[propertyName] : self.__data[propertyName];
+        val = self.hasOwnProperty(propertyName) ? self[propertyName] : self.__data[propertyName];
         if (val !== undefined && val !== null && val.toObject) {
           data[propertyName] = val.toObject(!schemaLess);
         } else {
@@ -238,13 +290,8 @@ ModelBaseClass.prototype.toObject = function (onlySchema) {
   return data;
 };
 
-// ModelBaseClass.prototype.hasOwnProperty = function (prop) {
-//     return this.__data && this.__data.hasOwnProperty(prop) ||
-//         Object.getOwnPropertyNames(this).indexOf(prop) !== -1;
-// };
-
 ModelBaseClass.prototype.toJSON = function () {
-  return this.toObject();
+  return this.toObject(false);
 };
 
 ModelBaseClass.prototype.fromObject = function (obj) {
@@ -291,10 +338,15 @@ ModelBaseClass.mixin = function (anotherClass, options) {
 
 ModelBaseClass.prototype.getDataSource = function () {
   return this.__dataSource || this.constructor.dataSource;
-}
+};
+
 ModelBaseClass.getDataSource = function () {
   return this.dataSource;
-}
+};
+
+ModelBaseClass.prototype.setStrict = function (strict) {
+  this.__strict = strict;
+};
 
 jutil.mixin(ModelBaseClass, Hookable);
 jutil.mixin(ModelBaseClass, validations.Validatable);

test/loopback-dl.test.js

@@ -46,11 +46,35 @@ describe('ModelBuilder define model', function () {
 
     User.modelName.should.equal('User');
     user.should.be.a('object');
-    assert(user.name === 'Joe');
+    user.should.have.property('name', 'Joe');
-    assert(user.age === undefined);
+    user.should.not.have.property('age');
-    assert(user.toObject().age === undefined);
+    user.toObject().should.not.have.property('age');
-    assert(user.toObject(true).age === undefined);
+    user.toObject(true).should.not.have.property('age');
-    assert(user.bio === undefined);
+    user.should.not.have.property('bio');
+    done(null, User);
+  });
+
+  it('should ignore non-predefined properties in strict mode', function (done) {
+    var modelBuilder = new ModelBuilder();
+
+    var User = modelBuilder.define('User', {name: String, bio: String}, {strict: true});
+
+    var user = new User({name: 'Joe'});
+    user.age = 10;
+    user.bio = 'me';
+
+    user.should.have.property('name', 'Joe');
+    user.should.have.property('bio', 'me');
+
+    // Non predefined property age should be ignored in strict mode if schemaOnly parameter is not false
+    user.toObject().should.not.have.property('age');
+    user.toObject(true).should.not.have.property('age');
+    user.toObject(false).should.have.property('age', 10);
+
+    // Predefined property bio should be kept in strict mode
+    user.toObject().should.have.property('bio', 'me');
+    user.toObject(true).should.have.property('bio', 'me');
+    user.toObject(false).should.have.property('bio', 'me');
     done(null, User);
   });
 
@@ -83,6 +107,31 @@ describe('ModelBuilder define model', function () {
     done(null, User);
   });
 
+  it('should take non-predefined properties in non-strict mode', function (done) {
+    var modelBuilder = new ModelBuilder();
+
+    var User = modelBuilder.define('User', {name: String, bio: String}, {strict: false});
+
+    var user = new User({name: 'Joe'});
+    user.age = 10;
+    user.bio = 'me';
+
+    user.should.have.property('name', 'Joe');
+    user.should.have.property('bio', 'me');
+
+    // Non predefined property age should be kept in non-strict mode
+    user.toObject().should.have.property('age', 10);
+    user.toObject(true).should.have.property('age', 10);
+    user.toObject(false).should.have.property('age', 10);
+
+    // Predefined property bio should be kept
+    user.toObject().should.have.property('bio', 'me');
+    user.toObject(true).should.have.property('bio', 'me');
+    user.toObject(false).should.have.property('bio', 'me');
+
+    done(null, User);
+  });
+
   it('should use false as the default value for strict', function (done) {
     var modelBuilder = new ModelBuilder();
 
@@ -416,6 +465,27 @@ describe('DataSource define model', function () {
     });
   });
 
+  it('supports instance level strict mode', function () {
+    var ds = new DataSource('memory');
+
+    var User = ds.define('User', {name: String, bio: String}, {strict: true});
+
+    var user = new User({name: 'Joe', age: 20}, {strict: false});
+
+    user.should.have.property('__strict', false);
+    user.should.be.a('object');
+    user.should.have.property('name', 'Joe');
+    user.should.have.property('age', 20);
+    user.toObject().should.have.property('age', 20);
+    user.toObject(true).should.have.property('age', 20);
+
+    user.setStrict(true);
+    user.toObject().should.not.have.property('age');
+    user.toObject(true).should.not.have.property('age');
+    user.toObject(false).should.have.property('age', 20);
+
+  });
+
   it('injects id by default', function (done) {
     var ds = new ModelBuilder();