From 65e36f7b3a538ae85e5c1bb70ea7916b66a7ecef Mon Sep 17 00:00:00 2001 From: Mick Hansen Date: Fri, 22 Nov 2013 12:59:42 +0100 Subject: [PATCH 1/4] first attempt at refactoring getters/setters to use defineproperty on the prototype --- lib/dao-factory.js | 47 ++++++++++++++++++++++++++++++++++------------ lib/dao.js | 37 ------------------------------------ 2 files changed, 35 insertions(+), 49 deletions(-) diff --git a/lib/dao-factory.js b/lib/dao-factory.js index 8c1c5fcbacab..28e9a5a33a55 100644 --- a/lib/dao-factory.js +++ b/lib/dao-factory.js @@ -141,26 +141,49 @@ module.exports = (function() { }) } - Utils._.each(['Get', 'Set'], function(type) { - var prop = type.toLowerCase() - , opt = prop + 'terMethods' - , meth = '__define' + type + 'ter__' + var 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(attr, name) { - if (attr.hasOwnProperty(prop)) { - funcs[name] = attr[prop] + + Utils._.each(self.rawAttributes, function(attribute, name) { + if (attribute.hasOwnProperty(type)) { + funcs[name] = attribute[type] + } else if (typeof funcs[name] === "undefined") { + if (type === 'get') { + funcs[name] = function() { return this.dataValues[attribute]; } + } + if (type === 'set') { + funcs[name] = 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 (!Utils._.isFunction(fct)) { - throw new Error(type + 'ter for "' + name + '" is not a function.') - } - - self.DAO.prototype[meth](name, fct) + if (!attributeManipulation[name]) attributeManipulation[name] = {} + attributeManipulation[name][type] = fct }) }) + + for (var attribute in attributeManipulation) { + if (attributeManipulation.hasOwnProperty(attribute)) { + Object.defineProperty(this.DAO.prototype, attribute, attributeManipulation[attribute]) + } + } this.DAO.prototype.attributes = Object.keys(this.DAO.prototype.rawAttributes); this.DAO.prototype.booleanValues = [] diff --git a/lib/dao.js b/lib/dao.js index 09975e562aa7..a0350aaf8293 100644 --- 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; } From 7811323372b390d388ed8d5bfdfa5942d10ffcd9 Mon Sep 17 00:00:00 2001 From: Mick Hansen Date: Fri, 22 Nov 2013 13:04:48 +0100 Subject: [PATCH 2/4] use the proper key --- lib/dao-factory.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/dao-factory.js b/lib/dao-factory.js index 28e9a5a33a55..59564b82eaa9 100644 --- a/lib/dao-factory.js +++ b/lib/dao-factory.js @@ -147,16 +147,15 @@ module.exports = (function() { var opt = type + 'terMethods' , funcs = Utils._.isObject(self.options[opt]) ? self.options[opt] : {} - - Utils._.each(self.rawAttributes, function(attribute, name) { - if (attribute.hasOwnProperty(type)) { - funcs[name] = attribute[type] - } else if (typeof funcs[name] === "undefined") { + 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[name] = function() { return this.dataValues[attribute]; } + funcs[attribute] = function() { return this.dataValues[attribute]; } } if (type === 'set') { - funcs[name] = function(value) { + 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) From 79c364db51a363aa0a9e55f358521121e04bd844 Mon Sep 17 00:00:00 2001 From: Mick Hansen Date: Fri, 22 Nov 2013 13:06:03 +0100 Subject: [PATCH 3/4] might aswell not loop --- lib/dao-factory.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/dao-factory.js b/lib/dao-factory.js index 59564b82eaa9..0f0b0eb56d13 100644 --- a/lib/dao-factory.js +++ b/lib/dao-factory.js @@ -178,11 +178,7 @@ module.exports = (function() { }) }) - for (var attribute in attributeManipulation) { - if (attributeManipulation.hasOwnProperty(attribute)) { - Object.defineProperty(this.DAO.prototype, attribute, attributeManipulation[attribute]) - } - } + Object.defineProperties(this.DAO.prototype, attributeManipulation) this.DAO.prototype.attributes = Object.keys(this.DAO.prototype.rawAttributes); this.DAO.prototype.booleanValues = [] From a46121ce682df92bb604825ecca48683345ef360 Mon Sep 17 00:00:00 2001 From: Mick Hansen Date: Fri, 22 Nov 2013 15:22:29 +0100 Subject: [PATCH 4/4] refactor a few things to support associations still ;) --- lib/associations/belongs-to.js | 4 +-- lib/associations/has-many.js | 6 ++-- lib/associations/has-one.js | 5 ++- lib/dao-factory.js | 61 +++++++++++++++++++--------------- test/dao-factory.test.js | 2 +- 5 files changed, 43 insertions(+), 35 deletions(-) diff --git a/lib/associations/belongs-to.js b/lib/associations/belongs-to.js index d23b94c62f84..2965ccfb2a66 100644 --- 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 index 87f4d3bc7fd7..4b68f51d1288 100644 --- 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 index 6f60c05f28b9..9c53db0045ef 100644 --- 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 index 0f0b0eb56d13..92f3cd17ad8b 100644 --- a/lib/dao-factory.js +++ b/lib/dao-factory.js @@ -141,7 +141,35 @@ module.exports = (function() { }) } - var attributeManipulation = {}; + this.refreshAttributes(); + + this.DAO.prototype.booleanValues = [] + this.DAO.prototype.defaultValues = {} + this.DAO.prototype.validators = {} + + Utils._.each(this.rawAttributes, function (definition, name) { + if (((definition === DataTypes.BOOLEAN) || (definition.type === DataTypes.BOOLEAN))) { + self.DAO.prototype.booleanValues.push(name); + } + if (definition.hasOwnProperty('defaultValue')) { + self.DAO.prototype.defaultValues[name] = Utils._.partial( + Utils.toDefaultValue, definition.defaultValue) + } + + if (definition.hasOwnProperty('validate')) { + self.DAO.prototype.validators[name] = definition.validate; + } + }) + + this.DAO.prototype.__factory = this + this.DAO.prototype.hasDefaultValues = !Utils._.isEmpty(this.DAO.prototype.defaultValues) + + return this + } + + DAOFactory.prototype.refreshAttributes = function() { + var self = this + , attributeManipulation = {}; Utils._.each(['get', 'set'], function(type) { var opt = type + 'terMethods' @@ -173,36 +201,17 @@ module.exports = (function() { }) Utils._.each(funcs, function(fct, name) { - if (!attributeManipulation[name]) attributeManipulation[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); - this.DAO.prototype.booleanValues = [] - this.DAO.prototype.defaultValues = {} - this.DAO.prototype.validators = {} - - Utils._.each(this.rawAttributes, function (definition, name) { - if (((definition === DataTypes.BOOLEAN) || (definition.type === DataTypes.BOOLEAN))) { - self.DAO.prototype.booleanValues.push(name); - } - if (definition.hasOwnProperty('defaultValue')) { - self.DAO.prototype.defaultValues[name] = Utils._.partial( - Utils.toDefaultValue, definition.defaultValue) - } - - if (definition.hasOwnProperty('validate')) { - self.DAO.prototype.validators[name] = definition.validate; - } - }) - - this.DAO.prototype.__factory = this - this.DAO.prototype.hasDefaultValues = !Utils._.isEmpty(this.DAO.prototype.defaultValues) - - return this + this.DAO.prototype.attributes = Object.keys(this.DAO.prototype.rawAttributes) } DAOFactory.prototype.sync = function(options) { diff --git a/test/dao-factory.test.js b/test/dao-factory.test.js index a73d8e945a1b..119e6e6f1f31 100644 --- 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() }) })