fix(11-07): hand after-commit callbacks a clean statement

- lagoon.Transaction, the lagoon:after_commit flush and the immediate
  AfterCommit path pass a handle with an empty statement on the write's
  connection (Session NewDB+Context, Clauses(), Session NewDB)
- a WithContext query through the handle no longer continues from the
  written model's statement (deferred from 11-05)
- TestTransactionAfterCommit/callback_handle_has_a_clean_statement covers
  the implicit, plain-transaction and lagoon.Transaction paths

modules/lagoon/README.md

@@ -14,7 +14,7 @@ Postgres data layer: the shared GORM connection, per-plugin migrations, model he
 
 - One shared pool: `lagoon.Open`, `lagoon.Use` and `lagoon.OpenFromApp` return a `*sql.DB` and a `*gorm.DB` built on that same pool; `lagoon.Publish` makes both available on the `backpack.App`.
 - Database-ready hooks: `lagoon.OnDatabase` runs a callback with the pool and GORM handle as soon as the database is published, immediately when it already is, otherwise when `lagoon.Publish` runs. Plugins register GORM callbacks through it from Boot, which runs before the `serve` command publishes the database.
-- After-commit work: `lagoon.Transaction` runs a function in a transaction and then the callbacks registered with `lagoon.AfterCommit`, in order, only after the commit succeeds; a nested `lagoon.Transaction` is a savepoint whose callbacks are dropped with it when it fails. A single-statement write for which GORM opens its own transaction runs its `lagoon.AfterCommit` callbacks from the `lagoon:after_commit` GORM callback (`lagoon.AfterCommitCallback`) once GORM commits, and never when the write fails. Outside both, including inside a plain GORM `Transaction`, `lagoon.AfterCommit` runs the callback immediately. A panicking callback is logged and never turns a committed write into an error.
+- After-commit work: `lagoon.Transaction` runs a function in a transaction and then the callbacks registered with `lagoon.AfterCommit`, in order, only after the commit succeeds; a nested `lagoon.Transaction` is a savepoint whose callbacks are dropped with it when it fails. A single-statement write for which GORM opens its own transaction runs its `lagoon.AfterCommit` callbacks from the `lagoon:after_commit` GORM callback (`lagoon.AfterCommitCallback`) once GORM commits, and never when the write fails. Outside both, including inside a plain GORM `Transaction`, `lagoon.AfterCommit` runs the callback immediately on that transaction's connection. The handle a callback receives always has an empty statement on the connection its work belongs to, so a query through it, even one that starts with `WithContext`, never continues from the written model's statement. A panicking callback is logged and never turns a committed write into an error.
 - Database check at connect time: `lagoon.CheckLocale` refuses a database whose default collation is not the ICU `pl-PL` locale, so ordering matches the database default without per-query `COLLATE`.
 - Per-plugin migrations: `lagoon.Migrate` runs the framework's `system_files` set (`attach.Migrations`), backend admin identity set (`lagoon.BackendAdminMigrations`) and job-queue set (`lagoon.QueueMigrations`: River's schema pinned at `lagoon.RiverSchemaVersion`, then the `lagoon.JobsTable` record table, under the `lagoon.QueueHistoryID` history), then every `pact.HasMigrations` set in plugin activation order, each in its own `summer_migrations_<plugin_id>` history table (`lagoon.HistoryTableName`). `lagoon.RollbackLast` and `lagoon.Status` cover rollback and history.
 - Mass assignment: `lagoon.Fill` copies only allow-listed keys onto a model by GORM column name and silently drops the rest, logging each dropped key once outside production. A `json.Number` (from a decoder using `UseNumber`) fills integer, unsigned and float fields. A value that does not fit its column (a fraction, an exponent or an overflow for an integer field, or a value of the wrong type) is a `lagoon.FillTypeError` naming the key, so a caller can answer it as a validation failure on that field. `lagoon.HasFillable` and `lagoon.HasHidden` are the Go forms of `$fillable` and `$hidden`.

modules/lagoon/transaction.go

@@ -70,7 +70,7 @@ func Transaction(ctx context.Context, gdb *gorm.DB, fn func(ctx context.Context,
 	}); err != nil {
 		return err
 	}
-	runAfterCommit(ctx, gdb.Session(&gorm.Session{NewDB: true, Context: ctx}), buf.take())
+	runAfterCommit(ctx, cleanHandle(gdb, ctx), buf.take())
 	return nil
 }
 
