fix: disable SQLite FK checks in synchronize / migrations (#7922)

* fix: disable SQLite FK checks in synchronize / migrations * test: add case for changing column in a table with foreign keys
typeorm · Feb 16, 2022 · f24822e · f24822e
1 parent d0f32b3
commit f24822e
Show file tree

Hide file tree

Showing 16 changed files with 199 additions and 3 deletions.
diff --git a/src/driver/better-sqlite3/BetterSqlite3QueryRunner.ts b/src/driver/better-sqlite3/BetterSqlite3QueryRunner.ts
@@ -58,6 +58,20 @@ export class BetterSqlite3QueryRunner extends AbstractSqliteQueryRunner {
         }
     }
 
+    /**
+     * Called before migrations are run.
+     */
+    async beforeMigration(): Promise<void> {
+        await this.query(`PRAGMA foreign_keys = OFF`);
+    }
+
+    /**
+     * Called after migrations are run.
+     */
+    async afterMigration(): Promise<void> {
+        await this.query(`PRAGMA foreign_keys = ON`);
+    }
+
     /**
      * Executes a given SQL query.
      */

diff --git a/src/driver/capacitor/CapacitorQueryRunner.ts b/src/driver/capacitor/CapacitorQueryRunner.ts
@@ -26,6 +26,20 @@ export class CapacitorQueryRunner extends AbstractSqliteQueryRunner {
         this.broadcaster = new Broadcaster(this);
     }
 
