Fix duplicate call of webhook (#7821) (#7824)

integrations/pull_merge_test.go

@@ -54,6 +54,10 @@ func testPullCleanUp(t *testing.T, session *TestSession, user, repo, pullnum str
 
 func TestPullMerge(t *testing.T) {
 	onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+		hookTasks, err := models.HookTasks(1, 1) //Retrieve previous hook number
+		assert.NoError(t, err)
+		hookTasksLenBefore := len(hookTasks)
+
 		session := loginUser(t, "user1")
 		testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
 		testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
@@ -63,11 +67,19 @@ func TestPullMerge(t *testing.T) {
 		elem := strings.Split(test.RedirectURL(resp), "/")
 		assert.EqualValues(t, "pulls", elem[3])
 		testPullMerge(t, session, elem[1], elem[2], elem[4], models.MergeStyleMerge)
+
+		hookTasks, err = models.HookTasks(1, 1)
+		assert.NoError(t, err)
+		assert.Len(t, hookTasks, hookTasksLenBefore+1)
 	})
 }
 
 func TestPullRebase(t *testing.T) {
 	onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+		hookTasks, err := models.HookTasks(1, 1) //Retrieve previous hook number
+		assert.NoError(t, err)
+		hookTasksLenBefore := len(hookTasks)
+
 		session := loginUser(t, "user1")
 		testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
 		testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
@@ -77,12 +89,21 @@ func TestPullRebase(t *testing.T) {
 		elem := strings.Split(test.RedirectURL(resp), "/")
 		assert.EqualValues(t, "pulls", elem[3])
 		testPullMerge(t, session, elem[1], elem[2], elem[4], models.MergeStyleRebase)
+
+		hookTasks, err = models.HookTasks(1, 1)
+		assert.NoError(t, err)
+		assert.Len(t, hookTasks, hookTasksLenBefore+1)
 	})
 }
 
 func TestPullRebaseMerge(t *testing.T) {
 	onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
 		prepareTestEnv(t)
+
+		hookTasks, err := models.HookTasks(1, 1) //Retrieve previous hook number
+		assert.NoError(t, err)
+		hookTasksLenBefore := len(hookTasks)
+
 		session := loginUser(t, "user1")
 		testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
 		testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
@@ -92,12 +113,21 @@ func TestPullRebaseMerge(t *testing.T) {
 		elem := strings.Split(test.RedirectURL(resp), "/")
 		assert.EqualValues(t, "pulls", elem[3])
 		testPullMerge(t, session, elem[1], elem[2], elem[4], models.MergeStyleRebaseMerge)
+
+		hookTasks, err = models.HookTasks(1, 1)
+		assert.NoError(t, err)
+		assert.Len(t, hookTasks, hookTasksLenBefore+1)
 	})
 }
 
 func TestPullSquash(t *testing.T) {
 	onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
 		prepareTestEnv(t)
+
+		hookTasks, err := models.HookTasks(1, 1) //Retrieve previous hook number
+		assert.NoError(t, err)
+		hookTasksLenBefore := len(hookTasks)
+
 		session := loginUser(t, "user1")
 		testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
 		testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
@@ -108,6 +138,10 @@ func TestPullSquash(t *testing.T) {
 		elem := strings.Split(test.RedirectURL(resp), "/")
 		assert.EqualValues(t, "pulls", elem[3])
 		testPullMerge(t, session, elem[1], elem[2], elem[4], models.MergeStyleSquash)
+
+		hookTasks, err = models.HookTasks(1, 1)
+		assert.NoError(t, err)
+		assert.Len(t, hookTasks, hookTasksLenBefore+1)
 	})
 }
 

models/webhook.go

@@ -890,7 +890,6 @@ func DeliverHooks() {
 	for _, t := range tasks {
 		if err = t.deliver(); err != nil {
 			log.Error("deliver: %v", err)
-			continue
 		}
 	}
 

modules/pull/merge.go

@@ -292,39 +292,6 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor
 		go models.HookQueue.Add(pr.Issue.Repo.ID)
 	}
 
