Skip to content

Commit e9bb75d

Browse files
sapklafriks
sapk
authored and
lafriks
committed
Fix duplicate call of webhook (#7821)
1 parent 8bfeb85 commit e9bb75d

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
@@ -396,32 +396,6 @@ func CreateOrUpdateRepoFile(repo *models.Repository, doer *models.User, opts *Up
396396
return nil, err
397397
}
398398

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