mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2025-12-08 06:29:47 +00:00
a9452d11d0
Although #9922 was deployed to Codeberg, it was reported on Matrix that a user observed a `-1` pull request count. @Gusted checked and verified that the stats stored in redis appeared incorrect, and that no errors occurred on Codeberg that included the repo ID (eg. deadlocks, SQL queries). ``` 127.0.0.1:6379> GET Repo:CountPulls:924266 "1" 127.0.0.1:6379> GET Repo:CountPullsClosed:924266 "2" ``` One possible cause is that when `UpdateRepoIssueNumbers` is invoked and invalidates the cache key for the repository, it is currently in a transaction; the next request for that cached count could be computed before the transaction is committed and the update is visible. It's been verified that `UpdateRepoIssueNumbers` is called within a transaction in most interactions (I put a panic in it if `db.InTransaction(ctx)`, and most related tests failed). This PR fixes that hole by performing the cache invalidation in an `AfterTx()` hook which is invoked after the transaction is committed to the database. (Another possible cause is documented in #10127) Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10130 Reviewed-by: Gusted <gusted@noreply.codeberg.org> Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net> Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
102 lines
2.1 KiB
Go
102 lines
2.1 KiB
Go
// Copyright 2022 The Gitea Authors. All rights reserved.
|
|
// SPDX-License-Identifier: MIT
|
|
|
|
package db // it's not db_test, because this file is for testing the private type halfCommitter
|
|
|
|
import (
|
|
"errors"
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/assert"
|
|
)
|
|
|
|
type MockCommitter struct {
|
|
wants []string
|
|
gots []string
|
|
}
|
|
|
|
func NewMockCommitter(wants ...string) *MockCommitter {
|
|
return &MockCommitter{
|
|
wants: wants,
|
|
}
|
|
}
|
|
|
|
func (c *MockCommitter) Commit() error {
|
|
c.gots = append(c.gots, "commit")
|
|
return nil
|
|
}
|
|
|
|
func (c *MockCommitter) Close() error {
|
|
c.gots = append(c.gots, "close")
|
|
return nil
|
|
}
|
|
|
|
func (c *MockCommitter) Assert(t *testing.T) {
|
|
assert.Equal(t, c.wants, c.gots, "want operations %v, but got %v", c.wants, c.gots)
|
|
}
|
|
|
|
func Test_halfCommitter(t *testing.T) {
|
|
/*
|
|
Do something like:
|
|
|
|
ctx, committer, err := db.TxContext(db.DefaultContext)
|
|
if err != nil {
|
|
return nil
|
|
}
|
|
defer committer.Close()
|
|
|
|
// ...
|
|
|
|
if err != nil {
|
|
return nil
|
|
}
|
|
|
|
// ...
|
|
|
|
return committer.Commit()
|
|
*/
|
|
|
|
testWithCommitter := func(committer Committer, f func(committer Committer) error) {
|
|
if err := f(&halfCommitter{committer: committer, txCtx: &Context{}}); err == nil {
|
|
committer.Commit()
|
|
}
|
|
committer.Close()
|
|
}
|
|
|
|
t.Run("commit and close", func(t *testing.T) {
|
|
mockCommitter := NewMockCommitter("commit", "close")
|
|
|
|
testWithCommitter(mockCommitter, func(committer Committer) error {
|
|
defer committer.Close()
|
|
return committer.Commit()
|
|
})
|
|
|
|
mockCommitter.Assert(t)
|
|
})
|
|
|
|
t.Run("rollback and close", func(t *testing.T) {
|
|
mockCommitter := NewMockCommitter("close", "close")
|
|
|
|
testWithCommitter(mockCommitter, func(committer Committer) error {
|
|
defer committer.Close()
|
|
if true {
|
|
return errors.New("error")
|
|
}
|
|
return committer.Commit()
|
|
})
|
|
|
|
mockCommitter.Assert(t)
|
|
})
|
|
|
|
t.Run("close and commit", func(t *testing.T) {
|
|
mockCommitter := NewMockCommitter("close", "close")
|
|
|
|
testWithCommitter(mockCommitter, func(committer Committer) error {
|
|
committer.Close()
|
|
committer.Commit()
|
|
return errors.New("error")
|
|
})
|
|
|
|
mockCommitter.Assert(t)
|
|
})
|
|
}
|