From 3b93802a250375ac0339173c2f8e7f042a8b4be3 Mon Sep 17 00:00:00 2001 From: Hein Date: Wed, 30 Sep 2026 22:53:07 +0200 Subject: [PATCH] fix(tx): run restheadspec AfterRead in a second short transaction --- pkg/restheadspec/handler.go | 8 +++-- pkg/restheadspec/read_tx_test.go | 62 ++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 pkg/restheadspec/read_tx_test.go diff --git a/pkg/restheadspec/handler.go b/pkg/restheadspec/handler.go index 89e9d93..649901f 100644 --- a/pkg/restheadspec/handler.go +++ b/pkg/restheadspec/handler.go @@ -1000,12 +1000,14 @@ func (h *Handler) handleRead(ctx context.Context, w common.ResponseWriter, id st logger.Debug("FetchRowNumber: Row number %d set in metadata", *fetchedRowNumber) } - // Execute AfterRead hooks (runs after the transaction commits, against the pooled db) - hookCtx.Tx = h.db + // Execute AfterRead hooks in a second short transaction: the read tx has + // already committed, and hooks must never get the pooled connection. hookCtx.Result = modelPtr hookCtx.Error = nil - if err := h.hooks.Execute(AfterRead, hookCtx); err != nil { + if err := h.runInTx(ctx, hookCtx, func(common.Database) error { + return h.hooks.Execute(AfterRead, hookCtx) + }); err != nil { logger.Error("AfterRead hook failed: %v", err) h.sendError(w, http.StatusInternalServerError, "hook_error", "Hook execution failed", err) return diff --git a/pkg/restheadspec/read_tx_test.go b/pkg/restheadspec/read_tx_test.go new file mode 100644 index 0000000..22ed88b --- /dev/null +++ b/pkg/restheadspec/read_tx_test.go @@ -0,0 +1,62 @@ +package restheadspec + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/DATA-DOG/go-sqlmock" + "github.com/uptrace/bun" + "github.com/uptrace/bun/dialect/pgdialect" + + "github.com/bitechdev/ResolveSpec/pkg/common" + "github.com/bitechdev/ResolveSpec/pkg/common/adapters/database" + "github.com/bitechdev/ResolveSpec/pkg/modelregistry" +) + +func TestAfterReadRunsInSecondTransaction(t *testing.T) { + sqlDB, mock, err := sqlmock.New() + if err != nil { + t.Fatal(err) + } + sqlDB.SetMaxOpenConns(1) + t.Cleanup(func() { _ = sqlDB.Close() }) + h := NewHandler(database.NewBunAdapter(bun.NewDB(sqlDB, pgdialect.New())), modelregistry.NewModelRegistry()) + var begins []common.Database + var readTx, afterTx common.Database + h.Hooks().Register(OnTxBegin, func(c *HookContext) error { begins = append(begins, c.Tx); return nil }) + h.Hooks().Register(BeforeRead, func(c *HookContext) error { readTx = c.Tx; return nil }) + h.Hooks().Register(AfterRead, func(c *HookContext) error { afterTx = c.Tx; return nil }) + + mock.ExpectBegin() + mock.ExpectQuery(`SELECT`).WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1)) + mock.ExpectQuery(`SELECT`).WillReturnRows(sqlmock.NewRows([]string{"id", "name"}).AddRow(7, "a")) + mock.ExpectCommit() + mock.ExpectBegin() + mock.ExpectCommit() + + rec := httptest.NewRecorder() + w, _ := common.WrapHTTPRequest(rec, httptest.NewRequest(http.MethodGet, "/", nil)) + base, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + ctx := WithSchema(base, "public") + ctx = WithEntity(ctx, "items") + ctx = WithTableName(ctx, "items") + ctx = WithModel(ctx, delItem{}) + h.handleRead(ctx, w, "7", ExtendedRequestOptions{}) + + if rec.Code != http.StatusOK { + t.Fatalf("status %d body %s", rec.Code, rec.Body) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatal(err) + } + if len(begins) != 2 || begins[0] == begins[1] { + t.Fatalf("OnTxBegin must fire once per transaction (2), got %d", len(begins)) + } + if readTx != begins[0] || afterTx != begins[1] || afterTx == h.db { + t.Fatal("BeforeRead must run on the first tx and AfterRead on the second") + } +}