From ee760bf608173a7110a82693c92b71bc71f6943d Mon Sep 17 00:00:00 2001 From: AJ ONeal Date: Thu, 9 Apr 2026 16:27:28 -0600 Subject: [PATCH] fix(sqlmigrate): address medium-severity review findings - Add ErrMissingScript sentinel for unknown applied migrations in Down (previously misused ErrMissingDown when entire script was missing) - Wrap Applied() errors with ErrQueryApplied in Up, Down, GetStatus - Check ctx.Err() between migration loop iterations in Up and Down - Tighten litemigrate error match to "no such table: _migrations" - Return error from logMigrationsSelect instead of calling os.Exit(1) - Check os.WriteFile errors in create command - Split main.go (1147 lines) into main.go, init.go, fixup.go, create.go --- cmd/sql-migrate/create.go | 110 ++++ cmd/sql-migrate/fixup.go | 182 +++++++ cmd/sql-migrate/init.go | 261 +++++++++ cmd/sql-migrate/main.go | 509 ------------------ .../sqlmigrate/litemigrate/litemigrate.go | 2 +- database/sqlmigrate/sqlmigrate.go | 27 +- database/sqlmigrate/sqlmigrate_test.go | 4 +- 7 files changed, 573 insertions(+), 522 deletions(-) create mode 100644 cmd/sql-migrate/create.go create mode 100644 cmd/sql-migrate/fixup.go create mode 100644 cmd/sql-migrate/init.go diff --git a/cmd/sql-migrate/create.go b/cmd/sql-migrate/create.go new file mode 100644 index 0000000..e294945 --- /dev/null +++ b/cmd/sql-migrate/create.go @@ -0,0 +1,110 @@ +package main + +import ( + "crypto/rand" + "encoding/hex" + "fmt" + "os" + "path/filepath" + "strconv" + "strings" +) + +func create(state *State, desc string) error { + dateStr := state.Date.Format("2006-01-02") + entries, err := os.ReadDir(state.MigrationsDir) + if err != nil { + return err + } + + maxNumber := 0 + datePrefix := dateStr + "-" + for _, entry := range entries { + if entry.IsDir() { + continue + } + + name := entry.Name() + if !strings.HasPrefix(name, datePrefix) { + continue + } + if !strings.HasSuffix(name, ".up.sql") { + continue + } + if strings.HasSuffix(name, "_"+desc+".up.sql") { + return fmt.Errorf("migration for %q already exists:\n %s", desc, state.MigrationsDir+"/"+name) + } + if strings.HasSuffix(name, ".down.sql") { + continue + } + + parts := strings.SplitN(name, "-", 4) + if len(parts) < 4 { + continue + } + numDesc := strings.SplitN(parts[3], "_", 2) + if len(numDesc) < 2 { + continue + } + num, err := strconv.Atoi(numDesc[0]) + if err != nil { + continue + } + + if num > maxNumber { + maxNumber = num + } + } + + number := maxNumber / 1_000 + number *= 1_000 + number += 1_000 + if number > 9_000 && number < 10_000 { + fmt.Fprintf(os.Stderr, "Achievement Unlocked: It's over 9000!\n") + } + if number >= 999_999 { + fmt.Fprintf(os.Stderr, "Error: cowardly refusing to generate such a suspiciously high number of migrations after running out of numbers\n") + os.Exit(1) + } + + basename := fmt.Sprintf("%s-%06d_%s", dateStr, number, desc) + upPath := filepath.Join(state.MigrationsDir, basename+".up.sql") + downPath := filepath.Join(state.MigrationsDir, basename+".down.sql") + + id := MustRandomHex(4) + + // Little Bobby Drop Tables says: + // We trust the person running the migrations to not use malicious names. + // (we don't want to embed db-specific logic here, and SQL doesn't define escaping) + migrationInsert := fmt.Sprintf("INSERT INTO _migrations (name, id) VALUES ('%s', '%s');", basename, id) + upContent := fmt.Appendf(nil, "-- %s (up)\nSELECT 'place your UP migration here';\n\n-- leave this as the last line\n%s\n", desc, migrationInsert) + if err := os.WriteFile(upPath, upContent, 0644); err != nil { + return fmt.Errorf("create up migration: %w", err) + } + migrationDelete := fmt.Sprintf("DELETE FROM _migrations WHERE id = '%s';", id) + downContent := fmt.Appendf(nil, "-- %s (down)\nSELECT 'place your DOWN migration here';\n\n-- leave this as the last line\n%s\n", desc, migrationDelete) + if err := os.WriteFile(downPath, downContent, 0644); err != nil { + return fmt.Errorf("create down migration: %w", err) + } + + fmt.Fprintf(os.Stderr, " created pair %s\n", filepathUnclean(upPath)) + fmt.Fprintf(os.Stderr, " %s\n", filepathUnclean(downPath)) + return nil +} + +func MustRandomHex(n int) string { + s, err := RandomHex(n) + if err != nil { + panic(err) + } + return s +} + +func RandomHex(n int) (string, error) { + b := make([]byte, n) // 4 bytes = 8 hex chars + _, err := rand.Read(b) + if err != nil { + return "", err + } + return hex.EncodeToString(b), nil +} diff --git a/cmd/sql-migrate/fixup.go b/cmd/sql-migrate/fixup.go new file mode 100644 index 0000000..fe3f0a3 --- /dev/null +++ b/cmd/sql-migrate/fixup.go @@ -0,0 +1,182 @@ +package main + +import ( + "bufio" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/therootcompany/golib/database/sqlmigrate" +) + +func (state *State) parseAndFixupBatches(text string) error { + text = strings.TrimSpace(text) + if text == "" { + return nil + } + + fixedUp := []string{} + fixedDown := []string{} + + state.Lines = strings.Split(text, "\n") + for i := range state.Lines { + line := strings.TrimSpace(state.Lines[i]) + migration := commentStartRe.ReplaceAllString(line, "") + migration = strings.TrimSpace(migration) + if migration != "" { + up, down, warn, err := fixupMigration(state.MigrationsDir, migration) + if warn != nil { + fmt.Fprintf(os.Stderr, "Warn: %s\n", warn) + } + if err != nil { + fmt.Fprintf(os.Stderr, "%v\n", err) + } + state.Migrated = append(state.Migrated, migration) + if up { + fixedUp = append(fixedUp, migration) + } + if down { + fixedDown = append(fixedDown, migration) + } + + } + state.Lines[i] = line + } + showFixes(fixedUp, fixedDown) + + return nil +} + +func showFixes(fixedUp, fixedDown []string) { + if len(fixedUp) > 0 { + fmt.Fprintf(os.Stderr, "Fixup: appended missing 'INSERT INTO _migrations ...' to:\n") + for _, up := range fixedUp { + fmt.Fprintf(os.Stderr, " %s\n", up) + } + fmt.Fprintf(os.Stderr, "\n") + } + + if len(fixedDown) > 0 { + fmt.Fprintf(os.Stderr, "Fixup: appended missing 'DELETE FROM _migrations ...' to:\n") + for _, down := range fixedDown { + fmt.Fprintf(os.Stderr, " %s\n", down) + } + fmt.Fprintf(os.Stderr, "\n") + } +} + +// attempts to add missing INSERT and DELETE without breaking what already works +func fixupMigration(dir string, basename string) (up, down bool, warn error, err error) { + var id string + + var insertsOnUp bool + upPath := filepath.Join(dir, basename+".up.sql") + upScan, err := os.Open(upPath) + if err != nil { + return false, false, nil, fmt.Errorf("failed (up): %w", err) + } + defer upScan.Close() + scanner := bufio.NewScanner(upScan) + for scanner.Scan() { + txt := scanner.Text() + txt = strings.TrimSpace(txt) + txt = strings.ToLower(txt) + if strings.HasPrefix(txt, "insert into _migrations") { + insertsOnUp = true + break + } + } + if !insertsOnUp { + id = MustRandomHex(4) + upScan.Close() + upBytes, err := os.ReadFile(upPath) + if err != nil { + warn = fmt.Errorf("failed to add 'INSERT INTO _migrations ...' to %s: %w", upPath, err) + return false, false, warn, nil + } + + migrationInsertLn := fmt.Sprintf("\n-- leave this as the last line\nINSERT INTO _migrations (name, id) VALUES ('%s', '%s');\n", basename, id) + upBytes = append(upBytes, []byte(migrationInsertLn)...) + if err = os.WriteFile(upPath, upBytes, 0644); err != nil { + warn = fmt.Errorf("failed to append 'INSERT INTO _migrations ...' to %s: %w", upPath, err) + return false, false, warn, nil + } + up = true + } + + var deletesOnDown bool + downPath := filepath.Join(dir, basename+".down.sql") + downScan, err := os.Open(downPath) + if err != nil { + return false, false, fmt.Errorf("failed (down): %w", err), nil + } + defer downScan.Close() + scanner = bufio.NewScanner(downScan) + for scanner.Scan() { + txt := scanner.Text() + txt = strings.TrimSpace(txt) + txt = strings.ToLower(txt) + if strings.HasPrefix(txt, "delete from _migrations") { + deletesOnDown = true + break + } + } + if !deletesOnDown { + if id == "" { + return false, false, fmt.Errorf("must manually append \"DELETE FROM _migrations WHERE id = ''\" to %s with id from %s", downPath, basename+"up.sql"), nil + } + downScan.Close() + downFile, err := os.OpenFile(downPath, os.O_APPEND|os.O_WRONLY, 0o644) + if err != nil { + warn = fmt.Errorf("failed to append 'DELETE FROM _migrations ...' to %s: %v", downPath, err) + return false, false, warn, nil + } + defer downFile.Close() + + migrationInsertLn := fmt.Sprintf("\nDELETE FROM _migrations WHERE id = '%s';\n", id) + _, err = downFile.Write(([]byte(migrationInsertLn))) + if err != nil { + warn = fmt.Errorf("failed to add 'DELETE FROM _migrations ...' to %s: %w", downPath, err) + return false, false, warn, nil + } + down = true + } + + return up, down, nil, nil +} + +// fixupAll runs fixupMigration on all known migrations (applied + pending). +func fixupAll(migrationsDir string, applied []string, migrations []sqlmigrate.Script) (fixedUp, fixedDown []string) { + seen := map[string]bool{} + var all []string + for _, name := range applied { + if !seen[name] { + all = append(all, name) + seen[name] = true + } + } + for _, m := range migrations { + if !seen[m.Name] { + all = append(all, m.Name) + seen[m.Name] = true + } + } + + for _, name := range all { + up, down, warn, err := fixupMigration(migrationsDir, name) + if warn != nil { + fmt.Fprintf(os.Stderr, "Warn: %s\n", warn) + } + if err != nil { + fmt.Fprintf(os.Stderr, "%v\n", err) + } + if up { + fixedUp = append(fixedUp, name) + } + if down { + fixedDown = append(fixedDown, name) + } + } + return fixedUp, fixedDown +} diff --git a/cmd/sql-migrate/init.go b/cmd/sql-migrate/init.go new file mode 100644 index 0000000..831f4e0 --- /dev/null +++ b/cmd/sql-migrate/init.go @@ -0,0 +1,261 @@ +package main + +import ( + "bufio" + "fmt" + "os" + "path/filepath" + "slices" + "strings" + + "github.com/therootcompany/golib/database/sqlmigrate/shmigrate" +) + +// logMigrationsSelect returns the DB-specific SELECT line for _migrations. +// The output format is "idname" per row, compatible with shmigrate.Applied(). +func logMigrationsSelect(sqlCommand string) (string, error) { + var selectExpr string + switch { + case strings.Contains(sqlCommand, "psql"): + selectExpr = "id || CHR(9) || name" + case strings.Contains(sqlCommand, "mysql") || strings.Contains(sqlCommand, "mariadb"): + selectExpr = "CONCAT(id, CHAR(9), name)" + case strings.Contains(sqlCommand, "sqlite"): + selectExpr = "id || CHAR(9) || name" + case strings.Contains(sqlCommand, "sqlcmd"): + selectExpr = "id + CHAR(9) + name" + default: + return "", fmt.Errorf("unrecognized --sql-command %q; cannot generate _migrations.sql", sqlCommand) + } + return fmt.Sprintf("SELECT %s FROM _migrations ORDER BY name;", selectExpr), nil +} + +// initializes all necessary files and directories +// - ./sql/migrations.log +// +// - ./sql/migrations +// +// - ./sql/migrations/0001-01-01-001000_init-migrations.up.sql +// - migrations_log: ./sql/migrations.log +// - sql_command: psql "$PG_URL" -v ON_ERROR_STOP=on --no-align --tuples-only --file %s +// +// - ./sql/migrations/0001-01-01-001000_init-migrations.down.sql +func mustInit(cfg *MainConfig) { + fmt.Fprintf(os.Stderr, "Initializing %q ...\n", cfg.migrationsDir) + + var resolvedLogPath = cfg.logPath + if cfg.sqlCommand != "" && !strings.Contains(cfg.sqlCommand, "%s") { + fmt.Fprintf(os.Stderr, "Error: --sql-command must contain a literal '%%s' to accept the path to the SQL file\n") + os.Exit(1) + } + + entries, err := os.ReadDir(cfg.migrationsDir) + if err != nil { + if !os.IsNotExist(err) { + fmt.Fprintf(os.Stderr, "Error: init failed to read %q: %v\n", cfg.migrationsDir, err) + os.Exit(1) + } + if err = os.MkdirAll(cfg.migrationsDir, 0755); err != nil { + fmt.Fprintf(os.Stderr, "Error: init failed to create %q: %v\n", cfg.migrationsDir, err) + os.Exit(1) + } + } + + ups, downs := migrationsList(cfg.migrationsDir, entries) + + mMigratorUpPath := filepath.Join(cfg.migrationsDir, M_MIGRATOR_UP_NAME) + mMigratorDownPath := filepath.Join(cfg.migrationsDir, M_MIGRATOR_DOWN_NAME) + + // write config + if slices.Contains(ups, M_MIGRATOR_NAME) { + fmt.Fprintf(os.Stderr, " found %s\n", filepathJoin(cfg.migrationsDir, M_MIGRATOR_UP_NAME)) + } else { + if cfg.logPath == "" { + migrationsParent := filepath.Dir(cfg.migrationsDir) + resolvedLogPath = filepath.Join(migrationsParent, defaultLogPath) + // resolvedLogPath, err = filepath.Rel(cfg.migrationsDir, cfg.logPath) + // if err != nil { + // fmt.Fprintf(os.Stderr, "Error: init couldn't resolve the migrations log relative to the migrations dir: %v\n", err) + // os.Exit(1) + // } + } + + migratorUpQuery := fmt.Sprintf(defaultMigratorUpTmpl, cfg.sqlCommand, resolvedLogPath) + if created, err := initFile(mMigratorUpPath, migratorUpQuery); err != nil { + fmt.Fprintf(os.Stderr, "Error: init couldn't create initial up migration: %v\n", err) + os.Exit(1) + } else if created { + fmt.Fprintf(os.Stderr, " created %s\n", filepathUnclean(mMigratorUpPath)) + } + } + + state := State{ + MigrationsDir: cfg.migrationsDir, + } + state.SQLCommand, state.LogPath, err = extractVars(mMigratorUpPath) + if err != nil { + fmt.Fprintf(os.Stderr, "Error: init couldn't read config from initial migration: %v\n", err) + os.Exit(1) + } + if cfg.logPath != "" && filepath.Clean(cfg.logPath) != filepath.Clean(state.LogPath) { + fmt.Fprintf(os.Stderr, + "--migrations-log %q does not match %q from %q\n(drop the --migrations-log flag, or update the add migrations file)\n", + cfg.logPath, state.LogPath, mMigratorUpPath, + ) + os.Exit(1) + } + if cfg.sqlCommand != "" && cfg.sqlCommand != state.SQLCommand { + fmt.Fprintf(os.Stderr, + "--sql-command %q does not match %q from %q\n(drop the --sql-command flag, or update the add migrations file)\n", + cfg.sqlCommand, state.SQLCommand, mMigratorUpPath, + ) + os.Exit(1) + } + + if slices.Contains(downs, M_MIGRATOR_NAME) { + fmt.Fprintf(os.Stderr, " found %s\n", filepathUnclean(mMigratorDownPath)) + } else { + migratorDownQuery := defaultMigratorDown + if created, err := initFile(mMigratorDownPath, migratorDownQuery); err != nil { + fmt.Fprintf(os.Stderr, "Error: init couldn't create initial up migration: %v\n", err) + os.Exit(1) + } else if created { + fmt.Fprintf(os.Stderr, " created %s\n", filepathUnclean(mMigratorDownPath)) + } + } + + logQueryPath := filepath.Join(state.MigrationsDir, LOG_QUERY_NAME) + queryHeader := logMigrationsQueryNote + if strings.Contains(state.SQLCommand, "sqlcmd") { + queryHeader += logMigrationsQuerySQLCmdNote + } + selectLine, err := logMigrationsSelect(state.SQLCommand) + if err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + os.Exit(1) + } + if created, err := initFile(logQueryPath, queryHeader+selectLine+"\n"); err != nil { + fmt.Fprintf(os.Stderr, "Error: init couldn't create migrations query: %v\n", err) + os.Exit(1) + } else if created { + fmt.Fprintf(os.Stderr, " created %s\n", filepathUnclean(logQueryPath)) + } else { + fmt.Fprintf(os.Stderr, " found %s\n", filepathUnclean(logQueryPath)) + } + + if fileExists(state.LogPath) { + fmt.Fprintf(os.Stderr, " found %s\n", filepathUnclean(state.LogPath)) + fmt.Fprintf(os.Stderr, "done\n") + return + } +} + +// maybeUpgradeLogQuery replaces the old name-only SELECT in _migrations.sql +// with the new id+name SELECT. Only the matching line is replaced; comments +// and other customizations are preserved. +func maybeUpgradeLogQuery(path, sqlCommand string) { + b, err := os.ReadFile(path) + if err != nil { + return + } + var replaced bool + var lines []string + for line := range strings.SplitSeq(string(b), "\n") { + if !replaced && strings.TrimSpace(line) == logMigrationsQueryPrev2_2_0 { + newLine, err := logMigrationsSelect(sqlCommand) + if err != nil { + fmt.Fprintf(os.Stderr, "Warn: %v\n", err) + return + } + line = newLine + replaced = true + } + lines = append(lines, line) + } + if !replaced { + return + } + + if err := os.WriteFile(path, []byte(strings.Join(lines, "\n")), 0644); err != nil { + fmt.Fprintf(os.Stderr, "Warn: couldn't upgrade %s: %v\n", path, err) + return + } + fmt.Fprintf(os.Stderr, " upgraded %s (added id to output)\n", filepathUnclean(path)) +} + +func initFile(path, contents string) (bool, error) { + if fileExists(path) { + return false, nil + } + + if err := os.WriteFile(path, []byte(contents), 0644); err != nil { + return false, err + } + + return true, nil +} + +func extractVars(curMigrationPath string) (sqlCommand string, logPath string, err error) { + f, err := os.Open(curMigrationPath) + if err != nil { + return "", "", err + } + defer f.Close() + + scanner := bufio.NewScanner(f) + + var logPathRel string + var logPathPrefix = "-- migrations_log:" + var commandPrefix = "-- sql_command:" + for scanner.Scan() { + txt := scanner.Text() + txt = strings.TrimSpace(txt) + if strings.HasPrefix(txt, logPathPrefix) { + logPathRel = strings.TrimSpace(txt[len(logPathPrefix):]) + continue + } else if strings.HasPrefix(txt, commandPrefix) { + sqlCommand = strings.TrimSpace(txt[len(commandPrefix):]) + continue + } + } + + if logPathRel == "" { + return "", "", fmt.Errorf("Could not find '-- migrations_log: ' in %q", curMigrationPath) + } + if sqlCommand == "" { + return "", "", fmt.Errorf("Could not find '-- sql_command: ' in %q", curMigrationPath) + } + + // migrationsDir := filepath.Dir(curMigrationPath) + // logPath = filepath.Join(migrationsDir, logPathRel) + // return sqlCommand, logPath, nil + return sqlCommand, logPathRel, nil +} + +func migrationsLogInit(state *State, subcmd string) error { + logDir := filepath.Dir(state.LogPath) + if err := os.MkdirAll(logDir, 0755); err != nil { + return err + } + + if subcmd != "up" { + fmt.Fprintf(os.Stderr, "\n") + fmt.Fprintf(os.Stderr, "Run the first migration to complete the initialization:\n") + fmt.Fprintf(os.Stderr, "(you'll need to provide DB credentials via .env or export)\n") + fmt.Fprintf(os.Stderr, "\n") + fmt.Fprintf(os.Stderr, " sql-migrate -d %q up > ./up.sh && sh ./up.sh\n", state.MigrationsDir) + fmt.Fprintf(os.Stderr, "\n") + } + return nil +} + +func syncLog(runner *shmigrate.Migrator) { + syncCmd := strings.Replace(runner.SqlCommand, "%s", filepathUnclean(runner.LogQueryPath), 1) + logPath := filepathUnclean(runner.LogPath) + + fmt.Printf(shmigrate.ShHeader) + fmt.Println("") + fmt.Println("# SYNC: reload migrations log from DB") + fmt.Printf("%s > %s || true\n", syncCmd, logPath) + fmt.Printf("cat %s\n", logPath) +} diff --git a/cmd/sql-migrate/main.go b/cmd/sql-migrate/main.go index ad72dcb..7f0ddbb 100644 --- a/cmd/sql-migrate/main.go +++ b/cmd/sql-migrate/main.go @@ -12,10 +12,7 @@ package main import ( - "bufio" "context" - "crypto/rand" - "encoding/hex" "flag" "fmt" "io" @@ -176,26 +173,6 @@ var ( commentStartRe = regexp.MustCompile(`(^|\s+)#.*`) ) -// logMigrationsSelect returns the DB-specific SELECT line for _migrations. -// The output format is "idname" per row, compatible with shmigrate.Applied(). -func logMigrationsSelect(sqlCommand string) string { - var selectExpr string - switch { - case strings.Contains(sqlCommand, "psql"): - selectExpr = "id || CHR(9) || name" - case strings.Contains(sqlCommand, "mysql") || strings.Contains(sqlCommand, "mariadb"): - selectExpr = "CONCAT(id, CHAR(9), name)" - case strings.Contains(sqlCommand, "sqlite"): - selectExpr = "id || CHAR(9) || name" - case strings.Contains(sqlCommand, "sqlcmd"): - selectExpr = "id + CHAR(9) + name" - default: - fmt.Fprintf(os.Stderr, "Error: unrecognized --sql-command %q; cannot generate _migrations.sql\n", sqlCommand) - os.Exit(1) - } - return fmt.Sprintf("SELECT %s FROM _migrations ORDER BY name;", selectExpr) -} - type State struct { Date time.Time SQLCommand string @@ -524,457 +501,6 @@ func filepathJoin(src, dst string) string { return filepathUnclean(filepath.Join(src, dst)) } -// initializes all necessary files and directories -// - ./sql/migrations.log -// -// - ./sql/migrations -// -// - ./sql/migrations/0001-01-01-001000_init-migrations.up.sql -// - migrations_log: ./sql/migrations.log -// - sql_command: psql "$PG_URL" -v ON_ERROR_STOP=on --no-align --tuples-only --file %s -// -// - ./sql/migrations/0001-01-01-001000_init-migrations.down.sql -func mustInit(cfg *MainConfig) { - fmt.Fprintf(os.Stderr, "Initializing %q ...\n", cfg.migrationsDir) - - var resolvedLogPath = cfg.logPath - if cfg.sqlCommand != "" && !strings.Contains(cfg.sqlCommand, "%s") { - fmt.Fprintf(os.Stderr, "Error: --sql-command must contain a literal '%%s' to accept the path to the SQL file\n") - os.Exit(1) - } - - entries, err := os.ReadDir(cfg.migrationsDir) - if err != nil { - if !os.IsNotExist(err) { - fmt.Fprintf(os.Stderr, "Error: init failed to read %q: %v\n", cfg.migrationsDir, err) - os.Exit(1) - } - if err = os.MkdirAll(cfg.migrationsDir, 0755); err != nil { - fmt.Fprintf(os.Stderr, "Error: init failed to create %q: %v\n", cfg.migrationsDir, err) - os.Exit(1) - } - } - - ups, downs := migrationsList(cfg.migrationsDir, entries) - - mMigratorUpPath := filepath.Join(cfg.migrationsDir, M_MIGRATOR_UP_NAME) - mMigratorDownPath := filepath.Join(cfg.migrationsDir, M_MIGRATOR_DOWN_NAME) - - // write config - if slices.Contains(ups, M_MIGRATOR_NAME) { - fmt.Fprintf(os.Stderr, " found %s\n", filepathJoin(cfg.migrationsDir, M_MIGRATOR_UP_NAME)) - } else { - if cfg.logPath == "" { - migrationsParent := filepath.Dir(cfg.migrationsDir) - resolvedLogPath = filepath.Join(migrationsParent, defaultLogPath) - // resolvedLogPath, err = filepath.Rel(cfg.migrationsDir, cfg.logPath) - // if err != nil { - // fmt.Fprintf(os.Stderr, "Error: init couldn't resolve the migrations log relative to the migrations dir: %v\n", err) - // os.Exit(1) - // } - } - - migratorUpQuery := fmt.Sprintf(defaultMigratorUpTmpl, cfg.sqlCommand, resolvedLogPath) - if created, err := initFile(mMigratorUpPath, migratorUpQuery); err != nil { - fmt.Fprintf(os.Stderr, "Error: init couldn't create initial up migration: %v\n", err) - os.Exit(1) - } else if created { - fmt.Fprintf(os.Stderr, " created %s\n", filepathUnclean(mMigratorUpPath)) - } - } - - state := State{ - MigrationsDir: cfg.migrationsDir, - } - state.SQLCommand, state.LogPath, err = extractVars(mMigratorUpPath) - if err != nil { - fmt.Fprintf(os.Stderr, "Error: init couldn't read config from initial migration: %v\n", err) - os.Exit(1) - } - if cfg.logPath != "" && filepath.Clean(cfg.logPath) != filepath.Clean(state.LogPath) { - fmt.Fprintf(os.Stderr, - "--migrations-log %q does not match %q from %q\n(drop the --migrations-log flag, or update the add migrations file)\n", - cfg.logPath, state.LogPath, mMigratorUpPath, - ) - os.Exit(1) - } - if cfg.sqlCommand != "" && cfg.sqlCommand != state.SQLCommand { - fmt.Fprintf(os.Stderr, - "--sql-command %q does not match %q from %q\n(drop the --sql-command flag, or update the add migrations file)\n", - cfg.sqlCommand, state.SQLCommand, mMigratorUpPath, - ) - os.Exit(1) - } - - if slices.Contains(downs, M_MIGRATOR_NAME) { - fmt.Fprintf(os.Stderr, " found %s\n", filepathUnclean(mMigratorDownPath)) - } else { - migratorDownQuery := defaultMigratorDown - if created, err := initFile(mMigratorDownPath, migratorDownQuery); err != nil { - fmt.Fprintf(os.Stderr, "Error: init couldn't create initial up migration: %v\n", err) - os.Exit(1) - } else if created { - fmt.Fprintf(os.Stderr, " created %s\n", filepathUnclean(mMigratorDownPath)) - } - } - - logQueryPath := filepath.Join(state.MigrationsDir, LOG_QUERY_NAME) - queryHeader := logMigrationsQueryNote - if strings.Contains(state.SQLCommand, "sqlcmd") { - queryHeader += logMigrationsQuerySQLCmdNote - } - if created, err := initFile(logQueryPath, queryHeader+logMigrationsSelect(state.SQLCommand)+"\n"); err != nil { - fmt.Fprintf(os.Stderr, "Error: init couldn't create migrations query: %v\n", err) - os.Exit(1) - } else if created { - fmt.Fprintf(os.Stderr, " created %s\n", filepathUnclean(logQueryPath)) - } else { - fmt.Fprintf(os.Stderr, " found %s\n", filepathUnclean(logQueryPath)) - } - - if fileExists(state.LogPath) { - fmt.Fprintf(os.Stderr, " found %s\n", filepathUnclean(state.LogPath)) - fmt.Fprintf(os.Stderr, "done\n") - return - } -} - -// maybeUpgradeLogQuery replaces the old name-only SELECT in _migrations.sql -// with the new id+name SELECT. Only the matching line is replaced; comments -// and other customizations are preserved. -func maybeUpgradeLogQuery(path, sqlCommand string) { - b, err := os.ReadFile(path) - if err != nil { - return - } - var replaced bool - var lines []string - for line := range strings.SplitSeq(string(b), "\n") { - if !replaced && strings.TrimSpace(line) == logMigrationsQueryPrev2_2_0 { - line = logMigrationsSelect(sqlCommand) - replaced = true - } - lines = append(lines, line) - } - if !replaced { - return - } - - if err := os.WriteFile(path, []byte(strings.Join(lines, "\n")), 0644); err != nil { - fmt.Fprintf(os.Stderr, "Warn: couldn't upgrade %s: %v\n", path, err) - return - } - fmt.Fprintf(os.Stderr, " upgraded %s (added id to output)\n", filepathUnclean(path)) -} - -func initFile(path, contents string) (bool, error) { - if fileExists(path) { - return false, nil - } - - if err := os.WriteFile(path, []byte(contents), 0644); err != nil { - return false, err - } - - return true, nil -} - -func extractVars(curMigrationPath string) (sqlCommand string, logPath string, err error) { - f, err := os.Open(curMigrationPath) - if err != nil { - return "", "", err - } - defer f.Close() - - scanner := bufio.NewScanner(f) - - var logPathRel string - var logPathPrefix = "-- migrations_log:" - var commandPrefix = "-- sql_command:" - for scanner.Scan() { - txt := scanner.Text() - txt = strings.TrimSpace(txt) - if strings.HasPrefix(txt, logPathPrefix) { - logPathRel = strings.TrimSpace(txt[len(logPathPrefix):]) - continue - } else if strings.HasPrefix(txt, commandPrefix) { - sqlCommand = strings.TrimSpace(txt[len(commandPrefix):]) - continue - } - } - - if logPathRel == "" { - return "", "", fmt.Errorf("Could not find '-- migrations_log: ' in %q", curMigrationPath) - } - if sqlCommand == "" { - return "", "", fmt.Errorf("Could not find '-- sql_command: ' in %q", curMigrationPath) - } - - // migrationsDir := filepath.Dir(curMigrationPath) - // logPath = filepath.Join(migrationsDir, logPathRel) - // return sqlCommand, logPath, nil - return sqlCommand, logPathRel, nil -} - -func migrationsLogInit(state *State, subcmd string) error { - logDir := filepath.Dir(state.LogPath) - if err := os.MkdirAll(logDir, 0755); err != nil { - return err - } - - if subcmd != "up" { - fmt.Fprintf(os.Stderr, "\n") - fmt.Fprintf(os.Stderr, "Run the first migration to complete the initialization:\n") - fmt.Fprintf(os.Stderr, "(you'll need to provide DB credentials via .env or export)\n") - fmt.Fprintf(os.Stderr, "\n") - fmt.Fprintf(os.Stderr, " sql-migrate -d %q up > ./up.sh && sh ./up.sh\n", state.MigrationsDir) - fmt.Fprintf(os.Stderr, "\n") - } - return nil -} - -func (state *State) parseAndFixupBatches(text string) error { - text = strings.TrimSpace(text) - if text == "" { - return nil - } - - fixedUp := []string{} - fixedDown := []string{} - - state.Lines = strings.Split(text, "\n") - for i := range state.Lines { - line := strings.TrimSpace(state.Lines[i]) - migration := commentStartRe.ReplaceAllString(line, "") - migration = strings.TrimSpace(migration) - if migration != "" { - up, down, warn, err := fixupMigration(state.MigrationsDir, migration) - if warn != nil { - fmt.Fprintf(os.Stderr, "Warn: %s\n", warn) - } - if err != nil { - fmt.Fprintf(os.Stderr, "%v\n", err) - } - state.Migrated = append(state.Migrated, migration) - if up { - fixedUp = append(fixedUp, migration) - } - if down { - fixedDown = append(fixedDown, migration) - } - - } - state.Lines[i] = line - } - showFixes(fixedUp, fixedDown) - - return nil -} - -func showFixes(fixedUp, fixedDown []string) { - if len(fixedUp) > 0 { - fmt.Fprintf(os.Stderr, "Fixup: appended missing 'INSERT INTO _migrations ...' to:\n") - for _, up := range fixedUp { - fmt.Fprintf(os.Stderr, " %s\n", up) - } - fmt.Fprintf(os.Stderr, "\n") - } - - if len(fixedDown) > 0 { - fmt.Fprintf(os.Stderr, "Fixup: appended missing 'DELETE FROM _migrations ...' to:\n") - for _, down := range fixedDown { - fmt.Fprintf(os.Stderr, " %s\n", down) - } - fmt.Fprintf(os.Stderr, "\n") - } -} - -func create(state *State, desc string) error { - dateStr := state.Date.Format("2006-01-02") - entries, err := os.ReadDir(state.MigrationsDir) - if err != nil { - return err - } - - maxNumber := 0 - datePrefix := dateStr + "-" - for _, entry := range entries { - if entry.IsDir() { - continue - } - - name := entry.Name() - if !strings.HasPrefix(name, datePrefix) { - continue - } - if !strings.HasSuffix(name, ".up.sql") { - continue - } - if strings.HasSuffix(name, "_"+desc+".up.sql") { - return fmt.Errorf("migration for %q already exists:\n %s", desc, state.MigrationsDir+"/"+name) - } - if strings.HasSuffix(name, ".down.sql") { - continue - } - - parts := strings.SplitN(name, "-", 4) - if len(parts) < 4 { - continue - } - numDesc := strings.SplitN(parts[3], "_", 2) - if len(numDesc) < 2 { - continue - } - num, err := strconv.Atoi(numDesc[0]) - if err != nil { - continue - } - - if num > maxNumber { - maxNumber = num - } - } - - number := maxNumber / 1_000 - number *= 1_000 - number += 1_000 - if number > 9_000 && number < 10_000 { - fmt.Fprintf(os.Stderr, "Achievement Unlocked: It's over 9000!\n") - } - if number >= 999_999 { - fmt.Fprintf(os.Stderr, "Error: cowardly refusing to generate such a suspiciously high number of migrations after running out of numbers\n") - os.Exit(1) - } - - basename := fmt.Sprintf("%s-%06d_%s", dateStr, number, desc) - upPath := filepath.Join(state.MigrationsDir, basename+".up.sql") - downPath := filepath.Join(state.MigrationsDir, basename+".down.sql") - - id := MustRandomHex(4) - - // Little Bobby Drop Tables says: - // We trust the person running the migrations to not use malicious names. - // (we don't want to embed db-specific logic here, and SQL doesn't define escaping) - migrationInsert := fmt.Sprintf("INSERT INTO _migrations (name, id) VALUES ('%s', '%s');", basename, id) - upContent := fmt.Appendf(nil, "-- %s (up)\nSELECT 'place your UP migration here';\n\n-- leave this as the last line\n%s\n", desc, migrationInsert) - _ = os.WriteFile(upPath, upContent, 0644) - migrationDelete := fmt.Sprintf("DELETE FROM _migrations WHERE id = '%s';", id) - downContent := fmt.Appendf(nil, "-- %s (down)\nSELECT 'place your DOWN migration here';\n\n-- leave this as the last line\n%s\n", desc, migrationDelete) - _ = os.WriteFile(downPath, downContent, 0644) - - fmt.Fprintf(os.Stderr, " created pair %s\n", filepathUnclean(upPath)) - fmt.Fprintf(os.Stderr, " %s\n", filepathUnclean(downPath)) - return nil -} - -func MustRandomHex(n int) string { - s, err := RandomHex(n) - if err != nil { - panic(err) - } - return s -} - -func RandomHex(n int) (string, error) { - b := make([]byte, n) // 4 bytes = 8 hex chars - _, err := rand.Read(b) - if err != nil { - return "", err - } - return hex.EncodeToString(b), nil -} - -// attempts to add missing INSERT and DELETE without breaking what already works -func fixupMigration(dir string, basename string) (up, down bool, warn error, err error) { - var id string - - var insertsOnUp bool - upPath := filepath.Join(dir, basename+".up.sql") - upScan, err := os.Open(upPath) - if err != nil { - return false, false, nil, fmt.Errorf("failed (up): %w", err) - } - defer upScan.Close() - scanner := bufio.NewScanner(upScan) - for scanner.Scan() { - txt := scanner.Text() - txt = strings.TrimSpace(txt) - txt = strings.ToLower(txt) - if strings.HasPrefix(txt, "insert into _migrations") { - insertsOnUp = true - break - } - } - if !insertsOnUp { - id = MustRandomHex(4) - upScan.Close() - upBytes, err := os.ReadFile(upPath) - if err != nil { - warn = fmt.Errorf("failed to add 'INSERT INTO _migrations ...' to %s: %w", upPath, err) - return false, false, warn, nil - } - - migrationInsertLn := fmt.Sprintf("\n-- leave this as the last line\nINSERT INTO _migrations (name, id) VALUES ('%s', '%s');\n", basename, id) - upBytes = append(upBytes, []byte(migrationInsertLn)...) - if err = os.WriteFile(upPath, upBytes, 0644); err != nil { - warn = fmt.Errorf("failed to append 'INSERT INTO _migrations ...' to %s: %w", upPath, err) - return false, false, warn, nil - } - up = true - } - - var deletesOnDown bool - downPath := filepath.Join(dir, basename+".down.sql") - downScan, err := os.Open(downPath) - if err != nil { - return false, false, fmt.Errorf("failed (down): %w", err), nil - } - defer downScan.Close() - scanner = bufio.NewScanner(downScan) - for scanner.Scan() { - txt := scanner.Text() - txt = strings.TrimSpace(txt) - txt = strings.ToLower(txt) - if strings.HasPrefix(txt, "delete from _migrations") { - deletesOnDown = true - break - } - } - if !deletesOnDown { - if id == "" { - return false, false, fmt.Errorf("must manually append \"DELETE FROM _migrations WHERE id = ''\" to %s with id from %s", downPath, basename+"up.sql"), nil - } - downScan.Close() - downFile, err := os.OpenFile(downPath, os.O_APPEND|os.O_WRONLY, 0o644) - if err != nil { - warn = fmt.Errorf("failed to append 'DELETE FROM _migrations ...' to %s: %v", downPath, err) - return false, false, warn, nil - } - defer downFile.Close() - - migrationInsertLn := fmt.Sprintf("\nDELETE FROM _migrations WHERE id = '%s';\n", id) - _, err = downFile.Write(([]byte(migrationInsertLn))) - if err != nil { - warn = fmt.Errorf("failed to add 'DELETE FROM _migrations ...' to %s: %w", downPath, err) - return false, false, warn, nil - } - down = true - } - - return up, down, nil, nil -} - -func syncLog(runner *shmigrate.Migrator) { - syncCmd := strings.Replace(runner.SqlCommand, "%s", filepathUnclean(runner.LogQueryPath), 1) - logPath := filepathUnclean(runner.LogPath) - - fmt.Printf(shmigrate.ShHeader) - fmt.Println("") - fmt.Println("# SYNC: reload migrations log from DB") - fmt.Printf("%s > %s || true\n", syncCmd, logPath) - fmt.Printf("cat %s\n", logPath) -} - func cmdUp(ctx context.Context, state *State, runner *shmigrate.Migrator, migrations []sqlmigrate.Script, n int) error { // fixup pending migrations before generating the script fixedUp, fixedDown := fixupAll(state.MigrationsDir, state.Migrated, migrations) @@ -1097,38 +623,3 @@ func cmdStatus(ctx context.Context, state *State, runner *shmigrate.Migrator, mi } return nil } - -// fixupAll runs fixupMigration on all known migrations (applied + pending). -func fixupAll(migrationsDir string, applied []string, migrations []sqlmigrate.Script) (fixedUp, fixedDown []string) { - seen := map[string]bool{} - var all []string - for _, name := range applied { - if !seen[name] { - all = append(all, name) - seen[name] = true - } - } - for _, m := range migrations { - if !seen[m.Name] { - all = append(all, m.Name) - seen[m.Name] = true - } - } - - for _, name := range all { - up, down, warn, err := fixupMigration(migrationsDir, name) - if warn != nil { - fmt.Fprintf(os.Stderr, "Warn: %s\n", warn) - } - if err != nil { - fmt.Fprintf(os.Stderr, "%v\n", err) - } - if up { - fixedUp = append(fixedUp, name) - } - if down { - fixedDown = append(fixedDown, name) - } - } - return fixedUp, fixedDown -} diff --git a/database/sqlmigrate/litemigrate/litemigrate.go b/database/sqlmigrate/litemigrate/litemigrate.go index c1fc8e0..53698c1 100644 --- a/database/sqlmigrate/litemigrate/litemigrate.go +++ b/database/sqlmigrate/litemigrate/litemigrate.go @@ -66,7 +66,7 @@ func (m *Migrator) Applied(ctx context.Context) ([]sqlmigrate.Migration, error) rows, err := m.Conn.QueryContext(ctx, "SELECT id, name FROM _migrations ORDER BY name") if err != nil { // SQLite reports "no such table: _migrations" — stable across versions - if strings.Contains(err.Error(), "no such table") { + if strings.Contains(err.Error(), "no such table: _migrations") { return nil, nil } return nil, fmt.Errorf("%w: %w", sqlmigrate.ErrQueryApplied, err) diff --git a/database/sqlmigrate/sqlmigrate.go b/database/sqlmigrate/sqlmigrate.go index 3e6e138..d197d80 100644 --- a/database/sqlmigrate/sqlmigrate.go +++ b/database/sqlmigrate/sqlmigrate.go @@ -20,12 +20,13 @@ import ( // Sentinel errors for migration operations. var ( - ErrMissingUp = errors.New("missing up migration") - ErrMissingDown = errors.New("missing down migration") - ErrWalkFailed = errors.New("walking migrations") - ErrExecFailed = errors.New("migration exec failed") - ErrQueryApplied = errors.New("querying applied migrations") - ErrInvalidN = errors.New("n must be positive or -1 for all") + ErrMissingUp = errors.New("missing up migration") + ErrMissingDown = errors.New("missing down migration") + ErrMissingScript = errors.New("unknown applied migration") + ErrWalkFailed = errors.New("walking migrations") + ErrExecFailed = errors.New("migration exec failed") + ErrQueryApplied = errors.New("querying applied migrations") + ErrInvalidN = errors.New("n must be positive or -1 for all") ) // Migration identifies a migration by its name and optional hex ID. @@ -192,7 +193,7 @@ func Up(ctx context.Context, r Migrator, ddls []Script, n int) ([]Migration, err applied, err := r.Applied(ctx) if err != nil { - return nil, err + return nil, fmt.Errorf("%w: %w", ErrQueryApplied, err) } var pending []Script @@ -211,6 +212,9 @@ func Up(ctx context.Context, r Migrator, ddls []Script, n int) ([]Migration, err var ran []Migration for _, d := range pending[:n] { + if err := ctx.Err(); err != nil { + return ran, err + } if err := r.ExecUp(ctx, d.Migration, d.Up); err != nil { return ran, fmt.Errorf("%s (up): %w", d.Name, err) } @@ -230,7 +234,7 @@ func Down(ctx context.Context, r Migrator, ddls []Script, n int) ([]Migration, e applied, err := r.Applied(ctx) if err != nil { - return nil, err + return nil, fmt.Errorf("%w: %w", ErrQueryApplied, err) } byName := map[string]Script{} @@ -252,9 +256,12 @@ func Down(ctx context.Context, r Migrator, ddls []Script, n int) ([]Migration, e var ran []Migration for _, a := range reversed[:n] { + if err := ctx.Err(); err != nil { + return ran, err + } d, ok := findScript(a, byName, byID) if !ok { - return ran, fmt.Errorf("%w: %s", ErrMissingDown, a.Name) + return ran, fmt.Errorf("%w: %s", ErrMissingScript, a.Name) } if err := r.ExecDown(ctx, a, d.Down); err != nil { return ran, fmt.Errorf("%s (down): %w", a.Name, err) @@ -269,7 +276,7 @@ func Down(ctx context.Context, r Migrator, ddls []Script, n int) ([]Migration, e func GetStatus(ctx context.Context, r Migrator, ddls []Script) (*Status, error) { applied, err := r.Applied(ctx) if err != nil { - return nil, err + return nil, fmt.Errorf("%w: %w", ErrQueryApplied, err) } var pending []Migration diff --git a/database/sqlmigrate/sqlmigrate_test.go b/database/sqlmigrate/sqlmigrate_test.go index c770515..6577681 100644 --- a/database/sqlmigrate/sqlmigrate_test.go +++ b/database/sqlmigrate/sqlmigrate_test.go @@ -394,8 +394,8 @@ func TestDown(t *testing.T) { t.Run("unknown migration in applied", func(t *testing.T) { m := &mockMigrator{applied: migs("001_init", "999_unknown")} _, err := sqlmigrate.Down(ctx, m, ddls, -1) - if !errors.Is(err, sqlmigrate.ErrMissingDown) { - t.Errorf("got %v, want ErrMissingDown", err) + if !errors.Is(err, sqlmigrate.ErrMissingScript) { + t.Errorf("got %v, want ErrMissingScript", err) } })