Skip to content

Commit 89915ca

Browse files
sapklunny
sapk
authored and
lunny
committed
Fix duplicate call of webhook (#7821) (#7824)
1 parent 24fa568 commit 89915ca

File tree

6 files changed

+34
-113
lines changed

6 files changed

+34
-113
lines changed

‎integrations/pull_merge_test.go

+34
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ func testPullCleanUp(t *testing.T, session *TestSession, user, repo, pullnum str
5454

5555
func TestPullMerge(t *testing.T) {
5656
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
57+
hookTasks, err := models.HookTasks(1, 1) //Retrieve previous hook number
58+
assert.NoError(t, err)
59+
hookTasksLenBefore := len(hookTasks)
60+
5761
session := loginUser(t, "user1")
5862
testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
5963
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
@@ -63,11 +67,19 @@ func TestPullMerge(t *testing.T) {
6367
elem := strings.Split(test.RedirectURL(resp), "/")
6468
assert.EqualValues(t, "pulls", elem[3])
6569
testPullMerge(t, session, elem[1], elem[2], elem[4], models.MergeStyleMerge)
70+
71+
hookTasks, err = models.HookTasks(1, 1)
72+
assert.NoError(t, err)
73+
assert.Len(t, hookTasks, hookTasksLenBefore+1)
6674
})
6775
}
6876

6977
func TestPullRebase(t *testing.T) {
7078
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
79+
hookTasks, err := models.HookTasks(1, 1) //Retrieve previous hook number
80+
assert.NoError(t, err)
81+
hookTasksLenBefore := len(hookTasks)
82+
7183
session := loginUser(t, "user1")
7284
testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
7385
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
@@ -77,12 +89,21 @@ func TestPullRebase(t *testing.T) {
7789
elem := strings.Split(test.RedirectURL(resp), "/")
7890
assert.EqualValues(t, "pulls", elem[3])
7991
testPullMerge(t, session, elem[1], elem[2], elem[4], models.MergeStyleRebase)
92+
93+
hookTasks, err = models.HookTasks(1, 1)
94+
assert.NoError(t, err)
95+
assert.Len(t, hookTasks, hookTasksLenBefore+1)
8096
})
8197
}
8298

8399
func TestPullRebaseMerge(t *testing.T) {
84100
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
85101
prepareTestEnv(t)
102+
103+
hookTasks, err := models.HookTasks(1, 1) //Retrieve previous hook number
104+
assert.NoError(t, err)
105+
hookTasksLenBefore := len(hookTasks)
106+
86107
session := loginUser(t, "user1")
87108
testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
88109
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
@@ -92,12 +113,21 @@ func TestPullRebaseMerge(t *testing.T) {
92113
elem := strings.Split(test.RedirectURL(resp), "/")
93114
assert.EqualValues(t, "pulls", elem[3])
94115
testPullMerge(t, session, elem[1], elem[2], elem[4], models.MergeStyleRebaseMerge)
116+
117+
hookTasks, err = models.HookTasks(1, 1)
118+
assert.NoError(t, err)
119+
assert.Len(t, hookTasks, hookTasksLenBefore+1)
95120
})
96121
}
97122

98123
func TestPullSquash(t *testing.T) {
99124
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
100125
prepareTestEnv(t)
126+
127+
hookTasks, err := models.HookTasks(1, 1) //Retrieve previous hook number
128+
assert.NoError(t, err)
129+
hookTasksLenBefore := len(hookTasks)
130+
101131
session := loginUser(t, "user1")
102132
testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
103133
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
@@ -108,6 +138,10 @@ func TestPullSquash(t *testing.T) {
108138
elem := strings.Split(test.RedirectURL(resp), "/")
109139
assert.EqualValues(t, "pulls", elem[3])
110140
testPullMerge(t, session, elem[1], elem[2], elem[4], models.MergeStyleSquash)
141+
142+
hookTasks, err = models.HookTasks(1, 1)
143+
assert.NoError(t, err)
144+
assert.Len(t, hookTasks, hookTasksLenBefore+1)
111145
})
112146
}
113147

‎models/webhook.go

-1
Original file line numberDiff line numberDiff line change
@@ -890,7 +890,6 @@ func DeliverHooks() {
890890
for _, t := range tasks {
891891
if err = t.deliver(); err != nil {
892892
log.Error("deliver: %v", err)
893-
continue
894893
}
895894
}
896895

‎modules/pull/merge.go

-33
Original file line numberDiff line numberDiff line change
@@ -292,39 +292,6 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor
292292
go models.HookQueue.Add(pr.Issue.Repo.ID)
293293
}
294294

