Replace index with id in actions routes (#36842)
This PR migrates the web Actions run/job routes from index-based `runIndex` or `jobIndex` to database IDs. **⚠️ BREAKING ⚠️**: Existing saved links/bookmarks that use the old index-based URLs will no longer resolve after this change. Improvements of this change: - Previously, `jobIndex` depended on list order, making it hard to locate a specific job. Using `jobID` provides stable addressing. - Web routes now align with API, which already use IDs. - Behavior is closer to GitHub, which exposes run/job IDs in URLs. - Provides a cleaner base for future features without relying on list order. - #36388 this PR improves the support for reusable workflows. If a job uses a reusable workflow, it may contain multiple child jobs, which makes relying on job index to locate a job much more complicated --------- Signed-off-by: Zettat123 <zettat123@gmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
Zettat123
GitHub
+9
@@ -0,0 +1,9 @@
|
||||
# type ActionRun struct {
|
||||
# ID int64 `xorm:"pk autoincr"`
|
||||
# RepoID int64 `xorm:"index"`
|
||||
# Index int64
|
||||
# }
|
||||
-
|
||||
id: 106
|
||||
repo_id: 1
|
||||
index: 7
|
||||
+10
@@ -0,0 +1,10 @@
|
||||
# type ActionRunJob struct {
|
||||
# ID int64 `xorm:"pk autoincr"`
|
||||
# RunID int64 `xorm:"index"`
|
||||
# }
|
||||
-
|
||||
id: 530
|
||||
run_id: 106
|
||||
-
|
||||
id: 531
|
||||
run_id: 106
|
||||
+29
@@ -0,0 +1,29 @@
|
||||
# type CommitStatus struct {
|
||||
# ID int64 `xorm:"pk autoincr"`
|
||||
# RepoID int64 `xorm:"index"`
|
||||
# TargetURL string
|
||||
# }
|
||||
-
|
||||
id: 10
|
||||
repo_id: 1
|
||||
target_url: /testuser/repo1/actions/runs/7/jobs/0
|
||||
-
|
||||
id: 11
|
||||
repo_id: 1
|
||||
target_url: /testuser/repo1/actions/runs/7/jobs/1
|
||||
-
|
||||
id: 12
|
||||
repo_id: 1
|
||||
target_url: /otheruser/badrepo/actions/runs/7/jobs/0
|
||||
-
|
||||
id: 13
|
||||
repo_id: 1
|
||||
target_url: /testuser/repo1/actions/runs/10/jobs/0
|
||||
-
|
||||
id: 14
|
||||
repo_id: 1
|
||||
target_url: /testuser/repo1/actions/runs/7/jobs/3
|
||||
-
|
||||
id: 15
|
||||
repo_id: 1
|
||||
target_url: https://ci.example.com/build/123
|
||||
+19
@@ -0,0 +1,19 @@
|
||||
# type CommitStatusSummary struct {
|
||||
# ID int64 `xorm:"pk autoincr"`
|
||||
# RepoID int64 `xorm:"index"`
|
||||
# SHA string `xorm:"VARCHAR(64) NOT NULL"`
|
||||
# State string `xorm:"VARCHAR(7) NOT NULL"`
|
||||
# TargetURL string
|
||||
# }
|
||||
-
|
||||
id: 20
|
||||
repo_id: 1
|
||||
sha: "012345"
|
||||
state: success
|
||||
target_url: /testuser/repo1/actions/runs/7/jobs/0
|
||||
-
|
||||
id: 21
|
||||
repo_id: 1
|
||||
sha: "678901"
|
||||
state: success
|
||||
target_url: https://ci.example.com/build/123
|
||||
+9
@@ -0,0 +1,9 @@
|
||||
# type Repository struct {
|
||||
# ID int64 `xorm:"pk autoincr"`
|
||||
# OwnerName string
|
||||
# Name string
|
||||
# }
|
||||
-
|
||||
id: 1
|
||||
owner_name: testuser
|
||||
name: repo1
|
||||
@@ -400,6 +400,7 @@ func prepareMigrationTasks() []*migration {
|
||||
newMigration(323, "Add support for actions concurrency", v1_26.AddActionsConcurrency),
|
||||
newMigration(324, "Fix closed milestone completeness for milestones with no issues", v1_26.FixClosedMilestoneCompleteness),
|
||||
newMigration(325, "Fix missed repo_id when migrate attachments", v1_26.FixMissedRepoIDWhenMigrateAttachments),
|
||||
newMigration(326, "Migrate commit status target URL to use run ID and job ID", v1_26.FixCommitStatusTargetURLToUseRunAndJobID),
|
||||
}
|
||||
return preparedMigrations
|
||||
}
|
||||
|
||||
@@ -0,0 +1,216 @@
|
||||
// Copyright 2026 The Gitea Authors. All rights reserved.
|
||||
// SPDX-License-Identifier: MIT
|
||||
|
||||
package v1_26
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/url"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"code.gitea.io/gitea/modules/log"
|
||||
"code.gitea.io/gitea/modules/setting"
|
||||
|
||||
"xorm.io/xorm"
|
||||
)
|
||||
|
||||
const actionsRunPath = "/actions/runs/"
|
||||
|
||||
type migrationRepository struct {
|
||||
ID int64
|
||||
OwnerName string
|
||||
Name string
|
||||
}
|
||||
|
||||
type migrationActionRun struct {
|
||||
ID int64
|
||||
RepoID int64
|
||||
Index int64
|
||||
}
|
||||
|
||||
type migrationActionRunJob struct {
|
||||
ID int64
|
||||
RunID int64
|
||||
}
|
||||
|
||||
type migrationCommitStatus struct {
|
||||
ID int64
|
||||
RepoID int64
|
||||
TargetURL string
|
||||
}
|
||||
|
||||
func FixCommitStatusTargetURLToUseRunAndJobID(x *xorm.Engine) error {
|
||||
runByIndexCache := make(map[int64]map[int64]*migrationActionRun)
|
||||
jobsByRunIDCache := make(map[int64][]int64)
|
||||
repoLinkCache := make(map[int64]string)
|
||||
|
||||
if err := migrateCommitStatusTargetURL(x, "commit_status", runByIndexCache, jobsByRunIDCache, repoLinkCache); err != nil {
|
||||
return err
|
||||
}
|
||||
return migrateCommitStatusTargetURL(x, "commit_status_summary", runByIndexCache, jobsByRunIDCache, repoLinkCache)
|
||||
}
|
||||
|
||||
func migrateCommitStatusTargetURL(
|
||||
x *xorm.Engine,
|
||||
table string,
|
||||
runByIndexCache map[int64]map[int64]*migrationActionRun,
|
||||
jobsByRunIDCache map[int64][]int64,
|
||||
repoLinkCache map[int64]string,
|
||||
) error {
|
||||
const batchSize = 500
|
||||
var lastID int64
|
||||
|
||||
for {
|
||||
var rows []migrationCommitStatus
|
||||
sess := x.Table(table).
|
||||
Where("target_url LIKE ?", "%"+actionsRunPath+"%").
|
||||
And("id > ?", lastID).
|
||||
Asc("id").
|
||||
Limit(batchSize)
|
||||
if err := sess.Find(&rows); err != nil {
|
||||
return fmt.Errorf("query %s: %w", table, err)
|
||||
}
|
||||
if len(rows) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
for _, row := range rows {
|
||||
lastID = row.ID
|
||||
if row.TargetURL == "" {
|
||||
continue
|
||||
}
|
||||
|
||||
repoLink, err := getRepoLinkCached(x, repoLinkCache, row.RepoID)
|
||||
if err != nil || repoLink == "" {
|
||||
if err != nil {
|
||||
log.Warn("convert %s id=%d getRepoLinkCached: %v", table, row.ID, err)
|
||||
} else {
|
||||
log.Warn("convert %s id=%d: repo=%d not found", table, row.ID, row.RepoID)
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
runNum, jobNum, ok := parseTargetURL(row.TargetURL, repoLink)
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
|
||||
run, err := getRunByIndexCached(x, runByIndexCache, row.RepoID, runNum)
|
||||
if err != nil || run == nil {
|
||||
if err != nil {
|
||||
log.Warn("convert %s id=%d getRunByIndexCached: %v", table, row.ID, err)
|
||||
} else {
|
||||
log.Warn("convert %s id=%d: run not found for repo_id=%d run_index=%d", table, row.ID, row.RepoID, runNum)
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
jobID, ok, err := getJobIDByIndexCached(x, jobsByRunIDCache, run.ID, jobNum)
|
||||
if err != nil || !ok {
|
||||
if err != nil {
|
||||
log.Warn("convert %s id=%d getJobIDByIndexCached: %v", table, row.ID, err)
|
||||
} else {
|
||||
log.Warn("convert %s id=%d: job not found for run_id=%d job_index=%d", table, row.ID, run.ID, jobNum)
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
oldURL := row.TargetURL
|
||||
newURL := fmt.Sprintf("%s%s%d/jobs/%d", repoLink, actionsRunPath, run.ID, jobID) // expect: {repo_link}/actions/runs/{run_id}/jobs/{job_id}
|
||||
if oldURL == newURL {
|
||||
continue
|
||||
}
|
||||
|
||||
if _, err := x.Table(table).ID(row.ID).Cols("target_url").Update(&migrationCommitStatus{TargetURL: newURL}); err != nil {
|
||||
return fmt.Errorf("update %s id=%d target_url from %s to %s: %w", table, row.ID, oldURL, newURL, err)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func getRepoLinkCached(x *xorm.Engine, cache map[int64]string, repoID int64) (string, error) {
|
||||
if link, ok := cache[repoID]; ok {
|
||||
return link, nil
|
||||
}
|
||||
repo := &migrationRepository{}
|
||||
has, err := x.Table("repository").Where("id=?", repoID).Get(repo)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
if !has {
|
||||
cache[repoID] = ""
|
||||
return "", nil
|
||||
}
|
||||
link := setting.AppSubURL + "/" + url.PathEscape(repo.OwnerName) + "/" + url.PathEscape(repo.Name)
|
||||
cache[repoID] = link
|
||||
return link, nil
|
||||
}
|
||||
|
||||
func getRunByIndexCached(x *xorm.Engine, cache map[int64]map[int64]*migrationActionRun, repoID, runIndex int64) (*migrationActionRun, error) {
|
||||
if repoCache, ok := cache[repoID]; ok {
|
||||
if run, ok := repoCache[runIndex]; ok {
|
||||
if run == nil {
|
||||
return nil, fmt.Errorf("run repo_id=%d run_index=%d not found", repoID, runIndex)
|
||||
}
|
||||
return run, nil
|
||||
}
|
||||
}
|
||||
|
||||
var run migrationActionRun
|
||||
has, err := x.Table("action_run").Where("repo_id=? AND `index`=?", repoID, runIndex).Get(&run)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if !has {
|
||||
if cache[repoID] == nil {
|
||||
cache[repoID] = make(map[int64]*migrationActionRun)
|
||||
}
|
||||
cache[repoID][runIndex] = nil
|
||||
return nil, fmt.Errorf("run repo_id=%d run_index=%d not found", repoID, runIndex)
|
||||
}
|
||||
if cache[repoID] == nil {
|
||||
cache[repoID] = make(map[int64]*migrationActionRun)
|
||||
}
|
||||
cache[repoID][runIndex] = &run
|
||||
return &run, nil
|
||||
}
|
||||
|
||||
func getJobIDByIndexCached(x *xorm.Engine, cache map[int64][]int64, runID, jobIndex int64) (int64, bool, error) {
|
||||
jobIDs, ok := cache[runID]
|
||||
if !ok {
|
||||
var jobs []migrationActionRunJob
|
||||
if err := x.Table("action_run_job").Where("run_id=?", runID).Asc("id").Cols("id").Find(&jobs); err != nil {
|
||||
return 0, false, err
|
||||
}
|
||||
jobIDs = make([]int64, 0, len(jobs))
|
||||
for _, job := range jobs {
|
||||
jobIDs = append(jobIDs, job.ID)
|
||||
}
|
||||
cache[runID] = jobIDs
|
||||
}
|
||||
if jobIndex < 0 || jobIndex >= int64(len(jobIDs)) {
|
||||
return 0, false, nil
|
||||
}
|
||||
return jobIDs[jobIndex], true, nil
|
||||
}
|
||||
|
||||
func parseTargetURL(targetURL, repoLink string) (runNum, jobNum int64, ok bool) {
|
||||
prefix := repoLink + actionsRunPath
|
||||
if !strings.HasPrefix(targetURL, prefix) {
|
||||
return 0, 0, false
|
||||
}
|
||||
rest := targetURL[len(prefix):]
|
||||
|
||||
parts := strings.Split(rest, "/") // expect: {run_num}/jobs/{job_num}
|
||||
if len(parts) == 3 && parts[1] == "jobs" {
|
||||
runNum, err1 := strconv.ParseInt(parts[0], 10, 64)
|
||||
jobNum, err2 := strconv.ParseInt(parts[2], 10, 64)
|
||||
if err1 != nil || err2 != nil {
|
||||
return 0, 0, false
|
||||
}
|
||||
return runNum, jobNum, true
|
||||
}
|
||||
|
||||
return 0, 0, false
|
||||
}
|
||||
@@ -0,0 +1,104 @@
|
||||
// Copyright 2026 The Gitea Authors. All rights reserved.
|
||||
// SPDX-License-Identifier: MIT
|
||||
|
||||
package v1_26
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"code.gitea.io/gitea/models/migrations/base"
|
||||
"code.gitea.io/gitea/modules/setting"
|
||||
"code.gitea.io/gitea/modules/test"
|
||||
|
||||
_ "code.gitea.io/gitea/models/actions"
|
||||
_ "code.gitea.io/gitea/models/git"
|
||||
_ "code.gitea.io/gitea/models/repo"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
"xorm.io/xorm"
|
||||
)
|
||||
|
||||
func Test_FixCommitStatusTargetURLToUseRunAndJobID(t *testing.T) {
|
||||
defer test.MockVariableValue(&setting.AppSubURL, "")()
|
||||
|
||||
type Repository struct {
|
||||
ID int64 `xorm:"pk autoincr"`
|
||||
OwnerName string
|
||||
Name string
|
||||
}
|
||||
|
||||
type ActionRun struct {
|
||||
ID int64 `xorm:"pk autoincr"`
|
||||
RepoID int64 `xorm:"index"`
|
||||
Index int64
|
||||
}
|
||||
|
||||
type ActionRunJob struct {
|
||||
ID int64 `xorm:"pk autoincr"`
|
||||
RunID int64 `xorm:"index"`
|
||||
}
|
||||
|
||||
type CommitStatus struct {
|
||||
ID int64 `xorm:"pk autoincr"`
|
||||
RepoID int64 `xorm:"index"`
|
||||
TargetURL string
|
||||
}
|
||||
|
||||
type CommitStatusSummary struct {
|
||||
ID int64 `xorm:"pk autoincr"`
|
||||
RepoID int64 `xorm:"index"`
|
||||
SHA string `xorm:"VARCHAR(64) NOT NULL"`
|
||||
State string `xorm:"VARCHAR(7) NOT NULL"`
|
||||
TargetURL string
|
||||
}
|
||||
|
||||
x, deferable := base.PrepareTestEnv(t, 0,
|
||||
new(Repository),
|
||||
new(ActionRun),
|
||||
new(ActionRunJob),
|
||||
new(CommitStatus),
|
||||
new(CommitStatusSummary),
|
||||
)
|
||||
defer deferable()
|
||||
|
||||
newURL1 := "/testuser/repo1/actions/runs/106/jobs/530"
|
||||
newURL2 := "/testuser/repo1/actions/runs/106/jobs/531"
|
||||
|
||||
invalidWrongRepo := "/otheruser/badrepo/actions/runs/7/jobs/0"
|
||||
invalidNonexistentRun := "/testuser/repo1/actions/runs/10/jobs/0"
|
||||
invalidNonexistentJob := "/testuser/repo1/actions/runs/7/jobs/3"
|
||||
externalTargetURL := "https://ci.example.com/build/123"
|
||||
|
||||
require.NoError(t, FixCommitStatusTargetURLToUseRunAndJobID(x))
|
||||
|
||||
cases := []struct {
|
||||
table string
|
||||
id int64
|
||||
want string
|
||||
}{
|
||||
{table: "commit_status", id: 10, want: newURL1},
|
||||
{table: "commit_status", id: 11, want: newURL2},
|
||||
{table: "commit_status", id: 12, want: invalidWrongRepo},
|
||||
{table: "commit_status", id: 13, want: invalidNonexistentRun},
|
||||
{table: "commit_status", id: 14, want: invalidNonexistentJob},
|
||||
{table: "commit_status", id: 15, want: externalTargetURL},
|
||||
{table: "commit_status_summary", id: 20, want: newURL1},
|
||||
{table: "commit_status_summary", id: 21, want: externalTargetURL},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
assertTargetURL(t, x, tc.table, tc.id, tc.want)
|
||||
}
|
||||
}
|
||||
|
||||
func assertTargetURL(t *testing.T, x *xorm.Engine, table string, id int64, want string) {
|
||||
t.Helper()
|
||||
|
||||
var row struct {
|
||||
TargetURL string
|
||||
}
|
||||
has, err := x.Table(table).Where("id=?", id).Cols("target_url").Get(&row)
|
||||
require.NoError(t, err)
|
||||
require.Truef(t, has, "row not found: table=%s id=%d", table, id)
|
||||
require.Equal(t, want, row.TargetURL)
|
||||
}
|
||||
Reference in New Issue
Block a user