-	l, err := baseGitRepo.CommitsBetweenIDs(pr.MergedCommitID, pr.MergeBase)
-	if err != nil {
-		log.Error("CommitsBetweenIDs: %v", err)
-		return nil
-	}
-
-	// It is possible that head branch is not fully sync with base branch for merge commits,
-	// so we need to get latest head commit and append merge commit manually
-	// to avoid strange diff commits produced.
-	mergeCommit, err := baseGitRepo.GetBranchCommit(pr.BaseBranch)
-	if err != nil {
-		log.Error("GetBranchCommit: %v", err)
-		return nil
-	}
-	if mergeStyle == models.MergeStyleMerge {
-		l.PushFront(mergeCommit)
-	}
-
-	p := &api.PushPayload{
-		Ref:        git.BranchPrefix + pr.BaseBranch,
-		Before:     pr.MergeBase,
-		After:      mergeCommit.ID.String(),
-		CompareURL: setting.AppURL + pr.BaseRepo.ComposeCompareURL(pr.MergeBase, pr.MergedCommitID),
-		Commits:    models.ListToPushCommits(l).ToAPIPayloadCommits(pr.BaseRepo.HTMLURL()),
-		Repo:       pr.BaseRepo.APIFormat(mode),
-		Pusher:     pr.HeadRepo.MustOwner().APIFormat(),
-		Sender:     doer.APIFormat(),
-	}
-	if err = models.PrepareWebhooks(pr.BaseRepo, models.HookEventPush, p); err != nil {
-		log.Error("PrepareWebhooks: %v", err)
-	} else {
-		go models.HookQueue.Add(pr.BaseRepo.ID)
-	}
 	return nil
 }
 

modules/repofiles/delete.go

@@ -178,32 +178,6 @@ func DeleteRepoFile(repo *models.Repository, doer *models.User, opts *DeleteRepo
 		return nil, err
 	}
 
-	// Simulate push event.
-	oldCommitID := opts.LastCommitID
-	if opts.NewBranch != opts.OldBranch {
-		oldCommitID = git.EmptySHA
-	}
-
-	if err = repo.GetOwner(); err != nil {
-		return nil, fmt.Errorf("GetOwner: %v", err)
-	}
-	err = PushUpdate(
-		repo,
-		opts.NewBranch,
-		models.PushUpdateOptions{
-			PusherID:     doer.ID,
-			PusherName:   doer.Name,
-			RepoUserName: repo.Owner.Name,
-			RepoName:     repo.Name,
-			RefFullName:  git.BranchPrefix + opts.NewBranch,
-			OldCommitID:  oldCommitID,
-			NewCommitID:  commitHash,
-		},
-	)
-	if err != nil {
-		return nil, fmt.Errorf("PushUpdate: %v", err)
-	}
-
 	commit, err = t.GetCommit(commitHash)
 	if err != nil {
 		return nil, err

modules/repofiles/update.go

@@ -394,32 +394,6 @@ func CreateOrUpdateRepoFile(repo *models.Repository, doer *models.User, opts *Up
 		return nil, err
 	}
 
-	// Simulate push event.
-	oldCommitID := opts.LastCommitID
-	if opts.NewBranch != opts.OldBranch || oldCommitID == "" {
-		oldCommitID = git.EmptySHA
-	}
-
-	if err = repo.GetOwner(); err != nil {
-		return nil, fmt.Errorf("GetOwner: %v", err)
-	}
-	err = PushUpdate(
-		repo,
-		opts.NewBranch,
-		models.PushUpdateOptions{
-			PusherID:     doer.ID,
-			PusherName:   doer.Name,
-			RepoUserName: repo.Owner.Name,
-			RepoName:     repo.Name,
-			RefFullName:  git.BranchPrefix + opts.NewBranch,
-			OldCommitID:  oldCommitID,
-			NewCommitID:  commitHash,
-		},
-	)
-	if err != nil {
-		return nil, fmt.Errorf("PushUpdate: %v", err)
-	}
-
 	commit, err = t.GetCommit(commitHash)
 	if err != nil {
 		return nil, err

modules/repofiles/upload.go

@@ -11,7 +11,6 @@ import (
 	"strings"
 
 	"code.gitea.io/gitea/models"
-	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/lfs"
 	"code.gitea.io/gitea/modules/setting"
 )
@@ -177,31 +176,5 @@ func UploadRepoFiles(repo *models.Repository, doer *models.User, opts *UploadRep
 		return err
 	}
 
-	// Simulate push event.
-	oldCommitID := opts.LastCommitID
-	if opts.NewBranch != opts.OldBranch {
-		oldCommitID = git.EmptySHA
-	}
-
-	if err = repo.GetOwner(); err != nil {
-		return fmt.Errorf("GetOwner: %v", err)
-	}
-	err = PushUpdate(
-		repo,
-		opts.NewBranch,
-		models.PushUpdateOptions{
-			PusherID:     doer.ID,
-			PusherName:   doer.Name,
-			RepoUserName: repo.Owner.Name,
-			RepoName:     repo.Name,
-			RefFullName:  git.BranchPrefix + opts.NewBranch,
-			OldCommitID:  oldCommitID,
-			NewCommitID:  commitHash,
-		},
-	)
-	if err != nil {
-		return fmt.Errorf("PushUpdate: %v", err)
-	}
-
 	return models.DeleteUploads(uploads...)
 }