fix(schema): fix handling of FKs in updateSchema() (#498)

Closes #494

lib/schema/SchemaGenerator.ts

@@ -146,11 +146,19 @@ export class SchemaGenerator {
   }
 
   private getUpdateTableFKsSQL(meta: EntityMetadata, schema: DatabaseSchema): string {
-    if (schema.getTable(meta.collection)) {
+    const table = schema.getTable(meta.collection);
+
+    if (!table) {
+      return this.dump(this.knex.schema.alterTable(meta.collection, table => this.createForeignKeys(table, meta)));
+    }
+
+    const { create } = this.computeTableDifference(meta, table, true);
+
+    if (create.length === 0) {
       return '';
     }
 
-    return this.dump(this.knex.schema.alterTable(meta.collection, table => this.createForeignKeys(table, meta)));
+    return this.dump(this.knex.schema.alterTable(meta.collection, table => this.createForeignKeys(table, meta, create)));
   }
 
   private async wrapSchema(sql: string, wrap = true): Promise<string> {
@@ -195,13 +203,12 @@ export class SchemaGenerator {
   }
 
   private updateTable(meta: EntityMetadata, table: DatabaseTable, safe: boolean): SchemaBuilder[] {
-    const { create, update, remove } = this.computeTableDifference(meta, table, safe);
+    const { create, update, remove, rename } = this.computeTableDifference(meta, table, safe);
 
-    if (create.length + update.length + remove.length === 0) {
+    if (create.length + update.length + remove.length + rename.length === 0) {
       return [];
     }
 
-    const rename = this.findRenamedColumns(create, remove);
     const ret: SchemaBuilder[] = [];
 
     for (const prop of rename) {
@@ -225,7 +232,7 @@ export class SchemaGenerator {
     return ret;
   }
 
-  private computeTableDifference(meta: EntityMetadata, table: DatabaseTable, safe: boolean): { create: EntityProperty[]; update: { prop: EntityProperty; column: Column; diff: IsSame }[]; remove: Column[] } {
+  private computeTableDifference(meta: EntityMetadata, table: DatabaseTable, safe: boolean): { create: EntityProperty[]; update: { prop: EntityProperty; column: Column; diff: IsSame }[]; rename: { from: Column; to: EntityProperty }[]; remove: Column[] } {
     const props = Object.values(meta.properties).filter(prop => this.shouldHaveColumn(meta, prop, true));
     const columns = table.getColumns();
     const create: EntityProperty[] = [];
@@ -236,11 +243,13 @@ export class SchemaGenerator {
       this.computeColumnDifference(table, prop, create, update);
     }
 
+    const rename = this.findRenamedColumns(create, remove);
+
     if (safe) {
-      return { create, update, remove: [] };
+      return { create, update, rename, remove: [] };
     }
 
-    return { create, update, remove };
+    return { create, update, rename, remove };
   }
 
   private computeColumnDifference(table: DatabaseTable, prop: EntityProperty, create: EntityProperty[], update: { prop: EntityProperty; column: Column; diff: IsSame }[], joinColumn?: string, idx = 0): void {
@@ -345,7 +354,14 @@ export class SchemaGenerator {
       table.dropForeign([column.fk.columnName], column.fk.constraintName);
     }
 
-    column.indexes.forEach(i => table.dropIndex([i.columnName], i.keyName));
+    for (const index of column.indexes) {
+      if (index.unique) {
+        table.dropUnique([index.columnName], index.keyName);
+      } else {
+        table.dropIndex([index.columnName], index.keyName);
+      }
+    }
+
     table.dropColumn(column.name);
   }
 
@@ -381,8 +397,9 @@ export class SchemaGenerator {
     return this.helper.getIndexName(meta.collection, [columnName], unique);
   }
 
-  private createForeignKeys(table: TableBuilder, meta: EntityMetadata): void {
+  private createForeignKeys(table: TableBuilder, meta: EntityMetadata, props?: EntityProperty[]): void {
     Object.values(meta.properties)
+      .filter(prop => !props || props.includes(prop))
       .filter(prop => prop.reference === ReferenceType.MANY_TO_ONE || (prop.reference === ReferenceType.ONE_TO_ONE && prop.owner))
       .forEach(prop => this.createForeignKey(table, meta, prop));
   }

lib/schema/SchemaHelper.ts

@@ -1,10 +1,9 @@
 import { TableBuilder } from 'knex';
 import { Dictionary, EntityProperty } from '../typings';
-import { AbstractSqlConnection } from '../connections/AbstractSqlConnection';
 import { Column, Index } from './DatabaseTable';
 import { ReferenceType } from '../entity';
 import { Utils } from '../utils';
-import { Connection } from '../connections';
+import { AbstractSqlConnection, Connection } from '../connections';
 
 export abstract class SchemaHelper {
 

tests/SchemaGenerator.test.ts

@@ -199,6 +199,14 @@ describe('SchemaGenerator', () => {
     await expect(generator.getUpdateSchemaSQL(false)).resolves.toMatchSnapshot('mysql-update-schema-drop-table');
     await generator.updateSchema();
 
+    // remove 1:1 relation
+    const fooBarMeta = meta.get('FooBar2');
+    const fooBazMeta = meta.get('FooBaz2');
+    delete fooBarMeta.properties.baz;
+    delete fooBazMeta.properties.bar;
+    await expect(generator.getUpdateSchemaSQL(false)).resolves.toMatchSnapshot('mysql-update-schema-drop-1:1');
+    await generator.updateSchema();
+
     await orm.close(true);
   });
 
@@ -509,6 +517,15 @@ describe('SchemaGenerator', () => {
     authorMeta.properties.favouriteBook = favouriteBookProp;
     await expect(generator.getUpdateSchemaSQL(false)).resolves.toMatchSnapshot('postgres-update-schema-add-column');
     await generator.updateSchema();
+    await expect(generator.getUpdateSchemaSQL(false)).resolves.toBe('');
+
+    // remove 1:1 relation
+    const fooBarMeta = meta.get('FooBar2');
+    const fooBazMeta = meta.get('FooBaz2');
+    delete fooBarMeta.properties.baz;
+    delete fooBazMeta.properties.bar;
+    await expect(generator.getUpdateSchemaSQL(false)).resolves.toMatchSnapshot('postgres-update-schema-drop-1:1');
+    await generator.updateSchema();
 
     meta.reset('Author2');
     meta.reset('NewTable');

tests/__snapshots__/SchemaGenerator.test.ts.snap

@@ -1124,6 +1124,8 @@ alter table \`author2\` add index \`author2_favourite_book_uuid_pk_index\`(\`fav
 
 alter table \`new_table\` add \`id\` int unsigned not null auto_increment primary key, add \`updated_at\` datetime(3) not null default current_timestamp(3);
 
+alter table \`author2\` add constraint \`author2_favourite_book_uuid_pk_foreign\` foreign key (\`favourite_book_uuid_pk\`) references \`book2\` (\`uuid_pk\`) on update no action on delete cascade;
+
 "
 `;
 
@@ -1150,6 +1152,15 @@ create table \`new_table\` (\`id\` int unsigned not null auto_increment primary
 "
 `;
 
+exports[`SchemaGenerator update schema [mysql]: mysql-update-schema-drop-1:1 1`] = `
+"alter table \`foo_bar2\` drop foreign key \`foo_bar2_baz_id_foreign\`;
+alter table \`foo_bar2\` drop index \`foo_bar2_baz_id_unique\`;
+alter table \`foo_bar2\` drop index \`foo_bar2_baz_id_index\`;
+alter table \`foo_bar2\` drop \`baz_id\`;
+
+"
+`;
+
 exports[`SchemaGenerator update schema [mysql]: mysql-update-schema-drop-column 1`] = `
 "alter table \`author2\` drop foreign key \`author2_favourite_book_uuid_pk_foreign\`;
 alter table \`author2\` drop index \`author2_favourite_book_uuid_pk_index\`;
@@ -1188,6 +1199,8 @@ exports[`SchemaGenerator update schema [postgres]: postgres-update-schema-add-co
 
 alter table \\"new_table\\" add column \\"id\\" serial primary key, add column \\"updated_at\\" timestamp(3) not null default current_timestamp(3);
 
+alter table \\"author2\\" add constraint \\"author2_favourite_book_uuid_pk_foreign\\" foreign key (\\"favourite_book_uuid_pk\\") references \\"book2\\" (\\"uuid_pk\\") on update no action on delete cascade;
+
 "
 `;
 
@@ -1215,6 +1228,14 @@ create table \\"new_table\\" (\\"id\\" serial primary key, \\"created_at\\" time
 "
 `;
 
+exports[`SchemaGenerator update schema [postgres]: postgres-update-schema-drop-1:1 1`] = `
+"alter table \\"foo_bar2\\" drop constraint \\"foo_bar2_baz_id_foreign\\";
+alter table \\"foo_bar2\\" drop constraint \\"foo_bar2_baz_id_unique\\";
+alter table \\"foo_bar2\\" drop column \\"baz_id\\";
+
+"
+`;
+
 exports[`SchemaGenerator update schema [postgres]: postgres-update-schema-drop-column 1`] = `
 "alter table \\"author2\\" drop constraint if exists \\"author2_name_check\\";
 alter table \\"author2\\" alter column \\"name\\" type int4 using (\\"name\\"::int4);