Optimize model instantiation and conversion

lib/dao.js

@@ -162,7 +162,6 @@ DataAccessObject.create = function (data, callback) {
         this._adapter().create(modelName, this.constructor._forDB(obj.toObject(true)), function (err, id, rev) {
           if (id) {
             obj.__data[_idName] = id;
-            obj.__dataWas[_idName] = id;
             defineReadonlyProp(obj, _idName, id);
           }
           if (rev) {
@@ -353,8 +352,7 @@ DataAccessObject.findById = function find(id, cb) {
       if (!getIdValue(this, data)) {
         setIdValue(this, data, id);
       }
-      obj = new this();
+      obj = new this(data, {applySetters: false});
-      obj._initProperties(data);
     }
     cb(err, obj);
   }.bind(this));
@@ -689,9 +687,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();
+        var obj = new self(d, {fields: query.fields, applySetters: false});
-
-        obj._initProperties(d, {fields: query.fields});
 
         if (query && query.include) {
           if (query.collect) {
@@ -1059,12 +1055,6 @@ DataAccessObject.prototype.updateAttributes = function updateAttributes(data, cb
           }
 
           inst._adapter().updateAttributes(model, getIdValue(inst.constructor, inst), inst.constructor._forDB(typedData), function (err) {
-            if (!err) {
-              // update $was attrs
-              for (var key in data) {
-                inst.__dataWas[key] = inst.__data[key];
-              }
-            }
             done.call(inst, function () {
               saveDone.call(inst, function () {
                 if(cb) cb(err, inst);

lib/model-builder.js

@@ -264,7 +264,11 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett
 
   // A function to loop through the properties
   ModelClass.forEachProperty = function (cb) {
-    Object.keys(ModelClass.definition.properties).forEach(cb);
+    var props = ModelClass.definition.properties;
+    var keys = Object.keys(props);
+    for (var i = 0, n = keys.length; i < n; i++) {
+      cb(keys[i], props[keys[i]]);
+    }
   };
 
   // A function to attach the model class to a data source
@@ -309,27 +313,22 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett
     }
 
     // Merging the properties
-    Object.keys(properties).forEach(function (key) {
+    var keys = Object.keys(properties);
+    for (var i = 0, n = keys.length; i < n; i++) {
+      var key = keys[i];
+
       if (idFound && properties[key].id) {
         // don't inherit id properties
-        return;
+        continue;
       }
       if (subclassProperties[key] === undefined) {
         subclassProperties[key] = properties[key];
       }
-    });
+    }
 
     // Merge the settings
     subclassSettings = mergeSettings(settings, subclassSettings);
 
-    /*
-     Object.keys(settings).forEach(function (key) {
-     if(subclassSettings[key] === undefined) {
-     subclassSettings[key] = settings[key];
-     }
-     });
-     */
-
     // Define the subclass
     var subClass = modelBuilder.define(className, subclassProperties, subclassSettings, ModelClass);
 
@@ -370,20 +369,15 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett
         var DataType = ModelClass.definition.properties[propertyName].type;
         if (Array.isArray(DataType) || DataType === Array) {
           DataType = List;
-        } else if (DataType.name === 'Date') {
+        } else if (DataType === Date) {
-          var OrigDate = Date;
+          DataType = DateType;
-          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 {
-          if (!this.__data) {
+          this.__data = this.__data || {};
-            this.__data = {};
-          }
           if (value === null || value === undefined) {
             this.__data[propertyName] = value;
           } else {
@@ -401,15 +395,6 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett
       enumerable: true
     });
 
-    // <propertyName>$was --> __dataWas.<propertyName>
-    Object.defineProperty(ModelClass.prototype, propertyName + '$was', {
-      get: function () {
-        return this.__dataWas && this.__dataWas[propertyName];
-      },
-      configurable: true,
-      enumerable: false
-    });
-
     // FIXME: [rfeng] Do we need to keep the raw data?
     // Use $ as the prefix to avoid conflicts with properties such as _id
     Object.defineProperty(ModelClass.prototype, '$' + propertyName, {
@@ -427,7 +412,13 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett
     });
   };
 
-  ModelClass.forEachProperty(ModelClass.registerProperty);
+  var props = ModelClass.definition.properties;
+  var keys = Object.keys(props);
+  var size = keys.length;
+  for (i = 0; i < size; i++) {
+    var propertyName = keys[i];
+    ModelClass.registerProperty(propertyName);
+  }
 
   ModelClass.emit('defined', ModelClass);
 
@@ -435,6 +426,11 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett
 
 };
 
+// DataType for Date
+function DateType(arg) {
+  return new Date(arg);
+}
+
 /**
  * Define single property named `propertyName` on `model`
  *
@@ -479,10 +475,11 @@ ModelBuilder.prototype.defineProperty = function (model, propertyName, propertyD
  */
 ModelBuilder.prototype.extendModel = function (model, props) {
   var t = this;
-  Object.keys(props).forEach(function (propName) {
+  var keys = Object.keys(props);
-    var definition = props[propName];
+  for (var i = 0; i < keys.length; i++) {
-    t.defineProperty(model, propName, definition);
+    var definition = props[keys[i]];
-  });
+    t.defineProperty(model, keys[i], definition);
+  }
 };
 
 ModelBuilder.prototype.copyModel = function copyModel(Master) {

lib/model.js

@@ -8,13 +8,20 @@ module.exports = ModelBaseClass;
  */
 
 var util = require('util');
-var traverse = require('traverse');
 var jutil = require('./jutil');
 var List = require('./list');
 var Hookable = require('./hooks');
 var validations = require('./validations.js');
 
-var BASE_TYPES = ['String', 'Boolean', 'Number', 'Date', 'Text', 'ObjectID'];
+// Set up an object for quick lookup
+var BASE_TYPES = {
+  'String': true,
+  'Boolean': true,
+  'Number': true,
+  'Date': true,
+  'Text': true,
+  'ObjectID': true
+};
 
 /**
  * Model class: base class for all persistent objects.
@@ -33,18 +40,6 @@ function ModelBaseClass(data, options) {
   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
-function clone(data) {
-  /*
-   if(!(data instanceof ModelBaseClass)) {
-   if(data && (Array.isArray(data) || 'object' === typeof data)) {
-   return traverse(data).clone();
-   }
-   }
-   */
-  return data;
-}
-
 /**
  * Initialize the model instance with a list of properties
  * @param {Object} data The data object
@@ -58,10 +53,10 @@ ModelBaseClass.prototype._initProperties = function (data, options) {
   var ctor = this.constructor;
 
   if(data instanceof ctor) {
-    // Convert the data to be plain object to avoid polutions
+    // Convert the data to be plain object to avoid pollutions
     data = data.toObject(false);
   }
-  var properties = ctor.definition.build();
+  var properties = ctor.definition.properties;
   data = data || {};
 
   options = options || {};
@@ -71,133 +66,124 @@ ModelBaseClass.prototype._initProperties = function (data, options) {
   if(strict === undefined) {
     strict = ctor.definition.settings.strict;
   }
-  Object.defineProperty(this, '__cachedRelations', {
-    writable: true,
-    enumerable: false,
-    configurable: true,
-    value: {}
-  });
 
-  Object.defineProperty(this, '__data', {
+  if (ctor.hideInternalProperties) {
-    writable: true,
+    // Object.defineProperty() is expensive. We only try to make the internal
-    enumerable: false,
+    // properties hidden (non-enumerable) if the model class has the
-    configurable: true,
+    // `hideInternalProperties` set to true
-    value: {}
+    Object.defineProperties(this, {
-  });
+      __cachedRelations: {
+        writable: true,
+        enumerable: false,
+        configurable: true,
+        value: {}
+      },
 
-  Object.defineProperty(this, '__dataWas', {
+      __data: {
-    writable: true,
+        writable: true,
-    enumerable: false,
+        enumerable: false,
-    configurable: true,
+        configurable: true,
-    value: {}
+        value: {}
-  });
+      },
 
-  /**
+      // Instance level data source
-   * Instance level data source
+      __dataSource: {
-   */
+        writable: true,
-  Object.defineProperty(this, '__dataSource', {
+        enumerable: false,
-    writable: true,
+        configurable: true,
-    enumerable: false,
+        value: options.dataSource
-    configurable: true,
+      },
-    value: options.dataSource
-  });
 
-  /**
+      // Instance level strict mode
-   * Instance level strict mode
+      __strict: {
-   */
+        writable: true,
-  Object.defineProperty(this, '__strict', {
+        enumerable: false,
-    writable: true,
+        configurable: true,
-    enumerable: false,
+        value: strict
-    configurable: true,
+      }
-    value: strict
+    });
-  });
+  } else {
+    this.__cachedRelations = {};
+    this.__data = {};
+    this.__dataSource = options.dataSource;
+    this.__strict = strict;
+  }
 
   if (data.__cachedRelations) {
     this.__cachedRelations = data.__cachedRelations;
   }
 
-  for (var i in data) {
+  var keys = Object.keys(data);
-    if (i in properties && typeof data[i] !== 'function') {
+  var size = keys.length;
-      this.__data[i] = this.__dataWas[i] = clone(data[i]);
+  var p, propVal;
-    } else if (i in ctor.relations) {
+  for (var k = 0; k < size; k++) {
-      if (ctor.relations[i].type === 'belongsTo' && data[i] !== null && data[i] !== undefined) {
+    p = keys[k];
+    propVal = data[p];
+    if (typeof propVal === 'function') {
+      continue;
+    }
+    if (properties[p]) {
+      // Managed property
+      if (applySetters) {
+        self[p] = propVal;
+      } else {
+        self.__data[p] = propVal;
+      }
+    } else if (ctor.relations[p]) {
+      // Relation
+      if (ctor.relations[p].type === 'belongsTo' && propVal != null) {
         // If the related model is populated
-        this.__data[ctor.relations[i].keyFrom] = this.__dataWas[i] = data[i][ctor.relations[i].keyTo];
+        self.__data[ctor.relations[p].keyFrom] = propVal[ctor.relations[p].keyTo];
       }
-      this.__cachedRelations[i] = data[i];
+      self.__cachedRelations[p] = propVal;
     } else {
+      // Un-managed property
       if (strict === false) {
-        this.__data[i] = this.__dataWas[i] = clone(data[i]);
+        self[p] = self.__data[p] = propVal;
       } else if (strict === 'throw') {
-        throw new Error('Unknown property: ' + i);
+        throw new Error('Unknown property: ' + p);
       }
     }
   }
 
-  var propertyName;
+  keys = Object.keys(properties);
-  if (applySetters === true) {
+  size = keys.length;
-    for (propertyName in data) {
+
-      if (typeof data[propertyName] !== 'function' && ((propertyName in properties) || (propertyName in ctor.relations))) {
+  for (k = 0; k < size; k++) {
-        self[propertyName] = self.__data[propertyName] || data[propertyName];
+    p = keys[k];
+    propVal = self.__data[p];
+
+    // Set default values
+    if (propVal === undefined) {
+      var def = properties[p]['default'];
+      if (def !== undefined) {
+        if (typeof def === 'function') {
+          self.__data[p] = def();
+        } else {
+          self.__data[p] = def;
+        }
       }
     }
-  }
 
-  // Set the unknown properties as properties to the object
+    // Handle complex types (JSON/Object)
-  if (strict === false) {
+    var type = properties[p].type;
-    for (propertyName in data) {
+    if (! BASE_TYPES[type.name]) {
-      if (typeof data[propertyName] !== 'function' && !(propertyName in properties)) {
+      if (typeof self.__data[p] !== 'object' && self.__data[p]) {
-        self[propertyName] = self.__data[propertyName] || data[propertyName];
-      }
-    }
-  }
-
-  ctor.forEachProperty(function (propertyName) {
-
-    if (undefined === self.__data[propertyName]) {
-      self.__data[propertyName] = self.__dataWas[propertyName] = getDefault(propertyName);
-    } else {
-      self.__dataWas[propertyName] = self.__data[propertyName];
-    }
-
-  });
-
-  ctor.forEachProperty(function (propertyName) {
-
-    var type = properties[propertyName].type;
-
-    if (BASE_TYPES.indexOf(type.name) === -1) {
-      if (typeof self.__data[propertyName] !== 'object' && self.__data[propertyName]) {
         try {
-          self.__data[propertyName] = JSON.parse(self.__data[propertyName] + '');
+          self.__data[p] = JSON.parse(self.__data[p] + '');
         } catch (e) {
-          self.__data[propertyName] = String(self.__data[propertyName]);
+          self.__data[p] = String(self.__data[p]);
         }
       }
       if (type.name === 'Array' || Array.isArray(type)) {
-        if (!(self.__data[propertyName] instanceof List)
+        if (!(self.__data[p] instanceof List)
-          && self.__data[propertyName] !== undefined
+          && self.__data[p] !== undefined
-          && self.__data[propertyName] !== null ) {
+          && self.__data[p] !== null ) {
-          self.__data[propertyName] = List(self.__data[propertyName], type, self);
+          self.__data[p] = List(self.__data[p], type, self);
         }
       }
     }
-
-  });
-
-  function getDefault(propertyName) {
-    var def = properties[propertyName]['default'];
-    if (def !== undefined) {
-      if (typeof def === 'function') {
-        return def();
-      } else {
-        return def;
-      }
-    } else {
-      return undefined;
-    }
   }
-
   this.trigger('initialize');
 };
 
@@ -242,7 +228,7 @@ ModelBaseClass.toString = function () {
  * @param {Boolean} onlySchema Restrict properties to dataSource only.  Default is false.  If true, the function returns only properties defined in the schema;  Otherwise it returns all enumerable properties.
  */
 ModelBaseClass.prototype.toObject = function (onlySchema, removeHidden) {
-  if(onlySchema === undefined) {
+  if (onlySchema === undefined) {
     onlySchema = true;
   }
   var data = {};
@@ -250,47 +236,63 @@ ModelBaseClass.prototype.toObject = function (onlySchema, removeHidden) {
   var Model = this.constructor;
 
   // if it is already an Object
-  if(Model === Object) return self;
+  if (Model === Object) {
+    return self;
+  }
 
   var strict = this.__strict;
   var schemaLess = (strict === false) || !onlySchema;
 
-  this.constructor.forEachProperty(function (propertyName) {
+  var props = Model.definition.properties;
-    if (removeHidden && Model.isHiddenProperty(propertyName)) {
+  var keys = Object.keys(props);
-      return;
+  var propertyName, val;
-    }
+  for (var i = 0; i < keys.length; i++) {
-    if (typeof self[propertyName] === 'function') {
+    propertyName = keys[i];
-      return;
+    val = self[propertyName];
-    }
+
-    
+    // Exclude functions
-    if (self[propertyName] instanceof List) {
+    if (typeof val === 'function') {
-      data[propertyName] = self[propertyName].toObject(!schemaLess, removeHidden);
+      continue;
-    } else if (self.__data.hasOwnProperty(propertyName)) {
+    }
-      if (self[propertyName] !== undefined && self[propertyName] !== null && self[propertyName].toObject) {
+    // Exclude hidden properties
-        data[propertyName] = self[propertyName].toObject(!schemaLess, removeHidden);
+    if (removeHidden && Model.isHiddenProperty(propertyName)) {
-      } else {
+      continue;
-        data[propertyName] = self[propertyName];
+    }
-      }
+
-    } else {
+    if (val instanceof List) {
-      data[propertyName] = null;
+      data[propertyName] = val.toObject(!schemaLess, removeHidden);
-    }
+    } else {
-  });
+      if (val !== undefined && val !== null && val.toObject) {
+        data[propertyName] = val.toObject(!schemaLess, removeHidden);
+      } else {
+        data[propertyName] = val;
+      }
+    }
+  }
 
-  var val = null;
   if (schemaLess) {
     // 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) {
+    keys = Object.keys(self);
-      if(removeHidden && Model.isHiddenProperty(propertyName)) {
+    var size = keys.length;
+    for (i = 0; i < size; i++) {
+      propertyName = keys[i];
+      if (props[propertyName]) {
         continue;
       }
-      if(self.hasOwnProperty(propertyName) && (!data.hasOwnProperty(propertyName))) {
+      if (propertyName.indexOf('__') === 0) {
-        val = self[propertyName];
+        continue;
+      }
+      if (removeHidden && Model.isHiddenProperty(propertyName)) {
+        continue;
+      }
+      val = self[propertyName];
+      if (val !== undefined && data[propertyName] === undefined) {
         if (typeof val === 'function') {
           continue;
         }
-        if (val !== undefined && val !== null && val.toObject) {
+        if (val !== null && val.toObject) {
           data[propertyName] = val.toObject(!schemaLess, removeHidden);
         } else {
           data[propertyName] = val;
@@ -298,15 +300,25 @@ ModelBaseClass.prototype.toObject = function (onlySchema, removeHidden) {
       }
     }
     // Now continue to check __data
-    for (propertyName in self.__data) {
+    keys = Object.keys(self.__data);
-      if (!data.hasOwnProperty(propertyName)) {
+    size = keys.length;
-        if(removeHidden && Model.isHiddenProperty(propertyName)) {
+    for (i = 0; i < size; i++) {
+      propertyName = keys[i];
+      if (propertyName.indexOf('__') === 0) {
+        continue;
+      }
+      if (data[propertyName] === undefined) {
+        if (removeHidden && Model.isHiddenProperty(propertyName)) {
           continue;
         }
-        val = self.hasOwnProperty(propertyName) ? self[propertyName] : self.__data[propertyName];
+        var ownVal = self[propertyName];
+        // The ownVal can be a relation function
+        val = (ownVal !== undefined && (typeof ownVal !== 'function'))
+          ? ownVal : self.__data[propertyName];
         if (typeof val === 'function') {
           continue;
         }
+
         if (val !== undefined && val !== null && val.toObject) {
           data[propertyName] = val.toObject(!schemaLess, removeHidden);
         } else {
@@ -319,12 +331,20 @@ ModelBaseClass.prototype.toObject = function (onlySchema, removeHidden) {
   return data;
 };
 
-ModelBaseClass.isHiddenProperty = function(propertyName) {
+ModelBaseClass.isHiddenProperty = function (propertyName) {
   var Model = this;
   var settings = Model.definition && Model.definition.settings;
-  var hiddenProperties = settings && settings.hidden;
+  var hiddenProperties = settings && (settings.hiddenProperties || settings.hidden);
-  if(hiddenProperties) {
+  if (Array.isArray(hiddenProperties)) {
-    return ~hiddenProperties.indexOf(propertyName);  
+    // Cache the hidden properties as an object for quick lookup
+    settings.hiddenProperties = {};
+    for (var i = 0; i < hiddenProperties.length; i++) {
+      settings.hiddenProperties[hiddenProperties[i]] = true;
+    }
+    hiddenProperties = settings.hiddenProperties;
+  }
+  if (hiddenProperties) {
+    return hiddenProperties[propertyName];
   } else {
     return false;
   }
@@ -340,16 +360,6 @@ ModelBaseClass.prototype.fromObject = function (obj) {
   }
 };
 
-/**
- * Checks is property changed based on current property and initial value
- *
- * @param {String} propertyName Property name
- * @return Boolean
- */
-ModelBaseClass.prototype.propertyChanged = function propertyChanged(propertyName) {
-  return this.__data[propertyName] !== this.__dataWas[propertyName];
-};
-
 /**
  * Reset dirty attributes.
  * This method does not perform any database operations; it just resets the object to its
@@ -361,9 +371,6 @@ ModelBaseClass.prototype.reset = function () {
     if (k !== 'id' && !obj.constructor.dataSource.definitions[obj.constructor.modelName].properties[k]) {
       delete obj[k];
     }
-    if (obj.propertyChanged(k)) {
-      obj[k] = obj[k + '$was'];
-    }
   }
 };
 

test/loopback-dl.test.js

@@ -1299,15 +1299,24 @@ describe('Load models from json', function () {
     customer.should.not.have.property('bio');
 
     // The properties are defined at prototype level
-    assert.equal(Object.keys(customer).length, 0);
+    assert.equal(Object.keys(customer).filter(function (k) {
+      // Remove internal properties
+      return k.indexOf('__') === -1;
+    }).length, 0);
     var count = 0;
     for (var p in customer) {
+      if (p.indexOf('__') === 0) {
+        continue;
+      }
       if (typeof customer[p] !== 'function') {
         count++;
       }
     }
     assert.equal(count, 7); // Please note there is an injected id from User prototype
-    assert.equal(Object.keys(customer.toObject()).length, 6);
+    assert.equal(Object.keys(customer.toObject()).filter(function (k) {
+      // Remove internal properties
+      return k.indexOf('__') === -1;
+    }).length, 6);
 
     done(null, customer);
   });

test/manipulation.test.js

@@ -133,14 +133,11 @@ describe('manipulation', function () {
       Person.findOne(function (err, p) {
         should.not.exist(err);
         p.name = 'Hans';
-        p.propertyChanged('name').should.be.true;
         p.save(function (err) {
           should.not.exist(err);
-          p.propertyChanged('name').should.be.false;
           Person.findOne(function (err, p) {
             should.not.exist(err);
             p.name.should.equal('Hans');
-            p.propertyChanged('name').should.be.false;
             done();
           });
         });
@@ -157,10 +154,8 @@ describe('manipulation', function () {
         p.name = 'Nana';
         p.save(function (err) {
           should.exist(err);
-          p.propertyChanged('name').should.be.true;
           p.save({validate: false}, function (err) {
             should.not.exist(err);
-            p.propertyChanged('name').should.be.false;
             done();
           });
         });
@@ -244,10 +239,7 @@ describe('manipulation', function () {
         person = new Person({name: hw});
 
       person.name.should.equal(hw);
-      person.propertyChanged('name').should.be.false;
       person.name = 'Goodbye, Lenin';
-      person.name$was.should.equal(hw);
-      person.propertyChanged('name').should.be.true;
       (person.createdAt >= now).should.be.true;
       person.isNewRecord().should.be.true;
     });