@@ -80,7 +80,13 @@ func Transaction(ctx context.Context, gdb *gorm.DB, fn func(ctx context.Context,
 // example from a GORM create callback) it runs after that commit through the
 // AfterCommitCallback callback, and not at all when the write fails.
 // Anywhere else, including inside a plain gorm Transaction, fn runs
-// immediately with db.
+// immediately on db's connection.
+//
+// The handle fn receives always has an empty statement on the connection
+// the work belongs to (the pool after a commit, the open transaction
+// inside a plain gorm Transaction), whatever handle AfterCommit was called
+// with. Queries through it never continue from the written model's
+// statement, even when they start with WithContext.
 func AfterCommit(ctx context.Context, db *gorm.DB, fn func(ctx context.Context, db *gorm.DB)) {
 	if fn == nil {
 		return
@@ -108,7 +114,7 @@ func AfterCommit(ctx context.Context, db *gorm.DB, fn func(ctx context.Context,
 			return
 		}
 	}
-	runAfterCommit(ctx, db, []func(context.Context, *gorm.DB){fn})
+	runAfterCommit(ctx, cleanHandle(db, ctx), []func(context.Context, *gorm.DB){fn})
 }
 
 func bufferFrom(ctx context.Context, db *gorm.DB) *afterCommitBuffer {
@@ -143,7 +149,21 @@ func flushStatementAfterCommit(db *gorm.DB) {
 	if ctx == nil {
 		ctx = context.Background()
 	}
-	runAfterCommit(ctx, db.Session(&gorm.Session{NewDB: true, Context: ctx}), fns)
+	runAfterCommit(ctx, cleanHandle(db, ctx), fns)
+}
+
+// cleanHandle returns a handle on db's connection with an empty statement.
+// db.Session with NewDB and a Context is not enough on a callback's
+// handle: the Context makes it clone the write's statement (model, table,
+// clauses), and a later WithContext on the result continues from that
+// clone, so a query would run against the written model's table. Clauses()
+// starts a fresh statement on the same connection, and the final Session
+// makes the next chained call start from it again. A nil db stays nil.
+func cleanHandle(db *gorm.DB, ctx context.Context) *gorm.DB {
+	if db == nil {
+		return nil
+	}
+	return db.Session(&gorm.Session{NewDB: true, Context: ctx}).Clauses().Session(&gorm.Session{NewDB: true})
 }
 
 func runAfterCommit(ctx context.Context, db *gorm.DB, fns []func(context.Context, *gorm.DB)) {

modules/lagoon/transaction_test.go

@@ -18,6 +18,13 @@ type acItem struct {
 
 func (acItem) TableName() string { return "lagoon_ac_items" }
 
+type acOther struct {
+	ID    uint   `gorm:"column:id;primaryKey"`
+	Label string `gorm:"column:label"`
+}
+
+func (acOther) TableName() string { return "lagoon_ac_others" }
+
 // recorder collects after-commit callback names and whether the row they
 // were registered for was visible to another connection when they ran.
 type recorder struct {
@@ -188,8 +195,11 @@ func TestTransactionAfterCommit(t *testing.T) {
 		var got *gorm.DB
 		err := gdb.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
 			AfterCommit(ctx, tx, func(_ context.Context, d *gorm.DB) { got = d })
-			if got != tx {
-				t.Error("callback did not run immediately with the tx handle")
+			if got == nil {
+				t.Fatal("callback did not run immediately")
+			}
+			if got.Statement.ConnPool != tx.Statement.ConnPool {
+				t.Error("callback did not run on the transaction's connection")
 			}
 			return nil
 		})
@@ -198,6 +208,65 @@ func TestTransactionAfterCommit(t *testing.T) {
 		}
 	})
 
+	// The handle a callback receives has an empty statement on the write's
+	// connection: a query through it (with WithContext, as application code
+	// writes it) must not continue from the written model's statement.
+	t.Run("callback_handle_has_a_clean_statement", func(t *testing.T) {
+		if err := gdb.Exec(`CREATE TABLE lagoon_ac_others (id SERIAL PRIMARY KEY, label TEXT NOT NULL)`).Error; err != nil {
+			t.Fatal(err)
+		}
+		if err := gdb.Exec(`INSERT INTO lagoon_ac_others (label) VALUES ('other')`).Error; err != nil {
+			t.Fatal(err)
+		}
+		g3, err := Use(ctx, db)
+		if err != nil {
+			t.Fatal(err)
+		}
+		var mu sync.Mutex
+		labels := map[string]string{}
+		if err := g3.Callback().Create().Before("gorm:create").Register("lagoon_test:clean_handle", func(stmt *gorm.DB) {
+			item, ok := stmt.Statement.Dest.(*acItem)
+			if !ok {
+				return
+			}
+			name := item.Name
+			AfterCommit(stmt.Statement.Context, stmt, func(ctx context.Context, d *gorm.DB) {
+				var o acOther
+				label := ""
+				if err := d.WithContext(ctx).Take(&o).Error; err != nil {
+					label = "error: " + err.Error()
+				} else {
+					label = o.Label
+				}
+				mu.Lock()
+				labels[name] = label
+				mu.Unlock()
+			})
+		}); err != nil {
+			t.Fatal(err)
+		}
+		if err := g3.WithContext(ctx).Create(&acItem{Name: "clean-implicit"}).Error; err != nil {
+			t.Fatal(err)
+		}
+		if err := g3.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
+			return tx.Create(&acItem{Name: "clean-plain-tx"}).Error
+		}); err != nil {
+			t.Fatal(err)
+		}
+		if err := Transaction(ctx, g3, func(ctx context.Context, tx *gorm.DB) error {
+			return tx.WithContext(ctx).Create(&acItem{Name: "clean-lagoon-tx"}).Error
+		}); err != nil {
+			t.Fatal(err)
+		}
+		mu.Lock()
+		defer mu.Unlock()
+		for _, name := range []string{"clean-implicit", "clean-plain-tx", "clean-lagoon-tx"} {
+			if got := labels[name]; got != "other" {
+				t.Errorf("%s: query through the callback handle read %q, want the lagoon_ac_others row \"other\"", name, got)
+			}
+		}
+	})
+
 	t.Run("outside_transaction_runs_now", func(t *testing.T) {
 		ran := false
 		AfterCommit(ctx, gdb, func(context.Context, *gorm.DB) { ran = true })