From 094176e675634bdf89f88807a738426ed5bdce46 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 31 Mar 2025 21:11:23 -0700 Subject: [PATCH 1/7] Cache gpg keys fetch from database when list commits --- routers/private/hook_verification.go | 4 +- routers/web/repo/commit.go | 3 +- routers/web/repo/view.go | 3 +- services/asymkey/commit.go | 68 +++++++++++--------- services/asymkey/sign.go | 15 +++-- services/convert/convert.go | 3 +- services/git/commit.go | 4 +- services/repository/files/commit.go | 4 +- services/repository/gitgraph/graph_models.go | 3 +- 9 files changed, 65 insertions(+), 42 deletions(-) diff --git a/routers/private/hook_verification.go b/routers/private/hook_verification.go index 7c06cf855725b..405fae5035a91 100644 --- a/routers/private/hook_verification.go +++ b/routers/private/hook_verification.go @@ -10,6 +10,7 @@ import ( "io" "os" + asymkey_model "code.gitea.io/gitea/models/asymkey" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" asymkey_service "code.gitea.io/gitea/services/asymkey" @@ -84,6 +85,7 @@ func readAndVerifyCommit(sha string, repo *git.Repository, env []string) error { }() commitID := git.MustIDFromString(sha) + keysCache := make(map[string][]*asymkey_model.GPGKey) return git.NewCommand("cat-file", "commit").AddDynamicArguments(sha). Run(repo.Ctx, &git.RunOpts{ @@ -96,7 +98,7 @@ func readAndVerifyCommit(sha string, repo *git.Repository, env []string) error { if err != nil { return err } - verification := asymkey_service.ParseCommitWithSignature(ctx, commit) + verification := asymkey_service.ParseCommitWithSignature(ctx, commit, keysCache) if !verification.Verified { cancel() return &errUnverifiedCommit{ diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index 3fd1eacb581eb..4b5ac2b77aabd 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -384,7 +384,8 @@ func Diff(ctx *context.Context) { ctx.Data["CommitStatus"] = git_model.CalcCommitStatus(statuses) ctx.Data["CommitStatuses"] = statuses - verification := asymkey_service.ParseCommitWithSignature(ctx, commit) + keysCache := make(map[string][]*asymkey_model.GPGKey) + verification := asymkey_service.ParseCommitWithSignature(ctx, commit, keysCache) ctx.Data["Verification"] = verification ctx.Data["Author"] = user_model.ValidateCommitWithEmail(ctx, commit) ctx.Data["Parents"] = parents diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 6ed5801d10fa7..8523a1d796de9 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -119,7 +119,8 @@ func loadLatestCommitData(ctx *context.Context, latestCommit *git.Commit) bool { // or of directory if not in root directory. ctx.Data["LatestCommit"] = latestCommit if latestCommit != nil { - verification := asymkey_service.ParseCommitWithSignature(ctx, latestCommit) + keysCache := make(map[string][]*asymkey_model.GPGKey) + verification := asymkey_service.ParseCommitWithSignature(ctx, latestCommit, keysCache) if err := asymkey_model.CalculateTrustStatus(verification, ctx.Repo.Repository.GetTrustModel(), func(user *user_model.User) (bool, error) { return repo_model.IsOwnerMemberCollaborator(ctx, ctx.Repo.Repository, user.ID) diff --git a/services/asymkey/commit.go b/services/asymkey/commit.go index df29133972156..4eaba73ff083d 100644 --- a/services/asymkey/commit.go +++ b/services/asymkey/commit.go @@ -18,7 +18,7 @@ import ( ) // ParseCommitWithSignature check if signature is good against keystore. -func ParseCommitWithSignature(ctx context.Context, c *git.Commit) *asymkey_model.CommitVerification { +func ParseCommitWithSignature(ctx context.Context, c *git.Commit, keysCache map[string][]*asymkey_model.GPGKey) *asymkey_model.CommitVerification { var committer *user_model.User if c.Committer != nil { var err error @@ -42,10 +42,10 @@ func ParseCommitWithSignature(ctx context.Context, c *git.Commit) *asymkey_model } } - return ParseCommitWithSignatureCommitter(ctx, c, committer) + return ParseCommitWithSignatureCommitter(ctx, c, committer, keysCache) } -func ParseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, committer *user_model.User) *asymkey_model.CommitVerification { +func ParseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, committer *user_model.User, keysCache map[string][]*asymkey_model.GPGKey) *asymkey_model.CommitVerification { // If no signature just report the committer if c.Signature == nil { return &asymkey_model.CommitVerification{ @@ -82,7 +82,8 @@ func ParseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, commi committer, keyID, setting.AppName, - ""); commitVerification != nil { + "", + keysCache); commitVerification != nil { if commitVerification.Reason == asymkey_model.BadSignature { defaultReason = asymkey_model.BadSignature } else { @@ -160,7 +161,7 @@ func ParseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, commi } if err := gpgSettings.LoadPublicKeyContent(); err != nil { log.Error("Error getting default signing key: %s %v", gpgSettings.KeyID, err) - } else if commitVerification := VerifyWithGPGSettings(ctx, &gpgSettings, sig, c.Signature.Payload, committer, keyID); commitVerification != nil { + } else if commitVerification := VerifyWithGPGSettings(ctx, &gpgSettings, sig, c.Signature.Payload, committer, keyID, keysCache); commitVerification != nil { if commitVerification.Reason == asymkey_model.BadSignature { defaultReason = asymkey_model.BadSignature } else { @@ -175,7 +176,7 @@ func ParseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, commi } else if defaultGPGSettings == nil { log.Warn("Unable to get defaultGPGSettings for unattached commit: %s", c.ID.String()) } else if defaultGPGSettings.Sign { - if commitVerification := VerifyWithGPGSettings(ctx, defaultGPGSettings, sig, c.Signature.Payload, committer, keyID); commitVerification != nil { + if commitVerification := VerifyWithGPGSettings(ctx, defaultGPGSettings, sig, c.Signature.Payload, committer, keyID, keysCache); commitVerification != nil { if commitVerification.Reason == asymkey_model.BadSignature { defaultReason = asymkey_model.BadSignature } else { @@ -225,21 +226,26 @@ func checkKeyEmails(ctx context.Context, email string, keys ...*asymkey_model.GP return false, email } -func HashAndVerifyForKeyID(ctx context.Context, sig *packet.Signature, payload string, committer *user_model.User, keyID, name, email string) *asymkey_model.CommitVerification { +func HashAndVerifyForKeyID(ctx context.Context, sig *packet.Signature, payload string, committer *user_model.User, keyID, name, email string, keysCache map[string][]*asymkey_model.GPGKey) *asymkey_model.CommitVerification { if keyID == "" { return nil } - keys, err := db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{ - KeyID: keyID, - IncludeSubKeys: true, - }) - if err != nil { - log.Error("GetGPGKeysByKeyID: %v", err) - return &asymkey_model.CommitVerification{ - CommittingUser: committer, - Verified: false, - Reason: "gpg.error.failed_retrieval_gpg_keys", + var err error + keys, ok := keysCache[keyID] + if !ok { + keys, err = db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{ + KeyID: keyID, + IncludeSubKeys: true, + }) + if err != nil { + log.Error("GetGPGKeysByKeyID: %v", err) + return &asymkey_model.CommitVerification{ + CommittingUser: committer, + Verified: false, + Reason: "gpg.error.failed_retrieval_gpg_keys", + } } + keysCache[keyID] = keys } if len(keys) == 0 { return nil @@ -247,17 +253,21 @@ func HashAndVerifyForKeyID(ctx context.Context, sig *packet.Signature, payload s for _, key := range keys { var primaryKeys []*asymkey_model.GPGKey if key.PrimaryKeyID != "" { - primaryKeys, err = db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{ - KeyID: key.PrimaryKeyID, - IncludeSubKeys: true, - }) - if err != nil { - log.Error("GetGPGKeysByKeyID: %v", err) - return &asymkey_model.CommitVerification{ - CommittingUser: committer, - Verified: false, - Reason: "gpg.error.failed_retrieval_gpg_keys", + primaryKeys, ok = keysCache[keyID] + if !ok { + primaryKeys, err = db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{ + KeyID: key.PrimaryKeyID, + IncludeSubKeys: true, + }) + if err != nil { + log.Error("GetGPGKeysByKeyID: %v", err) + return &asymkey_model.CommitVerification{ + CommittingUser: committer, + Verified: false, + Reason: "gpg.error.failed_retrieval_gpg_keys", + } } + keysCache[keyID] = primaryKeys } } @@ -297,9 +307,9 @@ func HashAndVerifyForKeyID(ctx context.Context, sig *packet.Signature, payload s } } -func VerifyWithGPGSettings(ctx context.Context, gpgSettings *git.GPGSettings, sig *packet.Signature, payload string, committer *user_model.User, keyID string) *asymkey_model.CommitVerification { +func VerifyWithGPGSettings(ctx context.Context, gpgSettings *git.GPGSettings, sig *packet.Signature, payload string, committer *user_model.User, keyID string, keysCache map[string][]*asymkey_model.GPGKey) *asymkey_model.CommitVerification { // First try to find the key in the db - if commitVerification := HashAndVerifyForKeyID(ctx, sig, payload, committer, gpgSettings.KeyID, gpgSettings.Name, gpgSettings.Email); commitVerification != nil { + if commitVerification := HashAndVerifyForKeyID(ctx, sig, payload, committer, gpgSettings.KeyID, gpgSettings.Name, gpgSettings.Email, keysCache); commitVerification != nil { return commitVerification } diff --git a/services/asymkey/sign.go b/services/asymkey/sign.go index 2216bca54ae67..eb65ca59c32af 100644 --- a/services/asymkey/sign.go +++ b/services/asymkey/sign.go @@ -176,6 +176,7 @@ func SignWikiCommit(ctx context.Context, repo *repo_model.Repository, u *user_mo if signingKey == "" { return false, "", nil, &ErrWontSign{noKey} } + keysCache := make(map[string][]*asymkey_model.GPGKey) Loop: for _, rule := range rules { @@ -216,7 +217,7 @@ Loop: if commit.Signature == nil { return false, "", nil, &ErrWontSign{parentSigned} } - verification := ParseCommitWithSignature(ctx, commit) + verification := ParseCommitWithSignature(ctx, commit, keysCache) if !verification.Verified { return false, "", nil, &ErrWontSign{parentSigned} } @@ -232,6 +233,7 @@ func SignCRUDAction(ctx context.Context, repoPath string, u *user_model.User, tm if signingKey == "" { return false, "", nil, &ErrWontSign{noKey} } + keysCache := make(map[string][]*asymkey_model.GPGKey) Loop: for _, rule := range rules { @@ -272,7 +274,7 @@ Loop: if commit.Signature == nil { return false, "", nil, &ErrWontSign{parentSigned} } - verification := ParseCommitWithSignature(ctx, commit) + verification := ParseCommitWithSignature(ctx, commit, keysCache) if !verification.Verified { return false, "", nil, &ErrWontSign{parentSigned} } @@ -297,6 +299,7 @@ func SignMerge(ctx context.Context, pr *issues_model.PullRequest, u *user_model. var gitRepo *git.Repository var err error + keysCache := make(map[string][]*asymkey_model.GPGKey) Loop: for _, rule := range rules { @@ -347,7 +350,7 @@ Loop: if err != nil { return false, "", nil, err } - verification := ParseCommitWithSignature(ctx, commit) + verification := ParseCommitWithSignature(ctx, commit, keysCache) if !verification.Verified { return false, "", nil, &ErrWontSign{baseSigned} } @@ -363,7 +366,7 @@ Loop: if err != nil { return false, "", nil, err } - verification := ParseCommitWithSignature(ctx, commit) + verification := ParseCommitWithSignature(ctx, commit, keysCache) if !verification.Verified { return false, "", nil, &ErrWontSign{headSigned} } @@ -379,7 +382,7 @@ Loop: if err != nil { return false, "", nil, err } - verification := ParseCommitWithSignature(ctx, commit) + verification := ParseCommitWithSignature(ctx, commit, keysCache) if !verification.Verified { return false, "", nil, &ErrWontSign{commitsSigned} } @@ -393,7 +396,7 @@ Loop: return false, "", nil, err } for _, commit := range commitList { - verification := ParseCommitWithSignature(ctx, commit) + verification := ParseCommitWithSignature(ctx, commit, keysCache) if !verification.Verified { return false, "", nil, &ErrWontSign{commitsSigned} } diff --git a/services/convert/convert.go b/services/convert/convert.go index ac2680766c040..cd6665a6195cf 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -254,7 +254,8 @@ func ToActionArtifact(repo *repo_model.Repository, art *actions_model.ActionArti // ToVerification convert a git.Commit.Signature to an api.PayloadCommitVerification func ToVerification(ctx context.Context, c *git.Commit) *api.PayloadCommitVerification { - verif := asymkey_service.ParseCommitWithSignature(ctx, c) + keysCache := make(map[string][]*asymkey_model.GPGKey) + verif := asymkey_service.ParseCommitWithSignature(ctx, c, keysCache) commitVerification := &api.PayloadCommitVerification{ Verified: verif.Verified, Reason: verif.Reason, diff --git a/services/git/commit.go b/services/git/commit.go index 8ab8f3d369007..27c6b3b9a888f 100644 --- a/services/git/commit.go +++ b/services/git/commit.go @@ -33,6 +33,8 @@ func ParseCommitsWithSignature(ctx context.Context, oldCommits []*user_model.Use return nil, err } + keysCache := make(map[string][]*asymkey_model.GPGKey) + for _, c := range oldCommits { committer, ok := emailUsers[c.Committer.Email] if !ok && c.Committer != nil { @@ -44,7 +46,7 @@ func ParseCommitsWithSignature(ctx context.Context, oldCommits []*user_model.Use signCommit := &asymkey_model.SignCommit{ UserCommit: c, - Verification: asymkey_service.ParseCommitWithSignatureCommitter(ctx, c.Commit, committer), + Verification: asymkey_service.ParseCommitWithSignatureCommitter(ctx, c.Commit, committer, keysCache), } _ = asymkey_model.CalculateTrustStatus(signCommit.Verification, repoTrustModel, isOwnerMemberCollaborator, &keyMap) diff --git a/services/repository/files/commit.go b/services/repository/files/commit.go index 3cc326d065b16..cefd579516647 100644 --- a/services/repository/files/commit.go +++ b/services/repository/files/commit.go @@ -6,6 +6,7 @@ package files import ( "context" + asymkey_model "code.gitea.io/gitea/models/asymkey" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/structs" @@ -24,7 +25,8 @@ func CountDivergingCommits(ctx context.Context, repo *repo_model.Repository, bra // GetPayloadCommitVerification returns the verification information of a commit func GetPayloadCommitVerification(ctx context.Context, commit *git.Commit) *structs.PayloadCommitVerification { verification := &structs.PayloadCommitVerification{} - commitVerification := asymkey_service.ParseCommitWithSignature(ctx, commit) + keysCache := make(map[string][]*asymkey_model.GPGKey) + commitVerification := asymkey_service.ParseCommitWithSignature(ctx, commit, keysCache) if commit.Signature != nil { verification.Signature = commit.Signature.Signature verification.Payload = commit.Signature.Payload diff --git a/services/repository/gitgraph/graph_models.go b/services/repository/gitgraph/graph_models.go index c45662836bcc3..81c6f8bb29cdb 100644 --- a/services/repository/gitgraph/graph_models.go +++ b/services/repository/gitgraph/graph_models.go @@ -97,6 +97,7 @@ func (graph *Graph) LoadAndProcessCommits(ctx context.Context, repository *repo_ emails := map[string]*user_model.User{} keyMap := map[string]bool{} + keysCache := make(map[string][]*asymkey_model.GPGKey) for _, c := range graph.Commits { if len(c.Rev) == 0 { @@ -115,7 +116,7 @@ func (graph *Graph) LoadAndProcessCommits(ctx context.Context, repository *repo_ } } - c.Verification = asymkey_service.ParseCommitWithSignature(ctx, c.Commit) + c.Verification = asymkey_service.ParseCommitWithSignature(ctx, c.Commit, keysCache) _ = asymkey_model.CalculateTrustStatus(c.Verification, repository.GetTrustModel(), func(user *user_model.User) (bool, error) { return repo_model.IsOwnerMemberCollaborator(ctx, repository, user.ID) From 23c0948f36f4380b113f0a4a70e05ff7ad803abd Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 31 Mar 2025 23:12:57 -0700 Subject: [PATCH 2/7] Use CacheContext to do cache --- routers/private/hook_verification.go | 6 +- routers/web/repo/commit.go | 4 +- routers/web/repo/view.go | 4 +- services/asymkey/commit.go | 69 ++++++++++---------- services/asymkey/sign.go | 20 +++--- services/convert/convert.go | 4 +- services/git/commit.go | 5 +- services/repository/files/commit.go | 5 +- services/repository/gitgraph/graph_models.go | 5 +- 9 files changed, 63 insertions(+), 59 deletions(-) diff --git a/routers/private/hook_verification.go b/routers/private/hook_verification.go index 405fae5035a91..4e050869d4925 100644 --- a/routers/private/hook_verification.go +++ b/routers/private/hook_verification.go @@ -10,7 +10,7 @@ import ( "io" "os" - asymkey_model "code.gitea.io/gitea/models/asymkey" + "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" asymkey_service "code.gitea.io/gitea/services/asymkey" @@ -85,7 +85,6 @@ func readAndVerifyCommit(sha string, repo *git.Repository, env []string) error { }() commitID := git.MustIDFromString(sha) - keysCache := make(map[string][]*asymkey_model.GPGKey) return git.NewCommand("cat-file", "commit").AddDynamicArguments(sha). Run(repo.Ctx, &git.RunOpts{ @@ -98,7 +97,8 @@ func readAndVerifyCommit(sha string, repo *git.Repository, env []string) error { if err != nil { return err } - verification := asymkey_service.ParseCommitWithSignature(ctx, commit, keysCache) + ctx = cache.WithCacheContext(ctx) + verification := asymkey_service.ParseCommitWithSignature(ctx, commit) if !verification.Verified { cancel() return &errUnverifiedCommit{ diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index 4b5ac2b77aabd..699794885441f 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -20,6 +20,7 @@ import ( unit_model "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/charset" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" @@ -384,8 +385,7 @@ func Diff(ctx *context.Context) { ctx.Data["CommitStatus"] = git_model.CalcCommitStatus(statuses) ctx.Data["CommitStatuses"] = statuses - keysCache := make(map[string][]*asymkey_model.GPGKey) - verification := asymkey_service.ParseCommitWithSignature(ctx, commit, keysCache) + verification := asymkey_service.ParseCommitWithSignature(cache.WithCacheContext(ctx), commit) ctx.Data["Verification"] = verification ctx.Data["Author"] = user_model.ValidateCommitWithEmail(ctx, commit) ctx.Data["Parents"] = parents diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 8523a1d796de9..b48dd765bb6c4 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -28,6 +28,7 @@ import ( unit_model "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/charset" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/lfs" @@ -119,8 +120,7 @@ func loadLatestCommitData(ctx *context.Context, latestCommit *git.Commit) bool { // or of directory if not in root directory. ctx.Data["LatestCommit"] = latestCommit if latestCommit != nil { - keysCache := make(map[string][]*asymkey_model.GPGKey) - verification := asymkey_service.ParseCommitWithSignature(ctx, latestCommit, keysCache) + verification := asymkey_service.ParseCommitWithSignature(cache.WithCacheContext(ctx), latestCommit) if err := asymkey_model.CalculateTrustStatus(verification, ctx.Repo.Repository.GetTrustModel(), func(user *user_model.User) (bool, error) { return repo_model.IsOwnerMemberCollaborator(ctx, ctx.Repo.Repository, user.ID) diff --git a/services/asymkey/commit.go b/services/asymkey/commit.go index 4eaba73ff083d..e4439cd346226 100644 --- a/services/asymkey/commit.go +++ b/services/asymkey/commit.go @@ -10,6 +10,7 @@ import ( asymkey_model "code.gitea.io/gitea/models/asymkey" "code.gitea.io/gitea/models/db" 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" @@ -18,7 +19,7 @@ import ( ) // ParseCommitWithSignature check if signature is good against keystore. -func ParseCommitWithSignature(ctx context.Context, c *git.Commit, keysCache map[string][]*asymkey_model.GPGKey) *asymkey_model.CommitVerification { +func ParseCommitWithSignature(ctx context.Context, c *git.Commit) *asymkey_model.CommitVerification { var committer *user_model.User if c.Committer != nil { var err error @@ -42,10 +43,12 @@ func ParseCommitWithSignature(ctx context.Context, c *git.Commit, keysCache map[ } } - return ParseCommitWithSignatureCommitter(ctx, c, committer, keysCache) + return ParseCommitWithSignatureCommitter(ctx, c, committer) } -func ParseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, committer *user_model.User, keysCache map[string][]*asymkey_model.GPGKey) *asymkey_model.CommitVerification { +const cacheUserEmailAddressKey = "gpg_user_email_address" + +func ParseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, committer *user_model.User) *asymkey_model.CommitVerification { // If no signature just report the committer if c.Signature == nil { return &asymkey_model.CommitVerification{ @@ -82,8 +85,7 @@ func ParseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, commi committer, keyID, setting.AppName, - "", - keysCache); commitVerification != nil { + ""); commitVerification != nil { if commitVerification.Reason == asymkey_model.BadSignature { defaultReason = asymkey_model.BadSignature } else { @@ -114,7 +116,9 @@ func ParseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, commi } } - committerEmailAddresses, _ := user_model.GetEmailAddresses(ctx, committer.ID) + committerEmailAddresses, _ := cache.GetWithContextCache(ctx, cacheUserEmailAddressKey, committer.ID, func() ([]*user_model.EmailAddress, error) { + return user_model.GetEmailAddresses(ctx, committer.ID) + }) activated := false for _, e := range committerEmailAddresses { if e.IsActivated && strings.EqualFold(e.Email, c.Committer.Email) { @@ -161,7 +165,7 @@ func ParseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, commi } if err := gpgSettings.LoadPublicKeyContent(); err != nil { log.Error("Error getting default signing key: %s %v", gpgSettings.KeyID, err) - } else if commitVerification := VerifyWithGPGSettings(ctx, &gpgSettings, sig, c.Signature.Payload, committer, keyID, keysCache); commitVerification != nil { + } else if commitVerification := VerifyWithGPGSettings(ctx, &gpgSettings, sig, c.Signature.Payload, committer, keyID); commitVerification != nil { if commitVerification.Reason == asymkey_model.BadSignature { defaultReason = asymkey_model.BadSignature } else { @@ -176,7 +180,7 @@ func ParseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, commi } else if defaultGPGSettings == nil { log.Warn("Unable to get defaultGPGSettings for unattached commit: %s", c.ID.String()) } else if defaultGPGSettings.Sign { - if commitVerification := VerifyWithGPGSettings(ctx, defaultGPGSettings, sig, c.Signature.Payload, committer, keyID, keysCache); commitVerification != nil { + if commitVerification := VerifyWithGPGSettings(ctx, defaultGPGSettings, sig, c.Signature.Payload, committer, keyID); commitVerification != nil { if commitVerification.Reason == asymkey_model.BadSignature { defaultReason = asymkey_model.BadSignature } else { @@ -226,26 +230,25 @@ func checkKeyEmails(ctx context.Context, email string, keys ...*asymkey_model.GP return false, email } -func HashAndVerifyForKeyID(ctx context.Context, sig *packet.Signature, payload string, committer *user_model.User, keyID, name, email string, keysCache map[string][]*asymkey_model.GPGKey) *asymkey_model.CommitVerification { +const cacheGroupKey = "gpg_key_id" + +func HashAndVerifyForKeyID(ctx context.Context, sig *packet.Signature, payload string, committer *user_model.User, keyID, name, email string) *asymkey_model.CommitVerification { if keyID == "" { return nil } - var err error - keys, ok := keysCache[keyID] - if !ok { - keys, err = db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{ + keys, err := cache.GetWithContextCache(ctx, cacheGroupKey, keyID, func() ([]*asymkey_model.GPGKey, error) { + return db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{ KeyID: keyID, IncludeSubKeys: true, }) - if err != nil { - log.Error("GetGPGKeysByKeyID: %v", err) - return &asymkey_model.CommitVerification{ - CommittingUser: committer, - Verified: false, - Reason: "gpg.error.failed_retrieval_gpg_keys", - } + }) + if err != nil { + log.Error("GetGPGKeysByKeyID: %v", err) + return &asymkey_model.CommitVerification{ + CommittingUser: committer, + Verified: false, + Reason: "gpg.error.failed_retrieval_gpg_keys", } - keysCache[keyID] = keys } if len(keys) == 0 { return nil @@ -253,21 +256,19 @@ func HashAndVerifyForKeyID(ctx context.Context, sig *packet.Signature, payload s for _, key := range keys { var primaryKeys []*asymkey_model.GPGKey if key.PrimaryKeyID != "" { - primaryKeys, ok = keysCache[keyID] - if !ok { - primaryKeys, err = db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{ + primaryKeys, err = cache.GetWithContextCache(ctx, cacheGroupKey, key.PrimaryKeyID, func() ([]*asymkey_model.GPGKey, error) { + return db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{ KeyID: key.PrimaryKeyID, IncludeSubKeys: true, }) - if err != nil { - log.Error("GetGPGKeysByKeyID: %v", err) - return &asymkey_model.CommitVerification{ - CommittingUser: committer, - Verified: false, - Reason: "gpg.error.failed_retrieval_gpg_keys", - } + }) + if err != nil { + log.Error("GetGPGKeysByKeyID: %v", err) + return &asymkey_model.CommitVerification{ + CommittingUser: committer, + Verified: false, + Reason: "gpg.error.failed_retrieval_gpg_keys", } - keysCache[keyID] = primaryKeys } } @@ -307,9 +308,9 @@ func HashAndVerifyForKeyID(ctx context.Context, sig *packet.Signature, payload s } } -func VerifyWithGPGSettings(ctx context.Context, gpgSettings *git.GPGSettings, sig *packet.Signature, payload string, committer *user_model.User, keyID string, keysCache map[string][]*asymkey_model.GPGKey) *asymkey_model.CommitVerification { +func VerifyWithGPGSettings(ctx context.Context, gpgSettings *git.GPGSettings, sig *packet.Signature, payload string, committer *user_model.User, keyID string) *asymkey_model.CommitVerification { // First try to find the key in the db - if commitVerification := HashAndVerifyForKeyID(ctx, sig, payload, committer, gpgSettings.KeyID, gpgSettings.Name, gpgSettings.Email, keysCache); commitVerification != nil { + if commitVerification := HashAndVerifyForKeyID(ctx, sig, payload, committer, gpgSettings.KeyID, gpgSettings.Name, gpgSettings.Email); commitVerification != nil { return commitVerification } diff --git a/services/asymkey/sign.go b/services/asymkey/sign.go index eb65ca59c32af..89e3cae0418b8 100644 --- a/services/asymkey/sign.go +++ b/services/asymkey/sign.go @@ -15,6 +15,7 @@ import ( issues_model "code.gitea.io/gitea/models/issues" repo_model "code.gitea.io/gitea/models/repo" 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/gitrepo" "code.gitea.io/gitea/modules/log" @@ -176,7 +177,8 @@ func SignWikiCommit(ctx context.Context, repo *repo_model.Repository, u *user_mo if signingKey == "" { return false, "", nil, &ErrWontSign{noKey} } - keysCache := make(map[string][]*asymkey_model.GPGKey) + + ctx = cache.WithCacheContext(ctx) Loop: for _, rule := range rules { @@ -217,7 +219,7 @@ Loop: if commit.Signature == nil { return false, "", nil, &ErrWontSign{parentSigned} } - verification := ParseCommitWithSignature(ctx, commit, keysCache) + verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { return false, "", nil, &ErrWontSign{parentSigned} } @@ -233,7 +235,7 @@ func SignCRUDAction(ctx context.Context, repoPath string, u *user_model.User, tm if signingKey == "" { return false, "", nil, &ErrWontSign{noKey} } - keysCache := make(map[string][]*asymkey_model.GPGKey) + ctx = cache.WithCacheContext(ctx) Loop: for _, rule := range rules { @@ -274,7 +276,7 @@ Loop: if commit.Signature == nil { return false, "", nil, &ErrWontSign{parentSigned} } - verification := ParseCommitWithSignature(ctx, commit, keysCache) + verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { return false, "", nil, &ErrWontSign{parentSigned} } @@ -299,7 +301,7 @@ func SignMerge(ctx context.Context, pr *issues_model.PullRequest, u *user_model. var gitRepo *git.Repository var err error - keysCache := make(map[string][]*asymkey_model.GPGKey) + ctx = cache.WithCacheContext(ctx) Loop: for _, rule := range rules { @@ -350,7 +352,7 @@ Loop: if err != nil { return false, "", nil, err } - verification := ParseCommitWithSignature(ctx, commit, keysCache) + verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { return false, "", nil, &ErrWontSign{baseSigned} } @@ -366,7 +368,7 @@ Loop: if err != nil { return false, "", nil, err } - verification := ParseCommitWithSignature(ctx, commit, keysCache) + verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { return false, "", nil, &ErrWontSign{headSigned} } @@ -382,7 +384,7 @@ Loop: if err != nil { return false, "", nil, err } - verification := ParseCommitWithSignature(ctx, commit, keysCache) + verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { return false, "", nil, &ErrWontSign{commitsSigned} } @@ -396,7 +398,7 @@ Loop: return false, "", nil, err } for _, commit := range commitList { - verification := ParseCommitWithSignature(ctx, commit, keysCache) + verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { return false, "", nil, &ErrWontSign{commitsSigned} } diff --git a/services/convert/convert.go b/services/convert/convert.go index cd6665a6195cf..8341a9431c13e 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -22,6 +22,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" @@ -254,8 +255,7 @@ func ToActionArtifact(repo *repo_model.Repository, art *actions_model.ActionArti // ToVerification convert a git.Commit.Signature to an api.PayloadCommitVerification func ToVerification(ctx context.Context, c *git.Commit) *api.PayloadCommitVerification { - keysCache := make(map[string][]*asymkey_model.GPGKey) - verif := asymkey_service.ParseCommitWithSignature(ctx, c, keysCache) + verif := asymkey_service.ParseCommitWithSignature(cache.WithCacheContext(ctx), c) commitVerification := &api.PayloadCommitVerification{ Verified: verif.Verified, Reason: verif.Reason, diff --git a/services/git/commit.go b/services/git/commit.go index 27c6b3b9a888f..fb64afc00172d 100644 --- a/services/git/commit.go +++ b/services/git/commit.go @@ -11,6 +11,7 @@ import ( git_model "code.gitea.io/gitea/models/git" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/git" asymkey_service "code.gitea.io/gitea/services/asymkey" @@ -33,7 +34,7 @@ func ParseCommitsWithSignature(ctx context.Context, oldCommits []*user_model.Use return nil, err } - keysCache := make(map[string][]*asymkey_model.GPGKey) + ctx = cache.WithCacheContext(ctx) for _, c := range oldCommits { committer, ok := emailUsers[c.Committer.Email] @@ -46,7 +47,7 @@ func ParseCommitsWithSignature(ctx context.Context, oldCommits []*user_model.Use signCommit := &asymkey_model.SignCommit{ UserCommit: c, - Verification: asymkey_service.ParseCommitWithSignatureCommitter(ctx, c.Commit, committer, keysCache), + Verification: asymkey_service.ParseCommitWithSignatureCommitter(ctx, c.Commit, committer), } _ = asymkey_model.CalculateTrustStatus(signCommit.Verification, repoTrustModel, isOwnerMemberCollaborator, &keyMap) diff --git a/services/repository/files/commit.go b/services/repository/files/commit.go index cefd579516647..ea3d2ef7a5f65 100644 --- a/services/repository/files/commit.go +++ b/services/repository/files/commit.go @@ -6,8 +6,8 @@ package files import ( "context" - asymkey_model "code.gitea.io/gitea/models/asymkey" repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/structs" asymkey_service "code.gitea.io/gitea/services/asymkey" @@ -25,8 +25,7 @@ func CountDivergingCommits(ctx context.Context, repo *repo_model.Repository, bra // GetPayloadCommitVerification returns the verification information of a commit func GetPayloadCommitVerification(ctx context.Context, commit *git.Commit) *structs.PayloadCommitVerification { verification := &structs.PayloadCommitVerification{} - keysCache := make(map[string][]*asymkey_model.GPGKey) - commitVerification := asymkey_service.ParseCommitWithSignature(ctx, commit, keysCache) + commitVerification := asymkey_service.ParseCommitWithSignature(cache.WithCacheContext(ctx), commit) if commit.Signature != nil { verification.Signature = commit.Signature.Signature verification.Payload = commit.Signature.Payload diff --git a/services/repository/gitgraph/graph_models.go b/services/repository/gitgraph/graph_models.go index 81c6f8bb29cdb..95e50abc9e226 100644 --- a/services/repository/gitgraph/graph_models.go +++ b/services/repository/gitgraph/graph_models.go @@ -15,6 +15,7 @@ import ( git_model "code.gitea.io/gitea/models/git" repo_model "code.gitea.io/gitea/models/repo" 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" asymkey_service "code.gitea.io/gitea/services/asymkey" @@ -97,7 +98,7 @@ func (graph *Graph) LoadAndProcessCommits(ctx context.Context, repository *repo_ emails := map[string]*user_model.User{} keyMap := map[string]bool{} - keysCache := make(map[string][]*asymkey_model.GPGKey) + ctx = cache.WithCacheContext(ctx) for _, c := range graph.Commits { if len(c.Rev) == 0 { @@ -116,7 +117,7 @@ func (graph *Graph) LoadAndProcessCommits(ctx context.Context, repository *repo_ } } - c.Verification = asymkey_service.ParseCommitWithSignature(ctx, c.Commit, keysCache) + c.Verification = asymkey_service.ParseCommitWithSignature(ctx, c.Commit) _ = asymkey_model.CalculateTrustStatus(c.Verification, repository.GetTrustModel(), func(user *user_model.User) (bool, error) { return repo_model.IsOwnerMemberCollaborator(ctx, repository, user.ID) From 22686cdefa11ff2af2dc12885310e7c08ecb3619 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 1 Apr 2025 12:33:16 -0700 Subject: [PATCH 3/7] some improvements --- models/user/user.go | 29 ++++++++++++++++------------- services/asymkey/commit.go | 31 ++++++++++++++++++++----------- services/git/commit.go | 10 ++++++---- 3 files changed, 42 insertions(+), 28 deletions(-) diff --git a/models/user/user.go b/models/user/user.go index 3b268a6f41e52..e2905b24530d1 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -1187,24 +1187,27 @@ func GetUsersByEmails(ctx context.Context, emails []string) (map[string]*User, e for _, email := range emailAddresses { userIDs.Add(email.UID) } - users, err := GetUsersMapByIDs(ctx, userIDs.Values()) - if err != nil { - return nil, err - } - results := make(map[string]*User, len(emails)) - for _, email := range emailAddresses { - user := users[email.UID] - if user != nil { - if user.KeepEmailPrivate { - results[user.LowerName+"@"+setting.Service.NoReplyAddress] = user - } else { - results[email.Email] = user + + if len(userIDs) > 0 { + users, err := GetUsersMapByIDs(ctx, userIDs.Values()) + if err != nil { + return nil, err + } + + for _, email := range emailAddresses { + user := users[email.UID] + if user != nil { + if user.KeepEmailPrivate { + results[user.LowerName+"@"+setting.Service.NoReplyAddress] = user + } else { + results[email.Email] = user + } } } } - users = make(map[int64]*User, len(needCheckUserNames)) + users := make(map[int64]*User, len(needCheckUserNames)) if err := db.GetEngine(ctx).In("lower_name", needCheckUserNames.Values()).Find(&users); err != nil { return nil, err } diff --git a/services/asymkey/commit.go b/services/asymkey/commit.go index 4160974a5f045..5e3b09a25f7bf 100644 --- a/services/asymkey/commit.go +++ b/services/asymkey/commit.go @@ -48,7 +48,11 @@ func ParseCommitWithSignature(ctx context.Context, c *git.Commit) *asymkey_model return ParseCommitWithSignatureCommitter(ctx, c, committer) } -const cacheUserEmailAddressKey = "gpg_user_email_address" +const ( + cacheUserEmailAddressKey = "gpg_user_email_address" + cacheUserKey = "gpg_user" + cacheGPGListKey = "gpg_key_list" +) func ParseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, committer *user_model.User) *asymkey_model.CommitVerification { // If no signature just report the committer @@ -214,10 +218,13 @@ func checkKeyEmails(ctx context.Context, email string, keys ...*asymkey_model.GP } if key.Verified && key.OwnerID != 0 { if uid != key.OwnerID { - userEmails, _ = user_model.GetEmailAddresses(ctx, key.OwnerID) + userEmails, _ = cache.GetWithContextCache(ctx, cacheUserEmailAddressKey, key.OwnerID, func() ([]*user_model.EmailAddress, error) { + return user_model.GetEmailAddresses(ctx, key.OwnerID) + }) uid = key.OwnerID - user = &user_model.User{ID: uid} - _, _ = user_model.GetUser(ctx, user) + user, _ = cache.GetWithContextCache(ctx, cacheUserKey, uid, func() (*user_model.User, error) { + return user_model.GetUserByID(ctx, uid) + }) } for _, e := range userEmails { if e.IsActivated && (email == "" || strings.EqualFold(e.Email, email)) { @@ -232,13 +239,11 @@ func checkKeyEmails(ctx context.Context, email string, keys ...*asymkey_model.GP return false, email } -const cacheGroupKey = "gpg_key_id" - func HashAndVerifyForKeyID(ctx context.Context, sig *packet.Signature, payload string, committer *user_model.User, keyID, name, email string) *asymkey_model.CommitVerification { if keyID == "" { return nil } - keys, err := cache.GetWithContextCache(ctx, cacheGroupKey, keyID, func() ([]*asymkey_model.GPGKey, error) { + keys, err := cache.GetWithContextCache(ctx, cacheGPGListKey, keyID, func() ([]*asymkey_model.GPGKey, error) { return db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{ KeyID: keyID, IncludeSubKeys: true, @@ -258,7 +263,7 @@ func HashAndVerifyForKeyID(ctx context.Context, sig *packet.Signature, payload s for _, key := range keys { var primaryKeys []*asymkey_model.GPGKey if key.PrimaryKeyID != "" { - primaryKeys, err = cache.GetWithContextCache(ctx, cacheGroupKey, key.PrimaryKeyID, func() ([]*asymkey_model.GPGKey, error) { + primaryKeys, err = cache.GetWithContextCache(ctx, cacheGPGListKey, key.PrimaryKeyID, func() ([]*asymkey_model.GPGKey, error) { return db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{ KeyID: key.PrimaryKeyID, IncludeSubKeys: true, @@ -283,8 +288,10 @@ func HashAndVerifyForKeyID(ctx context.Context, sig *packet.Signature, payload s Name: name, Email: email, } - if key.OwnerID != 0 { - owner, err := user_model.GetUserByID(ctx, key.OwnerID) + if key.OwnerID > 0 { + owner, err := cache.GetWithContextCache(ctx, cacheUserKey, committer.ID, func() (*user_model.User, error) { + return user_model.GetUserByID(ctx, key.OwnerID) + }) if err == nil { signer = owner } else if !user_model.IsErrUserNotExist(err) { @@ -392,7 +399,9 @@ func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committer * } } - committerEmailAddresses, err := user_model.GetEmailAddresses(ctx, committer.ID) + committerEmailAddresses, err := cache.GetWithContextCache(ctx, cacheUserEmailAddressKey, committer.ID, func() ([]*user_model.EmailAddress, error) { + return user_model.GetEmailAddresses(ctx, committer.ID) + }) if err != nil { log.Error("GetEmailAddresses: %v", err) } diff --git a/services/git/commit.go b/services/git/commit.go index fb64afc00172d..3821c4c5c87cf 100644 --- a/services/git/commit.go +++ b/services/git/commit.go @@ -18,7 +18,7 @@ import ( ) // ParseCommitsWithSignature checks if signaute of commits are corresponding to users gpg keys. -func ParseCommitsWithSignature(ctx context.Context, oldCommits []*user_model.UserCommit, repoTrustModel repo_model.TrustModelType, isOwnerMemberCollaborator func(*user_model.User) (bool, error)) ([]*asymkey_model.SignCommit, error) { +func ParseCommitsWithSignature(ctx context.Context, repo *repo_model.Repository, oldCommits []*user_model.UserCommit, repoTrustModel repo_model.TrustModelType) ([]*asymkey_model.SignCommit, error) { newCommits := make([]*asymkey_model.SignCommit, 0, len(oldCommits)) keyMap := map[string]bool{} @@ -50,6 +50,10 @@ func ParseCommitsWithSignature(ctx context.Context, oldCommits []*user_model.Use Verification: asymkey_service.ParseCommitWithSignatureCommitter(ctx, c.Commit, committer), } + isOwnerMemberCollaborator := func(user *user_model.User) (bool, error) { + return repo_model.IsOwnerMemberCollaborator(ctx, repo, user.ID) + } + _ = asymkey_model.CalculateTrustStatus(signCommit.Verification, repoTrustModel, isOwnerMemberCollaborator, &keyMap) newCommits = append(newCommits, signCommit) @@ -65,11 +69,9 @@ func ConvertFromGitCommit(ctx context.Context, commits []*git.Commit, repo *repo } signedCommits, err := ParseCommitsWithSignature( ctx, + repo, validatedCommits, repo.GetTrustModel(), - func(user *user_model.User) (bool, error) { - return repo_model.IsOwnerMemberCollaborator(ctx, repo, user.ID) - }, ) if err != nil { return nil, err From 919d7203700d69b64e5b88014a2e344510f522c2 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 2 Apr 2025 10:17:14 -0700 Subject: [PATCH 4/7] Fix copy typo --- services/asymkey/commit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/asymkey/commit.go b/services/asymkey/commit.go index 5e3b09a25f7bf..46200e309e3e5 100644 --- a/services/asymkey/commit.go +++ b/services/asymkey/commit.go @@ -289,7 +289,7 @@ func HashAndVerifyForKeyID(ctx context.Context, sig *packet.Signature, payload s Email: email, } if key.OwnerID > 0 { - owner, err := cache.GetWithContextCache(ctx, cacheUserKey, committer.ID, func() (*user_model.User, error) { + owner, err := cache.GetWithContextCache(ctx, cacheUserKey, key.OwnerID, func() (*user_model.User, error) { return user_model.GetUserByID(ctx, key.OwnerID) }) if err == nil { From 47d581ce59a2fd07ebcd328b242c0a302615dba7 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 7 Apr 2025 11:17:46 -0700 Subject: [PATCH 5/7] use user's email methods instead of duplicated code --- models/user/user.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/models/user/user.go b/models/user/user.go index e2905b24530d1..3039095e6d2a6 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -1198,11 +1198,7 @@ func GetUsersByEmails(ctx context.Context, emails []string) (map[string]*User, e for _, email := range emailAddresses { user := users[email.UID] if user != nil { - if user.KeepEmailPrivate { - results[user.LowerName+"@"+setting.Service.NoReplyAddress] = user - } else { - results[email.Email] = user - } + results[user.GetEmail()] = user } } } @@ -1212,7 +1208,7 @@ func GetUsersByEmails(ctx context.Context, emails []string) (map[string]*User, e return nil, err } for _, user := range users { - results[user.LowerName+"@"+setting.Service.NoReplyAddress] = user + results[user.GetPlaceholderEmail()] = user } return results, nil } From 328477e12bc80094d8bbba2e7582f162fa2d322a Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 8 Apr 2025 17:09:20 +0800 Subject: [PATCH 6/7] revert all cache.WithCacheContext --- routers/private/hook_verification.go | 2 -- routers/web/repo/commit.go | 3 +-- routers/web/repo/view.go | 3 +-- services/asymkey/sign.go | 5 ----- services/convert/convert.go | 3 +-- services/git/commit.go | 3 --- services/repository/files/commit.go | 3 +-- services/repository/gitgraph/graph_models.go | 2 -- 8 files changed, 4 insertions(+), 20 deletions(-) diff --git a/routers/private/hook_verification.go b/routers/private/hook_verification.go index c6f897c037c19..57d0964ead21d 100644 --- a/routers/private/hook_verification.go +++ b/routers/private/hook_verification.go @@ -9,7 +9,6 @@ import ( "io" "os" - "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" asymkey_service "code.gitea.io/gitea/services/asymkey" @@ -96,7 +95,6 @@ func readAndVerifyCommit(sha string, repo *git.Repository, env []string) error { if err != nil { return err } - ctx = cache.WithCacheContext(ctx) verification := asymkey_service.ParseCommitWithSignature(ctx, commit) if !verification.Verified { cancel() diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index 699794885441f..3fd1eacb581eb 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -20,7 +20,6 @@ import ( unit_model "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/base" - "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/charset" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" @@ -385,7 +384,7 @@ func Diff(ctx *context.Context) { ctx.Data["CommitStatus"] = git_model.CalcCommitStatus(statuses) ctx.Data["CommitStatuses"] = statuses - verification := asymkey_service.ParseCommitWithSignature(cache.WithCacheContext(ctx), commit) + verification := asymkey_service.ParseCommitWithSignature(ctx, commit) ctx.Data["Verification"] = verification ctx.Data["Author"] = user_model.ValidateCommitWithEmail(ctx, commit) ctx.Data["Parents"] = parents diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index e5d0a20f0f172..77240f043156c 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -28,7 +28,6 @@ import ( unit_model "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/base" - "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/charset" "code.gitea.io/gitea/modules/fileicon" "code.gitea.io/gitea/modules/git" @@ -121,7 +120,7 @@ func loadLatestCommitData(ctx *context.Context, latestCommit *git.Commit) bool { // or of directory if not in root directory. ctx.Data["LatestCommit"] = latestCommit if latestCommit != nil { - verification := asymkey_service.ParseCommitWithSignature(cache.WithCacheContext(ctx), latestCommit) + verification := asymkey_service.ParseCommitWithSignature(ctx, latestCommit) if err := asymkey_model.CalculateTrustStatus(verification, ctx.Repo.Repository.GetTrustModel(), func(user *user_model.User) (bool, error) { return repo_model.IsOwnerMemberCollaborator(ctx, ctx.Repo.Repository, user.ID) diff --git a/services/asymkey/sign.go b/services/asymkey/sign.go index 89e3cae0418b8..2216bca54ae67 100644 --- a/services/asymkey/sign.go +++ b/services/asymkey/sign.go @@ -15,7 +15,6 @@ import ( issues_model "code.gitea.io/gitea/models/issues" repo_model "code.gitea.io/gitea/models/repo" 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/gitrepo" "code.gitea.io/gitea/modules/log" @@ -178,8 +177,6 @@ func SignWikiCommit(ctx context.Context, repo *repo_model.Repository, u *user_mo return false, "", nil, &ErrWontSign{noKey} } - ctx = cache.WithCacheContext(ctx) - Loop: for _, rule := range rules { switch rule { @@ -235,7 +232,6 @@ func SignCRUDAction(ctx context.Context, repoPath string, u *user_model.User, tm if signingKey == "" { return false, "", nil, &ErrWontSign{noKey} } - ctx = cache.WithCacheContext(ctx) Loop: for _, rule := range rules { @@ -301,7 +297,6 @@ func SignMerge(ctx context.Context, pr *issues_model.PullRequest, u *user_model. var gitRepo *git.Repository var err error - ctx = cache.WithCacheContext(ctx) Loop: for _, rule := range rules { diff --git a/services/convert/convert.go b/services/convert/convert.go index 8341a9431c13e..ac2680766c040 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -22,7 +22,6 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" @@ -255,7 +254,7 @@ func ToActionArtifact(repo *repo_model.Repository, art *actions_model.ActionArti // ToVerification convert a git.Commit.Signature to an api.PayloadCommitVerification func ToVerification(ctx context.Context, c *git.Commit) *api.PayloadCommitVerification { - verif := asymkey_service.ParseCommitWithSignature(cache.WithCacheContext(ctx), c) + verif := asymkey_service.ParseCommitWithSignature(ctx, c) commitVerification := &api.PayloadCommitVerification{ Verified: verif.Verified, Reason: verif.Reason, diff --git a/services/git/commit.go b/services/git/commit.go index 3821c4c5c87cf..3faef7678290f 100644 --- a/services/git/commit.go +++ b/services/git/commit.go @@ -11,7 +11,6 @@ import ( git_model "code.gitea.io/gitea/models/git" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/git" asymkey_service "code.gitea.io/gitea/services/asymkey" @@ -34,8 +33,6 @@ func ParseCommitsWithSignature(ctx context.Context, repo *repo_model.Repository, return nil, err } - ctx = cache.WithCacheContext(ctx) - for _, c := range oldCommits { committer, ok := emailUsers[c.Committer.Email] if !ok && c.Committer != nil { diff --git a/services/repository/files/commit.go b/services/repository/files/commit.go index ea3d2ef7a5f65..3cc326d065b16 100644 --- a/services/repository/files/commit.go +++ b/services/repository/files/commit.go @@ -7,7 +7,6 @@ import ( "context" repo_model "code.gitea.io/gitea/models/repo" - "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/structs" asymkey_service "code.gitea.io/gitea/services/asymkey" @@ -25,7 +24,7 @@ func CountDivergingCommits(ctx context.Context, repo *repo_model.Repository, bra // GetPayloadCommitVerification returns the verification information of a commit func GetPayloadCommitVerification(ctx context.Context, commit *git.Commit) *structs.PayloadCommitVerification { verification := &structs.PayloadCommitVerification{} - commitVerification := asymkey_service.ParseCommitWithSignature(cache.WithCacheContext(ctx), commit) + commitVerification := asymkey_service.ParseCommitWithSignature(ctx, commit) if commit.Signature != nil { verification.Signature = commit.Signature.Signature verification.Payload = commit.Signature.Payload diff --git a/services/repository/gitgraph/graph_models.go b/services/repository/gitgraph/graph_models.go index 95e50abc9e226..c45662836bcc3 100644 --- a/services/repository/gitgraph/graph_models.go +++ b/services/repository/gitgraph/graph_models.go @@ -15,7 +15,6 @@ import ( git_model "code.gitea.io/gitea/models/git" repo_model "code.gitea.io/gitea/models/repo" 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" asymkey_service "code.gitea.io/gitea/services/asymkey" @@ -98,7 +97,6 @@ func (graph *Graph) LoadAndProcessCommits(ctx context.Context, repository *repo_ emails := map[string]*user_model.User{} keyMap := map[string]bool{} - ctx = cache.WithCacheContext(ctx) for _, c := range graph.Commits { if len(c.Rev) == 0 { From 3be445673d7e00146b3b3faffec5458ba9157544 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 8 Apr 2025 17:28:18 +0800 Subject: [PATCH 7/7] fix --- models/asymkey/gpg_key.go | 7 +++++ modules/cache/context.go | 8 +++--- modules/cache/context_test.go | 3 +- modules/cachegroup/cachegroup.go | 12 ++++++++ modules/repository/commits.go | 3 +- routers/private/hook_post_receive.go | 5 ++-- services/asymkey/commit.go | 41 ++++++---------------------- services/convert/pull.go | 13 +++++---- 8 files changed, 44 insertions(+), 48 deletions(-) create mode 100644 modules/cachegroup/cachegroup.go diff --git a/models/asymkey/gpg_key.go b/models/asymkey/gpg_key.go index 24c76f7b5cfb5..220f46ad1d44d 100644 --- a/models/asymkey/gpg_key.go +++ b/models/asymkey/gpg_key.go @@ -240,3 +240,10 @@ func DeleteGPGKey(ctx context.Context, doer *user_model.User, id int64) (err err return committer.Commit() } + +func FindGPGKeyWithSubKeys(ctx context.Context, keyID string) ([]*GPGKey, error) { + return db.Find[GPGKey](ctx, FindGPGKeyOptions{ + KeyID: keyID, + IncludeSubKeys: true, + }) +} diff --git a/modules/cache/context.go b/modules/cache/context.go index 484cee659a12b..85eb9e6790e24 100644 --- a/modules/cache/context.go +++ b/modules/cache/context.go @@ -166,15 +166,15 @@ func RemoveContextData(ctx context.Context, tp, key any) { } // GetWithContextCache returns the cache value of the given key in the given context. -func GetWithContextCache[T any](ctx context.Context, cacheGroupKey string, cacheTargetID any, f func() (T, error)) (T, error) { - v := GetContextData(ctx, cacheGroupKey, cacheTargetID) +func GetWithContextCache[T, K any](ctx context.Context, groupKey string, targetKey K, f func(context.Context, K) (T, error)) (T, error) { + v := GetContextData(ctx, groupKey, targetKey) if vv, ok := v.(T); ok { return vv, nil } - t, err := f() + t, err := f(ctx, targetKey) if err != nil { return t, err } - SetContextData(ctx, cacheGroupKey, cacheTargetID, t) + SetContextData(ctx, groupKey, targetKey, t) return t, nil } diff --git a/modules/cache/context_test.go b/modules/cache/context_test.go index decb532937cc9..23dd789dbc28b 100644 --- a/modules/cache/context_test.go +++ b/modules/cache/context_test.go @@ -4,6 +4,7 @@ package cache import ( + "context" "testing" "time" @@ -30,7 +31,7 @@ func TestWithCacheContext(t *testing.T) { v = GetContextData(ctx, field, "my_config1") assert.Nil(t, v) - vInt, err := GetWithContextCache(ctx, field, "my_config1", func() (int, error) { + vInt, err := GetWithContextCache(ctx, field, "my_config1", func(context.Context, string) (int, error) { return 1, nil }) assert.NoError(t, err) diff --git a/modules/cachegroup/cachegroup.go b/modules/cachegroup/cachegroup.go new file mode 100644 index 0000000000000..06085f860f788 --- /dev/null +++ b/modules/cachegroup/cachegroup.go @@ -0,0 +1,12 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package cachegroup + +const ( + User = "user" + EmailAvatarLink = "email_avatar_link" + UserEmailAddresses = "user_email_addresses" + GPGKeyWithSubKeys = "gpg_key_with_subkeys" + RepoUserPermission = "repo_user_permission" +) diff --git a/modules/repository/commits.go b/modules/repository/commits.go index 16520fb28a5b5..878fdc16039bb 100644 --- a/modules/repository/commits.go +++ b/modules/repository/commits.go @@ -13,6 +13,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/cache" + "code.gitea.io/gitea/modules/cachegroup" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" @@ -131,7 +132,7 @@ func (pc *PushCommits) ToAPIPayloadCommits(ctx context.Context, repo *repo_model func (pc *PushCommits) AvatarLink(ctx context.Context, email string) string { size := avatars.DefaultAvatarPixelSize * setting.Avatar.RenderedSizeFactor - v, _ := cache.GetWithContextCache(ctx, "push_commits", email, func() (string, error) { + v, _ := cache.GetWithContextCache(ctx, cachegroup.EmailAvatarLink, email, func(ctx context.Context, email string) (string, error) { u, err := user_model.GetUserByEmail(ctx, email) if err != nil { if !user_model.IsErrUserNotExist(err) { diff --git a/routers/private/hook_post_receive.go b/routers/private/hook_post_receive.go index 442d0a76c9086..8b1e849e7a442 100644 --- a/routers/private/hook_post_receive.go +++ b/routers/private/hook_post_receive.go @@ -14,6 +14,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/cache" + "code.gitea.io/gitea/modules/cachegroup" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/log" @@ -326,9 +327,7 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) { } func loadContextCacheUser(ctx context.Context, id int64) (*user_model.User, error) { - return cache.GetWithContextCache(ctx, "hook_post_receive_user", id, func() (*user_model.User, error) { - return user_model.GetUserByID(ctx, id) - }) + return cache.GetWithContextCache(ctx, cachegroup.User, id, user_model.GetUserByID) } // handlePullRequestMerging handle pull request merging, a pull request action should push at least 1 commit diff --git a/services/asymkey/commit.go b/services/asymkey/commit.go index 46200e309e3e5..105782a93a57c 100644 --- a/services/asymkey/commit.go +++ b/services/asymkey/commit.go @@ -12,6 +12,7 @@ import ( "code.gitea.io/gitea/models/db" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/cache" + "code.gitea.io/gitea/modules/cachegroup" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" @@ -48,12 +49,6 @@ func ParseCommitWithSignature(ctx context.Context, c *git.Commit) *asymkey_model return ParseCommitWithSignatureCommitter(ctx, c, committer) } -const ( - cacheUserEmailAddressKey = "gpg_user_email_address" - cacheUserKey = "gpg_user" - cacheGPGListKey = "gpg_key_list" -) - func ParseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, committer *user_model.User) *asymkey_model.CommitVerification { // If no signature just report the committer if c.Signature == nil { @@ -122,9 +117,7 @@ func ParseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, commi } } - committerEmailAddresses, _ := cache.GetWithContextCache(ctx, cacheUserEmailAddressKey, committer.ID, func() ([]*user_model.EmailAddress, error) { - return user_model.GetEmailAddresses(ctx, committer.ID) - }) + committerEmailAddresses, _ := cache.GetWithContextCache(ctx, cachegroup.UserEmailAddresses, committer.ID, user_model.GetEmailAddresses) activated := false for _, e := range committerEmailAddresses { if e.IsActivated && strings.EqualFold(e.Email, c.Committer.Email) { @@ -218,13 +211,9 @@ func checkKeyEmails(ctx context.Context, email string, keys ...*asymkey_model.GP } if key.Verified && key.OwnerID != 0 { if uid != key.OwnerID { - userEmails, _ = cache.GetWithContextCache(ctx, cacheUserEmailAddressKey, key.OwnerID, func() ([]*user_model.EmailAddress, error) { - return user_model.GetEmailAddresses(ctx, key.OwnerID) - }) + userEmails, _ = cache.GetWithContextCache(ctx, cachegroup.UserEmailAddresses, key.OwnerID, user_model.GetEmailAddresses) uid = key.OwnerID - user, _ = cache.GetWithContextCache(ctx, cacheUserKey, uid, func() (*user_model.User, error) { - return user_model.GetUserByID(ctx, uid) - }) + user, _ = cache.GetWithContextCache(ctx, cachegroup.User, uid, user_model.GetUserByID) } for _, e := range userEmails { if e.IsActivated && (email == "" || strings.EqualFold(e.Email, email)) { @@ -243,12 +232,7 @@ func HashAndVerifyForKeyID(ctx context.Context, sig *packet.Signature, payload s if keyID == "" { return nil } - keys, err := cache.GetWithContextCache(ctx, cacheGPGListKey, keyID, func() ([]*asymkey_model.GPGKey, error) { - return db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{ - KeyID: keyID, - IncludeSubKeys: true, - }) - }) + keys, err := cache.GetWithContextCache(ctx, cachegroup.GPGKeyWithSubKeys, keyID, asymkey_model.FindGPGKeyWithSubKeys) if err != nil { log.Error("GetGPGKeysByKeyID: %v", err) return &asymkey_model.CommitVerification{ @@ -263,12 +247,7 @@ func HashAndVerifyForKeyID(ctx context.Context, sig *packet.Signature, payload s for _, key := range keys { var primaryKeys []*asymkey_model.GPGKey if key.PrimaryKeyID != "" { - primaryKeys, err = cache.GetWithContextCache(ctx, cacheGPGListKey, key.PrimaryKeyID, func() ([]*asymkey_model.GPGKey, error) { - return db.Find[asymkey_model.GPGKey](ctx, asymkey_model.FindGPGKeyOptions{ - KeyID: key.PrimaryKeyID, - IncludeSubKeys: true, - }) - }) + primaryKeys, err = cache.GetWithContextCache(ctx, cachegroup.GPGKeyWithSubKeys, key.PrimaryKeyID, asymkey_model.FindGPGKeyWithSubKeys) if err != nil { log.Error("GetGPGKeysByKeyID: %v", err) return &asymkey_model.CommitVerification{ @@ -289,9 +268,7 @@ func HashAndVerifyForKeyID(ctx context.Context, sig *packet.Signature, payload s Email: email, } if key.OwnerID > 0 { - owner, err := cache.GetWithContextCache(ctx, cacheUserKey, key.OwnerID, func() (*user_model.User, error) { - return user_model.GetUserByID(ctx, key.OwnerID) - }) + owner, err := cache.GetWithContextCache(ctx, cachegroup.User, key.OwnerID, user_model.GetUserByID) if err == nil { signer = owner } else if !user_model.IsErrUserNotExist(err) { @@ -399,9 +376,7 @@ func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committer * } } - committerEmailAddresses, err := cache.GetWithContextCache(ctx, cacheUserEmailAddressKey, committer.ID, func() ([]*user_model.EmailAddress, error) { - return user_model.GetEmailAddresses(ctx, committer.ID) - }) + committerEmailAddresses, err := cache.GetWithContextCache(ctx, cachegroup.UserEmailAddresses, committer.ID, user_model.GetEmailAddresses) if err != nil { log.Error("GetEmailAddresses: %v", err) } diff --git a/services/convert/pull.go b/services/convert/pull.go index 34c3b1bf9a568..7798bebb08b32 100644 --- a/services/convert/pull.go +++ b/services/convert/pull.go @@ -14,6 +14,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/cache" + "code.gitea.io/gitea/modules/cachegroup" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/log" @@ -60,14 +61,14 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u doerID = doer.ID } - const repoDoerPermCacheKey = "repo_doer_perm_cache" - p, err := cache.GetWithContextCache(ctx, repoDoerPermCacheKey, fmt.Sprintf("%d_%d", pr.BaseRepoID, doerID), - func() (access_model.Permission, error) { + repoUserPerm, err := cache.GetWithContextCache(ctx, cachegroup.RepoUserPermission, fmt.Sprintf("%d-%d", pr.BaseRepoID, doerID), + func(ctx context.Context, _ string) (access_model.Permission, error) { return access_model.GetUserRepoPermission(ctx, pr.BaseRepo, doer) - }) + }, + ) if err != nil { log.Error("GetUserRepoPermission[%d]: %v", pr.BaseRepoID, err) - p.AccessMode = perm.AccessModeNone + repoUserPerm.AccessMode = perm.AccessModeNone } apiPullRequest := &api.PullRequest{ @@ -107,7 +108,7 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u Name: pr.BaseBranch, Ref: pr.BaseBranch, RepoID: pr.BaseRepoID, - Repository: ToRepo(ctx, pr.BaseRepo, p), + Repository: ToRepo(ctx, pr.BaseRepo, repoUserPerm), }, Head: &api.PRBranchInfo{ Name: pr.HeadBranch,