diff --git a/modules/cabana/README.md b/modules/cabana/README.md index 7696f07..8106d9d 100644 --- a/modules/cabana/README.md +++ b/modules/cabana/README.md @@ -384,3 +384,5 @@ The database-backed tests start a PostgreSQL container through testcontainers-go The Phase 18 pieces (money field and column, write-only settings secrets, richeditor and tabs on settings forms, danger record actions) are covered by Docker-free unit tests in `money_field_test.go` (TestMoneyFieldCompile, TestMoneyFieldColumnGate, TestMoneyFieldValidate, TestMoneyListColumn), `settings_secret_test.go` (TestSettingsSecretCompile, TestSettingsSecretProjection, TestSettingsSecretPut), `settings_tabs_test.go` (TestSettingsTabs, TestSettingsRichEditor) and `record_action_danger_test.go` (TestRecordActionDanger), and by Postgres tests in `money_field_postgres_test.go` (TestMoneyFieldSave), `settings_secret_postgres_test.go` (TestSettingsSecretPostgres), the smoke tests in `payments_smoke_test.go` and the Phase 18 payload cases of TestPhase10OpenAPIConformance. `scripts/check-phase18.sh` runs them by name. The Phase 18.1 pieces (the taglist field and the flat reorder) are covered by Docker-free unit tests in `taglist_field_test.go` (TestTaglistFieldCompile, TestTaglistFieldValidate for the save check in both modes, TestTaglistFieldHydrate for the record GET) and `reorder_flat_test.go` (TestReorderFlatCompile, which also checks the localized list schema, and TestReorderFlatNodes for the save's node check), and by Postgres tests in `catalog_smoke_test.go` (TestTaglistFieldSmoke, TestReorderFlatSmoke with a partitioned subtest) and `reorder_test.go` (TestReorderPostgres for the tree reorder). The admin specs are `admin/tests/form/TaglistField.test.ts` and `admin/tests/views/ReorderView.test.ts`. `scripts/check-phase18.1.sh` runs them by name. + +A Detached bulk action (`pact.AdminBulkAction.Detached`) is covered by the Postgres test TestDetachedBulkAction in `bulk_detached_test.go`, on the roster fixture with two test-only actions: Run sees no transaction (`TxFromContext`) and holds no row lock (another connection's `SELECT ... FOR UPDATE NOWAIT` succeeds during Run); the empty, absent and partial-selection rules match the locked path; a `ValidationError` from Run is 422 and the rows written before it stay written; the same action without Detached still runs in the transaction holding the row locks. `scripts/check-phase18.1.sh` requires it in its postgres stage. diff --git a/modules/cabana/bulk_detached_test.go b/modules/cabana/bulk_detached_test.go new file mode 100644 index 0000000..20dda62 --- /dev/null +++ b/modules/cabana/bulk_detached_test.go @@ -0,0 +1,226 @@ +package cabana_test + +import ( + "context" + "net/http" + "os" + "path/filepath" + "strings" + "sync" + "testing" + + "git.golem15.com/golem15/summercms/modules/cabana" + "git.golem15.com/golem15/summercms/modules/pact" + "gorm.io/gorm" +) + +const ( + // ledgerSettle is a Detached bulk action; ledgerSettleLocked is the same + // action without Detached, the control. + ledgerSettle = "settle" + ledgerSettleLocked = "settle_locked" + // ledgerInvalid is the sentinel name settle refuses with a + // cabana.ValidationError, after the earlier rows were written. + ledgerInvalid = "Invalid" +) + +// ledgerRun is what one Run of a settle action observed. +type ledgerRun struct { + names []string + // sawTx is whether cabana.TxFromContext reported a transaction. + sawTx bool + // lockErr is the error of another connection's SELECT ... FOR UPDATE + // NOWAIT on each selected row, taken while Run held the records. + lockErr []error +} + +type ledgerSpy struct { + mu sync.Mutex + runs []ledgerRun +} + +func (s *ledgerSpy) add(r ledgerRun) { + s.mu.Lock() + defer s.mu.Unlock() + s.runs = append(s.runs, r) +} + +func (s *ledgerSpy) take() []ledgerRun { + s.mu.Lock() + defer s.mu.Unlock() + out := s.runs + s.runs = nil + return out +} + +// ledgerController is the roster controller with two more bulk actions, +// settle (Detached) and settle_locked, registered for this test only. +type ledgerController struct { + rosterController + spy *ledgerSpy + db *gorm.DB +} + +func (c ledgerController) settle(detached bool) pact.AdminBulkAction { + name := ledgerSettleLocked + if detached { + name = ledgerSettle + } + return pact.AdminBulkAction{ + Name: name, Label: "acme.roster::lang.people.activate", + Permissions: []string{"acme.roster.access"}, + Detached: detached, + Run: func(ctx context.Context, in pact.AdminBulkActionInput) (pact.AdminBulkActionResult, error) { + run := ledgerRun{names: rosterNames(in.Records)} + tx, sawTx := cabana.TxFromContext(ctx) + run.sawTx = sawTx + for _, record := range in.Records { + id := record.(*rosterPerson).ID + run.lockErr = append(run.lockErr, c.db.Transaction(func(tx *gorm.DB) error { + var got []uint + return tx.Raw(`SELECT id FROM roster_people WHERE id = ? FOR UPDATE NOWAIT`, id).Scan(&got).Error + })) + } + c.spy.add(run) + changed := 0 + for _, record := range in.Records { + person := record.(*rosterPerson) + if person.Name == ledgerInvalid { + return pact.AdminBulkActionResult{}, &cabana.ValidationError{Details: map[string]any{"name": []string{"refused by the test"}}} + } + // A detached Run makes its own short write; the locked + // control writes through the framework transaction, which + // holds the row lock. + write := c.db.WithContext(ctx) + if sawTx { + write = tx + } + if err := write.Unscoped().Model(&rosterPerson{}).Where("id = ?", person.ID).Update("active", true).Error; err != nil { + return pact.AdminBulkActionResult{}, err + } + changed++ + } + return pact.AdminBulkActionResult{Affected: changed}, nil + }, + } +} + +func (c ledgerController) AdminBulkActions() []pact.AdminBulkAction { + return append(c.rosterController.AdminBulkActions(), c.settle(true), c.settle(false)) +} + +// newLedgerEnv is the roster environment whose people list also declares +// settle and settle_locked. +func newLedgerEnv(t *testing.T) (*rosterEnv, *gorm.DB, *ledgerSpy) { + t.Helper() + list := "controllers/people/config_list.yaml" + raw, err := os.ReadFile(filepath.Join(rosterDir, list)) + if err != nil { + t.Fatal(err) + } + const declared = "bulkActions: [activate, archive]" + if !strings.Contains(string(raw), declared) { + t.Fatalf("%s does not declare %q", list, declared) + } + body := strings.Replace(string(raw), declared, "bulkActions: [activate, archive, "+ledgerSettle+", "+ledgerSettleLocked+"]", 1) + spy := &ledgerSpy{} + env, gdb := newRosterEnvWith(t, func(p *rosterPlugin) { + p.fsys = rosterTree(t, map[string]string{list: body}) + db := p.db + p.wrap = func(ctl rosterController) pact.AdminController { + return ledgerController{rosterController: ctl, spy: spy, db: db} + } + }) + return env, gdb, spy +} + +// TestDetachedBulkAction covers pact.AdminBulkAction.Detached on Postgres +// (gap closure 18.1-08, WR-01): a detached Run sees no transaction on the +// context and holds no row lock (another connection's SELECT ... FOR UPDATE +// NOWAIT on a selected row succeeds while Run runs); the empty, absent and +// partial-selection rules are those of the locked path; a ValidationError +// from Run is 422 and the rows Run wrote before it stay written; the same +// action without Detached still runs in a transaction that holds the row +// locks. +func TestDetachedBulkAction(t *testing.T) { + env, gdb, spy := newLedgerEnv(t) + settle := rosterPeople + "/bulk/" + ledgerSettle + settleLocked := rosterPeople + "/bulk/" + ledgerSettleLocked + + t.Run("Run sees no transaction and holds no row lock", func(t *testing.T) { + ada := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Ada"}) + bob := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Bob"}) + rec := env.expect(t, http.StatusOK, http.MethodPost, settle, rosterIDs(bob, ada), "bearer") + if result := rosterBulkResult(t, rec); result.Affected != 2 { + t.Fatalf("affected = %d, want 2", result.Affected) + } + runs := spy.take() + if len(runs) != 1 || strings.Join(runs[0].names, ",") != "Ada,Bob" { + t.Fatalf("runs %+v, want one over Ada and Bob in id order", runs) + } + if runs[0].sawTx { + t.Fatal("a detached Run found a transaction on the context") + } + for i, err := range runs[0].lockErr { + if err != nil { + t.Fatalf("row %d was locked during a detached Run: %v", i, err) + } + } + if !rosterLoad(t, gdb, ada).Active || !rosterLoad(t, gdb, bob).Active { + t.Fatal("the detached writes were not stored") + } + }) + + t.Run("an empty selection is 422 and an absent one is affected 0, without Run", func(t *testing.T) { + rec := env.expect(t, http.StatusUnprocessableEntity, http.MethodPost, settle, `{"ids":[]}`, "bearer") + actErrorCode(t, rec.Body.Bytes(), "validation_failed") + rec = env.expect(t, http.StatusOK, http.MethodPost, settle, rosterIDs(999998, 999999), "bearer") + if result := rosterBulkResult(t, rec); result.Affected != 0 || result.Message != "" { + t.Fatalf("absent selection: %+v", result) + } + if runs := spy.take(); len(runs) != 0 { + t.Fatalf("Run was called: %+v", runs) + } + }) + + t.Run("a partial selection is 409 without Run", func(t *testing.T) { + own := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Own"}) + foreign := rosterInsert(t, gdb, rosterPerson{Tenant: "other", Name: "Foreign"}) + rec := env.expect(t, http.StatusConflict, http.MethodPost, settle, rosterIDs(own, foreign), "bearer") + actErrorCode(t, rec.Body.Bytes(), "conflict") + if runs := spy.take(); len(runs) != 0 { + t.Fatalf("Run was called for a partial selection: %+v", runs) + } + if rosterLoad(t, gdb, own).Active || rosterLoad(t, gdb, foreign).Active { + t.Fatal("a partial selection changed a row") + } + }) + + t.Run("a ValidationError is 422 and earlier writes stay", func(t *testing.T) { + first := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "First"}) + invalid := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: ledgerInvalid}) + rec := env.expect(t, http.StatusUnprocessableEntity, http.MethodPost, settle, rosterIDs(first, invalid), "bearer") + actErrorCode(t, rec.Body.Bytes(), "validation_failed") + if runs := spy.take(); len(runs) != 1 { + t.Fatalf("runs %+v, want 1", runs) + } + if !rosterLoad(t, gdb, first).Active { + t.Fatal("a detached action's write before its failure was rolled back") + } + if rosterLoad(t, gdb, invalid).Active { + t.Fatal("the refused row was written") + } + }) + + t.Run("without Detached Run holds a transaction and the row locks", func(t *testing.T) { + carl := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Carl"}) + env.expect(t, http.StatusOK, http.MethodPost, settleLocked, rosterIDs(carl), "bearer") + runs := spy.take() + if len(runs) != 1 || !runs[0].sawTx { + t.Fatalf("runs %+v, want one inside the framework transaction", runs) + } + if len(runs[0].lockErr) != 1 || runs[0].lockErr[0] == nil || !strings.Contains(runs[0].lockErr[0].Error(), "55P03") { + t.Fatalf("NOWAIT during a locked Run: %v, want lock_not_available (55P03)", runs[0].lockErr) + } + }) +}