diff --git a/models/actions/run.go b/models/actions/run.go index 2d83806b98..fe516d75d4 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -14,8 +14,10 @@ import ( "forgejo.org/models/db" repo_model "forgejo.org/models/repo" user_model "forgejo.org/models/user" + "forgejo.org/modules/cache" "forgejo.org/modules/git" "forgejo.org/modules/json" + "forgejo.org/modules/log" api "forgejo.org/modules/structs" "forgejo.org/modules/timeutil" "forgejo.org/modules/util" @@ -198,29 +200,32 @@ func (run *ActionRun) SetDefaultConcurrencyGroup() { )) } -func updateRepoRunsNumbers(ctx context.Context, repo *repo_model.Repository) error { - _, err := db.GetEngine(ctx).ID(repo.ID). - SetExpr("num_action_runs", - builder.Select("count(*)").From("action_run"). - Where(builder.Eq{"repo_id": repo.ID}), - ). - SetExpr("num_closed_action_runs", - builder.Select("count(*)").From("action_run"). - Where(builder.Eq{ - "repo_id": repo.ID, - }.And( - builder.In("status", - StatusSuccess, - StatusFailure, - StatusCancelled, - StatusSkipped, - ), - ), - ), - ). - Cols("num_action_runs", "num_closed_action_runs"). - Update(repo) - return err +func actionsCountOpenCacheKey(repoID int64) string { + return fmt.Sprintf("Actions:CountOpenActionRuns:%d", repoID) +} + +func RepoNumOpenActions(ctx context.Context, repoID int64) int { + num, err := cache.GetInt(actionsCountOpenCacheKey(repoID), func() (int, error) { + count, err := db.GetEngine(ctx). + Table("action_run"). + Where( + builder.Eq{"repo_id": repoID}.And( + builder.In("status", PendingStatuses()))). + Count() + if err != nil { + return 0, fmt.Errorf("query error: %v", err) + } + return int(count), nil + }) + if err != nil { + log.Error("failed to retrieve NumIssues: %v", err) + return 0 + } + return num +} + +func clearRepoRunCountCache(repo *repo_model.Repository) { + cache.Remove(actionsCountOpenCacheKey(repo.ID)) } // InsertRun inserts a run @@ -252,9 +257,7 @@ func InsertRun(ctx context.Context, run *ActionRun, jobs []*jobparser.SingleWork run.Repo = repo } - if err := updateRepoRunsNumbers(ctx, run.Repo); err != nil { - return err - } + clearRepoRunCountCache(run.Repo) runJobs := make([]*ActionRunJob, 0, len(jobs)) var hasWaiting bool @@ -416,9 +419,7 @@ func UpdateRunWithoutNotification(ctx context.Context, run *ActionRun, cols ...s } run.Repo = repo } - if err := updateRepoRunsNumbers(ctx, run.Repo); err != nil { - return err - } + clearRepoRunCountCache(run.Repo) } return nil diff --git a/models/actions/run_test.go b/models/actions/run_test.go index 834d1595ae..a6973bb14d 100644 --- a/models/actions/run_test.go +++ b/models/actions/run_test.go @@ -6,8 +6,10 @@ package actions import ( "testing" + "forgejo.org/models/db" repo_model "forgejo.org/models/repo" "forgejo.org/models/unittest" + "forgejo.org/modules/cache" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -41,48 +43,41 @@ func TestSetDefaultConcurrencyGroup(t *testing.T) { assert.Equal(t, "refs/heads/main_testing_pull_request__auto", run.ConcurrencyGroup) } -func TestUpdateRepoRunsNumbers(t *testing.T) { +func TestRepoNumOpenActions(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) + err := cache.Init() + require.NoError(t, err) - t.Run("Normal", func(t *testing.T) { - t.Run("Repo 1", func(t *testing.T) { - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - - require.NoError(t, updateRepoRunsNumbers(t.Context(), repo)) - - repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - assert.Equal(t, 1, repo.NumActionRuns) - assert.Equal(t, 1, repo.NumClosedActionRuns) - }) - - t.Run("Repo 4", func(t *testing.T) { - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) - - require.NoError(t, updateRepoRunsNumbers(t.Context(), repo)) - - repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) - assert.Equal(t, 4, repo.NumActionRuns) - assert.Equal(t, 4, repo.NumClosedActionRuns) - }) - - t.Run("Repo 63", func(t *testing.T) { - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 63}) - - require.NoError(t, updateRepoRunsNumbers(t.Context(), repo)) - - repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 63}) - assert.Equal(t, 3, repo.NumActionRuns) - assert.Equal(t, 2, repo.NumClosedActionRuns) - }) + t.Run("Repo 1", func(t *testing.T) { + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + clearRepoRunCountCache(repo) + assert.Equal(t, 0, RepoNumOpenActions(t.Context(), repo.ID)) }) - t.Run("Columns specifc", func(t *testing.T) { - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - repo.Name = "ishouldnotbeupdated" + t.Run("Repo 4", func(t *testing.T) { + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + clearRepoRunCountCache(repo) + assert.Equal(t, 0, RepoNumOpenActions(t.Context(), repo.ID)) + }) - require.NoError(t, updateRepoRunsNumbers(t.Context(), repo)) + t.Run("Repo 63", func(t *testing.T) { + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 63}) + clearRepoRunCountCache(repo) + assert.Equal(t, 1, RepoNumOpenActions(t.Context(), repo.ID)) + }) - repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - assert.Equal(t, "repo1", repo.Name) + t.Run("Cache Invalidation", func(t *testing.T) { + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 63}) + assert.Equal(t, 1, RepoNumOpenActions(t.Context(), repo.ID)) + + err = db.DeleteBeans(t.Context(), &ActionRun{RepoID: repo.ID}) + require.NoError(t, err) + + // Even though we've deleted ActionRun, expecting that the number of open runs is still 1 (cached) + assert.Equal(t, 1, RepoNumOpenActions(t.Context(), repo.ID)) + + // Now that we clear the cache, computation should be performed + clearRepoRunCountCache(repo) + assert.Equal(t, 0, RepoNumOpenActions(t.Context(), repo.ID)) }) } diff --git a/models/actions/status.go b/models/actions/status.go index e42c221121..73148f0887 100644 --- a/models/actions/status.go +++ b/models/actions/status.go @@ -43,6 +43,16 @@ func init() { } } +// Statuses where the result is final and won't change +func DoneStatuses() []Status { + return []Status{StatusSuccess, StatusFailure, StatusCancelled, StatusSkipped} +} + +// Statuses where the result is not yet final +func PendingStatuses() []Status { + return []Status{StatusUnknown, StatusWaiting, StatusRunning, StatusBlocked} +} + // String returns the string name of the Status func (s Status) String() string { return statusNames[s] @@ -55,7 +65,7 @@ func (s Status) LocaleString(lang translation.Locale) string { // IsDone returns whether the Status is final func (s Status) IsDone() bool { - return s.In(StatusSuccess, StatusFailure, StatusCancelled, StatusSkipped) + return s.In(DoneStatuses()...) } // HasRun returns whether the Status is a result of running diff --git a/models/actions/task.go b/models/actions/task.go index 9baedb9ce7..ad9e1857c8 100644 --- a/models/actions/task.go +++ b/models/actions/task.go @@ -270,7 +270,7 @@ func getConcurrencyCondition() builder.Cond { // Blocking runs have a running status... builder.Eq{"inner_run.status": StatusRunning}.Or( // Blocking runs don't have a IsDone status & are younger than the outer_run - builder.NotIn("inner_run.status", []Status{StatusSuccess, StatusFailure, StatusCancelled, StatusSkipped}). + builder.NotIn("inner_run.status", DoneStatuses()). And(builder.Lt{"inner_run.`index`": builder.Expr("outer_run.`index`")}))) // OK to pick if there are no blocking runs diff --git a/models/forgejo_migrations/v14b_rm-repository-actionrun-stat-fields.go b/models/forgejo_migrations/v14b_rm-repository-actionrun-stat-fields.go new file mode 100644 index 0000000000..62f693e780 --- /dev/null +++ b/models/forgejo_migrations/v14b_rm-repository-actionrun-stat-fields.go @@ -0,0 +1,41 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package forgejo_migrations + +import ( + "fmt" + + "xorm.io/xorm" + "xorm.io/xorm/schemas" +) + +func init() { + registerMigration(&Migration{ + Description: "remove from table repository fields num_action_runs, num_closed_action_runs", + Upgrade: removeRepositoryActionRunStatFields, + }) +} + +func removeRepositoryActionRunStatFields(x *xorm.Engine) error { + switch x.Dialect().URI().DBType { + case schemas.SQLITE: + // Can't drop multiple columns in one statement in SQLite. + _, err := x.Exec("ALTER TABLE `repository` DROP COLUMN num_action_runs") + if err != nil { + return err + } + _, err = x.Exec("ALTER TABLE `repository` DROP COLUMN num_closed_action_runs") + if err != nil { + return err + } + case schemas.MYSQL, schemas.POSTGRES: + _, err := x.Exec("ALTER TABLE `repository` DROP COLUMN num_action_runs, DROP COLUMN num_closed_action_runs") + if err != nil { + return err + } + default: + return fmt.Errorf("unsupported db dialect type %v", x.Dialect().URI().DBType) + } + return nil +} diff --git a/models/repo/repo.go b/models/repo/repo.go index 4ff3b22c8a..fd230f0f89 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -150,9 +150,6 @@ type Repository struct { NumProjects int `xorm:"NOT NULL DEFAULT 0"` NumClosedProjects int `xorm:"NOT NULL DEFAULT 0"` NumOpenProjects int `xorm:"-"` - NumActionRuns int `xorm:"NOT NULL DEFAULT 0"` - NumClosedActionRuns int `xorm:"NOT NULL DEFAULT 0"` - NumOpenActionRuns int `xorm:"-"` IsPrivate bool `xorm:"INDEX"` IsEmpty bool `xorm:"INDEX"` @@ -291,7 +288,6 @@ func (repo *Repository) MarkAsBrokenEmpty() { func (repo *Repository) AfterLoad() { repo.NumOpenMilestones = repo.NumMilestones - repo.NumClosedMilestones repo.NumOpenProjects = repo.NumProjects - repo.NumClosedProjects - repo.NumOpenActionRuns = repo.NumActionRuns - repo.NumClosedActionRuns } // LoadAttributes loads attributes of the repository. diff --git a/services/context/repo.go b/services/context/repo.go index 97ba21da09..ebc9109c96 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -16,6 +16,7 @@ import ( "strings" "forgejo.org/models" + "forgejo.org/models/actions" "forgejo.org/models/db" git_model "forgejo.org/models/git" issues_model "forgejo.org/models/issues" @@ -582,6 +583,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc { ctx.ServerError("GetPackageCountByRepoID", err) return nil } + ctx.Data["NumOpenActionRuns"] = actions.RepoNumOpenActions(ctx, ctx.Repo.Repository.ID) ctx.Data["Title"] = owner.Name + "/" + repo.Name ctx.Data["Repository"] = repo diff --git a/templates/repo/header.tmpl b/templates/repo/header.tmpl index ac57d98064..e5e9606d21 100644 --- a/templates/repo/header.tmpl +++ b/templates/repo/header.tmpl @@ -168,8 +168,8 @@ {{if and .EnableActions (not .UnitActionsGlobalDisabled) (.Permission.CanRead $.UnitTypeActions)}} {{svg "octicon-play"}} {{ctx.Locale.Tr "actions.actions"}} - {{if .Repository.NumOpenActionRuns}} - {{CountFmt .Repository.NumOpenActionRuns}} + {{if .NumOpenActionRuns}} + {{CountFmt .NumOpenActionRuns}} {{end}} {{end}} diff --git a/tests/integration/actions_view_test.go b/tests/integration/actions_view_test.go index 3b124beb65..8cd643ca4b 100644 --- a/tests/integration/actions_view_test.go +++ b/tests/integration/actions_view_test.go @@ -19,6 +19,7 @@ import ( files_service "forgejo.org/services/repository/files" "forgejo.org/tests" + "github.com/PuerkitoBio/goquery" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -188,3 +189,28 @@ func TestActionViewsViewAttemptOutOfRange(t *testing.T) { }) htmlDoc.AssertAttrEqual(t, selector, "data-initial-artifacts-response", "{\"artifacts\":[]}\n") } + +func TestActionTabAccessibleFromRepo(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + req := NewRequest(t, "GET", "/user2/repo1") + resp := MakeRequest(t, req, http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + htmlDoc.AssertElementPredicate(t, "a[href='/user2/repo1/actions']", func(selection *goquery.Selection) bool { + text := strings.TrimSpace(selection.Text()) + assert.Contains(t, text, "Actions") + return true + }) + + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + session := loginUser(t, user2.Name) + req = NewRequest(t, "GET", "/user2/test_action_run_search/actions") + resp = session.MakeRequest(t, req, http.StatusOK) + htmlDoc = NewHTMLParser(t, resp.Body) + htmlDoc.AssertElementPredicate(t, "a[href='/user2/test_action_run_search/actions']", func(selection *goquery.Selection) bool { + text := strings.TrimSpace(selection.Text()) + assert.Contains(t, text, "Actions") + assert.Contains(t, text, "1") // This repo has one running action run + return true + }) +} diff --git a/tests/integration/html_helper.go b/tests/integration/html_helper.go index 6912d0aeff..bcf63e9730 100644 --- a/tests/integration/html_helper.go +++ b/tests/integration/html_helper.go @@ -26,6 +26,13 @@ func NewHTMLParser(t testing.TB, body *bytes.Buffer) *HTMLDoc { return &HTMLDoc{doc: doc} } +func (doc *HTMLDoc) AssertElementPredicate(t testing.TB, selector string, predicate func(element *goquery.Selection) bool) bool { + t.Helper() + selection := doc.doc.Find(selector) + require.NotEmpty(t, selection, selector) + return predicate(selection) +} + func (doc *HTMLDoc) AssertAttrPredicate(t testing.TB, selector, attr string, predicate func(attrValue string) bool) bool { t.Helper() selection := doc.doc.Find(selector)