From 247c3235f6114a1426ab7eba838e4d63c910e5c3 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Thu, 24 Sep 2026 19:43:15 +0200 Subject: [PATCH] test(09-05): add failing tests for deterministic bulk delete - Empty selections are 422 and duplicates run once in primary-key order - A completed retry and an all-absent selection delete nothing and skip hooks - Mixed, hook, cancel, and concurrent requests keep the batch atomic --- cabana/bulk_test.go | 250 ++++++++++++++++++++++++++++++++++ cabana/crud.go | 6 + cabana/crud_lifecycle_test.go | 3 + cabana/crud_test.go | 39 +++++- cabana/http.go | 4 + 5 files changed, 300 insertions(+), 2 deletions(-) create mode 100644 cabana/bulk_test.go diff --git a/cabana/bulk_test.go b/cabana/bulk_test.go new file mode 100644 index 0000000..6db4a61 --- /dev/null +++ b/cabana/bulk_test.go @@ -0,0 +1,250 @@ +package cabana + +import ( + "bytes" + "context" + "fmt" + "net/http" + "net/http/httptest" + "sort" + "strings" + "sync" + "testing" + + "git.golem15.com/golem15/summercms/bouncer" + "gorm.io/gorm" +) + +func TestBulkDeleteEmpty(t *testing.T) { + cap := &captureRouter{} + (&service{}).mount(cap) + key := "POST /_admin/api/v1/{vendor}/{plugin}/{controller}/bulk-delete" + mw, ok := cap.middleware[key] + if !ok || !containsString(mw, "backend") { + t.Fatalf("missing %s in %v", key, cap.routes) + } + _, httpSvc, _, db, hooks := hookFixture(t) + hooks.perms = []string{"acme.demo.records"} + forbidden := crudBulk(httpSvc, []byte(`{"ids":[1]}`), principalCtx(hooks, &bouncer.Principal{ID: 4})) + if forbidden.Code != http.StatusForbidden || hooks.allocs.Load() != 0 || hooks.queries.Load() != 0 { + t.Fatalf("forbidden=%d allocs=%d queries=%d body=%s", forbidden.Code, hooks.allocs.Load(), hooks.queries.Load(), forbidden.Body.String()) + } + ctx := principalCtx(hooks, superUser()) + for _, body := range []string{`{"ids":[]}`, `{}`, `{"ids":null}`} { + rec := crudBulk(httpSvc, []byte(body), ctx) + if rec.Code != http.StatusUnprocessableEntity || !strings.Contains(rec.Body.String(), `"validation_failed"`) || !strings.Contains(rec.Body.String(), "The ids field is required.") { + t.Fatalf("body %s => %d %s", body, rec.Code, rec.Body.String()) + } + } + bad := crudBulk(httpSvc, []byte(`{"ids":["nope",1]}`), ctx) + if bad.Code != http.StatusUnprocessableEntity || !strings.Contains(bad.Body.String(), `"validation_failed"`) { + t.Fatalf("bad ids=%d %s", bad.Code, bad.Body.String()) + } + if n := countCrud(t, db); n != 0 { + t.Fatalf("rows=%d", n) + } +} + +func TestBulkDeleteDuplicates(t *testing.T) { + svc, _, cc, db, hooks := hookFixture(t) + rows := seedCrud(t, db, 2) + var got []uint + ctx := idCtx(hooks, &got) + res, err := svc.BulkDelete(ctx, cc, BulkDeleteInput{IDs: []any{rows[1].ID, rows[1].ID, rows[0].ID, float64(rows[0].ID)}}) + if err != nil || res.Deleted != 2 { + t.Fatalf("bulk=%+v err=%v", res, err) + } + if !sameUints(got, []uint{rows[0].ID, rows[1].ID}) { + t.Fatalf("hook ids=%v, want each id once in ascending order", got) + } + if n := countCrud(t, db); n != 0 { + t.Fatalf("rows=%d", n) + } +} + +func TestBulkDeleteOrder(t *testing.T) { + svc, _, cc, db, hooks := hookFixture(t) + rows := seedCrud(t, db, 3) + var got []uint + ctx := idCtx(hooks, &got) + res, err := svc.BulkDelete(ctx, cc, BulkDeleteInput{IDs: []any{rows[2].ID, rows[0].ID, rows[1].ID}}) + if err != nil || res.Deleted != 3 { + t.Fatalf("bulk=%+v err=%v", res, err) + } + want := []uint{rows[0].ID, rows[1].ID, rows[2].ID} + sort.Slice(want, func(i, j int) bool { return want[i] < want[j] }) + if !sameUints(got, want) { + t.Fatalf("hook ids=%v want %v", got, want) + } +} + +func TestBulkDeleteIdempotent(t *testing.T) { + svc, _, cc, db, hooks := hookFixture(t) + rows := seedCrud(t, db, 2) + var got []uint + ctx := idCtx(hooks, &got) + first, err := svc.BulkDelete(ctx, cc, BulkDeleteInput{IDs: idsOf(rows)}) + if err != nil || first.Deleted != 2 { + t.Fatalf("first=%+v err=%v", first, err) + } + got = nil + again, err := svc.BulkDelete(ctx, cc, BulkDeleteInput{IDs: idsOf(rows)}) + if err != nil || again.Deleted != 0 || len(got) != 0 { + t.Fatalf("repeat=%+v err=%v hooks=%v", again, err, got) + } + hooks.trackScope = true + outside := seedCrud(t, db, 1) + if err := db.Model(&outside[0]).Update("scope_id", 2).Error; err != nil { + t.Fatal(err) + } + got = nil + scoped := context.WithValue(ctx, scopeKey{}, uint(1)) + res, err := svc.BulkDelete(scoped, cc, BulkDeleteInput{IDs: []any{outside[0].ID}}) + if err != nil || res.Deleted != 0 || len(got) != 0 { + t.Fatalf("out of scope=%+v err=%v hooks=%v", res, err, got) + } + if n := countCrud(t, db); n != 1 { + t.Fatalf("rows=%d, out-of-scope delete changed the table", n) + } +} + +func TestBulkDeleteRollback(t *testing.T) { + svc, httpSvc, cc, db, hooks := hookFixture(t) + rows := seedCrud(t, db, 2) + var got []uint + ctx := idCtx(hooks, &got) + if _, err := svc.BulkDelete(ctx, cc, BulkDeleteInput{IDs: []any{rows[0].ID, uint(999999)}}); err == nil || len(got) != 0 || countCrud(t, db) != 2 { + t.Fatalf("mixed missing err=%v hooks=%v rows=%d", err, got, countCrud(t, db)) + } + rec := crudBulk(httpSvc, []byte(fmt.Sprintf(`{"ids":[%d,999999]}`, rows[0].ID)), ctx) + if rec.Code != http.StatusConflict || strings.Contains(rec.Body.String(), "999999") { + t.Fatalf("mixed http=%d %s", rec.Code, rec.Body.String()) + } + if countCrud(t, db) != 2 { + t.Fatal("mixed http committed a delete") + } + + hooks.trackScope = true + if err := db.Model(&rows[1]).Update("scope_id", 2).Error; err != nil { + t.Fatal(err) + } + got = nil + scoped := context.WithValue(ctx, scopeKey{}, uint(1)) + if _, err := svc.BulkDelete(scoped, cc, BulkDeleteInput{IDs: []any{rows[0].ID, rows[1].ID}}); err == nil || len(got) != 0 { + t.Fatalf("mixed scope err=%v hooks=%v", err, got) + } + if loadCrud(t, db, rows[0].Name).ScopeID != 1 || loadCrud(t, db, rows[1].Name).ScopeID != 2 { + t.Fatal("mixed scope committed") + } + + if err := db.Model(&rows[1]).Update("scope_id", 1).Error; err != nil { + t.Fatal(err) + } + hooks.trackScope = false + got = nil + failCtx := context.WithValue(ctx, failIDsKey{}, map[uint]bool{rows[1].ID: true}) + _, err := svc.BulkDelete(failCtx, cc, BulkDeleteInput{IDs: idsOf(rows)}) + if err == nil || strings.Contains(err.Error(), "secret-hook-boom") || !strings.Contains(err.Error(), "acme.demo.records") { + t.Fatalf("hook err=%v", err) + } + if countCrud(t, db) != 2 { + t.Fatal("hook failure committed a partial delete") + } + out := httptest.NewRecorder() + writeCRUDError(out, err) + if out.Code != http.StatusInternalServerError || strings.Contains(out.Body.String(), "secret-hook-boom") { + t.Fatalf("hook http=%d %s", out.Code, out.Body.String()) + } + + cancelCtx, cancel := context.WithCancel(ctx) + cancelCtx = context.WithValue(cancelCtx, cancelKey{}, cancel) + cancelCtx = context.WithValue(cancelCtx, cancelOnKey{}, rows[1].ID) + got = nil + if _, err := svc.BulkDelete(cancelCtx, cc, BulkDeleteInput{IDs: idsOf(rows)}); err == nil || countCrud(t, db) != 2 { + t.Fatalf("cancel err=%v rows=%d", err, countCrud(t, db)) + } +} + +func TestBulkDeleteConcurrent(t *testing.T) { + svc, _, cc, db, hooks := hookFixture(t) + rows := seedCrud(t, db, 3) + var got []uint + ctx := context.WithValue(idCtx(hooks, &got), bulkSlowKey{}, true) + in := BulkDeleteInput{IDs: idsOf(rows)} + var wg sync.WaitGroup + start := make(chan struct{}) + type outcome struct { + res BulkResult + err error + } + outs := make([]outcome, 2) + for i := 0; i < 2; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + <-start + outs[i].res, outs[i].err = svc.BulkDelete(ctx, cc, in) + }(i) + } + close(start) + wg.Wait() + sum := 0 + for i, out := range outs { + if out.err != nil || (out.res.Deleted != 0 && out.res.Deleted != 3) { + t.Fatalf("worker %d = %+v err=%v", i, out.res, out.err) + } + sum += out.res.Deleted + } + if sum != 3 || !sameUints(got, []uint{rows[0].ID, rows[1].ID, rows[2].ID}) || countCrud(t, db) != 0 { + t.Fatalf("sum=%d hooks=%v rows=%d", sum, got, countCrud(t, db)) + } +} + +func seedCrud(t *testing.T, db *gorm.DB, n int) []crudRow { + t.Helper() + rows := make([]crudRow, n) + for i := range rows { + rows[i] = crudRow{Name: fmt.Sprintf("row-%d", i), ScopeID: 1} + } + if err := db.Create(&rows).Error; err != nil { + t.Fatal(err) + } + return rows +} + +func idsOf(rows []crudRow) []any { + out := make([]any, len(rows)) + for i := range rows { + out[i] = rows[i].ID + } + return out +} + +func idCtx(hooks *hookController, got *[]uint) context.Context { + return context.WithValue(principalCtx(hooks, superUser()), idSinkKey{}, got) +} + +func sameUints(got, want []uint) bool { + if len(got) != len(want) { + return false + } + for i := range got { + if got[i] != want[i] { + return false + } + } + return true +} + +func crudBulk(svc *service, body []byte, ctx context.Context) *httptest.ResponseRecorder { + req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(body)) + req.SetPathValue("vendor", "acme") + req.SetPathValue("plugin", "demo") + req.SetPathValue("controller", "records") + if ctx != nil { + req = req.WithContext(ctx) + } + rec := httptest.NewRecorder() + svc.bulkDelete(rec, req) + return rec +} diff --git a/cabana/crud.go b/cabana/crud.go index 693efc1..caa29d7 100644 --- a/cabana/crud.go +++ b/cabana/crud.go @@ -168,6 +168,12 @@ func (s CRUDService) Delete(ctx context.Context, cc *CompiledController, id any) return result, nil } +// BulkDelete removes a normalized id set in one transaction. +// The deterministic lifecycle is added with the bulk route. +func (s CRUDService) BulkDelete(ctx context.Context, cc *CompiledController, in BulkDeleteInput) (BulkResult, error) { + return BulkResult{}, errors.New("cabana: bulk delete is not implemented") +} + // Show loads one scoped record. Missing and out-of-scope ids are identical. func (s CRUDService) Show(ctx context.Context, cc *CompiledController, id any) (map[string]any, error) { if s.DB == nil { diff --git a/cabana/crud_lifecycle_test.go b/cabana/crud_lifecycle_test.go index 7b9b741..d693b75 100644 --- a/cabana/crud_lifecycle_test.go +++ b/cabana/crud_lifecycle_test.go @@ -43,6 +43,9 @@ func (c *hookController) NewRecord() any { } return &crudRow{} } +func (c *hookController) ListExtendQuery(ctx context.Context, db *gorm.DB) *gorm.DB { + return c.FormExtendQuery(ctx, db) +} func (c *hookController) FormExtendQuery(ctx context.Context, db *gorm.DB) *gorm.DB { if c != nil { c.queries.Add(1) diff --git a/cabana/crud_test.go b/cabana/crud_test.go index 712828c..6a0251d 100644 --- a/cabana/crud_test.go +++ b/cabana/crud_test.go @@ -7,10 +7,10 @@ import ( "net/http" "net/http/httptest" "strings" + "sync" "testing" - "time" - "testing/fstest" + "time" "gorm.io/gorm" ) @@ -118,10 +118,23 @@ func (r *crudRow) AfterUpdate(tx *gorm.DB) error { func (r *crudRow) BeforeDelete(tx *gorm.DB) error { ctx := txContext(tx) + if _, ok := ctx.Value(bulkSlowKey{}).(bool); ok { + time.Sleep(300 * time.Millisecond) + } recordHook(ctx, "before_delete") + appendDeletedID(ctx, r.ID) if fail, _ := ctx.Value(failDeleteKey{}).(bool); fail { return errors.New("secret-hook-boom") } + if ids, ok := ctx.Value(failIDsKey{}).(map[uint]bool); ok && ids[r.ID] { + return errors.New("secret-hook-boom") + } + if id, ok := ctx.Value(cancelOnKey{}).(uint); ok && r.ID == id { + if cancel, ok := ctx.Value(cancelKey{}).(context.CancelFunc); ok { + cancel() + } + return context.Canceled + } return nil } @@ -132,6 +145,13 @@ func (r *crudRow) AfterDelete(tx *gorm.DB) error { type hookSinkKey struct{} type failDeleteKey struct{} +type failIDsKey struct{} +type idSinkKey struct{} +type bulkSlowKey struct{} +type cancelOnKey struct{} +type cancelKey struct{} + +var hookMu sync.Mutex func txContext(tx *gorm.DB) context.Context { if tx != nil && tx.Statement != nil && tx.Statement.Context != nil { @@ -144,6 +164,8 @@ func recordHook(ctx context.Context, name string) { if ctx == nil { return } + hookMu.Lock() + defer hookMu.Unlock() sink, _ := ctx.Value(hookSinkKey{}).(*[]string) if sink == nil { return @@ -151,6 +173,19 @@ func recordHook(ctx context.Context, name string) { *sink = append(*sink, name) } +func appendDeletedID(ctx context.Context, id uint) { + if ctx == nil { + return + } + hookMu.Lock() + defer hookMu.Unlock() + sink, _ := ctx.Value(idSinkKey{}).(*[]uint) + if sink == nil { + return + } + *sink = append(*sink, id) +} + type crudNoRules struct { ID uint `gorm:"column:id;primaryKey"` Name string `gorm:"column:name"` diff --git a/cabana/http.go b/cabana/http.go index 077ffe1..057429f 100644 --- a/cabana/http.go +++ b/cabana/http.go @@ -241,6 +241,10 @@ func (s *service) update(w http.ResponseWriter, r *http.Request) { }) } +func (s *service) bulkDelete(w http.ResponseWriter, r *http.Request) { + WriteError(w, http.StatusInternalServerError, "error", msgServerError) +} + func (s *service) deleteRecord(w http.ResponseWriter, r *http.Request) { s.protect(w, r, func(cc *CompiledController) { id, err := pathID(r)