Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion models/auth/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func RegenerateSession(ctx context.Context, oldKey, newKey string) (*Session, er
}
}

if _, err := db.Exec(ctx, "UPDATE "+db.TableName(&Session{})+" SET `key` = ? WHERE `key`=?", newKey, oldKey); err != nil {
if _, err := db.Exec(ctx, "UPDATE `session` SET `key` = ? WHERE `key`=?", newKey, oldKey); err != nil {
return nil, err
}

Expand Down
3 changes: 1 addition & 2 deletions models/auth/source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"testing"

auth_model "code.gitea.io/gitea/models/auth"
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/unittest"
"code.gitea.io/gitea/modules/json"

Expand Down Expand Up @@ -40,7 +39,7 @@ func (source *TestSource) ToDB() ([]byte, error) {
func TestDumpAuthSource(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())

authSourceSchema, err := db.TableInfo(new(auth_model.Source))
authSourceSchema, err := unittest.GetXORMEngine().TableInfo(new(auth_model.Source))
assert.NoError(t, err)

auth_model.RegisterTypeConfig(auth_model.OAuth2, new(TestSource))
Expand Down
13 changes: 5 additions & 8 deletions models/db/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,19 @@ func contextSafetyCheck(e Engine) {
callerNum := runtime.Callers(3, callers) // skip 3: runtime.Callers, contextSafetyCheck, GetEngine
for i := range callerNum {
if slices.Contains(contextSafetyDeniedFuncPCs, callers[i]) {
panic(errors.New("using database context in an iterator would cause corrupted results"))
panic(errors.New("using session context in an iterator would cause corrupted results"))
}
}
}

// GetEngine gets an existing db Engine/Statement or creates a new Session
func GetEngine(ctx context.Context) (e Engine) {
defer func() { contextSafetyCheck(e) }()
func GetEngine(ctx context.Context) Engine {
if engine, ok := ctx.Value(engineContextKey).(Engine); ok {
// if reusing the existing session, need to do "contextSafetyCheck" because the Iterate creates a "autoResetStatement=false" session
contextSafetyCheck(engine)
return engine
}
// no need to do "contextSafetyCheck" because it's a new Session
return xormEngine.Context(ctx)
}

Expand Down Expand Up @@ -301,11 +303,6 @@ func CountByBean(ctx context.Context, bean any) (int64, error) {
return GetEngine(ctx).Count(bean)
}

// TableName returns the table name according a bean object
func TableName(bean any) string {
return xormEngine.TableName(bean)
}

// InTransaction returns true if the engine is in a transaction otherwise return false
func InTransaction(ctx context.Context) bool {
return getTransactionSession(ctx) != nil
Expand Down
47 changes: 26 additions & 21 deletions models/db/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,31 +100,36 @@ func TestContextSafety(t *testing.T) {
assert.NoError(t, db.Insert(t.Context(), &TestModel2{ID: int64(-i)}))
}

actualCount := 0
// here: db.GetEngine(t.Context()) is a new *Session created from *Engine
_ = db.WithTx(t.Context(), func(ctx context.Context) error {
_ = db.GetEngine(ctx).Iterate(&TestModel1{}, func(i int, bean any) error {
// here: db.GetEngine(ctx) is always the unclosed "Iterate" *Session with autoResetStatement=false,
// and the internal states (including "cond" and others) are always there and not be reset in this callback.
m1 := bean.(*TestModel1)
assert.EqualValues(t, i+1, m1.ID)

// here: XORM bug, it fails because the SQL becomes "WHERE id=-1", "WHERE id=-1 AND id=-2", "WHERE id=-1 AND id=-2 AND id=-3" ...
// and it conflicts with the "Iterate"'s internal states.
// has, err := db.GetEngine(ctx).Get(&TestModel2{ID: -m1.ID})

actualCount++
t.Run("Show-XORM-Bug", func(t *testing.T) {
actualCount := 0
// here: db.GetEngine(t.Context()) is a new *Session created from *Engine
_ = db.WithTx(t.Context(), func(ctx context.Context) error {
_ = db.GetEngine(ctx).Iterate(&TestModel1{}, func(i int, bean any) error {
// here: db.GetEngine(ctx) is always the unclosed "Iterate" *Session with autoResetStatement=false,
// and the internal states (including "cond" and others) are always there and not be reset in this callback.
m1 := bean.(*TestModel1)
assert.EqualValues(t, i+1, m1.ID)

// here: XORM bug, it fails because the SQL becomes "WHERE id=-1", "WHERE id=-1 AND id=-2", "WHERE id=-1 AND id=-2 AND id=-3" ...
// and it conflicts with the "Iterate"'s internal states.
// has, err := db.GetEngine(ctx).Get(&TestModel2{ID: -m1.ID})

actualCount++
return nil
})
return nil
})
return nil
assert.Equal(t, testCount, actualCount)
})
assert.Equal(t, testCount, actualCount)

// deny the bad usages
assert.PanicsWithError(t, "using database context in an iterator would cause corrupted results", func() {
_ = unittest.GetXORMEngine().Iterate(&TestModel1{}, func(i int, bean any) error {
_ = db.GetEngine(t.Context())
return nil
t.Run("DenyBadUsage", func(t *testing.T) {
assert.PanicsWithError(t, "using session context in an iterator would cause corrupted results", func() {
_ = db.WithTx(t.Context(), func(ctx context.Context) error {
return db.GetEngine(ctx).Iterate(&TestModel1{}, func(i int, bean any) error {
_ = db.GetEngine(ctx)
return nil
})
})
})
})
}
6 changes: 0 additions & 6 deletions models/db/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"strings"

"xorm.io/xorm"
"xorm.io/xorm/schemas"

_ "github.com/go-sql-driver/mysql" // Needed for the MySQL driver
_ "github.com/lib/pq" // Needed for the Postgresql driver
Expand Down Expand Up @@ -67,11 +66,6 @@ var (
_ Engine = (*xorm.Session)(nil)
)

// TableInfo returns table's information via an object
func TableInfo(v any) (*schemas.Table, error) {
return xormEngine.TableInfo(v)
}

// RegisterModel registers model, if initFuncs provided, it will be invoked after data model sync
func RegisterModel(bean any, initFunc ...func() error) {
registeredModels = append(registeredModels, bean)
Expand Down
2 changes: 1 addition & 1 deletion models/db/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func TestPrimaryKeys(t *testing.T) {
}

for _, bean := range beans {
table, err := db.TableInfo(bean)
table, err := db.GetXORMEngineForTesting().TableInfo(bean)
if err != nil {
t.Fatal(err)
}
Expand Down
7 changes: 1 addition & 6 deletions models/db/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,7 @@ type ResourceIndex struct {
MaxIndex int64 `xorm:"index"`
}

var (
// ErrResouceOutdated represents an error when request resource outdated
ErrResouceOutdated = errors.New("resource outdated")
// ErrGetResourceIndexFailed represents an error when resource index retries 3 times
ErrGetResourceIndexFailed = errors.New("get resource index failed")
)
var ErrGetResourceIndexFailed = errors.New("get resource index failed")

// SyncMaxResourceIndex sync the max index with the resource
func SyncMaxResourceIndex(ctx context.Context, tableName string, groupID, maxIndex int64) (err error) {
Expand Down
7 changes: 1 addition & 6 deletions models/organization/org_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,6 @@ type MinimalOrg = Organization

// GetUserOrgsList returns all organizations the given user has access to
func GetUserOrgsList(ctx context.Context, user *user_model.User) ([]*MinimalOrg, error) {
schema, err := db.TableInfo(new(user_model.User))
if err != nil {
return nil, err
}

outputCols := []string{
"id",
"name",
Expand All @@ -122,7 +117,7 @@ func GetUserOrgsList(ctx context.Context, user *user_model.User) ([]*MinimalOrg,

selectColumns := &strings.Builder{}
for i, col := range outputCols {
fmt.Fprintf(selectColumns, "`%s`.%s", schema.Name, col)
_, _ = fmt.Fprintf(selectColumns, "`user`.%s", col)
if i < len(outputCols)-1 {
selectColumns.WriteString(", ")
}
Expand Down
2 changes: 1 addition & 1 deletion models/unittest/consistency.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func CheckConsistencyFor(t TestingT, beansToCheck ...any) {
}

func checkForConsistency(t TestingT, bean any) {
tb, err := db.TableInfo(bean)
tb, err := GetXORMEngine().TableInfo(bean)
assert.NoError(t, err)
f := consistencyCheckMap[tb.Name]
require.NotNil(t, f, "unknown bean type: %#v", bean)
Expand Down
2 changes: 1 addition & 1 deletion models/unittest/fixtures_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ func NewFixturesLoader(x *xorm.Engine, opts FixturesOptions) (FixturesLoader, er
xormBeans, _ := db.NamesToBean()
f.xormTableNames = map[string]bool{}
for _, bean := range xormBeans {
f.xormTableNames[db.TableName(bean)] = true
f.xormTableNames[x.TableName(bean)] = true
}

return f, nil
Expand Down
2 changes: 1 addition & 1 deletion models/unittest/unit_tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ func DumpQueryResult(t require.TestingT, sqlOrBean any, sqlArgs ...any) {
goDB := x.DB().DB
sql, ok := sqlOrBean.(string)
if !ok {
sql = "SELECT * FROM " + db.TableName(sqlOrBean)
sql = "SELECT * FROM " + x.TableName(sqlOrBean)
} else if !strings.Contains(sql, " ") {
sql = "SELECT * FROM " + sql
}
Expand Down