From 57a849d114a4285ebb0fd66f4909f38a16519db9 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Thu, 22 Oct 2026 04:03:19 +0200 Subject: [PATCH] fix(lagoon): migrate:status reads the sets migrate applies (WR-12) --- docs/console/setup-and-maintenance.md | 2 +- docs/database/migrations.md | 2 +- modules/lagoon/README.md | 4 +- modules/lagoon/commands.go | 7 +-- modules/lagoon/example_test.go | 11 ++++- modules/lagoon/migrations.go | 24 +++++---- modules/lagoon/migrations_test.go | 71 +++++++++++++++++---------- modules/lagoon/pending_test.go | 49 ++++++++++++++++-- 8 files changed, 122 insertions(+), 48 deletions(-) diff --git a/docs/console/setup-and-maintenance.md b/docs/console/setup-and-maintenance.md index 550c87c5..090eb39c 100644 --- a/docs/console/setup-and-maintenance.md +++ b/docs/console/setup-and-maintenance.md @@ -14,7 +14,7 @@ Every application binary carries the framework's runtime commands. The examples |---------|-------|---------| | `migrate` | `--pretend`, `--force` | Runs the framework migrations, then every plugin's migrations in dependency order, printing how many each set applied and `Applied N migrations` (or `Nothing to migrate`). `--pretend` lists the pending migrations as `: ` and applies nothing. | | `migrate:rollback` | `--plugin `, `--pretend`, `--force` | Rolls back the last migration of the plugin; without the flag, of the last activated plugin that has migrations. Prints `Rolled back of `; `--pretend` only names it. | -| `migrate:status` | `--json` | Prints a table of plugin, history table and applied migration IDs. `--json` prints every declared migration of those plugins with `plugin`, `table`, `id` and `applied`. | +| `migrate:status` | `--json` | Prints a table of the framework's own migration sets and every plugin with migrations, with history table and applied migration IDs. `--json` prints every declared migration of those sets with `plugin`, `table`, `id` and `applied`. | ```sh ./bin/acme migrate --pretend diff --git a/docs/database/migrations.md b/docs/database/migrations.md index 595c53e7..68703c1b 100644 --- a/docs/database/migrations.md +++ b/docs/database/migrations.md @@ -81,7 +81,7 @@ The application binary has the migration commands: `migrate:rollback` rolls back the last applied migration of one plugin. Without `--plugin` it picks the last activated plugin that has migrations. To fix a migration you just wrote, roll it back, edit it and run `migrate` again. -`migrate:status` prints each plugin's history table and applied IDs. +`migrate:status` prints the history table and applied IDs of the framework's own sets (under history IDs such as `summercms.attach`) and of each plugin with migrations. The same operations are Go functions, which is how tests migrate a fresh database: diff --git a/modules/lagoon/README.md b/modules/lagoon/README.md index 4079f1b1..c22aa2ff 100644 --- a/modules/lagoon/README.md +++ b/modules/lagoon/README.md @@ -135,7 +135,7 @@ func (p *Plugin) Migrations() []*gormigrate.Migration { | `lagoon.DSN` | Reads `database.dsn` from config. | | `lagoon.Migrate` | Runs framework and plugin migrations in order. | | `lagoon.RollbackLast` | Rolls back the last migration of one plugin. | -| `lagoon.Status` | Lists applied migration IDs per plugin as `lagoon.StatusRow` values. | +| `lagoon.Status` | Lists the applied migration IDs of every set `lagoon.Migrate` applies, framework sets first (under their history IDs, such as `summercms.attach`), then each plugin with migrations, as `lagoon.StatusRow` values. | | `lagoon.DatabaseLabel` | Describes a DSN for a person as `host:port/name` (parsed with `pgconn.ParseConfig`), never the user or password; `not configured` when empty, `configured` when it does not parse. Used by the startup summaries of `serve` and `queue:work`. | | `lagoon.PendingMigrations` | Lists the migrations `lagoon.Migrate` would apply, as `lagoon.MigrationRef` values (`Plugin`, `ID`): the framework sets first, then each plugin in order, each in declaration order. A missing history table means the whole set is pending. Read-only; `serve` reports the count in its startup summary. | | `lagoon.HistoryTableName` | Returns the gormigrate history table for a plugin ID. | @@ -258,7 +258,7 @@ storage: |---------|-------|-------------| | `migrate` | `--pretend`, `--force` | Runs the framework migrations (`system_files`, the backend admin tables, `deferred_bindings` and the job queue), then each plugin's migrations in dependency order; prints `: N applied` per set and `Applied N migrations`, or `Nothing to migrate`. The applied lines count what was actually applied: when a migration fails, `migrate` prints the lines for the sets applied before it, then the error, and no `Applied` line. `--pretend` prints each pending migration as `: ` and `N migrations would run`, and applies nothing. In production on an interactive terminal it asks first (`--force` skips the question); off a terminal it never asks. | | `migrate:rollback` | `--plugin `, `--pretend`, `--force` | Rolls back the last applied migration of the given plugin; without the flag, of the last activated plugin that has migrations. Prints `Rolled back of `; `--pretend` names the migration and changes nothing. Production confirmation as for `migrate`. | -| `migrate:status` | `--json` | Prints a table of plugin, history table and applied migration IDs. `--json` prints a JSON array of every declared migration of those plugins: `plugin`, `table`, `id`, `applied`. | +| `migrate:status` | `--json` | Prints a table of the framework's own migration sets and every plugin with migrations, with history table and applied migration IDs. `--json` prints a JSON array of every declared migration of those sets: `plugin`, `table`, `id`, `applied`. | | `deferred:purge` | `--days `, `--pretend`, `--force` | Deletes deferred bindings older than `n` days (default `database.deferred_bindings.purge_days`, else 5), with the unattached uploads they hold (blobs after the commit) and the records created under deferral, and prints the counts. Records that were only linked are kept. Model types come from every plugin's `pact.HasModels`; the uploads bucket from the published one or `storage.uploads.bucket_url`. `--pretend` counts the bindings past the cut-off and deletes nothing; production confirmation as for `migrate`. | | `key:generate` | `--json` | Prints a fresh base64 32-byte key for `app.key`; writes nothing. Under `-q` it prints only the key; `--json` prints `{"key": "..."}`. | diff --git a/modules/lagoon/commands.go b/modules/lagoon/commands.go index 936125fe..f51753ad 100644 --- a/modules/lagoon/commands.go +++ b/modules/lagoon/commands.go @@ -257,9 +257,10 @@ question.`, { Name: "migrate:status", Description: "Show per-plugin migration history", - Long: `Lists each plugin with migrations, its history table and the migration IDs -applied. --json prints every declared migration of those plugins with its -table and whether it is applied, for scripts.`, + Long: `Lists the framework's own migration sets (uploads, backend admin, deferred +bindings, job queue) and each plugin with migrations, with its history table +and the migration IDs applied. --json prints every declared migration of those +sets with its table and whether it is applied, for scripts.`, Flags: []bonfire.Flag{jsonFlag}, Examples: []bonfire.Example{ {Command: "migrate:status", Description: "Show the applied migrations per plugin"}, diff --git a/modules/lagoon/example_test.go b/modules/lagoon/example_test.go index 2e0573b8..483eb6fa 100644 --- a/modules/lagoon/example_test.go +++ b/modules/lagoon/example_test.go @@ -359,9 +359,16 @@ func TestDocsMigrate(t *testing.T) { if err != nil { t.Fatal(err) } + // The framework's own sets come first (WR-12), then the plugin. + framework := []string{"summercms.attach", "summercms.cabana", lagoon.DeferredHistoryID, lagoon.QueueHistoryID} want := "acme.blog summer_migrations_acme_blog [20260101000100_create_posts 20260101000200_create_comments_and_categories]" - if len(lines) != 1 || lines[0] != want { - t.Fatalf("status = %q, want %q", lines, want) + if len(lines) != len(framework)+1 || lines[len(lines)-1] != want { + t.Fatalf("status = %q, want the framework sets and then %q", lines, want) + } + for i, id := range framework { + if !strings.HasPrefix(lines[i], id+" ") { + t.Errorf("status line %d = %q, want the framework set %s", i, lines[i], id) + } } if db.Migrator().HasTable("acme_blog_comments") { t.Fatal("rollback left acme_blog_comments") diff --git a/modules/lagoon/migrations.go b/modules/lagoon/migrations.go index 89b0460d..a351efe9 100644 --- a/modules/lagoon/migrations.go +++ b/modules/lagoon/migrations.go @@ -130,34 +130,40 @@ func lastMigrationPlugin(plugins []party.Plugin) string { return "" } -// StatusRow is one plugin's recorded migration ids. +// StatusRow is one migration set's recorded IDs: Plugin is the plugin ID or +// the framework set's history ID (for example summercms.attach), Table its +// history table. type StatusRow struct { Plugin string Table string IDs []string } -// Status reads each plugin history table. +// Status reads the history table of every set Migrate applies, in the +// same order: the framework's own sets first (uploads, backend admin, +// deferred bindings, job queue), then each plugin with at least one +// migration. A missing history table gives a row without IDs. func Status(gdb *gorm.DB, plugins []party.Plugin) ([]StatusRow, error) { if gdb == nil { return nil, fmt.Errorf("lagoon: gorm db is nil") } + sets, err := migrationSets(gdb, plugins) + if err != nil { + return nil, err + } var rows []StatusRow - for _, p := range plugins { - if _, ok := p.(pact.HasMigrations); !ok { - continue - } - table, err := HistoryTableName(p.ID()) + for _, s := range sets { + table, err := HistoryTableName(s.id) if err != nil { return nil, err } - row := StatusRow{Plugin: p.ID(), Table: table} + row := StatusRow{Plugin: s.id, Table: table} if !gdb.Migrator().HasTable(table) { rows = append(rows, row) continue } if err := gdb.Table(table).Order("id").Pluck("id", &row.IDs).Error; err != nil { - return nil, fmt.Errorf("lagoon: status %s: %w", p.ID(), err) + return nil, fmt.Errorf("lagoon: status %s: %w", s.id, err) } if row.IDs == nil { row.IDs = []string{} diff --git a/modules/lagoon/migrations_test.go b/modules/lagoon/migrations_test.go index e04b6da6..3bdb4d49 100644 --- a/modules/lagoon/migrations_test.go +++ b/modules/lagoon/migrations_test.go @@ -186,18 +186,43 @@ func TestTwoPluginMigrationSetsIsolated(t *testing.T) { t.Fatalf("idempotent migrate: %v", err) } - status, err := Status(gdb, plugins) - if err != nil { - t.Fatal(err) + // Status lists the sets Migrate applies (WR-12): the framework sets + // first, in migrationSets order, then the plugins. + statusOf := func(t *testing.T) (alpha, beta StatusRow) { + t.Helper() + status, err := Status(gdb, plugins) + if err != nil { + t.Fatal(err) + } + framework := []string{"summercms.attach", "summercms.cabana", DeferredHistoryID, QueueHistoryID} + if len(status) != len(framework)+2 { + t.Fatalf("status rows = %+v", status) + } + for i, id := range framework { + if status[i].Plugin != id { + t.Fatalf("status row %d = %+v, want the framework set %s", i, status[i], id) + } + } + found := map[string]StatusRow{} + for _, row := range status[len(framework):] { + found[row.Plugin] = row + } + var ok bool + if alpha, ok = found["demo.alpha"]; !ok { + t.Fatalf("no demo.alpha row: %+v", status) + } + if beta, ok = found["demo.beta"]; !ok { + t.Fatalf("no demo.beta row: %+v", status) + } + return alpha, beta } - if len(status) != 2 { - t.Fatalf("status rows = %+v", status) + + alphaRow, betaRow := statusOf(t) + if alphaRow.Table != "summer_migrations_demo_alpha" || strings.Join(alphaRow.IDs, ",") != "202609170001_create_alpha" { + t.Fatalf("alpha status = %+v", alphaRow) } - if status[0].Plugin != "demo.alpha" || status[0].Table != "summer_migrations_demo_alpha" || strings.Join(status[0].IDs, ",") != "202609170001_create_alpha" { - t.Fatalf("alpha status = %+v", status[0]) - } - if status[1].Plugin != "demo.beta" || status[1].Table != "summer_migrations_demo_beta" || strings.Join(status[1].IDs, ",") != "202609170001_create_beta,202609170002_seed_beta" { - t.Fatalf("beta status = %+v", status[1]) + if betaRow.Table != "summer_migrations_demo_beta" || strings.Join(betaRow.IDs, ",") != "202609170001_create_beta,202609170002_seed_beta" { + t.Fatalf("beta status = %+v", betaRow) } if !gdb.Migrator().HasTable("lagoon_alpha") || !gdb.Migrator().HasTable("lagoon_beta") { @@ -225,15 +250,12 @@ func TestTwoPluginMigrationSetsIsolated(t *testing.T) { t.Fatal("schema rollback must not run on the first beta RollbackLast") } - status, err = Status(gdb, plugins) - if err != nil { - t.Fatal(err) + alphaRow, betaRow = statusOf(t) + if strings.Join(alphaRow.IDs, ",") != "202609170001_create_alpha" { + t.Fatalf("alpha history changed: %+v", alphaRow) } - if strings.Join(status[0].IDs, ",") != "202609170001_create_alpha" { - t.Fatalf("alpha history changed: %+v", status[0]) - } - if strings.Join(status[1].IDs, ",") != "202609170001_create_beta" { - t.Fatalf("beta history = %+v", status[1]) + if strings.Join(betaRow.IDs, ",") != "202609170001_create_beta" { + t.Fatalf("beta history = %+v", betaRow) } if err := RollbackLast(gdb, plugins, "demo.beta"); err != nil { @@ -246,15 +268,12 @@ func TestTwoPluginMigrationSetsIsolated(t *testing.T) { t.Fatal("alpha table must survive beta rollback") } - status, err = Status(gdb, plugins) - if err != nil { - t.Fatal(err) + alphaRow, betaRow = statusOf(t) + if len(alphaRow.IDs) != 1 { + t.Fatalf("alpha history after beta down = %+v", alphaRow) } - if len(status[0].IDs) != 1 { - t.Fatalf("alpha history after beta down = %+v", status[0]) - } - if len(status[1].IDs) != 0 { - t.Fatalf("beta history should be empty, got %+v", status[1]) + if len(betaRow.IDs) != 0 { + t.Fatalf("beta history should be empty, got %+v", betaRow) } } diff --git a/modules/lagoon/pending_test.go b/modules/lagoon/pending_test.go index 938e86a9..789b83cb 100644 --- a/modules/lagoon/pending_test.go +++ b/modules/lagoon/pending_test.go @@ -5,11 +5,13 @@ import ( "context" "encoding/json" "fmt" + "slices" "strings" "testing" "time" "git.golem15.com/golem15/summercms/modules/bonfire" + "git.golem15.com/golem15/summercms/modules/lagoon/attach" "git.golem15.com/golem15/summercms/modules/party" "github.com/go-gormigrate/gormigrate/v2" "gorm.io/gorm" @@ -259,8 +261,10 @@ func TestDeferredPurgePretend(t *testing.T) { } } -// TestMigrateStatusJSON: migrate:status --json lists every declared -// migration of the plugins with its table and applied flag, under -q too. +// TestMigrateStatusJSON (WR-12): migrate:status --json lists every declared +// migration of the framework sets and the plugins with its table and +// applied flag, under -q too: its pending entries are exactly what +// PendingMigrations returns, a pending framework migration included. func TestMigrateStatusJSON(t *testing.T) { db, dsn := dedicatedDB(t, "lagoon_status_json") gdb, err := Use(t.Context(), db) @@ -272,6 +276,20 @@ func TestMigrateStatusJSON(t *testing.T) { if err := Migrate(gdb, []party.Plugin{alpha}); err != nil { t.Fatal(err) } + // WR-12: a pending framework migration is listed too. Forget the newest + // attach migration so that it is pending again. + attachTable, err := HistoryTableName("summercms.attach") + if err != nil { + t.Fatal(err) + } + forgotten := attach.Migrations[len(attach.Migrations)-1].ID + if err := gdb.Exec(`DELETE FROM `+attachTable+` WHERE id = ?`, forgotten).Error; err != nil { + t.Fatal(err) + } + pending, err := PendingMigrations(gdb, plugins) + if err != nil { + t.Fatal(err) + } for _, args := range [][]string{{"migrate:status", "--json"}, {"migrate:status", "--json", "-q"}} { out, errOut, code, _ := runLagoon(t, "development", dsn, plugins, args...) if code != bonfire.ExitOK { @@ -281,13 +299,36 @@ func TestMigrateStatusJSON(t *testing.T) { if err := json.Unmarshal([]byte(out), &list); err != nil { t.Fatalf("%v: not JSON: %v\n%s", args, err, out) } + var unapplied, demo []statusJSON + for _, e := range list { + if !e.Applied { + unapplied = append(unapplied, e) + } + if strings.HasPrefix(e.Plugin, "demo.") { + demo = append(demo, e) + } + } + var wantUnapplied []statusJSON + for _, ref := range pending { + table, err := HistoryTableName(ref.Plugin) + if err != nil { + t.Fatal(err) + } + wantUnapplied = append(wantUnapplied, statusJSON{Plugin: ref.Plugin, Table: table, ID: ref.ID}) + } + if fmt.Sprint(unapplied) != fmt.Sprint(wantUnapplied) { + t.Errorf("%v: pending entries = %+v, want PendingMigrations' %+v", args, unapplied, wantUnapplied) + } + if !slices.Contains(list, statusJSON{Plugin: "summercms.attach", Table: attachTable, ID: forgotten}) { + t.Errorf("%v: the pending framework migration %s is not listed:\n%s", args, forgotten, out) + } want := []statusJSON{ {Plugin: "demo.alpha", Table: "summer_migrations_demo_alpha", ID: "202610210001_create_pending_alpha", Applied: true}, {Plugin: "demo.beta", Table: "summer_migrations_demo_beta", ID: "202610210001_create_pending_beta"}, {Plugin: "demo.beta", Table: "summer_migrations_demo_beta", ID: "202610210002_seed_pending_beta"}, } - if fmt.Sprint(list) != fmt.Sprint(want) { - t.Errorf("%v: status = %+v, want %+v", args, list, want) + if fmt.Sprint(demo) != fmt.Sprint(want) { + t.Errorf("%v: plugin status = %+v, want %+v", args, demo, want) } if !strings.Contains(out, `"applied": false`) { t.Errorf("%v: pending entries lack \"applied\": false:\n%s", args, out)