From 1658a46820714978e20158eed53c369d478c9ca3 Mon Sep 17 00:00:00 2001 From: Philip Hartmann <61052948+philip-hartmann@users.noreply.github.com> Date: Tue, 14 Nov 2023 05:35:09 +0100 Subject: [PATCH] Add --strict flag (#441) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add --strict-order flag * Limit --strict-order flag to appropriate verbs Limits the ´--strict-order´ flag to the up, migrate and status verb. * Rename --strict-flag to --strict * Throw error on strict numerical order conflict * Identify latest applied migration only in strict mode * Change wording of strict mode error message * Update usage text * Check pending migrations version in `Migrate()` instead of `FindMigrations()` * Change wording of strict mode error message * Check only pending migrations in `Migrate()` * Check only first pending migration FindMigrations returns a sorted list so we only need to check the first pending migration. * Abort migration if no pending migrations files exist * Abort migration with no error if no pending migrations files exist A return error is not in line with tests. * Derive highest applied version number from migration list * Undo refactor to find applied migrations * Dump schema even if no migrations are applied * Fix backwards-incompatible breaking condition * Add tests for strict mode * Fix tests * Remove whitespaces --------- Co-authored-by: Adrian Macneil --- README.md | 1 + main.go | 13 ++++++++ pkg/dbmate/db.go | 25 +++++++++++--- pkg/dbmate/db_test.go | 76 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 110 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 36fd2871..0b25094e 100644 --- a/README.md +++ b/README.md @@ -142,6 +142,7 @@ The following options are available with all commands. You must use command line - `--migrations-table "schema_migrations"` - database table to record migrations in. _(env: `DBMATE_MIGRATIONS_TABLE`)_ - `--schema-file, -s "./db/schema.sql"` - a path to keep the schema.sql file. _(env: `DBMATE_SCHEMA_FILE`)_ - `--no-dump-schema` - don't auto-update the schema.sql file on migrate/rollback _(env: `DBMATE_NO_DUMP_SCHEMA`)_ +- `--strict` - fail if migrations would be applied out of order _(env: `DBMATE_STRICT`)_ - `--wait` - wait for the db to become available before executing the subsequent command _(env: `DBMATE_WAIT`)_ - `--wait-timeout 60s` - timeout for --wait flag _(env: `DBMATE_WAIT_TIMEOUT`)_ diff --git a/main.go b/main.go index 83b85bf8..c32d1891 100644 --- a/main.go +++ b/main.go @@ -101,6 +101,11 @@ func NewApp() *cli.App { Name: "up", Usage: "Create database (if necessary) and migrate to the latest version", Flags: []cli.Flag{ + &cli.BoolFlag{ + Name: "strict", + EnvVars: []string{"DBMATE_STRICT"}, + Usage: "fail if migrations would be applied out of order", + }, &cli.BoolFlag{ Name: "verbose", Aliases: []string{"v"}, @@ -109,6 +114,7 @@ func NewApp() *cli.App { }, }, Action: action(func(db *dbmate.DB, c *cli.Context) error { + db.Strict = c.Bool("strict") db.Verbose = c.Bool("verbose") return db.CreateAndMigrate() }), @@ -131,6 +137,11 @@ func NewApp() *cli.App { Name: "migrate", Usage: "Migrate to the latest version", Flags: []cli.Flag{ + &cli.BoolFlag{ + Name: "strict", + EnvVars: []string{"DBMATE_STRICT"}, + Usage: "fail if migrations would be applied out of order", + }, &cli.BoolFlag{ Name: "verbose", Aliases: []string{"v"}, @@ -139,6 +150,7 @@ func NewApp() *cli.App { }, }, Action: action(func(db *dbmate.DB, c *cli.Context) error { + db.Strict = c.Bool("strict") db.Verbose = c.Bool("verbose") return db.Migrate() }), @@ -174,6 +186,7 @@ func NewApp() *cli.App { }, }, Action: action(func(db *dbmate.DB, c *cli.Context) error { + db.Strict = c.Bool("strict") setExitCode := c.Bool("exit-code") quiet := c.Bool("quiet") if quiet { diff --git a/pkg/dbmate/db.go b/pkg/dbmate/db.go index ae598ffd..9ebdf324 100644 --- a/pkg/dbmate/db.go +++ b/pkg/dbmate/db.go @@ -49,6 +49,8 @@ type DB struct { MigrationsTableName string // SchemaFile specifies the location for schema.sql file SchemaFile string + // Fail if migrations would be applied out of order + Strict bool // Verbose prints the result of each statement execution Verbose bool // WaitBefore will wait for database to become available before running any actions @@ -75,6 +77,7 @@ func New(databaseURL *url.URL) *DB { MigrationsDir: []string{"./db/migrations"}, MigrationsTableName: "schema_migrations", SchemaFile: "./db/schema.sql", + Strict: false, Verbose: false, WaitBefore: false, WaitInterval: time.Second, @@ -309,17 +312,29 @@ func (db *DB) Migrate() error { return ErrNoMigrationFiles } + highestAppliedMigrationVersion := "" + pendingMigrations := []Migration{} + for _, migration := range migrations { + if migration.Applied { + if db.Strict && highestAppliedMigrationVersion <= migration.Version { + highestAppliedMigrationVersion = migration.Version + } + } else { + pendingMigrations = append(pendingMigrations, migration) + } + } + + if len(pendingMigrations) > 0 && db.Strict && pendingMigrations[0].Version <= highestAppliedMigrationVersion { + return fmt.Errorf("migration `%s` is out of order with already applied migrations, the version number has to be higher than the applied migration `%s` in --strict mode", pendingMigrations[0].Version, highestAppliedMigrationVersion) + } + sqlDB, err := db.openDatabaseForMigration(drv) if err != nil { return err } defer dbutil.MustClose(sqlDB) - for _, migration := range migrations { - if migration.Applied { - continue - } - + for _, migration := range pendingMigrations { fmt.Fprintf(db.Log, "Applying: %s\n", migration.FileName) parsed, err := migration.Parse() diff --git a/pkg/dbmate/db_test.go b/pkg/dbmate/db_test.go index 6ba8d5bb..9205a795 100644 --- a/pkg/dbmate/db_test.go +++ b/pkg/dbmate/db_test.go @@ -583,3 +583,79 @@ func TestFindMigrationsFSMultipleDirs(t *testing.T) { require.Equal(t, "db/migrations_a/005_test_migration_a.sql", actual[4].FilePath) require.Equal(t, "db/migrations_c/006_test_migration_c.sql", actual[5].FilePath) } + +func TestMigrateUnrestrictedOrder(t *testing.T) { + emptyMigration := []byte("-- migrate:up\n-- migrate:down") + + // initialize database + u := dbutil.MustParseURL(os.Getenv("POSTGRES_TEST_URL")) + db := newTestDB(t, u) + + err := db.Drop() + require.NoError(t, err) + err = db.Create() + require.NoError(t, err) + + // test to apply new migrations on empty database + db.FS = fstest.MapFS{ + "db/migrations/001_test_migration_a.sql": {Data: emptyMigration}, + "db/migrations/100_test_migration_b.sql": {Data: emptyMigration}, + } + + err = db.Migrate() + require.NoError(t, err) + + // test to apply an out of order migration + db.FS = fstest.MapFS{ + "db/migrations/001_test_migration_a.sql": {Data: emptyMigration}, + "db/migrations/100_test_migration_b.sql": {Data: emptyMigration}, + "db/migrations/010_test_migration_c.sql": {Data: emptyMigration}, + } + + err = db.Migrate() + require.NoError(t, err) +} + +func TestMigrateStrictOrder(t *testing.T) { + emptyMigration := []byte("-- migrate:up\n-- migrate:down") + + // initialize database + u := dbutil.MustParseURL(os.Getenv("POSTGRES_TEST_URL")) + db := newTestDB(t, u) + db.Strict = true + + err := db.Drop() + require.NoError(t, err) + err = db.Create() + require.NoError(t, err) + + // test to apply new migrations on empty database + db.FS = fstest.MapFS{ + "db/migrations/001_test_migration_a.sql": {Data: emptyMigration}, + "db/migrations/010_test_migration_b.sql": {Data: emptyMigration}, + } + + err = db.Migrate() + require.NoError(t, err) + + // test to apply an in order migration + db.FS = fstest.MapFS{ + "db/migrations/001_test_migration_a.sql": {Data: emptyMigration}, + "db/migrations/010_test_migration_b.sql": {Data: emptyMigration}, + "db/migrations/100_test_migration_c.sql": {Data: emptyMigration}, + } + + err = db.Migrate() + require.NoError(t, err) + + // test to apply an out of order migration + db.FS = fstest.MapFS{ + "db/migrations/001_test_migration_a.sql": {Data: emptyMigration}, + "db/migrations/010_test_migration_b.sql": {Data: emptyMigration}, + "db/migrations/100_test_migration_c.sql": {Data: emptyMigration}, + "db/migrations/050_test_migration_d.sql": {Data: emptyMigration}, + } + + err = db.Migrate() + require.Error(t, err) +}