Skip to content

If a repository return no commitstatus, then still cache it but not query it from database #30700

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions services/repository/commitstatus/commitstatus.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,10 @@ func getCommitStatusCache(repoID int64, branchName string) *commitStatusCacheVal
if ok && statusStr != "" {
var cv commitStatusCacheValue
err := json.Unmarshal([]byte(statusStr), &cv)
if err == nil && cv.State != "" {
if err == nil {
return &cv
}
if err != nil {
log.Warn("getCommitStatusCache: json.Unmarshal failed: %v", err)
}
log.Warn("getCommitStatusCache: json.Unmarshal failed: %v", err)
}
return nil
}
Expand Down Expand Up @@ -128,15 +126,22 @@ 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))
allCached := true
for i, repo := range repos {
if cv := getCommitStatusCache(repo.ID, repo.DefaultBranch); cv != nil {
results[i] = &git_model.CommitStatus{
State: api.CommitStatusState(cv.State),
TargetURL: cv.TargetURL,
}
} else {
allCached = false
}
}

if allCached {
return results, nil
}

// collect the latest commit of each repo
// at most there are dozens of repos (limited by MaxResponseItems), so it's not a big problem at the moment
repoBranchNames := make(map[int64]string, len(repos))
Expand Down Expand Up @@ -165,10 +170,10 @@ func FindReposLastestCommitStatuses(ctx context.Context, repos []*repo_model.Rep
for i, repo := range repos {
if repo.ID == summary.RepoID {
results[i] = summary
_ = slices.DeleteFunc(repoSHAs, func(repoSHA git_model.RepoSHA) bool {
repoSHAs = slices.DeleteFunc(repoSHAs, func(repoSHA git_model.RepoSHA) bool {
return repoSHA.RepoID == repo.ID
})
if results[i].State != "" {
if results[i] != 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)
}
Expand All @@ -177,6 +182,9 @@ func FindReposLastestCommitStatuses(ctx context.Context, repos []*repo_model.Rep
}
}
}
if len(repoSHAs) == 0 {
return results, nil
}

// call the database O(1) times to get the commit statuses for all repos
repoToItsLatestCommitStatuses, err := git_model.GetLatestCommitStatusForPairs(ctx, repoSHAs)
Expand All @@ -187,7 +195,7 @@ func FindReposLastestCommitStatuses(ctx context.Context, repos []*repo_model.Rep
for i, repo := range repos {
if results[i] == nil {
results[i] = git_model.CalcCommitStatus(repoToItsLatestCommitStatuses[repo.ID])
if results[i].State != "" {
if results[i] != 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)
}
Expand Down