Merge eba7bd3 into 524735b

packages/cli/src/commands/MigrationCommandFactory.ts

@@ -145,13 +145,22 @@ export class MigrationCommandFactory {
   private static async handleCreateCommand(migrator: Migrator, args: Arguments<Options>, config: Configuration): Promise<void> {
     const ret = await migrator.createMigration(args.path, args.blank, args.initial);
 
-    if (ret.diff.length === 0) {
+    if (ret.diff.up.length === 0) {
       return CLIHelper.dump(c.green(`No changes required, schema is up-to-date`));
     }
 
     if (args.dump) {
       CLIHelper.dump(c.green('Creating migration with following queries:'));
-      CLIHelper.dump(ret.diff.map(sql => '  ' + sql).join('\n'), config);
+      CLIHelper.dump(c.green('up:'));
+      CLIHelper.dump(ret.diff.up.map(sql => '  ' + sql).join('\n'), config);
+
+      /* istanbul ignore else */
+      if (config.getDriver().getPlatform().supportsDownMigrations()) {
+        CLIHelper.dump(c.green('down:'));
+        CLIHelper.dump(ret.diff.down.map(sql => '  ' + sql).join('\n'), config);
+      } else {
+        CLIHelper.dump(c.yellow(`(${config.getDriver().constructor.name} does not support automatic down migrations)`));
+      }
     }
 
     CLIHelper.dump(c.green(`${ret.fileName} successfully created`));

packages/core/src/platforms/Platform.ts

@@ -369,4 +369,11 @@ export abstract class Platform {
     return [ReferenceType.SCALAR, ReferenceType.MANY_TO_ONE].includes(prop.reference) || (prop.reference === ReferenceType.ONE_TO_ONE && prop.owner);
   }
 
+  /**
+   * Currently not supported due to how knex does complex sqlite diffing (always based on current schema)
+   */
+  supportsDownMigrations(): boolean {
+    return true;
+  }
+
 }

packages/core/src/typings.ts

@@ -371,6 +371,7 @@ export interface ISchemaGenerator {
   getDropSchemaSQL(options?: { wrap?: boolean; dropMigrationsTable?: boolean }): Promise<string>;
   updateSchema(options?: { wrap?: boolean; safe?: boolean; dropDb?: boolean; dropTables?: boolean }): Promise<void>;
   getUpdateSchemaSQL(options?: { wrap?: boolean; safe?: boolean; dropDb?: boolean; dropTables?: boolean }): Promise<string>;
+  getUpdateSchemaMigrationSQL(options?: { wrap?: boolean; safe?: boolean; dropDb?: boolean; dropTables?: boolean }): Promise<{ up: string; down: string }>;
   createDatabase(name: string): Promise<void>;
   dropDatabase(name: string): Promise<void>;
   execute(sql: string, options?: { wrap?: boolean }): Promise<void>;

packages/knex/src/schema/SchemaComparator.ts

@@ -425,8 +425,10 @@ export class SchemaComparator {
     }
 
     if (to.mappedType instanceof BooleanType) {
-      const defaultValue = !['0', 'false', 'f', 'n', 'no', 'off'].includes(from.default!);
-      return '' + defaultValue === to.default;
+      const defaultValueFrom = !['0', 'false', 'f', 'n', 'no', 'off'].includes('' + from.default!);
+      const defaultValueTo = !['0', 'false', 'f', 'n', 'no', 'off'].includes('' + to.default!);
+
+      return defaultValueFrom === defaultValueTo;
     }
 
     if (from.default && to.default) {

packages/knex/src/schema/SchemaGenerator.ts

@@ -1,6 +1,6 @@
 import { Knex } from 'knex';
 import { CommitOrderCalculator, Dictionary, EntityMetadata } from '@mikro-orm/core';
-import { Column, ForeignKey, Index, TableDifference } from '../typings';
+import { Column, ForeignKey, Index, SchemaDifference, TableDifference } from '../typings';
 import { DatabaseSchema } from './DatabaseSchema';
 import { DatabaseTable } from './DatabaseTable';
 import { SqlEntityManager } from '../SqlEntityManager';
@@ -87,11 +87,11 @@ export class SchemaGenerator {
     let ret = '';
 
     for (const meta of metadata) {
-      ret += await this.dump(this.dropTable(meta.collection), '\n');
+      ret += await this.dump(this.dropTable(meta.collection, meta.schema), '\n');
     }
 
     if (options.dropMigrationsTable) {
-      ret += await this.dump(this.dropTable(this.config.get('migrations').tableName!), '\n');
+      ret += await this.dump(this.dropTable(this.config.get('migrations').tableName!, this.config.get('schema')), '\n');
     }
 
     return this.wrapSchema(ret + '\n', { wrap });
@@ -103,13 +103,35 @@ export class SchemaGenerator {
   }
 
   async getUpdateSchemaSQL(options: { wrap?: boolean; safe?: boolean; dropTables?: boolean; fromSchema?: DatabaseSchema } = {}): Promise<string> {
-    const wrap = options.wrap ?? true;
+    options.wrap = options.wrap ?? true;
     options.safe = options.safe ?? false;
     options.dropTables = options.dropTables ?? true;
     const toSchema = this.getTargetSchema();
+    /* istanbul ignore next */
     const fromSchema = options.fromSchema ?? await DatabaseSchema.create(this.connection, this.platform, this.config);
     const comparator = new SchemaComparator(this.platform);
-    const schemaDiff = comparator.compare(fromSchema, toSchema);
+    const diffUp = comparator.compare(fromSchema, toSchema);
+
+    return this.diffToSQL(diffUp, options);
+  }
+
+  async getUpdateSchemaMigrationSQL(options: { wrap?: boolean; safe?: boolean; dropTables?: boolean; fromSchema?: DatabaseSchema } = {}): Promise<{ up: string; down: string }> {
+    options.wrap = options.wrap ?? true;
+    options.safe = options.safe ?? false;
+    options.dropTables = options.dropTables ?? true;
+    const toSchema = this.getTargetSchema();
+    const fromSchema = options.fromSchema ?? await DatabaseSchema.create(this.connection, this.platform, this.config);
+    const comparator = new SchemaComparator(this.platform);
+    const diffUp = comparator.compare(fromSchema, toSchema);
+    const diffDown = comparator.compare(toSchema, fromSchema);
+
+    return {
+      up: await this.diffToSQL(diffUp, options),
+      down: this.platform.supportsDownMigrations() ? await this.diffToSQL(diffDown, options) : '',
+    };
+  }
+
+  async diffToSQL(schemaDiff: SchemaDifference, options: { wrap?: boolean; safe?: boolean; dropTables?: boolean }): Promise<string> {
     let ret = '';
 
     if (this.platform.supportsSchemas()) {
@@ -139,13 +161,13 @@ export class SchemaGenerator {
     }
 
     for (const changedTable of Object.values(schemaDiff.changedTables)) {
-      for (const builder of this.preAlterTable(changedTable, options.safe)) {
+      for (const builder of this.preAlterTable(changedTable, options.safe!)) {
         ret += await this.dump(builder);
       }
     }
 
     for (const changedTable of Object.values(schemaDiff.changedTables)) {
-      for (const builder of this.alterTable(changedTable, options.safe)) {
+      for (const builder of this.alterTable(changedTable, options.safe!)) {
         ret += await this.dump(builder);
       }
     }
@@ -156,7 +178,7 @@ export class SchemaGenerator {
       }
     }
 
-    return this.wrapSchema(ret, { wrap });
+    return this.wrapSchema(ret, options);
   }
 
   private createForeignKey(table: Knex.CreateTableBuilder, foreignKey: ForeignKey) {

packages/knex/src/schema/SchemaHelper.ts

@@ -109,9 +109,9 @@ export abstract class SchemaHelper {
     const guard = (key: string) => !changedProperties || changedProperties.has(key);
 
     if (changedProperties) {
-      Utils.runIfNotEmpty(() => col.defaultTo(column.default === undefined ? null : knex.raw(column.default)), guard('default'));
+      Utils.runIfNotEmpty(() => col.defaultTo(column.default == null ? null : knex.raw(column.default)), guard('default'));
     } else {
-      Utils.runIfNotEmpty(() => col.defaultTo(knex.raw(column.default!)), column.default !== undefined);
+      Utils.runIfNotEmpty(() => col.defaultTo(column.default == null ? null : knex.raw(column.default)), column.default !== undefined);
     }
 
     return col;

packages/migrations/src/MigrationGenerator.ts

@@ -8,7 +8,7 @@ export class MigrationGenerator {
               protected readonly namingStrategy: NamingStrategy,
               protected readonly options: MigrationsOptions) { }
 
-  async generate(diff: string[], path?: string): Promise<[string, string]> {
+  async generate(diff: { up: string[]; down: string[] }, path?: string): Promise<[string, string]> {
     path = Utils.normalizePath(path || this.options.path!);
     await ensureDir(path);
     const timestamp = new Date().toISOString().replace(/[-T:]|\.\d{3}z$/ig, '');
@@ -36,26 +36,41 @@ export class MigrationGenerator {
     return '\n';
   }
 
-  generateJSMigrationFile(className: string, diff: string[]): string {
+  generateJSMigrationFile(className: string, diff: { up: string[]; down: string[] }): string {
     let ret = `'use strict';\n`;
     ret += `Object.defineProperty(exports, '__esModule', { value: true });\n`;
     ret += `const Migration = require('@mikro-orm/migrations').Migration;\n\n`;
     ret += `class ${className} extends Migration {\n\n`;
     ret += `  async up() {\n`;
-    diff.forEach(sql => ret += this.createStatement(sql, 4));
+    diff.up.forEach(sql => ret += this.createStatement(sql, 4));
     ret += `  }\n\n`;
+
+    /* istanbul ignore else */
+    if (diff.down.length > 0) {
+      ret += `  async down() {\n`;
+      diff.down.forEach(sql => ret += this.createStatement(sql, 4));
+      ret += `  }\n\n`;
+    }
+
     ret += `}\n`;
     ret += `exports.${className} = ${className};\n`;
 
     return ret;
   }
 
-  generateTSMigrationFile(className: string, diff: string[]): string {
+  generateTSMigrationFile(className: string, diff: { up: string[]; down: string[] }): string {
     let ret = `import { Migration } from '@mikro-orm/migrations';\n\n`;
     ret += `export class ${className} extends Migration {\n\n`;
     ret += `  async up(): Promise<void> {\n`;
-    diff.forEach(sql => ret += this.createStatement(sql, 4));
+    diff.up.forEach(sql => ret += this.createStatement(sql, 4));
     ret += `  }\n\n`;
+
+    if (diff.down.length > 0) {
+      ret += `  async down(): Promise<void> {\n`;
+      diff.down.forEach(sql => ret += this.createStatement(sql, 4));
+      ret += `  }\n\n`;
+    }
+
     ret += `}\n`;
 
     return ret;

packages/migrations/src/Migrator.ts

@@ -49,7 +49,7 @@ export class Migrator {
     await this.ensureMigrationsDirExists();
     const diff = await this.getSchemaDiff(blank, initial);
 
-    if (diff.length === 0) {
+    if (diff.up.length === 0) {
       return { fileName: '', code: '', diff };
     }
 
@@ -200,33 +200,39 @@ export class Migrator {
     };
   }
 
-  private async getSchemaDiff(blank: boolean, initial: boolean): Promise<string[]> {
-    const lines: string[] = [];
+  private async getSchemaDiff(blank: boolean, initial: boolean): Promise<{ up: string[]; down: string[] }> {
+    const up: string[] = [];
+    const down: string[] = [];
 
     if (blank) {
-      lines.push('select 1');
+      up.push('select 1');
     } else if (initial) {
       const dump = await this.schemaGenerator.getCreateSchemaSQL({ wrap: false });
-      lines.push(...dump.split('\n'));
+      up.push(...dump.split('\n'));
     } else {
-      const dump = await this.schemaGenerator.getUpdateSchemaSQL({
+      const diff = await this.schemaGenerator.getUpdateSchemaMigrationSQL({
         wrap: false,
         safe: this.options.safe,
         dropTables: this.options.dropTables,
         fromSchema: await this.getCurrentSchema(),
       });
-      lines.push(...dump.split('\n'));
+      up.push(...diff.up.split('\n'));
+      down.push(...diff.down.split('\n'));
     }
 
-    for (let i = lines.length - 1; i >= 0; i--) {
-      if (lines[i]) {
-        break;
-      }
+    const cleanUp = (diff: string[]) => {
+      for (let i = diff.length - 1; i >= 0; i--) {
+        if (diff[i]) {
+          break;
+        }
 
-      lines.splice(i, 1);
-    }
+        diff.splice(i, 1);
+      }
+    };
+    cleanUp(up);
+    cleanUp(down);
 
-    return lines;
+    return { up, down };
   }
 
   private prefix<T extends string | string[] | { from?: string; to?: string; migrations?: string[]; transaction?: Transaction }>(options?: T): T {

packages/migrations/src/typings.ts

@@ -2,5 +2,5 @@ import { Transaction } from '@mikro-orm/core';
 
 export type UmzugMigration = { name?: string; path?: string; file: string };
 export type MigrateOptions = { from?: string | number; to?: string | number; migrations?: string[]; transaction?: Transaction };
-export type MigrationResult = { fileName: string; code: string; diff: string[] };
+export type MigrationResult = { fileName: string; code: string; diff: { up: string[]; down: string[] } };
 export type MigrationRow = { name: string; executed_at: Date };

packages/postgresql/src/PostgreSqlConnection.ts

@@ -68,15 +68,20 @@ export class PostgreSqlConnection extends AbstractSqlConnection {
     const colName = this.client.wrapIdentifier(col.getColumnName(), col.columnBuilder.queryContext());
     const constraintName = `${this.tableNameRaw}_${col.getColumnName()}_check`;
     this.pushQuery({ sql: `alter table ${quotedTableName} drop constraint if exists "${constraintName}"`, bindings: [] });
+    that.dropColumnDefault.call(this, col, colName);
 
     if (col.type === 'enu') {
       this.pushQuery({ sql: `alter table ${quotedTableName} alter column ${colName} type text using (${colName}::text)`, bindings: [] });
       this.pushQuery({ sql: `alter table ${quotedTableName} add constraint "${constraintName}" ${type.replace(/^text /, '')}`, bindings: [] });
+    } else if (type === 'uuid') {
+      // we need to drop the default as it would be invalid
+      this.pushQuery({ sql: `alter table ${quotedTableName} alter column ${colName} drop default`, bindings: [] });
+      this.pushQuery({ sql: `alter table ${quotedTableName} alter column ${colName} type ${type} using (${colName}::text::uuid)`, bindings: [] });
     } else {
       this.pushQuery({ sql: `alter table ${quotedTableName} alter column ${colName} type ${type} using (${colName}::${type})`, bindings: [] });
     }
 
-    that.alterColumnDefault.call(this, col, colName);
+    that.addColumnDefault.call(this, col, colName);
     that.alterColumnNullable.call(this, col, colName);
   }
 
@@ -95,20 +100,31 @@ export class PostgreSqlConnection extends AbstractSqlConnection {
     }
   }
 
-  private alterColumnDefault(this: any, col: Dictionary, colName: string): void {
+  private addColumnDefault(this: any, col: Dictionary, colName: string): void {
     const quotedTableName = this.tableName();
     const defaultTo = col.modified.defaultTo;
 
     if (!defaultTo) {
       return;
     }
 
-    if (defaultTo[0] === null) {
-      this.pushQuery({ sql: `alter table ${quotedTableName} alter column ${colName} drop default`, bindings: [] });
-    } else {
+    if (defaultTo[0] !== null) {
       const modifier = col.defaultTo(...defaultTo);
       this.pushQuery({ sql: `alter table ${quotedTableName} alter column ${colName} set ${modifier}`, bindings: [] });
     }
   }
 
+  private dropColumnDefault(this: any, col: Dictionary, colName: string): void {
+    const quotedTableName = this.tableName();
+    const defaultTo = col.modified.defaultTo;
+
+    if (!defaultTo) {
+      return;
+    }
+
+    if (defaultTo[0] === null) {
+      this.pushQuery({ sql: `alter table ${quotedTableName} alter column ${colName} drop default`, bindings: [] });
+    }
+  }
+
 }

packages/postgresql/src/PostgreSqlSchemaHelper.ts

@@ -1,5 +1,5 @@
 import { BigIntType, Dictionary, EnumType, Utils } from '@mikro-orm/core';
-import { AbstractSqlConnection, SchemaHelper, Column, Index, DatabaseTable } from '@mikro-orm/knex';
+import { AbstractSqlConnection, SchemaHelper, Column, Index, DatabaseTable, TableDifference } from '@mikro-orm/knex';
 import { Knex } from 'knex';
 
 export class PostgreSqlSchemaHelper extends SchemaHelper {
@@ -159,6 +159,17 @@ export class PostgreSqlSchemaHelper extends SchemaHelper {
     return col;
   }
 
+  getPreAlterTable(tableDiff: TableDifference, safe: boolean): string {
+    // changing uuid column type requires to cast it to text first
+    const uuid = Object.values(tableDiff.changedColumns).find(col => col.changedProperties.has('type') && col.fromColumn.type === 'uuid');
+
+    if (!uuid) {
+      return '';
+    }
+
+    return `alter table "${tableDiff.name}" alter column "${uuid.column.name}" type text using ("${uuid.column.name}"::text)`;
+  }
+
   getAlterColumnAutoincrement(tableName: string, column: Column): string {
     const ret: string[] = [];
     const quoted = (val: string) => this.platform.quoteIdentifier(val);

packages/sqlite/src/SqlitePlatform.ts

@@ -119,4 +119,8 @@ export class SqlitePlatform extends AbstractSqlPlatform {
     return super.getIndexName(tableName, columns, type);
   }
 
+  supportsDownMigrations(): boolean {
+    return false;
+  }
+
 }

tests/bootstrap.ts

@@ -148,6 +148,7 @@ export async function initORMSqlite2() {
     forceUndefined: true,
     logger: i => i,
     cache: { pretty: true },
+    migrations: { path: BASE_DIR + '/../temp/migrations', snapshot: false },
   });
   const schemaGenerator = new SchemaGenerator(orm.em);
   await schemaGenerator.dropSchema();

tests/features/cli/CreateMigrationCommand.test.ts

@@ -11,7 +11,7 @@ const close = jest.fn();
 jest.spyOn(MikroORM.prototype, 'close').mockImplementation(close);
 jest.spyOn(require('yargs'), 'showHelp').mockReturnValue('');
 const createMigrationMock = jest.spyOn(Migrator.prototype, 'createMigration');
-createMigrationMock.mockResolvedValue({ fileName: '1', code: '2', diff: ['3'] });
+createMigrationMock.mockResolvedValue({ fileName: '1', code: '2', diff: { up: ['3'], down: [] } });
 const dumpMock = jest.spyOn(CLIHelper, 'dump');
 dumpMock.mockImplementation(() => void 0);
 
@@ -45,7 +45,7 @@ describe('CreateMigrationCommand', () => {
     expect(close.mock.calls.length).toBe(2);
     expect(dumpMock).toHaveBeenLastCalledWith('1 successfully created');
 
-    createMigrationMock.mockImplementationOnce(async () => ({ fileName: '', code: '', diff: [] }));
+    createMigrationMock.mockImplementationOnce(async () => ({ fileName: '', code: '', diff: { up: [], down: [] } }));
     await expect(cmd.handler({} as any)).resolves.toBeUndefined();
     expect(createMigrationMock.mock.calls.length).toBe(3);
     expect(close.mock.calls.length).toBe(3);