Add --strict flag (#441)

* 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 <adrian@foxglove.dev>
amacneil · Nov 14, 2023 · 1658a46 · 1658a46
1 parent 8bbf517
commit 1658a46
Show file tree

Hide file tree

Showing 4 changed files with 110 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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)
+}