From 9141d4354883c407b365a0e145b6f187c38e6b09 Mon Sep 17 00:00:00 2001 From: Arek W Date: Tue, 27 Aug 2013 22:13:34 +1000 Subject: [PATCH 1/4] Properties refactor --- lib/Associations/One.js | 2 + lib/Drivers/DDL/mysql.js | 41 +++++++-------- lib/Drivers/DDL/postgres.js | 48 ++++++------------ lib/Drivers/DDL/sqlite.js | 36 ++++--------- lib/Drivers/DML/postgres.js | 15 ++++-- lib/Instance.js | 93 +++++++++++++++++++--------------- lib/Model.js | 81 +++++++++++++++++------------ test/integration/model-get.js | 9 ++-- test/integration/model-keys.js | 3 ++ test/integration/model-save.js | 9 ++-- test/support/spec_helper.js | 6 ++- 11 files changed, 169 insertions(+), 174 deletions(-) diff --git a/lib/Associations/One.js b/lib/Associations/One.js index a2a839f5..490b6ed9 100644 --- a/lib/Associations/One.js +++ b/lib/Associations/One.js @@ -51,6 +51,8 @@ exports.prepare = function (Model, associations, association_properties, model_f for (k in association.field) { association_properties.push(k); if (!association.reversed) { + Model.allProperties[k] = _.omit(association.field[k], k); + Model.allProperties[k].klass = 'hasOne'; model_fields.push(k); } } diff --git a/lib/Drivers/DDL/mysql.js b/lib/Drivers/DDL/mysql.js index c8187c8e..fecaeafb 100755 --- a/lib/Drivers/DDL/mysql.js +++ b/lib/Drivers/DDL/mysql.js @@ -23,39 +23,29 @@ exports.drop = function (driver, opts, cb) { exports.sync = function (driver, opts, cb) { var queries = []; var definitions = []; - var k, i, pending; + var k, i, pending, prop; var primary_keys = opts.id.map(function (k) { return driver.query.escapeId(k); }); var keys = []; - for (i = 0; i < opts.id.length; i++) { - if (opts.properties.hasOwnProperty(opts.id[i])) continue; - - keys.push(driver.query.escapeId(opts.id[i])); - } - - for (i = 0; i < keys.length; i++) { - definitions.push(keys[i] + " INT(10) UNSIGNED NOT NULL"); - } - if (opts.id.length == 1 && !opts.extension) { - definitions[definitions.length - 1] += " AUTO_INCREMENT"; - } - - for (k in opts.properties) { - definitions.push(buildColumnDefinition(driver, k, opts.properties[k])); + for (k in opts.allProperties) { + prop = opts.allProperties[k]; + definitions.push(buildColumnDefinition(driver, k, prop)); } - for (i = 0; i < opts.one_associations.length; i++) { - if (opts.one_associations[i].extension) continue; - if (opts.one_associations[i].reversed) continue; - for (k in opts.one_associations[i].field) { - definitions.push(buildColumnDefinition(driver, k, opts.one_associations[i].field[k])); + for (k in opts.allProperties) { + prop = opts.allProperties[k]; + if (prop.unique === true) { + definitions.push("UNIQUE (" + driver.query.escapeId(k) + ")"); + } else if (prop.index) { + definitions.push("INDEX (" + driver.query.escapeId(k) + ")"); } } - for (k in opts.properties) { - if (opts.properties[k].unique === true) { + for (k in opts.allProperties) { + prop = opts.allProperties[k]; + if (prop.unique === true) { definitions.push("UNIQUE KEY " + driver.query.escapeId(k) + " (" + driver.query.escapeId(k) + ")"); - } else if (opts.properties[k].index) { + } else if (prop.index) { definitions.push("INDEX (" + driver.query.escapeId(k) + ")"); } } @@ -142,6 +132,9 @@ function buildColumnDefinition(driver, name, prop) { def = driver.query.escapeId(name) + " VARCHAR(" + Math.min(Math.max(parseInt(prop.size, 10) || 255, 1), 65535) + ")"; } break; + case "serial": + def = driver.query.escapeId(name) + " INT(10) UNSIGNED NOT NULL AUTO_INCREMENT"; + break; case "number": if (prop.rational === false) { def = driver.query.escapeId(name) + " " + colTypes.integer[prop.size || 4]; diff --git a/lib/Drivers/DDL/postgres.js b/lib/Drivers/DDL/postgres.js index 4e0e38f5..f45eb062 100755 --- a/lib/Drivers/DDL/postgres.js +++ b/lib/Drivers/DDL/postgres.js @@ -24,46 +24,27 @@ exports.sync = function (driver, opts, cb) { var subqueries = []; var typequeries = []; var definitions = []; - var k, i, pending; + var k, i, pending, prop; var primary_keys = opts.id.map(function (k) { return driver.query.escapeId(k); }); var keys = []; - for (i = 0; i < opts.id.length; i++) { - if (opts.properties.hasOwnProperty(opts.id[i])) continue; - keys.push(driver.query.escapeId(opts.id[i])); - } - - for (i = 0; i < keys.length; i++) { - definitions.push(keys[i] + " INTEGER NOT NULL"); - } - - if (opts.id.length == 1 && !opts.extension) { - definitions[definitions.length - 1] = keys[0] + " SERIAL"; - } + for (k in opts.allProperties) { + prop = opts.allProperties[k]; + definitions.push(buildColumnDefinition(driver, opts.table, k, prop)); - for (k in opts.properties) { - definitions.push(buildColumnDefinition(driver, opts.table, k, opts.properties[k])); - - if (opts.properties[k].type == "enum") { + if (prop.type == "enum") { typequeries.push( "CREATE TYPE " + driver.query.escapeId("enum_" + opts.table + "_" + k) + " AS ENUM (" + - opts.properties[k].values.map(driver.query.escapeVal.bind(driver)) + ")" + prop.values.map(driver.query.escapeVal.bind(driver)) + ")" ); } } - for (i = 0; i < opts.one_associations.length; i++) { - if (opts.one_associations[i].extension) continue; - if (opts.one_associations[i].reversed) continue; - for (k in opts.one_associations[i].field) { - definitions.push(buildColumnDefinition(driver, opts.table, k, opts.one_associations[i].field[k])); - } - } - - for (k in opts.properties) { - if (opts.properties[k].unique === true) { + for (k in opts.allProperties) { + prop = opts.allProperties[k]; + if (prop.unique === true) { definitions.push("UNIQUE (" + driver.query.escapeId(k) + ")"); - } else if (opts.properties[k].index) { + } else if (prop.index) { definitions.push("INDEX (" + driver.query.escapeId(k) + ")"); } } @@ -77,7 +58,7 @@ exports.sync = function (driver, opts, cb) { typequeries: typequeries, subqueries : subqueries }); - + for (i = 0; i < opts.one_associations.length; i++) { if (opts.one_associations[i].extension) continue; if (opts.one_associations[i].reversed) continue; @@ -101,7 +82,7 @@ exports.sync = function (driver, opts, cb) { for (i = 0; i < opts.many_associations.length; i++) { definitions = []; typequeries = []; - + for (k in opts.many_associations[i].mergeId) { definitions.push(buildColumnDefinition(driver, opts.many_associations[i].mergeTable, k, opts.many_associations[i].mergeId[k])); } @@ -120,7 +101,7 @@ exports.sync = function (driver, opts, cb) { ); } } - + var index = null; for (k in opts.many_associations[i].mergeId) { if (index == null) index = driver.query.escapeId(k); @@ -207,6 +188,9 @@ function buildColumnDefinition(driver, table, name, prop) { def = driver.query.escapeId(name) + " VARCHAR(" + Math.max(parseInt(prop.size, 10) || 255, 1) + ")"; } break; + case "serial": + def = driver.query.escapeId(name) + " SERIAL"; + break; case "number": if (prop.rational === false) { def = driver.query.escapeId(name) + " " + colTypes.integer[prop.size || 4]; diff --git a/lib/Drivers/DDL/sqlite.js b/lib/Drivers/DDL/sqlite.js index d5bca17d..13996173 100644 --- a/lib/Drivers/DDL/sqlite.js +++ b/lib/Drivers/DDL/sqlite.js @@ -22,34 +22,13 @@ exports.drop = function (driver, opts, cb) { exports.sync = function (driver, opts, cb) { var queries = []; var definitions = []; - var k, i, pending; + var k, i, pending, prop; var primary_keys = opts.id.map(function (k) { return driver.query.escapeId(k); }); var keys = []; - for (i = 0; i < opts.id.length; i++) { - if (opts.properties.hasOwnProperty(opts.id[i])) continue; - - keys.push(driver.query.escapeId(opts.id[i])); - } - - for (i = 0; i < keys.length; i++) { - definitions.push(keys[i] + " INTEGER UNSIGNED NOT NULL"); - } - - if (opts.id.length == 1 && !opts.extension) { - definitions[definitions.length - 1] = keys[0] + " INTEGER PRIMARY KEY AUTOINCREMENT"; - } - - for (k in opts.properties) { - definitions.push(buildColumnDefinition(driver, k, opts.properties[k])); - } - - for (i = 0; i < opts.one_associations.length; i++) { - if (opts.one_associations[i].extension) continue; - if (opts.one_associations[i].reversed) continue; - for (k in opts.one_associations[i].field) { - definitions.push(buildColumnDefinition(driver, k, opts.one_associations[i].field[k])); - } + for (k in opts.allProperties) { + prop = opts.allProperties[k]; + definitions.push(buildColumnDefinition(driver, k, prop)); } if (keys.length > 1) { @@ -75,7 +54,7 @@ exports.sync = function (driver, opts, cb) { ); } } - + for (i = 0; i < opts.one_associations.length; i++) { if (opts.one_associations[i].extension) continue; if (opts.one_associations[i].reversed) continue; @@ -112,7 +91,7 @@ exports.sync = function (driver, opts, cb) { for (k in opts.many_associations[i].props) { definitions.push(buildColumnDefinition(driver, k, opts.many_associations[i].props[k])); } - + var index = null; for (k in opts.many_associations[i].mergeId) { if (index == null) index = driver.query.escapeId(k); @@ -153,6 +132,9 @@ function buildColumnDefinition(driver, name, prop) { case "text": def = driver.query.escapeId(name) + " TEXT"; break; + case "serial": + def = driver.query.escapeId(name) + " INTEGER PRIMARY KEY AUTOINCREMENT"; + break; case "number": if (prop.rational === false) { def = driver.query.escapeId(name) + " INTEGER"; diff --git a/lib/Drivers/DML/postgres.js b/lib/Drivers/DML/postgres.js index 834dcd6a..3661ca8f 100644 --- a/lib/Drivers/DML/postgres.js +++ b/lib/Drivers/DML/postgres.js @@ -238,6 +238,8 @@ Driver.prototype.clear = function (table, cb) { }; Driver.prototype.valueToProperty = function (value, property) { + var v; + switch (property.type) { case "object": if (typeof value == "object" && !Buffer.isBuffer(value)) { @@ -256,14 +258,17 @@ Driver.prototype.valueToProperty = function (value, property) { return { x : parseFloat(m[1], 10) , y : parseFloat(m[2], 10) }; } } - return value; + break; case "number": - if (value !== null) { - return Number(value); + if (typeof value != 'number' && value !== null) { + v = Number(value); + if (!isNaN(v)) { + return v; + } } - default: - return value; + break; } + return value; }; Driver.prototype.propertyToValue = function (value, property) { diff --git a/lib/Instance.js b/lib/Instance.js index c489d923..a1a3a84b 100755 --- a/lib/Instance.js +++ b/lib/Instance.js @@ -33,12 +33,6 @@ function Instance(Model, opts) { return saveError(cb, err); } - for (k in Model.properties) { - if (!Model.properties.hasOwnProperty(k)) continue; - if (opts.data[k] === null && Model.properties[k].hasOwnProperty("defaultValue")) { - opts.data[k] = Model.properties[k].defaultValue; - } - } for (i = 0; i < opts.one_associations.length; i++) { for (k in opts.one_associations[i].field) { if (opts.one_associations[i].required && opts.data[k] === null) { @@ -145,19 +139,23 @@ function Instance(Model, opts) { } }; var getInstanceData = function () { - var data = {}; + var data = {}, prop; for (var k in opts.data) { if (!opts.data.hasOwnProperty(k)) continue; + prop = Model.allProperties[k]; + + if (prop) { + if (prop.type === 'serial' && opts.data[k] == null) continue; - if (Model.properties[k]) { - data[k] = Property.validate(opts.data[k], Model.properties[k]); + data[k] = Property.validate(opts.data[k], prop); if (opts.driver.propertyToValue) { - data[k] = opts.driver.propertyToValue(data[k], Model.properties[k]); + data[k] = opts.driver.propertyToValue(data[k], prop); } } else { data[k] = opts.data[k]; } } + return data; }; var waitHooks = function (hooks, next) { @@ -387,20 +385,50 @@ function Instance(Model, opts) { }); }); }; + var setInstanceProperty = function (key, value) { + var prop = Model.allProperties[key] || opts.extra[key]; + + if (prop) { + if ('valueToProperty' in opts.driver) { + value = opts.driver.valueToProperty(value, prop); + } + if (opts.data[key] !== value) { + opts.data[key] = value; + return true; + } + } + return false; + } + var addInstanceProperty = function (key) { - // if (instance.hasOwnProperty(key)) return; + var defaultValue = null; + var prop = Model.allProperties[key]; + + // This code was first added, and then commented out in a later commit. + // Its presence doesn't affect tests, so I'm just gonna log if it ever gets called. + // If someone complains about noise, we know it does something, and figure it out then. + if (instance.hasOwnProperty(key)) console.log("Overwriting instance property"); + + if (key in opts.data) { + defaultValue = opts.data[key]; + } else if (prop && 'defaultValue' in prop) { + defaultValue = prop.defaultValue; + } + + setInstanceProperty(key, defaultValue); Object.defineProperty(instance, key, { get: function () { return opts.data[key]; }, set: function (val) { - if (opts.id.indexOf(key) >= 0 && opts.data.hasOwnProperty(key)) { - return; + if (Model.allProperties[key].key === true && opts.data[key] != null) { + return; } - if (opts.data[key] === val) return; - opts.data[key] = val; + if (!setInstanceProperty(key, val)) { + return; + } if (opts.autoSave) { saveInstanceProperty(key, val); @@ -420,7 +448,7 @@ function Instance(Model, opts) { return opts.data[key]; }, set: function (val) { - opts.data[key] = val; + setInstanceProperty(key, val); /*if (opts.autoSave) { saveInstanceProperty(key, val); @@ -433,34 +461,11 @@ function Instance(Model, opts) { }; var i, k; - for (i = 0; i < opts.id.length; i++) { - if (!opts.data.hasOwnProperty(opts.id[i])) { - addInstanceProperty(opts.id[i]); - } - } - for (k in Model.properties) { - if (Model.properties.hasOwnProperty(k) && !opts.data.hasOwnProperty(k) && opts.id.indexOf(k) === -1) { - opts.data[k] = null; - } + for (k in Model.allProperties) { + addInstanceProperty(k); } - - for (k in opts.data) { - if (!opts.data.hasOwnProperty(k)) continue; - if (!Model.properties.hasOwnProperty(k) && opts.id.indexOf(k) === -1 && opts.association_properties.indexOf(k) === -1) { - if (!opts.extra.hasOwnProperty(k)) continue; - - if (opts.driver.valueToProperty) { - opts.data[k] = opts.driver.valueToProperty(opts.data[k], opts.extra[k]); - } - addInstanceExtraProperty(k); - continue; - } - - if (Model.properties[k] && opts.driver.valueToProperty) { - opts.data[k] = opts.driver.valueToProperty(opts.data[k], Model.properties[k]); - } - + for (k in opts.extra) { addInstanceProperty(k); } @@ -471,6 +476,10 @@ function Instance(Model, opts) { }); } + for (k in opts.extra) { + addInstanceExtraProperty(k); + } + Object.defineProperty(instance, "on", { value: function (event, cb) { if (!events.hasOwnProperty(event)) { diff --git a/lib/Model.js b/lib/Model.js index 8250b6a6..3d1851fb 100644 --- a/lib/Model.js +++ b/lib/Model.js @@ -33,10 +33,7 @@ function Model(opts) { var extend_associations = []; var association_properties = []; var model_fields = []; - - for (var i = 0; i < opts.id.length; i++) { - model_fields.push(opts.id[i]); - } + var allProperties = {}; var createHookHelper = function (hook) { return function (cb) { @@ -184,35 +181,9 @@ function Model(opts) { }); }; - // Standardize validations - for (var k in opts.validations) { - if (!Array.isArray(opts.validations[k])) { - opts.validations[k] = [ opts.validations[k] ]; - } - } - - for (k in opts.properties) { - opts.properties[k] = Property.normalize(opts.properties[k], opts.settings); - - if (opts.properties[k].lazyload !== true && model_fields.indexOf(k) == -1) { - model_fields.push(k); - } - if (opts.properties[k].required) { - // Prepend `required` validation - if(opts.validations.hasOwnProperty(k)) { - opts.validations[k].splice(0, 0, Validators.required()); - } else { - opts.validations[k] = [Validators.required()]; - } - } - } - - for (k in AvailableHooks) { - model[AvailableHooks[k]] = createHookHelper(AvailableHooks[k]); - } - - model.properties = opts.properties; - model.settings = opts.settings; + model.allProperties = allProperties; + model.properties = opts.properties; + model.settings = opts.settings; model.drop = function (cb) { if (arguments.length === 0) { @@ -243,6 +214,7 @@ function Model(opts) { id : opts.id, table : opts.table, properties : opts.properties, + allProperties : allProperties, indexes : opts.indexes || [], one_associations : one_associations, many_associations : many_associations, @@ -639,6 +611,49 @@ function Model(opts) { enumerable: false }); + // Standardize validations + for (var k in opts.validations) { + if (!Array.isArray(opts.validations[k])) { + opts.validations[k] = [ opts.validations[k] ]; + } + } + + // standardize properties + for (k in opts.properties) { + opts.properties[k] = Property.normalize(opts.properties[k], opts.settings); + opts.properties[k].klass = 'primary'; + allProperties[k] = opts.properties[k]; + + if (opts.id.indexOf(k) != -1) { + opts.properties[k].key = true; + } + + if (opts.properties[k].lazyload !== true && model_fields.indexOf(k) == -1) { + model_fields.push(k); + } + if (opts.properties[k].required) { + // Prepend `required` validation + if(opts.validations.hasOwnProperty(k)) { + opts.validations[k].splice(0, 0, Validators.required()); + } else { + opts.validations[k] = [Validators.required()]; + } + } + } + + for (var i = 0; i < opts.id.length; i++) { + k = opts.id[i]; + allProperties[k] = opts.properties[k] || { + type: 'serial', rational: 'false', key: true, klass: 'key' + }; + } + model_fields = opts.id.concat(model_fields); + + // setup hooks + for (k in AvailableHooks) { + model[AvailableHooks[k]] = createHookHelper(AvailableHooks[k]); + } + OneAssociation.prepare(model, one_associations, association_properties, model_fields); ManyAssociation.prepare(model, many_associations); ExtendAssociation.prepare(opts.db, model, extend_associations); diff --git a/test/integration/model-get.js b/test/integration/model-get.js index 707551af..e0fca99e 100644 --- a/test/integration/model-get.js +++ b/test/integration/model-get.js @@ -1,5 +1,6 @@ var should = require('should'); var helper = require('../support/spec_helper'); +var common = require('../common'); var ORM = require('../../'); describe("Model.get()", function() { @@ -282,6 +283,8 @@ describe("Model.get()", function() { }); describe("with a point property type", function() { + if (common.protocol() == 'sqlite' || common.protocol() == 'mongodb') return; + it("should deserialize the point to an array", function (done) { db.settings.set('properties.primary_key', 'id'); @@ -292,11 +295,7 @@ describe("Model.get()", function() { ORM.singleton.clear(); - return helper.dropSync(Person, function (err) { - if (err) { - return done(); // not supported - } - + return helper.dropSync(Person, function () { Person.create({ name : "John Doe", location : { x : 51.5177, y : -0.0968 } diff --git a/test/integration/model-keys.js b/test/integration/model-keys.js index b958ed24..cd18dbb7 100644 --- a/test/integration/model-keys.js +++ b/test/integration/model-keys.js @@ -53,6 +53,9 @@ describe("Model keys option", function() { before(function (done) { DoorAccessHistory = db.define("door_access_history", { + year : { type: 'number', rational: false }, + month : { type: 'number', rational: false }, + day : { type: 'number', rational: false }, user : String, action : [ "in", "out" ] }, { diff --git a/test/integration/model-save.js b/test/integration/model-save.js index 54d98b9e..2d17abcd 100644 --- a/test/integration/model-save.js +++ b/test/integration/model-save.js @@ -1,5 +1,6 @@ var should = require('should'); var helper = require('../support/spec_helper'); +var common = require('../common'); var ORM = require('../../'); describe("Model.save()", function() { @@ -198,12 +199,10 @@ describe("Model.save()", function() { }); describe("with a point property", function () { - it("should save the instance as a geospatial point", function (done) { - setup({ type: "point" }, null)(function (err) { - if (err) { - return done(); // not supported - } + if (common.protocol() == 'sqlite' || common.protocol() == 'mongodb') return; + it("should save the instance as a geospatial point", function (done) { + setup({ type: "point" }, null)(function () { var John = new Person({ name: { x: 51.5177, y: -0.0968 } }); diff --git a/test/support/spec_helper.js b/test/support/spec_helper.js index 927d02f8..ea83eedf 100644 --- a/test/support/spec_helper.js +++ b/test/support/spec_helper.js @@ -1,5 +1,6 @@ var common = require('../common'); var async = require('async'); +var should = require('should'); module.exports.connect = function(cb) { common.createConnection(function (err, conn) { @@ -19,5 +20,8 @@ module.exports.dropSync = function (models, done) { item.sync(cb); }); - }, done); + }, function (err) { + should.not.exist(err); + done(err); + }); }; From 5f5bc617b9617ae3cbe857f09382b5c4c9a05f71 Mon Sep 17 00:00:00 2001 From: Arek W Date: Wed, 28 Aug 2013 20:03:30 +1000 Subject: [PATCH 2/4] Infinity & NaN support. Number fixes for sqlite. Should fix #310 --- lib/Drivers/DML/postgres.js | 12 ++-- lib/Drivers/DML/sqlite.js | 19 ++++-- test/integration/instance.js | 116 +++++++++++++++++++++++++++++++++-- 3 files changed, 132 insertions(+), 15 deletions(-) diff --git a/lib/Drivers/DML/postgres.js b/lib/Drivers/DML/postgres.js index 3661ca8f..a59ca9eb 100644 --- a/lib/Drivers/DML/postgres.js +++ b/lib/Drivers/DML/postgres.js @@ -243,28 +243,26 @@ Driver.prototype.valueToProperty = function (value, property) { switch (property.type) { case "object": if (typeof value == "object" && !Buffer.isBuffer(value)) { - return value; + break; } try { - return JSON.parse(value); + value = JSON.parse(value); } catch (e) { - return null; + value = null; } break; case "point": if (typeof value == "string") { var m = value.match(/\((\-?[\d\.]+)[\s,]+(\-?[\d\.]+)\)/); if (m) { - return { x : parseFloat(m[1], 10) , y : parseFloat(m[2], 10) }; + value = { x : parseFloat(m[1], 10) , y : parseFloat(m[2], 10) }; } } break; case "number": if (typeof value != 'number' && value !== null) { v = Number(value); - if (!isNaN(v)) { - return v; - } + if (!isNaN(v)) value = v; } break; } diff --git a/lib/Drivers/DML/sqlite.js b/lib/Drivers/DML/sqlite.js index baf1129c..5a4402be 100644 --- a/lib/Drivers/DML/sqlite.js +++ b/lib/Drivers/DML/sqlite.js @@ -205,9 +205,12 @@ Driver.prototype.clear = function (table, cb) { }; Driver.prototype.valueToProperty = function (value, property) { + var v; + switch (property.type) { case "boolean": - return !!value; + value = !!value; + break; case "object": if (typeof value == "object" && !Buffer.isBuffer(value)) { return value; @@ -218,16 +221,24 @@ Driver.prototype.valueToProperty = function (value, property) { return null; } break; + case "number": + if (typeof value != 'number' && value !== null) { + v = Number(value); + if (!isNaN(v)) { + return v; + } + } + break; case "date": if (typeof value === 'string') { if (value.indexOf('Z', value.length - 1) === -1) { return new Date(value + 'Z'); } } - return new Date(value); - default: - return value; + value = new Date(value); + break; } + return value; }; Driver.prototype.propertyToValue = function (value, property) { diff --git a/test/integration/instance.js b/test/integration/instance.js index 57b6f918..e89acae5 100644 --- a/test/integration/instance.js +++ b/test/integration/instance.js @@ -1,19 +1,24 @@ -var should = require('should'); -var helper = require('../support/spec_helper'); +var should = require('should'); +var helper = require('../support/spec_helper'); +var common = require('../common'); var ORM = require('../../'); describe("Model instance", function() { var db = null; var Person = null; + var protocol = common.protocol(); var setup = function () { return function (done) { db.settings.set('instance.returnAllErrors', true); Person = db.define("person", { - name : String, - age : { type: 'number', rational: false, required: false } + name : String, + age : { type: 'number', rational: false, required: false }, + height : { type: 'number', rational: false, required: false }, + weight : { type: 'number', required: false } }, { + cache: false, validations: { age: ORM.validators.rangeNumber(0, 150) } @@ -122,4 +127,107 @@ describe("Model instance", function() { }); }); }); + + describe("properties", function () { + describe("Number", function () { + it("should be saved for valid numbers", function (done) { + var person1 = new Person({ height: 190 }); + + person1.save(function (err) { + should.not.exist(err); + + Person.create({ height: 170 }, function (err, person2) { + should.not.exist(err); + + Person.get(person1[Person.id], function (err, item) { + should.not.exist(err); + should.equal(item.height, 190); + + Person.get(person2[Person.id], function (err, item) { + should.not.exist(err); + should.equal(item.height, 170); + done(); + }); + }); + }); + }); + }); + + if (protocol == 'postgres') { + // Only postgres raises propper errors. + // Sqlite & Mysql fail silently and insert nulls. + it("should raise an error for NaN integers", function (done) { + var person = new Person({ height: NaN }); + + person.save(function (err) { + should.exist(err); + var msg = { + postgres : 'invalid input syntax for integer: "NaN"' + }[protocol]; + + should.equal(err.message, msg); + + done(); + }); + }); + + it("should raise an error for Infinity integers", function (done) { + var person = new Person({ height: Infinity }); + + person.save(function (err) { + should.exist(err); + var msg = { + postgres : 'invalid input syntax for integer: "Infinity"' + }[protocol]; + + should.equal(err.message, msg); + + done(); + }); + }); + + it("should raise an error for nonsensical integers", function (done) { + var person = new Person({ height: 'bugz' }); + + person.save(function (err) { + should.exist(err); + var msg = { + postgres : 'invalid input syntax for integer: "bugz"' + }[protocol]; + + should.equal(err.message, msg); + + done(); + }); + }); + } + + if (protocol != 'mysql') { + // Mysql doesn't support IEEE floats (NaN, Infinity, -Infinity) + it("should store NaN & Infinite floats", function (done) { + var person = new Person({ weight: NaN }); + + person.save(function (err) { + should.not.exist(err); + + Person.get(person[Person.id], function (err, person) { + should.not.exist(err); + should(isNaN(person.weight)); + + person.save({ weight: Infinity, name: 'black hole' }, function (err) { + should.not.exist(err); + + Person.get(person[Person.id], function (err, person) { + should.not.exist(err); + should.strictEqual(person.weight, Infinity); + + done(); + }); + }); + }); + }); + }); + } + }); + }); }); From 70e280f19216fcc55fcf6d6ebe20ff187d643821 Mon Sep 17 00:00:00 2001 From: Arek W Date: Wed, 28 Aug 2013 20:32:09 +1000 Subject: [PATCH 3/4] Fix wrong code & bump sql-query version --- lib/Associations/One.js | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Associations/One.js b/lib/Associations/One.js index 490b6ed9..cbe45e42 100644 --- a/lib/Associations/One.js +++ b/lib/Associations/One.js @@ -51,7 +51,7 @@ exports.prepare = function (Model, associations, association_properties, model_f for (k in association.field) { association_properties.push(k); if (!association.reversed) { - Model.allProperties[k] = _.omit(association.field[k], k); + Model.allProperties[k] = _.omit(association.field[k], 'klass'); Model.allProperties[k].klass = 'hasOne'; model_fields.push(k); } diff --git a/package.json b/package.json index c6dbe45f..c13e9d7f 100644 --- a/package.json +++ b/package.json @@ -36,7 +36,7 @@ "analyse" : false, "dependencies": { "enforce" : "0.1.2", - "sql-query" : "0.1.11", + "sql-query" : "0.1.12", "hat" : "0.0.3", "lodash" : "1.3.1" }, From a23d206cc4c954cba79885ce617b1686726daa36 Mon Sep 17 00:00:00 2001 From: Arek W Date: Wed, 28 Aug 2013 20:48:47 +1000 Subject: [PATCH 4/4] Explain & add to property tests --- test/integration/instance.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/test/integration/instance.js b/test/integration/instance.js index e89acae5..71fbdc11 100644 --- a/test/integration/instance.js +++ b/test/integration/instance.js @@ -130,7 +130,7 @@ describe("Model instance", function() { describe("properties", function () { describe("Number", function () { - it("should be saved for valid numbers", function (done) { + it("should be saved for valid numbers, using both save & create", function (done) { var person1 = new Person({ height: 190 }); person1.save(function (err) { @@ -186,7 +186,7 @@ describe("Model instance", function() { }); }); - it("should raise an error for nonsensical integers", function (done) { + it("should raise an error for nonsensical integers, for both save & create", function (done) { var person = new Person({ height: 'bugz' }); person.save(function (err) { @@ -197,7 +197,12 @@ describe("Model instance", function() { should.equal(err.message, msg); - done(); + Person.create({ height: 'bugz' }, function (err, instance) { + should.exist(err); + should.equal(err.message, msg); + + done(); + }); }); }); }