From b7fa38774ba58b6cadd97cc651fdc3e7d7a46465 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Mon, 31 Jul 2023 08:53:14 +0000 Subject: [PATCH 1/9] improve --- models/git/commit_status.go | 156 ++++++++++--------------- models/migrations/migrations.go | 2 + models/migrations/v1_21/v271.go | 58 +++++++++ modules/gitgraph/graph_models.go | 6 +- modules/structs/commit_status.go | 41 +++---- routers/api/v1/repo/status.go | 6 +- routers/web/repo/branch.go | 7 +- routers/web/repo/commit.go | 6 +- routers/web/repo/pull.go | 20 ++-- routers/web/repo/repo.go | 4 +- routers/web/repo/view.go | 6 +- services/actions/commit_status.go | 4 +- services/convert/status.go | 10 +- services/pull/commit_status.go | 61 +++------- services/pull/pull.go | 3 +- templates/repo/commit_status.tmpl | 16 +-- templates/repo/pulls/status.tmpl | 14 +-- tests/integration/repo_commits_test.go | 12 +- 18 files changed, 205 insertions(+), 227 deletions(-) create mode 100644 models/migrations/v1_21/v271.go diff --git a/models/git/commit_status.go b/models/git/commit_status.go index 17090f30dd55b..3682895ae9dce 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -33,7 +33,7 @@ type CommitStatus struct { Index int64 `xorm:"INDEX UNIQUE(repo_sha_index)"` RepoID int64 `xorm:"INDEX UNIQUE(repo_sha_index)"` Repo *repo_model.Repository `xorm:"-"` - State api.CommitStatusState `xorm:"VARCHAR(7) NOT NULL"` + State api.CommitStatusState `xorm:"INDEX NOT NULL"` SHA string `xorm:"VARCHAR(64) NOT NULL INDEX UNIQUE(repo_sha_index)"` TargetURL string `xorm:"TEXT"` Description string `xorm:"TEXT"` @@ -191,26 +191,6 @@ func (status *CommitStatus) APIURL(ctx context.Context) string { return status.Repo.APIURL() + "/statuses/" + url.PathEscape(status.SHA) } -// CalcCommitStatus returns commit status state via some status, the commit statues should order by id desc -func CalcCommitStatus(statuses []*CommitStatus) *CommitStatus { - var lastStatus *CommitStatus - state := api.CommitStatusSuccess - for _, status := range statuses { - if status.State.NoBetterThan(state) { - state = status.State - lastStatus = status - } - } - if lastStatus == nil { - if len(statuses) > 0 { - lastStatus = statuses[0] - } else { - lastStatus = &CommitStatus{} - } - } - return lastStatus -} - // CommitStatusOptions holds the options for query commit statuses type CommitStatusOptions struct { db.ListOptions @@ -277,120 +257,104 @@ type CommitStatusIndex struct { } // GetLatestCommitStatus returns all statuses with a unique context for a given commit. -func GetLatestCommitStatus(ctx context.Context, repoID int64, sha string, listOptions db.ListOptions) ([]*CommitStatus, int64, error) { - ids := make([]int64, 0, 10) - sess := db.GetEngine(ctx).Table(&CommitStatus{}). - Where("repo_id = ?", repoID).And("sha = ?", sha). - Select("max( id ) as id"). - GroupBy("context_hash").OrderBy("max( id ) desc") +func GetLatestCommitStatuses(ctx context.Context, repoID int64, sha string, listOptions db.ListOptions) ([]*CommitStatus, *CommitStatus, int64, error) { + statuses := make([]*CommitStatus, 0, 10) + idCond := builder.Select("max( id ) as id").From("commit_status"). + And(builder.Eq{"repo_id": repoID, "sha": sha}).GroupBy("context_hash") - sess = db.SetSessionPagination(sess, &listOptions) + sess := db.GetEngine(ctx).In("id", idCond).OrderBy(db.SearchOrderByIDReverse.String()) - count, err := sess.FindAndCount(&ids) + count, err := db.SetSessionPagination(sess, &listOptions).FindAndCount(&statuses) if err != nil { - return nil, count, err + return nil, nil, count, err } - statuses := make([]*CommitStatus, 0, len(ids)) - if len(ids) == 0 { - return statuses, count, nil + + status := &CommitStatus{} + _, err = db.GetEngine(ctx).Select("*, min( state ) as state").In("id", idCond).Get(status) + if err != nil { + return nil, nil, count, err } - return statuses, count, db.GetEngine(ctx).In("id", ids).Find(&statuses) + return statuses, status, count, nil } // GetLatestCommitStatusForPairs returns all statuses with a unique context for a given list of repo-sha pairs -func GetLatestCommitStatusForPairs(ctx context.Context, repoIDsToLatestCommitSHAs map[int64]string, listOptions db.ListOptions) (map[int64][]*CommitStatus, error) { - type result struct { - ID int64 - RepoID int64 - } - - results := make([]result, 0, len(repoIDsToLatestCommitSHAs)) - - sess := db.GetEngine(ctx).Table(&CommitStatus{}) +func GetLatestCommitStatusesForPairs(ctx context.Context, repoIDsToLatestCommitSHAs map[int64]string, listOptions db.ListOptions) (map[int64][]*CommitStatus, map[int64]*CommitStatus, error) { + statuses := make([]*CommitStatus, 0, len(repoIDsToLatestCommitSHAs)) // Create a disjunction of conditions for each repoID and SHA pair conds := make([]builder.Cond, 0, len(repoIDsToLatestCommitSHAs)) for repoID, sha := range repoIDsToLatestCommitSHAs { conds = append(conds, builder.Eq{"repo_id": repoID, "sha": sha}) } - sess = sess.Where(builder.Or(conds...)). - Select("max( id ) as id, repo_id"). - GroupBy("context_hash, repo_id").OrderBy("max( id ) desc") - sess = db.SetSessionPagination(sess, &listOptions) + idCond := builder.Select("max( id ) as id").From("commit_status"). + Where(builder.Or(conds...)).GroupBy("context_hash, repo_id") + + sess := db.GetEngine(ctx).In("id", idCond).OrderBy(db.SearchOrderByIDReverse.String()) - err := sess.Find(&results) + err := db.SetSessionPagination(sess, &listOptions).Find(&statuses) if err != nil { - return nil, err + return nil, nil, err } - ids := make([]int64, 0, len(results)) repoStatuses := make(map[int64][]*CommitStatus) - for _, result := range results { - ids = append(ids, result.ID) + // Group the statuses by repo ID + for _, status := range statuses { + repoStatuses[status.RepoID] = append(repoStatuses[status.RepoID], status) } - statuses := make([]*CommitStatus, 0, len(ids)) - if len(ids) > 0 { - err = db.GetEngine(ctx).In("id", ids).Find(&statuses) - if err != nil { - return nil, err - } - - // Group the statuses by repo ID - for _, status := range statuses { - repoStatuses[status.RepoID] = append(repoStatuses[status.RepoID], status) - } + err = db.GetEngine(ctx).Select("*, min( state ) as state").In("id", idCond). + GroupBy("repo_id").Find(&statuses) + if err != nil { + return nil, nil, err } - return repoStatuses, nil + repoStatus := make(map[int64]*CommitStatus) + // Group the statuses by repo ID + for _, status := range statuses { + repoStatus[status.RepoID] = status + } + return repoStatuses, repoStatus, err } // GetLatestCommitStatusForRepoCommitIDs returns all statuses with a unique context for a given list of repo-sha pairs -func GetLatestCommitStatusForRepoCommitIDs(ctx context.Context, repoID int64, commitIDs []string) (map[string][]*CommitStatus, error) { - type result struct { - ID int64 - Sha string - } - - results := make([]result, 0, len(commitIDs)) - - sess := db.GetEngine(ctx).Table(&CommitStatus{}) +func GetLatestCommitStatusesForRepoCommitIDs(ctx context.Context, repoID int64, commitIDs []string) (map[string][]*CommitStatus, map[string]*CommitStatus, error) { + statuses := make([]*CommitStatus, 0, len(commitIDs)) // Create a disjunction of conditions for each repoID and SHA pair conds := make([]builder.Cond, 0, len(commitIDs)) for _, sha := range commitIDs { conds = append(conds, builder.Eq{"sha": sha}) } - sess = sess.Where(builder.Eq{"repo_id": repoID}.And(builder.Or(conds...))). - Select("max( id ) as id, sha"). - GroupBy("context_hash, sha").OrderBy("max( id ) desc") - err := sess.Find(&results) + idCond := builder.Select("max( id ) as id").From("commit_status"). + Where(builder.Eq{"repo_id": repoID}.And(builder.Or(conds...))). + GroupBy("context_hash, sha") + + err := db.GetEngine(ctx).In("id", idCond). + OrderBy(db.SearchOrderByIDReverse.String()).Find(&statuses) if err != nil { - return nil, err + return nil, nil, err } - ids := make([]int64, 0, len(results)) repoStatuses := make(map[string][]*CommitStatus) - for _, result := range results { - ids = append(ids, result.ID) + // Group the statuses by commit ID + for _, status := range statuses { + repoStatuses[status.SHA] = append(repoStatuses[status.SHA], status) } - statuses := make([]*CommitStatus, 0, len(ids)) - if len(ids) > 0 { - err = db.GetEngine(ctx).In("id", ids).Find(&statuses) - if err != nil { - return nil, err - } - - // Group the statuses by repo ID - for _, status := range statuses { - repoStatuses[status.SHA] = append(repoStatuses[status.SHA], status) - } + err = db.GetEngine(ctx).Select("*, min( state ) as state").In("id", idCond). + GroupBy("sha").Find(&statuses) + if err != nil { + return nil, nil, err } - return repoStatuses, nil + repoStatus := make(map[string]*CommitStatus) + // Group the statuses by repo ID + for _, status := range statuses { + repoStatus[status.SHA] = status + } + return repoStatuses, repoStatus, err } // FindRepoRecentCommitStatusContexts returns repository's recent commit status contexts @@ -482,12 +446,12 @@ func ParseCommitsWithStatus(ctx context.Context, oldCommits []*asymkey_model.Sig commit := &SignCommitWithStatuses{ SignCommit: c, } - statuses, _, err := GetLatestCommitStatus(ctx, repo.ID, commit.ID.String(), db.ListOptions{}) + statuses, status, _, err := GetLatestCommitStatuses(ctx, repo.ID, commit.ID.String(), db.ListOptions{}) if err != nil { - log.Error("GetLatestCommitStatus: %v", err) + log.Error("GetLatestCommitStatuses: %v", err) } else { commit.Statuses = statuses - commit.Status = CalcCommitStatus(statuses) + commit.Status = status } newCommits = append(newCommits, commit) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index b2140a1eb1327..fb55702a88e3e 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -523,6 +523,8 @@ var migrations = []Migration{ NewMigration("Drop deleted branch table", v1_21.DropDeletedBranchTable), // v270 -> v271 NewMigration("Fix PackageProperty typo", v1_21.FixPackagePropertyTypo), + // v270 -> v271 + NewMigration("Convert CommitStatus State into Int", v1_21.ConvertCommitStatusStateIntoInt), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_21/v271.go b/models/migrations/v1_21/v271.go new file mode 100644 index 0000000000000..9f5e69376af7c --- /dev/null +++ b/models/migrations/v1_21/v271.go @@ -0,0 +1,58 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_21 //nolint + +import ( + "code.gitea.io/gitea/modules/log" + "xorm.io/xorm" +) + +func ConvertCommitStatusStateIntoInt(x *xorm.Engine) error { + // CommitStatusState holds the state of a CommitStatus + // It can be "pending", "success", "error" and "failure" + type CommitStatusState int + + const ( + // CommitStatusError is for when the CommitStatus is Error + CommitStatusError CommitStatusState = iota + 1 + // CommitStatusFailure is for when the CommitStatus is Failure + CommitStatusFailure + // CommitStatusPending is for when the CommitStatus is Pending + CommitStatusPending + // CommitStatusSuccess is for when the CommitStatus is Success + CommitStatusSuccess + ) + + // CommitStatus holds a single Status of a single Commit + type CommitStatus struct { + State CommitStatusState `xorm:"INDEX NOT NULL"` + } + + var commitStatusConvertMap = map[string]CommitStatusState{ + "error": CommitStatusError, + "failure": CommitStatusFailure, + "pending": CommitStatusPending, + "success": CommitStatusSuccess, + } + + sess := x.NewSession() + defer sess.Close() + if err := sess.Begin(); err != nil { + return err + } + + if err := sess.Sync2(new(CommitStatus)); err != nil { + return err + } + + for origin, target := range commitStatusConvertMap { + count, err := sess.Where("`state` = ?", origin).Update(&CommitStatus{State: target}) + if err != nil { + return err + } + log.Debug("Updated %d commit status with %s status", count, origin) + } + + return sess.Commit() +} diff --git a/modules/gitgraph/graph_models.go b/modules/gitgraph/graph_models.go index 748f7f307574e..4647ffa68fef5 100644 --- a/modules/gitgraph/graph_models.go +++ b/modules/gitgraph/graph_models.go @@ -119,11 +119,11 @@ func (graph *Graph) LoadAndProcessCommits(ctx context.Context, repository *repo_ return repo_model.IsOwnerMemberCollaborator(repository, user.ID) }, &keyMap) - statuses, _, err := git_model.GetLatestCommitStatus(db.DefaultContext, repository.ID, c.Commit.ID.String(), db.ListOptions{}) + _, status, _, err := git_model.GetLatestCommitStatuses(db.DefaultContext, repository.ID, c.Commit.ID.String(), db.ListOptions{}) if err != nil { - log.Error("GetLatestCommitStatus: %v", err) + log.Error("GetLatestCommitStatuses: %v", err) } else { - c.Status = git_model.CalcCommitStatus(statuses) + c.Status = status } } return nil diff --git a/modules/structs/commit_status.go b/modules/structs/commit_status.go index de1d8fa566cd0..1f5cdf66680da 100644 --- a/modules/structs/commit_status.go +++ b/modules/structs/commit_status.go @@ -5,39 +5,34 @@ package structs // CommitStatusState holds the state of a CommitStatus // It can be "pending", "success", "error" and "failure" -type CommitStatusState string +type CommitStatusState int const ( - // CommitStatusPending is for when the CommitStatus is Pending - CommitStatusPending CommitStatusState = "pending" - // CommitStatusSuccess is for when the CommitStatus is Success - CommitStatusSuccess CommitStatusState = "success" // CommitStatusError is for when the CommitStatus is Error - CommitStatusError CommitStatusState = "error" + CommitStatusError CommitStatusState = iota + 1 // CommitStatusFailure is for when the CommitStatus is Failure - CommitStatusFailure CommitStatusState = "failure" + CommitStatusFailure + // CommitStatusPending is for when the CommitStatus is Pending + CommitStatusPending + // CommitStatusSuccess is for when the CommitStatus is Success + CommitStatusSuccess ) -var commitStatusPriorities = map[CommitStatusState]int{ - CommitStatusError: 0, - CommitStatusFailure: 1, - CommitStatusPending: 2, - CommitStatusSuccess: 3, +var commitStatusNames = map[CommitStatusState]string{ + CommitStatusError: "error", + CommitStatusFailure: "failure", + CommitStatusPending: "pending", + CommitStatusSuccess: "success", +} + +// String returns the string name of the CommitStatusState +func (css CommitStatusState) String() string { + return commitStatusNames[css] } // NoBetterThan returns true if this State is no better than the given State -// This function only handles the states defined in CommitStatusPriorities func (css CommitStatusState) NoBetterThan(css2 CommitStatusState) bool { - // NoBetterThan only handles the 4 states above - if _, exist := commitStatusPriorities[css]; !exist { - return false - } - - if _, exist := commitStatusPriorities[css2]; !exist { - return false - } - - return commitStatusPriorities[css] <= commitStatusPriorities[css2] + return int(css) <= int(css2) } // IsPending represents if commit status state is pending diff --git a/routers/api/v1/repo/status.go b/routers/api/v1/repo/status.go index 028e3083c60a8..eeb103393421c 100644 --- a/routers/api/v1/repo/status.go +++ b/routers/api/v1/repo/status.go @@ -253,9 +253,9 @@ func GetCombinedCommitStatusByRef(ctx *context.APIContext) { repo := ctx.Repo.Repository - statuses, count, err := git_model.GetLatestCommitStatus(ctx, repo.ID, sha, utils.GetListOptions(ctx)) + statuses, status, count, err := git_model.GetLatestCommitStatuses(ctx, repo.ID, sha, utils.GetListOptions(ctx)) if err != nil { - ctx.Error(http.StatusInternalServerError, "GetLatestCommitStatus", fmt.Errorf("GetLatestCommitStatus[%s, %s]: %w", repo.FullName(), sha, err)) + ctx.Error(http.StatusInternalServerError, "GetLatestCommitStatuses", fmt.Errorf("GetLatestCommitStatuses[%s, %s]: %w", repo.FullName(), sha, err)) return } @@ -264,7 +264,7 @@ func GetCombinedCommitStatusByRef(ctx *context.APIContext) { return } - combiStatus := convert.ToCombinedStatus(ctx, statuses, convert.ToRepo(ctx, repo, ctx.Repo.Permission)) + combiStatus := convert.ToCombinedStatus(ctx, statuses, status, convert.ToRepo(ctx, repo, ctx.Repo.Permission)) ctx.SetTotalCountHeader(count) ctx.JSON(http.StatusOK, combiStatus) diff --git a/routers/web/repo/branch.go b/routers/web/repo/branch.go index d71d555bc2b54..081857ce0fe5b 100644 --- a/routers/web/repo/branch.go +++ b/routers/web/repo/branch.go @@ -62,17 +62,12 @@ func Branches(ctx *context.Context) { commitIDs = append(commitIDs, branch.DBBranch.CommitID) } - commitStatuses, err := git_model.GetLatestCommitStatusForRepoCommitIDs(ctx, ctx.Repo.Repository.ID, commitIDs) + commitStatuses, commitStatus, err := git_model.GetLatestCommitStatusesForRepoCommitIDs(ctx, ctx.Repo.Repository.ID, commitIDs) if err != nil { ctx.ServerError("LoadBranches", err) return } - commitStatus := make(map[string]*git_model.CommitStatus) - for commitID, cs := range commitStatuses { - commitStatus[commitID] = git_model.CalcCommitStatus(cs) - } - ctx.Data["Branches"] = branches ctx.Data["CommitStatus"] = commitStatus ctx.Data["CommitStatuses"] = commitStatuses diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index 5b32591b89140..c46c0fd71d88e 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -349,12 +349,12 @@ func Diff(ctx *context.Context) { ctx.Data["Commit"] = commit ctx.Data["Diff"] = diff - statuses, _, err := git_model.GetLatestCommitStatus(ctx, ctx.Repo.Repository.ID, commitID, db.ListOptions{}) + statuses, status, _, err := git_model.GetLatestCommitStatuses(ctx, ctx.Repo.Repository.ID, commitID, db.ListOptions{}) if err != nil { - log.Error("GetLatestCommitStatus: %v", err) + log.Error("GetLatestCommitStatuses: %v", err) } - ctx.Data["CommitStatus"] = git_model.CalcCommitStatus(statuses) + ctx.Data["CommitStatus"] = status ctx.Data["CommitStatuses"] = statuses verification := asymkey_model.ParseCommitWithSignature(ctx, commit) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index a24ef43367717..94657f20fd035 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -469,14 +469,14 @@ func PrepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue) if len(compareInfo.Commits) != 0 { sha := compareInfo.Commits[0].ID.String() - commitStatuses, _, err := git_model.GetLatestCommitStatus(ctx, ctx.Repo.Repository.ID, sha, db.ListOptions{}) + commitStatuses, commitStatus, _, err := git_model.GetLatestCommitStatuses(ctx, ctx.Repo.Repository.ID, sha, db.ListOptions{}) if err != nil { - ctx.ServerError("GetLatestCommitStatus", err) + ctx.ServerError("GetLatestCommitStatuses", err) return nil } if len(commitStatuses) != 0 { ctx.Data["LatestCommitStatuses"] = commitStatuses - ctx.Data["LatestCommitStatus"] = git_model.CalcCommitStatus(commitStatuses) + ctx.Data["LatestCommitStatus"] = commitStatus } } @@ -531,14 +531,14 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C ctx.ServerError(fmt.Sprintf("GetRefCommitID(%s)", pull.GetGitRefName()), err) return nil } - commitStatuses, _, err := git_model.GetLatestCommitStatus(ctx, repo.ID, sha, db.ListOptions{}) + commitStatuses, commitStatus, _, err := git_model.GetLatestCommitStatuses(ctx, repo.ID, sha, db.ListOptions{}) if err != nil { - ctx.ServerError("GetLatestCommitStatus", err) + ctx.ServerError("GetLatestCommitStatuses", err) return nil } if len(commitStatuses) > 0 { ctx.Data["LatestCommitStatuses"] = commitStatuses - ctx.Data["LatestCommitStatus"] = git_model.CalcCommitStatus(commitStatuses) + ctx.Data["LatestCommitStatus"] = commitStatus } compareInfo, err := baseGitRepo.GetCompareInfo(pull.BaseRepo.RepoPath(), @@ -623,14 +623,14 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C return nil } - commitStatuses, _, err := git_model.GetLatestCommitStatus(ctx, repo.ID, sha, db.ListOptions{}) + commitStatuses, commitStatus, _, err := git_model.GetLatestCommitStatuses(ctx, repo.ID, sha, db.ListOptions{}) if err != nil { - ctx.ServerError("GetLatestCommitStatus", err) + ctx.ServerError("GetLatestCommitStatuses", err) return nil } if len(commitStatuses) > 0 { ctx.Data["LatestCommitStatuses"] = commitStatuses - ctx.Data["LatestCommitStatus"] = git_model.CalcCommitStatus(commitStatuses) + ctx.Data["LatestCommitStatus"] = commitStatus } if pb != nil && pb.EnableStatusCheck { @@ -642,7 +642,7 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C } return false } - ctx.Data["RequiredStatusCheckState"] = pull_service.MergeRequiredContextsCommitStatus(commitStatuses, pb.StatusCheckContexts) + ctx.Data["RequiredStatusCheckState"] = pull_service.MergeRequiredContextsCommitStatus(commitStatuses, commitStatus, pb.StatusCheckContexts) } ctx.Data["HeadBranchMovedOn"] = headBranchSha != sha diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go index 0de804dbce6fa..a129e68ddd0eb 100644 --- a/routers/web/repo/repo.go +++ b/routers/web/repo/repo.go @@ -592,7 +592,7 @@ func SearchRepo(ctx *context.Context) { } // call the database O(1) times to get the commit statuses for all repos - repoToItsLatestCommitStatuses, err := git_model.GetLatestCommitStatusForPairs(ctx, repoIDsToLatestCommitSHAs, db.ListOptions{}) + _, repoToItsLatestCommitStatus, err := git_model.GetLatestCommitStatusesForPairs(ctx, repoIDsToLatestCommitSHAs, db.ListOptions{}) if err != nil { log.Error("GetLatestCommitStatusForPairs: %v", err) return @@ -613,7 +613,7 @@ func SearchRepo(ctx *context.Context) { Link: repo.Link(), Internal: !repo.IsPrivate && repo.Owner.Visibility == api.VisibleTypePrivate, }, - LatestCommitStatus: git_model.CalcCommitStatus(repoToItsLatestCommitStatuses[repo.ID]), + LatestCommitStatus: repoToItsLatestCommitStatus[repo.ID], } } diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 221c1f4c4f62b..d1247f21c00e7 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -842,12 +842,12 @@ func renderDirectoryFiles(ctx *context.Context, timeout time.Duration) git.Entri ctx.Data["LatestCommitVerification"] = verification ctx.Data["LatestCommitUser"] = user_model.ValidateCommitWithEmail(ctx, latestCommit) - statuses, _, err := git_model.GetLatestCommitStatus(ctx, ctx.Repo.Repository.ID, latestCommit.ID.String(), db.ListOptions{}) + statuses, status, _, err := git_model.GetLatestCommitStatuses(ctx, ctx.Repo.Repository.ID, latestCommit.ID.String(), db.ListOptions{}) if err != nil { - log.Error("GetLatestCommitStatus: %v", err) + log.Error("GetLatestCommitStatuses: %v", err) } - ctx.Data["LatestCommitStatus"] = git_model.CalcCommitStatus(statuses) + ctx.Data["LatestCommitStatus"] = status ctx.Data["LatestCommitStatuses"] = statuses } diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go index 925d0a33968aa..8c688f83e8f57 100644 --- a/services/actions/commit_status.go +++ b/services/actions/commit_status.go @@ -75,7 +75,7 @@ func createCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er } ctxname := fmt.Sprintf("%s / %s (%s)", runName, job.Name, event) state := toCommitStatus(job.Status) - if statuses, _, err := git_model.GetLatestCommitStatus(ctx, repo.ID, sha, db.ListOptions{}); err == nil { + if statuses, _, _, err := git_model.GetLatestCommitStatuses(ctx, repo.ID, sha, db.ListOptions{}); err == nil { for _, v := range statuses { if v.Context == ctxname { if v.State == state { @@ -86,7 +86,7 @@ func createCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er } } } else { - return fmt.Errorf("GetLatestCommitStatus: %w", err) + return fmt.Errorf("GetLatestCommitStatuses: %w", err) } description := "" diff --git a/services/convert/status.go b/services/convert/status.go index 6cef63c1cdadf..c3a7c347fe237 100644 --- a/services/convert/status.go +++ b/services/convert/status.go @@ -33,7 +33,7 @@ func ToCommitStatus(ctx context.Context, status *git_model.CommitStatus) *api.Co } // ToCombinedStatus converts List of CommitStatus to a CombinedStatus -func ToCombinedStatus(ctx context.Context, statuses []*git_model.CommitStatus, repo *api.Repository) *api.CombinedStatus { +func ToCombinedStatus(ctx context.Context, statuses []*git_model.CommitStatus, status *git_model.CommitStatus, repo *api.Repository) *api.CombinedStatus { if len(statuses) == 0 { return nil } @@ -46,12 +46,10 @@ func ToCombinedStatus(ctx context.Context, statuses []*git_model.CommitStatus, r } retStatus.Statuses = make([]*api.CommitStatus, 0, len(statuses)) - for _, status := range statuses { - retStatus.Statuses = append(retStatus.Statuses, ToCommitStatus(ctx, status)) - if retStatus.State == "" || status.State.NoBetterThan(retStatus.State) { - retStatus.State = status.State - } + for _, s := range statuses { + retStatus.Statuses = append(retStatus.Statuses, ToCommitStatus(ctx, s)) } + retStatus.State = status.State // According to https://docs.github.com/en/rest/commits/statuses?apiVersion=2022-11-28#get-the-combined-status-for-a-specific-reference // > Additionally, a combined state is returned. The state is one of: // > failure if any of the contexts report as error or failure diff --git a/services/pull/commit_status.go b/services/pull/commit_status.go index 51ba06da27586..952c82d852912 100644 --- a/services/pull/commit_status.go +++ b/services/pull/commit_status.go @@ -19,7 +19,7 @@ import ( ) // MergeRequiredContextsCommitStatus returns a commit status state for given required contexts -func MergeRequiredContextsCommitStatus(commitStatuses []*git_model.CommitStatus, requiredContexts []string) structs.CommitStatusState { +func MergeRequiredContextsCommitStatus(commitStatuses []*git_model.CommitStatus, commitStatus *git_model.CommitStatus, requiredContexts []string) structs.CommitStatusState { // matchedCount is the number of `CommitStatus.Context` that match any context of `requiredContexts` matchedCount := 0 returnedStatus := structs.CommitStatusSuccess @@ -44,16 +44,15 @@ func MergeRequiredContextsCommitStatus(commitStatuses []*git_model.CommitStatus, } } - if targetStatus != "" && targetStatus.NoBetterThan(returnedStatus) { + if targetStatus != 0 && targetStatus.NoBetterThan(returnedStatus) { returnedStatus = targetStatus } } } if matchedCount == 0 { - status := git_model.CalcCommitStatus(commitStatuses) - if status != nil { - return status.State + if commitStatus != nil { + return commitStatus.State } return structs.CommitStatusSuccess } @@ -61,41 +60,11 @@ func MergeRequiredContextsCommitStatus(commitStatuses []*git_model.CommitStatus, return returnedStatus } -// IsCommitStatusContextSuccess returns true if all required status check contexts succeed. -func IsCommitStatusContextSuccess(commitStatuses []*git_model.CommitStatus, requiredContexts []string) bool { - // If no specific context is required, require that last commit status is a success - if len(requiredContexts) == 0 { - status := git_model.CalcCommitStatus(commitStatuses) - if status == nil || status.State != structs.CommitStatusSuccess { - return false - } - return true - } - - for _, ctx := range requiredContexts { - var found bool - for _, commitStatus := range commitStatuses { - if commitStatus.Context == ctx { - if commitStatus.State != structs.CommitStatusSuccess { - return false - } - - found = true - break - } - } - if !found { - return false - } - } - return true -} - // IsPullCommitStatusPass returns if all required status checks PASS func IsPullCommitStatusPass(ctx context.Context, pr *issues_model.PullRequest) (bool, error) { pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch) if err != nil { - return false, errors.Wrap(err, "GetLatestCommitStatus") + return false, errors.Wrap(err, "GetFirstMatchProtectedBranchRule") } if pb == nil || !pb.EnableStatusCheck { return true, nil @@ -112,21 +81,21 @@ func IsPullCommitStatusPass(ctx context.Context, pr *issues_model.PullRequest) ( func GetPullRequestCommitStatusState(ctx context.Context, pr *issues_model.PullRequest) (structs.CommitStatusState, error) { // Ensure HeadRepo is loaded if err := pr.LoadHeadRepo(ctx); err != nil { - return "", errors.Wrap(err, "LoadHeadRepo") + return 0, errors.Wrap(err, "LoadHeadRepo") } // check if all required status checks are successful headGitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, pr.HeadRepo.RepoPath()) if err != nil { - return "", errors.Wrap(err, "OpenRepository") + return 0, errors.Wrap(err, "OpenRepository") } defer closer.Close() if pr.Flow == issues_model.PullRequestFlowGithub && !headGitRepo.IsBranchExist(pr.HeadBranch) { - return "", errors.New("Head branch does not exist, can not merge") + return 0, errors.New("Head branch does not exist, can not merge") } if pr.Flow == issues_model.PullRequestFlowAGit && !git.IsReferenceExist(ctx, headGitRepo.Path, pr.GetGitRefName()) { - return "", errors.New("Head branch does not exist, can not merge") + return 0, errors.New("Head branch does not exist, can not merge") } var sha string @@ -136,26 +105,26 @@ func GetPullRequestCommitStatusState(ctx context.Context, pr *issues_model.PullR sha, err = headGitRepo.GetRefCommitID(pr.GetGitRefName()) } if err != nil { - return "", err + return 0, err } if err := pr.LoadBaseRepo(ctx); err != nil { - return "", errors.Wrap(err, "LoadBaseRepo") + return 0, errors.Wrap(err, "LoadBaseRepo") } - commitStatuses, _, err := git_model.GetLatestCommitStatus(ctx, pr.BaseRepo.ID, sha, db.ListOptions{}) + commitStatuses, commitStatus, _, err := git_model.GetLatestCommitStatuses(ctx, pr.BaseRepo.ID, sha, db.ListOptions{}) if err != nil { - return "", errors.Wrap(err, "GetLatestCommitStatus") + return 0, errors.Wrap(err, "GetLatestCommitStatuses") } pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch) if err != nil { - return "", errors.Wrap(err, "LoadProtectedBranch") + return 0, errors.Wrap(err, "LoadProtectedBranch") } var requiredContexts []string if pb != nil { requiredContexts = pb.StatusCheckContexts } - return MergeRequiredContextsCommitStatus(commitStatuses, requiredContexts), nil + return MergeRequiredContextsCommitStatus(commitStatuses, commitStatus, requiredContexts), nil } diff --git a/services/pull/pull.go b/services/pull/pull.go index 8c0b65fd8b8b5..e27df3a2dc2f7 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -804,8 +804,7 @@ func getAllCommitStatus(gitRepo *git.Repository, pr *issues_model.PullRequest) ( return nil, nil, shaErr } - statuses, _, err = git_model.GetLatestCommitStatus(db.DefaultContext, pr.BaseRepo.ID, sha, db.ListOptions{}) - lastStatus = git_model.CalcCommitStatus(statuses) + statuses, lastStatus, _, err = git_model.GetLatestCommitStatuses(db.DefaultContext, pr.BaseRepo.ID, sha, db.ListOptions{}) return statuses, lastStatus, err } diff --git a/templates/repo/commit_status.tmpl b/templates/repo/commit_status.tmpl index ebd8a55f65e38..e83e546c2df44 100644 --- a/templates/repo/commit_status.tmpl +++ b/templates/repo/commit_status.tmpl @@ -1,13 +1,13 @@ -{{if eq .State "pending"}} - {{svg "octicon-dot-fill" 18 "commit-status icon text yellow"}} -{{end}} -{{if eq .State "success"}} - {{svg "octicon-check" 18 "commit-status icon text green"}} -{{end}} -{{if eq .State "error"}} +{{if eq .State 1}} {{svg "gitea-exclamation" 18 "commit-status icon text red"}} {{end}} -{{if eq .State "failure"}} +{{if eq .State 2}} {{svg "octicon-x" 18 "commit-status icon text red"}} {{end}} +{{if eq .State 3}} + {{svg "octicon-dot-fill" 18 "commit-status icon text yellow"}} +{{end}} +{{if eq .State 4}} + {{svg "octicon-check" 18 "commit-status icon text green"}} +{{end}} diff --git a/templates/repo/pulls/status.tmpl b/templates/repo/pulls/status.tmpl index 25d8954658f34..75c2556c30ff4 100644 --- a/templates/repo/pulls/status.tmpl +++ b/templates/repo/pulls/status.tmpl @@ -1,16 +1,14 @@ {{if $.LatestCommitStatus}} {{if not $.Issue.PullRequest.HasMerged}}
- {{if eq .LatestCommitStatus.State "pending"}} - {{$.locale.Tr "repo.pulls.status_checking"}} - {{else if eq .LatestCommitStatus.State "success"}} - {{$.locale.Tr "repo.pulls.status_checks_success"}} - {{else if eq .LatestCommitStatus.State "warning"}} - {{$.locale.Tr "repo.pulls.status_checks_warning"}} - {{else if eq .LatestCommitStatus.State "failure"}} + {{if eq .LatestCommitStatus.State 1}} {{$.locale.Tr "repo.pulls.status_checks_failure"}} - {{else if eq .LatestCommitStatus.State "error"}} + {{else if eq .LatestCommitStatus.State 2}} {{$.locale.Tr "repo.pulls.status_checks_error"}} + {{else if eq .LatestCommitStatus.State 3}} + {{$.locale.Tr "repo.pulls.status_checking"}} + {{else if eq .LatestCommitStatus.State 4}} + {{$.locale.Tr "repo.pulls.status_checks_success"}} {{else}} {{$.locale.Tr "repo.pulls.status_checking"}} {{end}} diff --git a/tests/integration/repo_commits_test.go b/tests/integration/repo_commits_test.go index 789b5f7cc301d..8962837769407 100644 --- a/tests/integration/repo_commits_test.go +++ b/tests/integration/repo_commits_test.go @@ -35,7 +35,7 @@ func TestRepoCommits(t *testing.T) { assert.NotEmpty(t, commitURL) } -func doTestRepoCommitWithStatus(t *testing.T, state string, classes ...string) { +func doTestRepoCommitWithStatus(t *testing.T, state int, classes ...string) { defer tests.PrepareTestEnv(t)() session := loginUser(t, "user2") @@ -89,7 +89,7 @@ func doTestRepoCommitWithStatus(t *testing.T, state string, classes ...string) { testRepoCommitsWithStatus(t, session.MakeRequest(t, req, http.StatusOK), session.MakeRequest(t, reqOne, http.StatusOK), state) } -func testRepoCommitsWithStatus(t *testing.T, resp, respOne *httptest.ResponseRecorder, state string) { +func testRepoCommitsWithStatus(t *testing.T, resp, respOne *httptest.ResponseRecorder, state int) { var statuses []*api.CommitStatus assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), &statuses)) var status api.CombinedStatus @@ -110,19 +110,19 @@ func testRepoCommitsWithStatus(t *testing.T, resp, respOne *httptest.ResponseRec } func TestRepoCommitsWithStatusPending(t *testing.T) { - doTestRepoCommitWithStatus(t, "pending", "octicon-dot-fill", "yellow") + doTestRepoCommitWithStatus(t, 3, "octicon-dot-fill", "yellow") } func TestRepoCommitsWithStatusSuccess(t *testing.T) { - doTestRepoCommitWithStatus(t, "success", "octicon-check", "green") + doTestRepoCommitWithStatus(t, 4, "octicon-check", "green") } func TestRepoCommitsWithStatusError(t *testing.T) { - doTestRepoCommitWithStatus(t, "error", "gitea-exclamation", "red") + doTestRepoCommitWithStatus(t, 1, "gitea-exclamation", "red") } func TestRepoCommitsWithStatusFailure(t *testing.T) { - doTestRepoCommitWithStatus(t, "failure", "octicon-x", "red") + doTestRepoCommitWithStatus(t, 2, "octicon-x", "red") } func TestRepoCommitsStatusParallel(t *testing.T) { From cccd2ef453bee2d53c33c5738887393ce55c74cd Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Tue, 1 Aug 2023 02:08:24 +0000 Subject: [PATCH 2/9] improve test --- models/fixtures/commit_status.yml | 39 +++++++------------ models/fixtures/commit_status_index.yml | 2 +- models/git/commit_status_test.go | 50 ++++++++++++++++--------- 3 files changed, 47 insertions(+), 44 deletions(-) diff --git a/models/fixtures/commit_status.yml b/models/fixtures/commit_status.yml index 20d57975ef40d..f60a6d505f485 100644 --- a/models/fixtures/commit_status.yml +++ b/models/fixtures/commit_status.yml @@ -2,40 +2,29 @@ id: 1 index: 1 repo_id: 1 - state: "pending" + state: 1 # error sha: "1234123412341234123412341234123412341234" target_url: https://example.com/builds/ - description: My awesome CI-service - context: ci/awesomeness + description: My awesome deploy service + context: deploy/awesomeness creator_id: 2 - id: 2 index: 2 repo_id: 1 - state: "warning" + state: 2 # failure sha: "1234123412341234123412341234123412341234" - target_url: https://example.com/converage/ - description: My awesome Coverage service - context: cov/awesomeness + target_url: https://example.com/builds/ + description: My awesome CI-service + context: ci/awesomeness creator_id: 2 - id: 3 index: 3 repo_id: 1 - state: "success" - sha: "1234123412341234123412341234123412341234" - target_url: https://example.com/converage/ - description: My awesome Coverage service - context: cov/awesomeness - creator_id: 2 - -- - id: 4 - index: 4 - repo_id: 1 - state: "failure" + state: 3 # pending sha: "1234123412341234123412341234123412341234" target_url: https://example.com/builds/ description: My awesome CI-service @@ -43,12 +32,12 @@ creator_id: 2 - - id: 5 - index: 5 + id: 4 + index: 4 repo_id: 1 - state: "error" + state: 4 # success sha: "1234123412341234123412341234123412341234" - target_url: https://example.com/builds/ - description: My awesome deploy service - context: deploy/awesomeness + target_url: https://example.com/converage/ + description: My awesome Coverage service + context: cov/awesomeness creator_id: 2 diff --git a/models/fixtures/commit_status_index.yml b/models/fixtures/commit_status_index.yml index 3f252e87ef092..13ab6debd7ab3 100644 --- a/models/fixtures/commit_status_index.yml +++ b/models/fixtures/commit_status_index.yml @@ -2,4 +2,4 @@ id: 1 repo_id: 1 sha: "1234123412341234123412341234123412341234" - max_index: 5 \ No newline at end of file + max_index: 4 \ No newline at end of file diff --git a/models/git/commit_status_test.go b/models/git/commit_status_test.go index a86941a0fec8c..89834845b1bec 100644 --- a/models/git/commit_status_test.go +++ b/models/git/commit_status_test.go @@ -21,25 +21,39 @@ func TestGetCommitStatuses(t *testing.T) { repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) sha1 := "1234123412341234123412341234123412341234" + count := 4 statuses, maxResults, err := git_model.GetCommitStatuses(db.DefaultContext, repo1, sha1, &git_model.CommitStatusOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 50}}) assert.NoError(t, err) - assert.Equal(t, int(maxResults), 5) - assert.Len(t, statuses, 5) - - assert.Equal(t, "ci/awesomeness", statuses[0].Context) - assert.Equal(t, structs.CommitStatusPending, statuses[0].State) - assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[0].APIURL(db.DefaultContext)) - - assert.Equal(t, "cov/awesomeness", statuses[2].Context) - assert.Equal(t, structs.CommitStatusSuccess, statuses[2].State) - assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[2].APIURL(db.DefaultContext)) - - assert.Equal(t, "ci/awesomeness", statuses[3].Context) - assert.Equal(t, structs.CommitStatusFailure, statuses[3].State) - assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[3].APIURL(db.DefaultContext)) - - assert.Equal(t, "deploy/awesomeness", statuses[4].Context) - assert.Equal(t, structs.CommitStatusError, statuses[4].State) - assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[4].APIURL(db.DefaultContext)) + assert.Equal(t, int(maxResults), count) + assert.Len(t, statuses, count) + + wants := []struct { + context string + commitStatus structs.CommitStatusState + }{ + { + context: "deploy/awesomeness", + commitStatus: structs.CommitStatusError, + }, + { + context: "ci/awesomeness", + commitStatus: structs.CommitStatusFailure, + }, + { + context: "ci/awesomeness", + commitStatus: structs.CommitStatusPending, + }, + { + context: "cov/awesomeness", + commitStatus: structs.CommitStatusSuccess, + }, + } + for i := 0; i < count; i++ { + t.Run("", func(t *testing.T) { + assert.Equal(t, wants[i].context, statuses[i].Context) + assert.Equal(t, wants[i].commitStatus, statuses[i].State) + assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[i].APIURL(db.DefaultContext)) + }) + } } From f99fbaf5c491ab63cfd5315bd52ec2e766487437 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Tue, 1 Aug 2023 02:31:50 +0000 Subject: [PATCH 3/9] use CommitStatusState.IsXXXX --- templates/repo/commit_status.tmpl | 8 ++++---- templates/repo/pulls/status.tmpl | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/templates/repo/commit_status.tmpl b/templates/repo/commit_status.tmpl index e83e546c2df44..5495a91debf6c 100644 --- a/templates/repo/commit_status.tmpl +++ b/templates/repo/commit_status.tmpl @@ -1,13 +1,13 @@ -{{if eq .State 1}} +{{if .State.IsError}} {{svg "gitea-exclamation" 18 "commit-status icon text red"}} {{end}} -{{if eq .State 2}} +{{if .State.IsFailure}} {{svg "octicon-x" 18 "commit-status icon text red"}} {{end}} -{{if eq .State 3}} +{{if .State.IsPending}} {{svg "octicon-dot-fill" 18 "commit-status icon text yellow"}} {{end}} -{{if eq .State 4}} +{{if .State.IsSuccess}} {{svg "octicon-check" 18 "commit-status icon text green"}} {{end}} diff --git a/templates/repo/pulls/status.tmpl b/templates/repo/pulls/status.tmpl index 75c2556c30ff4..416e620b05e0f 100644 --- a/templates/repo/pulls/status.tmpl +++ b/templates/repo/pulls/status.tmpl @@ -1,13 +1,13 @@ {{if $.LatestCommitStatus}} {{if not $.Issue.PullRequest.HasMerged}}
- {{if eq .LatestCommitStatus.State 1}} - {{$.locale.Tr "repo.pulls.status_checks_failure"}} - {{else if eq .LatestCommitStatus.State 2}} + {{if .LatestCommitStatus.State.IsError}} {{$.locale.Tr "repo.pulls.status_checks_error"}} - {{else if eq .LatestCommitStatus.State 3}} + {{else if .LatestCommitStatus.State.IsFailure}} + {{$.locale.Tr "repo.pulls.status_checks_failure"}} + {{else if .LatestCommitStatus.State.IsPending}} {{$.locale.Tr "repo.pulls.status_checking"}} - {{else if eq .LatestCommitStatus.State 4}} + {{else if .LatestCommitStatus.State.IsSuccess}} {{$.locale.Tr "repo.pulls.status_checks_success"}} {{else}} {{$.locale.Tr "repo.pulls.status_checking"}} From 1fad846d747c4691e2c259c931585f1174161467 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Tue, 1 Aug 2023 02:57:06 +0000 Subject: [PATCH 4/9] add IsValid --- modules/structs/commit_status.go | 6 ++++++ services/pull/commit_status.go | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/modules/structs/commit_status.go b/modules/structs/commit_status.go index 1f5cdf66680da..b140453e21f58 100644 --- a/modules/structs/commit_status.go +++ b/modules/structs/commit_status.go @@ -30,7 +30,13 @@ func (css CommitStatusState) String() string { return commitStatusNames[css] } +func (css CommitStatusState) IsValid() bool { + _, ok := commitStatusNames[css] + return ok +} + // NoBetterThan returns true if this State is no better than the given State +// You should ensure the States are valid by call IsValid() func (css CommitStatusState) NoBetterThan(css2 CommitStatusState) bool { return int(css) <= int(css2) } diff --git a/services/pull/commit_status.go b/services/pull/commit_status.go index 952c82d852912..0ce3097ff893e 100644 --- a/services/pull/commit_status.go +++ b/services/pull/commit_status.go @@ -44,7 +44,7 @@ func MergeRequiredContextsCommitStatus(commitStatuses []*git_model.CommitStatus, } } - if targetStatus != 0 && targetStatus.NoBetterThan(returnedStatus) { + if targetStatus.IsValid() && targetStatus.NoBetterThan(returnedStatus) { returnedStatus = targetStatus } } From 302e5836b66409af9998f227b441b61667ae85cc Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Tue, 1 Aug 2023 02:57:38 +0000 Subject: [PATCH 5/9] fix lint --- models/migrations/v1_21/v271.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/models/migrations/v1_21/v271.go b/models/migrations/v1_21/v271.go index 9f5e69376af7c..7eb8592666510 100644 --- a/models/migrations/v1_21/v271.go +++ b/models/migrations/v1_21/v271.go @@ -5,6 +5,7 @@ package v1_21 //nolint import ( "code.gitea.io/gitea/modules/log" + "xorm.io/xorm" ) @@ -29,7 +30,7 @@ func ConvertCommitStatusStateIntoInt(x *xorm.Engine) error { State CommitStatusState `xorm:"INDEX NOT NULL"` } - var commitStatusConvertMap = map[string]CommitStatusState{ + commitStatusConvertMap := map[string]CommitStatusState{ "error": CommitStatusError, "failure": CommitStatusFailure, "pending": CommitStatusPending, From 4852352177b355a32116c330e91e0a6b3801c0cf Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Tue, 1 Aug 2023 05:15:57 +0000 Subject: [PATCH 6/9] fix --- routers/web/repo/branch.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/web/repo/branch.go b/routers/web/repo/branch.go index 081857ce0fe5b..f4e91390a4778 100644 --- a/routers/web/repo/branch.go +++ b/routers/web/repo/branch.go @@ -64,7 +64,7 @@ func Branches(ctx *context.Context) { commitStatuses, commitStatus, err := git_model.GetLatestCommitStatusesForRepoCommitIDs(ctx, ctx.Repo.Repository.ID, commitIDs) if err != nil { - ctx.ServerError("LoadBranches", err) + ctx.ServerError("GetLatestCommitStatusesForRepoCommitIDs", err) return } From ab6a8d5c1e6420e0eb8a0b22cf89f4fb81556d27 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Tue, 1 Aug 2023 08:02:35 +0000 Subject: [PATCH 7/9] improve --- models/git/commit_status.go | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/models/git/commit_status.go b/models/git/commit_status.go index 3682895ae9dce..bf947281652e5 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -270,10 +270,15 @@ func GetLatestCommitStatuses(ctx context.Context, repoID int64, sha string, list } status := &CommitStatus{} - _, err = db.GetEngine(ctx).Select("*, min( state ) as state").In("id", idCond).Get(status) + _, err = db.GetEngine(ctx). + Where(builder.And( + builder.In("id", idCond), + builder.In("state", minStateCond(idCond, "")), + )).Get(status) if err != nil { return nil, nil, count, err } + return statuses, status, count, nil } @@ -303,8 +308,11 @@ func GetLatestCommitStatusesForPairs(ctx context.Context, repoIDsToLatestCommitS repoStatuses[status.RepoID] = append(repoStatuses[status.RepoID], status) } - err = db.GetEngine(ctx).Select("*, min( state ) as state").In("id", idCond). - GroupBy("repo_id").Find(&statuses) + err = db.GetEngine(ctx). + Where(builder.And( + builder.In("id", idCond), + builder.In("state", minStateCond(idCond, "repo_id")), + )).Find(&statuses) if err != nil { return nil, nil, err } @@ -331,8 +339,7 @@ func GetLatestCommitStatusesForRepoCommitIDs(ctx context.Context, repoID int64, Where(builder.Eq{"repo_id": repoID}.And(builder.Or(conds...))). GroupBy("context_hash, sha") - err := db.GetEngine(ctx).In("id", idCond). - OrderBy(db.SearchOrderByIDReverse.String()).Find(&statuses) + err := db.GetEngine(ctx).In("id", idCond).OrderBy(db.SearchOrderByIDReverse.String()).Find(&statuses) if err != nil { return nil, nil, err } @@ -343,8 +350,11 @@ func GetLatestCommitStatusesForRepoCommitIDs(ctx context.Context, repoID int64, repoStatuses[status.SHA] = append(repoStatuses[status.SHA], status) } - err = db.GetEngine(ctx).Select("*, min( state ) as state").In("id", idCond). - GroupBy("sha").Find(&statuses) + err = db.GetEngine(ctx). + Where(builder.And( + builder.In("id", idCond), + builder.In("state", minStateCond(idCond, "sha")), + )).Find(&statuses) if err != nil { return nil, nil, err } @@ -357,6 +367,16 @@ func GetLatestCommitStatusesForRepoCommitIDs(ctx context.Context, repoID int64, return repoStatuses, repoStatus, err } +func minStateCond(idCond *builder.Builder, groupBy string) *builder.Builder { + cond := builder.Select("min( state ) as state").From("commit_status"). + Where(builder.In("id", idCond)) + + if groupBy != "" { + cond = cond.GroupBy(groupBy) + } + return cond +} + // FindRepoRecentCommitStatusContexts returns repository's recent commit status contexts func FindRepoRecentCommitStatusContexts(ctx context.Context, repoID int64, before time.Duration) ([]string, error) { start := timeutil.TimeStampNow().AddDuration(-before) From ec1b8ebe89b4837d1c118cccc6c6253860a535d3 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Tue, 1 Aug 2023 08:35:44 +0000 Subject: [PATCH 8/9] improve --- models/actions/run.go | 41 +++++++++++++++++++++++++++ routers/web/repo/pull.go | 48 +++++++++++++++++++------------- templates/repo/pulls/status.tmpl | 4 +++ 3 files changed, 73 insertions(+), 20 deletions(-) diff --git a/models/actions/run.go b/models/actions/run.go index ab6e319b1cc85..8c28dc59561bf 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -92,6 +92,23 @@ func (run *ActionRun) LoadAttributes(ctx context.Context) error { return nil } + if err := run.LoadRepo(ctx); err != nil { + return err + } + + if err := run.LoadTriggerUser(ctx); err != nil { + return err + } + + return nil +} + +// LoadRepo load Repo if not loaded +func (run *ActionRun) LoadRepo(ctx context.Context) error { + if run == nil { + return nil + } + if run.Repo == nil { repo, err := repo_model.GetRepositoryByID(ctx, run.RepoID) if err != nil { @@ -103,6 +120,15 @@ func (run *ActionRun) LoadAttributes(ctx context.Context) error { return err } + return nil +} + +// LoadTriggerUser load TriggerUser if not loaded +func (run *ActionRun) LoadTriggerUser(ctx context.Context) error { + if run == nil { + return nil + } + if run.TriggerUser == nil { u, err := user_model.GetPossibleUserByID(ctx, run.TriggerUserID) if err != nil { @@ -332,6 +358,21 @@ func GetRunByIndex(ctx context.Context, repoID, index int64) (*ActionRun, error) return run, nil } +func GetRunByCommitSHA(ctx context.Context, repoID int64, commitSHA string) (*ActionRun, error) { + run := &ActionRun{ + RepoID: repoID, + CommitSHA: commitSHA, + } + has, err := db.GetEngine(ctx).Get(run) + if err != nil { + return nil, err + } else if !has { + return nil, fmt.Errorf("run with commitSHA %d %d: %w", repoID, commitSHA, util.ErrNotExist) + } + + return run, nil +} + func UpdateRun(ctx context.Context, run *ActionRun, cols ...string) error { sess := db.GetEngine(ctx).ID(run.ID) if len(cols) > 0 { diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 94657f20fd035..c066d9d4a400d 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -16,6 +16,7 @@ import ( "time" "code.gitea.io/gitea/models" + actions_model "code.gitea.io/gitea/models/actions" activities_model "code.gitea.io/gitea/models/activities" "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" @@ -441,6 +442,26 @@ func GetMergedBaseCommitID(ctx *context.Context, issue *issues_model.Issue) stri return baseCommit } +func PrepareCommitStatusInfo(ctx *context.Context, sha string) ([]*git_model.CommitStatus, *git_model.CommitStatus, error) { + commitStatuses, commitStatus, _, err := git_model.GetLatestCommitStatuses(ctx, ctx.Repo.Repository.ID, sha, db.ListOptions{}) + if err != nil { + return nil, nil, err + } + if len(commitStatuses) > 0 { + ctx.Data["LatestCommitStatuses"] = commitStatuses + ctx.Data["LatestCommitStatus"] = commitStatus + run, err := actions_model.GetRunByCommitSHA(ctx, ctx.Repo.Repository.ID, sha) + if err != nil { + return nil, nil, err + } + if err := run.LoadRepo(ctx); err != nil { + return nil, nil, err + } + ctx.Data["TargetRunURL"] = run.Link() + } + return commitStatuses, commitStatus, nil +} + // PrepareMergedViewPullInfo show meta information for a merged pull request view page func PrepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.CompareInfo { pull := issue.PullRequest @@ -469,15 +490,11 @@ func PrepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue) if len(compareInfo.Commits) != 0 { sha := compareInfo.Commits[0].ID.String() - commitStatuses, commitStatus, _, err := git_model.GetLatestCommitStatuses(ctx, ctx.Repo.Repository.ID, sha, db.ListOptions{}) - if err != nil { - ctx.ServerError("GetLatestCommitStatuses", err) + + if _, _, err := PrepareCommitStatusInfo(ctx, sha); err != nil { + ctx.ServerError("PrepareCommitStatusInfo", err) return nil } - if len(commitStatuses) != 0 { - ctx.Data["LatestCommitStatuses"] = commitStatuses - ctx.Data["LatestCommitStatus"] = commitStatus - } } return compareInfo @@ -531,15 +548,10 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C ctx.ServerError(fmt.Sprintf("GetRefCommitID(%s)", pull.GetGitRefName()), err) return nil } - commitStatuses, commitStatus, _, err := git_model.GetLatestCommitStatuses(ctx, repo.ID, sha, db.ListOptions{}) - if err != nil { - ctx.ServerError("GetLatestCommitStatuses", err) + if _, _, err := PrepareCommitStatusInfo(ctx, sha); err != nil { + ctx.ServerError("PrepareCommitStatusInfo", err) return nil } - if len(commitStatuses) > 0 { - ctx.Data["LatestCommitStatuses"] = commitStatuses - ctx.Data["LatestCommitStatus"] = commitStatus - } compareInfo, err := baseGitRepo.GetCompareInfo(pull.BaseRepo.RepoPath(), pull.MergeBase, pull.GetGitRefName(), false, false) @@ -623,15 +635,11 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C return nil } - commitStatuses, commitStatus, _, err := git_model.GetLatestCommitStatuses(ctx, repo.ID, sha, db.ListOptions{}) + commitStatuses, commitStatus, err := PrepareCommitStatusInfo(ctx, sha) if err != nil { - ctx.ServerError("GetLatestCommitStatuses", err) + ctx.ServerError("PrepareCommitStatusInfo", err) return nil } - if len(commitStatuses) > 0 { - ctx.Data["LatestCommitStatuses"] = commitStatuses - ctx.Data["LatestCommitStatus"] = commitStatus - } if pb != nil && pb.EnableStatusCheck { ctx.Data["is_context_required"] = func(context string) bool { diff --git a/templates/repo/pulls/status.tmpl b/templates/repo/pulls/status.tmpl index 416e620b05e0f..417e6985341c8 100644 --- a/templates/repo/pulls/status.tmpl +++ b/templates/repo/pulls/status.tmpl @@ -12,7 +12,11 @@ {{else}} {{$.locale.Tr "repo.pulls.status_checking"}} {{end}} + + +
{{end}} {{range $.LatestCommitStatuses}} From 30b5b1890593c353d9e5d6939d5f187c0a76d8c0 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Tue, 1 Aug 2023 09:00:31 +0000 Subject: [PATCH 9/9] fix lint --- models/actions/run.go | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/models/actions/run.go b/models/actions/run.go index 8c28dc59561bf..f0040b2765f0a 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -96,11 +96,7 @@ func (run *ActionRun) LoadAttributes(ctx context.Context) error { return err } - if err := run.LoadTriggerUser(ctx); err != nil { - return err - } - - return nil + return run.LoadTriggerUser(ctx) } // LoadRepo load Repo if not loaded @@ -116,11 +112,7 @@ func (run *ActionRun) LoadRepo(ctx context.Context) error { } run.Repo = repo } - if err := run.Repo.LoadAttributes(ctx); err != nil { - return err - } - - return nil + return run.Repo.LoadAttributes(ctx) } // LoadTriggerUser load TriggerUser if not loaded @@ -367,7 +359,7 @@ func GetRunByCommitSHA(ctx context.Context, repoID int64, commitSHA string) (*Ac if err != nil { return nil, err } else if !has { - return nil, fmt.Errorf("run with commitSHA %d %d: %w", repoID, commitSHA, util.ErrNotExist) + return nil, fmt.Errorf("run with commitSHA %d %s: %w", repoID, commitSHA, util.ErrNotExist) } return run, nil