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

feat: allow skipping re-runs #342

Merged
merged 4 commits into from
Oct 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ module.exports = {
'prefer-const': 'error',
'no-console': 'warn',
'no-var': 'error',
'no-shadow': 'error',
strict: ['error', 'never'],

'no-await-in-loop': 'off',
Expand All @@ -38,6 +37,7 @@ module.exports = {
'@typescript-eslint/ban-ts-comment': ['warn', { 'ts-expect-error': false }],
'@typescript-eslint/prefer-function-type': 'error',
'@typescript-eslint/restrict-template-expressions': 'error',
'@typescript-eslint/no-shadow': 'error',
'@typescript-eslint/no-unused-vars': [
'error',
{
Expand Down Expand Up @@ -75,6 +75,7 @@ module.exports = {
'@typescript-eslint/comma-dangle': 'off',
'@typescript-eslint/indent': 'off',
'@typescript-eslint/quotes': 'off',
'@typescript-eslint/semi': 'off',
mmkal marked this conversation as resolved.
Show resolved Hide resolved

// covered by `@typescript-eslint/no-unsued-vars`
'no-unused-vars': 'off',
Expand Down
8 changes: 5 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -344,9 +344,11 @@ The options for `Umguz#up` and `Umzug#down` have changed:
- `umzug.down({ migrations: ['m1', 'm2'] })` is still valid but the shorthand `umzug.down(['m1', 'm2'])` has been removed.
- `umzug.up({ migrations: ['m1', 'already-run'] })` will throw an error, if `already-run` is not found in the list of pending migrations.
- `umzug.down({ migrations: ['m1', 'has-not-been-run'] })` will throw an error, if `has-not-been-run` is not found in the list of executed migrations.
- `umzug.up({ migrations: ['m1', 'm2'], force: true })` will re-apply migrations `m1` and `m2` even if they've already been run.
- `umzug.down({ migrations: ['m1', 'm2'], force: true })` will "revert" migrations `m1` and `m2` even if they've never been run.
- `umzug.up({ migrations: ['m1', 'does-not-exist', 'm2'] })` will throw an error if the migration name is not found. Note that the error will be thrown and no migrations run unless _all_ migration names are found - whether or not `force: true` is added.
- `umzug.up({ migrations: ['m1', 'm2'], rerun: 'ALLOW' })` will re-apply migrations `m1` and `m2` even if they've already been run.
- `umzug.up({ migrations: ['m1', 'm2'], rerun: 'SKIP' })` will skip migrations `m1` and `m2` if they've already been run.
- `umzug.down({ migrations: ['m1', 'm2'], rerun: 'ALLOW' })` will "revert" migrations `m1` and `m2` even if they've never been run.
- `umzug.down({ migrations: ['m1', 'm2'], rerun: 'SKIP' })` will skip reverting migrations `m1` and `m2` if they haven't been run or are already reverted.
- `umzug.up({ migrations: ['m1', 'does-not-exist', 'm2'] })` will throw an error if the migration name is not found. Note that the error will be thrown and no migrations run unless _all_ migration names are found - whether or not `rerun: 'ALLOW'` is added.

The `context` parameter replaces `params`, and is passed in as a property to migration functions as an options object, alongs side `name` and `path`. This means the signature for migrations, which in v2 was `(context) => Promise<void>`, has changed slightly in v3, to `({ name, path, context }) => Promise<void>`. The `resolve` function can also be used to upgrade your umzug version to v3 when you have existing v2-compatible migrations:

Expand Down
2 changes: 1 addition & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
export { Umzug } from './umzug';
export * from './umzug';
export * from './storage';
45 changes: 34 additions & 11 deletions src/umzug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,17 @@ export type InputMigrations<T> =
/** A function which takes a migration name, path and context, and returns an object with `up` and `down` functions */
export type Resolver<T> = (params: { path: string; name: string; context: T }) => RunnableMigration;

export const RerunBehavior = {
/** Hard error if an up migration that has already been run, or a down migration that hasn't, is encountered */
THROW: 'THROW',
/** Silently skip up migrations that have already been run, or down migrations that haven't */
SKIP: 'SKIP',
/** Re-run up migrations that have already been run, or down migrations that haven't */
ALLOW: 'ALLOW',
} as const;

export type RerunBehavior = keyof typeof RerunBehavior;

export type MigrateUpOptions =
| {
/** If specified, migrations up to and including this name will be run. Otherwise, all pending migrations will be run */
Expand All @@ -71,14 +82,14 @@ export type MigrateUpOptions =
migrations?: never;

/** Should not be specified with `to` */
force?: never;
rerun?: never;
}
| {
/** If specified, only the migrations with these names migrations will be run. An error will be thrown if any of the names are not found in the list of available migrations */
migrations: string[];

/** Allow re-applying already-executed migrations. Use with caution. */
force?: boolean;
/** What to do if a migration that has already been run is explicitly specified. Default is `THROW`. */
rerun?: RerunBehavior;

/** Should not be specified with `migrations` */
to?: never;
Expand All @@ -93,7 +104,7 @@ export type MigrateDownOptions =
migrations?: never;

/** Should not be specified with `to` */
force?: never;
rerun?: never;
}
| {
/**
Expand All @@ -102,8 +113,8 @@ export type MigrateDownOptions =
*/
migrations: string[];

/** Allow reverting migrations which have not been run yet. Use with caution. */
force?: boolean;
/** What to do if a migration that has not been run is explicitly specified. Default is `THROW`. */
rerun?: RerunBehavior;

/** Should not be specified with `migrations` */
to?: never;
Expand Down Expand Up @@ -219,16 +230,22 @@ export class Umzug<Ctx> extends EventEmitter {

/**
* Apply migrations. By default, runs all pending migrations.
* @see MigrateUpOptions for other use cases using `to`, `migrations` and `force`.
* @see MigrateUpOptions for other use cases using `to`, `migrations` and `rerun`.
*/
async up(options: MigrateUpOptions = {}): Promise<void> {
const eligibleMigrations = async () => {
if (options.migrations && options.force) {
// `force` means the specified migrations should be run even if they've run before - so get all migrations, not just pending
if (options.migrations && options.rerun === RerunBehavior.ALLOW) {
// Allow rerun means the specified migrations should be run even if they've run before - so get all migrations, not just pending
const list = await this.migrations();
return this.findMigrations(list, options.migrations);
}

if (options.migrations && options.rerun === RerunBehavior.SKIP) {
const executedNames = new Set((await this._executed()).map(m => m.name));
const filteredMigrations = options.migrations.filter(m => !executedNames.has(m));
return this.findMigrations(await this.migrations(), filteredMigrations);
}

if (options.migrations) {
return this.findMigrations(await this._pending(), options.migrations);
}
Expand Down Expand Up @@ -262,15 +279,21 @@ export class Umzug<Ctx> extends EventEmitter {

/**
* Revert migrations. By default, the last executed migration is reverted.
* @see MigrateDownOptions for other use cases using `to`, `migrations` and `force`.
* @see MigrateDownOptions for other use cases using `to`, `migrations` and `rerun`.
*/
async down(options: MigrateDownOptions = {}): Promise<void> {
const eligibleMigrations = async () => {
if (options.migrations && options.force) {
if (options.migrations && options.rerun === RerunBehavior.ALLOW) {
const list = await this.migrations();
return this.findMigrations(list, options.migrations);
}

if (options.migrations && options.rerun === RerunBehavior.SKIP) {
const pendingNames = new Set((await this._pending()).map(m => m.name));
const filteredMigrations = options.migrations.filter(m => !pendingNames.has(m));
return this.findMigrations(await this.migrations(), filteredMigrations);
}

if (options.migrations) {
return this.findMigrations(await this._executed(), options.migrations);
}
Expand Down
72 changes: 61 additions & 11 deletions test/umzug.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Umzug } from '../src/umzug';
import { RerunBehavior, Umzug } from '../src';
import { memoryStorage } from '../src';
import * as path from 'path';
import { fsSyncer } from 'fs-syncer';
Expand Down Expand Up @@ -141,19 +141,25 @@ describe('alternate migration inputs', () => {
/Couldn't find migration to apply with name "m2"/
);

await umzug.up({ migrations: ['m2', 'm4'], force: true });
// rerun behavior 'SKIP' silently ignores already-executed migrations
await umzug.up({ migrations: ['m2', 'm4'], rerun: RerunBehavior.SKIP });
expect(names(await umzug.executed())).toEqual(['m2', 'm4']);
expect(spy.mock.calls).toEqual([['up-m2'], ['up-m4']]);

// rerun behavior 'ALLOW' runs already-executed migrations again
await umzug.up({ migrations: ['m2', 'm4'], rerun: RerunBehavior.ALLOW });

expect(names(await umzug.executed())).toEqual(['m2', 'm4']);
expect(spy.mock.calls).toEqual([['up-m2'], ['up-m4'], ['up-m2'], ['up-m4']]);

// you can use migration names to run migrations in the "wrong" order:
await umzug.up({ migrations: ['m5', 'm3'], force: true });
await umzug.up({ migrations: ['m5', 'm3'], rerun: RerunBehavior.ALLOW });

expect(names(await umzug.executed())).toEqual(['m2', 'm3', 'm4', 'm5']);
expect(spy.mock.calls).toEqual([['up-m2'], ['up-m4'], ['up-m2'], ['up-m4'], ['up-m5'], ['up-m3']]);

// invalid migration names result in an error:
await expect(umzug.up({ migrations: ['m1', 'typo'], force: true })).rejects.toThrowError(
await expect(umzug.up({ migrations: ['m1', 'typo'], rerun: RerunBehavior.ALLOW })).rejects.toThrowError(
/Couldn't find migration to apply with name "typo"/
);
// even though m1 _is_ a valid name, it shouldn't have been called - all listed migrations are verified before running any
Expand All @@ -170,11 +176,16 @@ describe('alternate migration inputs', () => {
expect(names(await umzug.executed())).toEqual(['m2', 'm4', 'm6']);
expect(spy.mock.calls).toEqual([['down-m1'], ['down-m3'], ['down-m5'], ['down-m7']]);

// rerun behavior 'SKIP' ignores down migrations that have already been reverted
await umzug.down({ migrations: ['m1', 'm3', 'm5', 'm7'], rerun: RerunBehavior.SKIP });
expect(names(await umzug.executed())).toEqual(['m2', 'm4', 'm6']);
expect(spy.mock.calls).toEqual([['down-m1'], ['down-m3'], ['down-m5'], ['down-m7']]);

await expect(umzug.down({ migrations: ['m1', 'm3', 'm5', 'm7'] })).rejects.toThrowError(
/Couldn't find migration to apply with name "m1"/
);

await umzug.down({ migrations: ['m1', 'm3', 'm5', 'm7'], force: true });
await umzug.down({ migrations: ['m1', 'm3', 'm5', 'm7'], rerun: RerunBehavior.ALLOW });
expect(names(await umzug.executed())).toEqual(['m2', 'm4', 'm6']);
expect(spy.mock.calls).toEqual([
['down-m1'],
Expand Down Expand Up @@ -388,24 +399,63 @@ describe('types', () => {
});
});

test('rerun behavior is a map of its keys to themselves', () => {
expectTypeOf(RerunBehavior).toEqualTypeOf<{ readonly [K in RerunBehavior]: K }>();
});
Comment on lines +402 to +404
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, you've read my mind. I almost mentioned this in my last review, but I ended up thinking it would be too much hassle for you 😅 but you did it, that's even cooler.


test('up and down', () => {
expectTypeOf(Umzug).instance.toHaveProperty('up').toBeCallableWith({ to: 'migration123' });
const up = expectTypeOf(Umzug).instance.toHaveProperty('up');
const down = expectTypeOf(Umzug).instance.toHaveProperty('down');

up.toBeCallableWith({ to: 'migration123' });

expectTypeOf(Umzug).instance.toHaveProperty('down').toBeCallableWith({ to: 'migration123' });
up.toBeCallableWith({ migrations: ['m1'], rerun: RerunBehavior.ALLOW });
up.toBeCallableWith({ migrations: ['m1'], rerun: RerunBehavior.SKIP });
up.toBeCallableWith({ migrations: ['m1'], rerun: RerunBehavior.THROW });

expectTypeOf(Umzug).instance.toHaveProperty('down').toBeCallableWith({ to: 0 });
up.toBeCallableWith({ migrations: ['m1'], rerun: 'ALLOW' });
up.toBeCallableWith({ migrations: ['m1'], rerun: 'SKIP' });
up.toBeCallableWith({ migrations: ['m1'], rerun: 'THROW' });

// don't allow general strings for rerun behavior
// @ts-expect-error
up.toBeCallableWith({ migrations: ['m1'], rerun: 'xyztypo' });

// rerun must be specified with `migrations`
// @ts-expect-error
up.toBeCallableWith({ rerun: 'xyztypo' });

// can't go up "to" 0
// @ts-expect-error
expectTypeOf(Umzug).instance.toHaveProperty('up').toBeCallableWith({ to: 0 });
up.toBeCallableWith({ to: 0 });

down.toBeCallableWith({ to: 'migration123' });

down.toBeCallableWith({ migrations: ['m1'], rerun: RerunBehavior.ALLOW });
down.toBeCallableWith({ migrations: ['m1'], rerun: RerunBehavior.SKIP });
down.toBeCallableWith({ migrations: ['m1'], rerun: RerunBehavior.THROW });

down.toBeCallableWith({ migrations: ['m1'], rerun: 'ALLOW' });
down.toBeCallableWith({ migrations: ['m1'], rerun: 'SKIP' });
down.toBeCallableWith({ migrations: ['m1'], rerun: 'THROW' });

// don't allow general strings for rerun behavior
// @ts-expect-error
down.toBeCallableWith({ migrations: ['m1'], rerun: 'xyztypo' });

// rerun can only be specified with `migrations`
// @ts-expect-error
down.toBeCallableWith({ rerun: 'xyztypo' });

down.toBeCallableWith({ to: 0 });

// `{ to: 0 }` is a special case. `{ to: 1 }` shouldn't be allowed:

// @ts-expect-error
expectTypeOf(Umzug).instance.toHaveProperty('down').toBeCallableWith({ to: 1 });
down.toBeCallableWith({ to: 1 });

// @ts-expect-error
expectTypeOf(Umzug).instance.toHaveProperty('up').toBeCallableWith({ to: 1 });
up.toBeCallableWith({ to: 1 });
});

test('pending', () => {
Expand Down