From ebff0513dbe8abd2c9807789c006f41d002416b5 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 11 Sep 2023 18:14:01 +0800 Subject: Fix context cache bug & enable context cache for dashabord commits' authors (#26991) Unfortunately, when a system setting hasn't been stored in the database, it cannot be cached. Meanwhile, this PR also uses context cache for push email avatar display which should avoid to read user table via email address again and again. According to my local test, this should reduce dashboard elapsed time from 150ms -> 80ms . --- modules/repository/commits.go | 58 ++++++++++++-------------------------- modules/repository/commits_test.go | 8 ++---- 2 files changed, 21 insertions(+), 45 deletions(-) (limited to 'modules') diff --git a/modules/repository/commits.go b/modules/repository/commits.go index 96844d5b1d..ede60429a1 100644 --- a/modules/repository/commits.go +++ b/modules/repository/commits.go @@ -11,6 +11,7 @@ import ( "code.gitea.io/gitea/models/avatars" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" @@ -34,42 +35,36 @@ type PushCommits struct { HeadCommit *PushCommit CompareURL string Len int - - avatars map[string]string - emailUsers map[string]*user_model.User } // NewPushCommits creates a new PushCommits object. func NewPushCommits() *PushCommits { - return &PushCommits{ - avatars: make(map[string]string), - emailUsers: make(map[string]*user_model.User), - } + return &PushCommits{} } // toAPIPayloadCommit converts a single PushCommit to an api.PayloadCommit object. -func (pc *PushCommits) toAPIPayloadCommit(ctx context.Context, repoPath, repoLink string, commit *PushCommit) (*api.PayloadCommit, error) { +func (pc *PushCommits) toAPIPayloadCommit(ctx context.Context, emailUsers map[string]*user_model.User, repoPath, repoLink string, commit *PushCommit) (*api.PayloadCommit, error) { var err error authorUsername := "" - author, ok := pc.emailUsers[commit.AuthorEmail] + author, ok := emailUsers[commit.AuthorEmail] if !ok { author, err = user_model.GetUserByEmail(ctx, commit.AuthorEmail) if err == nil { authorUsername = author.Name - pc.emailUsers[commit.AuthorEmail] = author + emailUsers[commit.AuthorEmail] = author } } else { authorUsername = author.Name } committerUsername := "" - committer, ok := pc.emailUsers[commit.CommitterEmail] + committer, ok := emailUsers[commit.CommitterEmail] if !ok { committer, err = user_model.GetUserByEmail(ctx, commit.CommitterEmail) if err == nil { // TODO: check errors other than email not found. committerUsername = committer.Name - pc.emailUsers[commit.CommitterEmail] = committer + emailUsers[commit.CommitterEmail] = committer } } else { committerUsername = committer.Name @@ -107,11 +102,10 @@ func (pc *PushCommits) ToAPIPayloadCommits(ctx context.Context, repoPath, repoLi commits := make([]*api.PayloadCommit, len(pc.Commits)) var headCommit *api.PayloadCommit - if pc.emailUsers == nil { - pc.emailUsers = make(map[string]*user_model.User) - } + emailUsers := make(map[string]*user_model.User) + for i, commit := range pc.Commits { - apiCommit, err := pc.toAPIPayloadCommit(ctx, repoPath, repoLink, commit) + apiCommit, err := pc.toAPIPayloadCommit(ctx, emailUsers, repoPath, repoLink, commit) if err != nil { return nil, nil, err } @@ -123,7 +117,7 @@ func (pc *PushCommits) ToAPIPayloadCommits(ctx context.Context, repoPath, repoLi } if pc.HeadCommit != nil && headCommit == nil { var err error - headCommit, err = pc.toAPIPayloadCommit(ctx, repoPath, repoLink, pc.HeadCommit) + headCommit, err = pc.toAPIPayloadCommit(ctx, emailUsers, repoPath, repoLink, pc.HeadCommit) if err != nil { return nil, nil, err } @@ -134,35 +128,21 @@ func (pc *PushCommits) ToAPIPayloadCommits(ctx context.Context, repoPath, repoLi // AvatarLink tries to match user in database with e-mail // in order to show custom avatar, and falls back to general avatar link. func (pc *PushCommits) AvatarLink(ctx context.Context, email string) string { - if pc.avatars == nil { - pc.avatars = make(map[string]string) - } - avatar, ok := pc.avatars[email] - if ok { - return avatar - } - size := avatars.DefaultAvatarPixelSize * setting.Avatar.RenderedSizeFactor - u, ok := pc.emailUsers[email] - if !ok { - var err error - u, err = user_model.GetUserByEmail(ctx, email) + v, _ := cache.GetWithContextCache(ctx, "push_commits", email, func() (string, error) { + u, err := user_model.GetUserByEmail(ctx, email) if err != nil { - pc.avatars[email] = avatars.GenerateEmailAvatarFastLink(ctx, email, size) if !user_model.IsErrUserNotExist(err) { log.Error("GetUserByEmail: %v", err) - return "" + return "", err } - } else { - pc.emailUsers[email] = u + return avatars.GenerateEmailAvatarFastLink(ctx, email, size), nil } - } - if u != nil { - pc.avatars[email] = u.AvatarLinkWithSize(ctx, size) - } + return u.AvatarLinkWithSize(ctx, size), nil + }) - return pc.avatars[email] + return v } // CommitToPushCommit transforms a git.Commit to PushCommit type. @@ -189,7 +169,5 @@ func GitToPushCommits(gitCommits []*git.Commit) *PushCommits { HeadCommit: nil, CompareURL: "", Len: len(commits), - avatars: make(map[string]string), - emailUsers: make(map[string]*user_model.User), } } diff --git a/modules/repository/commits_test.go b/modules/repository/commits_test.go index b6ad967d4c..0931092597 100644 --- a/modules/repository/commits_test.go +++ b/modules/repository/commits_test.go @@ -103,11 +103,9 @@ func TestPushCommits_ToAPIPayloadCommits(t *testing.T) { assert.EqualValues(t, []string{"readme.md"}, headCommit.Modified) } -func enableGravatar(t *testing.T) { - err := system_model.SetSettingNoVersion(db.DefaultContext, system_model.KeyPictureDisableGravatar, "false") - assert.NoError(t, err) +func initGravatarSource(t *testing.T) { setting.GravatarSource = "https://secure.gravatar.com/avatar" - err = system_model.Init(db.DefaultContext) + err := system_model.Init(db.DefaultContext) assert.NoError(t, err) } @@ -134,7 +132,7 @@ func TestPushCommits_AvatarLink(t *testing.T) { }, } - enableGravatar(t) + initGravatarSource(t) assert.Equal(t, "https://secure.gravatar.com/avatar/ab53a2911ddf9b4817ac01ddcd3d975f?d=identicon&s="+strconv.Itoa(28*setting.Avatar.RenderedSizeFactor), -- cgit v1.2.3