mirror of
https://github.com/go-gitea/gitea.git
synced 2026-10-03 00:02:43 +02:00
fix(api): add index tiebreaker to commit status ordering (#39508)
Commit status list orders only by `created_unix`/`updated_unix`, which
have 1-second resolution while CI often posts many statuses per second.
With LIMIT/OFFSET paging, databases (e.g. PostgreSQL using a Sort plan)
may order tied rows differently per page, so `GET
/repos/{owner}/{repo}/commits/{ref}/statuses` returns some statuses
twice and never returns others.
This became visible after https://github.com/go-gitea/gitea/pull/36521
made requests without `page` paginated. Clients like Renovate that page
until `X-Total-Count` can miss a context's newest status and see a stale
`pending`, blocking automerge.
Fix: add `index` (unique per commit) as a tiebreaker to the
timestamp-based orders.
Co-authored-by: silverwind <me@silverwind.io>
This commit is contained in:
1 parent
9b5c87a6b6
commit
aae0a218c3
2 files changed
+11
-24
No files matched your search
@@ -311,17 +311,17 @@ func (opts *CommitStatusOptions) ToConds() builder.Cond {
|
||||
func (opts *CommitStatusOptions) ToOrders() string {
|
||||
switch opts.SortType {
|
||||
case "oldest":
|
||||
return "created_unix ASC"
|
||||
return "created_unix ASC, `index` ASC"
|
||||
case "recentupdate":
|
||||
return "updated_unix DESC"
|
||||
return "updated_unix DESC, `index` DESC"
|
||||
case "leastupdate":
|
||||
return "updated_unix ASC"
|
||||
return "updated_unix ASC, `index` ASC"
|
||||
case "leastindex":
|
||||
return "`index` DESC"
|
||||
case "highestindex":
|
||||
return "`index` ASC"
|
||||
default:
|
||||
return "created_unix DESC"
|
||||
return "created_unix DESC, `index` DESC" // timestamps have 1s resolution, `index` keeps paging stable
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -32,28 +32,15 @@ func TestGetCommitStatuses(t *testing.T) {
|
||||
})
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, 5, int(maxResults))
|
||||
assert.Len(t, statuses, 5)
|
||||
|
||||
assert.Equal(t, "ci/awesomeness", statuses[0].Context)
|
||||
assert.Equal(t, commitstatus.CommitStatusPending, statuses[0].State)
|
||||
var indexes []int64
|
||||
for _, status := range statuses {
|
||||
indexes = append(indexes, status.Index)
|
||||
}
|
||||
assert.Equal(t, []int64{5, 4, 3, 2, 1}, indexes)
|
||||
assert.Equal(t, "deploy/awesomeness", statuses[0].Context)
|
||||
assert.Equal(t, commitstatus.CommitStatusError, statuses[0].State)
|
||||
assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[0].APIURL(t.Context()))
|
||||
|
||||
assert.Equal(t, "cov/awesomeness", statuses[1].Context)
|
||||
assert.Equal(t, commitstatus.CommitStatusWarning, statuses[1].State)
|
||||
assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[1].APIURL(t.Context()))
|
||||
|
||||
assert.Equal(t, "cov/awesomeness", statuses[2].Context)
|
||||
assert.Equal(t, commitstatus.CommitStatusSuccess, statuses[2].State)
|
||||
assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[2].APIURL(t.Context()))
|
||||
|
||||
assert.Equal(t, "ci/awesomeness", statuses[3].Context)
|
||||
assert.Equal(t, commitstatus.CommitStatusFailure, statuses[3].State)
|
||||
assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[3].APIURL(t.Context()))
|
||||
|
||||
assert.Equal(t, "deploy/awesomeness", statuses[4].Context)
|
||||
assert.Equal(t, commitstatus.CommitStatusError, statuses[4].State)
|
||||
assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[4].APIURL(t.Context()))
|
||||
|
||||
statuses, maxResults, err = db.FindAndCount[git_model.CommitStatus](t.Context(), &git_model.CommitStatusOptions{
|
||||
ListOptions: db.ListOptions{Page: 2, PageSize: 50},
|
||||
RepoID: repo1.ID,
|
||||
|
||||
Reference in new issue
Block a user