Fix schedule tasks bugs (go-gitea#28691)

Fix go-gitea#28157 

This PR fix the possible bugs about actions schedule.

## The Changes

- Move `UpdateRepositoryUnit` and `SetRepoDefaultBranch` from models to
service layer
- Remove schedules plan from database and cancel waiting & running
schedules tasks in this repository when actions unit has been disabled
or global disabled.
- Remove schedules plan from database and cancel waiting & running
schedules tasks in this repository when default branch changed.

models/actions/run.go

@@ -168,13 +168,14 @@ func updateRepoRunsNumbers(ctx context.Context, repo *repo_model.Repository) err
 }
 
 // CancelRunningJobs cancels all running and waiting jobs associated with a specific workflow.
-func CancelRunningJobs(ctx context.Context, repoID int64, ref, workflowID string) error {
+func CancelRunningJobs(ctx context.Context, repoID int64, ref, workflowID string, event webhook_module.HookEventType) error {
  // Find all runs in the specified repository, reference, and workflow with statuses 'Running' or 'Waiting'.
  runs, total, err := db.FindAndCount[ActionRun](ctx, FindRunOptions{
- RepoID: repoID,
- Ref: ref,
- WorkflowID: workflowID,
- Status: []Status{StatusRunning, StatusWaiting},
+ RepoID: repoID,
+ Ref: ref,
+ WorkflowID: workflowID,
+ TriggerEvent: event,
+ Status: []Status{StatusRunning, StatusWaiting},
  })
  if err != nil {
  return err

models/actions/run_list.go

@@ -10,6 +10,7 @@ import (
  repo_model "code.gitea.io/gitea/models/repo"
  user_model "code.gitea.io/gitea/models/user"
  "code.gitea.io/gitea/modules/container"
+ webhook_module "code.gitea.io/gitea/modules/webhook"
 
  "xorm.io/builder"
 )
@@ -71,6 +72,7 @@ type FindRunOptions struct {
  WorkflowID string
  Ref string // the commit/tag/… that caused this workflow
  TriggerUserID int64
+ TriggerEvent webhook_module.HookEventType
  Approved bool // not util.OptionalBool, it works only when it's true
  Status []Status
 }
@@ -98,6 +100,9 @@ func (opts FindRunOptions) ToConds() builder.Cond {
  if opts.Ref != "" {
  cond = cond.And(builder.Eq{"ref": opts.Ref})
  }
+ if opts.TriggerEvent != "" {
+ cond = cond.And(builder.Eq{"trigger_event": opts.TriggerEvent})
+ }
  return cond
 }
 

models/actions/schedule.go

@@ -5,6 +5,7 @@ package actions
 
 import (
  "context"
+ "fmt"
  "time"
 
  "code.gitea.io/gitea/models/db"
@@ -118,3 +119,22 @@ func DeleteScheduleTaskByRepo(ctx context.Context, id int64) error {
 
  return committer.Commit()
 }
+
+func CleanRepoScheduleTasks(ctx context.Context, repo *repo_model.Repository) error {
+ // If actions disabled when there is schedule task, this will remove the outdated schedule tasks
+ // There is no other place we can do this because the app.ini will be changed manually
+ if err := DeleteScheduleTaskByRepo(ctx, repo.ID); err != nil {
+ return fmt.Errorf("DeleteCronTaskByRepo: %v", err)
+ }
+ // cancel running cron jobs of this repository and delete old schedules
+ if err := CancelRunningJobs(
+ ctx,
+ repo.ID,
+ repo.DefaultBranch,
+ "",
+ webhook_module.HookEventSchedule,
+ ); err != nil {
+ return fmt.Errorf("CancelRunningJobs: %v", err)
+ }
+ return nil
+}

models/git/branch.go

@@ -283,7 +283,7 @@ func FindRenamedBranch(ctx context.Context, repoID int64, from string) (branch *
 }
 
 // RenameBranch rename a branch
-func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to string, gitAction func(isDefault bool) error) (err error) {
+func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to string, gitAction func(ctx context.Context, isDefault bool) error) (err error) {
  ctx, committer, err := db.TxContext(ctx)
  if err != nil {
  return err
@@ -358,7 +358,7 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to str
  }
 
  // 5. do git action
- if err = gitAction(isDefault); err != nil {
+ if err = gitAction(ctx, isDefault); err != nil {
  return err
  }
 

models/git/branch_test.go

@@ -4,6 +4,7 @@
 package git_test
 
 import (
+ "context"
  "testing"
 
  "code.gitea.io/gitea/models/db"
@@ -132,7 +133,7 @@ func TestRenameBranch(t *testing.T) {
  }, git_model.WhitelistOptions{}))
  assert.NoError(t, committer.Commit())
 
- assert.NoError(t, git_model.RenameBranch(db.DefaultContext, repo1, "master", "main", func(isDefault bool) error {
+ assert.NoError(t, git_model.RenameBranch(db.DefaultContext, repo1, "master", "main", func(ctx context.Context, isDefault bool) error {
  _isDefault = isDefault
  return nil
  }))

models/repo/repo_unit.go

@@ -283,29 +283,3 @@ func UpdateRepoUnit(ctx context.Context, unit *RepoUnit) error {
  _, err := db.GetEngine(ctx).ID(unit.ID).Update(unit)
  return err
 }
-
-// UpdateRepositoryUnits updates a repository's units
-func UpdateRepositoryUnits(ctx context.Context, repo *Repository, units []RepoUnit, deleteUnitTypes []unit.Type) (err error) {
- ctx, committer, err := db.TxContext(ctx)
- if err != nil {
- return err
- }
- defer committer.Close()
-
- // Delete existing settings of units before adding again
- for _, u := range units {
- deleteUnitTypes = append(deleteUnitTypes, u.Type)
- }
-
- if _, err = db.GetEngine(ctx).Where("repo_id = ?", repo.ID).In("type", deleteUnitTypes).Delete(new(RepoUnit)); err != nil {
- return err
- }
-
- if len(units) > 0 {
- if err = db.Insert(ctx, units); err != nil {
- return err
- }
- }
-
- return committer.Commit()
-}

modules/actions/github.go

@@ -22,6 +22,7 @@ const (
  GithubEventRelease = "release"
  GithubEventPullRequestComment = "pull_request_comment"
  GithubEventGollum = "gollum"
+ GithubEventSchedule = "schedule"
 )
 
 // canGithubEventMatch check if the input Github event can match any Gitea event.
@@ -69,6 +70,9 @@ func canGithubEventMatch(eventName string, triggedEvent webhook_module.HookEvent
  return false
  }
 
+ case GithubEventSchedule:
+ return triggedEvent == webhook_module.HookEventSchedule
+
  default:
  return eventName == string(triggedEvent)
  }

modules/actions/workflows.go

@@ -22,7 +22,7 @@ import (
 
 type DetectedWorkflow struct {
  EntryName string
- TriggerEvent string
+ TriggerEvent *jobparser.Event
  Content []byte
 }
 
@@ -100,6 +100,7 @@ func DetectWorkflows(
  commit *git.Commit,
  triggedEvent webhook_module.HookEventType,
  payload api.Payloader,
+ detectSchedule bool,
 ) ([]*DetectedWorkflow, []*DetectedWorkflow, error) {
  entries, err := ListWorkflows(commit)
  if err != nil {
@@ -114,6 +115,7 @@ func DetectWorkflows(
  return nil, nil, err
  }
 
+ // one workflow may have multiple events
  events, err := GetEventsFromContent(content)
  if err != nil {
  log.Warn("ignore invalid workflow %q: %v", entry.Name(), err)
@@ -122,17 +124,18 @@ func DetectWorkflows(
  for _, evt := range events {
  log.Trace("detect workflow %q for event %#v matching %q", entry.Name(), evt, triggedEvent)
  if evt.IsSchedule() {
- dwf := &DetectedWorkflow{
- EntryName: entry.Name(),
- TriggerEvent: evt.Name,
- Content: content,
+ if detectSchedule {
+ dwf := &DetectedWorkflow{
+ EntryName: entry.Name(),
+ TriggerEvent: evt,
+ Content: content,
+ }
+ schedules = append(schedules, dwf)
  }
- schedules = append(schedules, dwf)
- }
- if detectMatched(gitRepo, commit, triggedEvent, payload, evt) {
+ } else if detectMatched(gitRepo, commit, triggedEvent, payload, evt) {
  dwf := &DetectedWorkflow{
  EntryName: entry.Name(),
- TriggerEvent: evt.Name,
+ TriggerEvent: evt,
  Content: content,
  }
  workflows = append(workflows, dwf)
@@ -153,7 +156,8 @@ func detectMatched(gitRepo *git.Repository, commit *git.Commit, triggedEvent web
  webhook_module.HookEventCreate,
  webhook_module.HookEventDelete,
  webhook_module.HookEventFork,
- webhook_module.HookEventWiki:
+ webhook_module.HookEventWiki,
+ webhook_module.HookEventSchedule:
  if len(evt.Acts()) != 0 {
  log.Warn("Ignore unsupported %s event arguments %v", triggedEvent, evt.Acts())
  }

modules/actions/workflows_test.go

@@ -118,6 +118,13 @@ func TestDetectMatched(t *testing.T) {
  yamlOn: "on: gollum",
  expected: true,
  },
+ {
+ desc: "HookEventSchedue(schedule) matches GithubEventSchedule(schedule)",
+ triggedEvent: webhook_module.HookEventSchedule,
+ payload: nil,
+ yamlOn: "on: schedule",
+ expected: true,
+ },
  }
 
  for _, tc := range testCases {

modules/webhook/type.go

@@ -31,6 +31,7 @@ const (
  HookEventRepository HookEventType = "repository"
  HookEventRelease HookEventType = "release"
  HookEventPackage HookEventType = "package"
+ HookEventSchedule HookEventType = "schedule"
 )
 
 // Event returns the HookEventType as an event string

routers/api/v1/repo/repo.go

@@ -983,7 +983,7 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error {
  }
 
  if len(units)+len(deleteUnitTypes) > 0 {
- if err := repo_model.UpdateRepositoryUnits(ctx, repo, units, deleteUnitTypes); err != nil {
+ if err := repo_service.UpdateRepositoryUnits(ctx, repo, units, deleteUnitTypes); err != nil {
  ctx.Error(http.StatusInternalServerError, "UpdateRepositoryUnits", err)
  return err
  }

routers/web/repo/setting/default_branch.go

@@ -6,13 +6,12 @@ package setting
 import (
  "net/http"
 
- repo_model "code.gitea.io/gitea/models/repo"
+ git_model "code.gitea.io/gitea/models/git"
  "code.gitea.io/gitea/modules/context"
- "code.gitea.io/gitea/modules/git"
  "code.gitea.io/gitea/modules/log"
  "code.gitea.io/gitea/modules/setting"
  "code.gitea.io/gitea/routers/web/repo"
- notify_service "code.gitea.io/gitea/services/notify"
+ repo_service "code.gitea.io/gitea/services/repository"
 )
 
 // SetDefaultBranchPost set default branch
@@ -35,23 +34,14 @@ func SetDefaultBranchPost(ctx *context.Context) {
  }
 
  branch := ctx.FormString("branch")
- if !ctx.Repo.GitRepo.IsBranchExist(branch) {
- ctx.Status(http.StatusNotFound)
- return
- } else if repo.DefaultBranch != branch {
- repo.DefaultBranch = branch
- if err := ctx.Repo.GitRepo.SetDefaultBranch(branch); err != nil {
- if !git.IsErrUnsupportedVersion(err) {
- ctx.ServerError("SetDefaultBranch", err)
- return
- }
- }
- if err := repo_model.UpdateDefaultBranch(ctx, repo); err != nil {
+ if err := repo_service.SetRepoDefaultBranch(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, branch); err != nil {
+ switch {
+ case git_model.IsErrBranchNotExist(err):
+ ctx.Status(http.StatusNotFound)
+ default:
  ctx.ServerError("SetDefaultBranch", err)
- return
  }
-
- notify_service.ChangeDefaultBranch(ctx, repo)
+ return
  }
 
  log.Trace("Repository basic settings updated: %s/%s", ctx.Repo.Owner.Name, repo.Name)

routers/web/repo/setting/setting.go

@@ -594,7 +594,7 @@ func SettingsPost(ctx *context.Context) {
  return
  }
 
- if err := repo_model.UpdateRepositoryUnits(ctx, repo, units, deleteUnitTypes); err != nil {
+ if err := repo_service.UpdateRepositoryUnits(ctx, repo, units, deleteUnitTypes); err != nil {
  ctx.ServerError("UpdateRepositoryUnits", err)
  return
  }