Merge remote-tracking branch 'upstream/main'

* upstream/main:
  Replace `can not` with `cannot` (go-gitea#22372)
  Fix set system setting failure once it cached (go-gitea#22333)
  Bump json5 from 1.0.1 to 1.0.2 (go-gitea#22365)
  Always reuse transaction (go-gitea#22362)
  make /{username}.png redirect to user/org avatar (go-gitea#22356)
  Remove old HookEventType (go-gitea#22358)

models/activities/notification.go

@@ -141,7 +141,7 @@ func CountNotifications(ctx context.Context, opts *FindNotificationOptions) (int
 
 // CreateRepoTransferNotification creates  notification for the user a repository was transferred to
 func CreateRepoTransferNotification(ctx context.Context, doer, newOwner *user_model.User, repo *repo_model.Repository) error {
-	return db.AutoTx(ctx, func(ctx context.Context) error {
+	return db.WithTx(ctx, func(ctx context.Context) error {
 		var notify []*Notification
 
 		if newOwner.IsOrganization() {

models/db/context.go

@@ -71,14 +71,22 @@ type Engined interface {
 
 // GetEngine will get a db Engine from this context or return an Engine restricted to this context
 func GetEngine(ctx context.Context) Engine {
+	if e := getEngine(ctx); e != nil {
+		return e
+	}
+	return x.Context(ctx)
+}
+
+// getEngine will get a db Engine from this context or return nil
+func getEngine(ctx context.Context) Engine {
 	if engined, ok := ctx.(Engined); ok {
 		return engined.Engine()
 	}
 	enginedInterface := ctx.Value(enginedContextKey)
 	if enginedInterface != nil {
 		return enginedInterface.(Engined).Engine()
 	}
-	return x.Context(ctx)
+	return nil
 }
 
 // Committer represents an interface to Commit or Close the Context
@@ -87,10 +95,22 @@ type Committer interface {
 	Close() error
 }
 
-// TxContext represents a transaction Context
+// halfCommitter is a wrapper of Committer.
+// It can be closed early, but can't be committed early, it is useful for reusing a transaction.
+type halfCommitter struct {
+	Committer
+}
+
+func (*halfCommitter) Commit() error {
+	// do nothing
+	return nil
+}
+
+// TxContext represents a transaction Context,
+// it will reuse the existing transaction in the parent context or create a new one.
 func TxContext(parentCtx context.Context) (*Context, Committer, error) {
-	if InTransaction(parentCtx) {
-		return nil, nil, ErrAlreadyInTransaction
+	if sess, ok := inTransaction(parentCtx); ok {
+		return newContext(parentCtx, sess, true), &halfCommitter{Committer: sess}, nil
 	}
 
 	sess := x.NewSession()
@@ -102,20 +122,11 @@ func TxContext(parentCtx context.Context) (*Context, Committer, error) {
 	return newContext(DefaultContext, sess, true), sess, nil
 }
 
-// WithTx represents executing database operations on a transaction
-// This function will always open a new transaction, if a transaction exist in parentCtx return an error.
-func WithTx(parentCtx context.Context, f func(ctx context.Context) error) error {
-	if InTransaction(parentCtx) {
-		return ErrAlreadyInTransaction
-	}
-	return txWithNoCheck(parentCtx, f)
-}
-
-// AutoTx represents executing database operations on a transaction, if the transaction exist,
+// WithTx represents executing database operations on a transaction, if the transaction exist,
 // this function will reuse it otherwise will create a new one and close it when finished.
-func AutoTx(parentCtx context.Context, f func(ctx context.Context) error) error {
-	if InTransaction(parentCtx) {
-		return f(newContext(parentCtx, GetEngine(parentCtx), true))
+func WithTx(parentCtx context.Context, f func(ctx context.Context) error) error {
+	if sess, ok := inTransaction(parentCtx); ok {
+		return f(newContext(parentCtx, sess, true))
 	}
 	return txWithNoCheck(parentCtx, f)
 }
@@ -202,25 +213,25 @@ func EstimateCount(ctx context.Context, bean interface{}) (int64, error) {
 
 // InTransaction returns true if the engine is in a transaction otherwise return false
 func InTransaction(ctx context.Context) bool {
-	var e Engine
-	if engined, ok := ctx.(Engined); ok {
-		e = engined.Engine()
-	} else {
-		enginedInterface := ctx.Value(enginedContextKey)
-		if enginedInterface != nil {
-			e = enginedInterface.(Engined).Engine()
-		}
-	}
+	_, ok := inTransaction(ctx)
+	return ok
+}
+
+func inTransaction(ctx context.Context) (*xorm.Session, bool) {
+	e := getEngine(ctx)
 	if e == nil {
-		return false
+		return nil, false
 	}
 
 	switch t := e.(type) {
 	case *xorm.Engine:
-		return false
+		return nil, false
 	case *xorm.Session:
-		return t.IsInTx()
+		if t.IsInTx() {
+			return t, true
+		}
+		return nil, false
 	default:
-		return false
+		return nil, false
 	}
 }

models/db/context_test.go

@@ -25,8 +25,62 @@ func TestInTransaction(t *testing.T) {
 	assert.NoError(t, err)
 	defer committer.Close()
 	assert.True(t, db.InTransaction(ctx))
-	assert.Error(t, db.WithTx(ctx, func(ctx context.Context) error {
+	assert.NoError(t, db.WithTx(ctx, func(ctx context.Context) error {
 		assert.True(t, db.InTransaction(ctx))
 		return nil
 	}))
 }
+
+func TestTxContext(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	{ // create new transaction
+		ctx, committer, err := db.TxContext(db.DefaultContext)
+		assert.NoError(t, err)
+		assert.True(t, db.InTransaction(ctx))
+		assert.NoError(t, committer.Commit())
+	}
+
+	{ // reuse the transaction created by TxContext and commit it
+		ctx, committer, err := db.TxContext(db.DefaultContext)
+		engine := db.GetEngine(ctx)
+		assert.NoError(t, err)
+		assert.True(t, db.InTransaction(ctx))
+		{
+			ctx, committer, err := db.TxContext(ctx)
+			assert.NoError(t, err)
+			assert.True(t, db.InTransaction(ctx))
+			assert.Equal(t, engine, db.GetEngine(ctx))
+			assert.NoError(t, committer.Commit())
+		}
+		assert.NoError(t, committer.Commit())
+	}
+
+	{ // reuse the transaction created by TxContext and close it
+		ctx, committer, err := db.TxContext(db.DefaultContext)
+		engine := db.GetEngine(ctx)
+		assert.NoError(t, err)
+		assert.True(t, db.InTransaction(ctx))
+		{
+			ctx, committer, err := db.TxContext(ctx)
+			assert.NoError(t, err)
+			assert.True(t, db.InTransaction(ctx))
+			assert.Equal(t, engine, db.GetEngine(ctx))
+			assert.NoError(t, committer.Close())
+		}
+		assert.NoError(t, committer.Close())
+	}
+
+	{ // reuse the transaction created by WithTx
+		assert.NoError(t, db.WithTx(db.DefaultContext, func(ctx context.Context) error {
+			assert.True(t, db.InTransaction(ctx))
+			{
+				ctx, committer, err := db.TxContext(ctx)
+				assert.NoError(t, err)
+				assert.True(t, db.InTransaction(ctx))
+				assert.NoError(t, committer.Commit())
+			}
+			return nil
+		}))
+	}
+}

models/db/error.go

@@ -4,14 +4,11 @@
 package db
 
 import (
-	"errors"
 	"fmt"
 
 	"code.gitea.io/gitea/modules/util"
 )
 
-var ErrAlreadyInTransaction = errors.New("database connection has already been in a transaction")
-
 // ErrCancelled represents an error due to context cancellation
 type ErrCancelled struct {
 	Message string

models/issues/issue.go

@@ -2365,7 +2365,7 @@ func CountOrphanedIssues(ctx context.Context) (int64, error) {
 // DeleteOrphanedIssues delete issues without a repo
 func DeleteOrphanedIssues(ctx context.Context) error {
 	var attachmentPaths []string
-	err := db.AutoTx(ctx, func(ctx context.Context) error {
+	err := db.WithTx(ctx, func(ctx context.Context) error {
 		var ids []int64
 
 		if err := db.GetEngine(ctx).Table("issue").Distinct("issue.repo_id").

models/project/project.go

@@ -300,7 +300,7 @@ func changeProjectStatus(ctx context.Context, p *Project, isClosed bool) error {
 // DeleteProjectByID deletes a project from a repository. if it's not in a database
 // transaction, it will start a new database transaction
 func DeleteProjectByID(ctx context.Context, id int64) error {
-	return db.AutoTx(ctx, func(ctx context.Context) error {
+	return db.WithTx(ctx, func(ctx context.Context) error {
 		p, err := GetProjectByID(ctx, id)
 		if err != nil {
 			if IsErrProjectNotExist(err) {

models/repo/collaboration.go

@@ -105,7 +105,7 @@ func ChangeCollaborationAccessMode(ctx context.Context, repo *Repository, uid in
 		return nil
 	}
 
-	return db.AutoTx(ctx, func(ctx context.Context) error {
+	return db.WithTx(ctx, func(ctx context.Context) error {
 		e := db.GetEngine(ctx)
 
 		collaboration := &Collaboration{

models/repo_transfer.go

@@ -155,7 +155,7 @@ func TestRepositoryReadyForTransfer(status repo_model.RepositoryStatus) error {
 // CreatePendingRepositoryTransfer transfer a repo from one owner to a new one.
 // it marks the repository transfer as "pending"
 func CreatePendingRepositoryTransfer(ctx context.Context, doer, newOwner *user_model.User, repoID int64, teams []*organization.Team) error {
-	return db.AutoTx(ctx, func(ctx context.Context) error {
+	return db.WithTx(ctx, func(ctx context.Context) error {
 		repo, err := repo_model.GetRepositoryByID(ctx, repoID)
 		if err != nil {
 			return err

models/system/setting.go

@@ -12,7 +12,7 @@ import (
 
 	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/modules/cache"
-	"code.gitea.io/gitea/modules/setting"
+	setting_module "code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/timeutil"
 
 	"strk.kbt.io/projects/go/libravatar"
@@ -88,7 +88,7 @@ func GetSettingNoCache(key string) (*Setting, error) {
 	if len(v) == 0 {
 		return nil, ErrSettingIsNotExist{key}
 	}
-	return v[key], nil
+	return v[strings.ToLower(key)], nil
 }
 
 // GetSetting returns the setting value via the key
@@ -131,7 +131,7 @@ func GetSettings(keys []string) (map[string]*Setting, error) {
 type AllSettings map[string]*Setting
 
 func (settings AllSettings) Get(key string) Setting {
-	if v, ok := settings[key]; ok {
+	if v, ok := settings[strings.ToLower(key)]; ok {
 		return *v
 	}
 	return Setting{}
@@ -184,14 +184,17 @@ func SetSettingNoVersion(key, value string) error {
 
 // SetSetting updates a users' setting for a specific key
 func SetSetting(setting *Setting) error {
-	_, err := cache.GetString(genSettingCacheKey(setting.SettingKey), func() (string, error) {
-		return setting.SettingValue, upsertSettingValue(strings.ToLower(setting.SettingKey), setting.SettingValue, setting.Version)
-	})
-	if err != nil {
+	if err := upsertSettingValue(strings.ToLower(setting.SettingKey), setting.SettingValue, setting.Version); err != nil {
 		return err
 	}
 
 	setting.Version++
+
+	cc := cache.GetCache()
+	if cc != nil {
+		return cc.Put(genSettingCacheKey(setting.SettingKey), setting.SettingValue, setting_module.CacheService.TTLSeconds())
+	}
+
 	return nil
 }
 
@@ -243,7 +246,7 @@ func Init() error {
 	var disableGravatar bool
 	disableGravatarSetting, err := GetSettingNoCache(KeyPictureDisableGravatar)
 	if IsErrSettingIsNotExist(err) {
-		disableGravatar = setting.GetDefaultDisableGravatar()
+		disableGravatar = setting_module.GetDefaultDisableGravatar()
 		disableGravatarSetting = &Setting{SettingValue: strconv.FormatBool(disableGravatar)}
 	} else if err != nil {
 		return err
@@ -254,24 +257,24 @@ func Init() error {
 	var enableFederatedAvatar bool
 	enableFederatedAvatarSetting, err := GetSettingNoCache(KeyPictureEnableFederatedAvatar)
 	if IsErrSettingIsNotExist(err) {
-		enableFederatedAvatar = setting.GetDefaultEnableFederatedAvatar(disableGravatar)
+		enableFederatedAvatar = setting_module.GetDefaultEnableFederatedAvatar(disableGravatar)
 		enableFederatedAvatarSetting = &Setting{SettingValue: strconv.FormatBool(enableFederatedAvatar)}
 	} else if err != nil {
 		return err
 	} else {
 		enableFederatedAvatar = disableGravatarSetting.GetValueBool()
 	}
 
-	if setting.OfflineMode {
+	if setting_module.OfflineMode {
 		disableGravatar = true
 		enableFederatedAvatar = false
 	}
 
 	if enableFederatedAvatar || !disableGravatar {
 		var err error
-		GravatarSourceURL, err = url.Parse(setting.GravatarSource)
+		GravatarSourceURL, err = url.Parse(setting_module.GravatarSource)
 		if err != nil {
-			return fmt.Errorf("Failed to parse Gravatar URL(%s): %w", setting.GravatarSource, err)
+			return fmt.Errorf("Failed to parse Gravatar URL(%s): %w", setting_module.GravatarSource, err)
 		}
 	}
 

models/system/setting_test.go

@@ -33,10 +33,14 @@ func TestSettings(t *testing.T) {
 	assert.EqualValues(t, newSetting.SettingValue, settings[strings.ToLower(keyName)].SettingValue)
 
 	// updated setting
-	updatedSetting := &system.Setting{SettingKey: keyName, SettingValue: "100", Version: newSetting.Version}
+	updatedSetting := &system.Setting{SettingKey: keyName, SettingValue: "100", Version: settings[strings.ToLower(keyName)].Version}
 	err = system.SetSetting(updatedSetting)
 	assert.NoError(t, err)
 
+	value, err := system.GetSetting(keyName)
+	assert.NoError(t, err)
+	assert.EqualValues(t, updatedSetting.SettingValue, value)
+
 	// get all settings
 	settings, err = system.GetAllSettings()
 	assert.NoError(t, err)