[BUG] Store JSON in contributors commit cache
- The code that gets contributor stats tried to store an `map[string]*ContributorData` type in the cache, this works for the memory cache but not for other caches such as Redis. - The cache implementation for Redis would convert this map via `fmt.Sprintf` to an string, which would simply print the pointer and not the value of the pointer. Storing pointers is a no-go as this will get GC-ed eventually within a few minutes. Therefore store everything with json, that does properly store the value of the pointers. - Adds unit test that verifies JSON is being used. - Resolves https://codeberg.org/forgejo/forgejo/issues/3158 (cherry picked from commit 4c8b67c4b2f8a616b6e20304f7135934e7aab7e7)
This commit is contained in:
parent
b519ae39d7
commit
d60152587c
2 changed files with 22 additions and 4 deletions
|
@ -20,6 +20,7 @@ import (
|
||||||
"code.gitea.io/gitea/modules/git"
|
"code.gitea.io/gitea/modules/git"
|
||||||
"code.gitea.io/gitea/modules/gitrepo"
|
"code.gitea.io/gitea/modules/gitrepo"
|
||||||
"code.gitea.io/gitea/modules/graceful"
|
"code.gitea.io/gitea/modules/graceful"
|
||||||
|
"code.gitea.io/gitea/modules/json"
|
||||||
"code.gitea.io/gitea/modules/log"
|
"code.gitea.io/gitea/modules/log"
|
||||||
api "code.gitea.io/gitea/modules/structs"
|
api "code.gitea.io/gitea/modules/structs"
|
||||||
|
|
||||||
|
@ -108,8 +109,9 @@ func GetContributorStats(ctx context.Context, cache cache.Cache, repo *repo_mode
|
||||||
switch v := cache.Get(cacheKey).(type) {
|
switch v := cache.Get(cacheKey).(type) {
|
||||||
case error:
|
case error:
|
||||||
return nil, v
|
return nil, v
|
||||||
case map[string]*ContributorData:
|
case string:
|
||||||
return v, nil
|
var cachedStats map[string]*ContributorData
|
||||||
|
return cachedStats, json.Unmarshal([]byte(v), &cachedStats)
|
||||||
default:
|
default:
|
||||||
return nil, fmt.Errorf("unexpected type in cache detected")
|
return nil, fmt.Errorf("unexpected type in cache detected")
|
||||||
}
|
}
|
||||||
|
@ -309,7 +311,16 @@ func generateContributorStats(genDone chan struct{}, cache cache.Cache, cacheKey
|
||||||
total.TotalCommits++
|
total.TotalCommits++
|
||||||
}
|
}
|
||||||
|
|
||||||
_ = cache.Put(cacheKey, contributorsCommitStats, contributorStatsCacheTimeout)
|
data, err := json.Marshal(contributorsCommitStats)
|
||||||
|
if err != nil {
|
||||||
|
err := fmt.Errorf("couldn't marshal the data: %w", err)
|
||||||
|
_ = cache.Put(cacheKey, err, contributorStatsCacheTimeout)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
// Store the data as an string, to make it uniform what data type is returned
|
||||||
|
// from caches.
|
||||||
|
_ = cache.Put(cacheKey, string(data), contributorStatsCacheTimeout)
|
||||||
generateLock.Delete(cacheKey)
|
generateLock.Delete(cacheKey)
|
||||||
if genDone != nil {
|
if genDone != nil {
|
||||||
genDone <- struct{}{}
|
genDone <- struct{}{}
|
||||||
|
|
|
@ -11,6 +11,7 @@ import (
|
||||||
repo_model "code.gitea.io/gitea/models/repo"
|
repo_model "code.gitea.io/gitea/models/repo"
|
||||||
"code.gitea.io/gitea/models/unittest"
|
"code.gitea.io/gitea/models/unittest"
|
||||||
"code.gitea.io/gitea/modules/git"
|
"code.gitea.io/gitea/modules/git"
|
||||||
|
"code.gitea.io/gitea/modules/json"
|
||||||
|
|
||||||
"gitea.com/go-chi/cache"
|
"gitea.com/go-chi/cache"
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
|
@ -32,8 +33,14 @@ func TestRepository_ContributorsGraph(t *testing.T) {
|
||||||
assert.ErrorAs(t, err, &git.ErrNotExist{})
|
assert.ErrorAs(t, err, &git.ErrNotExist{})
|
||||||
|
|
||||||
generateContributorStats(nil, mockCache, "key2", repo, "master")
|
generateContributorStats(nil, mockCache, "key2", repo, "master")
|
||||||
data, isData := mockCache.Get("key2").(map[string]*ContributorData)
|
dataString, isData := mockCache.Get("key2").(string)
|
||||||
assert.True(t, isData)
|
assert.True(t, isData)
|
||||||
|
// Verify that JSON is actually stored in the cache.
|
||||||
|
assert.EqualValues(t, `{"ethantkoenig@gmail.com":{"name":"Ethan Koenig","login":"","avatar_link":"https://secure.gravatar.com/avatar/b42fb195faa8c61b8d88abfefe30e9e3?d=identicon","home_link":"","total_commits":1,"weeks":{"1511654400000":{"week":1511654400000,"additions":3,"deletions":0,"commits":1}}},"jimmy.praet@telenet.be":{"name":"Jimmy Praet","login":"","avatar_link":"https://secure.gravatar.com/avatar/93c49b7c89eb156971d11161c9b52795?d=identicon","home_link":"","total_commits":1,"weeks":{"1624752000000":{"week":1624752000000,"additions":2,"deletions":0,"commits":1}}},"jon@allspice.io":{"name":"Jon","login":"","avatar_link":"https://secure.gravatar.com/avatar/00388ce725e6886f3e07c3733007289b?d=identicon","home_link":"","total_commits":1,"weeks":{"1607817600000":{"week":1607817600000,"additions":10,"deletions":0,"commits":1}}},"total":{"name":"Total","login":"","avatar_link":"","home_link":"","total_commits":3,"weeks":{"1511654400000":{"week":1511654400000,"additions":3,"deletions":0,"commits":1},"1607817600000":{"week":1607817600000,"additions":10,"deletions":0,"commits":1},"1624752000000":{"week":1624752000000,"additions":2,"deletions":0,"commits":1}}}}`, dataString)
|
||||||
|
|
||||||
|
var data map[string]*ContributorData
|
||||||
|
assert.NoError(t, json.Unmarshal([]byte(dataString), &data))
|
||||||
|
|
||||||
var keys []string
|
var keys []string
|
||||||
for k := range data {
|
for k := range data {
|
||||||
keys = append(keys, k)
|
keys = append(keys, k)
|
||||||
|
|
Loading…
Reference in a new issue