Refactor push update and notifications

modules/notification/action/action.go

@@ -13,7 +13,7 @@ import (
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/notification/base"
-	"code.gitea.io/gitea/modules/repository"
+	repo_module "code.gitea.io/gitea/modules/repository"
 )
 
 type actionNotifier struct {
@@ -266,7 +266,76 @@ func (*actionNotifier) NotifyMergePullRequest(pr *models.PullRequest, doer *mode
 	}
 }
 
-func (a *actionNotifier) NotifySyncPushCommits(pusher *models.User, repo *models.Repository, refName, oldCommitID, newCommitID string, commits *repository.PushCommits) {
+func (a *actionNotifier) NotifyPushCommits(pusher *models.User, repo *models.Repository, opts *repo_module.PushUpdateOptions, commits *repo_module.PushCommits) {
+	data, err := json.Marshal(commits)
+	if err != nil {
+		log.Error("Marshal: %v", err)
+		return
+	}
+
+	opType := models.ActionCommitRepo
+
+	// Check it's tag push or branch.
+	if opts.IsTag() {
+		opType = models.ActionPushTag
+		if opts.IsDelRef() {
+			opType = models.ActionDeleteTag
+		}
+	} else if opts.IsDelRef() {
+		opType = models.ActionDeleteBranch
+	}
+
+	if err = models.NotifyWatchers(&models.Action{
+		ActUserID: pusher.ID,
+		ActUser:   pusher,
+		OpType:    opType,
+		Content:   string(data),
+		RepoID:    repo.ID,
+		Repo:      repo,
+		RefName:   opts.RefFullName,
+		IsPrivate: repo.IsPrivate,
+	}); err != nil {
+		log.Error("notifyWatchers: %v", err)
+	}
+}
+
+func (a *actionNotifier) NotifyCreateRef(doer *models.User, repo *models.Repository, refType, refFullName string) {
+	opType := models.ActionCommitRepo
+	if refType == "tag" {
+		opType = models.ActionPushTag
+	}
+	if err := models.NotifyWatchers(&models.Action{
+		ActUserID: doer.ID,
+		ActUser:   doer,
+		OpType:    opType,
+		RepoID:    repo.ID,
+		Repo:      repo,
+		IsPrivate: repo.IsPrivate,
+		RefName:   refFullName,
+	}); err != nil {
+		log.Error("notifyWatchers: %v", err)
+	}
+}
+
+func (a *actionNotifier) NotifyDeleteRef(doer *models.User, repo *models.Repository, refType, refFullName string) {
+	opType := models.ActionDeleteBranch
+	if refType == "tag" {
+		opType = models.ActionDeleteTag
+	}
+	if err := models.NotifyWatchers(&models.Action{
+		ActUserID: doer.ID,
+		ActUser:   doer,
+		OpType:    opType,
+		RepoID:    repo.ID,
+		Repo:      repo,
+		IsPrivate: repo.IsPrivate,
+		RefName:   refFullName,
+	}); err != nil {
+		log.Error("notifyWatchers: %v", err)
+	}
+}
+
+func (a *actionNotifier) NotifySyncPushCommits(pusher *models.User, repo *models.Repository, refName, oldCommitID, newCommitID string, commits *repo_module.PushCommits) {
 	data, err := json.Marshal(commits)
 	if err != nil {
 		log.Error("json.Marshal: %v", err)

modules/notification/base/notifier.go

@@ -6,7 +6,7 @@ package base
 
 import (
 	"code.gitea.io/gitea/models"
-	"code.gitea.io/gitea/modules/repository"
+	repo_module "code.gitea.io/gitea/modules/repository"
 )
 
 // Notifier defines an interface to notify receiver
@@ -45,11 +45,11 @@ type Notifier interface {
 	NotifyUpdateRelease(doer *models.User, rel *models.Release)
 	NotifyDeleteRelease(doer *models.User, rel *models.Release)
 
-	NotifyPushCommits(pusher *models.User, repo *models.Repository, refName, oldCommitID, newCommitID string, commits *repository.PushCommits)
+	NotifyPushCommits(pusher *models.User, repo *models.Repository, opts *repo_module.PushUpdateOptions, commits *repo_module.PushCommits)
 	NotifyCreateRef(doer *models.User, repo *models.Repository, refType, refFullName string)
 	NotifyDeleteRef(doer *models.User, repo *models.Repository, refType, refFullName string)
 
-	NotifySyncPushCommits(pusher *models.User, repo *models.Repository, refName, oldCommitID, newCommitID string, commits *repository.PushCommits)
+	NotifySyncPushCommits(pusher *models.User, repo *models.Repository, refName, oldCommitID, newCommitID string, commits *repo_module.PushCommits)
 	NotifySyncCreateRef(doer *models.User, repo *models.Repository, refType, refFullName string)
 	NotifySyncDeleteRef(doer *models.User, repo *models.Repository, refType, refFullName string)
 }

modules/notification/base/null.go

@@ -6,7 +6,7 @@ package base
 
 import (
 	"code.gitea.io/gitea/models"
-	"code.gitea.io/gitea/modules/repository"
+	repo_module "code.gitea.io/gitea/modules/repository"
 )
 
 // NullNotifier implements a blank notifier
@@ -116,7 +116,7 @@ func (*NullNotifier) NotifyMigrateRepository(doer *models.User, u *models.User,
 }
 
 // NotifyPushCommits notifies commits pushed to notifiers
-func (*NullNotifier) NotifyPushCommits(pusher *models.User, repo *models.Repository, refName, oldCommitID, newCommitID string, commits *repository.PushCommits) {
+func (*NullNotifier) NotifyPushCommits(pusher *models.User, repo *models.Repository, opts *repo_module.PushUpdateOptions, commits *repo_module.PushCommits) {
 }
 
 // NotifyCreateRef notifies branch or tag creation to notifiers
@@ -136,7 +136,7 @@ func (*NullNotifier) NotifyTransferRepository(doer *models.User, repo *models.Re
 }
 
 // NotifySyncPushCommits places a place holder function
-func (*NullNotifier) NotifySyncPushCommits(pusher *models.User, repo *models.Repository, refName, oldCommitID, newCommitID string, commits *repository.PushCommits) {
+func (*NullNotifier) NotifySyncPushCommits(pusher *models.User, repo *models.Repository, refName, oldCommitID, newCommitID string, commits *repo_module.PushCommits) {
 }
 
 // NotifySyncCreateRef places a place holder function

modules/notification/indexer/indexer.go

@@ -12,7 +12,7 @@ import (
 	stats_indexer "code.gitea.io/gitea/modules/indexer/stats"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/notification/base"
-	"code.gitea.io/gitea/modules/repository"
+	repo_module "code.gitea.io/gitea/modules/repository"
 	"code.gitea.io/gitea/modules/setting"
 )
 
@@ -123,8 +123,8 @@ func (r *indexerNotifier) NotifyMigrateRepository(doer *models.User, u *models.U
 	}
 }
 
-func (r *indexerNotifier) NotifyPushCommits(pusher *models.User, repo *models.Repository, refName, oldCommitID, newCommitID string, commits *repository.PushCommits) {
-	if setting.Indexer.RepoIndexerEnabled && refName == git.BranchPrefix+repo.DefaultBranch {
+func (r *indexerNotifier) NotifyPushCommits(pusher *models.User, repo *models.Repository, options *repo_module.PushUpdateOptions, commits *repo_module.PushCommits) {
+	if setting.Indexer.RepoIndexerEnabled && options.RefFullName == git.BranchPrefix+repo.DefaultBranch {
 		code_indexer.UpdateRepoIndexer(repo)
 	}
 	if err := stats_indexer.UpdateRepoIndexer(repo); err != nil {

modules/notification/notification.go

@@ -12,7 +12,7 @@ import (
 	"code.gitea.io/gitea/modules/notification/mail"
 	"code.gitea.io/gitea/modules/notification/ui"
 	"code.gitea.io/gitea/modules/notification/webhook"
-	"code.gitea.io/gitea/modules/repository"
+	repo_module "code.gitea.io/gitea/modules/repository"
 	"code.gitea.io/gitea/modules/setting"
 )
 
@@ -215,9 +215,9 @@ func NotifyRenameRepository(doer *models.User, repo *models.Repository, oldName
 }
 
 // NotifyPushCommits notifies commits pushed to notifiers
-func NotifyPushCommits(pusher *models.User, repo *models.Repository, refName, oldCommitID, newCommitID string, commits *repository.PushCommits) {
+func NotifyPushCommits(pusher *models.User, repo *models.Repository, options *repo_module.PushUpdateOptions, commits *repo_module.PushCommits) {
 	for _, notifier := range notifiers {
-		notifier.NotifyPushCommits(pusher, repo, refName, oldCommitID, newCommitID, commits)
+		notifier.NotifyPushCommits(pusher, repo, options, commits)
 	}
 }
 
@@ -236,7 +236,7 @@ func NotifyDeleteRef(pusher *models.User, repo *models.Repository, refType, refF
 }
 
 // NotifySyncPushCommits notifies commits pushed to notifiers
-func NotifySyncPushCommits(pusher *models.User, repo *models.Repository, refName, oldCommitID, newCommitID string, commits *repository.PushCommits) {
+func NotifySyncPushCommits(pusher *models.User, repo *models.Repository, refName, oldCommitID, newCommitID string, commits *repo_module.PushCommits) {
 	for _, notifier := range notifiers {
 		notifier.NotifySyncPushCommits(pusher, repo, refName, oldCommitID, newCommitID, commits)
 	}

modules/notification/webhook/webhook.go

@@ -10,7 +10,7 @@ import (
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/notification/base"
-	"code.gitea.io/gitea/modules/repository"
+	repo_module "code.gitea.io/gitea/modules/repository"
 	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
 	webhook_module "code.gitea.io/gitea/modules/webhook"
@@ -501,7 +501,7 @@ func (m *webhookNotifier) NotifyIssueChangeMilestone(doer *models.User, issue *m
 	}
 }
 
-func (m *webhookNotifier) NotifyPushCommits(pusher *models.User, repo *models.Repository, refName, oldCommitID, newCommitID string, commits *repository.PushCommits) {
+func (m *webhookNotifier) NotifyPushCommits(pusher *models.User, repo *models.Repository, options *repo_module.PushUpdateOptions, commits *repo_module.PushCommits) {
 	apiPusher := pusher.APIFormat()
 	apiCommits, err := commits.ToAPIPayloadCommits(repo.RepoPath(), repo.HTMLURL())
 	if err != nil {
@@ -510,9 +510,9 @@ func (m *webhookNotifier) NotifyPushCommits(pusher *models.User, repo *models.Re
 	}
 
 	if err := webhook_module.PrepareWebhooks(repo, models.HookEventPush, &api.PushPayload{
-		Ref:        refName,
-		Before:     oldCommitID,
-		After:      newCommitID,
+		Ref:        options.RefFullName,
+		Before:     options.OldCommitID,
+		After:      options.NewCommitID,
 		CompareURL: setting.AppURL + commits.CompareURL,
 		Commits:    apiCommits,
 		Repo:       repo.APIFormat(models.AccessModeOwner),
@@ -729,7 +729,7 @@ func (m *webhookNotifier) NotifyDeleteRelease(doer *models.User, rel *models.Rel
 	sendReleaseHook(doer, rel, api.HookReleaseDeleted)
 }
 
-func (m *webhookNotifier) NotifySyncPushCommits(pusher *models.User, repo *models.Repository, refName, oldCommitID, newCommitID string, commits *repository.PushCommits) {
+func (m *webhookNotifier) NotifySyncPushCommits(pusher *models.User, repo *models.Repository, refName, oldCommitID, newCommitID string, commits *repo_module.PushCommits) {
 	apiPusher := pusher.APIFormat()
 	apiCommits, err := commits.ToAPIPayloadCommits(repo.RepoPath(), repo.HTMLURL())
 	if err != nil {

modules/repofiles/action.go

@@ -5,7 +5,6 @@
 package repofiles
 
 import (
-	"encoding/json"
 	"fmt"
 	"html"
 
@@ -14,7 +13,7 @@ import (
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/notification"
 	"code.gitea.io/gitea/modules/references"
-	"code.gitea.io/gitea/modules/repository"
+	repo_module "code.gitea.io/gitea/modules/repository"
 	"code.gitea.io/gitea/modules/setting"
 )
 
@@ -59,7 +58,7 @@ func changeIssueStatus(repo *models.Repository, issue *models.Issue, doer *model
 }
 
 // UpdateIssuesCommit checks if issues are manipulated by commit message.
-func UpdateIssuesCommit(doer *models.User, repo *models.Repository, commits []*repository.PushCommit, branchName string) error {
+func UpdateIssuesCommit(doer *models.User, repo *models.Repository, commits []*repo_module.PushCommit, branchName string) error {
 	// Commits are appended in the reverse order.
 	for i := len(commits) - 1; i >= 0; i-- {
 		c := commits[i]
@@ -150,20 +149,19 @@ func UpdateIssuesCommit(doer *models.User, repo *models.Repository, commits []*r
 
 // CommitRepoActionOptions represent options of a new commit action.
 type CommitRepoActionOptions struct {
-	PushUpdateOptions
+	repo_module.PushUpdateOptions
 
 	RepoOwnerID int64
-	Commits     *repository.PushCommits
+	Commits     *repo_module.PushCommits
 }
 
 // CommitRepoAction adds new commit action to the repository, and prepare
 // corresponding webhooks.
 func CommitRepoAction(optsList ...*CommitRepoActionOptions) error {
 	var pusher *models.User
 	var repo *models.Repository
-	actions := make([]*models.Action, len(optsList))
 
-	for i, opts := range optsList {
+	for _, opts := range optsList {
 		if pusher == nil || pusher.Name != opts.PusherName {
 			var err error
 			pusher, err = models.GetUserByName(opts.PusherName)
@@ -206,69 +204,31 @@ func CommitRepoAction(optsList ...*CommitRepoActionOptions) error {
 			}
 		}
 
-		opType := models.ActionCommitRepo
-
 		// Check it's tag push or branch.
 		if opts.IsTag() {
-			opType = models.ActionPushTag
 			if opts.IsDelRef() {
-				opType = models.ActionDeleteTag
+				notification.NotifyDeleteRef(pusher, repo, "tag", opts.RefFullName)
+			} else {
+				notification.NotifyCreateRef(pusher, repo, "tag", opts.RefFullName)
 			}
-			opts.Commits = &repository.PushCommits{}
 		} else if opts.IsDelRef() {
-			opType = models.ActionDeleteBranch
-			opts.Commits = &repository.PushCommits{}
+			notification.NotifyDeleteRef(pusher, repo, "branch", opts.RefFullName)
 		} else {
 			// if not the first commit, set the compare URL.
 			if !opts.IsNewRef() {
 				opts.Commits.CompareURL = repo.ComposeCompareURL(opts.OldCommitID, opts.NewCommitID)
+			} else {
+				notification.NotifyCreateRef(pusher, repo, "branch", opts.RefFullName)
 			}
 
 			if err := UpdateIssuesCommit(pusher, repo, opts.Commits.Commits, refName); err != nil {
 				log.Error("updateIssuesCommit: %v", err)
 			}
-		}
-
-		if len(opts.Commits.Commits) > setting.UI.FeedMaxCommitNum {
-			opts.Commits.Commits = opts.Commits.Commits[:setting.UI.FeedMaxCommitNum]
-		}
-
-		data, err := json.Marshal(opts.Commits)
-		if err != nil {
-			return fmt.Errorf("Marshal: %v", err)
-		}
 
-		actions[i] = &models.Action{
-			ActUserID: pusher.ID,
-			ActUser:   pusher,
-			OpType:    opType,
-			Content:   string(data),
-			RepoID:    repo.ID,
-			Repo:      repo,
-			RefName:   refName,
-			IsPrivate: repo.IsPrivate,
-		}
-
-		var isHookEventPush = true
-		switch opType {
-		case models.ActionCommitRepo: // Push
-			if opts.IsNewBranch() {
-				notification.NotifyCreateRef(pusher, repo, "branch", opts.RefFullName)
+			if len(opts.Commits.Commits) > setting.UI.FeedMaxCommitNum {
+				opts.Commits.Commits = opts.Commits.Commits[:setting.UI.FeedMaxCommitNum]
 			}
-		case models.ActionDeleteBranch: // Delete Branch
-			notification.NotifyDeleteRef(pusher, repo, "branch", opts.RefFullName)
-
-		case models.ActionPushTag: // Create
-			notification.NotifyCreateRef(pusher, repo, "tag", opts.RefFullName)
-
-		case models.ActionDeleteTag: // Delete Tag
-			notification.NotifyDeleteRef(pusher, repo, "tag", opts.RefFullName)
-		default:
-			isHookEventPush = false
-		}
-
-		if isHookEventPush {
-			notification.NotifyPushCommits(pusher, repo, opts.RefFullName, opts.OldCommitID, opts.NewCommitID, opts.Commits)
+			notification.NotifyPushCommits(pusher, repo, &opts.PushUpdateOptions, opts.Commits)
 		}
 	}
 
@@ -279,8 +239,5 @@ func CommitRepoAction(optsList ...*CommitRepoActionOptions) error {
 		}
 	}
 
-	if err := models.NotifyWatchers(actions...); err != nil {
-		return fmt.Errorf("NotifyWatchers: %v", err)
-	}
 	return nil
 }