Skip to content

Commit 62af2dc

Browse files
committed
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
2 parents 315aff0 + 52e2416 commit 62af2dc

File tree

81 files changed

+593
-230
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

81 files changed

+593
-230
lines changed

models/db/iterate_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ func TestIterate(t *testing.T) {
2525
return nil
2626
})
2727
assert.NoError(t, err)
28-
assert.EqualValues(t, 83, repoCnt)
28+
assert.EqualValues(t, 84, repoCnt)
2929

3030
err = db.Iterate(db.DefaultContext, nil, func(ctx context.Context, repoUnit *repo_model.RepoUnit) error {
3131
reopUnit2 := repo_model.RepoUnit{ID: repoUnit.ID}

models/db/list_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,11 @@ func TestFind(t *testing.T) {
3535
var repoUnits []repo_model.RepoUnit
3636
err := db.Find(db.DefaultContext, &opts, &repoUnits)
3737
assert.NoError(t, err)
38-
assert.EqualValues(t, 83, len(repoUnits))
38+
assert.EqualValues(t, 84, len(repoUnits))
3939

4040
cnt, err := db.Count(db.DefaultContext, &opts, new(repo_model.RepoUnit))
4141
assert.NoError(t, err)
42-
assert.EqualValues(t, 83, cnt)
42+
assert.EqualValues(t, 84, cnt)
4343

4444
repoUnits = make([]repo_model.RepoUnit, 0, 10)
4545
newCnt, err := db.FindAndCount(db.DefaultContext, &opts, &repoUnits)

models/fixtures/repo_unit.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -569,3 +569,9 @@
569569
type: 3
570570
config: "{\"IgnoreWhitespaceConflicts\":false,\"AllowMerge\":true,\"AllowRebase\":true,\"AllowRebaseMerge\":true,\"AllowSquash\":true}"
571571
created_unix: 946684810
572+
573+
-
574+
id: 84
575+
repo_id: 56
576+
type: 1
577+
created_unix: 946684810

models/fixtures/repository.yml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1634,3 +1634,16 @@
16341634
is_private: true
16351635
num_issues: 1
16361636
status: 0
1637+
1638+
-
1639+
id: 56
1640+
owner_id: 2
1641+
owner_name: user2
1642+
lower_name: readme-test
1643+
name: readme-test
1644+
default_branch: master
1645+
is_empty: false
1646+
is_archived: false
1647+
is_private: true
1648+
status: 0
1649+
num_issues: 0

models/fixtures/user.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@
6666
num_followers: 2
6767
num_following: 1
6868
num_stars: 2
69-
num_repos: 11
69+
num_repos: 12
7070
num_teams: 0
7171
num_members: 0
7272
visibility: 0

models/git/lfs_lock.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ package git
66
import (
77
"context"
88
"fmt"
9-
"path"
109
"strings"
1110
"time"
1211

@@ -17,6 +16,7 @@ import (
1716
"code.gitea.io/gitea/models/unit"
1817
user_model "code.gitea.io/gitea/models/user"
1918
"code.gitea.io/gitea/modules/setting"
19+
"code.gitea.io/gitea/modules/util"
2020
)
2121

2222
// LFSLock represents a git lfs lock of repository.
@@ -34,11 +34,7 @@ func init() {
3434

3535
// BeforeInsert is invoked from XORM before inserting an object of this type.
3636
func (l *LFSLock) BeforeInsert() {
37-
l.Path = cleanPath(l.Path)
38-
}
39-
40-
func cleanPath(p string) string {
41-
return path.Clean("/" + p)[1:]
37+
l.Path = util.CleanPath(l.Path)
4238
}
4339

4440
// CreateLFSLock creates a new lock.
@@ -53,7 +49,7 @@ func CreateLFSLock(ctx context.Context, repo *repo_model.Repository, lock *LFSLo
5349
return nil, err
5450
}
5551

56-
lock.Path = cleanPath(lock.Path)
52+
lock.Path = util.CleanPath(lock.Path)
5753
lock.RepoID = repo.ID
5854

5955
l, err := GetLFSLock(dbCtx, repo, lock.Path)
@@ -73,7 +69,7 @@ func CreateLFSLock(ctx context.Context, repo *repo_model.Repository, lock *LFSLo
7369

7470
// GetLFSLock returns release by given path.
7571
func GetLFSLock(ctx context.Context, repo *repo_model.Repository, path string) (*LFSLock, error) {
76-
path = cleanPath(path)
72+
path = util.CleanPath(path)
7773
rel := &LFSLock{RepoID: repo.ID}
7874
has, err := db.GetEngine(ctx).Where("lower(path) = ?", strings.ToLower(path)).Get(rel)
7975
if err != nil {

modules/cache/context.go

Lines changed: 103 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package cache
66
import (
77
"context"
88
"sync"
9+
"time"
910

1011
"code.gitea.io/gitea/modules/log"
1112
)
@@ -14,65 +15,151 @@ import (
1415
// This is useful for caching data that is expensive to calculate and is likely to be
1516
// used multiple times in a request.
1617
type cacheContext struct {
17-
ctx context.Context
18-
data map[any]map[any]any
19-
lock sync.RWMutex
18+
data map[any]map[any]any
19+
lock sync.RWMutex
20+
created time.Time
21+
discard bool
2022
}
2123

2224
func (cc *cacheContext) Get(tp, key any) any {
2325
cc.lock.RLock()
2426
defer cc.lock.RUnlock()
25-
if cc.data[tp] == nil {
26-
return nil
27-
}
2827
return cc.data[tp][key]
2928
}
3029

3130
func (cc *cacheContext) Put(tp, key, value any) {
3231
cc.lock.Lock()
3332
defer cc.lock.Unlock()
34-
if cc.data[tp] == nil {
35-
cc.data[tp] = make(map[any]any)
33+
34+
if cc.discard {
35+
return
36+
}
37+
38+
d := cc.data[tp]
39+
if d == nil {
40+
d = make(map[any]any)
41+
cc.data[tp] = d
3642
}
37-
cc.data[tp][key] = value
43+
d[key] = value
3844
}
3945

4046
func (cc *cacheContext) Delete(tp, key any) {
4147
cc.lock.Lock()
4248
defer cc.lock.Unlock()
43-
if cc.data[tp] == nil {
44-
return
45-
}
4649
delete(cc.data[tp], key)
4750
}
4851

52+
func (cc *cacheContext) Discard() {
53+
cc.lock.Lock()
54+
defer cc.lock.Unlock()
55+
cc.data = nil
56+
cc.discard = true
57+
}
58+
59+
func (cc *cacheContext) isDiscard() bool {
60+
cc.lock.RLock()
61+
defer cc.lock.RUnlock()
62+
return cc.discard
63+
}
64+
65+
// cacheContextLifetime is the max lifetime of cacheContext.
66+
// Since cacheContext is used to cache data in a request level context, 10s is enough.
67+
// If a cacheContext is used more than 10s, it's probably misuse.
68+
const cacheContextLifetime = 10 * time.Second
69+
70+
var timeNow = time.Now
71+
72+
func (cc *cacheContext) Expired() bool {
73+
return timeNow().Sub(cc.created) > cacheContextLifetime
74+
}
75+
4976
var cacheContextKey = struct{}{}
5077

78+
/*
79+
Since there are both WithCacheContext and WithNoCacheContext,
80+
it may be confusing when there is nesting.
81+
82+
Some cases to explain the design:
83+
84+
When:
85+
- A, B or C means a cache context.
86+
- A', B' or C' means a discard cache context.
87+
- ctx means context.Backgrand().
88+
- A(ctx) means a cache context with ctx as the parent context.
89+
- B(A(ctx)) means a cache context with A(ctx) as the parent context.
90+
- With is alias of WithCacheContext.
91+
- WithNo is alias of WithNoCacheContext.
92+
93+
So:
94+
- With(ctx) -> A(ctx)
95+
- With(With(ctx)) -> A(ctx), not B(A(ctx)), always reuse parent cache context if possible.
96+
- With(With(With(ctx))) -> A(ctx), not C(B(A(ctx))), ditto.
97+
- WithNo(ctx) -> ctx, not A'(ctx), don't create new cache context if we don't have to.
98+
- WithNo(With(ctx)) -> A'(ctx)
99+
- WithNo(WithNo(With(ctx))) -> A'(ctx), not B'(A'(ctx)), don't create new cache context if we don't have to.
100+
- With(WithNo(With(ctx))) -> B(A'(ctx)), not A(ctx), never reuse a discard cache context.
101+
- WithNo(With(WithNo(With(ctx)))) -> B'(A'(ctx))
102+
- With(WithNo(With(WithNo(With(ctx))))) -> C(B'(A'(ctx))), so there's always only one not-discard cache context.
103+
*/
104+
51105
func WithCacheContext(ctx context.Context) context.Context {
106+
if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok {
107+
if !c.isDiscard() {
108+
// reuse parent context
109+
return ctx
110+
}
111+
}
52112
return context.WithValue(ctx, cacheContextKey, &cacheContext{
53-
ctx: ctx,
54-
data: make(map[any]map[any]any),
113+
data: make(map[any]map[any]any),
114+
created: timeNow(),
55115
})
56116
}
57117

118+
func WithNoCacheContext(ctx context.Context) context.Context {
119+
if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok {
120+
// The caller want to run long-life tasks, but the parent context is a cache context.
121+
// So we should disable and clean the cache data, or it will be kept in memory for a long time.
122+
c.Discard()
123+
return ctx
124+
}
125+
126+
return ctx
127+
}
128+
58129
func GetContextData(ctx context.Context, tp, key any) any {
59130
if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok {
131+
if c.Expired() {
132+
// The warning means that the cache context is misused for long-life task,
133+
// it can be resolved with WithNoCacheContext(ctx).
134+
log.Warn("cache context is expired, may be misused for long-life tasks: %v", c)
135+
return nil
136+
}
60137
return c.Get(tp, key)
61138
}
62-
log.Warn("cannot get cache context when getting data: %v", ctx)
63139
return nil
64140
}
65141

66142
func SetContextData(ctx context.Context, tp, key, value any) {
67143
if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok {
144+
if c.Expired() {
145+
// The warning means that the cache context is misused for long-life task,
146+
// it can be resolved with WithNoCacheContext(ctx).
147+
log.Warn("cache context is expired, may be misused for long-life tasks: %v", c)
148+
return
149+
}
68150
c.Put(tp, key, value)
69151
return
70152
}
71-
log.Warn("cannot get cache context when setting data: %v", ctx)
72153
}
73154

74155
func RemoveContextData(ctx context.Context, tp, key any) {
75156
if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok {
157+
if c.Expired() {
158+
// The warning means that the cache context is misused for long-life task,
159+
// it can be resolved with WithNoCacheContext(ctx).
160+
log.Warn("cache context is expired, may be misused for long-life tasks: %v", c)
161+
return
162+
}
76163
c.Delete(tp, key)
77164
}
78165
}

modules/cache/context_test.go

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package cache
66
import (
77
"context"
88
"testing"
9+
"time"
910

1011
"github.com/stretchr/testify/assert"
1112
)
@@ -25,7 +26,7 @@ func TestWithCacheContext(t *testing.T) {
2526
assert.EqualValues(t, 1, v.(int))
2627

2728
RemoveContextData(ctx, field, "my_config1")
28-
RemoveContextData(ctx, field, "my_config2") // remove an non-exist key
29+
RemoveContextData(ctx, field, "my_config2") // remove a non-exist key
2930

3031
v = GetContextData(ctx, field, "my_config1")
3132
assert.Nil(t, v)
@@ -38,4 +39,40 @@ func TestWithCacheContext(t *testing.T) {
3839

3940
v = GetContextData(ctx, field, "my_config1")
4041
assert.EqualValues(t, 1, v)
42+
43+
now := timeNow
44+
defer func() {
45+
timeNow = now
46+
}()
47+
timeNow = func() time.Time {
48+
return now().Add(10 * time.Second)
49+
}
50+
v = GetContextData(ctx, field, "my_config1")
51+
assert.Nil(t, v)
52+
}
53+
54+
func TestWithNoCacheContext(t *testing.T) {
55+
ctx := context.Background()
56+
57+
const field = "system_setting"
58+
59+
v := GetContextData(ctx, field, "my_config1")
60+
assert.Nil(t, v)
61+
SetContextData(ctx, field, "my_config1", 1)
62+
v = GetContextData(ctx, field, "my_config1")
63+
assert.Nil(t, v) // still no cache
64+
65+
ctx = WithCacheContext(ctx)
66+
v = GetContextData(ctx, field, "my_config1")
67+
assert.Nil(t, v)
68+
SetContextData(ctx, field, "my_config1", 1)
69+
v = GetContextData(ctx, field, "my_config1")
70+
assert.NotNil(t, v)
71+
72+
ctx = WithNoCacheContext(ctx)
73+
v = GetContextData(ctx, field, "my_config1")
74+
assert.Nil(t, v)
75+
SetContextData(ctx, field, "my_config1", 1)
76+
v = GetContextData(ctx, field, "my_config1")
77+
assert.Nil(t, v) // still no cache
4178
}

modules/context/api.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ func APIContexter() func(http.Handler) http.Handler {
244244
}
245245
}
246246

247-
httpcache.AddCacheControlToHeader(ctx.Resp.Header(), 0, "no-transform")
247+
httpcache.SetCacheControlInHeader(ctx.Resp.Header(), 0, "no-transform")
248248
ctx.Resp.Header().Set(`X-Frame-Options`, setting.CORSConfig.XFrameOptions)
249249

250250
ctx.Data["Context"] = &ctx

modules/context/context.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ func (ctx *Context) SetServeHeaders(opts *ServeHeaderOptions) {
388388
if duration == 0 {
389389
duration = 5 * time.Minute
390390
}
391-
httpcache.AddCacheControlToHeader(header, duration)
391+
httpcache.SetCacheControlInHeader(header, duration)
392392

393393
if !opts.LastModified.IsZero() {
394394
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 {
753753
}
754754
}
755755

756-
httpcache.AddCacheControlToHeader(ctx.Resp.Header(), 0, "no-transform")
756+
httpcache.SetCacheControlInHeader(ctx.Resp.Header(), 0, "no-transform")
757757
ctx.Resp.Header().Set(`X-Frame-Options`, setting.CORSConfig.XFrameOptions)
758758

759759
ctx.Data["CsrfToken"] = ctx.csrf.GetToken()

0 commit comments

Comments
 (0)