From 1ec419aa42b11e25a66a779104375db82af2361f Mon Sep 17 00:00:00 2001 From: Anatoliy Chakkaev Date: Sat, 13 Oct 2012 17:59:25 +0400 Subject: [PATCH] Better performance on big datasets read --- lib/abstract-class.js | 139 +++++++++++------------- lib/adapters/mysql.js | 2 + lib/adapters/postgres.js | 4 + lib/adapters/sqlite3.js | 2 + lib/schema.js | 89 +++++++++++++-- lib/validatable.js | 9 +- package.json | 2 +- test/common_test.js | 8 +- test/validations_test.coffee | 205 +---------------------------------- 9 files changed, 163 insertions(+), 297 deletions(-) diff --git a/lib/abstract-class.js b/lib/abstract-class.js index 89cc4b36..f9655504 100644 --- a/lib/abstract-class.js +++ b/lib/abstract-class.js @@ -37,10 +37,6 @@ AbstractClass.prototype._initProperties = function (data, applySetters) { var properties = ds.properties; data = data || {}; - if (data.id) { - defineReadonlyProp(this, 'id', data.id); - } - Object.defineProperty(this, 'cachedRelations', { writable: true, enumerable: false, @@ -48,71 +44,58 @@ AbstractClass.prototype._initProperties = function (data, applySetters) { value: {} }); - Object.keys(properties).forEach(function (attr) { - var _attr = '_' + attr, - attr_was = attr + '_was'; + Object.defineProperty(this, '__data', { + writable: true, + enumerable: false, + configurable: true, + value: {} + }); - // Hidden property to store currrent value - Object.defineProperty(this, _attr, { - writable: true, - enumerable: false, - configurable: true, - value: isdef(data[attr]) ? data[attr] : - (isdef(this[attr]) ? this[attr] : ( - getDefault(attr) - )) + Object.defineProperty(this, '__dataWas', { + writable: true, + enumerable: false, + configurable: true, + value: {} + }); + + for (var i in data) this.__data[i] = this.__dataWas[i] = data[i]; + + if (applySetters && ctor.setter) { + Object.keys(ctor.setter).forEach(function (attr) { + if (self.__data.hasOwnProperty(attr)) { + ctor.setter[attr].call(self, self.__data[attr]); + } }); + } + + ctor.forEachProperty(function (attr) { + + if (!self.__data.hasOwnProperty(attr)) { + self.__data[attr] = self.__dataWas[attr] = getDefault(attr); + } else { + self.__dataWas[attr] = self.__data[attr]; + } + + }); + + ctor.forEachProperty(function (attr) { var type = properties[attr].type; if (BASE_TYPES.indexOf(type.name) === -1) { - if (typeof this[_attr] !== 'object' && this[_attr]) { + if (typeof self.__data[attr] !== 'object' && self.__data[attr]) { try { - this[_attr] = JSON.parse(this[_attr] + ''); + self.__data[attr] = JSON.parse(self.__data[attr] + ''); } catch (e) { console.log(e.stack); } } if (type.name === 'Array' || typeof type === 'object' && type.constructor.name === 'Array') { - this[_attr] = new List(this[_attr], type, this); + self.__data[attr] = new List(self.__data[attr], type, self); } } - // Public setters and getters - Object.defineProperty(this, attr, { - get: function () { - if (ctor.getter[attr]) { - return ctor.getter[attr].call(this); - } else { - return this[_attr]; - } - }, - set: function (value) { - if (ctor.setter[attr]) { - ctor.setter[attr].call(this, value); - } else { - this[_attr] = value; - } - }, - configurable: true, - enumerable: true - }); - - if (data.hasOwnProperty(attr)) { - if (applySetters && ctor.setter[attr]) { - ctor.setter[attr].call(this, data[attr]); - } - - // Getter for initial property - Object.defineProperty(this, attr_was, { - writable: true, - value: this[_attr], - configurable: true, - enumerable: false - }); - } - - }.bind(this)); + }); function getDefault(attr) { var def = properties[attr]['default']; @@ -535,17 +518,34 @@ AbstractClass.prototype.toObject = function (onlySchema) { var data = {}; var ds = this.constructor.schema.definitions[this.constructor.modelName]; var properties = ds.properties; - // weird - Object.keys(onlySchema ? properties : this).concat(['id']).forEach(function (property) { - if (this[property] instanceof List) { - data[property] = this[property].toObject(); + var self = this; + + this.constructor.forEachProperty(function (attr) { + if (self[attr] instanceof List) { + data[attr] = self[attr].toObject(); + } else if (self.hasOwnProperty(attr)) { + data[attr] = self[attr]; } else { - data[property] = this[property]; + data[attr] = null; } - }.bind(this)); + }); + + if (!onlySchema) { + Object.keys(self).forEach(function (attr) { + if (!data.hasOwnProperty(attr)) { + data[attr] = this[attr]; + } + }); + } + return data; }; +AbstractClass.prototype.hasOwnProperty = function (prop) { + return this.__data.hasOwnProperty(prop) || + Object.getOwnPropertyNames(this).indexOf(prop) !== -1; +}; + AbstractClass.prototype.toJSON = function () { return this.toObject(); }; @@ -623,18 +623,10 @@ AbstractClass.prototype.updateAttributes = function updateAttributes(data, cb) { inst._adapter().updateAttributes(model, inst.id, inst.constructor._forDB(data), function (err) { if (!err) { - inst._initProperties(data, false); - /* + // update _was attrs Object.keys(data).forEach(function (key) { - inst[key] = data[key]; - Object.defineProperty(inst, key + '_was', { - writable: false, - configurable: true, - enumerable: false, - value: data[key] - }); + inst.__dataWas[key] = inst.__data[key]; }); - */ } done.call(inst, function () { saveDone.call(inst, function () { @@ -660,7 +652,7 @@ AbstractClass.prototype.fromObject = function (obj) { * @return Boolean */ AbstractClass.prototype.propertyChanged = function propertyChanged(attr) { - return this['_' + attr] !== this[attr + '_was']; + return this.__data[attr] !== this.__dataWas[attr]; }; /** @@ -801,7 +793,7 @@ function defineScope(cls, targetClass, name, params, methods) { cls._scopeMeta = {}; } - // anly make sence to add scope in meta if base and target classes + // only makes sence to add scope in meta if base and target classes // are same if (cls === targetClass) { cls._scopeMeta[name] = params; @@ -852,8 +844,7 @@ function defineScope(cls, targetClass, name, params, methods) { // and it should have create/build methods with binded thisModelNameId param function build(data) { - data = data || {}; - return new targetClass(mergeParams(this._scope, {where:data}).where); + return new targetClass(mergeParams(this._scope, {where:data || {}}).where); } function create(data, cb) { diff --git a/lib/adapters/mysql.js b/lib/adapters/mysql.js index add85402..a10a7eed 100644 --- a/lib/adapters/mysql.js +++ b/lib/adapters/mysql.js @@ -365,6 +365,7 @@ MySQL.prototype.alterTable = function (model, actualFields, actualIndexes, done, // change/add new fields propNames.forEach(function (propName) { + if (propName === 'id') return; var found; actualFields.forEach(function (f) { if (f.Field === propName) { @@ -488,6 +489,7 @@ MySQL.prototype.propertiesSQL = function (model) { var self = this; var sql = ['`id` INT(11) NOT NULL AUTO_INCREMENT UNIQUE PRIMARY KEY']; Object.keys(this._models[model].properties).forEach(function (prop) { + if (prop === 'id') return; sql.push('`' + prop + '` ' + self.propertySettingsSQL(model, prop)); }); return sql.join(',\n '); diff --git a/lib/adapters/postgres.js b/lib/adapters/postgres.js index cc0caf7d..1e634de1 100644 --- a/lib/adapters/postgres.js +++ b/lib/adapters/postgres.js @@ -115,6 +115,7 @@ PG.prototype.toFields = function (model, data, forCreate) { var columns = []; Object.keys(data).forEach(function (key) { if (props[key]) { + if (key === 'id') return; columns.push('"' + key + '"'); fields.push(this.toDatabase(props[key], data[key])); } @@ -363,6 +364,7 @@ function getColumnsToAdd(model, actualFields){ var propNames = Object.keys(m.properties); var sql = []; propNames.forEach(function (propName) { + if (propName === 'id') return; var found = searchForPropertyInActual.call(self, propName, actualFields); if(!found && propertyHasNotBeenDeleted.call(self, model, propName)){ sql.push(addPropertyToActual.call(self, model, propName)); @@ -396,6 +398,7 @@ function getPropertiesToModify(model, actualFields){ var propNames = Object.keys(m.properties); var found; propNames.forEach(function (propName) { + if (propName === 'id') return; found = searchForPropertyInActual.call(self, propName, actualFields); if(found && propertyHasNotBeenDeleted.call(self, model, propName)){ if (datatypeChanged(propName, found)) { @@ -484,6 +487,7 @@ PG.prototype.propertiesSQL = function (model) { var self = this; var sql = ['"id" SERIAL PRIMARY KEY']; Object.keys(this._models[model].properties).forEach(function (prop) { + if (prop === 'id') return; sql.push('"' + prop + '" ' + self.propertySettingsSQL(model, prop)); }); return sql.join(',\n '); diff --git a/lib/adapters/sqlite3.js b/lib/adapters/sqlite3.js index b3a5e084..58d5a30b 100644 --- a/lib/adapters/sqlite3.js +++ b/lib/adapters/sqlite3.js @@ -330,6 +330,7 @@ SQLite3.prototype.alterTable = function (model, actualFields, done) { // change/add new fields propNames.forEach(function (propName) { + if (propName === 'id') return; var found; actualFields.forEach(function (f) { if (f.Field === propName) { @@ -378,6 +379,7 @@ SQLite3.prototype.propertiesSQL = function (model) { var self = this; var sql = ['`id` INTEGER PRIMARY KEY']; Object.keys(this._models[model].properties).forEach(function (prop) { + if (prop === 'id') return; sql.push('`' + prop + '` ' + self.propertySettingsSQL(model, prop)); }); return sql.join(',\n '); diff --git a/lib/schema.js b/lib/schema.js index 37638e49..c72e6a02 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -146,24 +146,24 @@ Schema.prototype.define = function defineClass(className, properties, settings) standartize(properties, settings); // every class can receive hash of data as optional param - var newClass = function ModelConstructor(data) { + var NewClass = function ModelConstructor(data) { if (!(this instanceof ModelConstructor)) { return new ModelConstructor(data); } AbstractClass.call(this, data); }; - hiddenProperty(newClass, 'schema', schema); - hiddenProperty(newClass, 'modelName', className); - hiddenProperty(newClass, 'cache', {}); - hiddenProperty(newClass, 'mru', []); + hiddenProperty(NewClass, 'schema', schema); + hiddenProperty(NewClass, 'modelName', className); + hiddenProperty(NewClass, 'cache', {}); + hiddenProperty(NewClass, 'mru', []); // setup inheritance - newClass.__proto__ = AbstractClass; - util.inherits(newClass, AbstractClass); + NewClass.__proto__ = AbstractClass; + util.inherits(NewClass, AbstractClass); // store class in model pool - this.models[className] = newClass; + this.models[className] = NewClass; this.definitions[className] = { properties: properties, settings: settings @@ -171,12 +171,60 @@ Schema.prototype.define = function defineClass(className, properties, settings) // pass controll to adapter this.adapter.define({ - model: newClass, + model: NewClass, properties: properties, settings: settings }); - return newClass; + NewClass.prototype.__defineGetter__('id', function () { + return this.__data.id; + }); + + properties.id = properties.id || { type: Number }; + + NewClass.forEachProperty = function (cb) { + Object.keys(properties).forEach(cb); + }; + + NewClass.registerProperty = function (attr) { + Object.defineProperty(NewClass.prototype, attr, { + get: function () { + if (NewClass.getter[attr]) { + return NewClass.getter[attr].call(this); + } else { + return this.__data[attr]; + } + }, + set: function (value) { + if (NewClass.setter[attr]) { + NewClass.setter[attr].call(this, value); + } else { + this.__data[attr] = value; + } + }, + configurable: true, + enumerable: true + }); + + NewClass.prototype.__defineGetter__(attr + '_was', function () { + return this.__dataWas[attr]; + }); + + Object.defineProperty(NewClass.prototype, '_' + attr, { + get: function () { + return this.__data[attr]; + }, + set: function (value) { + this.__data[attr] = value; + }, + configurable: true, + enumerable: false + }); + }; + + NewClass.forEachProperty(NewClass.registerProperty); + + return NewClass; function standartize(properties, settings) { Object.keys(properties).forEach(function (key) { @@ -206,6 +254,7 @@ Schema.prototype.define = function defineClass(className, properties, settings) */ Schema.prototype.defineProperty = function (model, prop, params) { this.definitions[model].properties[prop] = params; + this.models[model].registerProperty(prop); if (this.adapter.defineProperty) { this.adapter.defineProperty(model, prop, params); } @@ -285,7 +334,7 @@ Schema.prototype.tableName = function (modelName) { * @param {String} key - name of key field */ Schema.prototype.defineForeignKey = function defineForeignKey(className, key) { - // return if already defined + // quit if key already defined if (this.definitions[className].properties[key]) return; if (this.adapter.defineForeignKey) { @@ -296,6 +345,7 @@ Schema.prototype.defineForeignKey = function defineForeignKey(className, key) { } else { this.definitions[className].properties[key] = {type: Number}; } + this.models[className].registerProperty(key); }; /** @@ -303,6 +353,7 @@ Schema.prototype.defineForeignKey = function defineForeignKey(className, key) { */ Schema.prototype.disconnect = function disconnect() { if (typeof this.adapter.disconnect === 'function') { + this.connected = true; this.adapter.disconnect(); } }; @@ -319,3 +370,19 @@ function hiddenProperty(where, property, value) { }); } +/** + * Define readonly property on object + * + * @param {Object} obj + * @param {String} key + * @param {Mixed} value + */ +function defineReadonlyProp(obj, key, value) { + Object.defineProperty(obj, key, { + writable: false, + enumerable: true, + configurable: true, + value: value + }); +} + diff --git a/lib/validatable.js b/lib/validatable.js index 3600e6f4..13679ec7 100644 --- a/lib/validatable.js +++ b/lib/validatable.js @@ -420,17 +420,18 @@ function validationFailed(inst, v, cb) { } function skipValidation(inst, conf, kind) { + console.log(conf, kind, inst.hasOwnProperty(conf[kind]), inst[conf[kind]]); var doValidate = true; if (typeof conf[kind] === 'function') { doValidate = conf[kind].call(inst); if (kind === 'unless') doValidate = !doValidate; } else if (typeof conf[kind] === 'string') { - if (inst.hasOwnProperty(conf[kind])) { - doValidate = inst[conf[kind]]; - if (kind === 'unless') doValidate = !doValidate; - } else if (typeof inst[conf[kind]] === 'function') { + if (typeof inst[conf[kind]] === 'function') { doValidate = inst[conf[kind]].call(inst); if (kind === 'unless') doValidate = !doValidate; + } else if (inst.hasOwnProperty(conf[kind])) { + doValidate = inst[conf[kind]]; + if (kind === 'unless') doValidate = !doValidate; } else { doValidate = kind === 'if'; } diff --git a/package.json b/package.json index 32dabc8f..2228f695 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ }, "main": "index.js", "scripts": { - "test": "EXCEPT=cradle nodeunit test/*_test*" + "test": "EXCEPT=cradle,neo4j nodeunit test/*_test*" }, "engines": [ "node >= 0.4.12" diff --git a/test/common_test.js b/test/common_test.js index 41b9d03a..fb34e633 100644 --- a/test/common_test.js +++ b/test/common_test.js @@ -150,7 +150,7 @@ function testOrm(schema) { anotherPost = Post({title: 'Resig style constructor'}); test.equal(post.title, hw); - test.ok(!post.propertyChanged('title')); + test.ok(!post.propertyChanged('title'), 'property changed: title'); post.title = 'Goodbye, Lenin'; test.equal(post.title_was, hw); test.ok(post.propertyChanged('title')); @@ -164,7 +164,7 @@ function testOrm(schema) { it('should be expoted to JSON', function (test) { test.equal(JSON.stringify(new Post({id: 1, title: 'hello, json', date: 1})), - '{"id":1,"title":"hello, json","subject":null,"content":null,"date":1,"published":false,"likes":[],"related":[],"userId":null}'); + '{"title":"hello, json","subject":null,"content":null,"date":1,"published":false,"likes":[],"related":[],"id":1,"userId":null}'); test.done(); }); @@ -344,6 +344,7 @@ function testOrm(schema) { post.updateAttribute('title', 'New title', function () { test.equal(post.title, 'New title'); test.ok(!post.propertyChanged('title')); + console.log('hahaha', post.content, post.__data.content); test.equal(post.content, 'New content', 'dirty state saved'); test.ok(post.propertyChanged('content')); post.reload(function (err, post) { @@ -447,7 +448,7 @@ function testOrm(schema) { test.ok(Post.scope, 'Scope supported'); Post.scope('published', {where: {published: true}}); test.ok(typeof Post.published === 'function'); - test.ok(Post.published._scope.published = true); + test.ok(Post.published._scope.where.published === true); var post = Post.published.build(); test.ok(post.published, 'Can build'); test.ok(post.isNewRecord()); @@ -462,6 +463,7 @@ function testOrm(schema) { if (err) return console.log(err); test.ok(typeof u.posts.published == 'function'); test.ok(u.posts.published._scope.where.published); + console.log(u.posts.published._scope); test.equal(u.posts.published._scope.where.userId, u.id); done(); }); diff --git a/test/validations_test.coffee b/test/validations_test.coffee index bc1d962c..72e9b333 100644 --- a/test/validations_test.coffee +++ b/test/validations_test.coffee @@ -16,6 +16,7 @@ User = schema.define 'User', domain: String pendingPeriod: Number createdByAdmin: Boolean + createdByScript: Boolean updatedAt: Date validAttributes = @@ -85,209 +86,5 @@ it 'should allow to skip validations', (test) -> user.domain = 'xyz' test.ok not user.isValid() # is: 3 passed, but is: 2 failed - test.done() -it 'should throw error on save if required', (test) -> - user = new User - - test.throws () -> - user.save throws: true - - test.done() - - -it 'should allow to skip validation on save', (test) -> - user = new User - test.ok user.isNewRecord(), 'User not saved yet' - test.ok not user.isValid(), 'User not valid' - - user.save validate: false - - test.ok not user.isNewRecord(), 'User saved' - test.ok not user.isValid(), 'User data still not valid' - test.done() - -it 'should perform validation on updateAttributes', (test) -> - User.create email: 'anatoliy@localhost', name: 'anatoliy', (err, user) -> - user.updateAttributes name: null, (err, name) -> - test.ok(err) - test.ok user.errors - test.ok user.errors.name - test.done() - -it 'should perform validation on create', (test) -> - User.create (err, user) -> - test.ok err, 'We have an error' - # we got an user, - test.ok user, 'We got an user' - # but it's not saved - test.ok user.isNewRecord(), 'User not saved' - # and we have errors - test.ok user.errors, 'User have errors' - # explaining what happens - test.ok user.errors.name, 'Errors contain name' - test.ok user.errors.email, 'Errors contain email' - - test.done() - -it 'should validate length', (test) -> - User.validatesLengthOf 'password', min: 3, max: 10, allowNull: true - User.validatesLengthOf 'state', is: 2, allowBlank: true - user = new User validAttributes - - user.password = 'qw' - test.ok not user.isValid(), 'Invalid: too short' - test.equal user.errors.password[0], 'too short' - - user.password = '12345678901' - test.ok not user.isValid(), 'Invalid: too long' - test.equal user.errors.password[0], 'too long' - - user.password = 'hello' - test.ok user.isValid(), 'Valid with value' - test.ok not user.errors - - user.password = null - test.ok user.isValid(), 'Valid without value' - test.ok not user.errors - - user.state = 'Texas' - test.ok not user.isValid(), 'Invalid state' - test.equal user.errors.state[0], 'length is wrong' - - user.state = 'TX' - test.ok user.isValid(), 'Valid with value of state' - test.ok not user.errors - - test.done() - -it 'should validate numericality', (test) -> - User.validatesNumericalityOf 'age', int: true - user = new User validAttributes - - user.age = '26' - test.ok not user.isValid(), 'User is not valid: not a number' - test.equal user.errors.age[0], 'is not a number' - - user.age = 26.1 - test.ok not user.isValid(), 'User is not valid: not integer' - test.equal user.errors.age[0], 'is not an integer' - - user.age = 26 - test.ok user.isValid(), 'User valid: integer age' - test.ok not user.errors - - test.done() - -it 'should validate inclusion', (test) -> - User.validatesInclusionOf 'gender', in: ['male', 'female'] - user = new User validAttributes - - user.gender = 'any' - test.ok not user.isValid() - test.equal user.errors.gender[0], 'is not included in the list' - - user.gender = 'female' - test.ok user.isValid() - - user.gender = 'male' - test.ok user.isValid() - - user.gender = 'man' - test.ok not user.isValid() - test.equal user.errors.gender[0], 'is not included in the list' - - test.done() - -it 'should validate exclusion', (test) -> - User.validatesExclusionOf 'domain', in: ['www', 'admin'] - user = new User validAttributes - - user.domain = 'www' - test.ok not user.isValid() - test.equal user.errors.domain[0], 'is reserved' - - user.domain = 'my' - test.ok user.isValid() - - test.done() - -it 'should validate format', (test) -> - User.validatesFormatOf 'email', with: /^([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})$/i - user = new User validAttributes - - user.email = 'invalid email' - test.ok not user.isValid() - - user.email = 'valid@email.tld' - test.ok user.isValid() - - test.done() - -it 'should validate a field using a custom validator', (test) -> - - validator = (err) -> - err('crash') if @name == 'bad name' - - User.validate 'name', validator, message: crash: 'custom message' - - user = new User validAttributes - test.ok user.isValid() - - user = new User validAttributes - user.name = 'bad name' - test.ok not user.isValid(), 'invalid due custom name validator' - test.equal user.errors.name[0], 'custom message' - - test.done() - -it 'should validate asynchronously', (test) -> - - validator = (err, done) -> - setTimeout => - err 'async' if @name == 'bad name' - done() - , 10 - - User.validateAsync 'name', validator, message: async: 'hello' - - user = new User validAttributes - test.ok not user.isValid(), 'not valid because async validation' - user.isValid (valid) -> - test.ok valid, 'valid name' - - user.name = 'bad name' - user.isValid (valid) -> - test.ok not valid, 'not valid name' - test.done() - -it 'should validate uniqueness', (test) -> - User.validatesUniquenessOf 'email' - User.create getValidAttributes(), (err, user) -> - user = new User getValidAttributes() - - # test.ok not user.isValid(), 'not valid because async validation' - user.isValid (valid) -> - test.ok not valid, 'email is not unique' - user.email = 'unique@email.tld' - user.isValid (valid) -> - test.ok valid, 'valid with unique email' - user.save (err) -> - test.ok not user.propertyChanged('email'), 'Email changed' - user.updateAttributes { updatedAt: new Date, createdByAdmin: false }, (err) -> - User.all where: email: 'unique@email.tld', (err, users) -> - test.ok users[0] - test.ok users[0].email == 'unique@email.tld' - test.ok !err, 'Updated' - test.done() - -it 'should save dirty state when validating uniqueness', (test) -> - User.all where: email: 'unique@email.tld', (err, users) -> - u = users[0] - u.name = 'Hulk' - u.isValid (valid) -> - test.ok valid, 'Invalid user' - test.equal u.name, 'Hulk' - test.done() -