Revert "Fix schedule tasks bugs (go-gitea#28691)"

earl-warren · earl-warren · commit fe8622dae302 · 2024-02-05T16:54:42.000+01:00
This reverts commit 97292da. (cherry picked from commit 83e5eba0311dc601518fb1a07a7e8538e573a837) (cherry picked from commit f6ef8f3819b5990858b0997cac72af000a3f3e3a) Conflicts: services/repository/setting.go
diff --git a/models/actions/run.go b/models/actions/run.go
@@ -171,14 +171,13 @@ 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, event webhook_module.HookEventType) error {
+func CancelRunningJobs(ctx context.Context, repoID int64, ref, workflowID string) 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,
-		TriggerEvent: event,
-		Status:       []Status{StatusRunning, StatusWaiting},
+		RepoID:     repoID,
+		Ref:        ref,
+		WorkflowID: workflowID,
+		Status:     []Status{StatusRunning, StatusWaiting},
 	})
 	if err != nil {
 		return err
diff --git a/models/actions/run_list.go b/models/actions/run_list.go
@@ -10,7 +10,6 @@ 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"
 )
@@ -72,7 +71,6 @@ 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
 }
@@ -100,9 +98,6 @@ 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
 }
 
diff --git a/models/actions/schedule.go b/models/actions/schedule.go
@@ -5,7 +5,6 @@ package actions
 
 import (
 	"context"
-	"fmt"
 	"time"
 
 	"code.gitea.io/gitea/models/db"
@@ -119,22 +118,3 @@ 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
-}
diff --git a/models/git/branch.go b/models/git/branch.go
@@ -287,7 +287,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(ctx context.Context, isDefault bool) error) (err error) {
+func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to string, gitAction func(isDefault bool) error) (err error) {
 	ctx, committer, err := db.TxContext(ctx)
 	if err != nil {
 		return err
@@ -362,7 +362,7 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to str
 	}
 
 	// 5. do git action
-	if err = gitAction(ctx, isDefault); err != nil {
+	if err = gitAction(isDefault); err != nil {
 		return err
 	}
 
diff --git a/models/git/branch_test.go b/models/git/branch_test.go
@@ -4,7 +4,6 @@
 package git_test
 
 import (
-	"context"
 	"testing"
 
 	"code.gitea.io/gitea/models/db"
@@ -133,7 +132,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(ctx context.Context, isDefault bool) error {
+	assert.NoError(t, git_model.RenameBranch(db.DefaultContext, repo1, "master", "main", func(isDefault bool) error {
 		_isDefault = isDefault
 		return nil
 	}))
diff --git a/models/repo/repo_unit.go b/models/repo/repo_unit.go
@@ -314,3 +314,29 @@ 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()
+}
diff --git a/modules/actions/github.go b/modules/actions/github.go
@@ -22,7 +22,6 @@ const (
 	GithubEventRelease                  = "release"
 	GithubEventPullRequestComment       = "pull_request_comment"
 	GithubEventGollum                   = "gollum"
-	GithubEventSchedule                 = "schedule"
 )
 
 // canGithubEventMatch check if the input Github event can match any Gitea event.
@@ -70,9 +69,6 @@ func canGithubEventMatch(eventName string, triggedEvent webhook_module.HookEvent
 			return false
 		}
 
-	case GithubEventSchedule:
-		return triggedEvent == webhook_module.HookEventSchedule
-
 	default:
 		return eventName == string(triggedEvent)
 	}
diff --git a/modules/actions/workflows.go b/modules/actions/workflows.go
@@ -22,7 +22,7 @@ import (
 
 type DetectedWorkflow struct {
 	EntryName    string
-	TriggerEvent *jobparser.Event
+	TriggerEvent string
 	Content      []byte
 }
 
@@ -103,7 +103,6 @@ func DetectWorkflows(
 	commit *git.Commit,
 	triggedEvent webhook_module.HookEventType,
 	payload api.Payloader,
-	detectSchedule bool,
 ) ([]*DetectedWorkflow, []*DetectedWorkflow, error) {
 	entries, err := ListWorkflows(commit)
 	if err != nil {
@@ -118,7 +117,6 @@ 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)
@@ -127,18 +125,17 @@ func DetectWorkflows(
 		for _, evt := range events {
 			log.Trace("detect workflow %q for event %#v matching %q", entry.Name(), evt, triggedEvent)
 			if evt.IsSchedule() {
-				if detectSchedule {
-					dwf := &DetectedWorkflow{
-						EntryName:    entry.Name(),
-						TriggerEvent: evt,
-						Content:      content,
-					}
-					schedules = append(schedules, dwf)
+				dwf := &DetectedWorkflow{
+					EntryName:    entry.Name(),
+					TriggerEvent: evt.Name,
+					Content:      content,
 				}
-			} else if detectMatched(gitRepo, commit, triggedEvent, payload, evt) {
+				schedules = append(schedules, dwf)
+			}
+			if detectMatched(gitRepo, commit, triggedEvent, payload, evt) {
 				dwf := &DetectedWorkflow{
 					EntryName:    entry.Name(),
-					TriggerEvent: evt,
+					TriggerEvent: evt.Name,
 					Content:      content,
 				}
 				workflows = append(workflows, dwf)
@@ -159,8 +156,7 @@ func detectMatched(gitRepo *git.Repository, commit *git.Commit, triggedEvent web
 		webhook_module.HookEventCreate,
 		webhook_module.HookEventDelete,
 		webhook_module.HookEventFork,
-		webhook_module.HookEventWiki,
-		webhook_module.HookEventSchedule:
+		webhook_module.HookEventWiki:
 		if len(evt.Acts()) != 0 {
 			log.Warn("Ignore unsupported %s event arguments %v", triggedEvent, evt.Acts())
 		}
diff --git a/modules/actions/workflows_test.go b/modules/actions/workflows_test.go
@@ -118,13 +118,6 @@ 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 {
diff --git a/modules/webhook/type.go b/modules/webhook/type.go
@@ -31,7 +31,6 @@ const (
 	HookEventRepository                HookEventType = "repository"
 	HookEventRelease                   HookEventType = "release"
 	HookEventPackage                   HookEventType = "package"
-	HookEventSchedule                  HookEventType = "schedule"
 )
 
 // Event returns the HookEventType as an event string
diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go
@@ -984,7 +984,7 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error {
 	}
 
 	if len(units)+len(deleteUnitTypes) > 0 {
-		if err := repo_service.UpdateRepositoryUnits(ctx, repo, units, deleteUnitTypes); err != nil {
+		if err := repo_model.UpdateRepositoryUnits(ctx, repo, units, deleteUnitTypes); err != nil {
 			ctx.Error(http.StatusInternalServerError, "UpdateRepositoryUnits", err)
 			return err
 		}
diff --git a/routers/web/repo/setting/default_branch.go b/routers/web/repo/setting/default_branch.go
@@ -6,12 +6,13 @@ package setting
 import (
 	"net/http"
 
-	git_model "code.gitea.io/gitea/models/git"
+	repo_model "code.gitea.io/gitea/models/repo"
 	"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"
-	repo_service "code.gitea.io/gitea/services/repository"
+	notify_service "code.gitea.io/gitea/services/notify"
 )
 
 // SetDefaultBranchPost set default branch
@@ -34,14 +35,23 @@ func SetDefaultBranchPost(ctx *context.Context) {
 		}
 
 		branch := ctx.FormString("branch")
-		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:
+		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 {
 				ctx.ServerError("SetDefaultBranch", err)
+				return
 			}
-			return
+
+			notify_service.ChangeDefaultBranch(ctx, repo)
 		}
 
 		log.Trace("Repository basic settings updated: %s/%s", ctx.Repo.Owner.Name, repo.Name)
diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go
@@ -601,7 +601,7 @@ func SettingsPost(ctx *context.Context) {
 			return
 		}
 
-		if err := repo_service.UpdateRepositoryUnits(ctx, repo, units, deleteUnitTypes); err != nil {
+		if err := repo_model.UpdateRepositoryUnits(ctx, repo, units, deleteUnitTypes); err != nil {
 			ctx.ServerError("UpdateRepositoryUnits", err)
 			return
 		}
diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go
diff --git a/services/actions/schedule_tasks.go b/services/actions/schedule_tasks.go
diff --git a/services/repository/branch.go b/services/repository/branch.go
diff --git a/services/repository/setting.go b/services/repository/setting.go
diff --git a/services/wiki/wiki.go b/services/wiki/wiki.go
diff --git a/tests/integration/actions_trigger_test.go b/tests/integration/actions_trigger_test.go

Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,6 @@ import (`
`10`	`10`	`repo_model "code.gitea.io/gitea/models/repo"`
`11`	`11`	`user_model "code.gitea.io/gitea/models/user"`
`12`	`12`	`"code.gitea.io/gitea/modules/container"`
`13`		`- webhook_module "code.gitea.io/gitea/modules/webhook"`
`14`	`13`
`15`	`14`	`"xorm.io/builder"`
`16`	`15`	`)`
`@@ -72,7 +71,6 @@ type FindRunOptions struct {`
`72`	`71`	`WorkflowID string`
`73`	`72`	`Ref string // the commit/tag/… that caused this workflow`
`74`	`73`	`TriggerUserID int64`
`75`		`- TriggerEvent webhook_module.HookEventType`
`76`	`74`	`Approved bool // not util.OptionalBool, it works only when it's true`
`77`	`75`	`Status []Status`
`78`	`76`	`}`
`@@ -100,9 +98,6 @@ func (opts FindRunOptions) ToConds() builder.Cond {`
`100`	`98`	`if opts.Ref != "" {`
`101`	`99`	`cond = cond.And(builder.Eq{"ref": opts.Ref})`
`102`	`100`	`}`
`103`		`- if opts.TriggerEvent != "" {`
`104`		`- cond = cond.And(builder.Eq{"trigger_event": opts.TriggerEvent})`
`105`		`- }`
`106`	`101`	`return cond`
`107`	`102`	`}`
`108`	`103`
Original file line number	Diff line number	Diff line change
`@@ -287,7 +287,7 @@ func FindRenamedBranch(ctx context.Context, repoID int64, from string) (branch *`
`287`	`287`	`}`
`288`	`288`
`289`	`289`	`// RenameBranch rename a branch`
`290`		`-func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to string, gitAction func(ctx context.Context, isDefault bool) error) (err error) {`
	`290`	`+func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to string, gitAction func(isDefault bool) error) (err error) {`
`291`	`291`	`ctx, committer, err := db.TxContext(ctx)`
`292`	`292`	`if err != nil {`
`293`	`293`	`return err`
`@@ -362,7 +362,7 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to str`
`362`	`362`	`}`
`363`	`363`
`364`	`364`	`// 5. do git action`
`365`		`- if err = gitAction(ctx, isDefault); err != nil {`
	`365`	`+ if err = gitAction(isDefault); err != nil {`
`366`	`366`	`return err`
`367`	`367`	`}`
`368`	`368`
Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,6 @@ const (`
`22`	`22`	`GithubEventRelease = "release"`
`23`	`23`	`GithubEventPullRequestComment = "pull_request_comment"`
`24`	`24`	`GithubEventGollum = "gollum"`
`25`		`- GithubEventSchedule = "schedule"`
`26`	`25`	`)`
`27`	`26`
`28`	`27`	`// canGithubEventMatch check if the input Github event can match any Gitea event.`
`@@ -70,9 +69,6 @@ func canGithubEventMatch(eventName string, triggedEvent webhook_module.HookEvent`
`70`	`69`	`return false`
`71`	`70`	`}`
`72`	`71`
`73`		`- case GithubEventSchedule:`
`74`		`- return triggedEvent == webhook_module.HookEventSchedule`
`75`		`-`
`76`	`72`	`default:`
`77`	`73`	`return eventName == string(triggedEvent)`
`78`	`74`	`}`
Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,6 @@ const (`
`31`	`31`	`HookEventRepository HookEventType = "repository"`
`32`	`32`	`HookEventRelease HookEventType = "release"`
`33`	`33`	`HookEventPackage HookEventType = "package"`
`34`		`- HookEventSchedule HookEventType = "schedule"`
`35`	`34`	`)`
`36`	`35`
`37`	`36`	`// Event returns the HookEventType as an event string`