From c5566a60722c85a0569774541e055b5158dcf823 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 12 Apr 2024 10:59:28 +0800 Subject: [PATCH 1/5] Fix commit status cache which missed target_url --- .../repository/commitstatus/commitstatus.go | 44 ++++++++++++++----- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/services/repository/commitstatus/commitstatus.go b/services/repository/commitstatus/commitstatus.go index 167a5330ddc49..d81efc2164cba 100644 --- a/services/repository/commitstatus/commitstatus.go +++ b/services/repository/commitstatus/commitstatus.go @@ -6,6 +6,7 @@ package commitstatus import ( "context" "crypto/sha256" + "encoding/json" "fmt" "slices" @@ -26,12 +27,33 @@ func getCacheKey(repoID int64, brancheName string) string { return fmt.Sprintf("commit_status:%x", hashBytes) } -func updateCommitStatusCache(ctx context.Context, repoID int64, branchName string, status api.CommitStatusState) error { +type commitStatusCacheValue struct { + State string + TargetURL string +} + +func getCommitStatusCache(repoID int64, branchName string) *commitStatusCacheValue { c := cache.GetCache() - return c.Put(getCacheKey(repoID, branchName), string(status), 3*24*60) + statusStr, ok := c.Get(getCacheKey(repoID, branchName)).(string) + if ok && statusStr != "" { + var cv commitStatusCacheValue + if err := json.Unmarshal([]byte(statusStr), &cv); err == nil && cv.State != "" { + return &cv + } + } + return nil } -func deleteCommitStatusCache(ctx context.Context, repoID int64, branchName string) error { +func updateCommitStatusCache(repoID int64, branchName string, state api.CommitStatusState, targetURL string) error { + c := cache.GetCache() + bs, _ := json.Marshal(commitStatusCacheValue{ + State: string(state), + TargetURL: targetURL, + }) + return c.Put(getCacheKey(repoID, branchName), string(bs), 3*24*60) +} + +func deleteCommitStatusCache(repoID int64, branchName string) error { c := cache.GetCache() return c.Delete(getCacheKey(repoID, branchName)) } @@ -81,7 +103,7 @@ func CreateCommitStatus(ctx context.Context, repo *repo_model.Repository, creato } if commit.ID.String() == defaultBranchCommit.ID.String() { // since one commit status updated, the combined commit status should be invalid - if err := deleteCommitStatusCache(ctx, repo.ID, repo.DefaultBranch); err != nil { + if err := deleteCommitStatusCache(repo.ID, repo.DefaultBranch); err != nil { log.Error("deleteCommitStatusCache[%d:%s] failed: %v", repo.ID, repo.DefaultBranch, err) } } @@ -98,12 +120,12 @@ func CreateCommitStatus(ctx context.Context, repo *repo_model.Repository, creato // FindReposLastestCommitStatuses loading repository default branch latest combinded commit status with cache func FindReposLastestCommitStatuses(ctx context.Context, repos []*repo_model.Repository) ([]*git_model.CommitStatus, error) { results := make([]*git_model.CommitStatus, len(repos)) - c := cache.GetCache() - for i, repo := range repos { - status, ok := c.Get(getCacheKey(repo.ID, repo.DefaultBranch)).(string) - if ok && status != "" { - results[i] = &git_model.CommitStatus{State: api.CommitStatusState(status)} + if cv := getCommitStatusCache(repo.ID, repo.DefaultBranch); cv != nil { + results[i] = &git_model.CommitStatus{ + State: api.CommitStatusState(cv.State), + TargetURL: cv.TargetURL, + } } } @@ -139,7 +161,7 @@ func FindReposLastestCommitStatuses(ctx context.Context, repos []*repo_model.Rep return repoSHA.RepoID == repo.ID }) if results[i].State != "" { - if err := updateCommitStatusCache(ctx, repo.ID, repo.DefaultBranch, results[i].State); err != nil { + if err := updateCommitStatusCache(repo.ID, repo.DefaultBranch, results[i].State, results[i].TargetURL); err != nil { log.Error("updateCommitStatusCache[%d:%s] failed: %v", repo.ID, repo.DefaultBranch, err) } } @@ -158,7 +180,7 @@ func FindReposLastestCommitStatuses(ctx context.Context, repos []*repo_model.Rep if results[i] == nil { results[i] = git_model.CalcCommitStatus(repoToItsLatestCommitStatuses[repo.ID]) if results[i].State != "" { - if err := updateCommitStatusCache(ctx, repo.ID, repo.DefaultBranch, results[i].State); err != nil { + if err := updateCommitStatusCache(repo.ID, repo.DefaultBranch, results[i].State, results[i].TargetURL); err != nil { log.Error("updateCommitStatusCache[%d:%s] failed: %v", repo.ID, repo.DefaultBranch, err) } } From b3095da00ec89fc095cd593987624c8f8d3c8623 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 12 Apr 2024 14:45:46 +0800 Subject: [PATCH 2/5] Fix lint --- services/repository/commitstatus/commitstatus.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/repository/commitstatus/commitstatus.go b/services/repository/commitstatus/commitstatus.go index d81efc2164cba..dcd76015b9804 100644 --- a/services/repository/commitstatus/commitstatus.go +++ b/services/repository/commitstatus/commitstatus.go @@ -6,7 +6,6 @@ package commitstatus import ( "context" "crypto/sha256" - "encoding/json" "fmt" "slices" @@ -17,6 +16,7 @@ import ( "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" + "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/services/automerge" From cd1f6815539c844c002f5e7a57afffd6cc7c0109 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 12 Apr 2024 14:57:10 +0800 Subject: [PATCH 3/5] Use state.String instead of string(state) --- services/repository/commitstatus/commitstatus.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/repository/commitstatus/commitstatus.go b/services/repository/commitstatus/commitstatus.go index dcd76015b9804..5c262ce3ac124 100644 --- a/services/repository/commitstatus/commitstatus.go +++ b/services/repository/commitstatus/commitstatus.go @@ -47,7 +47,7 @@ func getCommitStatusCache(repoID int64, branchName string) *commitStatusCacheVal func updateCommitStatusCache(repoID int64, branchName string, state api.CommitStatusState, targetURL string) error { c := cache.GetCache() bs, _ := json.Marshal(commitStatusCacheValue{ - State: string(state), + State: state.String(), TargetURL: targetURL, }) return c.Put(getCacheKey(repoID, branchName), string(bs), 3*24*60) From 2a21a4655c3d2c807a40c945866be2f6aa2bdbe6 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 12 Apr 2024 15:27:23 +0800 Subject: [PATCH 4/5] Add warning when marshar/unmarshal failed --- services/repository/commitstatus/commitstatus.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/services/repository/commitstatus/commitstatus.go b/services/repository/commitstatus/commitstatus.go index 5c262ce3ac124..2e1fc96674290 100644 --- a/services/repository/commitstatus/commitstatus.go +++ b/services/repository/commitstatus/commitstatus.go @@ -37,19 +37,27 @@ func getCommitStatusCache(repoID int64, branchName string) *commitStatusCacheVal statusStr, ok := c.Get(getCacheKey(repoID, branchName)).(string) if ok && statusStr != "" { var cv commitStatusCacheValue - if err := json.Unmarshal([]byte(statusStr), &cv); err == nil && cv.State != "" { + err := json.Unmarshal([]byte(statusStr), &cv) + if err == nil && cv.State != "" { return &cv } + if err != nil { + log.Warn("getCommitStatusCache: json.Unmarshal failed: %v", err) + } } return nil } func updateCommitStatusCache(repoID int64, branchName string, state api.CommitStatusState, targetURL string) error { c := cache.GetCache() - bs, _ := json.Marshal(commitStatusCacheValue{ + bs, err := json.Marshal(commitStatusCacheValue{ State: state.String(), TargetURL: targetURL, }) + if err != nil { + log.Warn("updateCommitStatusCache: json.Marshal failed: %v", err) + return nil + } return c.Put(getCacheKey(repoID, branchName), string(bs), 3*24*60) } From d651109d21ecf293c7d364ad6b4430065ae6090e Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 12 Apr 2024 16:22:51 +0800 Subject: [PATCH 5/5] Update services/repository/commitstatus/commitstatus.go Co-authored-by: Jason Song --- services/repository/commitstatus/commitstatus.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/repository/commitstatus/commitstatus.go b/services/repository/commitstatus/commitstatus.go index 2e1fc96674290..7c1c6c260938c 100644 --- a/services/repository/commitstatus/commitstatus.go +++ b/services/repository/commitstatus/commitstatus.go @@ -28,8 +28,8 @@ func getCacheKey(repoID int64, brancheName string) string { } type commitStatusCacheValue struct { - State string - TargetURL string + State string `json:"state"` + TargetURL string `json:"target_url"` } func getCommitStatusCache(repoID int64, branchName string) *commitStatusCacheValue {