295-
l, err := baseGitRepo.CommitsBetweenIDs(pr.MergedCommitID, pr.MergeBase)
296-
if err != nil {
297-
log.Error("CommitsBetweenIDs: %v", err)
298-
return nil
299-
}
300-
301-
// It is possible that head branch is not fully sync with base branch for merge commits,
302-
// so we need to get latest head commit and append merge commit manually
303-
// to avoid strange diff commits produced.
304-
mergeCommit, err := baseGitRepo.GetBranchCommit(pr.BaseBranch)
305-
if err != nil {
306-
log.Error("GetBranchCommit: %v", err)
307-
return nil
308-
}
309-
if mergeStyle == models.MergeStyleMerge {
310-
l.PushFront(mergeCommit)
311-
}
312-
313-
p := &api.PushPayload{
314-
Ref: git.BranchPrefix + pr.BaseBranch,
315-
Before: pr.MergeBase,
316-
After: mergeCommit.ID.String(),
317-
CompareURL: setting.AppURL + pr.BaseRepo.ComposeCompareURL(pr.MergeBase, pr.MergedCommitID),
318-
Commits: models.ListToPushCommits(l).ToAPIPayloadCommits(pr.BaseRepo.HTMLURL()),
319-
Repo: pr.BaseRepo.APIFormat(mode),
320-
Pusher: pr.HeadRepo.MustOwner().APIFormat(),
321-
Sender: doer.APIFormat(),
322-
}
323-
if err = models.PrepareWebhooks(pr.BaseRepo, models.HookEventPush, p); err != nil {
324-
log.Error("PrepareWebhooks: %v", err)
325-
} else {
326-
go models.HookQueue.Add(pr.BaseRepo.ID)
327-
}
328295
return nil
329296
}
330297

‎modules/repofiles/delete.go

-26
Original file line numberDiff line numberDiff line change
@@ -178,32 +178,6 @@ func DeleteRepoFile(repo *models.Repository, doer *models.User, opts *DeleteRepo
178178
return nil, err
179179
}
180180

181-
// Simulate push event.
182-
oldCommitID := opts.LastCommitID
183-
if opts.NewBranch != opts.OldBranch {
184-
oldCommitID = git.EmptySHA
185-
}
186-
187-
if err = repo.GetOwner(); err != nil {
188-
return nil, fmt.Errorf("GetOwner: %v", err)
189-
}
190-
err = PushUpdate(
191-
repo,
192-
opts.NewBranch,
193-
models.PushUpdateOptions{
194-
PusherID: doer.ID,
195-
PusherName: doer.Name,
196-
RepoUserName: repo.Owner.Name,
197-
RepoName: repo.Name,
198-
RefFullName: git.BranchPrefix + opts.NewBranch,
199-
OldCommitID: oldCommitID,
200-
NewCommitID: commitHash,
201-
},
202-
)
203-
if err != nil {
204-
return nil, fmt.Errorf("PushUpdate: %v", err)
205-
}
206-
207181
commit, err = t.GetCommit(commitHash)
208182
if err != nil {
209183
return nil, err

‎modules/repofiles/update.go

-26
Original file line numberDiff line numberDiff line change
@@ -394,32 +394,6 @@ func CreateOrUpdateRepoFile(repo *models.Repository, doer *models.User, opts *Up
394394
return nil, err
395395
}
396396

397-
// Simulate push event.
398-
oldCommitID := opts.LastCommitID
399-
if opts.NewBranch != opts.OldBranch || oldCommitID == "" {
400-
oldCommitID = git.EmptySHA
401-
}
402-
403-
if err = repo.GetOwner(); err != nil {
404-
return nil, fmt.Errorf("GetOwner: %v", err)
405-
}
406-
err = PushUpdate(
407-
repo,
408-
opts.NewBranch,
409-
models.PushUpdateOptions{
410-
PusherID: doer.ID,
411-
PusherName: doer.Name,
412-
RepoUserName: repo.Owner.Name,
413-
RepoName: repo.Name,
414-
RefFullName: git.BranchPrefix + opts.NewBranch,
415-
OldCommitID: oldCommitID,
416-
NewCommitID: commitHash,
417-
},
418-
)
419-
if err != nil {
420-
return nil, fmt.Errorf("PushUpdate: %v", err)
421-
}
422-
423397
commit, err = t.GetCommit(commitHash)
424398
if err != nil {
425399
return nil, err

‎modules/repofiles/upload.go

-27
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"strings"
1212

1313
"code.gitea.io/gitea/models"
14-
"code.gitea.io/gitea/modules/git"
1514
"code.gitea.io/gitea/modules/lfs"
1615
"code.gitea.io/gitea/modules/setting"
1716
)
@@ -177,31 +176,5 @@ func UploadRepoFiles(repo *models.Repository, doer *models.User, opts *UploadRep
177176
return err
178177
}
179178

180-
// Simulate push event.
181-
oldCommitID := opts.LastCommitID
182-
if opts.NewBranch != opts.OldBranch {
183-
oldCommitID = git.EmptySHA
184-
}
185-
186-
if err = repo.GetOwner(); err != nil {
187-
return fmt.Errorf("GetOwner: %v", err)
188-
}
189-
err = PushUpdate(
190-
repo,
191-
opts.NewBranch,
192-
models.PushUpdateOptions{
193-
PusherID: doer.ID,
194-
PusherName: doer.Name,
195-
RepoUserName: repo.Owner.Name,
196-
RepoName: repo.Name,
197-
RefFullName: git.BranchPrefix + opts.NewBranch,
198-
OldCommitID: oldCommitID,
199-
NewCommitID: commitHash,
200-
},
201-
)
202-
if err != nil {
203-
return fmt.Errorf("PushUpdate: %v", err)
204-
}
205-
206179
return models.DeleteUploads(uploads...)
207180
}

0 commit comments

Comments
 (0)