From b3af184377244029acbb4689a8dacb9545dfa92f Mon Sep 17 00:00:00 2001 From: Harminder virk Date: Tue, 26 Nov 2019 16:48:47 +0530 Subject: [PATCH] refactor(Migrator): Include a complete trace of migrated files and their respective status --- adonis-typings/migrator.ts | 13 +- src/Migrator/MigrationSource.ts | 7 +- src/Migrator/index.ts | 111 +++++++---- test/migrations/migrator.spec.ts | 324 ++++++++++++++++++++++++++----- 4 files changed, 360 insertions(+), 95 deletions(-) diff --git a/adonis-typings/migrator.ts b/adonis-typings/migrator.ts index 7df828e4..a0fafc0f 100644 --- a/adonis-typings/migrator.ts +++ b/adonis-typings/migrator.ts @@ -40,9 +40,16 @@ declare module '@ioc:Adonis/Lucid/Migrator' { export interface MigratorContract { dryRun: boolean direction: 'up' | 'down' - status: 'completed' | 'skipped' | 'pending' - migratedFiles: string[] - migratedQueries: { [file: string]: string[] } + status: 'completed' | 'skipped' | 'pending' | 'error' + error: null | Error + migratedFiles: { + [file: string]: { + status: 'completed' | 'error' | 'pending', + queries: string[], + migration: MigrationNode, + batch: number, + }, + } run (): Promise getList (): Promise<{ batch: number, name: string, migration_time: Date }[]> close (): Promise diff --git a/src/Migrator/MigrationSource.ts b/src/Migrator/MigrationSource.ts index 71a0698b..7c419b58 100644 --- a/src/Migrator/MigrationSource.ts +++ b/src/Migrator/MigrationSource.ts @@ -31,8 +31,10 @@ export class MigrationSource { * paths are resolved from the project root */ private _getDirectoryFiles (directoryPath: string): Promise { + const basePath = this._app.cliCwd || this._app.appRoot + return new Promise((resolve, reject) => { - const path = isAbsolute(directoryPath) ? directoryPath : join(this._app.appRoot, directoryPath) + const path = isAbsolute(directoryPath) ? directoryPath : join(basePath, directoryPath) readdir(path, (error, files) => { if (error) { reject(error) @@ -55,7 +57,8 @@ export class MigrationSource { * are not defined, then `database/migrations` fallback is used */ private _getMigrationsPath (): string[] { - return (this._config.migrations && this._config.migrations.paths) || ['database/migrations'] + const directories = (this._config.migrations || {}).paths + return directories && directories.length ? directories : ['database/migrations'] } /** diff --git a/src/Migrator/index.ts b/src/Migrator/index.ts index 01f59067..198e8647 100644 --- a/src/Migrator/index.ts +++ b/src/Migrator/index.ts @@ -37,12 +37,14 @@ export class Migrator implements MigratorContract { /** * Reference to the migrations config for the given connection */ - private _migrationsConfig = { + private _migrationsConfig = Object.assign({ tableName: 'adonis_schema', disableTransactions: false, - ...this._config.migrations, - } + }, this._config.migrations) + /** + * Whether or not the migrator has been booted + */ private _booted: boolean = false /** @@ -50,11 +52,6 @@ export class Migrator implements MigratorContract { */ private _migrationSource = new MigrationSource(this._config, this._app) - /** - * The latest batch in the database before we start the migrations - */ - private _latestBatch?: number - /** * Mode decides in which mode the migrator is executing migrations. The migrator * instance can only run in one mode at a time. @@ -72,18 +69,31 @@ export class Migrator implements MigratorContract { * An array of files we have successfully migrated. The files are * collected regardless of `up` or `down` methods */ - public migratedFiles: string[] = [] + public migratedFiles: { + [file: string]: { + status: 'completed' | 'error' | 'pending', + queries: string[], + migration: MigrationNode, + batch: number, + }, + } = {} /** - * Copy of migrated queries, available only in dry run + * Last error occurred when executing migrations */ - public migratedQueries = {} + public error: null | Error = null /** * Current status of the migrator */ public get status () { - return !this._booted ? 'pending' : (this.migratedFiles.length ? 'completed' : 'skipped') + return !this._booted + ? 'pending' + : ( + this.error + ? 'error' + : (Object.keys(this.migratedFiles).length ? 'completed' : 'skipped') + ) } constructor ( @@ -144,13 +154,13 @@ export class Migrator implements MigratorContract { executionResponse: boolean | string[], ) { if (this.dryRun) { - this.migratedQueries[name] = executionResponse as string[] + this.migratedFiles[name].queries = executionResponse as string[] return } await client.insertQuery().table(this._migrationsConfig.tableName).insert({ name, - batch: this._latestBatch! + 1, + batch: this.migratedFiles[name].batch, }) } @@ -164,7 +174,7 @@ export class Migrator implements MigratorContract { executionResponse: boolean | string[], ) { if (this.dryRun) { - this.migratedQueries[name] = executionResponse as string[] + this.migratedFiles[name].queries = executionResponse as string[] return } @@ -190,8 +200,9 @@ export class Migrator implements MigratorContract { } await this._commit(client) - this.migratedFiles.push(migration.name) + this.migratedFiles[migration.name].status = 'completed' } catch (error) { + this.migratedFiles[migration.name].status = 'error' await this._rollback(client) throw error } @@ -279,17 +290,16 @@ export class Migrator implements MigratorContract { } /** - * Returns an array of files migrated till now + * Returns an array of files migrated till now. The latest + * migrations are on top */ private async _getMigratedFilesTillBatch (batch) { - const rows = await this._client - .query<{ name: string }[]>() + return this._client + .query<{ name: string, batch: number }[]>() .from(this._migrationsConfig.tableName) - .select('name') + .select('name', 'batch') .where('batch', '>', batch) .orderBy('id', 'desc') - - return rows.map(({ name }) => name) } /** @@ -300,7 +310,6 @@ export class Migrator implements MigratorContract { this._booted = true await this._acquireLock() await this._makeMigrationsTable() - this._latestBatch = await this._getLatestBatch() } /** @@ -314,16 +323,27 @@ export class Migrator implements MigratorContract { * Migrate up */ private async _runUp () { + const batch = await this._getLatestBatch() const existing = await this._getMigratedFiles() const collected = await this._migrationSource.getMigrations() - for (let migration of collected) { - /** - * Only execute non migrated files - */ + /** + * Upfront collecting the files to be executed + */ + collected.forEach((migration) => { if (!existing.has(migration.name)) { - await this._executeMigration(migration) + this.migratedFiles[migration.name] = { + status: 'pending', + queries: [], + migration: migration, + batch: batch + 1, + } } + }) + + const filesToMigrate = Object.keys(this.migratedFiles) + for (let name of filesToMigrate) { + await this._executeMigration(this.migratedFiles[name].migration) } } @@ -338,22 +358,27 @@ export class Migrator implements MigratorContract { * Finding schema files for migrations to rollback. We do not perform * rollback when any of the files are missing */ - const migrations = existing.reduce((migrations: MigrationNode[], file) => { - const migration = collected.find((migration) => migration.name === file) + existing.forEach((file) => { + const migration = collected.find((migration) => migration.name === file.name) if (!migration) { throw new Exception( - `Cannot perform rollback. Schema file {${file}} is missing`, + `Cannot perform rollback. Schema file {${file.name}} is missing`, 500, 'E_MISSING_SCHEMA_FILES', ) } - migrations.push(migration) - return migrations - }, []) + this.migratedFiles[migration.name] = { + status: 'pending', + queries: [], + migration: migration, + batch: file.batch, + } + }) - for (let migration of migrations) { - await this._executeMigration(migration) + const filesToMigrate = Object.keys(this.migratedFiles) + for (let name of filesToMigrate) { + await this._executeMigration(this.migratedFiles[name].migration) } } @@ -372,12 +397,16 @@ export class Migrator implements MigratorContract { * Migrate the database by calling the up method */ public async run () { - await this._boot() + try { + await this._boot() - if (this.direction === 'up') { - await this._runUp() - } else if (this._options.direction === 'down') { - await this._runDown(this._options.batch) + if (this.direction === 'up') { + await this._runUp() + } else if (this._options.direction === 'down') { + await this._runDown(this._options.batch) + } + } catch (error) { + this.error = error } await this._shutdown() diff --git a/test/migrations/migrator.spec.ts b/test/migrations/migrator.spec.ts index c8c67a6a..761e6667 100644 --- a/test/migrations/migrator.spec.ts +++ b/test/migrations/migrator.spec.ts @@ -48,7 +48,7 @@ test.group('Migrator', (group) => { await migrator.run() const hasSchemaTable = await db.connection().schema.hasTable('adonis_schema') assert.isTrue(hasSchemaTable) - assert.deepEqual(migrator.migratedFiles, []) + assert.deepEqual(migrator.migratedFiles, {}) assert.equal(migrator.status, 'skipped') }) @@ -75,12 +75,23 @@ test.group('Migrator', (group) => { const migrated = await db.connection().from('adonis_schema').select('*') const hasUsersTable = await db.connection().schema.hasTable('schema_users') + const migratedFiles = Object.keys(migrator.migratedFiles).map((file) => { + return { + status: migrator.migratedFiles[file].status, + file: file, + queries: migrator.migratedFiles[file].queries, + } + }) assert.lengthOf(migrated, 1) assert.equal(migrated[0].name, 'database/migrations/users') assert.equal(migrated[0].batch, 1) assert.isTrue(hasUsersTable) - assert.deepEqual(migrator.migratedFiles, ['database/migrations/users']) + assert.deepEqual(migratedFiles, [{ + status: 'completed', + file: 'database/migrations/users', + queries: [], + }]) assert.equal(migrator.status, 'completed') }) @@ -116,23 +127,39 @@ test.group('Migrator', (group) => { connectionName: 'primary', }) - try { - await migrator.run() - } catch (error) { - assert.exists(error) - } + await migrator.run() const migrated = await db.connection().from('adonis_schema').select('*') const hasUsersTable = await db.connection().schema.hasTable('schema_users') const hasAccountsTable = await db.connection().schema.hasTable('schema_accounts') + const migratedFiles = Object.keys(migrator.migratedFiles).map((file) => { + return { + status: migrator.migratedFiles[file].status, + file: file, + queries: migrator.migratedFiles[file].queries, + } + }) assert.lengthOf(migrated, 1) assert.equal(migrated[0].name, 'database/migrations/accounts') assert.equal(migrated[0].batch, 1) assert.isFalse(hasUsersTable, 'Has users table') assert.isTrue(hasAccountsTable, 'Has accounts table') - assert.deepEqual(migrator.migratedFiles, ['database/migrations/accounts']) - assert.equal(migrator.status, 'completed') + assert.deepEqual(migratedFiles, [ + { + status: 'completed', + file: 'database/migrations/accounts', + queries: [], + }, + { + status: 'error', + file: 'database/migrations/users', + queries: [], + }, + ]) + + assert.equal(migrator.status, 'error') + assert.equal(migrator.error!.message, 'table.badMethod is not a function') }) test('do not migrate when dryRun is true', async (assert) => { @@ -171,30 +198,105 @@ test.group('Migrator', (group) => { const migrated = await db.connection().from('adonis_schema').select('*') const hasUsersTable = await db.connection().schema.hasTable('schema_users') const hasAccountsTable = await db.connection().schema.hasTable('schema_accounts') + const migratedFiles = Object.keys(migrator.migratedFiles).map((file) => { + return { + status: migrator.migratedFiles[file].status, + file: file, + queries: migrator.migratedFiles[file].queries, + } + }) assert.lengthOf(migrated, 0) assert.isFalse(hasUsersTable, 'Has users table') assert.isFalse(hasAccountsTable, 'Has accounts table') - assert.deepEqual(migrator.migratedFiles, [ - 'database/migrations/accounts', - 'database/migrations/users', + assert.deepEqual(migratedFiles, [ + { + status: 'completed', + file: 'database/migrations/accounts', + queries: [ + db.connection().schema.createTable('schema_accounts', (table) => { + table.increments() + }).toQuery(), + ], + }, + { + status: 'completed', + file: 'database/migrations/users', + queries: [ + db.connection().schema.createTable('schema_users', (table) => { + table.increments() + }).toQuery(), + ], + }, ]) - assert.deepEqual(migrator.migratedQueries, { - 'database/migrations/accounts': [ - db.connection().schema.createTable('schema_accounts', (table) => { - table.increments() - }).toQuery(), - ], - 'database/migrations/users': [ - db.connection().schema.createTable('schema_users', (table) => { - table.increments() - }).toQuery(), - ], + assert.equal(migrator.status, 'completed') + }) + + test('catch and report errors in dryRun', async (assert) => { + const app = new Application(fs.basePath, {} as any, {} as any, {}) + + await fs.add('database/migrations/users.ts', ` + import { Schema } from '../../../../../src/Schema' + module.exports = class User extends Schema { + public async up () { + this.schema.createTable('schema_users', (table) => { + table.increments() + }) + } + } + `) + + await fs.add('database/migrations/accounts.ts', ` + import { Schema } from '../../../../../src/Schema' + module.exports = class Accounts extends Schema { + public async up () { + this.schema.createTable('schema_accounts', (table) => { + table.increments() + table['badMethod']('account_id') + }) + } + } + `) + + const migrator = getMigrator(db, app, { + direction: 'up', + connectionName: 'primary', + dryRun: true, }) - assert.equal(migrator.status, 'completed') + await migrator.run() + + const migrated = await db.connection().from('adonis_schema').select('*') + const hasUsersTable = await db.connection().schema.hasTable('schema_users') + const hasAccountsTable = await db.connection().schema.hasTable('schema_accounts') + const migratedFiles = Object.keys(migrator.migratedFiles).map((file) => { + return { + status: migrator.migratedFiles[file].status, + file: file, + queries: migrator.migratedFiles[file].queries, + } + }) + + assert.lengthOf(migrated, 0) + assert.isFalse(hasUsersTable, 'Has users table') + assert.isFalse(hasAccountsTable, 'Has accounts table') + + assert.deepEqual(migratedFiles, [ + { + status: 'error', + file: 'database/migrations/accounts', + queries: [], + }, + { + status: 'pending', + file: 'database/migrations/users', + queries: [], + }, + ]) + + assert.equal(migrator.status, 'error') }) test('do not migrate a schema file twice', async (assert) => { @@ -293,11 +395,22 @@ test.group('Migrator', (group) => { const migrated = await db.connection().from('adonis_schema').select('*') const hasUsersTable = await db.connection().schema.hasTable('schema_users') const hasAccountsTable = await db.connection().schema.hasTable('schema_accounts') + const migratedFiles = Object.keys(migrator2.migratedFiles).map((file) => { + return { + status: migrator2.migratedFiles[file].status, + file: file, + queries: migrator2.migratedFiles[file].queries, + } + }) assert.lengthOf(migrated, 1) assert.isTrue(hasUsersTable) assert.isFalse(hasAccountsTable) - assert.deepEqual(migrator2.migratedFiles, ['database/migrations/accounts']) + assert.deepEqual(migratedFiles, [{ + status: 'completed', + file: 'database/migrations/accounts', + queries: [], + }]) }) test('rollback all down to batch 0', async (assert) => { @@ -345,15 +458,30 @@ test.group('Migrator', (group) => { const migrated = await db.connection().from('adonis_schema').select('*') const hasUsersTable = await db.connection().schema.hasTable('schema_users') const hasAccountsTable = await db.connection().schema.hasTable('schema_accounts') + const migratedFiles = Object.keys(migrator2.migratedFiles).map((file) => { + return { + status: migrator2.migratedFiles[file].status, + file: file, + queries: migrator2.migratedFiles[file].queries, + } + }) assert.lengthOf(migrated, 0) assert.isFalse(hasUsersTable) assert.isFalse(hasAccountsTable) assert.equal(migrator2.status, 'completed') - assert.deepEqual(migrator2.migratedFiles, [ - 'database/migrations/accounts', - 'database/migrations/users', + assert.deepEqual(migratedFiles, [ + { + status: 'completed', + file: 'database/migrations/accounts', + queries: [], + }, + { + status: 'completed', + file: 'database/migrations/users', + queries: [], + }, ]) }) @@ -406,17 +534,41 @@ test.group('Migrator', (group) => { const hasUsersTable = await db.connection().schema.hasTable('schema_users') const hasAccountsTable = await db.connection().schema.hasTable('schema_accounts') + const migrator2Files = Object.keys(migrator2.migratedFiles).map((file) => { + return { + status: migrator2.migratedFiles[file].status, + file: file, + queries: migrator2.migratedFiles[file].queries, + } + }) + + const migrator3Files = Object.keys(migrator3.migratedFiles).map((file) => { + return { + status: migrator3.migratedFiles[file].status, + file: file, + queries: migrator3.migratedFiles[file].queries, + } + }) + assert.lengthOf(migrated, 0) assert.isFalse(hasUsersTable) assert.isFalse(hasAccountsTable) assert.equal(migrator2.status, 'completed') assert.equal(migrator3.status, 'skipped') - assert.deepEqual(migrator2.migratedFiles, [ - 'database/migrations/accounts', - 'database/migrations/users', + assert.deepEqual(migrator2Files, [ + { + status: 'completed', + file: 'database/migrations/accounts', + queries: [], + }, + { + status: 'completed', + file: 'database/migrations/users', + queries: [], + }, ]) - assert.deepEqual(migrator3.migratedFiles, []) + assert.deepEqual(migrator3Files, []) }) test('do not rollback in dryRun', async (assert) => { @@ -469,25 +621,35 @@ test.group('Migrator', (group) => { const migrated = await db.connection().from('adonis_schema').select('*') const hasUsersTable = await db.connection().schema.hasTable('schema_users') const hasAccountsTable = await db.connection().schema.hasTable('schema_accounts') + const migrator2Files = Object.keys(migrator2.migratedFiles).map((file) => { + return { + status: migrator2.migratedFiles[file].status, + file: file, + queries: migrator2.migratedFiles[file].queries, + } + }) assert.lengthOf(migrated, 2) assert.isTrue(hasUsersTable) assert.isTrue(hasAccountsTable) assert.equal(migrator2.status, 'completed') - assert.deepEqual(migrator2.migratedFiles, [ - 'database/migrations/accounts', - 'database/migrations/users', + assert.deepEqual(migrator2Files, [ + { + status: 'completed', + file: 'database/migrations/accounts', + queries: [ + db.connection().schema.dropTable('schema_accounts').toQuery(), + ], + }, + { + status: 'completed', + file: 'database/migrations/users', + queries: [ + db.connection().schema.dropTable('schema_users').toQuery(), + ], + }, ]) - - assert.deepEqual(migrator2.migratedQueries, { - 'database/migrations/accounts': [ - db.connection().schema.dropTable('schema_accounts').toQuery(), - ], - 'database/migrations/users': [ - db.connection().schema.dropTable('schema_users').toQuery(), - ], - }) }) test('do not rollback when a schema file goes missing', async (assert) => { @@ -535,11 +697,7 @@ test.group('Migrator', (group) => { connectionName: 'primary', }) - try { - await migrator1.run() - } catch ({ message }) { - assert.equal(message, 'E_MISSING_SCHEMA_FILES: Cannot perform rollback. Schema file {database/migrations/accounts} is missing') - } + await migrator1.run() const migrated = await db.connection().from('adonis_schema').select('*') const hasUsersTable = await db.connection().schema.hasTable('schema_users') @@ -548,6 +706,7 @@ test.group('Migrator', (group) => { assert.lengthOf(migrated, 2) assert.isTrue(hasUsersTable) assert.isTrue(hasAccountsTable) + assert.equal(migrator1.error!.message, 'E_MISSING_SCHEMA_FILES: Cannot perform rollback. Schema file {database/migrations/accounts} is missing') }) test('get list of migrated files', async (assert) => { @@ -594,4 +753,71 @@ test.group('Migrator', (group) => { assert.equal(files[1].name, 'database/migrations/users') assert.equal(files[1].batch, 1) }) + + test('skip upcoming migrations after failure', async (assert) => { + const app = new Application(fs.basePath, {} as any, {} as any, {}) + + await fs.add('database/migrations/users.ts', ` + import { Schema } from '../../../../../src/Schema' + module.exports = class User extends Schema { + public async up () { + this.schema.createTable('schema_users', (table) => { + table.increments() + }) + } + } + `) + + await fs.add('database/migrations/accounts.ts', ` + import { Schema } from '../../../../../src/Schema' + module.exports = class Accounts extends Schema { + public async up () { + this.schema.createTable('schema_accounts', (table) => { + table.increments() + table['badMethod']('account_id') + }) + } + } + `) + + const migrator = getMigrator(db, app, { + direction: 'up', + connectionName: 'primary', + }) + + try { + await migrator.run() + } catch (error) { + assert.exists(error) + } + + const migrated = await db.connection().from('adonis_schema').select('*') + const hasUsersTable = await db.connection().schema.hasTable('schema_users') + const hasAccountsTable = await db.connection().schema.hasTable('schema_accounts') + const migratedFiles = Object.keys(migrator.migratedFiles).map((file) => { + return { + status: migrator.migratedFiles[file].status, + file: file, + queries: migrator.migratedFiles[file].queries, + } + }) + + assert.lengthOf(migrated, 0) + assert.isFalse(hasUsersTable, 'Has users table') + assert.isFalse(hasAccountsTable, 'Has accounts table') + assert.deepEqual(migratedFiles, [ + { + status: 'error', + file: 'database/migrations/accounts', + queries: [], + }, + { + status: 'pending', + file: 'database/migrations/users', + queries: [], + }, + ]) + + assert.equal(migrator.status, 'error') + }) })