mirror of
https://github.com/go-gitea/gitea.git
synced 2026-08-07 07:16:09 +00:00
fix(actions): make runner list pagination order deterministic (#38313)
The runner-listing API endpoints (`admin`, `org`, `user`, `repo`, and `shared`) return runners in a non-deterministic order across pages. When paging through runners, some runners from page 1 could reappear on page 2 (and others get skipped entirely). The cause is in `FindRunnerOptions.ToOrders()`. Most sort modes order by a **non-unique** column only: - default / `online` / `offline` → `last_online` - `alphabetically` / `reversealphabetically` → `name` When multiple runners tie on the sort key (e.g. every offline runner shares `last_online = 0`, or two runners have the same name), the database is free to return the tied rows in any order between separate queries. Combined with `LIMIT`/`OFFSET` pagination, this means the same runner can land on more than one page. ## Fix Append the unique primary key `id` as a stable tiebreaker to each non-unique sort order: The `newest`/`oldest` modes already sort by the unique `id`, so they are left unchanged.
This commit is contained in:
@@ -251,21 +251,24 @@ func (opts FindRunnerOptions) ToConds() builder.Cond {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (opts FindRunnerOptions) ToOrders() string {
|
func (opts FindRunnerOptions) ToOrders() string {
|
||||||
|
// A unique tiebreaker (id) is appended so that runners sharing the same
|
||||||
|
// last_online or name keep a deterministic order across paginated queries,
|
||||||
|
// otherwise the same runner may appear on more than one page.
|
||||||
switch opts.Sort {
|
switch opts.Sort {
|
||||||
case "online":
|
case "online":
|
||||||
return "last_online DESC"
|
return "last_online DESC, id ASC"
|
||||||
case "offline":
|
case "offline":
|
||||||
return "last_online ASC"
|
return "last_online ASC, id ASC"
|
||||||
case "alphabetically":
|
case "alphabetically":
|
||||||
return "name ASC"
|
return "name ASC, id ASC"
|
||||||
case "reversealphabetically":
|
case "reversealphabetically":
|
||||||
return "name DESC"
|
return "name DESC, id ASC"
|
||||||
case "newest":
|
case "newest":
|
||||||
return "id DESC"
|
return "id DESC"
|
||||||
case "oldest":
|
case "oldest":
|
||||||
return "id ASC"
|
return "id ASC"
|
||||||
}
|
}
|
||||||
return "last_online DESC"
|
return "last_online DESC, id ASC"
|
||||||
}
|
}
|
||||||
|
|
||||||
// GetRunnerByUUID returns a runner via uuid
|
// GetRunnerByUUID returns a runner via uuid
|
||||||
|
|||||||
@@ -0,0 +1,77 @@
|
|||||||
|
// Copyright 2026 The Gitea Authors. All rights reserved.
|
||||||
|
// SPDX-License-Identifier: MIT
|
||||||
|
|
||||||
|
package actions
|
||||||
|
|
||||||
|
import (
|
||||||
|
"fmt"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"gitea.dev/models/db"
|
||||||
|
"gitea.dev/models/unittest"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestFindRunnerOptions_ToOrders_StableTiebreaker(t *testing.T) {
|
||||||
|
// Sorts on a non-unique column must end with the unique id tiebreaker so
|
||||||
|
// pagination is deterministic; without it, runners sharing the same
|
||||||
|
// last_online or name can appear on more than one page. Sorts already on
|
||||||
|
// the unique id need no tiebreaker.
|
||||||
|
expected := map[string]string{
|
||||||
|
"": "last_online DESC, id ASC",
|
||||||
|
"online": "last_online DESC, id ASC",
|
||||||
|
"offline": "last_online ASC, id ASC",
|
||||||
|
"alphabetically": "name ASC, id ASC",
|
||||||
|
"reversealphabetically": "name DESC, id ASC",
|
||||||
|
"newest": "id DESC",
|
||||||
|
"oldest": "id ASC",
|
||||||
|
}
|
||||||
|
for sort, want := range expected {
|
||||||
|
assert.Equal(t, want, FindRunnerOptions{Sort: sort}.ToOrders(), "sort %q", sort)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestFindRunners_PaginationNoDuplicates(t *testing.T) {
|
||||||
|
require.NoError(t, unittest.PrepareTestDatabase())
|
||||||
|
ctx := t.Context()
|
||||||
|
|
||||||
|
// Create several runners that all share the same last_online value so the
|
||||||
|
// primary sort key (last_online) is tied for all of them.
|
||||||
|
const ownerID = 1000
|
||||||
|
const count = 6
|
||||||
|
for i := range count {
|
||||||
|
runner := &ActionRunner{
|
||||||
|
Name: "paginated-runner",
|
||||||
|
UUID: fmt.Sprintf("PAGINATE-TEST-0000-0000-00000000000%d", i),
|
||||||
|
TokenHash: fmt.Sprintf("paginate-test-token-hash-%d", i),
|
||||||
|
OwnerID: ownerID,
|
||||||
|
RepoID: 0,
|
||||||
|
LastOnline: 42,
|
||||||
|
}
|
||||||
|
require.NoError(t, db.Insert(ctx, runner))
|
||||||
|
}
|
||||||
|
|
||||||
|
// Page through the runners and ensure every id is returned exactly once.
|
||||||
|
seen := make(map[int64]int)
|
||||||
|
const pageSize = 2
|
||||||
|
for page := 1; ; page++ {
|
||||||
|
runners, err := db.Find[ActionRunner](ctx, FindRunnerOptions{
|
||||||
|
ListOptions: db.ListOptions{Page: page, PageSize: pageSize},
|
||||||
|
OwnerID: ownerID,
|
||||||
|
})
|
||||||
|
require.NoError(t, err)
|
||||||
|
if len(runners) == 0 {
|
||||||
|
break
|
||||||
|
}
|
||||||
|
for _, r := range runners {
|
||||||
|
seen[r.ID]++
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
assert.Len(t, seen, count, "each runner should be returned exactly once across all pages")
|
||||||
|
for id, n := range seen {
|
||||||
|
assert.Equal(t, 1, n, "runner %d appeared on %d pages", id, n)
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user