diff --git a/tests/e2e/postgres/allow_missing_test.go b/tests/e2e/postgres/allow_missing_test.go index 2e61d8eb0..26fa05a1e 100644 --- a/tests/e2e/postgres/allow_missing_test.go +++ b/tests/e2e/postgres/allow_missing_test.go @@ -40,8 +40,8 @@ func TestNotAllowMissing(t *testing.T) { // detected. _, err = te.provider.Up(ctx) check.HasError(t, err) - // found 1 missing migration: 6 - check.Contains(t, err.Error(), "missing migration") + // found 1 missing (out-of-order) migration: [00006_f.sql] + check.Contains(t, err.Error(), "missing (out-of-order) migration") // Confirm db version is unchanged. current, err = te.provider.GetDBVersion(ctx) check.NoError(t, err) @@ -49,8 +49,8 @@ func TestNotAllowMissing(t *testing.T) { _, err = te.provider.UpByOne(ctx) check.HasError(t, err) - // found 1 missing migration: 6 - check.Contains(t, err.Error(), "missing migration") + // found 1 missing (out-of-order) migration: [00006_f.sql] + check.Contains(t, err.Error(), "missing (out-of-order) migration") // Confirm db version is unchanged. current, err = te.provider.GetDBVersion(ctx) check.NoError(t, err) @@ -58,8 +58,8 @@ func TestNotAllowMissing(t *testing.T) { _, err = te.provider.UpTo(ctx, math.MaxInt64) check.HasError(t, err) - // found 1 missing migration: 6 - check.Contains(t, err.Error(), "missing migration") + // found 1 missing (out-of-order) migration: [00006_f.sql] + check.Contains(t, err.Error(), "missing (out-of-order) migration") // Confirm db version is unchanged. current, err = te.provider.GetDBVersion(ctx) check.NoError(t, err) @@ -303,3 +303,42 @@ func TestMigrateAllowMissingDown(t *testing.T) { } } } + +func TestWithWithoutAllowMissing(t *testing.T) { + t.Parallel() + ctx := context.Background() + // Test for https://github.com/pressly/goose/issues/521 + + // Apply 1,2,4,3 then run up without allow missing. If the next requested migration is + // 4 then it should not raise an error because it has already been applied. + // If goose attempts to apply 4 again then it will raise an error (SQLSTATE 42701) because it + // has already been applied. Specifically it will raise a error. + // Create and apply first 5 migrations. + opt := goose.DefaultOptions(). + SetAllowMissing(true) + te := newTestEnv(t, migrationsDir, &opt) + _, err := te.provider.UpTo(ctx, 2) + check.NoError(t, err) + _, err = te.provider.ApplyVersion(ctx, 4, true) + check.NoError(t, err) + res, err := te.provider.UpTo(ctx, 4) + check.NoError(t, err) + check.Number(t, len(res), 1) + check.Number(t, res[0].Version, 3) + + { + // Create a new goose provider WITHOUT allow missing, and attempt to apply migration 4. + opt := goose.DefaultOptions(). + SetDir(migrationsDir). + SetVerbose(testing.Verbose()) + provider, err := goose.NewProvider(goose.DialectPostgres, te.db, opt) + check.NoError(t, err) + results, err := provider.UpTo(ctx, 4) + check.NoError(t, err) + check.Number(t, len(results), 0) + // And now try to apply the next migration in the sequence. + result, err := provider.UpByOne(ctx) + check.NoError(t, err) + check.Number(t, result.Version, 5) + } +} diff --git a/up.go b/up.go index 558f1271d..6e91b2e38 100644 --- a/up.go +++ b/up.go @@ -5,8 +5,8 @@ import ( "errors" "fmt" "math" + "path/filepath" "sort" - "strconv" "strings" "github.com/pressly/goose/v4/internal/dialectadapter" @@ -92,20 +92,20 @@ func (p *Provider) up(ctx context.Context, upByOne bool, version int64) (_ []*Mi if len(missingMigrations) > 0 && !p.opt.AllowMissing { var collected []string for _, v := range missingMigrations { - collected = append(collected, strconv.FormatInt(v, 10)) + collected = append(collected, v.filename) } msg := "migration" if len(collected) > 1 { msg += "s" } - return nil, fmt.Errorf("found %d missing %s: %s", + return nil, fmt.Errorf("found %d missing (out-of-order) %s: [%s]", len(missingMigrations), msg, strings.Join(collected, ",")) } var migrationsToApply []*migration.Migration if p.opt.AllowMissing { for _, v := range missingMigrations { - m, err := p.getMigration(v) + m, err := p.getMigration(v.versionID) if err != nil { return nil, err } @@ -133,12 +133,17 @@ func (p *Provider) up(ctx context.Context, upByOne bool, version int64) (_ []*Mi return p.runMigrations(ctx, conn, migrationsToApply, sqlparser.DirectionUp, upByOne) } +type missingMigration struct { + versionID int64 + filename string +} + // findMissingMigrations returns a list of migrations that are missing from the database. A missing // migration is one that has a version less than the max version in the database. func findMissingMigrations( dbMigrations []*dialectadapter.ListMigrationsResult, fsMigrations []*migration.Migration, -) []int64 { +) []missingMigration { existing := make(map[int64]bool) var max int64 for _, m := range dbMigrations { @@ -147,14 +152,17 @@ func findMissingMigrations( max = m.Version } } - var missing []int64 + var missing []missingMigration for _, m := range fsMigrations { if !existing[m.Version] && m.Version < max { - missing = append(missing, m.Version) + missing = append(missing, missingMigration{ + versionID: m.Version, + filename: filepath.Base(m.Fullpath), + }) } } sort.Slice(missing, func(i, j int) bool { - return missing[i] < missing[j] + return missing[i].versionID < missing[j].versionID }) return missing } diff --git a/up_test.go b/up_test.go index a95f1b96f..8ff610050 100644 --- a/up_test.go +++ b/up_test.go @@ -34,8 +34,8 @@ func TestFindMissingMigrations(t *testing.T) { } got := findMissingMigrations(dbMigrations, fsMigrations) check.Number(t, len(got), 2) - check.Number(t, got[0], 2) - check.Number(t, got[1], 6) + check.Number(t, got[0].versionID, 2) + check.Number(t, got[1].versionID, 6) // Sanity check. check.Number(t, len(findMissingMigrations(nil, nil)), 0)