From 2f275816375a58c9ad136d5f02dc3508797152c2 Mon Sep 17 00:00:00 2001 From: David Recuenco Date: Fri, 18 Oct 2019 14:42:49 +0200 Subject: [PATCH] fix: migrations run in reverse order for mongodb (#4702) * update `loadExecutedMigrations` to get migrations sorted by id from database * add missing await statements when running mongodb migrations --- src/migration/MigrationExecutor.ts | 22 ++++++---- .../4697/entity/config.entity.ts | 16 +++++++ test/github-issues/4697/entity/item.entity.ts | 19 ++++++++ test/github-issues/4697/issue-4697.ts | 28 ++++++++++++ .../migration/1566560354098-UpdateContacts.ts | 30 +++++++++++++ .../migration/1567689639607-MergeConfigs.ts | 44 +++++++++++++++++++ 6 files changed, 151 insertions(+), 8 deletions(-) create mode 100644 test/github-issues/4697/entity/config.entity.ts create mode 100644 test/github-issues/4697/entity/item.entity.ts create mode 100644 test/github-issues/4697/issue-4697.ts create mode 100644 test/github-issues/4697/migration/1566560354098-UpdateContacts.ts create mode 100644 test/github-issues/4697/migration/1567689639607-MergeConfigs.ts diff --git a/src/migration/MigrationExecutor.ts b/src/migration/MigrationExecutor.ts index 71566bfa48..532977c90d 100644 --- a/src/migration/MigrationExecutor.ts +++ b/src/migration/MigrationExecutor.ts @@ -291,16 +291,22 @@ export class MigrationExecutor { } /** - * Loads all migrations that were executed and saved into the database. + * Loads all migrations that were executed and saved into the database (sorts by id). */ protected async loadExecutedMigrations(queryRunner: QueryRunner): Promise { if (this.connection.driver instanceof MongoDriver) { const mongoRunner = queryRunner as MongoQueryRunner; - return await mongoRunner.databaseConnection.db(this.connection.driver.database!).collection(this.migrationsTableName).find().toArray(); + return await mongoRunner.databaseConnection + .db(this.connection.driver.database!) + .collection(this.migrationsTableName) + .find() + .sort("_id", -1) + .toArray(); } else { const migrationsRaw: ObjectLiteral[] = await this.connection.manager .createQueryBuilder(queryRunner) .select() + .orderBy("id", "DESC") .from(this.migrationsTable, this.migrationsTableName) .getRawMany(); return migrationsRaw.map(migrationRaw => { @@ -335,10 +341,10 @@ export class MigrationExecutor { } /** - * Finds the latest migration (sorts by id) in the given array of migrations. + * Finds the latest migration in the given array of migrations. + * PRE: Migration array must be sorted by descending id. */ - protected getLatestExecutedMigration(migrations: Migration[]): Migration|undefined { - const sortedMigrations = migrations.map(migration => migration).sort((a, b) => ((a.id || 0) - (b.id || 0)) * -1); + protected getLatestExecutedMigration(sortedMigrations: Migration[]): Migration|undefined { return sortedMigrations.length > 0 ? sortedMigrations[0] : undefined; } @@ -354,9 +360,9 @@ export class MigrationExecutor { values["timestamp"] = migration.timestamp; values["name"] = migration.name; } - if (this.connection.driver instanceof MongoDriver) { + if (this.connection.driver instanceof MongoDriver) { const mongoRunner = queryRunner as MongoQueryRunner; - mongoRunner.databaseConnection.db(this.connection.driver.database!).collection(this.migrationsTableName).insert(values); + await mongoRunner.databaseConnection.db(this.connection.driver.database!).collection(this.migrationsTableName).insert(values); } else { const qb = queryRunner.manager.createQueryBuilder(); await qb.insert() @@ -382,7 +388,7 @@ export class MigrationExecutor { if (this.connection.driver instanceof MongoDriver) { const mongoRunner = queryRunner as MongoQueryRunner; - mongoRunner.databaseConnection.db(this.connection.driver.database!).collection(this.migrationsTableName).deleteOne(conditions); + await mongoRunner.databaseConnection.db(this.connection.driver.database!).collection(this.migrationsTableName).deleteOne(conditions); } else { const qb = queryRunner.manager.createQueryBuilder(); await qb.delete() diff --git a/test/github-issues/4697/entity/config.entity.ts b/test/github-issues/4697/entity/config.entity.ts new file mode 100644 index 0000000000..769b0232c4 --- /dev/null +++ b/test/github-issues/4697/entity/config.entity.ts @@ -0,0 +1,16 @@ +import {Entity, ObjectIdColumn, ObjectID, Column} from "../../../../src"; + +/** + * @deprecated use item config instead + */ +@Entity() +export class Config { + @ObjectIdColumn() + _id: ObjectID; + + @Column() + itemId: string; + + @Column({ type: "json" }) + data: any; +} diff --git a/test/github-issues/4697/entity/item.entity.ts b/test/github-issues/4697/entity/item.entity.ts new file mode 100644 index 0000000000..e62974812c --- /dev/null +++ b/test/github-issues/4697/entity/item.entity.ts @@ -0,0 +1,19 @@ +import {Entity, ObjectIdColumn, ObjectID, Column} from "../../../../src"; + +@Entity() +export class Item { + @ObjectIdColumn() + public _id: ObjectID; + + /** + * @deprecated use contacts instead + */ + @Column() + public contact?: string; + + @Column({ array: true }) + public contacts: Array; + + @Column({ type: "json" }) + config: any; +} diff --git a/test/github-issues/4697/issue-4697.ts b/test/github-issues/4697/issue-4697.ts new file mode 100644 index 0000000000..f17fe1b0d3 --- /dev/null +++ b/test/github-issues/4697/issue-4697.ts @@ -0,0 +1,28 @@ +import "reflect-metadata"; +import {createTestingConnections, closeTestingConnections, reloadTestingDatabases} from "../../utils/test-utils"; +import {Connection} from "../../../src/connection/Connection"; + +describe("github issues > #4697 Revert migrations running in reverse order.", () => { + + let connections: Connection[]; + before(async () => connections = await createTestingConnections({ + entities: [__dirname + "/entity/*{.js,.ts}"], + migrations: [__dirname + "/migration/*.js"], + enabledDrivers: ["mongodb"], + schemaCreate: true, + dropSchema: true, + })); + beforeEach(() => reloadTestingDatabases(connections)); + after(() => closeTestingConnections(connections)); + + it("should revert migrations in the right order", () => Promise.all(connections.map(async connection => { + await connection.runMigrations(); + + await connection.undoLastMigration(); + + const [lastMigration] = await connection.runMigrations(); + + lastMigration.should.have.property("timestamp", 1567689639607); + lastMigration.should.have.property("name", "MergeConfigs1567689639607"); + }))); +}); diff --git a/test/github-issues/4697/migration/1566560354098-UpdateContacts.ts b/test/github-issues/4697/migration/1566560354098-UpdateContacts.ts new file mode 100644 index 0000000000..794f3e1016 --- /dev/null +++ b/test/github-issues/4697/migration/1566560354098-UpdateContacts.ts @@ -0,0 +1,30 @@ +import {MigrationInterface, QueryRunner} from "../../../../src"; +import {Item} from "../entity/item.entity"; + +export class UpdateContacts1566560354098 implements MigrationInterface { + + public async up({connection}: QueryRunner): Promise { + const repo = connection.getMongoRepository(Item); + const items: Array = await repo.find(); + + items.forEach((item) => { + if (!item.contacts) { + item.contacts = [item.contact || ""]; + } + }); + + await repo.save(items); + } + + public async down({connection}: QueryRunner): Promise { + const repo = connection.getMongoRepository(Item); + const items: Array = await repo.find(); + + items.forEach((item) => { + item.contact = item.contacts[0]; + }); + + await repo.save(items); + } + +} diff --git a/test/github-issues/4697/migration/1567689639607-MergeConfigs.ts b/test/github-issues/4697/migration/1567689639607-MergeConfigs.ts new file mode 100644 index 0000000000..d9f3e40782 --- /dev/null +++ b/test/github-issues/4697/migration/1567689639607-MergeConfigs.ts @@ -0,0 +1,44 @@ +import {MigrationInterface, QueryRunner} from "../../../../src"; +import {Item} from "../entity/item.entity"; +import {Config} from "../entity/config.entity"; + +export class MergeConfigs1567689639607 implements MigrationInterface { + + public async up({connection}: QueryRunner): Promise { + const itemRepository = connection.getMongoRepository(Item); + const configRepository = connection.getMongoRepository(Config); + + const configs = await configRepository.find(); + + await Promise.all(configs.map(async ({itemId, data}) => { + const item = await itemRepository.findOne(itemId); + + if (item) { + item.config = data; + + return itemRepository.save(item); + } else { + console.warn(`No item found with id: ${ itemId }. Ignoring.`); + + return null; + } + })); + } + + public async down({connection}: QueryRunner): Promise { + const itemRepository = connection.getRepository(Item); + const configRepository = connection.getRepository(Config); + + const items = await itemRepository.find(); + + await Promise.all(items.map((item) => { + const config = new Config(); + + config.itemId = item._id.toString(); + config.data = item.config; + + return configRepository.save(config); + })); + } + +}