From 8b63e196344b170aadc3384b01a6be2814929a9d Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 20:02:18 +0000 Subject: [PATCH 1/2] fix: cache the project registration actually created, and only on commit Register and the SSO finish-setup handler both cached a five-field copy of the project they had just created instead of the project itself, so the cached row disagreed with the database on every field the copy omitted: - DropHealthyHealthchecks is true on a new project but false in the copy, and UseClientAuth hands that cached object straight to FilterHealthchecks, so healthy healthchecks were ingested rather than dropped. - CreateWithOrganization mints a source map token for ios projects; the copy left it nil, so GetBySourceMapToken missed and the first symbol upload 401d. - CreatedAt stayed zero, which reorders GetAll(). On the PostgreSQL build the project_cache_changed notification triggers a full refresh that overwrites the bad entry within milliseconds, so this only persisted on the SQLite and DuckDB builds, where NotifyProjectCacheChanged is a no-op and nothing refreshes the cache after boot. It lasted until the process restarted or someone saved the project's settings. Both call sites also ran under middleware.Transactional and mutated the cache before the commit, so a later failure in the same handler rolled the transaction back and left a project in the cache that does not exist. Queue the write with middleware.OnCommit instead, matching what project_batch.go already does. ToProjectWithBackendUrl takes a value receiver and copies, so passing the repository's own pointer aliases nothing the handler goes on to mutate. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01S7f9Ad34w2iGg3mqangVsw --- backend/app/controllers/auth.controller.go | 8 +- backend/app/controllers/oauth.controller.go | 8 +- .../project_cache_registration_test.go | 142 ++++++++++++++++++ 3 files changed, 144 insertions(+), 14 deletions(-) create mode 100644 backend/app/controllers/project_cache_registration_test.go diff --git a/backend/app/controllers/auth.controller.go b/backend/app/controllers/auth.controller.go index 12a3c310..18f259ff 100644 --- a/backend/app/controllers/auth.controller.go +++ b/backend/app/controllers/auth.controller.go @@ -200,13 +200,7 @@ func (a authController) Register(c *gin.Context) { var projectWithUrl *models.ProjectWithBackendUrl if project != nil { - cache.ProjectCache.AddProject(&models.Project{ - Id: project.Id, - Name: project.Name, - Token: project.Token, - Framework: project.Framework, - OrganizationId: project.OrganizationId, - }) + middleware.OnCommit(c, func() { cache.ProjectCache.AddProject(project) }) projectWithUrl = project.ToProjectWithBackendUrl() } diff --git a/backend/app/controllers/oauth.controller.go b/backend/app/controllers/oauth.controller.go index 60b6dd89..ca0a54a0 100644 --- a/backend/app/controllers/oauth.controller.go +++ b/backend/app/controllers/oauth.controller.go @@ -293,13 +293,7 @@ func (a oauthController) FinishSetup(c *gin.Context) { return } - cache.ProjectCache.AddProject(&models.Project{ - Id: project.Id, - Name: project.Name, - Token: project.Token, - Framework: project.Framework, - OrganizationId: project.OrganizationId, - }) + middleware.OnCommit(c, func() { cache.ProjectCache.AddProject(project) }) projectWithUrl = project.ToProjectWithBackendUrl() } diff --git a/backend/app/controllers/project_cache_registration_test.go b/backend/app/controllers/project_cache_registration_test.go new file mode 100644 index 00000000..518e2fba --- /dev/null +++ b/backend/app/controllers/project_cache_registration_test.go @@ -0,0 +1,142 @@ +//go:build !telemetry_ch && !transactional_pg && !telemetry_duckdb + +package controllers + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/gin-gonic/gin" + "github.com/tracewayapp/traceway/backend/app/cache" + "github.com/tracewayapp/traceway/backend/app/config" + "github.com/tracewayapp/traceway/backend/app/db" + "github.com/tracewayapp/traceway/backend/app/middleware" + "github.com/tracewayapp/traceway/backend/app/models" + "github.com/tracewayapp/traceway/backend/app/repositories/transactional" + "github.com/tracewayapp/traceway/backend/app/services" +) + +func initRegistrationJWT(t *testing.T) { + t.Helper() + prevSecret := config.Config.JWTSecret + config.Config.JWTSecret = strings.Repeat("s", 32) + if err := services.InitJWT(); err != nil { + t.Fatalf("init jwt: %v", err) + } + services.InitTurnstile() + t.Cleanup(func() { + config.Config.JWTSecret = prevSecret + _ = services.InitJWT() + }) +} + +func findProjectByName(t *testing.T, name string) *models.Project { + t.Helper() + tx, err := db.DB.Begin() + if err != nil { + t.Fatalf("begin: %v", err) + } + defer tx.Rollback() + + projects, err := transactional.ProjectRepository.FindAll(tx) + if err != nil { + t.Fatalf("find all projects: %v", err) + } + for _, p := range projects { + if p.Name == name { + return p + } + } + return nil +} + +// The register handler used to cache a five-field copy of the project it had +// just created, so the cached ingest settings disagreed with the row. On the +// SQLite builds nothing ever refreshes the cache, so the wrong copy served +// every /api/report request until the process restarted. +func TestRegisterCachesTheProjectItCreated(t *testing.T) { + setupSetupControllerDB(t) + initRegistrationJWT(t) + + gin.SetMode(gin.TestMode) + router := gin.New() + router.POST("/api/register", middleware.Transactional, AuthController.Register) + + body := `{"email":"cached@example.com","name":"A","password":"password1","organizationName":"CachedOrg","timezone":"UTC","projectName":"Mobile App","framework":"ios"}` + recorder := httptest.NewRecorder() + request := httptest.NewRequest(http.MethodPost, "/api/register", strings.NewReader(body)) + request.Header.Set("Content-Type", "application/json") + router.ServeHTTP(recorder, request) + + if recorder.Code != http.StatusCreated { + t.Fatalf("status = %d, body = %s", recorder.Code, recorder.Body.String()) + } + + stored := findProjectByName(t, "Mobile App") + if stored == nil { + t.Fatal("project was not created") + } + t.Cleanup(func() { cache.ProjectCache.RemoveProject(stored.Id) }) + + cached := cache.ProjectCache.GetByToken(stored.Token) + if cached == nil { + t.Fatal("project token is not in the cache after a committed registration") + } + + if cached.DropHealthyHealthchecks != stored.DropHealthyHealthchecks { + t.Errorf("cached DropHealthyHealthchecks = %v, stored = %v; healthy healthchecks would be ingested", + cached.DropHealthyHealthchecks, stored.DropHealthyHealthchecks) + } + if stored.SourceMapToken == nil { + t.Fatal("an ios project should be created with a source map token") + } + if cached.SourceMapToken == nil || *cached.SourceMapToken != *stored.SourceMapToken { + t.Errorf("cached SourceMapToken = %v, stored = %v", cached.SourceMapToken, *stored.SourceMapToken) + } + if cache.ProjectCache.GetBySourceMapToken(*stored.SourceMapToken) == nil { + t.Error("source map token does not resolve from the cache; symbol upload would 401") + } + if len(cached.AiFlaggedLanguages) != len(stored.AiFlaggedLanguages) { + t.Errorf("cached AiFlaggedLanguages = %v, stored = %v", cached.AiFlaggedLanguages, stored.AiFlaggedLanguages) + } + if cached.CreatedAt.IsZero() { + t.Error("cached CreatedAt is zero, which reorders GetAll()") + } +} + +// The cache write is queued with middleware.OnCommit, so a registration whose +// transaction never commits must not leave the project behind. Calling the +// handler without the Transactional middleware runs no commit hooks, which is +// the same observation point as a rollback. +func TestRegisterDoesNotCacheBeforeCommit(t *testing.T) { + setupSetupControllerDB(t) + initRegistrationJWT(t) + + tx, err := db.DB.Begin() + if err != nil { + t.Fatalf("begin: %v", err) + } + + body := `{"email":"uncommitted@example.com","name":"A","password":"password1","organizationName":"UncommittedOrg","timezone":"UTC","projectName":"Pending App","framework":"react"}` + c, recorder := newControllerTestContext(t, tx, 0, http.MethodPost, "/api/register", body) + AuthController.Register(c) + if recorder.Code != http.StatusCreated { + tx.Rollback() + t.Fatalf("status = %d, body = %s", recorder.Code, recorder.Body.String()) + } + if err := tx.Commit(); err != nil { + t.Fatalf("commit: %v", err) + } + + stored := findProjectByName(t, "Pending App") + if stored == nil { + t.Fatal("project was not created") + } + t.Cleanup(func() { cache.ProjectCache.RemoveProject(stored.Id) }) + + if cache.ProjectCache.GetByToken(stored.Token) != nil { + t.Error("the handler cached the project inline; a rolled back registration would leave a phantom entry") + } +} From db7e86ef8dab8b274eaa59e94d533996fb38c7c5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 21:42:08 +0000 Subject: [PATCH 2/2] test: cover the commit-hook guarantee the cache fix relies on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the previous commit, from a review pass over it. middleware.OnCommit and runCommitHooks had no tests at all, so the property the cache fix depends on — a queued side effect fires only once its write is durable — was unverified. Moving runCommitHooks onto the rollback branch left the whole suite green. Cover both outcomes and the ordering. Also from the review: - Assert the cached project against every field of the row rather than the four a past bug happened to drop. The old length-only check on AiFlaggedLanguages passed for a same-length but different pack, which changes which flagged-term packs ingest scans. - Register the ProjectCache cleanup before issuing the request. The commit hook inserts the entry during ServeHTTP, so a t.Fatal between the request and the old cleanup registration leaked an entry pointing at a project whose in-memory database was about to close. - Move initRegistrationJWT into setup_test.go, which owns the shared harness, now that TestRegisterWithoutProject calls it. Its inline copy is gone. - Widen the OnCommit doc comment: three of its five callers are now cache writes, and "visible to other connections" read as excluding an in-process map. Note that only routes carrying the middleware run queued hooks. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01S7f9Ad34w2iGg3mqangVsw --- .../project_cache_registration_test.go | 125 +++++++++++++----- backend/app/controllers/setup_test.go | 25 ++-- backend/app/middleware/commit_hooks_test.go | 119 +++++++++++++++++ .../middleware/transactional.middleware.go | 10 +- 4 files changed, 235 insertions(+), 44 deletions(-) create mode 100644 backend/app/middleware/commit_hooks_test.go diff --git a/backend/app/controllers/project_cache_registration_test.go b/backend/app/controllers/project_cache_registration_test.go index 518e2fba..49934629 100644 --- a/backend/app/controllers/project_cache_registration_test.go +++ b/backend/app/controllers/project_cache_registration_test.go @@ -5,30 +5,34 @@ package controllers import ( "net/http" "net/http/httptest" + "slices" "strings" "testing" "github.com/gin-gonic/gin" + "github.com/google/uuid" "github.com/tracewayapp/traceway/backend/app/cache" - "github.com/tracewayapp/traceway/backend/app/config" "github.com/tracewayapp/traceway/backend/app/db" "github.com/tracewayapp/traceway/backend/app/middleware" "github.com/tracewayapp/traceway/backend/app/models" "github.com/tracewayapp/traceway/backend/app/repositories/transactional" - "github.com/tracewayapp/traceway/backend/app/services" ) -func initRegistrationJWT(t *testing.T) { +// ProjectCache is a process-global that outlives the per-test in-memory +// database, so whatever a test puts in it has to come back out even when the +// test fails before it knows the project's id. +func restoreProjectCacheAfterTest(t *testing.T) { t.Helper() - prevSecret := config.Config.JWTSecret - config.Config.JWTSecret = strings.Repeat("s", 32) - if err := services.InitJWT(); err != nil { - t.Fatalf("init jwt: %v", err) + before := map[uuid.UUID]bool{} + for _, p := range cache.ProjectCache.GetAll() { + before[p.Id] = true } - services.InitTurnstile() t.Cleanup(func() { - config.Config.JWTSecret = prevSecret - _ = services.InitJWT() + for _, p := range cache.ProjectCache.GetAll() { + if !before[p.Id] { + cache.ProjectCache.RemoveProject(p.Id) + } + } }) } @@ -52,6 +56,71 @@ func findProjectByName(t *testing.T, name string) *models.Project { return nil } +// Every field the cache serves has to agree with the row, not just the ones a +// past bug happened to drop. +func assertCachedProjectMatchesRow(t *testing.T, cached, stored *models.Project) { + t.Helper() + + if cached.Id != stored.Id { + t.Errorf("cached Id = %v, stored = %v", cached.Id, stored.Id) + } + if cached.Name != stored.Name { + t.Errorf("cached Name = %q, stored = %q", cached.Name, stored.Name) + } + if cached.Token != stored.Token { + t.Errorf("cached Token = %q, stored = %q", cached.Token, stored.Token) + } + if cached.Framework != stored.Framework { + t.Errorf("cached Framework = %q, stored = %q", cached.Framework, stored.Framework) + } + if !equalIntPtr(cached.OrganizationId, stored.OrganizationId) { + t.Errorf("cached OrganizationId = %v, stored = %v", cached.OrganizationId, stored.OrganizationId) + } + // Second granularity: the cached copy carries the value the repository + // built, the row carries whatever survived the round trip through SQLite. + if cached.CreatedAt.Unix() != stored.CreatedAt.Unix() { + t.Errorf("cached CreatedAt = %v, stored = %v", cached.CreatedAt, stored.CreatedAt) + } + if cached.CreatedAt.IsZero() { + t.Error("cached CreatedAt is zero, which reorders GetAll()") + } + if cached.DropHealthyHealthchecks != stored.DropHealthyHealthchecks { + t.Errorf("cached DropHealthyHealthchecks = %v, stored = %v; healthy healthchecks would be ingested", + cached.DropHealthyHealthchecks, stored.DropHealthyHealthchecks) + } + if !equalStringPtr(cached.SourceMapToken, stored.SourceMapToken) { + t.Errorf("cached SourceMapToken = %v, stored = %v", cached.SourceMapToken, stored.SourceMapToken) + } + // nil and empty are the same allowlist; copyProject turns one into the other. + if !slices.Equal(cached.HealthcheckPaths, stored.HealthcheckPaths) { + t.Errorf("cached HealthcheckPaths = %v, stored = %v", cached.HealthcheckPaths, stored.HealthcheckPaths) + } + if !slices.Equal(cached.ProfileLabelAllowlist, stored.ProfileLabelAllowlist) { + t.Errorf("cached ProfileLabelAllowlist = %v, stored = %v", cached.ProfileLabelAllowlist, stored.ProfileLabelAllowlist) + } + if !slices.Equal(cached.AiFlaggedTerms, stored.AiFlaggedTerms) { + t.Errorf("cached AiFlaggedTerms = %v, stored = %v", cached.AiFlaggedTerms, stored.AiFlaggedTerms) + } + if !slices.Equal(cached.AiFlaggedLanguages, stored.AiFlaggedLanguages) { + t.Errorf("cached AiFlaggedLanguages = %v, stored = %v; ingest would scan the wrong term packs", + cached.AiFlaggedLanguages, stored.AiFlaggedLanguages) + } +} + +func equalIntPtr(a, b *int) bool { + if a == nil || b == nil { + return a == b + } + return *a == *b +} + +func equalStringPtr(a, b *string) bool { + if a == nil || b == nil { + return a == b + } + return *a == *b +} + // The register handler used to cache a five-field copy of the project it had // just created, so the cached ingest settings disagreed with the row. On the // SQLite builds nothing ever refreshes the cache, so the wrong copy served @@ -59,6 +128,7 @@ func findProjectByName(t *testing.T, name string) *models.Project { func TestRegisterCachesTheProjectItCreated(t *testing.T) { setupSetupControllerDB(t) initRegistrationJWT(t) + restoreProjectCacheAfterTest(t) gin.SetMode(gin.TestMode) router := gin.New() @@ -78,54 +148,48 @@ func TestRegisterCachesTheProjectItCreated(t *testing.T) { if stored == nil { t.Fatal("project was not created") } - t.Cleanup(func() { cache.ProjectCache.RemoveProject(stored.Id) }) cached := cache.ProjectCache.GetByToken(stored.Token) if cached == nil { t.Fatal("project token is not in the cache after a committed registration") } + assertCachedProjectMatchesRow(t, cached, stored) - if cached.DropHealthyHealthchecks != stored.DropHealthyHealthchecks { - t.Errorf("cached DropHealthyHealthchecks = %v, stored = %v; healthy healthchecks would be ingested", - cached.DropHealthyHealthchecks, stored.DropHealthyHealthchecks) - } + // The by-source-map-token index is separate from the by-token one, and it + // is what a symbol upload authenticates against. if stored.SourceMapToken == nil { t.Fatal("an ios project should be created with a source map token") } - if cached.SourceMapToken == nil || *cached.SourceMapToken != *stored.SourceMapToken { - t.Errorf("cached SourceMapToken = %v, stored = %v", cached.SourceMapToken, *stored.SourceMapToken) - } if cache.ProjectCache.GetBySourceMapToken(*stored.SourceMapToken) == nil { t.Error("source map token does not resolve from the cache; symbol upload would 401") } - if len(cached.AiFlaggedLanguages) != len(stored.AiFlaggedLanguages) { - t.Errorf("cached AiFlaggedLanguages = %v, stored = %v", cached.AiFlaggedLanguages, stored.AiFlaggedLanguages) - } - if cached.CreatedAt.IsZero() { - t.Error("cached CreatedAt is zero, which reorders GetAll()") - } } -// The cache write is queued with middleware.OnCommit, so a registration whose -// transaction never commits must not leave the project behind. Calling the -// handler without the Transactional middleware runs no commit hooks, which is -// the same observation point as a rollback. -func TestRegisterDoesNotCacheBeforeCommit(t *testing.T) { +// The handler itself must not touch the cache; the write is queued with +// middleware.OnCommit so a registration whose transaction rolls back leaves +// nothing behind. Driving the handler without that middleware runs no commit +// hooks, which is the same observation point as a rollback. +func TestRegisterCachesOnlyViaCommitHook(t *testing.T) { setupSetupControllerDB(t) initRegistrationJWT(t) + restoreProjectCacheAfterTest(t) tx, err := db.DB.Begin() if err != nil { t.Fatalf("begin: %v", err) } + defer tx.Rollback() body := `{"email":"uncommitted@example.com","name":"A","password":"password1","organizationName":"UncommittedOrg","timezone":"UTC","projectName":"Pending App","framework":"react"}` c, recorder := newControllerTestContext(t, tx, 0, http.MethodPost, "/api/register", body) AuthController.Register(c) if recorder.Code != http.StatusCreated { - tx.Rollback() t.Fatalf("status = %d, body = %s", recorder.Code, recorder.Body.String()) } + // Commit rather than roll back: the harness caps the main DB at one + // connection, so findProjectByName cannot open its transaction until this + // one closes, and it needs the row to be visible. Commit hooks still never + // run, because middleware.Transactional is what runs them. if err := tx.Commit(); err != nil { t.Fatalf("commit: %v", err) } @@ -134,7 +198,6 @@ func TestRegisterDoesNotCacheBeforeCommit(t *testing.T) { if stored == nil { t.Fatal("project was not created") } - t.Cleanup(func() { cache.ProjectCache.RemoveProject(stored.Id) }) if cache.ProjectCache.GetByToken(stored.Token) != nil { t.Error("the handler cached the project inline; a rolled back registration would leave a phantom entry") diff --git a/backend/app/controllers/setup_test.go b/backend/app/controllers/setup_test.go index 87d3d728..5fb1cc31 100644 --- a/backend/app/controllers/setup_test.go +++ b/backend/app/controllers/setup_test.go @@ -64,6 +64,20 @@ func setupSetupControllerDB(t *testing.T) { } } +func initRegistrationJWT(t *testing.T) { + t.Helper() + prevSecret := config.Config.JWTSecret + config.Config.JWTSecret = strings.Repeat("s", 32) + if err := services.InitJWT(); err != nil { + t.Fatalf("init jwt: %v", err) + } + services.InitTurnstile() + t.Cleanup(func() { + config.Config.JWTSecret = prevSecret + _ = services.InitJWT() + }) +} + func createSetupTestAccount(t *testing.T, tx *sql.Tx, email, role string) (userId int, orgId int) { t.Helper() user, err := transactional.UserRepository.Create(tx, email, "Test User", "hashed-password") @@ -246,16 +260,7 @@ func TestBatchCreateProjectsLimitHookAbortsBatch(t *testing.T) { func TestRegisterWithoutProject(t *testing.T) { setupSetupControllerDB(t) - prevSecret := config.Config.JWTSecret - config.Config.JWTSecret = strings.Repeat("s", 32) - if err := services.InitJWT(); err != nil { - t.Fatalf("init jwt: %v", err) - } - services.InitTurnstile() - t.Cleanup(func() { - config.Config.JWTSecret = prevSecret - _ = services.InitJWT() - }) + initRegistrationJWT(t) register := func(t *testing.T, body string) (*httptest.ResponseRecorder, map[string]any) { t.Helper() diff --git a/backend/app/middleware/commit_hooks_test.go b/backend/app/middleware/commit_hooks_test.go new file mode 100644 index 00000000..045d589e --- /dev/null +++ b/backend/app/middleware/commit_hooks_test.go @@ -0,0 +1,119 @@ +//go:build !transactional_pg && !telemetry_ch && !telemetry_duckdb + +package middleware + +import ( + "database/sql" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + "github.com/tracewayapp/traceway/backend/app/db" + _ "modernc.org/sqlite" +) + +func newCommitHookTestDB(t *testing.T) *sql.DB { + t.Helper() + conn, err := sql.Open("sqlite", ":memory:") + if err != nil { + t.Fatal(err) + } + conn.SetMaxOpenConns(1) + t.Cleanup(func() { conn.Close() }) + + if _, err := conn.Exec("CREATE TABLE things (id INTEGER PRIMARY KEY)"); err != nil { + t.Fatalf("create table: %v", err) + } + + prev := db.DB + db.DB = conn + t.Cleanup(func() { db.DB = prev }) + return conn +} + +// A queued side effect must fire only when the write it describes is durable, +// and must be dropped otherwise. Callers rely on this to publish state that +// outlives the request — cache.ProjectCache entries, for one, which would +// survive a rollback and then authenticate a project that does not exist. +func TestOnCommitRunsQueuedHooksOnlyAfterCommit(t *testing.T) { + gin.SetMode(gin.TestMode) + + for _, tc := range []struct { + name string + status int + wantHook bool + wantCommit bool + }{ + {name: "committed", status: http.StatusCreated, wantHook: true, wantCommit: true}, + {name: "rolled back", status: http.StatusInternalServerError, wantHook: false, wantCommit: false}, + } { + t.Run(tc.name, func(t *testing.T) { + conn := newCommitHookTestDB(t) + + ran := 0 + r := gin.New() + r.POST("/thing", Transactional, func(c *gin.Context) { + tx := db.GetTx(c) + if _, err := tx.Exec("INSERT INTO things (id) VALUES (1)"); err != nil { + t.Fatalf("insert: %v", err) + } + OnCommit(c, func() { ran++ }) + c.JSON(tc.status, gin.H{}) + }) + + recorder := httptest.NewRecorder() + r.ServeHTTP(recorder, httptest.NewRequest(http.MethodPost, "/thing", nil)) + + if recorder.Code != tc.status { + t.Fatalf("status = %d, want %d", recorder.Code, tc.status) + } + + wantRuns := 0 + if tc.wantHook { + wantRuns = 1 + } + if ran != wantRuns { + t.Errorf("hook ran %d times, want %d", ran, wantRuns) + } + + var rows int + if err := conn.QueryRow("SELECT COUNT(*) FROM things").Scan(&rows); err != nil { + t.Fatalf("count: %v", err) + } + wantRows := 0 + if tc.wantCommit { + wantRows = 1 + } + if rows != wantRows { + t.Errorf("rows = %d, want %d; the hook and the write must agree", rows, wantRows) + } + }) + } +} + +// Hooks run in the order they were queued, so a later hook may depend on what +// an earlier one published. +func TestOnCommitRunsHooksInOrder(t *testing.T) { + gin.SetMode(gin.TestMode) + newCommitHookTestDB(t) + + var order []int + r := gin.New() + r.POST("/thing", Transactional, func(c *gin.Context) { + for i := range 3 { + OnCommit(c, func() { order = append(order, i) }) + } + c.JSON(http.StatusOK, gin.H{}) + }) + + recorder := httptest.NewRecorder() + r.ServeHTTP(recorder, httptest.NewRequest(http.MethodPost, "/thing", nil)) + + if recorder.Code != http.StatusOK { + t.Fatalf("status = %d", recorder.Code) + } + if len(order) != 3 || order[0] != 0 || order[1] != 1 || order[2] != 2 { + t.Errorf("hook order = %v, want [0 1 2]", order) + } +} diff --git a/backend/app/middleware/transactional.middleware.go b/backend/app/middleware/transactional.middleware.go index dd40a849..7f78ff66 100644 --- a/backend/app/middleware/transactional.middleware.go +++ b/backend/app/middleware/transactional.middleware.go @@ -50,9 +50,13 @@ const commitHooksContextKey = "txCommitHooks" // OnCommit queues fn to run after the Transactional middleware successfully // commits the request transaction; queued fns are dropped on rollback. Use it -// for side effects that must only fire once the transaction's writes are -// visible to other connections (e.g. waking the outbox drain worker, which -// would otherwise poll before the enqueued row exists and go back to sleep). +// for side effects that must not observe the write before it lands: waking the +// outbox drain worker, which would otherwise poll before the enqueued row +// exists and go back to sleep, or filling a process-local cache that concurrent +// requests read (cache.ProjectCache, whose entry would outlive a rollback). +// Only routes carrying this middleware run the queued fns; a handler that +// commits its own transaction via db.ExecuteTransaction should call its side +// effect directly instead. func OnCommit(c *gin.Context, fn func()) { hooks, _ := c.Get(commitHooksContextKey) fns, _ := hooks.([]func())