Skip to content

Commit 2130399

Browse files
lunnylunny
committed
Refactor commit status state
1 parent 7149c9c commit 2130399

File tree

18 files changed

+437
-272
lines changed

18 files changed

+437
-272
lines changed

‎models/git/commit_status.go‎

Lines changed: 27 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ import (
1717
"code.gitea.io/gitea/models/db"
1818
repo_model "code.gitea.io/gitea/models/repo"
1919
user_model "code.gitea.io/gitea/models/user"
20+
"code.gitea.io/gitea/modules/commitstatus"
2021
"code.gitea.io/gitea/modules/git"
2122
"code.gitea.io/gitea/modules/log"
2223
"code.gitea.io/gitea/modules/setting"
23-
api "code.gitea.io/gitea/modules/structs"
2424
"code.gitea.io/gitea/modules/timeutil"
2525
"code.gitea.io/gitea/modules/translation"
2626

@@ -30,17 +30,17 @@ import (
3030

3131
// CommitStatus holds a single Status of a single Commit
3232
type CommitStatus struct {
33-
ID int64 `xorm:"pk autoincr"`
34-
Index int64 `xorm:"INDEX UNIQUE(repo_sha_index)"`
35-
RepoID int64 `xorm:"INDEX UNIQUE(repo_sha_index)"`
36-
Repo *repo_model.Repository `xorm:"-"`
37-
State api.CommitStatusState `xorm:"VARCHAR(7) NOT NULL"`
38-
SHA string `xorm:"VARCHAR(64) NOT NULL INDEX UNIQUE(repo_sha_index)"`
39-
TargetURL string `xorm:"TEXT"`
40-
Description string `xorm:"TEXT"`
41-
ContextHash string `xorm:"VARCHAR(64) index"`
42-
Context string `xorm:"TEXT"`
43-
Creator *user_model.User `xorm:"-"`
33+
ID int64 `xorm:"pk autoincr"`
34+
Index int64 `xorm:"INDEX UNIQUE(repo_sha_index)"`
35+
RepoID int64 `xorm:"INDEX UNIQUE(repo_sha_index)"`
36+
Repo *repo_model.Repository `xorm:"-"`
37+
State commitstatus.CommitStatusState `xorm:"VARCHAR(7) NOT NULL"`
38+
SHA string `xorm:"VARCHAR(64) NOT NULL INDEX UNIQUE(repo_sha_index)"`
39+
TargetURL string `xorm:"TEXT"`
40+
Description string `xorm:"TEXT"`
41+
ContextHash string `xorm:"VARCHAR(64) index"`
42+
Context string `xorm:"TEXT"`
43+
Creator *user_model.User `xorm:"-"`
4444
CreatorID int64
4545

4646
CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
@@ -230,28 +230,25 @@ func (status *CommitStatus) HideActionsURL(ctx context.Context) {
230230

231231
// CalcCommitStatus returns commit status state via some status, the commit statues should order by id desc
232232
func CalcCommitStatus(statuses []*CommitStatus) *CommitStatus {
233-
// This function is widely used, but it is not quite right.
234-
// Ideally it should return something like "CommitStatusSummary" with properly aggregated state.
235-
// GitHub's behavior: if all statuses are "skipped", GitHub will return "success" as the combined status.
236-
var lastStatus *CommitStatus
237-
state := api.CommitStatusSuccess
233+
if len(statuses) == 0 {
234+
return nil
235+
}
236+
237+
states := make(commitstatus.CommitStatusStates, 0, len(statuses))
238+
targetURL := ""
238239
for _, status := range statuses {
239-
if state == status.State || status.State.HasHigherPriorityThan(state) {
240-
state = status.State
241-
lastStatus = status
240+
states = append(states, status.State)
241+
if status.TargetURL != "" {
242+
targetURL = status.TargetURL
242243
}
243244
}
244-
if lastStatus == nil {
245-
if len(statuses) > 0 {
246-
// FIXME: a bad case: Gitea just returns the first commit status, its status is "skipped" in this case.
247-
lastStatus = statuses[0]
248-
} else {
249-
// FIXME: another bad case: if the "statuses" slice is empty, the returned value is an invalid CommitStatus, all its fields are empty.
250-
// Frontend code (tmpl&vue) sometimes depend on the empty fields to skip rendering commit status elements (need to double check in the future)
251-
lastStatus = &CommitStatus{}
252-
}
245+
246+
return &CommitStatus{
247+
RepoID: statuses[0].RepoID,
248+
SHA: statuses[0].SHA,
249+
State: states.Combine(),
250+
TargetURL: targetURL,
253251
}
254-
return lastStatus
255252
}
256253

257254
// CommitStatusOptions holds the options for query commit statuses

‎models/git/commit_status_summary.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,19 @@ import (
77
"context"
88

99
"code.gitea.io/gitea/models/db"
10+
"code.gitea.io/gitea/modules/commitstatus"
1011
"code.gitea.io/gitea/modules/setting"
11-
api "code.gitea.io/gitea/modules/structs"
1212

1313
"xorm.io/builder"
1414
)
1515

1616
// CommitStatusSummary holds the latest commit Status of a single Commit
1717
type CommitStatusSummary struct {
18-
ID int64 `xorm:"pk autoincr"`
19-
RepoID int64 `xorm:"INDEX UNIQUE(repo_id_sha)"`
20-
SHA string `xorm:"VARCHAR(64) NOT NULL INDEX UNIQUE(repo_id_sha)"`
21-
State api.CommitStatusState `xorm:"VARCHAR(7) NOT NULL"`
22-
TargetURL string `xorm:"TEXT"`
18+
ID int64 `xorm:"pk autoincr"`
19+
RepoID int64 `xorm:"INDEX UNIQUE(repo_id_sha)"`
20+
SHA string `xorm:"VARCHAR(64) NOT NULL INDEX UNIQUE(repo_id_sha)"`
21+
State commitstatus.CommitStatusState `xorm:"VARCHAR(7) NOT NULL"`
22+
TargetURL string `xorm:"TEXT"`
2323
}
2424

2525
func init() {

‎models/git/commit_status_test.go‎

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@ import (
1414
repo_model "code.gitea.io/gitea/models/repo"
1515
"code.gitea.io/gitea/models/unittest"
1616
user_model "code.gitea.io/gitea/models/user"
17+
"code.gitea.io/gitea/modules/commitstatus"
1718
"code.gitea.io/gitea/modules/git"
1819
"code.gitea.io/gitea/modules/gitrepo"
19-
"code.gitea.io/gitea/modules/structs"
2020

2121
"github.com/stretchr/testify/assert"
2222
)
@@ -38,23 +38,23 @@ func TestGetCommitStatuses(t *testing.T) {
3838
assert.Len(t, statuses, 5)
3939

4040
assert.Equal(t, "ci/awesomeness", statuses[0].Context)
41-
assert.Equal(t, structs.CommitStatusPending, statuses[0].State)
41+
assert.Equal(t, commitstatus.CommitStatusPending, statuses[0].State)
4242
assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[0].APIURL(db.DefaultContext))
4343

4444
assert.Equal(t, "cov/awesomeness", statuses[1].Context)
45-
assert.Equal(t, structs.CommitStatusWarning, statuses[1].State)
45+
assert.Equal(t, commitstatus.CommitStatusWarning, statuses[1].State)
4646
assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[1].APIURL(db.DefaultContext))
4747

4848
assert.Equal(t, "cov/awesomeness", statuses[2].Context)
49-
assert.Equal(t, structs.CommitStatusSuccess, statuses[2].State)
49+
assert.Equal(t, commitstatus.CommitStatusSuccess, statuses[2].State)
5050
assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[2].APIURL(db.DefaultContext))
5151

5252
assert.Equal(t, "ci/awesomeness", statuses[3].Context)
53-
assert.Equal(t, structs.CommitStatusFailure, statuses[3].State)
53+
assert.Equal(t, commitstatus.CommitStatusFailure, statuses[3].State)
5454
assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[3].APIURL(db.DefaultContext))
5555

5656
assert.Equal(t, "deploy/awesomeness", statuses[4].Context)
57-
assert.Equal(t, structs.CommitStatusError, statuses[4].State)
57+
assert.Equal(t, commitstatus.CommitStatusError, statuses[4].State)
5858
assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[4].APIURL(db.DefaultContext))
5959

6060
statuses, maxResults, err = db.FindAndCount[git_model.CommitStatus](db.DefaultContext, &git_model.CommitStatusOptions{
@@ -75,104 +75,104 @@ func Test_CalcCommitStatus(t *testing.T) {
7575
{
7676
statuses: []*git_model.CommitStatus{
7777
{
78-
State: structs.CommitStatusPending,
78+
State: commitstatus.CommitStatusPending,
7979
},
8080
},
8181
expected: &git_model.CommitStatus{
82-
State: structs.CommitStatusPending,
82+
State: commitstatus.CommitStatusPending,
8383
},
8484
},
8585
{
8686
statuses: []*git_model.CommitStatus{
8787
{
88-
State: structs.CommitStatusSuccess,
88+
State: commitstatus.CommitStatusSuccess,
8989
},
9090
{
91-
State: structs.CommitStatusPending,
91+
State: commitstatus.CommitStatusPending,
9292
},
9393
},
9494
expected: &git_model.CommitStatus{
95-
State: structs.CommitStatusPending,
95+
State: commitstatus.CommitStatusPending,
9696
},
9797
},
9898
{
9999
statuses: []*git_model.CommitStatus{
100100
{
101-
State: structs.CommitStatusSuccess,
101+
State: commitstatus.CommitStatusSuccess,
102102
},
103103
{
104-
State: structs.CommitStatusPending,
104+
State: commitstatus.CommitStatusPending,
105105
},
106106
{
107-
State: structs.CommitStatusSuccess,
107+
State: commitstatus.CommitStatusSuccess,
108108
},
109109
},
110110
expected: &git_model.CommitStatus{
111-
State: structs.CommitStatusPending,
111+
State: commitstatus.CommitStatusPending,
112112
},
113113
},
114114
{
115115
statuses: []*git_model.CommitStatus{
116116
{
117-
State: structs.CommitStatusError,
117+
State: commitstatus.CommitStatusError,
118118
},
119119
{
120-
State: structs.CommitStatusPending,
120+
State: commitstatus.CommitStatusPending,
121121
},
122122
{
123-
State: structs.CommitStatusSuccess,
123+
State: commitstatus.CommitStatusSuccess,
124124
},
125125
},
126126
expected: &git_model.CommitStatus{
127-
State: structs.CommitStatusError,
127+
State: commitstatus.CommitStatusError,
128128
},
129129
},
130130
{
131131
statuses: []*git_model.CommitStatus{
132132
{
133-
State: structs.CommitStatusWarning,
133+
State: commitstatus.CommitStatusWarning,
134134
},
135135
{
136-
State: structs.CommitStatusPending,
136+
State: commitstatus.CommitStatusPending,
137137
},
138138
{
139-
State: structs.CommitStatusSuccess,
139+
State: commitstatus.CommitStatusSuccess,
140140
},
141141
},
142142
expected: &git_model.CommitStatus{
143-
State: structs.CommitStatusWarning,
143+
State: commitstatus.CommitStatusWarning,
144144
},
145145
},
146146
{
147147
statuses: []*git_model.CommitStatus{
148148
{
149-
State: structs.CommitStatusSuccess,
149+
State: commitstatus.CommitStatusSuccess,
150150
},
151151
{
152-
State: structs.CommitStatusSuccess,
152+
State: commitstatus.CommitStatusSuccess,
153153
},
154154
{
155-
State: structs.CommitStatusSuccess,
155+
State: commitstatus.CommitStatusSuccess,
156156
},
157157
},
158158
expected: &git_model.CommitStatus{
159-
State: structs.CommitStatusSuccess,
159+
State: commitstatus.CommitStatusSuccess,
160160
},
161161
},
162162
{
163163
statuses: []*git_model.CommitStatus{
164164
{
165-
State: structs.CommitStatusFailure,
165+
State: commitstatus.CommitStatusFailure,
166166
},
167167
{
168-
State: structs.CommitStatusError,
168+
State: commitstatus.CommitStatusError,
169169
},
170170
{
171-
State: structs.CommitStatusWarning,
171+
State: commitstatus.CommitStatusWarning,
172172
},
173173
},
174174
expected: &git_model.CommitStatus{
175-
State: structs.CommitStatusError,
175+
State: commitstatus.CommitStatusError,
176176
},
177177
},
178178
}
@@ -208,7 +208,7 @@ func TestFindRepoRecentCommitStatusContexts(t *testing.T) {
208208
Creator: user2,
209209
SHA: commit.ID,
210210
CommitStatus: &git_model.CommitStatus{
211-
State: structs.CommitStatusFailure,
211+
State: commitstatus.CommitStatusFailure,
212212
TargetURL: "https://example.com/tests/",
213213
Context: "compliance/lint-backend",
214214
},
@@ -220,7 +220,7 @@ func TestFindRepoRecentCommitStatusContexts(t *testing.T) {
220220
Creator: user2,
221221
SHA: commit.ID,
222222
CommitStatus: &git_model.CommitStatus{
223-
State: structs.CommitStatusSuccess,
223+
State: commitstatus.CommitStatusSuccess,
224224
TargetURL: "https://example.com/tests/",
225225
Context: "compliance/lint-backend",
226226
},
@@ -270,9 +270,9 @@ func TestGetCountLatestCommitStatus(t *testing.T) {
270270
})
271271
assert.NoError(t, err)
272272
assert.Len(t, commitStatuses, 2)
273-
assert.Equal(t, structs.CommitStatusFailure, commitStatuses[0].State)
273+
assert.Equal(t, commitstatus.CommitStatusFailure, commitStatuses[0].State)
274274
assert.Equal(t, "ci/awesomeness", commitStatuses[0].Context)
275-
assert.Equal(t, structs.CommitStatusError, commitStatuses[1].State)
275+
assert.Equal(t, commitstatus.CommitStatusError, commitStatuses[1].State)
276276
assert.Equal(t, "deploy/awesomeness", commitStatuses[1].Context)
277277

278278
count, err := git_model.CountLatestCommitStatus(db.DefaultContext, repo1.ID, sha1)

‎modules/structs/commit_status.go‎ renamed to ‎modules/commitstatus/commit_status.go‎

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
// Copyright 2020 The Gitea Authors. All rights reserved.
22
// SPDX-License-Identifier: MIT
33

4-
package structs
4+
package commitstatus
55

66
// CommitStatusState holds the state of a CommitStatus
7-
// It can be "pending", "success", "error" and "failure"
8-
type CommitStatusState string
7+
type CommitStatusState string //nolint
98

109
const (
1110
// CommitStatusPending is for when the CommitStatus is Pending
@@ -22,25 +21,10 @@ const (
2221
CommitStatusSkipped CommitStatusState = "skipped"
2322
)
2423

25-
var commitStatusPriorities = map[CommitStatusState]int{
26-
CommitStatusError: 0,
27-
CommitStatusFailure: 1,
28-
CommitStatusWarning: 2,
29-
CommitStatusPending: 3,
30-
CommitStatusSuccess: 4,
31-
CommitStatusSkipped: 5,
32-
}
33-
3424
func (css CommitStatusState) String() string {
3525
return string(css)
3626
}
3727

38-
// HasHigherPriorityThan returns true if this state has higher priority than the other
39-
// Undefined states are considered to have the highest priority like CommitStatusError(0)
40-
func (css CommitStatusState) HasHigherPriorityThan(other CommitStatusState) bool {
41-
return commitStatusPriorities[css] < commitStatusPriorities[other]
42-
}
43-
4428
// IsPending represents if commit status state is pending
4529
func (css CommitStatusState) IsPending() bool {
4630
return css == CommitStatusPending
@@ -65,3 +49,32 @@ func (css CommitStatusState) IsFailure() bool {
6549
func (css CommitStatusState) IsWarning() bool {
6650
return css == CommitStatusWarning
6751
}
52+
53+
// IsSkipped represents if commit status state is skipped
54+
func (css CommitStatusState) IsSkipped() bool {
55+
return css == CommitStatusSkipped
56+
}
57+
58+
type CommitStatusStates []CommitStatusState //nolint
59+
60+
// According to https://docs.github.com/en/rest/commits/statuses?apiVersion=2022-11-28#get-the-combined-status-for-a-specific-reference
61+
// > Additionally, a combined state is returned. The state is one of:
62+
// > failure if any of the contexts report as error or failure
63+
// > pending if there are no statuses or a context is pending
64+
// > success if the latest status for all contexts is success
65+
func (css CommitStatusStates) Combine() CommitStatusState {
66+
successCnt := 0
67+
for _, state := range css {
68+
switch {
69+
case state.IsError() || state.IsFailure():
70+
return CommitStatusFailure
71+
case state.IsPending():
72+
case state.IsSuccess() || state.IsWarning() || state.IsSkipped():
73+
successCnt++
74+
}
75+
}
76+
if successCnt > 0 && successCnt == len(css) {
77+
return CommitStatusSuccess
78+
}
79+
return CommitStatusPending
80+
}

0 commit comments

Comments
 (0)