Merge pull request sequelize#1071 from mickhansen/refactor-attribute-…

…manipulation Performance optimizations for build
snit-ram · Nov 23, 2013 · f8ee94b · f8ee94b
2 parents 53007f2 + a46121c
commit f8ee94b
Show file tree

Hide file tree

Showing 6 changed files with 56 additions and 67 deletions.
diff --git a/lib/associations/belongs-to.js b/lib/associations/belongs-to.js
@@ -32,8 +32,8 @@ module.exports = (function() {
     Helpers.addForeignKeyConstraints(newAttributes[this.identifier], this.target, this.source, this.options)
     Utils._.defaults(this.source.rawAttributes, newAttributes)
 
-    // Sync attributes to DAO proto each time a new assoc is added
-    this.source.DAO.prototype.attributes = Object.keys(this.source.DAO.prototype.rawAttributes)
+    // Sync attributes and setters/getters to DAO prototype
+    this.source.refreshAttributes()
 
     return this
   }

diff --git a/lib/associations/has-many.js b/lib/associations/has-many.js
@@ -109,9 +109,9 @@ module.exports = (function() {
       Utils._.defaults(this.target.rawAttributes, newAttributes)
     }
 
-    // Sync attributes to DAO proto each time a new assoc is added
-    this.target.DAO.prototype.attributes = Object.keys(this.target.DAO.prototype.rawAttributes);
-    this.source.DAO.prototype.attributes = Object.keys(this.source.DAO.prototype.rawAttributes);
+    // Sync attributes and setters/getters to DAO prototype
+    this.target.refreshAttributes()
+    this.source.refreshAttributes()
 
     return this
   }

diff --git a/lib/associations/has-one.js b/lib/associations/has-one.js
@@ -37,9 +37,8 @@ module.exports = (function() {
     Helpers.addForeignKeyConstraints(newAttributes[this.identifier], this.source, this.target, this.options)
     Utils._.defaults(this.target.rawAttributes, newAttributes)
 
-    // Sync attributes to DAO proto each time a new assoc is added
-    this.target.DAO.prototype.attributes = Object.keys(this.target.DAO.prototype.rawAttributes);
-
+    // Sync attributes and setters/getters to DAO prototype
+    this.target.refreshAttributes()
     return this
   }
 

diff --git a/lib/dao-factory.js b/lib/dao-factory.js
@@ -141,28 +141,8 @@ module.exports = (function() {
       })
     }
 
-    Utils._.each(['Get', 'Set'], function(type) {
-      var prop  = type.toLowerCase()
-        , opt   = prop + 'terMethods'
-        , meth  = '__define' + type + 'ter__'
-        , funcs = Utils._.isObject(self.options[opt]) ? self.options[opt] : {}
-
-      Utils._.each(self.rawAttributes, function(attr, name) {
-        if (attr.hasOwnProperty(prop)) {
-          funcs[name] = attr[prop]
-        }
-      })
+    this.refreshAttributes();
 
-      Utils._.each(funcs, function(fct, name) {
-        if (!Utils._.isFunction(fct)) {
-          throw new Error(type + 'ter for "' + name + '" is not a function.')
-        }
-
-        self.DAO.prototype[meth](name, fct)
-      })
-    })
-
-    this.DAO.prototype.attributes    = Object.keys(this.DAO.prototype.rawAttributes);
     this.DAO.prototype.booleanValues = []
     this.DAO.prototype.defaultValues = {}
     this.DAO.prototype.validators    = {}
@@ -187,6 +167,53 @@ module.exports = (function() {
     return this
   }
 
+  DAOFactory.prototype.refreshAttributes = function() {
+    var self = this
+      , attributeManipulation = {};
+
+    Utils._.each(['get', 'set'], function(type) {
+      var opt   = type + 'terMethods'
+        , funcs = Utils._.isObject(self.options[opt]) ? self.options[opt] : {}
+
+      Utils._.each(self.rawAttributes, function(options, attribute) {
+        if (options.hasOwnProperty(type)) {
+          funcs[attribute] = options[type]
+        } else if (typeof funcs[attribute] === "undefined") {
+          if (type === 'get') {
+            funcs[attribute] = function()  { return this.dataValues[attribute]; }
+          }
+          if (type === 'set') {
+            funcs[attribute] = function(value) {
+              if (Utils.hasChanged(this.dataValues[attribute], value)) {
+                //Only dirty the object if the change is not due to id, touchedAt, createdAt or updatedAt being initiated
+                var updatedAtAttr = Utils._.underscoredIf(this.__options.updatedAt, this.__options.underscored)
+                  , createdAtAttr = Utils._.underscoredIf(this.__options.createdAt, this.__options.underscored)
+                  , touchedAtAttr = Utils._.underscoredIf(this.__options.touchedAt, this.__options.underscored)
+
+                if (this.dataValues[attribute] || (attribute != 'id' && attribute != touchedAtAttr && attribute != createdAtAttr && attribute != updatedAtAttr)) {
+                  this.isDirty = true
+                }
+              }
+              this.dataValues[attribute] = value
+            }
+          }
+        }
+      })
+
+      Utils._.each(funcs, function(fct, name) {
+        if (!attributeManipulation[name]) {
+          attributeManipulation[name] = {
+            configurable: true
+          }
+        }
+        attributeManipulation[name][type] = fct
+      })
+    })
+
+    Object.defineProperties(this.DAO.prototype, attributeManipulation)
+    this.DAO.prototype.attributes = Object.keys(this.DAO.prototype.rawAttributes)
+  }
+
   DAOFactory.prototype.sync = function(options) {
     options = Utils._.extend({}, this.options, options || {})
 

diff --git a/lib/dao.js b/lib/dao.js
@@ -427,43 +427,6 @@ module.exports = (function() {
       value = !!value
     }
 
-    var has = (function(o) {
-      var predef = Object.getOwnPropertyDescriptor(o, attribute);
-
-      if (predef && predef.hasOwnProperty('value')) {
-        return true // true here means 'this property exist as a simple value property, do not place setters or getters at all'
-      }
-
-      return {
-        get: (predef && predef.hasOwnProperty('get') ? predef.get : null) || o.__lookupGetter__(attribute),
-        set: (predef && predef.hasOwnProperty('set') ? predef.set : null) || o.__lookupSetter__(attribute)
-      };
-    })(this);
-
-    // @ node-v0.8.19:
-    //    calling __defineGetter__ destroys any previously defined setters for the attribute in
-    //    question *if* that property setter was defined on the object's prototype (which is what
-    //    we do in dao-factory) ... therefore we need to [re]define both the setter and getter
-    //    here with either the function that already existed OR the default/automatic definition
-    //
-    //    (the same is true for __defineSetter and 'prototype' getters)
-    if (has !== true) {
-      this.__defineGetter__(attribute, has.get || function()  { return this.dataValues[attribute]; });
-      this.__defineSetter__(attribute, has.set || function(v) {
-        if (Utils.hasChanged(this.dataValues[attribute], v)) {
-          //Only dirty the object if the change is not due to id, touchedAt, createdAt or updatedAt being initiated
-          var updatedAtAttr = Utils._.underscoredIf(this.__options.updatedAt, this.__options.underscored)
-            , createdAtAttr = Utils._.underscoredIf(this.__options.createdAt, this.__options.underscored)
-            , touchedAtAttr = Utils._.underscoredIf(this.__options.touchedAt, this.__options.underscored)
-
-          if (this.dataValues[attribute] || (attribute != 'id' && attribute != touchedAtAttr && attribute != createdAtAttr && attribute != updatedAtAttr)) {
-            this.isDirty = true
-          }
-        }
-        this.dataValues[attribute] = v
-      });
-    }
-
     this[attribute] = value;
   }
 

diff --git a/test/dao-factory.test.js b/test/dao-factory.test.js
@@ -1953,7 +1953,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
         this.User.find({ limit: 10 }).success(function(user) {
           // it returns an object instead of an array
           expect(Array.isArray(user)).to.not.be.ok
-          expect(user.hasOwnProperty('username')).to.be.ok
+          expect(user.dataValues.hasOwnProperty('username')).to.be.ok
           done()
         })
       })