+    /**
+     * Called before migrations are run.
+     */
+    async beforeMigration(): Promise<void> {
+        await this.query(`PRAGMA foreign_keys = OFF`);
+    }
+
+    /**
+     * Called after migrations are run.
+     */
+    async afterMigration(): Promise<void> {
+        await this.query(`PRAGMA foreign_keys = ON`);
+    }
+
     async executeSet(set: { statement: string; values?: any[] }[]) {
         if (this.isReleased) throw new QueryRunnerAlreadyReleasedError();
 

diff --git a/src/driver/cordova/CordovaQueryRunner.ts b/src/driver/cordova/CordovaQueryRunner.ts
@@ -28,6 +28,20 @@ export class CordovaQueryRunner extends AbstractSqliteQueryRunner {
         this.broadcaster = new Broadcaster(this);
     }
 
+    /**
+     * Called before migrations are run.
+     */
+    async beforeMigration(): Promise<void> {
+        await this.query(`PRAGMA foreign_keys = OFF`);
+    }
+
+    /**
+     * Called after migrations are run.
+     */
+    async afterMigration(): Promise<void> {
+        await this.query(`PRAGMA foreign_keys = ON`);
+    }
+
     /**
      * Executes a given SQL query.
      */

diff --git a/src/driver/expo/ExpoQueryRunner.ts b/src/driver/expo/ExpoQueryRunner.ts
@@ -113,6 +113,20 @@ export class ExpoQueryRunner extends AbstractSqliteQueryRunner {
         await this.broadcaster.broadcast('AfterTransactionRollback');
     }
 
+    /**
+     * Called before migrations are run.
+     */
+    async beforeMigration(): Promise<void> {
+        await this.query(`PRAGMA foreign_keys = OFF`);
+    }
+
+    /**
+     * Called after migrations are run.
+     */
+    async afterMigration(): Promise<void> {
+        await this.query(`PRAGMA foreign_keys = ON`);
+    }
+
     /**
      * Executes a given SQL query.
      */

diff --git a/src/driver/mongodb/MongoQueryRunner.ts b/src/driver/mongodb/MongoQueryRunner.ts
@@ -121,6 +121,20 @@ export class MongoQueryRunner implements QueryRunner {
     // Public Methods
     // -------------------------------------------------------------------------
 
+    /**
+     * Called before migrations are run.
+     */
+    async beforeMigration(): Promise<void> {
+        // Do nothing
+    }
+
+    /**
+     * Called after migrations are run.
+     */
+    async afterMigration(): Promise<void> {
+        // Do nothing
+    }
+
     /**
      * Creates a cursor for a query that can be used to iterate over results from MongoDB.
      */

diff --git a/src/driver/nativescript/NativescriptQueryRunner.ts b/src/driver/nativescript/NativescriptQueryRunner.ts
@@ -27,6 +27,20 @@ export class NativescriptQueryRunner extends AbstractSqliteQueryRunner {
         this.broadcaster = new Broadcaster(this);
     }
 
+    /**
+     * Called before migrations are run.
+     */
+    async beforeMigration(): Promise<void> {
+        await this.query(`PRAGMA foreign_keys = OFF`);
+    }
+
+    /**
+     * Called after migrations are run.
+     */
+    async afterMigration(): Promise<void> {
+        await this.query(`PRAGMA foreign_keys = ON`);
+    }
+
     /**
      * Executes a given SQL query.
      */

diff --git a/src/driver/react-native/ReactNativeQueryRunner.ts b/src/driver/react-native/ReactNativeQueryRunner.ts
@@ -27,6 +27,20 @@ export class ReactNativeQueryRunner extends AbstractSqliteQueryRunner {
         this.broadcaster = new Broadcaster(this);
     }
 
+    /**
+     * Called before migrations are run.
+     */
+    async beforeMigration(): Promise<void> {
+        await this.query(`PRAGMA foreign_keys = OFF`);
+    }
+
+    /**
+     * Called after migrations are run.
+     */
+    async afterMigration(): Promise<void> {
+        await this.query(`PRAGMA foreign_keys = ON`);
+    }
+
     /**
      * Executes a given SQL query.
      */

diff --git a/src/driver/sqlite/SqliteQueryRunner.ts b/src/driver/sqlite/SqliteQueryRunner.ts
@@ -31,6 +31,20 @@ export class SqliteQueryRunner extends AbstractSqliteQueryRunner {
         this.broadcaster = new Broadcaster(this);
     }
 
+    /**
+     * Called before migrations are run.
+     */
+    async beforeMigration(): Promise<void> {
+        await this.query(`PRAGMA foreign_keys = OFF`);
+    }
+
+    /**
+     * Called after migrations are run.
+     */
+    async afterMigration(): Promise<void> {
+        await this.query(`PRAGMA foreign_keys = ON`);
+    }
+
     /**
      * Executes a given SQL query.
      */

diff --git a/src/driver/sqljs/SqljsQueryRunner.ts b/src/driver/sqljs/SqljsQueryRunner.ts
@@ -35,6 +35,20 @@ export class SqljsQueryRunner extends AbstractSqliteQueryRunner {
     // Public methods
     // -------------------------------------------------------------------------
 
+    /**
+     * Called before migrations are run.
+     */
+    async beforeMigration(): Promise<void> {
+        await this.query(`PRAGMA foreign_keys = OFF`);
+    }
+
+    /**
+     * Called after migrations are run.
+     */
+    async afterMigration(): Promise<void> {
+        await this.query(`PRAGMA foreign_keys = ON`);
+    }
+
     private async flush() {
         if (this.isDirty) {
             await this.driver.autoSave();

diff --git a/src/migration/MigrationExecutor.ts b/src/migration/MigrationExecutor.ts
@@ -61,7 +61,9 @@ export class MigrationExecutor {
     public async executeMigration(migration: Migration): Promise<Migration> {
         return this.withQueryRunner(async (queryRunner) => {
             await this.createMigrationsTableIfNotExist(queryRunner);
+            await queryRunner.beforeMigration();
             await (migration.instance as any).up(queryRunner);
+            await queryRunner.afterMigration();
             await this.insertExecutedMigration(queryRunner, migration);
 
             return migration;
@@ -309,7 +311,10 @@ export class MigrationExecutor {
         }
 
         try {
+            await queryRunner.beforeMigration();
             await migrationToRevert.instance!.down(queryRunner);
+            await queryRunner.afterMigration();
+
             await this.deleteExecutedMigration(queryRunner, migrationToRevert);
             this.connection.logger.logSchemaBuild(`Migration ${migrationToRevert.name} has been reverted successfully.`);
 

diff --git a/src/query-runner/BaseQueryRunner.ts b/src/query-runner/BaseQueryRunner.ts
@@ -112,6 +112,20 @@ export abstract class BaseQueryRunner {
     // Public Methods
     // -------------------------------------------------------------------------
 
+    /**
+     * Called before migrations are run.
+     */
+    async beforeMigration(): Promise<void> {
+        // Do nothing
+    }
+
+    /**
+     * Called after migrations are run.
+     */
+    async afterMigration(): Promise<void> {
+        // Do nothing
+    }
+
     /**
      * Loads given table's data from the database.
      */

diff --git a/src/query-runner/QueryRunner.ts b/src/query-runner/QueryRunner.ts
@@ -73,6 +73,16 @@ export interface QueryRunner {
      */
     connect(): Promise<any>;
 
+    /**
+     * Called before migrations are run.
+     */
+    beforeMigration(): Promise<void>;
+
+    /**
+     * Called after migrations are run.
+     */
+    afterMigration(): Promise<void>;
+
     /**
      * Releases used database connection.
      * You cannot use query runner methods after connection is released.

diff --git a/src/schema-builder/RdbmsSchemaBuilder.ts b/src/schema-builder/RdbmsSchemaBuilder.ts
@@ -72,6 +72,8 @@ export class RdbmsSchemaBuilder implements SchemaBuilder {
             this.connection.options.migrationsTransactionMode !== "none"
         );
 
+        await this.queryRunner.beforeMigration();
+
         if (isUsingTransactions) {
             await this.queryRunner.startTransaction();
         }
@@ -104,6 +106,9 @@ export class RdbmsSchemaBuilder implements SchemaBuilder {
             throw error;
 
         } finally {
+
+            await this.queryRunner.afterMigration();
+
             await this.queryRunner.release();
         }
     }

diff --git a/test/functional/schema-builder/change-column.ts b/test/functional/schema-builder/change-column.ts
@@ -297,4 +297,40 @@ describe("schema builder > change column", () => {
         expect(teacherTableB!.findColumnByName("id")!.comment).to.be.undefined;
 
     })));
+
+    it("should correctly change column type when FK relationships impact it", () => Promise.all(connections.map(async connection => {
+        await connection.getRepository(Post)
+            .insert({
+                id: 1234,
+                version: '5',
+                text: 'a',
+                tag: 'b',
+                likesCount: 45
+            });
+
+        const post = await connection.getRepository(Post).findOneOrFail(1234);
+
+        await connection.getRepository(PostVersion)
+            .insert({
+                id: 1,
+                post,
+                details: 'Example'
+            })
+
+        const postMetadata = connection.getMetadata(Post);
+        const nameColumn = postMetadata.findColumnWithPropertyName("name")!;
+        nameColumn.length = "500";
+
+        await connection.synchronize();
+
+        const queryRunner = connection.createQueryRunner();
+        const postVersionTable = await queryRunner.getTable("post_version");
+        await queryRunner.release();
+
+        postVersionTable!.foreignKeys.length.should.be.equal(1);
+
+        // revert changes
+        nameColumn.length = "255";
+    })));
+
 });
diff --git a/test/functional/schema-builder/entity/Post.ts b/test/functional/schema-builder/entity/Post.ts
@@ -20,7 +20,7 @@ export class Post {
     @Column({ default: "My post" })
     name: string;
 
-    @Column()
+    @Column({ nullable: true })
     text: string;
 
     @Column()

diff --git a/test/functional/sqljs/auto-save.ts b/test/functional/sqljs/auto-save.ts
@@ -34,7 +34,7 @@ describe("sqljs driver > autosave", () => {
         await connection.createQueryBuilder().insert().into(Post).values(posts).execute();
         await connection.createQueryBuilder().update(Post).set({title: "Many posts"}).execute();
         await connection.createQueryBuilder().delete().from(Post).where("title = ?", {title: "third post"}).execute();
-        
+
         const repository = connection.getRepository(Post);
         let post = new Post();
         post.title = "A post";
@@ -52,7 +52,7 @@ describe("sqljs driver > autosave", () => {
 
         await connection.close();
 
-        expect(saves).to.be.equal(7);
+        expect(saves).to.be.equal(8);
     })));
 });