From bd5ded0d85361a0080f41abdc439906c24dda942 Mon Sep 17 00:00:00 2001 From: DavesBorges Date: Sun, 5 Jan 2025 01:26:07 +0000 Subject: [PATCH] #843 - Use JS sort method to sort migrations by name (#844) Co-authored-by: DavesBorges <66228145+DavesBorges@users.noreply.github.com> Co-authored-by: Igal Klebanov --- src/migration/migrator.ts | 31 +++++++++-- test/node/src/camel-case.test.ts | 2 +- test/node/src/migration.test.ts | 89 ++++++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+), 4 deletions(-) diff --git a/src/migration/migrator.ts b/src/migration/migrator.ts index 7c923ace8..c8719713b 100644 --- a/src/migration/migrator.ts +++ b/src/migration/migrator.ts @@ -83,6 +83,7 @@ export class Migrator { .withPlugin(this.#schemaPlugin) .selectFrom(this.#migrationTable) .select(['name', 'timestamp']) + .$narrowType<{ name: string; timestamp: string }>() .execute() : [] @@ -577,11 +578,27 @@ export class Migrator { const executedMigrations = await db .withPlugin(this.#schemaPlugin) .selectFrom(this.#migrationTable) - .select('name') - .orderBy(['timestamp', 'name']) + .select(['name', 'timestamp']) + .$narrowType<{ name: string; timestamp: string }>() .execute() - return executedMigrations.map((it) => it.name) + const nameComparator = + this.#props.nameComparator || ((a, b) => a.localeCompare(b)) + + return ( + executedMigrations + // https://github.com/kysely-org/kysely/issues/843 + .sort((a, b) => { + if (a.timestamp === b.timestamp) { + return nameComparator(a.name, b.name) + } + + return ( + new Date(a.timestamp).getTime() - new Date(b.timestamp).getTime() + ) + }) + .map((it) => it.name) + ) } #ensureNoMissingMigrations( @@ -797,6 +814,14 @@ export interface MigratorProps { * order. */ readonly allowUnorderedMigrations?: boolean + + /** + * A function that compares migration names, used when sorting migrations in + * ascending order. + * + * Default is `name0.localeCompare(name1)`. + */ + readonly nameComparator?: (name0: string, name1: string) => number } /** diff --git a/test/node/src/camel-case.test.ts b/test/node/src/camel-case.test.ts index 37bc49a8c..27719fea2 100644 --- a/test/node/src/camel-case.test.ts +++ b/test/node/src/camel-case.test.ts @@ -313,7 +313,7 @@ for (const dialect of DIALECTS) { }) }) - it.only('should respect `underscoreBeforeDigits` and not add a second underscore in a nested query', async () => { + it('should respect `underscoreBeforeDigits` and not add a second underscore in a nested query', async () => { const db = camelDb .withoutPlugins() .withPlugin(new CamelCasePlugin({ underscoreBeforeDigits: true })) diff --git a/test/node/src/migration.test.ts b/test/node/src/migration.test.ts index 4fca280ff..e612ad791 100644 --- a/test/node/src/migration.test.ts +++ b/test/node/src/migration.test.ts @@ -597,6 +597,57 @@ for (const dialect of DIALECTS) { expect(executedUpMethods2).to.eql([]) expect(executedDownMethods2).to.eql(['migration5', 'migration3']) }) + + it('should migrate down if allowUnorderedMigrations is enabled and migration names are not in order', async () => { + const [migrator1, executedUpMethods1] = createMigrations( + ['migration1', 'migration3'], + { allowUnorderedMigrations: true }, + ) + + const { results: results1 } = await migrator1.migrateToLatest() + + const [migrator2, executedUpMethods2] = createMigrations( + ['migration1', 'migration2', 'migration3'], + { + allowUnorderedMigrations: true, + }, + ) + + const { results: results2 } = await migrator2.migrateToLatest() + + expect(results1).to.eql([ + { migrationName: 'migration1', direction: 'Up', status: 'Success' }, + { migrationName: 'migration3', direction: 'Up', status: 'Success' }, + ]) + + expect(results2).to.eql([ + { + migrationName: 'migration2', + direction: 'Up', + status: 'Success', + }, + ]) + + expect(executedUpMethods1).to.eql(['migration1', 'migration3']) + expect(executedUpMethods2).to.eql(['migration2']) + + const [migrator3, executedUpMethods3, executedDownMethods3] = + createMigrations(['migration1', 'migration2', 'migration3'], { + allowUnorderedMigrations: true, + }) + + const { results: results3 } = await migrator3.migrateDown() + expect(results3).to.eql([ + { + migrationName: 'migration2', + direction: 'Down', + status: 'Success', + }, + ]) + + expect(executedUpMethods3).to.eql([]) + expect(executedDownMethods3).to.eql(['migration2']) + }) }) }) @@ -792,6 +843,44 @@ for (const dialect of DIALECTS) { 'migration1', ]) }) + + describe('Migrate up should work when timestamps are equal', () => { + let originalToIsoString: typeof Date.prototype.toISOString + + before(() => { + originalToIsoString = Date.prototype.toISOString + const defaultDateIsoString = new Date(2024, 0, 11).toISOString() + Date.prototype.toISOString = () => defaultDateIsoString + }) + + after(() => { + Date.prototype.toISOString = originalToIsoString + }) + + it('should use the same ordering strategy for migrations for both not executed migrations and executed migrations', async () => { + const [migrator1, executedUpMethods1] = createMigrations([ + '2024-01-01-create-table', + '2024-01-01.2-update-table', + ]) + + await migrator1.migrateToLatest() + + const [migrator2, executedUpMethods2] = createMigrations([ + '2024-01-01-create-table', + '2024-01-01.2-update-table', + ]) + + const { results: results2, error } = await migrator2.migrateToLatest() + expect(error).to.be.undefined + expect(results2).to.eql([]) + + expect(executedUpMethods1).to.eql([ + '2024-01-01-create-table', + '2024-01-01.2-update-table', + ]) + expect(executedUpMethods2).to.eql([]) + }) + }) }) if (dialect === 'postgres') {