Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#843 - Use JS sort method to sort migrations by name #844

Merged
31 changes: 28 additions & 3 deletions src/migration/migrator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ export class Migrator {
.withPlugin(this.#schemaPlugin)
.selectFrom(this.#migrationTable)
.select(['name', 'timestamp'])
.$narrowType<{ name: string; timestamp: string }>()
.execute()
: []

Expand Down Expand Up @@ -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)
)
}

DavesBorges marked this conversation as resolved.
Show resolved Hide resolved
#ensureNoMissingMigrations(
Expand Down Expand Up @@ -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
}

/**
Expand Down
2 changes: 1 addition & 1 deletion test/node/src/camel-case.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }))
Expand Down
89 changes: 89 additions & 0 deletions test/node/src/migration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'])
})
})
})

Expand Down Expand Up @@ -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') {
Expand Down
Loading