loading up the forgejo repo on tangled to test page performance
Fix misuse of `TxContext` (#30061)
Help #29999, or its tests cannot pass.
Also, add some comments to clarify the usage of `TxContext`.
I don't check all usages of `TxContext` because there are too many
(almost 140+). It's a better idea to replace them with `WithTx` instead
of checking them one by one. However, that may be another refactoring
PR.
(cherry picked from commit c6c4d66004c70b24abc8048b39b660b8361a0395)
authored by
Jason Song
and committed by
Earl Warren
2c4e8542
8e3e31a5
+11
-1
+10
models/db/context.go
+10
models/db/context.go
···
120
120
121
121
// TxContext represents a transaction Context,
122
122
// it will reuse the existing transaction in the parent context or create a new one.
123
+
// Some tips to use:
124
+
//
125
+
// 1 It's always recommended to use `WithTx` in new code instead of `TxContext`, since `WithTx` will handle the transaction automatically.
126
+
// 2. To maintain the old code which uses `TxContext`:
127
+
// a. Always call `Close()` before returning regardless of whether `Commit()` has been called.
128
+
// b. Always call `Commit()` before returning if there are no errors, even if the code did not change any data.
129
+
// c. Remember the `Committer` will be a halfCommitter when a transaction is being reused.
130
+
// So calling `Commit()` will do nothing, but calling `Close()` without calling `Commit()` will rollback the transaction.
131
+
// And all operations submitted by the caller stack will be rollbacked as well, not only the operations in the current function.
132
+
// d. It doesn't mean rollback is forbidden, but always do it only when there is an error, and you do want to rollback.
123
133
func TxContext(parentCtx context.Context) (*Context, Committer, error) {
124
134
if sess, ok := inTransaction(parentCtx); ok {
125
135
return newContext(parentCtx, sess, true), &halfCommitter{committer: sess}, nil
+1
-1
models/issues/review.go
+1
-1
models/issues/review.go
···
620
620
621
621
// skip it when reviewer hase been request to review
622
622
if review != nil && review.Type == ReviewTypeRequest {
623
-
return nil, nil
623
+
return nil, committer.Commit() // still commit the transaction, or committer.Close() will rollback it, even if it's a reused transaction.
624
624
}
625
625
626
626
// if the reviewer is an official reviewer,