diff --git a/.gitignore b/.gitignore index ea4a28d..b9747ed 100644 --- a/.gitignore +++ b/.gitignore @@ -36,3 +36,10 @@ io/transform/gsheet2csv/cmd/gsheet2tsv/gsheet2tsv # vendor/ .DS_Store + +# Agent session files +agents/TASKS.md +agents/REVIEW.md +agents/HANDOFF.md +agents/issues.d/ +agents/tmp/ diff --git a/database/sqlmigrate/litemigrate/go.mod b/database/sqlmigrate/litemigrate/go.mod index 533d98d..809b352 100644 --- a/database/sqlmigrate/litemigrate/go.mod +++ b/database/sqlmigrate/litemigrate/go.mod @@ -2,4 +2,19 @@ module github.com/therootcompany/golib/database/sqlmigrate/litemigrate go 1.26.1 -require github.com/therootcompany/golib/database/sqlmigrate v1.0.2 +require ( + github.com/therootcompany/golib/database/sqlmigrate v1.0.2 + modernc.org/sqlite v1.48.2 +) + +require ( + github.com/dustin/go-humanize v1.0.1 // indirect + github.com/google/uuid v1.6.0 // indirect + github.com/mattn/go-isatty v0.0.20 // indirect + github.com/ncruces/go-strftime v1.0.0 // indirect + github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect + golang.org/x/sys v0.42.0 // indirect + modernc.org/libc v1.70.0 // indirect + modernc.org/mathutil v1.7.1 // indirect + modernc.org/memory v1.11.0 // indirect +) diff --git a/database/sqlmigrate/litemigrate/go.sum b/database/sqlmigrate/litemigrate/go.sum index af5eed8..9bcc534 100644 --- a/database/sqlmigrate/litemigrate/go.sum +++ b/database/sqlmigrate/litemigrate/go.sum @@ -1,2 +1,53 @@ +github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY= +github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto= +github.com/google/pprof v0.0.0-20250317173921-a4b03ec1a45e h1:ijClszYn+mADRFY17kjQEVQ1XRhq2/JR1M3sGqeJoxs= +github.com/google/pprof v0.0.0-20250317173921-a4b03ec1a45e/go.mod h1:boTsfXsheKC2y+lKOCMpSfarhxDeIzfZG1jqGcPl3cA= +github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= +github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k= +github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM= +github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= +github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= +github.com/ncruces/go-strftime v1.0.0 h1:HMFp8mLCTPp341M/ZnA4qaf7ZlsbTc+miZjCLOFAw7w= +github.com/ncruces/go-strftime v1.0.0/go.mod h1:Fwc5htZGVVkseilnfgOVb9mKy6w1naJmn9CehxcKcls= +github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec h1:W09IVJc94icq4NjY3clb7Lk8O1qJ8BdBEF8z0ibU0rE= +github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec/go.mod h1:qqbHyh8v60DhA7CoWK5oRCqLrMHRGoxYCSS9EjAz6Eo= github.com/therootcompany/golib/database/sqlmigrate v1.0.2 h1:hcmhYyUFVj/GqyChP+0Ry2WZCHnoruFMbsy+2KVzsfA= github.com/therootcompany/golib/database/sqlmigrate v1.0.2/go.mod h1:7PQUjwT78Hx+SftcIKI2PH4zSFlrSO0V9h618PJqC38= +golang.org/x/mod v0.33.0 h1:tHFzIWbBifEmbwtGz65eaWyGiGZatSrT9prnU8DbVL8= +golang.org/x/mod v0.33.0/go.mod h1:swjeQEj+6r7fODbD2cqrnje9PnziFuw4bmLbBZFrQ5w= +golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4= +golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= +golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.42.0 h1:omrd2nAlyT5ESRdCLYdm3+fMfNFE/+Rf4bDIQImRJeo= +golang.org/x/sys v0.42.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/tools v0.42.0 h1:uNgphsn75Tdz5Ji2q36v/nsFSfR/9BRFvqhGBaJGd5k= +golang.org/x/tools v0.42.0/go.mod h1:Ma6lCIwGZvHK6XtgbswSoWroEkhugApmsXyrUmBhfr0= +modernc.org/cc/v4 v4.27.1 h1:9W30zRlYrefrDV2JE2O8VDtJ1yPGownxciz5rrbQZis= +modernc.org/cc/v4 v4.27.1/go.mod h1:uVtb5OGqUKpoLWhqwNQo/8LwvoiEBLvZXIQ/SmO6mL0= +modernc.org/ccgo/v4 v4.32.0 h1:hjG66bI/kqIPX1b2yT6fr/jt+QedtP2fqojG2VrFuVw= +modernc.org/ccgo/v4 v4.32.0/go.mod h1:6F08EBCx5uQc38kMGl+0Nm0oWczoo1c7cgpzEry7Uc0= +modernc.org/fileutil v1.4.0 h1:j6ZzNTftVS054gi281TyLjHPp6CPHr2KCxEXjEbD6SM= +modernc.org/fileutil v1.4.0/go.mod h1:EqdKFDxiByqxLk8ozOxObDSfcVOv/54xDs/DUHdvCUU= +modernc.org/gc/v2 v2.6.5 h1:nyqdV8q46KvTpZlsw66kWqwXRHdjIlJOhG6kxiV/9xI= +modernc.org/gc/v2 v2.6.5/go.mod h1:YgIahr1ypgfe7chRuJi2gD7DBQiKSLMPgBQe9oIiito= +modernc.org/gc/v3 v3.1.2 h1:ZtDCnhonXSZexk/AYsegNRV1lJGgaNZJuKjJSWKyEqo= +modernc.org/gc/v3 v3.1.2/go.mod h1:HFK/6AGESC7Ex+EZJhJ2Gni6cTaYpSMmU/cT9RmlfYY= +modernc.org/goabi0 v0.2.0 h1:HvEowk7LxcPd0eq6mVOAEMai46V+i7Jrj13t4AzuNks= +modernc.org/goabi0 v0.2.0/go.mod h1:CEFRnnJhKvWT1c1JTI3Avm+tgOWbkOu5oPA8eH8LnMI= +modernc.org/libc v1.70.0 h1:U58NawXqXbgpZ/dcdS9kMshu08aiA6b7gusEusqzNkw= +modernc.org/libc v1.70.0/go.mod h1:OVmxFGP1CI/Z4L3E0Q3Mf1PDE0BucwMkcXjjLntvHJo= +modernc.org/mathutil v1.7.1 h1:GCZVGXdaN8gTqB1Mf/usp1Y/hSqgI2vAGGP4jZMCxOU= +modernc.org/mathutil v1.7.1/go.mod h1:4p5IwJITfppl0G4sUEDtCr4DthTaT47/N3aT6MhfgJg= +modernc.org/memory v1.11.0 h1:o4QC8aMQzmcwCK3t3Ux/ZHmwFPzE6hf2Y5LbkRs+hbI= +modernc.org/memory v1.11.0/go.mod h1:/JP4VbVC+K5sU2wZi9bHoq2MAkCnrt2r98UGeSK7Mjw= +modernc.org/opt v0.1.4 h1:2kNGMRiUjrp4LcaPuLY2PzUfqM/w9N23quVwhKt5Qm8= +modernc.org/opt v0.1.4/go.mod h1:03fq9lsNfvkYSfxrfUhZCWPk1lm4cq4N+Bh//bEtgns= +modernc.org/sortutil v1.2.1 h1:+xyoGf15mM3NMlPDnFqrteY07klSFxLElE2PVuWIJ7w= +modernc.org/sortutil v1.2.1/go.mod h1:7ZI3a3REbai7gzCLcotuw9AC4VZVpYMjDzETGsSMqJE= +modernc.org/sqlite v1.48.2 h1:5CnW4uP8joZtA0LedVqLbZV5GD7F/0x91AXeSyjoh5c= +modernc.org/sqlite v1.48.2/go.mod h1:hWjRO6Tj/5Ik8ieqxQybiEOUXy0NJFNp2tpvVpKlvig= +modernc.org/strutil v1.2.1 h1:UneZBkQA+DX2Rp35KcM69cSsNES9ly8mQWD71HKlOA0= +modernc.org/strutil v1.2.1/go.mod h1:EHkiggD70koQxjVdSBM3JKM7k6L0FbGE5eymy9i3B9A= +modernc.org/token v1.1.0 h1:Xl7Ap9dKaEs5kLoOQeQmPWevfnk/DM5qcLcYlA8ys6Y= +modernc.org/token v1.1.0/go.mod h1:UGzOrNV1mAFSEB63lOFHIpNRUVMvYTc6yu1SMY/XTDM= diff --git a/database/sqlmigrate/litemigrate/litemigrate.go b/database/sqlmigrate/litemigrate/litemigrate.go index c1fc8e0..64eaf33 100644 --- a/database/sqlmigrate/litemigrate/litemigrate.go +++ b/database/sqlmigrate/litemigrate/litemigrate.go @@ -13,8 +13,8 @@ package litemigrate import ( "context" "database/sql" + "errors" "fmt" - "strings" "github.com/therootcompany/golib/database/sqlmigrate" ) @@ -62,16 +62,30 @@ func (m *Migrator) execInTx(ctx context.Context, sqlStr string) error { // Applied returns all applied migrations from the _migrations table. // Returns an empty slice if the table does not exist. +// +// We probe sqlite_master first rather than catching the SELECT error. +// SQLite returns the generic SQLITE_ERROR code (1) for "no such table", +// which is too coarse to distinguish from other errors via the typed +// driver error. The probe lets us use errors.Is(sql.ErrNoRows) instead +// of string-matching the error message. func (m *Migrator) Applied(ctx context.Context) ([]sqlmigrate.Migration, error) { + var name string + err := m.Conn.QueryRowContext( + ctx, + "SELECT name FROM sqlite_master WHERE type = 'table' AND name = '_migrations'", + ).Scan(&name) + if errors.Is(err, sql.ErrNoRows) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("%w: probing sqlite_master: %w", sqlmigrate.ErrQueryApplied, err) + } + 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") { - return nil, nil - } return nil, fmt.Errorf("%w: %w", sqlmigrate.ErrQueryApplied, err) } - defer rows.Close() + defer func() { _ = rows.Close() }() var applied []sqlmigrate.Migration for rows.Next() { diff --git a/database/sqlmigrate/litemigrate/litemigrate_test.go b/database/sqlmigrate/litemigrate/litemigrate_test.go new file mode 100644 index 0000000..8b58522 --- /dev/null +++ b/database/sqlmigrate/litemigrate/litemigrate_test.go @@ -0,0 +1,124 @@ +package litemigrate_test + +import ( + "database/sql" + "testing" + + _ "modernc.org/sqlite" + + "github.com/therootcompany/golib/database/sqlmigrate/litemigrate" +) + +// openMem opens a fresh in-memory SQLite database and returns the conn. +// The cleanup closes both the conn and the underlying *sql.DB. +func openMem(t *testing.T) *sql.Conn { + t.Helper() + db, err := sql.Open("sqlite", ":memory:") + if err != nil { + t.Fatalf("open: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + + conn, err := db.Conn(t.Context()) + if err != nil { + t.Fatalf("conn: %v", err) + } + t.Cleanup(func() { _ = conn.Close() }) + + return conn +} + +// TestAppliedNoMigrationsTable verifies Applied returns (nil, nil) when +// the _migrations table does not exist. Regression test for the +// table-missing handling — caught a class of bugs where the error type +// or message changes between SQLite driver versions. +func TestAppliedNoMigrationsTable(t *testing.T) { + conn := openMem(t) + + m := litemigrate.New(conn) + applied, err := m.Applied(t.Context()) + if err != nil { + t.Fatalf("Applied() error = %v, want nil", err) + } + if len(applied) != 0 { + t.Errorf("Applied() len = %d, want 0", len(applied)) + } +} + +// TestAppliedWithMigrationsTable verifies Applied reads existing rows. +func TestAppliedWithMigrationsTable(t *testing.T) { + conn := openMem(t) + ctx := t.Context() + + if _, err := conn.ExecContext(ctx, ` + CREATE TABLE _migrations (id TEXT, name TEXT); + INSERT INTO _migrations (id, name) VALUES ('abc12345', '0001_init'); + INSERT INTO _migrations (id, name) VALUES ('def67890', '0002_users'); + `); err != nil { + t.Fatalf("setup: %v", err) + } + + m := litemigrate.New(conn) + applied, err := m.Applied(ctx) + if err != nil { + t.Fatalf("Applied() error = %v", err) + } + if len(applied) != 2 { + t.Fatalf("Applied() len = %d, want 2", len(applied)) + } + if applied[0].Name != "0001_init" || applied[0].ID != "abc12345" { + t.Errorf("applied[0] = %+v, want {abc12345 0001_init}", applied[0]) + } + if applied[1].Name != "0002_users" || applied[1].ID != "def67890" { + t.Errorf("applied[1] = %+v, want {def67890 0002_users}", applied[1]) + } +} + +// TestAppliedEmptyMigrationsTable verifies Applied returns an empty slice +// (not an error) when _migrations exists but has no rows. +func TestAppliedEmptyMigrationsTable(t *testing.T) { + conn := openMem(t) + ctx := t.Context() + + if _, err := conn.ExecContext(ctx, `CREATE TABLE _migrations (id TEXT, name TEXT)`); err != nil { + t.Fatalf("setup: %v", err) + } + + m := litemigrate.New(conn) + applied, err := m.Applied(ctx) + if err != nil { + t.Fatalf("Applied() error = %v", err) + } + if len(applied) != 0 { + t.Errorf("Applied() len = %d, want 0", len(applied)) + } +} + +// TestAppliedAfterDropTable verifies Applied still returns (nil, nil) after +// the _migrations table is dropped — exercises any prepared-statement +// caching the driver may do. +func TestAppliedAfterDropTable(t *testing.T) { + conn := openMem(t) + ctx := t.Context() + + if _, err := conn.ExecContext(ctx, `CREATE TABLE _migrations (id TEXT, name TEXT)`); err != nil { + t.Fatalf("create: %v", err) + } + + m := litemigrate.New(conn) + if _, err := m.Applied(ctx); err != nil { + t.Fatalf("first Applied: %v", err) + } + + if _, err := conn.ExecContext(ctx, `DROP TABLE _migrations`); err != nil { + t.Fatalf("drop: %v", err) + } + + applied, err := m.Applied(ctx) + if err != nil { + t.Fatalf("Applied() after DROP TABLE error = %v, want nil", err) + } + if len(applied) != 0 { + t.Errorf("Applied() len = %d, want 0", len(applied)) + } +} diff --git a/database/sqlmigrate/msmigrate/msmigrate.go b/database/sqlmigrate/msmigrate/msmigrate.go index 1af4ab3..41c65b1 100644 --- a/database/sqlmigrate/msmigrate/msmigrate.go +++ b/database/sqlmigrate/msmigrate/msmigrate.go @@ -59,11 +59,13 @@ func (m *Migrator) execInTx(ctx context.Context, sqlStr string) error { // Applied returns all applied migrations from the _migrations table. // Returns an empty slice if the table does not exist (SQL Server error 208). +// +// The table-missing check is applied at both Query and rows.Err — some +// drivers may surface the error lazily after iteration begins. 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 { - // SQL Server error 208: "Invalid object name '_migrations'" - if msErr, ok := errors.AsType[mssql.Error](err); ok && msErr.Number == 208 { + if isUndefinedTable(err) { return nil, nil } return nil, fmt.Errorf("%w: %w", sqlmigrate.ErrQueryApplied, err) @@ -79,8 +81,19 @@ func (m *Migrator) Applied(ctx context.Context) ([]sqlmigrate.Migration, error) applied = append(applied, a) } if err := rows.Err(); err != nil { + if isUndefinedTable(err) { + return nil, nil + } return nil, fmt.Errorf("%w: reading rows: %w", sqlmigrate.ErrQueryApplied, err) } return applied, nil } + +// isUndefinedTable reports whether err is SQL Server error 208 +// ("Invalid object name '_migrations'"), which is what we get when +// _migrations doesn't exist yet. +func isUndefinedTable(err error) bool { + msErr, ok := errors.AsType[mssql.Error](err) + return ok && msErr.Number == 208 +} diff --git a/database/sqlmigrate/msmigrate/msmigrate_test.go b/database/sqlmigrate/msmigrate/msmigrate_test.go new file mode 100644 index 0000000..417530b --- /dev/null +++ b/database/sqlmigrate/msmigrate/msmigrate_test.go @@ -0,0 +1,145 @@ +package msmigrate_test + +import ( + "database/sql" + "os" + "testing" + + _ "github.com/microsoft/go-mssqldb" + + "github.com/therootcompany/golib/database/sqlmigrate/msmigrate" +) + +// connect opens a *sql.Conn from MSSQL_TEST_URL, skips the test if the +// env var is unset, and ensures _migrations does not exist on entry, +// with cleanup on exit. +// +// Note: SQL Server does not have per-connection search_path. Tests run +// against the user's default schema and clean up _migrations directly, +// rather than using a per-test schema. +func connect(t *testing.T) *sql.Conn { + t.Helper() + url := os.Getenv("MSSQL_TEST_URL") + if url == "" { + t.Skip("MSSQL_TEST_URL not set") + } + + ctx := t.Context() + db, err := sql.Open("sqlserver", url) + if err != nil { + t.Fatalf("open: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + + conn, err := db.Conn(ctx) + if err != nil { + t.Fatalf("conn: %v", err) + } + t.Cleanup(func() { _ = conn.Close() }) + + if _, err := conn.ExecContext(ctx, "DROP TABLE IF EXISTS _migrations"); err != nil { + t.Fatalf("pre-cleanup _migrations: %v", err) + } + t.Cleanup(func() { + _, _ = conn.ExecContext(ctx, "DROP TABLE IF EXISTS _migrations") + }) + + return conn +} + +// TestAppliedNoMigrationsTable verifies Applied returns (nil, nil) when +// the _migrations table does not exist (SQL Server error 208). Defensive +// regression test against drivers that may surface the error lazily. +func TestAppliedNoMigrationsTable(t *testing.T) { + conn := connect(t) + + m := msmigrate.New(conn) + applied, err := m.Applied(t.Context()) + if err != nil { + t.Fatalf("Applied() error = %v, want nil", err) + } + if len(applied) != 0 { + t.Errorf("Applied() len = %d, want 0", len(applied)) + } +} + +// TestAppliedWithMigrationsTable verifies Applied reads existing rows. +func TestAppliedWithMigrationsTable(t *testing.T) { + conn := connect(t) + ctx := t.Context() + + if _, err := conn.ExecContext(ctx, ` + CREATE TABLE _migrations (id NVARCHAR(16), name NVARCHAR(255)) + `); err != nil { + t.Fatalf("create: %v", err) + } + if _, err := conn.ExecContext(ctx, + `INSERT INTO _migrations (id, name) VALUES ('abc12345', '0001_init'), ('def67890', '0002_users')`, + ); err != nil { + t.Fatalf("insert: %v", err) + } + + m := msmigrate.New(conn) + applied, err := m.Applied(ctx) + if err != nil { + t.Fatalf("Applied() error = %v", err) + } + if len(applied) != 2 { + t.Fatalf("Applied() len = %d, want 2", len(applied)) + } + if applied[0].Name != "0001_init" || applied[0].ID != "abc12345" { + t.Errorf("applied[0] = %+v, want {abc12345 0001_init}", applied[0]) + } + if applied[1].Name != "0002_users" || applied[1].ID != "def67890" { + t.Errorf("applied[1] = %+v, want {def67890 0002_users}", applied[1]) + } +} + +// TestAppliedEmptyMigrationsTable verifies Applied returns an empty slice +// (not an error) when _migrations exists but has no rows. +func TestAppliedEmptyMigrationsTable(t *testing.T) { + conn := connect(t) + ctx := t.Context() + + if _, err := conn.ExecContext(ctx, `CREATE TABLE _migrations (id NVARCHAR(16), name NVARCHAR(255))`); err != nil { + t.Fatalf("create: %v", err) + } + + m := msmigrate.New(conn) + applied, err := m.Applied(ctx) + if err != nil { + t.Fatalf("Applied() error = %v", err) + } + if len(applied) != 0 { + t.Errorf("Applied() len = %d, want 0", len(applied)) + } +} + +// TestAppliedAfterDropTable verifies Applied still returns (nil, nil) +// after the _migrations table is dropped — exercises any prepared- +// statement caching the driver may do. +func TestAppliedAfterDropTable(t *testing.T) { + conn := connect(t) + ctx := t.Context() + + if _, err := conn.ExecContext(ctx, `CREATE TABLE _migrations (id NVARCHAR(16), name NVARCHAR(255))`); err != nil { + t.Fatalf("create: %v", err) + } + + m := msmigrate.New(conn) + if _, err := m.Applied(ctx); err != nil { + t.Fatalf("first Applied: %v", err) + } + + if _, err := conn.ExecContext(ctx, `DROP TABLE _migrations`); err != nil { + t.Fatalf("drop: %v", err) + } + + applied, err := m.Applied(ctx) + if err != nil { + t.Fatalf("Applied() after DROP TABLE error = %v, want nil", err) + } + if len(applied) != 0 { + t.Errorf("Applied() len = %d, want 0", len(applied)) + } +} diff --git a/database/sqlmigrate/mymigrate/mymigrate.go b/database/sqlmigrate/mymigrate/mymigrate.go index fcc9da1..c3344b9 100644 --- a/database/sqlmigrate/mymigrate/mymigrate.go +++ b/database/sqlmigrate/mymigrate/mymigrate.go @@ -87,10 +87,13 @@ func (m *Migrator) exec(ctx context.Context, sqlStr string) error { // Applied returns all applied migrations from the _migrations table. // Returns an empty slice if the table does not exist (MySQL error 1146). +// +// The table-missing check is applied at both Query and rows.Err — some +// drivers may surface the error lazily after iteration begins. 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 { - if mysqlErr, ok := errors.AsType[*mysql.MySQLError](err); ok && mysqlErr.Number == 1146 { + if isUndefinedTable(err) { return nil, nil } return nil, fmt.Errorf("%w: %w", sqlmigrate.ErrQueryApplied, err) @@ -106,8 +109,18 @@ func (m *Migrator) Applied(ctx context.Context) ([]sqlmigrate.Migration, error) applied = append(applied, a) } if err := rows.Err(); err != nil { + if isUndefinedTable(err) { + return nil, nil + } return nil, fmt.Errorf("%w: reading rows: %w", sqlmigrate.ErrQueryApplied, err) } return applied, nil } + +// isUndefinedTable reports whether err is MySQL error 1146 (table doesn't exist), +// which is what we get when _migrations doesn't exist yet. +func isUndefinedTable(err error) bool { + mysqlErr, ok := errors.AsType[*mysql.MySQLError](err) + return ok && mysqlErr.Number == 1146 +} diff --git a/database/sqlmigrate/mymigrate/mymigrate_test.go b/database/sqlmigrate/mymigrate/mymigrate_test.go new file mode 100644 index 0000000..f0a7b7e --- /dev/null +++ b/database/sqlmigrate/mymigrate/mymigrate_test.go @@ -0,0 +1,165 @@ +package mymigrate_test + +import ( + "database/sql" + "os" + "testing" + + _ "github.com/go-sql-driver/mysql" + + "github.com/therootcompany/golib/database/sqlmigrate/mymigrate" +) + +// connect opens a *sql.Conn from MYSQL_TEST_DSN, skips the test if the +// env var is unset, and isolates the test in its own database with +// automatic cleanup. +func connect(t *testing.T) *sql.Conn { + t.Helper() + dsn := os.Getenv("MYSQL_TEST_DSN") + if dsn == "" { + t.Skip("MYSQL_TEST_DSN not set") + } + + ctx := t.Context() + db, err := sql.Open("mysql", dsn) + if err != nil { + t.Fatalf("open: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + + // Use a per-test database so concurrent tests don't collide and + // _migrations is guaranteed not to exist on entry. + dbName := "mymigrate_test_" + sanitize(t.Name()) + if _, err := db.ExecContext(ctx, "DROP DATABASE IF EXISTS "+dbName); err != nil { + t.Fatalf("drop database: %v", err) + } + if _, err := db.ExecContext(ctx, "CREATE DATABASE "+dbName); err != nil { + t.Fatalf("create database: %v", err) + } + t.Cleanup(func() { + _, _ = db.ExecContext(ctx, "DROP DATABASE IF EXISTS "+dbName) + }) + + conn, err := db.Conn(ctx) + if err != nil { + t.Fatalf("conn: %v", err) + } + t.Cleanup(func() { _ = conn.Close() }) + + if _, err := conn.ExecContext(ctx, "USE "+dbName); err != nil { + t.Fatalf("use database: %v", err) + } + + return conn +} + +// sanitize converts a test name to a valid MySQL identifier suffix. +func sanitize(s string) string { + out := make([]byte, 0, len(s)) + for _, c := range []byte(s) { + switch { + case c >= 'a' && c <= 'z', c >= 'A' && c <= 'Z', c >= '0' && c <= '9': + out = append(out, c) + default: + out = append(out, '_') + } + } + return string(out) +} + +// TestAppliedNoMigrationsTable verifies Applied returns (nil, nil) when +// the _migrations table does not exist (MySQL error 1146). Defensive +// regression test against drivers that may surface the error lazily. +func TestAppliedNoMigrationsTable(t *testing.T) { + conn := connect(t) + + m := mymigrate.New(conn) + applied, err := m.Applied(t.Context()) + if err != nil { + t.Fatalf("Applied() error = %v, want nil", err) + } + if len(applied) != 0 { + t.Errorf("Applied() len = %d, want 0", len(applied)) + } +} + +// TestAppliedWithMigrationsTable verifies Applied reads existing rows. +func TestAppliedWithMigrationsTable(t *testing.T) { + conn := connect(t) + ctx := t.Context() + + if _, err := conn.ExecContext(ctx, ` + CREATE TABLE _migrations (id VARCHAR(16), name VARCHAR(255)) + `); err != nil { + t.Fatalf("create table: %v", err) + } + if _, err := conn.ExecContext(ctx, + `INSERT INTO _migrations (id, name) VALUES ('abc12345', '0001_init'), ('def67890', '0002_users')`, + ); err != nil { + t.Fatalf("insert: %v", err) + } + + m := mymigrate.New(conn) + applied, err := m.Applied(ctx) + if err != nil { + t.Fatalf("Applied() error = %v", err) + } + if len(applied) != 2 { + t.Fatalf("Applied() len = %d, want 2", len(applied)) + } + if applied[0].Name != "0001_init" || applied[0].ID != "abc12345" { + t.Errorf("applied[0] = %+v, want {abc12345 0001_init}", applied[0]) + } + if applied[1].Name != "0002_users" || applied[1].ID != "def67890" { + t.Errorf("applied[1] = %+v, want {def67890 0002_users}", applied[1]) + } +} + +// TestAppliedEmptyMigrationsTable verifies Applied returns an empty +// slice (not an error) when _migrations exists but has no rows. +func TestAppliedEmptyMigrationsTable(t *testing.T) { + conn := connect(t) + ctx := t.Context() + + if _, err := conn.ExecContext(ctx, `CREATE TABLE _migrations (id VARCHAR(16), name VARCHAR(255))`); err != nil { + t.Fatalf("create table: %v", err) + } + + m := mymigrate.New(conn) + applied, err := m.Applied(ctx) + if err != nil { + t.Fatalf("Applied() error = %v", err) + } + if len(applied) != 0 { + t.Errorf("Applied() len = %d, want 0", len(applied)) + } +} + +// TestAppliedAfterDropTable verifies Applied still returns (nil, nil) after +// the _migrations table is dropped — exercises any prepared-statement +// caching the driver may do. +func TestAppliedAfterDropTable(t *testing.T) { + conn := connect(t) + ctx := t.Context() + + if _, err := conn.ExecContext(ctx, `CREATE TABLE _migrations (id VARCHAR(16), name VARCHAR(255))`); err != nil { + t.Fatalf("create: %v", err) + } + + m := mymigrate.New(conn) + if _, err := m.Applied(ctx); err != nil { + t.Fatalf("first Applied: %v", err) + } + + if _, err := conn.ExecContext(ctx, `DROP TABLE _migrations`); err != nil { + t.Fatalf("drop: %v", err) + } + + applied, err := m.Applied(ctx) + if err != nil { + t.Fatalf("Applied() after DROP TABLE error = %v, want nil", err) + } + if len(applied) != 0 { + t.Errorf("Applied() len = %d, want 0", len(applied)) + } +} diff --git a/database/sqlmigrate/pgmigrate/pgmigrate_test.go b/database/sqlmigrate/pgmigrate/pgmigrate_test.go new file mode 100644 index 0000000..0083465 --- /dev/null +++ b/database/sqlmigrate/pgmigrate/pgmigrate_test.go @@ -0,0 +1,160 @@ +package pgmigrate_test + +import ( + "os" + "testing" + + "github.com/jackc/pgx/v5" + + "github.com/therootcompany/golib/database/sqlmigrate/pgmigrate" +) + +// connect opens a pgx connection from PG_TEST_URL, skips the test if +// the env var is unset, and isolates the test in its own schema with +// automatic cleanup. +func connect(t *testing.T) *pgx.Conn { + t.Helper() + pgURL := os.Getenv("PG_TEST_URL") + if pgURL == "" { + t.Skip("PG_TEST_URL not set") + } + + ctx := t.Context() + conn, err := pgx.Connect(ctx, pgURL) + if err != nil { + t.Fatalf("connect: %v", err) + } + t.Cleanup(func() { _ = conn.Close(ctx) }) + + // Use a per-test schema so concurrent tests don't collide and + // _migrations is guaranteed not to exist on entry. + schema := "pgmigrate_test_" + sanitize(t.Name()) + if _, err := conn.Exec(ctx, "DROP SCHEMA IF EXISTS "+schema+" CASCADE"); err != nil { + t.Fatalf("drop schema: %v", err) + } + if _, err := conn.Exec(ctx, "CREATE SCHEMA "+schema); err != nil { + t.Fatalf("create schema: %v", err) + } + t.Cleanup(func() { + _, _ = conn.Exec(ctx, "DROP SCHEMA IF EXISTS "+schema+" CASCADE") + }) + if _, err := conn.Exec(ctx, "SET search_path TO "+schema); err != nil { + t.Fatalf("set search_path: %v", err) + } + + return conn +} + +// sanitize converts a test name to a valid PostgreSQL identifier suffix. +func sanitize(s string) string { + out := make([]byte, 0, len(s)) + for _, c := range []byte(s) { + switch { + case c >= 'a' && c <= 'z', c >= 'A' && c <= 'Z', c >= '0' && c <= '9': + out = append(out, c) + default: + out = append(out, '_') + } + } + return string(out) +} + +// TestAppliedNoMigrationsTable is the regression test for the bug where +// pgx surfaces error 42P01 lazily at rows.Err() rather than at Query(). +// Before the fix, this returned: reading rows: ERROR: relation +// "_migrations" does not exist (SQLSTATE 42P01). +func TestAppliedNoMigrationsTable(t *testing.T) { + conn := connect(t) + + m := pgmigrate.New(conn) + applied, err := m.Applied(t.Context()) + if err != nil { + t.Fatalf("Applied() error = %v, want nil", err) + } + if len(applied) != 0 { + t.Errorf("Applied() len = %d, want 0", len(applied)) + } +} + +// TestAppliedWithMigrationsTable verifies Applied reads existing rows. +func TestAppliedWithMigrationsTable(t *testing.T) { + conn := connect(t) + ctx := t.Context() + + if _, err := conn.Exec(ctx, ` + CREATE TABLE _migrations (id TEXT, name TEXT); + INSERT INTO _migrations (id, name) VALUES ('abc12345', '0001_init'); + INSERT INTO _migrations (id, name) VALUES ('def67890', '0002_users'); + `); err != nil { + t.Fatalf("setup: %v", err) + } + + m := pgmigrate.New(conn) + applied, err := m.Applied(ctx) + if err != nil { + t.Fatalf("Applied() error = %v", err) + } + if len(applied) != 2 { + t.Fatalf("Applied() len = %d, want 2", len(applied)) + } + if applied[0].Name != "0001_init" || applied[0].ID != "abc12345" { + t.Errorf("applied[0] = %+v, want {abc12345 0001_init}", applied[0]) + } + if applied[1].Name != "0002_users" || applied[1].ID != "def67890" { + t.Errorf("applied[1] = %+v, want {def67890 0002_users}", applied[1]) + } +} + +// TestAppliedEmptyMigrationsTable verifies Applied returns an empty +// slice (not an error) when _migrations exists but has no rows. +func TestAppliedEmptyMigrationsTable(t *testing.T) { + conn := connect(t) + ctx := t.Context() + + if _, err := conn.Exec(ctx, `CREATE TABLE _migrations (id TEXT, name TEXT)`); err != nil { + t.Fatalf("setup: %v", err) + } + + m := pgmigrate.New(conn) + applied, err := m.Applied(ctx) + if err != nil { + t.Fatalf("Applied() error = %v", err) + } + if len(applied) != 0 { + t.Errorf("Applied() len = %d, want 0", len(applied)) + } +} + +// TestAppliedAfterDropTable verifies Applied handles the case where the +// _migrations table once existed (so pgx may have cached its prepared +// statement) but has been dropped. This is the scenario most likely to +// trigger pgx's lazy 42P01 error at rows.Err() rather than at Query(). +func TestAppliedAfterDropTable(t *testing.T) { + conn := connect(t) + ctx := t.Context() + + if _, err := conn.Exec(ctx, `CREATE TABLE _migrations (id TEXT, name TEXT)`); err != nil { + t.Fatalf("create: %v", err) + } + + m := pgmigrate.New(conn) + // Prime pgx's prepared-statement cache by calling Applied successfully. + if _, err := m.Applied(ctx); err != nil { + t.Fatalf("first Applied: %v", err) + } + + // Now drop the table out from under pgx. The cached prepared statement + // references a relation that no longer exists; the next Applied call + // must still return (nil, nil), not an error. + if _, err := conn.Exec(ctx, `DROP TABLE _migrations`); err != nil { + t.Fatalf("drop: %v", err) + } + + applied, err := m.Applied(ctx) + if err != nil { + t.Fatalf("Applied() after DROP TABLE error = %v, want nil", err) + } + if len(applied) != 0 { + t.Errorf("Applied() len = %d, want 0", len(applied)) + } +}