[WIP] Improve CommitStatus

models/actions/run.go

@@ -92,15 +92,33 @@ func (run *ActionRun) LoadAttributes(ctx context.Context) error {
 		return nil
 	}
 
+	if err := run.LoadRepo(ctx); err != nil {
+		return err
+	}
+
+	return run.LoadTriggerUser(ctx)
+}
+
+// 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 {
 			return err
 		}
 		run.Repo = repo
 	}
-	if err := run.Repo.LoadAttributes(ctx); err != nil {
-		return err
+	return run.Repo.LoadAttributes(ctx)
+}
+
+// LoadTriggerUser load TriggerUser if not loaded
+func (run *ActionRun) LoadTriggerUser(ctx context.Context) error {
+	if run == nil {
+		return nil
 	}
 
 	if run.TriggerUser == nil {
@@ -332,6 +350,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 %s: %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 {

models/fixtures/commit_status.yml

@@ -2,53 +2,42 @@
   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
   context: ci/awesomeness
   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

models/fixtures/commit_status_index.yml

@@ -2,4 +2,4 @@
   id: 1
   repo_id: 1
   sha: "1234123412341234123412341234123412341234"
-  max_index: 5
+  max_index: 4

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,124 @@ 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")
-	if !listOptions.IsListAll() {
-		sess = db.SetSessionPagination(sess, &listOptions)
-	}
-	count, err := sess.FindAndCount(&ids)
+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.GetEngine(ctx).In("id", idCond).OrderBy(db.SearchOrderByIDReverse.String())
+
+	count, err := db.SetSessionPagination(sess, &listOptions).FindAndCount(&statuses)
 	if err != nil {
-		return nil, count, err
-	}
-	statuses := make([]*CommitStatus, 0, len(ids))
-	if len(ids) == 0 {
-		return statuses, count, nil
+		return nil, nil, count, err
 	}
-	return statuses, count, db.GetEngine(ctx).In("id", ids).Find(&statuses)
-}
 
-// 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
+	status := &CommitStatus{}
+	_, 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
 	}
 
-	results := make([]result, 0, len(repoIDsToLatestCommitSHAs))
+	return statuses, status, count, nil
+}
 
-	sess := db.GetEngine(ctx).Table(&CommitStatus{})
+// GetLatestCommitStatusForPairs returns all statuses with a unique context for a given list of repo-sha pairs
+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).
+		Where(builder.And(
+			builder.In("id", idCond),
+			builder.In("state", minStateCond(idCond, "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
-		}
+	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
+	}
 
-		// Group the statuses by repo ID
-		for _, status := range statuses {
-			repoStatuses[status.SHA] = append(repoStatuses[status.SHA], status)
-		}
+	repoStatus := make(map[string]*CommitStatus)
+	// Group the statuses by repo ID
+	for _, status := range statuses {
+		repoStatus[status.SHA] = status
 	}
+	return repoStatuses, repoStatus, err
+}
 
-	return repoStatuses, nil
+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
@@ -482,12 +466,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)