Merge remote-tracking branch 'giteaofficial/main'

* giteaofficial/main:
  Test renderReadmeFile (go-gitea#23185)
  [skip ci] Updated translations via Crowdin
  Set `X-Gitea-Debug` header once (go-gitea#23361)
  Improve cache context (go-gitea#23330)
  add user visibility in dashboard navbar (go-gitea#22747)
  Fix panic when getting notes by ref (go-gitea#23372)
  Use CleanPath instead of path.Clean (go-gitea#23371)
  Reduce duplicate and useless code in options (go-gitea#23369)
  Clean Path in Options (go-gitea#23006)
  Do not recognize text files as audio (go-gitea#23355)
  Fix incorrect display for comment context menu  (go-gitea#23343)

# Conflicts:
#	templates/repo/issue/view_content/context_menu.tmpl

models/db/iterate_test.go

@@ -25,7 +25,7 @@ func TestIterate(t *testing.T) {
 		return nil
 	})
 	assert.NoError(t, err)
-	assert.EqualValues(t, 83, repoCnt)
+	assert.EqualValues(t, 84, repoCnt)
 
 	err = db.Iterate(db.DefaultContext, nil, func(ctx context.Context, repoUnit *repo_model.RepoUnit) error {
 		reopUnit2 := repo_model.RepoUnit{ID: repoUnit.ID}

models/db/list_test.go

@@ -35,11 +35,11 @@ func TestFind(t *testing.T) {
 	var repoUnits []repo_model.RepoUnit
 	err := db.Find(db.DefaultContext, &opts, &repoUnits)
 	assert.NoError(t, err)
-	assert.EqualValues(t, 83, len(repoUnits))
+	assert.EqualValues(t, 84, len(repoUnits))
 
 	cnt, err := db.Count(db.DefaultContext, &opts, new(repo_model.RepoUnit))
 	assert.NoError(t, err)
-	assert.EqualValues(t, 83, cnt)
+	assert.EqualValues(t, 84, cnt)
 
 	repoUnits = make([]repo_model.RepoUnit, 0, 10)
 	newCnt, err := db.FindAndCount(db.DefaultContext, &opts, &repoUnits)

models/fixtures/repo_unit.yml

@@ -569,3 +569,9 @@
   type: 3
   config: "{\"IgnoreWhitespaceConflicts\":false,\"AllowMerge\":true,\"AllowRebase\":true,\"AllowRebaseMerge\":true,\"AllowSquash\":true}"
   created_unix: 946684810
+
+-
+  id: 84
+  repo_id: 56
+  type: 1
+  created_unix: 946684810

models/fixtures/repository.yml

@@ -1634,3 +1634,16 @@
   is_private: true
   num_issues: 1
   status: 0
+
+-
+  id: 56
+  owner_id: 2
+  owner_name: user2
+  lower_name: readme-test
+  name: readme-test
+  default_branch: master
+  is_empty: false
+  is_archived: false
+  is_private: true
+  status: 0
+  num_issues: 0

models/fixtures/user.yml

@@ -66,7 +66,7 @@
   num_followers: 2
   num_following: 1
   num_stars: 2
-  num_repos: 11
+  num_repos: 12
   num_teams: 0
   num_members: 0
   visibility: 0

models/git/lfs_lock.go

@@ -6,7 +6,6 @@ package git
 import (
 	"context"
 	"fmt"
-	"path"
 	"strings"
 	"time"
 
@@ -17,6 +16,7 @@ import (
 	"code.gitea.io/gitea/models/unit"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/util"
 )
 
 // LFSLock represents a git lfs lock of repository.
@@ -34,11 +34,7 @@ func init() {
 
 // BeforeInsert is invoked from XORM before inserting an object of this type.
 func (l *LFSLock) BeforeInsert() {
-	l.Path = cleanPath(l.Path)
-}
-
-func cleanPath(p string) string {
-	return path.Clean("/" + p)[1:]
+	l.Path = util.CleanPath(l.Path)
 }
 
 // CreateLFSLock creates a new lock.
@@ -53,7 +49,7 @@ func CreateLFSLock(ctx context.Context, repo *repo_model.Repository, lock *LFSLo
 		return nil, err
 	}
 
-	lock.Path = cleanPath(lock.Path)
+	lock.Path = util.CleanPath(lock.Path)
 	lock.RepoID = repo.ID
 
 	l, err := GetLFSLock(dbCtx, repo, lock.Path)
@@ -73,7 +69,7 @@ func CreateLFSLock(ctx context.Context, repo *repo_model.Repository, lock *LFSLo
 
 // GetLFSLock returns release by given path.
 func GetLFSLock(ctx context.Context, repo *repo_model.Repository, path string) (*LFSLock, error) {
-	path = cleanPath(path)
+	path = util.CleanPath(path)
 	rel := &LFSLock{RepoID: repo.ID}
 	has, err := db.GetEngine(ctx).Where("lower(path) = ?", strings.ToLower(path)).Get(rel)
 	if err != nil {

modules/cache/context.go

@@ -6,6 +6,7 @@ package cache
 import (
 	"context"
 	"sync"
+	"time"
 
 	"code.gitea.io/gitea/modules/log"
 )
@@ -14,65 +15,151 @@ import (
 // This is useful for caching data that is expensive to calculate and is likely to be
 // used multiple times in a request.
 type cacheContext struct {
-	ctx  context.Context
-	data map[any]map[any]any
-	lock sync.RWMutex
+	data    map[any]map[any]any
+	lock    sync.RWMutex
+	created time.Time
+	discard bool
 }
 
 func (cc *cacheContext) Get(tp, key any) any {
 	cc.lock.RLock()
 	defer cc.lock.RUnlock()
-	if cc.data[tp] == nil {
-		return nil
-	}
 	return cc.data[tp][key]
 }
 
 func (cc *cacheContext) Put(tp, key, value any) {
 	cc.lock.Lock()
 	defer cc.lock.Unlock()
-	if cc.data[tp] == nil {
-		cc.data[tp] = make(map[any]any)
+
+	if cc.discard {
+		return
+	}
+
+	d := cc.data[tp]
+	if d == nil {
+		d = make(map[any]any)
+		cc.data[tp] = d
 	}
-	cc.data[tp][key] = value
+	d[key] = value
 }
 
 func (cc *cacheContext) Delete(tp, key any) {
 	cc.lock.Lock()
 	defer cc.lock.Unlock()
-	if cc.data[tp] == nil {
-		return
-	}
 	delete(cc.data[tp], key)
 }
 
+func (cc *cacheContext) Discard() {
+	cc.lock.Lock()
+	defer cc.lock.Unlock()
+	cc.data = nil
+	cc.discard = true
+}
+
+func (cc *cacheContext) isDiscard() bool {
+	cc.lock.RLock()
+	defer cc.lock.RUnlock()
+	return cc.discard
+}
+
+// cacheContextLifetime is the max lifetime of cacheContext.
+// Since cacheContext is used to cache data in a request level context, 10s is enough.
+// If a cacheContext is used more than 10s, it's probably misuse.
+const cacheContextLifetime = 10 * time.Second
+
+var timeNow = time.Now
+
+func (cc *cacheContext) Expired() bool {
+	return timeNow().Sub(cc.created) > cacheContextLifetime
+}
+
 var cacheContextKey = struct{}{}
 
+/*
+Since there are both WithCacheContext and WithNoCacheContext,
+it may be confusing when there is nesting.
+
+Some cases to explain the design:
+
+When:
+- A, B or C means a cache context.
+- A', B' or C' means a discard cache context.
+- ctx means context.Backgrand().
+- A(ctx) means a cache context with ctx as the parent context.
+- B(A(ctx)) means a cache context with A(ctx) as the parent context.
+- With is alias of WithCacheContext.
+- WithNo is alias of WithNoCacheContext.
+
+So:
+- With(ctx) -> A(ctx)
+- With(With(ctx)) -> A(ctx), not B(A(ctx)), always reuse parent cache context if possible.
+- With(With(With(ctx))) -> A(ctx), not C(B(A(ctx))), ditto.
+- WithNo(ctx) -> ctx, not A'(ctx), don't create new cache context if we don't have to.
+- WithNo(With(ctx)) -> A'(ctx)
+- WithNo(WithNo(With(ctx))) -> A'(ctx), not B'(A'(ctx)), don't create new cache context if we don't have to.
+- With(WithNo(With(ctx))) -> B(A'(ctx)), not A(ctx), never reuse a discard cache context.
+- WithNo(With(WithNo(With(ctx)))) -> B'(A'(ctx))
+- With(WithNo(With(WithNo(With(ctx))))) -> C(B'(A'(ctx))), so there's always only one not-discard cache context.
+*/
+
 func WithCacheContext(ctx context.Context) context.Context {
+	if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok {
+		if !c.isDiscard() {
+			// reuse parent context
+			return ctx
+		}
+	}
 	return context.WithValue(ctx, cacheContextKey, &cacheContext{
-		ctx:  ctx,
-		data: make(map[any]map[any]any),
+		data:    make(map[any]map[any]any),
+		created: timeNow(),
 	})
 }
 
+func WithNoCacheContext(ctx context.Context) context.Context {
+	if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok {
+		// The caller want to run long-life tasks, but the parent context is a cache context.
+		// So we should disable and clean the cache data, or it will be kept in memory for a long time.
+		c.Discard()
+		return ctx
+	}
+
+	return ctx
+}
+
 func GetContextData(ctx context.Context, tp, key any) any {
 	if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok {
+		if c.Expired() {
+			// The warning means that the cache context is misused for long-life task,
+			// it can be resolved with WithNoCacheContext(ctx).
+			log.Warn("cache context is expired, may be misused for long-life tasks: %v", c)
+			return nil
+		}
 		return c.Get(tp, key)
 	}
-	log.Warn("cannot get cache context when getting data: %v", ctx)
 	return nil
 }
 
 func SetContextData(ctx context.Context, tp, key, value any) {
 	if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok {
+		if c.Expired() {
+			// The warning means that the cache context is misused for long-life task,
+			// it can be resolved with WithNoCacheContext(ctx).
+			log.Warn("cache context is expired, may be misused for long-life tasks: %v", c)
+			return
+		}
 		c.Put(tp, key, value)
 		return
 	}
-	log.Warn("cannot get cache context when setting data: %v", ctx)
 }
 
 func RemoveContextData(ctx context.Context, tp, key any) {
 	if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok {
+		if c.Expired() {
+			// The warning means that the cache context is misused for long-life task,
+			// it can be resolved with WithNoCacheContext(ctx).
+			log.Warn("cache context is expired, may be misused for long-life tasks: %v", c)
+			return
+		}
 		c.Delete(tp, key)
 	}
 }

modules/cache/context_test.go

@@ -6,6 +6,7 @@ package cache
 import (
 	"context"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -25,7 +26,7 @@ func TestWithCacheContext(t *testing.T) {
 	assert.EqualValues(t, 1, v.(int))
 
 	RemoveContextData(ctx, field, "my_config1")
-	RemoveContextData(ctx, field, "my_config2") // remove an non-exist key
+	RemoveContextData(ctx, field, "my_config2") // remove a non-exist key
 
 	v = GetContextData(ctx, field, "my_config1")
 	assert.Nil(t, v)
@@ -38,4 +39,40 @@ func TestWithCacheContext(t *testing.T) {
 
 	v = GetContextData(ctx, field, "my_config1")
 	assert.EqualValues(t, 1, v)
+
+	now := timeNow
+	defer func() {
+		timeNow = now
+	}()
+	timeNow = func() time.Time {
+		return now().Add(10 * time.Second)
+	}
+	v = GetContextData(ctx, field, "my_config1")
+	assert.Nil(t, v)
+}
+
+func TestWithNoCacheContext(t *testing.T) {
+	ctx := context.Background()
+
+	const field = "system_setting"
+
+	v := GetContextData(ctx, field, "my_config1")
+	assert.Nil(t, v)
+	SetContextData(ctx, field, "my_config1", 1)
+	v = GetContextData(ctx, field, "my_config1")
+	assert.Nil(t, v) // still no cache
+
+	ctx = WithCacheContext(ctx)
+	v = GetContextData(ctx, field, "my_config1")
+	assert.Nil(t, v)
+	SetContextData(ctx, field, "my_config1", 1)
+	v = GetContextData(ctx, field, "my_config1")
+	assert.NotNil(t, v)
+
+	ctx = WithNoCacheContext(ctx)
+	v = GetContextData(ctx, field, "my_config1")
+	assert.Nil(t, v)
+	SetContextData(ctx, field, "my_config1", 1)
+	v = GetContextData(ctx, field, "my_config1")
+	assert.Nil(t, v) // still no cache
 }

modules/context/api.go

@@ -244,7 +244,7 @@ func APIContexter() func(http.Handler) http.Handler {
 				}
 			}
 
-			httpcache.AddCacheControlToHeader(ctx.Resp.Header(), 0, "no-transform")
+			httpcache.SetCacheControlInHeader(ctx.Resp.Header(), 0, "no-transform")
 			ctx.Resp.Header().Set(`X-Frame-Options`, setting.CORSConfig.XFrameOptions)
 
 			ctx.Data["Context"] = &ctx

modules/context/context.go

@@ -388,7 +388,7 @@ func (ctx *Context) SetServeHeaders(opts *ServeHeaderOptions) {
 	if duration == 0 {
 		duration = 5 * time.Minute
 	}
-	httpcache.AddCacheControlToHeader(header, duration)
+	httpcache.SetCacheControlInHeader(header, duration)
 
 	if !opts.LastModified.IsZero() {
 		header.Set("Last-Modified", opts.LastModified.UTC().Format(http.TimeFormat))
@@ -753,7 +753,7 @@ func Contexter(ctx context.Context) func(next http.Handler) http.Handler {
 				}
 			}
 
-			httpcache.AddCacheControlToHeader(ctx.Resp.Header(), 0, "no-transform")
+			httpcache.SetCacheControlInHeader(ctx.Resp.Header(), 0, "no-transform")
 			ctx.Resp.Header().Set(`X-Frame-Options`, setting.CORSConfig.XFrameOptions)
 
 			ctx.Data["CsrfToken"] = ctx.csrf.GetToken()