Skip to content

Commit 045cf8b

Browse files
wxiaoguangwxiaoguang
committed
fix
1 parent d35d45d commit 045cf8b

File tree

4 files changed

+171
-207
lines changed

4 files changed

+171
-207
lines changed

‎models/packages/descriptor.go‎

Lines changed: 12 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
repo_model "code.gitea.io/gitea/models/repo"
1313
user_model "code.gitea.io/gitea/models/user"
14+
"code.gitea.io/gitea/modules/cache"
1415
"code.gitea.io/gitea/modules/json"
1516
"code.gitea.io/gitea/modules/packages/alpine"
1617
"code.gitea.io/gitea/modules/packages/arch"
@@ -102,26 +103,26 @@ func (pd *PackageDescriptor) CalculateBlobSize() int64 {
102103

103104
// GetPackageDescriptor gets the package description for a version
104105
func GetPackageDescriptor(ctx context.Context, pv *PackageVersion) (*PackageDescriptor, error) {
105-
return getPackageDescriptor(ctx, pv, newQueryCache())
106+
return getPackageDescriptor(ctx, pv, cache.NewEphemeralCache())
106107
}
107108

108-
func getPackageDescriptor(ctx context.Context, pv *PackageVersion, c *cache) (*PackageDescriptor, error) {
109-
p, err := c.QueryPackage(ctx, pv.PackageID)
109+
func getPackageDescriptor(ctx context.Context, pv *PackageVersion, c *cache.EphemeralCache) (*PackageDescriptor, error) {
110+
p, err := cache.GetWithEphemeralCache(ctx, c, "package", pv.PackageID, GetPackageByID)
110111
if err != nil {
111112
return nil, err
112113
}
113-
o, err := c.QueryUser(ctx, p.OwnerID)
114+
o, err := cache.GetWithEphemeralCache(ctx, c, "user", p.OwnerID, user_model.GetUserByID)
114115
if err != nil {
115116
return nil, err
116117
}
117118
var repository *repo_model.Repository
118119
if p.RepoID > 0 {
119-
repository, err = c.QueryRepository(ctx, p.RepoID)
120+
repository, err = cache.GetWithEphemeralCache(ctx, c, "repo", p.RepoID, repo_model.GetRepositoryByID)
120121
if err != nil && !repo_model.IsErrRepoNotExist(err) {
121122
return nil, err
122123
}
123124
}
124-
creator, err := c.QueryUser(ctx, pv.CreatorID)
125+
creator, err := cache.GetWithEphemeralCache(ctx, c, "user", pv.CreatorID, user_model.GetUserByID)
125126
if err != nil {
126127
if errors.Is(err, util.ErrNotExist) {
127128
creator = user_model.NewGhostUser()
@@ -229,11 +230,11 @@ func getPackageDescriptor(ctx context.Context, pv *PackageVersion, c *cache) (*P
229230

230231
// GetPackageFileDescriptor gets a package file descriptor for a package file
231232
func GetPackageFileDescriptor(ctx context.Context, pf *PackageFile) (*PackageFileDescriptor, error) {
232-
return getPackageFileDescriptor(ctx, pf, newQueryCache())
233+
return getPackageFileDescriptor(ctx, pf, cache.NewEphemeralCache())
233234
}
234235

235-
func getPackageFileDescriptor(ctx context.Context, pf *PackageFile, c *cache) (*PackageFileDescriptor, error) {
236-
pb, err := c.QueryBlob(ctx, pf.BlobID)
236+
func getPackageFileDescriptor(ctx context.Context, pf *PackageFile, c *cache.EphemeralCache) (*PackageFileDescriptor, error) {
237+
pb, err := cache.GetWithEphemeralCache(ctx, c, "package_file_blob", pf.BlobID, GetBlobByID)
237238
if err != nil {
238239
return nil, err
239240
}
@@ -263,10 +264,10 @@ func GetPackageFileDescriptors(ctx context.Context, pfs []*PackageFile) ([]*Pack
263264

264265
// GetPackageDescriptors gets the package descriptions for the versions
265266
func GetPackageDescriptors(ctx context.Context, pvs []*PackageVersion) ([]*PackageDescriptor, error) {
266-
return getPackageDescriptors(ctx, pvs, newQueryCache())
267+
return getPackageDescriptors(ctx, pvs, cache.NewEphemeralCache())
267268
}
268269

269-
func getPackageDescriptors(ctx context.Context, pvs []*PackageVersion, c *cache) ([]*PackageDescriptor, error) {
270+
func getPackageDescriptors(ctx context.Context, pvs []*PackageVersion, c *cache.EphemeralCache) ([]*PackageDescriptor, error) {
270271
pds := make([]*PackageDescriptor, 0, len(pvs))
271272
for _, pv := range pvs {
272273
pd, err := getPackageDescriptor(ctx, pv, c)
@@ -277,62 +278,3 @@ func getPackageDescriptors(ctx context.Context, pvs []*PackageVersion, c *cache)
277278
}
278279
return pds, nil
279280
}
280-
281-
type cache struct {
282-
Packages map[int64]*Package
283-
Users map[int64]*user_model.User
284-
Repositories map[int64]*repo_model.Repository
285-
Blobs map[int64]*PackageBlob
286-
}
287-
288-
func newQueryCache() *cache {
289-
return &cache{
290-
Packages: make(map[int64]*Package),
291-
Users: make(map[int64]*user_model.User),
292-
Repositories: map[int64]*repo_model.Repository{0: nil}, // 0 is an expected value
293-
Blobs: make(map[int64]*PackageBlob),
294-
}
295-
}
296-
297-
func (c *cache) QueryPackage(ctx context.Context, id int64) (*Package, error) {
298-
if p, found := c.Packages[id]; found {
299-
return p, nil
300-
}
301-
302-
p, err := GetPackageByID(ctx, id)
303-
c.Packages[id] = p
304-
return p, err
305-
}
306-
307-
func (c *cache) QueryUser(ctx context.Context, id int64) (*user_model.User, error) {
308-
if u, found := c.Users[id]; found {
309-
return u, nil
310-
}
311-
312-
u, err := user_model.GetUserByID(ctx, id)
313-
c.Users[id] = u
314-
return u, err
315-
}
316-
317-
func (c *cache) QueryRepository(ctx context.Context, id int64) (*repo_model.Repository, error) {
318-
if r, found := c.Repositories[id]; found {
319-
return r, nil
320-
}
321-
322-
r, err := repo_model.GetRepositoryByID(ctx, id)
323-
if err != nil && !repo_model.IsErrRepoNotExist(err) {
324-
err = nil
325-
}
326-
c.Repositories[id] = r
327-
return r, err
328-
}
329-
330-
func (c *cache) QueryBlob(ctx context.Context, id int64) (*PackageBlob, error) {
331-
if b, found := c.Blobs[id]; found {
332-
return b, nil
333-
}
334-
335-
b, err := GetBlobByID(ctx, id)
336-
c.Blobs[id] = b
337-
return b, err
338-
}

‎modules/cache/context.go‎

Lines changed: 27 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -5,75 +5,17 @@ package cache
55

66
import (
77
"context"
8-
"sync"
98
"time"
10-
11-
"code.gitea.io/gitea/modules/log"
129
)
1310

14-
// cacheContext is a context that can be used to cache data in a request level context
15-
// This is useful for caching data that is expensive to calculate and is likely to be
16-
// used multiple times in a request.
17-
type cacheContext struct {
18-
data map[any]map[any]any
19-
lock sync.RWMutex
20-
created time.Time
21-
discard bool
22-
}
23-
24-
func (cc *cacheContext) Get(tp, key any) any {
25-
cc.lock.RLock()
26-
defer cc.lock.RUnlock()
27-
return cc.data[tp][key]
28-
}
29-
30-
func (cc *cacheContext) Put(tp, key, value any) {
31-
cc.lock.Lock()
32-
defer cc.lock.Unlock()
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
42-
}
43-
d[key] = value
44-
}
45-
46-
func (cc *cacheContext) Delete(tp, key any) {
47-
cc.lock.Lock()
48-
defer cc.lock.Unlock()
49-
delete(cc.data[tp], key)
50-
}
51-
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, 5 minutes is enough.
67-
// If a cacheContext is used more than 5 minutes, it's probably misuse.
68-
const cacheContextLifetime = 5 * time.Minute
69-
70-
var timeNow = time.Now
11+
type cacheContextKeyType struct{}
7112

72-
func (cc *cacheContext) Expired() bool {
73-
return timeNow().Sub(cc.created) > cacheContextLifetime
74-
}
13+
var cacheContextKey = cacheContextKeyType{}
7514

76-
var cacheContextKey = struct{}{}
15+
// contextCacheLifetime is the max lifetime of context cache.
16+
// Since context cache is used to cache data in a request level context, 5 minutes is enough.
17+
// If a context cache is used more than 5 minutes, it's probably abused.
18+
const contextCacheLifetime = 5 * time.Minute
7719

7820
/*
7921
Since there are both WithCacheContext and WithNoCacheContext,
@@ -103,78 +45,52 @@ So:
10345
*/
10446

10547
func WithCacheContext(ctx context.Context) context.Context {
106-
if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok {
48+
if c, ok := ctx.Value(cacheContextKey).(*EphemeralCache); ok {
10749
if !c.isDiscard() {
108-
// reuse parent context
109-
return ctx
50+
return ctx // reuse parent context
11051
}
11152
}
112-
// FIXME: review the use of this nolint directive
113-
return context.WithValue(ctx, cacheContextKey, &cacheContext{ //nolint:staticcheck
114-
data: make(map[any]map[any]any),
115-
created: timeNow(),
116-
})
53+
return context.WithValue(ctx, cacheContextKey, NewEphemeralCache(contextCacheLifetime))
11754
}
11855

119-
func WithNoCacheContext(ctx context.Context) context.Context {
120-
if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok {
56+
func withNoCacheContext(ctx context.Context) context.Context {
57+
if c, ok := ctx.Value(cacheContextKey).(*EphemeralCache); ok {
12158
// The caller want to run long-life tasks, but the parent context is a cache context.
12259
// So we should disable and clean the cache data, or it will be kept in memory for a long time.
123-
c.Discard()
60+
c.discard()
12461
return ctx
12562
}
126-
12763
return ctx
12864
}
12965

130-
func GetContextData(ctx context.Context, tp, key any) any {
131-
if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok {
132-
if c.Expired() {
133-
// The warning means that the cache context is misused for long-life task,
134-
// it can be resolved with WithNoCacheContext(ctx).
135-
log.Warn("cache context is expired, is highly likely to be misused for long-life tasks: %v", c)
136-
return nil
137-
}
66+
func getContextData(ctx context.Context, tp, key any) (any, bool) {
67+
if c, ok := ctx.Value(cacheContextKey).(*EphemeralCache); ok {
13868
return c.Get(tp, key)
13969
}
140-
return nil
70+
return nil, false
14171
}
14272

143-
func SetContextData(ctx context.Context, tp, key, value any) {
144-
if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok {
145-
if c.Expired() {
146-
// The warning means that the cache context is misused for long-life task,
147-
// it can be resolved with WithNoCacheContext(ctx).
148-
log.Warn("cache context is expired, is highly likely to be misused for long-life tasks: %v", c)
149-
return
150-
}
73+
func setContextData(ctx context.Context, tp, key, value any) {
74+
if c, ok := ctx.Value(cacheContextKey).(*EphemeralCache); ok {
15175
c.Put(tp, key, value)
152-
return
15376
}
15477
}
15578

156-
func RemoveContextData(ctx context.Context, tp, key any) {
157-
if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok {
158-
if c.Expired() {
159-
// The warning means that the cache context is misused for long-life task,
160-
// it can be resolved with WithNoCacheContext(ctx).
161-
log.Warn("cache context is expired, is highly likely to be misused for long-life tasks: %v", c)
162-
return
163-
}
79+
func removeContextData(ctx context.Context, tp, key any) {
80+
if c, ok := ctx.Value(cacheContextKey).(*EphemeralCache); ok {
16481
c.Delete(tp, key)
16582
}
16683
}
16784

16885
// GetWithContextCache returns the cache value of the given key in the given context.
86+
// FIXME: in most cases, the "context cache" should not be used, because it has uncontrollable behaviors
87+
// For example, these calls:
88+
// * GetWithContextCache(TargetID) -> OtherCodeCreateModel(TargetID) -> GetWithContextCache(TargetID)
89+
// Will cause the second call is not able to get the correct created target.
90+
// UNLESS it is certain that the target won't be changed during the request, DO NOT use it.
16991
func GetWithContextCache[T, K any](ctx context.Context, groupKey string, targetKey K, f func(context.Context, K) (T, error)) (T, error) {
170-
v := GetContextData(ctx, groupKey, targetKey)
171-
if vv, ok := v.(T); ok {
172-
return vv, nil
173-
}
174-
t, err := f(ctx, targetKey)
175-
if err != nil {
176-
return t, err
92+
if c, ok := ctx.Value(cacheContextKey).(*EphemeralCache); ok {
93+
return GetWithEphemeralCache(ctx, c, groupKey, targetKey, f)
17794
}
178-
SetContextData(ctx, groupKey, targetKey, t)
179-
return t, nil
95+
return f(ctx, targetKey)
18096
}

0 commit comments

Comments
 (0)