From ab22ab4a37110e989e2060fb088798e783dfcec7 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 8 Dec 2020 10:23:18 +0800 Subject: [PATCH] Refactor push update (#13381) * Refactor Push update * Remove the push_test since the function has been removed. * Use default branch setting instead master --- modules/notification/action/action.go | 69 +++++++++++ services/repository/push.go | 167 ++++++-------------------- services/repository/push_test.go | 139 --------------------- 3 files changed, 107 insertions(+), 268 deletions(-) delete mode 100644 services/repository/push_test.go diff --git a/modules/notification/action/action.go b/modules/notification/action/action.go index 13c2ca81d..75d2aa301 100644 --- a/modules/notification/action/action.go +++ b/modules/notification/action/action.go @@ -275,6 +275,75 @@ func (*actionNotifier) NotifyMergePullRequest(pr *models.PullRequest, doer *mode } } +func (a *actionNotifier) NotifyPushCommits(pusher *models.User, repo *models.Repository, opts *repository.PushUpdateOptions, commits *repository.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, opts *repository.PushUpdateOptions, commits *repository.PushCommits) { data, err := json.Marshal(commits) if err != nil { diff --git a/services/repository/push.go b/services/repository/push.go index b7c261ec3..2e7f1a152 100644 --- a/services/repository/push.go +++ b/services/repository/push.go @@ -6,7 +6,6 @@ package repository import ( "container/list" - "encoding/json" "fmt" "time" @@ -90,7 +89,6 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { addTags := make([]string, 0, len(optsList)) delTags := make([]string, 0, len(optsList)) - actions := make([]*commitRepoActionOptions, 0, len(optsList)) var pusher *models.User for _, opts := range optsList { @@ -102,8 +100,10 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { tagName := opts.TagName() if opts.IsDelRef() { delTags = append(delTags, tagName) + notification.NotifyDeleteRef(pusher, repo, "tag", opts.RefFullName) } else { // is new tag addTags = append(addTags, tagName) + notification.NotifyCreateRef(pusher, repo, "tag", opts.RefFullName) } } else if opts.IsBranch() { // If is branch reference if pusher == nil || pusher.ID != opts.PusherID { @@ -123,13 +123,32 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { return fmt.Errorf("gitRepo.GetCommit: %v", err) } + refName := opts.RefName() + // Push new branch. var l *list.List if opts.IsNewRef() { + if repo.IsEmpty { // Change default branch and empty status only if pushed ref is non-empty branch. + repo.DefaultBranch = refName + repo.IsEmpty = false + if repo.DefaultBranch != setting.Repository.DefaultBranch { + if err := gitRepo.SetDefaultBranch(repo.DefaultBranch); err != nil { + if !git.IsErrUnsupportedVersion(err) { + return err + } + } + } + // Update the is empty and default_branch columns + if err := models.UpdateRepositoryCols(repo, "default_branch", "is_empty"); err != nil { + return fmt.Errorf("UpdateRepositoryCols: %v", err) + } + } + l, err = newCommit.CommitsBeforeLimit(10) if err != nil { return fmt.Errorf("newCommit.CommitsBeforeLimit: %v", err) } + notification.NotifyCreateRef(pusher, repo, "branch", opts.RefFullName) } else { l, err = newCommit.CommitsBeforeUntil(opts.OldCommitID) if err != nil { @@ -152,6 +171,15 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { } commits = repo_module.ListToPushCommits(l) + if len(commits.Commits) > setting.UI.FeedMaxCommitNum { + commits.Commits = commits.Commits[:setting.UI.FeedMaxCommitNum] + } + commits.CompareURL = repo.ComposeCompareURL(opts.OldCommitID, opts.NewCommitID) + notification.NotifyPushCommits(pusher, repo, opts, commits) + + if err := repofiles.UpdateIssuesCommit(pusher, repo, commits.Commits, refName); err != nil { + log.Error("updateIssuesCommit: %v", err) + } if err = models.RemoveDeletedBranch(repo.ID, branch); err != nil { log.Error("models.RemoveDeletedBranch %s/%s failed: %v", repo.ID, branch, err) @@ -161,149 +189,30 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { if err := repo_module.CacheRef(repo, gitRepo, opts.RefFullName); err != nil { log.Error("repo_module.CacheRef %s/%s failed: %v", repo.ID, branch, err) } - } else if err = pull_service.CloseBranchPulls(pusher, repo.ID, branch); err != nil { - // close all related pulls - log.Error("close related pull request failed: %v", err) + } else { + notification.NotifyDeleteRef(pusher, repo, "branch", opts.RefFullName) + if err = pull_service.CloseBranchPulls(pusher, repo.ID, branch); err != nil { + // close all related pulls + log.Error("close related pull request failed: %v", err) + } } // Even if user delete a branch on a repository which he didn't watch, he will be watch that. if err = models.WatchIfAuto(opts.PusherID, repo.ID, true); err != nil { log.Warn("Fail to perform auto watch on user %v for repo %v: %v", opts.PusherID, repo.ID, err) } + } else { + log.Trace("Non-tag and non-branch commits pushed.") } - actions = append(actions, &commitRepoActionOptions{ - PushUpdateOptions: *opts, - Pusher: pusher, - RepoOwnerID: repo.OwnerID, - Commits: commits, - }) } if err := repo_module.PushUpdateAddDeleteTags(repo, gitRepo, addTags, delTags); err != nil { return fmt.Errorf("PushUpdateAddDeleteTags: %v", err) } - if err := commitRepoAction(repo, gitRepo, actions...); err != nil { - return fmt.Errorf("commitRepoAction: %v", err) - } - - return nil -} - -// commitRepoActionOptions represent options of a new commit action. -type commitRepoActionOptions struct { - repo_module.PushUpdateOptions - - Pusher *models.User - RepoOwnerID int64 - Commits *repo_module.PushCommits -} - -// commitRepoAction adds new commit action to the repository, and prepare -// corresponding webhooks. -func commitRepoAction(repo *models.Repository, gitRepo *git.Repository, optsList ...*commitRepoActionOptions) error { - actions := make([]*models.Action, len(optsList)) - - for i, opts := range optsList { - if opts.Pusher == nil || opts.Pusher.Name != opts.PusherName { - var err error - opts.Pusher, err = models.GetUserByName(opts.PusherName) - if err != nil { - return fmt.Errorf("GetUserByName [%s]: %v", opts.PusherName, err) - } - } - - refName := git.RefEndName(opts.RefFullName) - - // Change default branch and empty status only if pushed ref is non-empty branch. - if repo.IsEmpty && opts.IsBranch() && !opts.IsDelRef() { - repo.DefaultBranch = refName - repo.IsEmpty = false - if refName != "master" { - if err := gitRepo.SetDefaultBranch(repo.DefaultBranch); err != nil { - if !git.IsErrUnsupportedVersion(err) { - return err - } - } - } - // Update the is empty and default_branch columns - if err := models.UpdateRepositoryCols(repo, "default_branch", "is_empty"); err != nil { - return fmt.Errorf("UpdateRepositoryCols: %v", err) - } - } - - opType := models.ActionCommitRepo - - // Check it's tag push or branch. - if opts.IsTag() { - opType = models.ActionPushTag - if opts.IsDelRef() { - opType = models.ActionDeleteTag - } - opts.Commits = &repo_module.PushCommits{} - } else if opts.IsDelRef() { - opType = models.ActionDeleteBranch - opts.Commits = &repo_module.PushCommits{} - } else { - // if not the first commit, set the compare URL. - if !opts.IsNewRef() { - opts.Commits.CompareURL = repo.ComposeCompareURL(opts.OldCommitID, opts.NewCommitID) - } - - if err := repofiles.UpdateIssuesCommit(opts.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: opts.Pusher.ID, - ActUser: opts.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(opts.Pusher, repo, "branch", opts.RefFullName) - } - case models.ActionDeleteBranch: // Delete Branch - notification.NotifyDeleteRef(opts.Pusher, repo, "branch", opts.RefFullName) - - case models.ActionPushTag: // Create - notification.NotifyCreateRef(opts.Pusher, repo, "tag", opts.RefFullName) - - case models.ActionDeleteTag: // Delete Tag - notification.NotifyDeleteRef(opts.Pusher, repo, "tag", opts.RefFullName) - default: - isHookEventPush = false - } - - if isHookEventPush { - notification.NotifyPushCommits(opts.Pusher, repo, &opts.PushUpdateOptions, opts.Commits) - } - } - // Change repository last updated time. if err := models.UpdateRepositoryUpdatedTime(repo.ID, time.Now()); err != nil { return fmt.Errorf("UpdateRepositoryUpdatedTime: %v", err) } - if err := models.NotifyWatchers(actions...); err != nil { - return fmt.Errorf("NotifyWatchers: %v", err) - } return nil } diff --git a/services/repository/push_test.go b/services/repository/push_test.go deleted file mode 100644 index da24c19d5..000000000 --- a/services/repository/push_test.go +++ /dev/null @@ -1,139 +0,0 @@ -// Copyright 2020 The Gitea Authors. All rights reserved. -// Use of this source code is governed by a MIT-style -// license that can be found in the LICENSE file. - -package repository - -import ( - "testing" - - "code.gitea.io/gitea/models" - "code.gitea.io/gitea/modules/git" - repo_module "code.gitea.io/gitea/modules/repository" - - "github.com/stretchr/testify/assert" -) - -func testCorrectRepoAction(t *testing.T, repo *models.Repository, gitRepo *git.Repository, opts *commitRepoActionOptions, actionBean *models.Action) { - models.AssertNotExistsBean(t, actionBean) - assert.NoError(t, commitRepoAction(repo, gitRepo, opts)) - models.AssertExistsAndLoadBean(t, actionBean) - models.CheckConsistencyFor(t, &models.Action{}) -} - -func TestCommitRepoAction(t *testing.T) { - samples := []struct { - userID int64 - repositoryID int64 - commitRepoActionOptions commitRepoActionOptions - action models.Action - }{ - { - userID: 2, - repositoryID: 16, - commitRepoActionOptions: commitRepoActionOptions{ - PushUpdateOptions: repo_module.PushUpdateOptions{ - RefFullName: "refName", - OldCommitID: "oldCommitID", - NewCommitID: "newCommitID", - }, - Commits: &repo_module.PushCommits{ - Commits: []*repo_module.PushCommit{ - { - Sha1: "69554a6", - CommitterEmail: "user2@example.com", - CommitterName: "User2", - AuthorEmail: "user2@example.com", - AuthorName: "User2", - Message: "not signed commit", - }, - { - Sha1: "27566bd", - CommitterEmail: "user2@example.com", - CommitterName: "User2", - AuthorEmail: "user2@example.com", - AuthorName: "User2", - Message: "good signed commit (with not yet validated email)", - }, - }, - Len: 2, - }, - }, - action: models.Action{ - OpType: models.ActionCommitRepo, - RefName: "refName", - }, - }, - { - userID: 2, - repositoryID: 1, - commitRepoActionOptions: commitRepoActionOptions{ - PushUpdateOptions: repo_module.PushUpdateOptions{ - RefFullName: git.TagPrefix + "v1.1", - OldCommitID: git.EmptySHA, - NewCommitID: "newCommitID", - }, - Commits: &repo_module.PushCommits{}, - }, - action: models.Action{ - OpType: models.ActionPushTag, - RefName: "v1.1", - }, - }, - { - userID: 2, - repositoryID: 1, - commitRepoActionOptions: commitRepoActionOptions{ - PushUpdateOptions: repo_module.PushUpdateOptions{ - RefFullName: git.TagPrefix + "v1.1", - OldCommitID: "oldCommitID", - NewCommitID: git.EmptySHA, - }, - Commits: &repo_module.PushCommits{}, - }, - action: models.Action{ - OpType: models.ActionDeleteTag, - RefName: "v1.1", - }, - }, - { - userID: 2, - repositoryID: 1, - commitRepoActionOptions: commitRepoActionOptions{ - PushUpdateOptions: repo_module.PushUpdateOptions{ - RefFullName: git.BranchPrefix + "feature/1", - OldCommitID: "oldCommitID", - NewCommitID: git.EmptySHA, - }, - Commits: &repo_module.PushCommits{}, - }, - action: models.Action{ - OpType: models.ActionDeleteBranch, - RefName: "feature/1", - }, - }, - } - - for _, s := range samples { - models.PrepareTestEnv(t) - - user := models.AssertExistsAndLoadBean(t, &models.User{ID: s.userID}).(*models.User) - repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: s.repositoryID, OwnerID: user.ID}).(*models.Repository) - repo.Owner = user - - gitRepo, err := git.OpenRepository(repo.RepoPath()) - assert.NoError(t, err) - - s.commitRepoActionOptions.PusherName = user.Name - s.commitRepoActionOptions.RepoOwnerID = user.ID - s.commitRepoActionOptions.RepoName = repo.Name - - s.action.ActUserID = user.ID - s.action.RepoID = repo.ID - s.action.Repo = repo - s.action.IsPrivate = repo.IsPrivate - - testCorrectRepoAction(t, repo, gitRepo, &s.commitRepoActionOptions, &s.action) - gitRepo.Close() - } -}