diff --git a/lib/discovery.js b/lib/discovery.js index 087cbab..2ff4696 100644 --- a/lib/discovery.js +++ b/lib/discovery.js @@ -141,18 +141,28 @@ function mixinDiscovery(MySQL) { function queryColumns(owner, table) { var sql = null; if (owner) { - sql = paginateSQL('SELECT table_schema AS "owner", table_name AS "tableName", column_name AS "columnName", data_type AS "dataType",' - + ' character_octet_length AS "dataLength", numeric_precision AS "dataPrecision", numeric_scale AS "dataScale", is_nullable AS "nullable"' - + ' FROM information_schema.columns' - + ' WHERE table_schema=\'' + owner + '\'' - + (table ? ' AND table_name=\'' + table + '\'' : ''), - 'table_name, ordinal_position', {}); + sql = paginateSQL('SELECT table_schema AS "owner",' + + ' table_name AS "tableName", column_name AS "columnName",' + + ' data_type AS "dataType",' + + ' character_octet_length AS "dataLength",' + + ' numeric_precision AS "dataPrecision",' + + ' numeric_scale AS "dataScale",' + + ' is_nullable = \'YES\' AS "nullable"' + + ' FROM information_schema.columns' + + ' WHERE table_schema=\'' + owner + '\'' + + (table ? ' AND table_name=\'' + table + '\'' : ''), + 'table_name, ordinal_position', {}); } else { - sql = paginateSQL('SELECT table_schema AS "owner", table_name AS "tableName", column_name AS "columnName", data_type AS "dataType",' - + ' character_octet_length AS "dataLength", numeric_precision AS "dataPrecision", numeric_scale AS "dataScale", is_nullable AS "nullable"' - + ' FROM information_schema.columns' - + (table ? ' WHERE table_name=\'' + table + '\'' : ''), - 'table_name, ordinal_position', {}); + sql = paginateSQL('SELECT table_schema AS "owner",' + + ' table_name AS "tableName", column_name AS "columnName",' + + ' data_type AS "dataType",' + + ' character_octet_length AS "dataLength",' + + ' numeric_precision AS "dataPrecision",' + + ' numeric_scale AS "dataScale",' + + ' is_nullable = \'YES\' AS "nullable"' + + ' FROM information_schema.columns' + + (table ? ' WHERE table_name=\'' + table + '\'' : ''), + 'table_name, ordinal_position', {}); } return sql; } @@ -178,6 +188,7 @@ function mixinDiscovery(MySQL) { } else { results.map(function (r) { r.type = mysqlDataTypeToJSONType(r.dataType, r.dataLength); + r.nullable = r.nullable ? 'Y' : 'N'; }); cb(err, results); } diff --git a/lib/mysql.js b/lib/mysql.js index 1015465..3390a56 100644 --- a/lib/mysql.js +++ b/lib/mysql.js @@ -716,6 +716,20 @@ MySQL.prototype.isActual = function(cb) { }); }; +// Check if a property is nullable +function isNullable(p) { + if (p.required || p.id) { + return false; + } + if (p.nullable || p['null'] || p.allowNull) { + return true; + } + if (p.nullable === false || p['null'] === false || p.allowNull === false) { + return false; + } + return true; +} + MySQL.prototype.alterTable = function (model, actualFields, actualIndexes, done, checkOnly) { var self = this; var m = this._models[model]; @@ -856,7 +870,8 @@ MySQL.prototype.alterTable = function (model, actualFields, actualIndexes, done, function actualize(propName, oldSettings) { var newSettings = m.properties[propName]; if (newSettings && changed(newSettings, oldSettings)) { - sql.push('CHANGE COLUMN `' + propName + '` `' + propName + '` ' + self.propertySettingsSQL(model, propName)); + sql.push('CHANGE COLUMN `' + propName + '` `' + propName + '` ' + + self.propertySettingsSQL(model, propName)); } } @@ -864,14 +879,21 @@ MySQL.prototype.alterTable = function (model, actualFields, actualIndexes, done, if (oldSettings.Null === 'YES') { // Used to allow null and does not now. if (newSettings.allowNull === false) return true; if (newSettings.null === false) return true; + if (newSettings.nullable === false) return true; + if (newSettings.required || newSettings.id) return true; } if (oldSettings.Null === 'NO') { // Did not allow null and now does. if (newSettings.allowNull === true) return true; if (newSettings.null === true) return true; - if (newSettings.null === undefined && newSettings.allowNull === undefined) return true; + if (newSettings.nullable === true) return true; + if (newSettings.null === undefined && + newSettings.allowNull === undefined && + !newSettings.required && + !newSettings.id) return true; } - if (oldSettings.Type.toUpperCase() !== datatype(newSettings).toUpperCase()) return true; + if (oldSettings.Type.toUpperCase() !== datatype(newSettings).toUpperCase()) + return true; return false; } }; @@ -971,7 +993,9 @@ MySQL.prototype.indexSettingsSQL = function (model, prop) { MySQL.prototype.propertySettingsSQL = function (model, prop) { var p = this._models[model].properties[prop]; var line = this.columnDataType(model, prop) + ' ' + - (p.nullable === false || p.allowNull === false || p['null'] === false ? 'NOT NULL' : 'NULL'); + (p.required || p.id || p.nullable === false || + p.allowNull === false || p['null'] === false ? + 'NOT NULL' : 'NULL'); return line; }; diff --git a/test/migration.test.js b/test/migration.test.js index ce35c69..5b23430 100644 --- a/test/migration.test.js +++ b/test/migration.test.js @@ -72,7 +72,8 @@ describe('migrations', function () { }); it('UserData should have correct indexes', function (done) { - // Note: getIndexes truncates multi-key indexes to the first member. Hence index1 is correct. + // Note: getIndexes truncates multi-key indexes to the first member. + // Hence index1 is correct. getIndexes('UserData', function (err, fields) { assert.deepEqual(fields, { PRIMARY: { Table: 'UserData', Non_unique: 0, @@ -182,7 +183,7 @@ describe('migrations', function () { Extra: '' }, mediumInt: { Field: 'mediumInt', Type: 'mediumint(8) unsigned', - Null: 'YES', + Null: 'NO', Key: '', Default: null, Extra: '' }, @@ -236,9 +237,12 @@ describe('migrations', function () { assert.ok(yep, 'User does not exist'); }); UserData.defineProperty('email', { type: String }); - UserData.defineProperty('name', {type: String, dataType: 'char', limit: 50}); - UserData.defineProperty('newProperty', {type: Number, unsigned: true, dataType: 'bigInt'}); - // UserData.defineProperty('pendingPeriod', false); This will not work as expected. + UserData.defineProperty('name', {type: String, + dataType: 'char', limit: 50}); + UserData.defineProperty('newProperty', {type: Number, unsigned: true, + dataType: 'bigInt'}); + // UserData.defineProperty('pendingPeriod', false); + // This will not work as expected. db.autoupdate(function (err) { getFields('UserData', function (err, fields) { // change nullable for email @@ -248,10 +252,12 @@ describe('migrations', function () { // add new column assert.ok(fields.newProperty, 'New column was not added'); if (fields.newProperty) { - assert.equal(fields.newProperty.Type, 'bigint(20) unsigned', 'New column type is not bigint(20) unsigned'); + assert.equal(fields.newProperty.Type, 'bigint(20) unsigned', + 'New column type is not bigint(20) unsigned'); } // drop column - will not happen. - // assert.ok(!fields.pendingPeriod, 'Did not drop column pendingPeriod'); + // assert.ok(!fields.pendingPeriod, + // 'Did not drop column pendingPeriod'); // user still exists userExists(function (yep) { assert.ok(yep, 'User does not exist'); @@ -276,7 +282,8 @@ describe('migrations', function () { }); it('should allow numbers with decimals', function (done) { - NumberData.create({number: 1.1234567, tinyInt: 123456, mediumInt: -1234567, floater: 123456789.1234567 }, function (err, obj) { + NumberData.create({number: 1.1234567, tinyInt: 123456, mediumInt: -1234567, + floater: 123456789.1234567 }, function (err, obj) { assert.ok(!err); assert.ok(obj); NumberData.findById(obj.id, function (err, found) { @@ -297,8 +304,10 @@ describe('migrations', function () { assert.ok(!err); assert.ok(obj); DateData.findById(obj.id, function (err, found) { - assert.equal(found.dateTime.toGMTString(), 'Fri, 09 Aug 1996 07:47:33 GMT'); - assert.equal(found.timestamp.toGMTString(), 'Sat, 22 Sep 2007 17:12:22 GMT'); + assert.equal(found.dateTime.toGMTString(), + 'Fri, 09 Aug 1996 07:47:33 GMT'); + assert.equal(found.timestamp.toGMTString(), + 'Sat, 22 Sep 2007 17:12:22 GMT'); done(); }); }); @@ -345,7 +354,8 @@ function setup(done) { StringData = db.define('StringData', { idString: {type: String, id: true}, - smallString: {type: String, null: false, index: true, dataType: 'char', limit: 127}, + smallString: {type: String, null: false, index: true, + dataType: 'char', limit: 127}, mediumString: {type: String, null: false, dataType: 'varchar', limit: 255}, tinyText: {type: String, dataType: 'tinyText'}, giantJSON: {type: Schema.JSON, dataType: 'longText'}, @@ -353,9 +363,11 @@ function setup(done) { }); NumberData = db.define('NumberData', { - number: {type: Number, null: false, index: true, unsigned: true, dataType: 'decimal', precision: 10, scale: 3}, + number: {type: Number, null: false, index: true, unsigned: true, + dataType: 'decimal', precision: 10, scale: 3}, tinyInt: {type: Number, dataType: 'tinyInt', display: 2}, - mediumInt: {type: Number, dataType: 'mediumInt', unsigned: true}, + mediumInt: {type: Number, dataType: 'mediumInt', unsigned: true, + required: true}, floater: {type: Number, dataType: 'double', precision: 14, scale: 6} }); diff --git a/test/mysql.autoupdate.test.js b/test/mysql.autoupdate.test.js new file mode 100644 index 0000000..eb1f89d --- /dev/null +++ b/test/mysql.autoupdate.test.js @@ -0,0 +1,137 @@ +var assert = require('assert'); +require('./init'); +var ds; + +before(function () { + ds = getDataSource(); +}); + +describe('MySQL connector', function () { + it('should auto migrate/update tables', function (done) { + + var schema_v1 = + { + "name": "CustomerTest", + "options": { + "idInjection": false, + "mysql": { + "schema": "myapp_test", + "table": "customer_test" + } + }, + "properties": { + "id": { + "type": "String", + "length": 20, + "id": 1 + }, + "name": { + "type": "String", + "required": false, + "length": 40 + }, + "email": { + "type": "String", + "required": true, + "length": 40 + }, + "age": { + "type": "Number", + "required": false + } + } + } + + var schema_v2 = + { + "name": "CustomerTest", + "options": { + "idInjection": false, + "mysql": { + "schema": "myapp_test", + "table": "customer_test" + } + }, + "properties": { + "id": { + "type": "String", + "length": 20, + "id": 1 + }, + "email": { + "type": "String", + "required": false, + "length": 60, + "mysql": { + "columnName": "email", + "dataType": "varchar", + "dataLength": 60, + "nullable": "YES" + } + }, + "firstName": { + "type": "String", + "required": false, + "length": 40 + }, + "lastName": { + "type": "String", + "required": false, + "length": 40 + } + } + } + + ds.createModel(schema_v1.name, schema_v1.properties, schema_v1.options); + + ds.automigrate(function () { + + ds.discoverModelProperties('customer_test', function (err, props) { + assert.equal(props.length, 4); + var names = props.map(function (p) { + return p.columnName; + }); + assert.equal(props[0].nullable, 'N'); + assert.equal(props[1].nullable, 'Y'); + assert.equal(props[2].nullable, 'N'); + assert.equal(props[3].nullable, 'Y'); + assert.equal(names[0], 'id'); + assert.equal(names[1], 'name'); + assert.equal(names[2], 'email'); + assert.equal(names[3], 'age'); + + ds.createModel(schema_v2.name, schema_v2.properties, schema_v2.options); + + ds.autoupdate(function (err, result) { + ds.discoverModelProperties('customer_test', function (err, props) { + assert.equal(props.length, 4); + var names = props.map(function (p) { + return p.columnName; + }); + assert.equal(names[0], 'id'); + assert.equal(names[1], 'email'); + assert.equal(names[2], 'firstName'); + assert.equal(names[3], 'lastName'); + done(err, result); + }); + }); + }); + }); + }); + + it('should report errors for automigrate', function(done) { + ds.automigrate('XYZ', function(err) { + assert(err); + done(); + }); + }); + + it('should report errors for autoupdate', function(done) { + ds.autoupdate('XYZ', function(err) { + assert(err); + done(); + }); + }); + +}); + diff --git a/test/mysql.discover.test.js b/test/mysql.discover.test.js index ec9c7ad..f0d749e 100644 --- a/test/mysql.discover.test.js +++ b/test/mysql.discover.test.js @@ -200,12 +200,14 @@ describe('Discover LDL schema from a table', function () { assert(schema.options.mysql.schema === 'STRONGLOOP'); assert(schema.options.mysql.table === 'INVENTORY'); assert(schema.properties.productId); + assert(schema.properties.productId.required); assert(schema.properties.productId.type === 'String'); assert(schema.properties.productId.mysql.columnName === 'PRODUCT_ID'); assert(schema.properties.locationId); assert(schema.properties.locationId.type === 'String'); assert(schema.properties.locationId.mysql.columnName === 'LOCATION_ID'); assert(schema.properties.available); + assert(schema.properties.available.required === false); assert(schema.properties.available.type === 'Number'); assert(schema.properties.total); assert(schema.properties.total.type === 'Number');