From 3461dcbc7db9e6c1fe81ab6db7142c088ad93c62 Mon Sep 17 00:00:00 2001 From: AJ ONeal Date: Mon, 6 Apr 2026 00:19:52 -0600 Subject: [PATCH] fix(sqlmigrate): Down n=0 means rollback all, add tests and docs - Change Down(n<=0) to mean "rollback all" (matching Up semantics). CLIs now explicitly pass 1 when no count is given. - Add 22 tests for Collect, NamesOnly, Up, Down, and GetStatus using mock Migrator and fstest.MapFS. - Fix shmigrate.Applied to stream from file instead of ReadFile double-wrap; add optional FS field for embed.FS support. - Update package doc to list all 5 backends. - Add minimal README linking to pkg.go.dev. --- cmd/sql-migrate/examples/pg-todos/main.go | 6 +- cmd/sql-migrate/main.go | 6 +- database/sqlmigrate/README.md | 22 ++ database/sqlmigrate/shmigrate/shmigrate.go | 19 +- database/sqlmigrate/sqlmigrate.go | 19 +- database/sqlmigrate/sqlmigrate_test.go | 392 +++++++++++++++++++++ 6 files changed, 446 insertions(+), 18 deletions(-) create mode 100644 database/sqlmigrate/README.md create mode 100644 database/sqlmigrate/sqlmigrate_test.go diff --git a/cmd/sql-migrate/examples/pg-todos/main.go b/cmd/sql-migrate/examples/pg-todos/main.go index 0c34256..f14d79a 100644 --- a/cmd/sql-migrate/examples/pg-todos/main.go +++ b/cmd/sql-migrate/examples/pg-todos/main.go @@ -177,11 +177,11 @@ func runMigrate(ctx context.Context, runner sqlmigrate.Migrator, migrations []sq return err case "down": - n := 0 + n := 1 if len(args) > 1 { var err error n, err = strconv.Atoi(args[1]) - if err != nil || n < 0 { + if err != nil || n < 1 { return fmt.Errorf("%q is not a valid count", args[1]) } } @@ -209,7 +209,7 @@ func runMigrate(ctx context.Context, runner sqlmigrate.Migrator, migrations []sq return nil case "reset": - rolled, err := sqlmigrate.Down(ctx, runner, migrations, len(migrations)) + rolled, err := sqlmigrate.Down(ctx, runner, migrations, 0) if len(rolled) == 0 && err == nil { fmt.Println("Nothing to reset.") } else if err == nil { diff --git a/cmd/sql-migrate/main.go b/cmd/sql-migrate/main.go index 9b2f79b..7faf9fc 100644 --- a/cmd/sql-migrate/main.go +++ b/cmd/sql-migrate/main.go @@ -355,13 +355,13 @@ func main() { log.Fatal(err) } case "down": - var downN int + downN := 1 switch len(leafArgs) { case 0: - // ignore + // default: roll back one case 1: downN, err = strconv.Atoi(leafArgs[0]) - if err != nil || downN < 0 { + if err != nil || downN < 1 { fmt.Fprintf(os.Stderr, "Error: %s is not a positive number\n", leafArgs[0]) os.Exit(1) } diff --git a/database/sqlmigrate/README.md b/database/sqlmigrate/README.md new file mode 100644 index 0000000..48ac473 --- /dev/null +++ b/database/sqlmigrate/README.md @@ -0,0 +1,22 @@ +# sqlmigrate + +Database-agnostic SQL migration library for Go. + +[![Go Reference](https://pkg.go.dev/badge/github.com/therootcompany/golib/database/sqlmigrate.svg)](https://pkg.go.dev/github.com/therootcompany/golib/database/sqlmigrate) + +## Backend packages + +Each backend is a separate Go module to avoid pulling unnecessary drivers: + +| Package | Database | Driver | +|---------|----------|--------| +| [pgmigrate](https://pkg.go.dev/github.com/therootcompany/golib/database/sqlmigrate/pgmigrate) | PostgreSQL | pgx/v5 | +| [mymigrate](https://pkg.go.dev/github.com/therootcompany/golib/database/sqlmigrate/mymigrate) | MySQL / MariaDB | go-sql-driver/mysql | +| [litemigrate](https://pkg.go.dev/github.com/therootcompany/golib/database/sqlmigrate/litemigrate) | SQLite | database/sql (caller imports driver) | +| [msmigrate](https://pkg.go.dev/github.com/therootcompany/golib/database/sqlmigrate/msmigrate) | SQL Server | go-mssqldb | +| [shmigrate](https://pkg.go.dev/github.com/therootcompany/golib/database/sqlmigrate/shmigrate) | Shell scripts | (generates POSIX sh) | + +## CLI + +The [sql-migrate](https://pkg.go.dev/github.com/therootcompany/golib/cmd/sql-migrate/v2) CLI +uses shmigrate to generate shell scripts for managing migrations without a Go dependency at runtime. diff --git a/database/sqlmigrate/shmigrate/shmigrate.go b/database/sqlmigrate/shmigrate/shmigrate.go index f2b7db9..500a761 100644 --- a/database/sqlmigrate/shmigrate/shmigrate.go +++ b/database/sqlmigrate/shmigrate/shmigrate.go @@ -8,6 +8,7 @@ import ( "context" "fmt" "io" + "io/fs" "os" "path/filepath" "strings" @@ -46,6 +47,10 @@ type Migrator struct { // LogPath is the path to the migrations.log file. LogPath string + // FS is an optional filesystem for reading the migrations log. + // When nil, the OS filesystem is used. + FS fs.FS + counter int } @@ -80,18 +85,26 @@ func (r *Migrator) exec(name, suffix, label string) error { } // Applied reads the migrations log file and returns applied migration names. -// Returns an empty slice if the file does not exist. +// Returns an empty slice if the file does not exist. When FS is set, reads +// from that filesystem; otherwise reads from the OS filesystem. func (r *Migrator) Applied(ctx context.Context) ([]string, error) { - data, err := os.ReadFile(r.LogPath) + var f io.ReadCloser + var err error + if r.FS != nil { + f, err = r.FS.Open(r.LogPath) + } else { + f, err = os.Open(r.LogPath) + } if err != nil { if os.IsNotExist(err) { return nil, nil } return nil, fmt.Errorf("reading migrations log: %w", err) } + defer f.Close() var names []string - scanner := bufio.NewScanner(strings.NewReader(string(data))) + scanner := bufio.NewScanner(f) for scanner.Scan() { line := strings.TrimSpace(scanner.Text()) // strip inline comments diff --git a/database/sqlmigrate/sqlmigrate.go b/database/sqlmigrate/sqlmigrate.go index 9bab522..ac8ff02 100644 --- a/database/sqlmigrate/sqlmigrate.go +++ b/database/sqlmigrate/sqlmigrate.go @@ -1,6 +1,11 @@ -// Package sqlmigrate provides types and utilities for SQL migration files. -// It is database-agnostic — see pgmigrate for PostgreSQL execution, -// shmigrate for shell script generation. +// Package sqlmigrate provides a database-agnostic SQL migration interface. +// +// Backend implementations (each a separate Go module): +// - pgmigrate: PostgreSQL via pgx/v5 +// - mymigrate: MySQL/MariaDB via go-sql-driver/mysql +// - litemigrate: SQLite via database/sql +// - msmigrate: SQL Server via go-mssqldb +// - shmigrate: POSIX shell script generation package sqlmigrate import ( @@ -159,17 +164,13 @@ func Up(ctx context.Context, r Migrator, migrations []Migration, n int) ([]strin } // Down rolls back up to n applied migrations, most recent first. -// If n <= 0, rolls back one. Returns the names of rolled-back migrations. +// If n <= 0, rolls back all applied. Returns the names of rolled-back migrations. func Down(ctx context.Context, r Migrator, migrations []Migration, n int) ([]string, error) { appliedNames, err := r.Applied(ctx) if err != nil { return nil, err } - if n <= 0 { - n = 1 - } - byName := map[string]Migration{} for _, m := range migrations { byName[m.Name] = m @@ -179,7 +180,7 @@ func Down(ctx context.Context, r Migrator, migrations []Migration, n int) ([]str copy(reversed, appliedNames) slices.Reverse(reversed) - if n > len(reversed) { + if n <= 0 || n > len(reversed) { n = len(reversed) } diff --git a/database/sqlmigrate/sqlmigrate_test.go b/database/sqlmigrate/sqlmigrate_test.go new file mode 100644 index 0000000..83f3792 --- /dev/null +++ b/database/sqlmigrate/sqlmigrate_test.go @@ -0,0 +1,392 @@ +package sqlmigrate_test + +import ( + "context" + "errors" + "slices" + "testing" + "testing/fstest" + + "github.com/therootcompany/golib/database/sqlmigrate" +) + +// mockMigrator tracks applied migrations in memory. +type mockMigrator struct { + applied []string + execErr error // if set, ExecUp/ExecDown return this on every call + upCalls []string + downCalls []string +} + +func (m *mockMigrator) ExecUp(_ context.Context, mig sqlmigrate.Migration) error { + m.upCalls = append(m.upCalls, mig.Name) + if m.execErr != nil { + return m.execErr + } + m.applied = append(m.applied, mig.Name) + slices.Sort(m.applied) + return nil +} + +func (m *mockMigrator) ExecDown(_ context.Context, mig sqlmigrate.Migration) error { + m.downCalls = append(m.downCalls, mig.Name) + if m.execErr != nil { + return m.execErr + } + m.applied = slices.DeleteFunc(m.applied, func(s string) bool { return s == mig.Name }) + return nil +} + +func (m *mockMigrator) Applied(_ context.Context) ([]string, error) { + return slices.Clone(m.applied), nil +} + +// --- Collect --- + +func TestCollect(t *testing.T) { + t.Run("pairs and sorts", func(t *testing.T) { + fsys := fstest.MapFS{ + "002_second.up.sql": {Data: []byte("CREATE TABLE b;")}, + "002_second.down.sql": {Data: []byte("DROP TABLE b;")}, + "001_first.up.sql": {Data: []byte("CREATE TABLE a;")}, + "001_first.down.sql": {Data: []byte("DROP TABLE a;")}, + } + migrations, err := sqlmigrate.Collect(fsys) + if err != nil { + t.Fatal(err) + } + if len(migrations) != 2 { + t.Fatalf("got %d migrations, want 2", len(migrations)) + } + if migrations[0].Name != "001_first" { + t.Errorf("first = %q, want %q", migrations[0].Name, "001_first") + } + if migrations[1].Name != "002_second" { + t.Errorf("second = %q, want %q", migrations[1].Name, "002_second") + } + if migrations[0].Up != "CREATE TABLE a;" { + t.Errorf("first.Up = %q", migrations[0].Up) + } + if migrations[0].Down != "DROP TABLE a;" { + t.Errorf("first.Down = %q", migrations[0].Down) + } + }) + + t.Run("missing down", func(t *testing.T) { + fsys := fstest.MapFS{ + "001_only-up.up.sql": {Data: []byte("CREATE TABLE x;")}, + } + _, err := sqlmigrate.Collect(fsys) + if !errors.Is(err, sqlmigrate.ErrMissingDown) { + t.Errorf("got %v, want ErrMissingDown", err) + } + }) + + t.Run("missing up", func(t *testing.T) { + fsys := fstest.MapFS{ + "001_only-down.down.sql": {Data: []byte("DROP TABLE x;")}, + } + _, err := sqlmigrate.Collect(fsys) + if !errors.Is(err, sqlmigrate.ErrMissingUp) { + t.Errorf("got %v, want ErrMissingUp", err) + } + }) + + t.Run("ignores non-sql files", func(t *testing.T) { + fsys := fstest.MapFS{ + "001_init.up.sql": {Data: []byte("UP")}, + "001_init.down.sql": {Data: []byte("DOWN")}, + "README.md": {Data: []byte("# Migrations")}, + "_migrations.sql": {Data: []byte("SELECT name FROM _migrations;")}, + } + migrations, err := sqlmigrate.Collect(fsys) + if err != nil { + t.Fatal(err) + } + if len(migrations) != 1 { + t.Fatalf("got %d migrations, want 1", len(migrations)) + } + }) + + t.Run("empty fs", func(t *testing.T) { + fsys := fstest.MapFS{} + migrations, err := sqlmigrate.Collect(fsys) + if err != nil { + t.Fatal(err) + } + if len(migrations) != 0 { + t.Fatalf("got %d migrations, want 0", len(migrations)) + } + }) +} + +// --- NamesOnly --- + +func TestNamesOnly(t *testing.T) { + names := []string{"001_init", "002_users"} + migrations := sqlmigrate.NamesOnly(names) + if len(migrations) != 2 { + t.Fatalf("got %d, want 2", len(migrations)) + } + for i, m := range migrations { + if m.Name != names[i] { + t.Errorf("[%d].Name = %q, want %q", i, m.Name, names[i]) + } + if m.Up != "" || m.Down != "" { + t.Errorf("[%d] has non-empty content", i) + } + } +} + +// --- Up --- + +func TestUp(t *testing.T) { + ctx := t.Context() + migrations := []sqlmigrate.Migration{ + {Name: "001_init", Up: "CREATE TABLE a;", Down: "DROP TABLE a;"}, + {Name: "002_users", Up: "CREATE TABLE b;", Down: "DROP TABLE b;"}, + {Name: "003_posts", Up: "CREATE TABLE c;", Down: "DROP TABLE c;"}, + } + + t.Run("apply all", func(t *testing.T) { + m := &mockMigrator{} + applied, err := sqlmigrate.Up(ctx, m, migrations, 0) + if err != nil { + t.Fatal(err) + } + if len(applied) != 3 { + t.Fatalf("applied %d, want 3", len(applied)) + } + if !slices.Equal(applied, []string{"001_init", "002_users", "003_posts"}) { + t.Errorf("applied = %v", applied) + } + }) + + t.Run("apply n", func(t *testing.T) { + m := &mockMigrator{} + applied, err := sqlmigrate.Up(ctx, m, migrations, 2) + if err != nil { + t.Fatal(err) + } + if len(applied) != 2 { + t.Fatalf("applied %d, want 2", len(applied)) + } + if applied[0] != "001_init" || applied[1] != "002_users" { + t.Errorf("applied = %v", applied) + } + }) + + t.Run("none pending", func(t *testing.T) { + m := &mockMigrator{applied: []string{"001_init", "002_users", "003_posts"}} + applied, err := sqlmigrate.Up(ctx, m, migrations, 0) + if err != nil { + t.Fatal(err) + } + if len(applied) != 0 { + t.Fatalf("applied %d, want 0", len(applied)) + } + }) + + t.Run("partial pending", func(t *testing.T) { + m := &mockMigrator{applied: []string{"001_init"}} + applied, err := sqlmigrate.Up(ctx, m, migrations, 0) + if err != nil { + t.Fatal(err) + } + if !slices.Equal(applied, []string{"002_users", "003_posts"}) { + t.Errorf("applied = %v", applied) + } + }) + + t.Run("n exceeds pending", func(t *testing.T) { + m := &mockMigrator{applied: []string{"001_init"}} + applied, err := sqlmigrate.Up(ctx, m, migrations, 99) + if err != nil { + t.Fatal(err) + } + if len(applied) != 2 { + t.Fatalf("applied %d, want 2", len(applied)) + } + }) + + t.Run("exec error stops and returns partial", func(t *testing.T) { + callCount := 0 + m := &mockMigrator{} + // fail on second call + origExecUp := m.ExecUp + _ = origExecUp + m2 := &failOnNthMigrator{failAt: 1} + applied, err := sqlmigrate.Up(ctx, m2, migrations, 0) + if err == nil { + t.Fatal("expected error") + } + if len(applied) != 1 { + t.Errorf("applied %d before error, want 1", len(applied)) + } + _ = callCount + }) +} + +// failOnNthMigrator fails on the Nth ExecUp call (0-indexed). +type failOnNthMigrator struct { + applied []string + calls int + failAt int +} + +func (m *failOnNthMigrator) ExecUp(_ context.Context, mig sqlmigrate.Migration) error { + if m.calls == m.failAt { + m.calls++ + return errors.New("connection lost") + } + m.calls++ + m.applied = append(m.applied, mig.Name) + slices.Sort(m.applied) + return nil +} + +func (m *failOnNthMigrator) ExecDown(_ context.Context, mig sqlmigrate.Migration) error { + m.applied = slices.DeleteFunc(m.applied, func(s string) bool { return s == mig.Name }) + return nil +} + +func (m *failOnNthMigrator) Applied(_ context.Context) ([]string, error) { + return slices.Clone(m.applied), nil +} + +// --- Down --- + +func TestDown(t *testing.T) { + ctx := t.Context() + migrations := []sqlmigrate.Migration{ + {Name: "001_init", Up: "CREATE TABLE a;", Down: "DROP TABLE a;"}, + {Name: "002_users", Up: "CREATE TABLE b;", Down: "DROP TABLE b;"}, + {Name: "003_posts", Up: "CREATE TABLE c;", Down: "DROP TABLE c;"}, + } + + t.Run("rollback all", func(t *testing.T) { + m := &mockMigrator{applied: []string{"001_init", "002_users", "003_posts"}} + rolled, err := sqlmigrate.Down(ctx, m, migrations, 0) + if err != nil { + t.Fatal(err) + } + if len(rolled) != 3 { + t.Fatalf("rolled %d, want 3", len(rolled)) + } + // most recent first + if rolled[0] != "003_posts" { + t.Errorf("first rollback = %q, want 003_posts", rolled[0]) + } + if rolled[2] != "001_init" { + t.Errorf("last rollback = %q, want 001_init", rolled[2]) + } + }) + + t.Run("rollback n", func(t *testing.T) { + m := &mockMigrator{applied: []string{"001_init", "002_users", "003_posts"}} + rolled, err := sqlmigrate.Down(ctx, m, migrations, 2) + if err != nil { + t.Fatal(err) + } + if !slices.Equal(rolled, []string{"003_posts", "002_users"}) { + t.Errorf("rolled = %v", rolled) + } + }) + + t.Run("none applied", func(t *testing.T) { + m := &mockMigrator{} + rolled, err := sqlmigrate.Down(ctx, m, migrations, 0) + if err != nil { + t.Fatal(err) + } + if len(rolled) != 0 { + t.Fatalf("rolled %d, want 0", len(rolled)) + } + }) + + t.Run("exec error", func(t *testing.T) { + m := &mockMigrator{ + applied: []string{"001_init", "002_users"}, + execErr: errors.New("permission denied"), + } + rolled, err := sqlmigrate.Down(ctx, m, migrations, 1) + if err == nil { + t.Fatal("expected error") + } + if len(rolled) != 0 { + t.Errorf("rolled %d on error, want 0", len(rolled)) + } + }) + + t.Run("unknown migration in applied", func(t *testing.T) { + m := &mockMigrator{applied: []string{"001_init", "999_unknown"}} + _, err := sqlmigrate.Down(ctx, m, migrations, 0) + if !errors.Is(err, sqlmigrate.ErrMissingDown) { + t.Errorf("got %v, want ErrMissingDown", err) + } + }) + + t.Run("n exceeds applied", func(t *testing.T) { + m := &mockMigrator{applied: []string{"001_init"}} + rolled, err := sqlmigrate.Down(ctx, m, migrations, 99) + if err != nil { + t.Fatal(err) + } + if len(rolled) != 1 { + t.Fatalf("rolled %d, want 1", len(rolled)) + } + }) +} + +// --- GetStatus --- + +func TestGetStatus(t *testing.T) { + ctx := t.Context() + migrations := []sqlmigrate.Migration{ + {Name: "001_init"}, + {Name: "002_users"}, + {Name: "003_posts"}, + } + + t.Run("all pending", func(t *testing.T) { + m := &mockMigrator{} + status, err := sqlmigrate.GetStatus(ctx, m, migrations) + if err != nil { + t.Fatal(err) + } + if len(status.Applied) != 0 { + t.Errorf("applied = %d, want 0", len(status.Applied)) + } + if len(status.Pending) != 3 { + t.Errorf("pending = %d, want 3", len(status.Pending)) + } + }) + + t.Run("partial", func(t *testing.T) { + m := &mockMigrator{applied: []string{"001_init"}} + status, err := sqlmigrate.GetStatus(ctx, m, migrations) + if err != nil { + t.Fatal(err) + } + if len(status.Applied) != 1 || status.Applied[0] != "001_init" { + t.Errorf("applied = %v", status.Applied) + } + if !slices.Equal(status.Pending, []string{"002_users", "003_posts"}) { + t.Errorf("pending = %v", status.Pending) + } + }) + + t.Run("all applied", func(t *testing.T) { + m := &mockMigrator{applied: []string{"001_init", "002_users", "003_posts"}} + status, err := sqlmigrate.GetStatus(ctx, m, migrations) + if err != nil { + t.Fatal(err) + } + if len(status.Applied) != 3 { + t.Errorf("applied = %d, want 3", len(status.Applied)) + } + if len(status.Pending) != 0 { + t.Errorf("pending = %d, want 0", len(status.Pending)) + } + }) +}