diff --git a/lib/migration.js b/lib/migration.js index 7449ceb..ae5f616 100644 --- a/lib/migration.js +++ b/lib/migration.js @@ -21,6 +21,7 @@ function mixinMigration(MySQL, mysql) { */ MySQL.prototype.autoupdate = function(models, cb) { var self = this; + var foreignKeyStatements = []; if ((!cb) && ('function' === typeof models)) { cb = models; @@ -50,14 +51,46 @@ function mixinMigration(MySQL, mysql) { if (err) console.log('Failed to discover "' + table + '" foreign keys', err); if (!err && fields && fields.length) { - self.alterTable(model, fields, indexes, foreignKeys, done); + //if we already have a definition, update this table + self.alterTable(model, fields, indexes, foreignKeys, function(err, changed, res) { + //check to see if there were any new foreign keys for this table. + //If so, we'll create them once all tables have been updated + if (!err && res && res.newFks && res.newFks.length) { + foreignKeyStatements.push(res.newFks); + } + + done(err); + }); } else { - self.createTable(model, done); + //if there is not yet a definition, create this table + var res = self.createTable(model, function(err) { + if (!err) { + //get a list of the alter statements needed to add the defined foreign keys + var newFks = self.getForeignKeySQL(model, foreignKeys); + + //check to see if there were any new foreign keys for this table. + //If so, we'll create them once all tables have been updated + if (newFks && newFks.length) { + foreignKeyStatements.push(self.getAlterStatement(model, newFks)); + } + } + + done(err); + }); } }); }); }); - }, cb); + }, function(err) { + if (err) return cb(err); + + //add any new foreign keys + async.each(foreignKeyStatements, function(addFkStmt, execDone) { + self.execute(addFkStmt, execDone); + }, function(err) { + cb(err); + }); + }); }; /*! @@ -120,13 +153,35 @@ function mixinMigration(MySQL, mysql) { }); }); }, function(err) { - if (err) { - return err; - } - cb(null, !ok); + cb(err, !ok); }); }; + MySQL.prototype.getForeignKeySQL = function(model, actualFks) { + var self = this; + var m = this.getModelDefinition(model); + var addFksSql = []; + var newFks = m.settings.foreignKeys || {}; + var newFkNames = Object.keys(newFks).filter(function(name) { + return !!m.settings.foreignKeys[name]; + }); + + //add new foreign keys + if (newFkNames.length) { + //narrow down our key names to only those that don't already exist + var oldKeyNames = actualFks.map(function(oldKey) { return oldKey.fkName; }); + newFkNames.filter(function(key) { return !~oldKeyNames.indexOf(key); }) + .forEach(function(key) { + var constraint = self.buildForeignKeyDefinition(model, key); + if (constraint) { + addFksSql.push('ADD ' + constraint); + } + }); + } + + return addFksSql; + }; + MySQL.prototype.alterTable = function(model, actualFields, actualIndexes, actualFks, done, checkOnly) { //if this is using an old signature, then grab the correct callback and check boolean if ('function' == typeof actualFks && typeof done !== 'function') { @@ -143,11 +198,9 @@ function mixinMigration(MySQL, mysql) { var indexNames = Object.keys(indexes).filter(function(name) { return !!m.settings.indexes[name]; }); - var newFks = m.settings.foreignKeys || {}; - var newFkNames = Object.keys(newFks).filter(function(name) { - return !!m.settings.foreignKeys[name]; - }); - var dropConstraintSql = []; + + //add new foreign keys + var correctFks = m.settings.foreignKeys || {}; var sql = []; var ai = {}; @@ -191,10 +244,11 @@ function mixinMigration(MySQL, mysql) { var removedFks = []; actualFks.forEach(function(fk) { var needsToDrop = false; - var newFk = newFks[fk.fkName]; + var newFk = correctFks[fk.fkName]; if (newFk) { var fkCol = expectedColName(newFk.foreignKey); - var fkRefKey = expectedColNameForModel(newFk.entityKey, newFk.entity); + var fkEntity = self.getModelDefinition(newFk.entity); + var fkRefKey = expectedColNameForModel(newFk.entityKey, fkEntity); var fkRefTable = newFk.entity.name; //TODO check for mysql name needsToDrop = fkCol != fk.fkColumnName || fkRefKey != fk.pkColumnName || @@ -204,7 +258,7 @@ function mixinMigration(MySQL, mysql) { } if (needsToDrop) { - dropConstraintSql.push('DROP FOREIGN KEY ' + fk.fkName); + sql.push('DROP FOREIGN KEY ' + fk.fkName); removedFks.push(fk); //keep track that we removed these } }); @@ -234,7 +288,7 @@ function mixinMigration(MySQL, mysql) { if (indexName === 'PRIMARY' || (m.properties[indexName] && self.id(model, indexName))) return; - if (newFkNames.indexOf(indexName) > -1) return; //this index is from an FK + if (Object.keys(actualFks).indexOf(indexName) > -1) return; //this index is from an FK if (indexNames.indexOf(indexName) === -1 && !m.properties[indexName] || m.properties[indexName] && !m.properties[indexName].index) { sql.push('DROP INDEX ' + self.client.escapeId(indexName)); @@ -351,34 +405,34 @@ function mixinMigration(MySQL, mysql) { } }); - //add new foreign keys - if (newFkNames.length) { - //TODO validate that these are in the same DB, etc. - var oldKeyNames = actualFks.map(function(oldKey) { return oldKey.fkName; }); - newFkNames.filter(function(key) { return !~oldKeyNames.indexOf(key); }) - .forEach(function(key) { - var constraint = self.buildForeignKeyDefinition(model, key); - if (constraint) { - sql.push('ADD ' + constraint); - } - }); - } + //since we're passing the actualFks list to this, + //this code has be called after foreign keys have been dropped + //(in case we're replacing FKs) + var addFksSql = this.getForeignKeySQL(model, actualFks); - if (sql.length || dropConstraintSql.length) { - var stmtList = [dropConstraintSql, sql] - .filter(function(stmts) { return stmts.length; }) - .map(function(statements) { - return 'ALTER TABLE ' + self.tableEscaped(model) + - ' ' + statements.join(',\n'); - }); + //determine if there are column, index, or foreign keys changes (all require update) + if (sql.length || addFksSql.length) { + //get the required alter statements + var alterStmt = self.getAlterStatement(model, sql); + var newFksStatement = self.getAlterStatement(model, addFksSql); + var stmtList = [alterStmt, newFksStatement].filter(function(s) { return s.length; }); - if (checkOnly) { - done(null, true, {statements: stmtList, query: stmtList.join(';')}); + //set up an object to pass back all changes, changes that have been run, + //and foreign key statements that haven't been run + var retValues = { + statements: stmtList, + query: stmtList.join(';'), + newFks: newFksStatement, + }; + + //if we're running in read only mode OR if the only changes are foreign keys additions, + //then just return the object directly + if (checkOnly || !alterStmt.length) { + done(null, true, retValues); } else { - async.eachSeries(stmtList, function(stmt, execDone) { - self.execute(stmt, execDone); - }, function(err) { - done(err); + //if there are changes in the alter statement, then execute them and return the object + self.execute(alterStmt, function(err) { + done(err, true, retValues); }); } } else { @@ -432,6 +486,12 @@ function mixinMigration(MySQL, mysql) { } }; + MySQL.prototype.getAlterStatement = function(model, statements) { + return statements.length ? + 'ALTER TABLE ' + this.tableEscaped(model) + ' ' + statements.join(',\n') : + ''; + }; + MySQL.prototype.buildForeignKeyDefinition = function(model, keyName) { //TODO validate that these are in the same DB, etc. //TODO this is probably a better place to check this than in autoupdate or buildColumnDefinitions diff --git a/test/mysql.autoupdate.test.js b/test/mysql.autoupdate.test.js index 07be0fe..98077d2 100644 --- a/test/mysql.autoupdate.test.js +++ b/test/mysql.autoupdate.test.js @@ -261,8 +261,77 @@ describe('MySQL connector', function() { }, }, }; + var customer3_schema = + { + 'name': 'CustomerTest3', + 'options': { + 'idInjection': false, + 'mysql': { + 'schema': 'myapp_test', + 'table': 'customer_test3', + }, + }, + '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_v1 = + { + 'name': 'OrderTest', + 'options': { + 'idInjection': false, + 'mysql': { + 'schema': 'myapp_test', + 'table': 'order_test', + }, + 'foreignKeys': { + 'fk_ordertest_customerId': { + 'name': 'fk_ordertest_customerId', + 'entity': 'CustomerTest3', + 'entityKey': 'id', + 'foreignKey': 'customerId', + }, + }, + }, + 'properties': { + 'id': { + 'type': 'String', + 'length': 20, + 'id': 1, + }, + 'customerId': { + 'type': 'String', + 'length': 20, + 'id': 1, + }, + 'description': { + 'type': 'String', + 'required': false, + 'length': 40, + }, + }, + }; + + var schema_v2 = { 'name': 'OrderTest', 'options': { @@ -299,7 +368,7 @@ describe('MySQL connector', function() { }, }; - var schema_v2 = + var schema_v3 = { 'name': 'OrderTest', 'options': { @@ -329,42 +398,73 @@ describe('MySQL connector', function() { }; var foreignKeySelect = - 'SELECT TABLE_NAME,COLUMN_NAME,CONSTRAINT_NAME,REFERENCED_COLUMN_NAME ' + + 'SELECT COLUMN_NAME,CONSTRAINT_NAME,REFERENCED_TABLE_NAME, REFERENCED_COLUMN_NAME ' + 'FROM INFORMATION_SCHEMA.KEY_COLUMN_USAGE ' + 'WHERE REFERENCED_TABLE_SCHEMA = "myapp_test" ' + - 'AND REFERENCED_TABLE_NAME = "customer_test2"'; + 'AND TABLE_NAME = "order_test"'; ds.createModel(customer2_schema.name, customer2_schema.properties, customer2_schema.options); + ds.createModel(customer3_schema.name, customer3_schema.properties, customer3_schema.options); ds.createModel(schema_v1.name, schema_v1.properties, schema_v1.options); - ds.automigrate(function() { - ds.autoupdate(function() { //foreign keys won't be created on table create - ds.discoverModelProperties('order_test', function(err, props) { - assert.equal(props.length, 3); + //do initial update/creation of table + ds.autoupdate(function() { + ds.discoverModelProperties('order_test', function(err, props) { + //validate that we have the correct number of properties + assert.equal(props.length, 3); - ds.connector.execute(foreignKeySelect, function(err, foreignKeys) { + //get the foreign keys for this table + ds.connector.execute(foreignKeySelect, function(err, foreignKeys) { + if (err) return done (err); + //validate that the foreign key exists and points to the right column + assert(foreignKeys); + assert(foreignKeys.length.should.be.equal(1)); + assert.equal(foreignKeys[0].REFERENCED_TABLE_NAME, 'customer_test3'); + assert.equal(foreignKeys[0].COLUMN_NAME, 'customerId'); + assert.equal(foreignKeys[0].CONSTRAINT_NAME, 'fk_ordertest_customerId'); + assert.equal(foreignKeys[0].REFERENCED_COLUMN_NAME, 'id'); + + //update our model (move foreign key) and run autoupdate to migrate + ds.createModel(schema_v2.name, schema_v2.properties, schema_v2.options); + ds.autoupdate(function(err, result) { if (err) return done (err); - assert(foreignKeys); - assert(foreignKeys.length.should.be.equal(1)); - assert.equal(foreignKeys[0].TABLE_NAME, 'order_test'); - assert.equal(foreignKeys[0].COLUMN_NAME, 'customerId'); - assert.equal(foreignKeys[0].CONSTRAINT_NAME, 'fk_ordertest_customerId'); - assert.equal(foreignKeys[0].REFERENCED_COLUMN_NAME, 'id'); - ds.createModel(schema_v2.name, schema_v2.properties, schema_v2.options); - ds.autoupdate(function(err, result) { + //get and validate the properties on this model + ds.discoverModelProperties('order_test', function(err, props) { if (err) return done (err); - ds.discoverModelProperties('order_test', function(err, props) { + + assert.equal(props.length, 3); + + //get the foreign keys that exist after the migration + ds.connector.execute(foreignKeySelect, function(err, updatedForeignKeys) { if (err) return done (err); + //validate that the foreign keys was moved to the new column + assert(updatedForeignKeys); + assert(updatedForeignKeys.length.should.be.equal(1)); + assert.equal(updatedForeignKeys[0].REFERENCED_TABLE_NAME, 'customer_test2'); + assert.equal(updatedForeignKeys[0].COLUMN_NAME, 'customerId'); + assert.equal(updatedForeignKeys[0].CONSTRAINT_NAME, 'fk_ordertest_customerId'); + assert.equal(updatedForeignKeys[0].REFERENCED_COLUMN_NAME, 'id'); - assert.equal(props.length, 3); - - ds.connector.execute(foreignKeySelect, function(err, updatedForeignKeys) { + //update model (to drop foreign key) and autoupdate + ds.createModel(schema_v3.name, schema_v3.properties, schema_v3.options); + ds.autoupdate(function(err, result) { if (err) return done (err); - assert(updatedForeignKeys); - assert(updatedForeignKeys.length.should.be.equal(0)); + //validate the properties + ds.discoverModelProperties('order_test', function(err, props) { + if (err) return done (err); - done(err, result); + assert.equal(props.length, 3); + + //get the foreign keys and validate the foreign key has been dropped + ds.connector.execute(foreignKeySelect, function(err, thirdForeignKeys) { + if (err) return done (err); + assert(thirdForeignKeys); + assert(thirdForeignKeys.length.should.be.equal(0)); + + done(err, result); + }); + }); }); }); });