diff --git a/cluster/registry/registry.go b/cluster/registry/registry.go index 64a26635..590a212f 100644 --- a/cluster/registry/registry.go +++ b/cluster/registry/registry.go @@ -78,7 +78,7 @@ var ( // migrations maps a schema version to the migration upgrading it to the next version. // Always append migrations at the end, never insert in the middle. - migrations = map[schema.MigrationKey]schema.Migration{ + migrations = schema.Migrations{ {From: 1, To: 2}: schema.Query(fmt.Sprintf(`ALTER TABLE node_registry ADD COLUMN IF NOT EXISTS role TEXT NOT NULL DEFAULT '%s'`, RoleActive)), } ) diff --git a/db/schema/schema.go b/db/schema/schema.go index 96ab93d0..ad4c3d70 100644 --- a/db/schema/schema.go +++ b/db/schema/schema.go @@ -48,13 +48,13 @@ const ( // Migrate creates or upgrades the named store's schema in one transaction, or creates a new // database using the "create" function. Both versions follow from the steps: the target is the -// version after the last step (see targetVersionOf), the minimum the version after the newest +// version after the last step (see Migrations.targetVersion), the minimum the version after the newest // breaking step, or 1 (see package doc). -func Migrate(db *sql.DB, dialect Dialect, store string, create MigrateFunc, migrations map[MigrationKey]Migration) error { +func Migrate(db *sql.DB, dialect Dialect, store string, create MigrateFunc, migrations Migrations) error { if dialect != Postgres && dialect != SQLite { return fmt.Errorf("unsupported schema dialect %d", dialect) } - targetVersion, err := targetVersionOf(migrations) + targetVersion, err := migrations.targetVersion() if err != nil { return fmt.Errorf("invalid %s migration steps: %w", store, err) } @@ -70,7 +70,7 @@ func Migrate(db *sql.DB, dialect Dialect, store string, create MigrateFunc, migr return fmt.Errorf("cannot acquire %s schema advisory lock: %w", store, err) } } - minVersion := minVersionOf(migrations) + minVersion := migrations.minVersion() if err := ensureVersionTable(tx, dialect); err != nil { return err } @@ -112,10 +112,10 @@ func Migrate(db *sql.DB, dialect Dialect, store string, create MigrateFunc, migr return tx.Commit() } -// targetVersionOf returns the schema version a store's steps lead to: the last step's To, or 1 +// targetVersion returns the schema version the steps lead to: the last step's To, or 1 // without steps. Steps go one version at a time and must be contiguous, so a forgotten or // mis-keyed one fails at startup. -func targetVersionOf(migrations map[MigrationKey]Migration) (int, error) { +func (migrations Migrations) targetVersion() (int, error) { if len(migrations) == 0 { return 1, nil } @@ -137,9 +137,9 @@ func targetVersionOf(migrations map[MigrationKey]Migration) (int, error) { return last, nil } -// minVersionOf returns the oldest schema version whose code still runs on the schema the steps +// minVersion returns the oldest schema version whose code still runs on the schema the steps // lead to: the To of the newest breaking step, or 1 if no step is breaking -func minVersionOf(migrations map[MigrationKey]Migration) int { +func (migrations Migrations) minVersion() int { minVersion := 1 for k, step := range migrations { if step.Breaking && k.To > minVersion { diff --git a/db/schema/schema_internal_test.go b/db/schema/schema_internal_test.go index 861b1284..c0d121e5 100644 --- a/db/schema/schema_internal_test.go +++ b/db/schema/schema_internal_test.go @@ -6,56 +6,56 @@ import ( "github.com/stretchr/testify/require" ) -func TestMinVersionOf(t *testing.T) { +func TestMigrations_MinVersion(t *testing.T) { breaking := Breaking(Nop) cases := []struct { name string - migrations map[MigrationKey]Migration + migrations Migrations expected int }{ - {"no steps", map[MigrationKey]Migration{}, 1}, - {"only additive steps", map[MigrationKey]Migration{{From: 1, To: 2}: Nop, {From: 2, To: 3}: Nop, {From: 3, To: 4}: Nop}, 1}, - {"one breaking step", map[MigrationKey]Migration{{From: 1, To: 2}: Nop, {From: 2, To: 3}: Nop, {From: 3, To: 4}: breaking, {From: 4, To: 5}: Nop, {From: 5, To: 6}: Nop}, 4}, - {"newest breaking step wins", map[MigrationKey]Migration{{From: 1, To: 2}: Nop, {From: 2, To: 3}: breaking, {From: 3, To: 4}: Nop, {From: 4, To: 5}: Nop, {From: 5, To: 6}: breaking, {From: 6, To: 7}: Nop}, 6}, - {"breaking last step", map[MigrationKey]Migration{{From: 4, To: 5}: Nop, {From: 5, To: 6}: breaking}, 6}, - {"breaking first step", map[MigrationKey]Migration{{From: 1, To: 2}: breaking, {From: 2, To: 3}: Nop}, 2}, + {"no steps", Migrations{}, 1}, + {"only additive steps", Migrations{{From: 1, To: 2}: Nop, {From: 2, To: 3}: Nop, {From: 3, To: 4}: Nop}, 1}, + {"one breaking step", Migrations{{From: 1, To: 2}: Nop, {From: 2, To: 3}: Nop, {From: 3, To: 4}: breaking, {From: 4, To: 5}: Nop, {From: 5, To: 6}: Nop}, 4}, + {"newest breaking step wins", Migrations{{From: 1, To: 2}: Nop, {From: 2, To: 3}: breaking, {From: 3, To: 4}: Nop, {From: 4, To: 5}: Nop, {From: 5, To: 6}: breaking, {From: 6, To: 7}: Nop}, 6}, + {"breaking last step", Migrations{{From: 4, To: 5}: Nop, {From: 5, To: 6}: breaking}, 6}, + {"breaking first step", Migrations{{From: 1, To: 2}: breaking, {From: 2, To: 3}: Nop}, 2}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - require.Equal(t, c.expected, minVersionOf(c.migrations)) + require.Equal(t, c.expected, c.migrations.minVersion()) }) } } -func TestTargetVersionOf(t *testing.T) { +func TestMigrations_TargetVersion(t *testing.T) { cases := []struct { name string - migrations map[MigrationKey]Migration + migrations Migrations expected int }{ {"no steps", nil, 1}, - {"one step", map[MigrationKey]Migration{{From: 1, To: 2}: Nop}, 2}, - {"steps starting later", map[MigrationKey]Migration{{From: 14, To: 15}: Nop, {From: 15, To: 16}: Nop, {From: 16, To: 17}: Nop}, 17}, // A store added to Postgres at version 14 + {"one step", Migrations{{From: 1, To: 2}: Nop}, 2}, + {"steps starting later", Migrations{{From: 14, To: 15}: Nop, {From: 15, To: 16}: Nop, {From: 16, To: 17}: Nop}, 17}, // A store added to Postgres at version 14 } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - version, err := targetVersionOf(c.migrations) + version, err := c.migrations.targetVersion() require.Nil(t, err) require.Equal(t, c.expected, version) }) } } -func TestTargetVersionOf_Invalid(t *testing.T) { - _, err := targetVersionOf(map[MigrationKey]Migration{{From: 1, To: 2}: Nop, {From: 3, To: 4}: Nop, {From: 4, To: 5}: Nop}) +func TestMigrations_TargetVersion_Invalid(t *testing.T) { + _, err := Migrations{{From: 1, To: 2}: Nop, {From: 3, To: 4}: Nop, {From: 4, To: 5}: Nop}.targetVersion() require.ErrorContains(t, err, "missing migration step from version 2 to 3") - _, err = targetVersionOf(map[MigrationKey]Migration{{From: 0, To: 1}: Nop, {From: 1, To: 2}: Nop}) + _, err = Migrations{{From: 0, To: 1}: Nop, {From: 1, To: 2}: Nop}.targetVersion() require.ErrorContains(t, err, "migration step from version 0 is invalid") // A step goes exactly one version: a key skipping one would make fresh installs record a // version that existing installs never reach - _, err = targetVersionOf(map[MigrationKey]Migration{{From: 1, To: 2}: Nop, {From: 2, To: 4}: Nop}) + _, err = Migrations{{From: 1, To: 2}: Nop, {From: 2, To: 4}: Nop}.targetVersion() require.ErrorContains(t, err, "migration step from version 2 to 4 is invalid, a step goes one version at a time") - _, err = targetVersionOf(map[MigrationKey]Migration{{From: 3, To: 3}: Nop}) + _, err = Migrations{{From: 3, To: 3}: Nop}.targetVersion() require.ErrorContains(t, err, "migration step from version 3 to 3 is invalid, a step goes one version at a time") } diff --git a/db/schema/schema_test.go b/db/schema/schema_test.go index 0d0de581..e4343b32 100644 --- a/db/schema/schema_test.go +++ b/db/schema/schema_test.go @@ -65,7 +65,7 @@ func TestMigrate_AppliesMigrationSteps(t *testing.T) { forEachDialect(t, func(t *testing.T, d *sql.DB, dialect schema.Dialect) { require.Nil(t, schema.Migrate(d, dialect, "things", testCreate, nil)) // A newer version of the code migrates 1 -> 3 step by step, in order - migrations := map[schema.MigrationKey]schema.Migration{ + migrations := schema.Migrations{ {From: 1, To: 2}: schema.Func(func(tx *sql.Tx) error { _, err := tx.Exec(`ALTER TABLE things ADD COLUMN color TEXT NOT NULL DEFAULT ''`) return err @@ -88,7 +88,7 @@ func TestMigrate_TargetVersionFollowsSteps(t *testing.T) { forEachDialect(t, func(t *testing.T, d *sql.DB, dialect schema.Dialect) { require.Nil(t, schema.Migrate(d, dialect, "things", testCreate, nil)) require.Equal(t, 1, storeVersion(t, d, dialect, "things")) - migrations := map[schema.MigrationKey]schema.Migration{ + migrations := schema.Migrations{ {From: 1, To: 2}: schema.Query(`ALTER TABLE things ADD COLUMN color TEXT NOT NULL DEFAULT ''`), {From: 2, To: 3}: schema.Query(`ALTER TABLE things ADD COLUMN size INT NOT NULL DEFAULT 0`), } @@ -102,7 +102,7 @@ func TestMigrate_TargetVersionFollowsSteps(t *testing.T) { func TestMigrate_GapInStepsFails(t *testing.T) { // A missing step in the middle fails before anything is created or migrated forEachDialect(t, func(t *testing.T, d *sql.DB, dialect schema.Dialect) { - err := schema.Migrate(d, dialect, "things", testCreate, map[schema.MigrationKey]schema.Migration{{From: 1, To: 2}: schema.Nop, {From: 3, To: 4}: schema.Nop}) + err := schema.Migrate(d, dialect, "things", testCreate, schema.Migrations{{From: 1, To: 2}: schema.Nop, {From: 3, To: 4}: schema.Nop}) require.ErrorContains(t, err, "missing migration step from version 2 to 3") _, err = d.Exec(`INSERT INTO things (id, name) VALUES ('a', 'thing a')`) require.Error(t, err) // Not created @@ -112,8 +112,8 @@ func TestMigrate_GapInStepsFails(t *testing.T) { func TestMigrate_ClosureCarriesConfig(t *testing.T) { // Migrations needing config take it via closure at map-construction time; there is no // params plumbing in the framework itself - migrationsFor := func(defaultName string) map[schema.MigrationKey]schema.Migration { - return map[schema.MigrationKey]schema.Migration{ + migrationsFor := func(defaultName string) schema.Migrations { + return schema.Migrations{ {From: 1, To: 2}: schema.Query(fmt.Sprintf(`ALTER TABLE things ADD COLUMN nick TEXT NOT NULL DEFAULT '%s'`, defaultName)), } } @@ -151,7 +151,7 @@ func TestMigrate_NewerVersionWarnsAndStarts(t *testing.T) { func TestMigrate_BreakingNewerSchemaFails(t *testing.T) { // A breaking migration raises the store's minimum version: older code must not run on it forEachDialect(t, func(t *testing.T, d *sql.DB, dialect schema.Dialect) { - require.Nil(t, schema.Migrate(d, dialect, "things", testCreate, map[schema.MigrationKey]schema.Migration{{From: 1, To: 2}: schema.Nop, {From: 2, To: 3}: schema.Breaking(schema.Nop)})) + require.Nil(t, schema.Migrate(d, dialect, "things", testCreate, schema.Migrations{{From: 1, To: 2}: schema.Nop, {From: 2, To: 3}: schema.Breaking(schema.Nop)})) err := schema.Migrate(d, dialect, "things", testCreate, stepsTo(2)) require.ErrorContains(t, err, "things database schema version 3 needs ntfy that supports at least version 3, this version supports up to 2") }) @@ -161,7 +161,7 @@ func TestMigrate_BreakingStepRaisesMinVersion(t *testing.T) { // The minimum version follows from the steps: the version after the newest breaking step forEachDialect(t, func(t *testing.T, d *sql.DB, dialect schema.Dialect) { require.Nil(t, schema.Migrate(d, dialect, "things", testCreate, nil)) - require.Nil(t, schema.Migrate(d, dialect, "things", testCreate, map[schema.MigrationKey]schema.Migration{ + require.Nil(t, schema.Migrate(d, dialect, "things", testCreate, schema.Migrations{ {From: 1, To: 2}: schema.Breaking(schema.Nop), {From: 2, To: 3}: schema.Nop, })) @@ -172,7 +172,7 @@ func TestMigrate_BreakingStepRaisesMinVersion(t *testing.T) { func TestMigrate_FreshCreateTakesMinVersionFromSteps(t *testing.T) { // A fresh database is created at the target version directly, but its minimum still comes from the steps forEachDialect(t, func(t *testing.T, d *sql.DB, dialect schema.Dialect) { - require.Nil(t, schema.Migrate(d, dialect, "things", testCreate, map[schema.MigrationKey]schema.Migration{ + require.Nil(t, schema.Migrate(d, dialect, "things", testCreate, schema.Migrations{ {From: 1, To: 2}: schema.Nop, {From: 2, To: 3}: schema.Breaking(schema.Nop), })) @@ -200,7 +200,7 @@ func TestMigrate_AddsMinVersionToAnExistingVersionTable(t *testing.T) { require.Nil(t, err) // A breaking migration to version 2 locks out code that supports only version 1 - require.Nil(t, schema.Migrate(d, dialect, "things", testCreate, map[schema.MigrationKey]schema.Migration{{From: 1, To: 2}: schema.Breaking(schema.Nop)})) + require.Nil(t, schema.Migrate(d, dialect, "things", testCreate, schema.Migrations{{From: 1, To: 2}: schema.Breaking(schema.Nop)})) require.Equal(t, 2, storeMinVersion(t, d, dialect, "things")) require.Error(t, schema.Migrate(d, dialect, "things", testCreate, nil)) }) @@ -210,7 +210,7 @@ func TestMigrate_VersionTableCreatedWhenMissing(t *testing.T) { // A database without a version table gets one with both columns, filled from the steps forEachDialect(t, func(t *testing.T, d *sql.DB, dialect schema.Dialect) { require.Empty(t, versionTableColumns(t, d, dialect)) - require.Nil(t, schema.Migrate(d, dialect, "things", testCreate, map[schema.MigrationKey]schema.Migration{{From: 1, To: 2}: schema.Nop, {From: 2, To: 3}: schema.Breaking(schema.Nop)})) + require.Nil(t, schema.Migrate(d, dialect, "things", testCreate, schema.Migrations{{From: 1, To: 2}: schema.Nop, {From: 2, To: 3}: schema.Breaking(schema.Nop)})) require.ElementsMatch(t, []string{versionTableKeyColumn(dialect), "version", "min_version"}, versionTableColumns(t, d, dialect)) require.Equal(t, 3, storeVersion(t, d, dialect, "things")) require.Equal(t, 3, storeMinVersion(t, d, dialect, "things")) @@ -240,7 +240,7 @@ func TestMigrate_OldVersionTableGetsMinVersion(t *testing.T) { func TestMigrate_CurrentVersionTableIsLeftAlone(t *testing.T) { // A version table that already has min_version is not altered again, and its values stay forEachDialect(t, func(t *testing.T, d *sql.DB, dialect schema.Dialect) { - migrations := map[schema.MigrationKey]schema.Migration{{From: 1, To: 2}: schema.Breaking(schema.Nop)} + migrations := schema.Migrations{{From: 1, To: 2}: schema.Breaking(schema.Nop)} require.Nil(t, schema.Migrate(d, dialect, "things", testCreate, migrations)) require.Nil(t, schema.Migrate(d, dialect, "things", testCreate, migrations)) require.ElementsMatch(t, []string{versionTableKeyColumn(dialect), "version", "min_version"}, versionTableColumns(t, d, dialect)) @@ -252,7 +252,7 @@ func TestMigrate_CurrentVersionTableIsLeftAlone(t *testing.T) { func TestMigrate_MissingStepFails(t *testing.T) { forEachDialect(t, func(t *testing.T, d *sql.DB, dialect schema.Dialect) { require.Nil(t, schema.Migrate(d, dialect, "things", testCreate, nil)) - err := schema.Migrate(d, dialect, "things", testCreate, map[schema.MigrationKey]schema.Migration{{From: 2, To: 3}: schema.Nop}) // No step 1 -> 2 registered + err := schema.Migrate(d, dialect, "things", testCreate, schema.Migrations{{From: 2, To: 3}: schema.Nop}) // No step 1 -> 2 registered require.ErrorContains(t, err, "cannot find things migration step from version 1 to 2") }) } @@ -281,7 +281,7 @@ func TestMigrate_SQLiteReadsExistingSchemaVersionTable(t *testing.T) { require.Nil(t, err) _, err = d.Exec(`INSERT INTO schemaVersion VALUES (1, 1)`) require.Nil(t, err) - migrations := map[schema.MigrationKey]schema.Migration{ + migrations := schema.Migrations{ {From: 1, To: 2}: schema.Func(func(tx *sql.Tx) error { _, err := tx.Exec(`ALTER TABLE things ADD COLUMN color TEXT NOT NULL DEFAULT ''`) return err @@ -366,8 +366,8 @@ func versionTableKeyColumn(dialect schema.Dialect) string { } // stepsTo returns no-op steps leading to the given schema version -func stepsTo(version int) map[schema.MigrationKey]schema.Migration { - migrations := make(map[schema.MigrationKey]schema.Migration) +func stepsTo(version int) schema.Migrations { + migrations := make(schema.Migrations) for v := 1; v < version; v++ { migrations[schema.MigrationKey{From: v, To: v + 1}] = schema.Nop } diff --git a/db/schema/types.go b/db/schema/types.go index ca691006..a8d948c1 100644 --- a/db/schema/types.go +++ b/db/schema/types.go @@ -13,7 +13,7 @@ const ( // MigrateFunc applies one schema change inside the setup transaction: the initial creation of // a store's tables, or one step upgrading a store from version N to N+1. Migrations needing -// config capture it via closure, e.g. func migrations(cacheDuration time.Duration) map[MigrationKey]Migration. +// config capture it via closure, e.g. func migrations(cacheDuration time.Duration) Migrations. type MigrateFunc func(tx *sql.Tx) error // AsMigrateFunc converts a simple query to a migration function @@ -36,6 +36,10 @@ type Migration struct { Breaking bool // Older code cannot run on the result (see package doc): the store's minimum version becomes the step's To } +// Migrations are a store's migration steps, keyed by the versions each upgrades between. Steps go +// one version at a time without gaps, which Migrate checks at startup. +type Migrations map[MigrationKey]Migration + // Nop is a migration step that does nothing, for versions where a dialect has no work to do (e.g. // when only the other dialect's schema changed) var Nop = Func(func(_ *sql.Tx) error { diff --git a/message/cache_postgres_schema.go b/message/cache_postgres_schema.go index 6acd449d..9f35bff9 100644 --- a/message/cache_postgres_schema.go +++ b/message/cache_postgres_schema.go @@ -74,7 +74,7 @@ var ( // postgresMigrations maps a schema version to the migration upgrading it to the next // version. Always append migrations at the end, never insert in the middle. - postgresMigrations = map[schema.MigrationKey]schema.Migration{ + postgresMigrations = schema.Migrations{ {From: 14, To: 15}: schema.Query(postgresMigrate14To15CreateIndexQuery), {From: 15, To: 16}: schema.Query(postgresMigrate15To16AddClaimedAtQuery), {From: 16, To: 17}: schema.Query(postgresMigrate16To17CreateIndexQuery), diff --git a/message/cache_sqlite_schema.go b/message/cache_sqlite_schema.go index ae3bb07c..5bccb17c 100644 --- a/message/cache_sqlite_schema.go +++ b/message/cache_sqlite_schema.go @@ -183,8 +183,8 @@ var ( // sqliteMigrations returns the migration steps, keyed by the version they upgrade FROM. The // cache duration is carried into the 9 -> 10 step via closure (it backfills "expires" from it). // Always append migrations at the end, never insert in the middle. -func sqliteMigrations(cacheDuration time.Duration) map[schema.MigrationKey]schema.Migration { - return map[schema.MigrationKey]schema.Migration{ +func sqliteMigrations(cacheDuration time.Duration) schema.Migrations { + return schema.Migrations{ {From: 1, To: 2}: schema.Query(sqliteMigrate1To2AlterMessagesTableQuery), {From: 2, To: 3}: schema.Query(sqliteMigrate2To3AlterMessagesTableQuery), {From: 3, To: 4}: schema.Query(sqliteMigrate3To4AlterMessagesTableQuery), diff --git a/user/manager_postgres_schema.go b/user/manager_postgres_schema.go index 73f75fae..82590d4d 100644 --- a/user/manager_postgres_schema.go +++ b/user/manager_postgres_schema.go @@ -128,7 +128,7 @@ var ( // postgresMigrations maps a schema version to the migration upgrading it to the next // version. Always append migrations at the end, never insert in the middle. - postgresMigrations = map[schema.MigrationKey]schema.Migration{ + postgresMigrations = schema.Migrations{ {From: 6, To: 7}: schema.Query(postgresMigrate6To7UpdateQueries), {From: 7, To: 8}: schema.Query(postgresMigrate7To8UpdateQueries), {From: 8, To: 9}: schema.Nop, // 8 -> 9 repairs a SQLite-only foreign key defect; nothing to do on Postgres diff --git a/user/manager_sqlite_schema.go b/user/manager_sqlite_schema.go index 6c7ddb28..dd9dc44f 100644 --- a/user/manager_sqlite_schema.go +++ b/user/manager_sqlite_schema.go @@ -369,7 +369,7 @@ var ( // sqliteMigrations maps a schema version to the migration upgrading it to the next // version. Always append migrations at the end, never insert in the middle. - sqliteMigrations = map[schema.MigrationKey]schema.Migration{ + sqliteMigrations = schema.Migrations{ {From: 1, To: 2}: schema.Func(sqliteMigrateFrom1), {From: 2, To: 3}: schema.Query(sqliteMigrate2To3UpdateQueries), {From: 3, To: 4}: schema.Query(sqliteMigrate3To4UpdateQueries),