Skip to content

Commit 23971a7

Browse files
lunnylunny
authored
Add tests for webhook and fix some webhook bugs (#33396) (#33442)
This PR created a mock webhook server in the tests and added integration tests for generic webhooks. It also fixes bugs in package webhooks and pull request comment webhooks. This also corrected an error on the package webhook. The previous implementation uses a `User` struct as an organization, now it has been corrected but it will not be consistent with the previous implementation, some fields which not belong to the organization have been removed. Backport #33396 Backport part of #33337
1 parent ebac324 commit 23971a7

26 files changed

+682
-105
lines changed

‎models/webhook/webhook.go

+6
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,11 @@ func (w *Webhook) HasPackageEvent() bool {
299299
(w.ChooseEvents && w.HookEvents.Package)
300300
}
301301

302+
func (w *Webhook) HasStatusEvent() bool {
303+
return w.SendEverything ||
304+
(w.ChooseEvents && w.HookEvents.Status)
305+
}
306+
302307
// HasPullRequestReviewRequestEvent returns true if hook enabled pull request review request event.
303308
func (w *Webhook) HasPullRequestReviewRequestEvent() bool {
304309
return w.SendEverything ||
@@ -337,6 +342,7 @@ func (w *Webhook) EventCheckers() []struct {
337342
{w.HasReleaseEvent, webhook_module.HookEventRelease},
338343
{w.HasPackageEvent, webhook_module.HookEventPackage},
339344
{w.HasPullRequestReviewRequestEvent, webhook_module.HookEventPullRequestReviewRequest},
345+
{w.HasStatusEvent, webhook_module.HookEventStatus},
340346
}
341347
}
342348

‎models/webhook/webhook_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func TestWebhook_EventsArray(t *testing.T) {
7474
"pull_request", "pull_request_assign", "pull_request_label", "pull_request_milestone",
7575
"pull_request_comment", "pull_request_review_approved", "pull_request_review_rejected",
7676
"pull_request_review_comment", "pull_request_sync", "wiki", "repository", "release",
77-
"package", "pull_request_review_request",
77+
"package", "pull_request_review_request", "status",
7878
},
7979
(&Webhook{
8080
HookEvent: &webhook_module.HookEvent{SendEverything: true},

‎modules/structs/hook.go

+2-58
Original file line numberDiff line numberDiff line change
@@ -116,14 +116,7 @@ var (
116116
_ Payloader = &PackagePayload{}
117117
)
118118

119-
// _________ __
120-
// \_ ___ \_______ ____ _____ _/ |_ ____
121-
// / \ \/\_ __ \_/ __ \\__ \\ __\/ __ \
122-
// \ \____| | \/\ ___/ / __ \| | \ ___/
123-
// \______ /|__| \___ >____ /__| \___ >
124-
// \/ \/ \/ \/
125-
126-
// CreatePayload FIXME
119+
// CreatePayload represents a payload information of create event.
127120
type CreatePayload struct {
128121
Sha string `json:"sha"`
129122
Ref string `json:"ref"`
@@ -157,13 +150,6 @@ func ParseCreateHook(raw []byte) (*CreatePayload, error) {
157150
return hook, nil
158151
}
159152

160-
// ________ .__ __
161-
// \______ \ ____ | | _____/ |_ ____
162-
// | | \_/ __ \| | _/ __ \ __\/ __ \
163-
// | ` \ ___/| |_\ ___/| | \ ___/
164-
// /_______ /\___ >____/\___ >__| \___ >
165-
// \/ \/ \/ \/
166-
167153
// PusherType define the type to push
168154
type PusherType string
169155

@@ -186,13 +172,6 @@ func (p *DeletePayload) JSONPayload() ([]byte, error) {
186172
return json.MarshalIndent(p, "", " ")
187173
}
188174

189-
// ___________ __
190-
// \_ _____/__________| | __
191-
// | __)/ _ \_ __ \ |/ /
192-
// | \( <_> ) | \/ <
193-
// \___ / \____/|__| |__|_ \
194-
// \/ \/
195-
196175
// ForkPayload represents fork payload
197176
type ForkPayload struct {
198177
Forkee *Repository `json:"forkee"`
@@ -232,13 +211,6 @@ func (p *IssueCommentPayload) JSONPayload() ([]byte, error) {
232211
return json.MarshalIndent(p, "", " ")
233212
}
234213

235-
// __________ .__
236-
// \______ \ ____ | | ____ _____ ______ ____
237-
// | _// __ \| | _/ __ \\__ \ / ___// __ \
238-
// | | \ ___/| |_\ ___/ / __ \_\___ \\ ___/
239-
// |____|_ /\___ >____/\___ >____ /____ >\___ >
240-
// \/ \/ \/ \/ \/ \/
241-
242214
// HookReleaseAction defines hook release action type
243215
type HookReleaseAction string
244216

@@ -302,13 +274,6 @@ func (p *PushPayload) Branch() string {
302274
return strings.ReplaceAll(p.Ref, "refs/heads/", "")
303275
}
304276

305-
// .___
306-
// | | ______ ________ __ ____
307-
// | |/ ___// ___/ | \_/ __ \
308-
// | |\___ \ \___ \| | /\ ___/
309-
// |___/____ >____ >____/ \___ >
310-
// \/ \/ \/
311-
312277
// HookIssueAction FIXME
313278
type HookIssueAction string
314279

@@ -371,13 +336,6 @@ type ChangesPayload struct {
371336
Ref *ChangesFromPayload `json:"ref,omitempty"`
372337
}
373338

374-
// __________ .__ .__ __________ __
375-
// \______ \__ __| | | | \______ \ ____ ________ __ ____ _______/ |_
376-
// | ___/ | \ | | | | _// __ \/ ____/ | \_/ __ \ / ___/\ __\
377-
// | | | | / |_| |__ | | \ ___< <_| | | /\ ___/ \___ \ | |
378-
// |____| |____/|____/____/ |____|_ /\___ >__ |____/ \___ >____ > |__|
379-
// \/ \/ |__| \/ \/
380-
381339
// PullRequestPayload represents a payload information of pull request event.
382340
type PullRequestPayload struct {
383341
Action HookIssueAction `json:"action"`
@@ -402,13 +360,6 @@ type ReviewPayload struct {
402360
Content string `json:"content"`
403361
}
404362

405-
// __ __.__ __ .__
406-
// / \ / \__| | _|__|
407-
// \ \/\/ / | |/ / |
408-
// \ /| | <| |
409-
// \__/\ / |__|__|_ \__|
410-
// \/ \/
411-
412363
// HookWikiAction an action that happens to a wiki page
413364
type HookWikiAction string
414365

@@ -435,13 +386,6 @@ func (p *WikiPayload) JSONPayload() ([]byte, error) {
435386
return json.MarshalIndent(p, "", " ")
436387
}
437388

438-
//__________ .__ __
439-
//\______ \ ____ ______ ____ _____|__|/ |_ ___________ ___.__.
440-
// | _// __ \\____ \ / _ \/ ___/ \ __\/ _ \_ __ < | |
441-
// | | \ ___/| |_> > <_> )___ \| || | ( <_> ) | \/\___ |
442-
// |____|_ /\___ > __/ \____/____ >__||__| \____/|__| / ____|
443-
// \/ \/|__| \/ \/
444-
445389
// HookRepoAction an action that happens to a repo
446390
type HookRepoAction string
447391

@@ -480,7 +424,7 @@ type PackagePayload struct {
480424
Action HookPackageAction `json:"action"`
481425
Repository *Repository `json:"repository"`
482426
Package *Package `json:"package"`
483-
Organization *User `json:"organization"`
427+
Organization *Organization `json:"organization"`
484428
Sender *User `json:"sender"`
485429
}
486430

‎modules/webhook/structs.go

+1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ type HookEvents struct {
2626
Repository bool `json:"repository"`
2727
Release bool `json:"release"`
2828
Package bool `json:"package"`
29+
Status bool `json:"status"`
2930
}
3031

3132
// HookEvent represents events that will delivery hook.

‎modules/webhook/type.go

+2-15
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,6 @@ const (
3838
// Event returns the HookEventType as an event string
3939
func (h HookEventType) Event() string {
4040
switch h {
41-
case HookEventCreate:
42-
return "create"
43-
case HookEventDelete:
44-
return "delete"
45-
case HookEventFork:
46-
return "fork"
47-
case HookEventPush:
48-
return "push"
4941
case HookEventIssues, HookEventIssueAssign, HookEventIssueLabel, HookEventIssueMilestone:
5042
return "issues"
5143
case HookEventPullRequest, HookEventPullRequestAssign, HookEventPullRequestLabel, HookEventPullRequestMilestone,
@@ -59,14 +51,9 @@ func (h HookEventType) Event() string {
5951
return "pull_request_rejected"
6052
case HookEventPullRequestReviewComment:
6153
return "pull_request_comment"
62-
case HookEventWiki:
63-
return "wiki"
64-
case HookEventRepository:
65-
return "repository"
66-
case HookEventRelease:
67-
return "release"
54+
default:
55+
return string(h)
6856
}
69-
return ""
7057
}
7158

7259
// HookType is the type of a webhook

‎routers/api/v1/utils/hook.go

+2
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,8 @@ func addHook(ctx *context.APIContext, form *api.CreateHookOption, ownerID, repoI
205205
Wiki: util.SliceContainsString(form.Events, string(webhook_module.HookEventWiki), true),
206206
Repository: util.SliceContainsString(form.Events, string(webhook_module.HookEventRepository), true),
207207
Release: util.SliceContainsString(form.Events, string(webhook_module.HookEventRelease), true),
208+
Package: util.SliceContainsString(form.Events, string(webhook_module.HookEventPackage), true),
209+
Status: util.SliceContainsString(form.Events, string(webhook_module.HookEventStatus), true),
208210
},
209211
BranchFilter: form.BranchFilter,
210212
},

‎services/webhook/dingtalk.go

+4
Original file line numberDiff line numberDiff line change
@@ -190,3 +190,7 @@ func newDingtalkRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_
190190
var pc payloadConvertor[DingtalkPayload] = dingtalkConvertor{}
191191
return newJSONRequest(pc, w, t, true)
192192
}
193+
194+
func init() {
195+
RegisterWebhookRequester(webhook_module.DINGTALK, newDingtalkRequest)
196+
}

‎services/webhook/discord.go

+4
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,10 @@ func newDiscordRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_m
277277
return newJSONRequest(pc, w, t, true)
278278
}
279279

280+
func init() {
281+
RegisterWebhookRequester(webhook_module.DISCORD, newDiscordRequest)
282+
}
283+
280284
func parseHookPullRequestEventType(event webhook_module.HookEventType) (string, error) {
281285
switch event {
282286
case webhook_module.HookEventPullRequestReviewApproved:

‎services/webhook/feishu.go

+4
Original file line numberDiff line numberDiff line change
@@ -170,3 +170,7 @@ func newFeishuRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_mo
170170
var pc payloadConvertor[FeishuPayload] = feishuConvertor{}
171171
return newJSONRequest(pc, w, t, true)
172172
}
173+
174+
func init() {
175+
RegisterWebhookRequester(webhook_module.FEISHU, newFeishuRequest)
176+
}

‎services/webhook/general_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -319,8 +319,8 @@ func packageTestPayload() *api.PackagePayload {
319319
AvatarURL: "http://localhost:3000/user1/avatar",
320320
},
321321
Repository: nil,
322-
Organization: &api.User{
323-
UserName: "org1",
322+
Organization: &api.Organization{
323+
Name: "org1",
324324
AvatarURL: "http://localhost:3000/org1/avatar",
325325
},
326326
Package: &api.Package{

‎services/webhook/matrix.go

+4
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ import (
2424
webhook_module "code.gitea.io/gitea/modules/webhook"
2525
)
2626

27+
func init() {
28+
RegisterWebhookRequester(webhook_module.MATRIX, newMatrixRequest)
29+
}
30+
2731
func newMatrixRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_model.HookTask) (*http.Request, []byte, error) {
2832
meta := &MatrixMeta{}
2933
if err := json.Unmarshal([]byte(w.Meta), meta); err != nil {

‎services/webhook/msteams.go

+4
Original file line numberDiff line numberDiff line change
@@ -349,3 +349,7 @@ func newMSTeamsRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_m
349349
var pc payloadConvertor[MSTeamsPayload] = msteamsConvertor{}
350350
return newJSONRequest(pc, w, t, true)
351351
}
352+
353+
func init() {
354+
RegisterWebhookRequester(webhook_module.MSTEAMS, newMSTeamsRequest)
355+
}

‎services/webhook/notifier.go

+10-3
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88

99
git_model "code.gitea.io/gitea/models/git"
1010
issues_model "code.gitea.io/gitea/models/issues"
11+
"code.gitea.io/gitea/models/organization"
1112
packages_model "code.gitea.io/gitea/models/packages"
1213
"code.gitea.io/gitea/models/perm"
1314
access_model "code.gitea.io/gitea/models/perm/access"
@@ -924,10 +925,16 @@ func notifyPackage(ctx context.Context, sender *user_model.User, pd *packages_mo
924925
return
925926
}
926927

928+
var org *api.Organization
929+
if pd.Owner.IsOrganization() {
930+
org = convert.ToOrganization(ctx, organization.OrgFromUser(pd.Owner))
931+
}
932+
927933
if err := PrepareWebhooks(ctx, source, webhook_module.HookEventPackage, &api.PackagePayload{
928-
Action: action,
929-
Package: apiPackage,
930-
Sender: convert.ToUser(ctx, sender, nil),
934+
Action: action,
935+
Package: apiPackage,
936+
Organization: org,
937+
Sender: convert.ToUser(ctx, sender, nil),
931938
}); err != nil {
932939
log.Error("PrepareWebhooks: %v", err)
933940
}

‎services/webhook/packagist.go

+4
Original file line numberDiff line numberDiff line change
@@ -120,3 +120,7 @@ func newPackagistRequest(_ context.Context, w *webhook_model.Webhook, t *webhook
120120
}
121121
return newJSONRequest(pc, w, t, true)
122122
}
123+
124+
func init() {
125+
RegisterWebhookRequester(webhook_module.PACKAGIST, newPackagistRequest)
126+
}

‎services/webhook/slack.go

+4
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,10 @@ func newSlackRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_mod
295295
return newJSONRequest(pc, w, t, true)
296296
}
297297

298+
func init() {
299+
RegisterWebhookRequester(webhook_module.SLACK, newSlackRequest)
300+
}
301+
298302
var slackChannel = regexp.MustCompile(`^#?[a-z0-9_-]{1,80}$`)
299303

300304
// IsValidSlackChannel validates a channel name conforms to what slack expects:

‎services/webhook/telegram.go

+4
Original file line numberDiff line numberDiff line change
@@ -187,3 +187,7 @@ func newTelegramRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_
187187
var pc payloadConvertor[TelegramPayload] = telegramConvertor{}
188188
return newJSONRequest(pc, w, t, true)
189189
}
190+
191+
func init() {
192+
RegisterWebhookRequester(webhook_module.TELEGRAM, newTelegramRequest)
193+
}

‎services/webhook/webhook.go

+6-10
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,12 @@ import (
2727
"github.com/gobwas/glob"
2828
)
2929

30-
var webhookRequesters = map[webhook_module.HookType]func(context.Context, *webhook_model.Webhook, *webhook_model.HookTask) (req *http.Request, body []byte, err error){
31-
webhook_module.SLACK: newSlackRequest,
32-
webhook_module.DISCORD: newDiscordRequest,
33-
webhook_module.DINGTALK: newDingtalkRequest,
34-
webhook_module.TELEGRAM: newTelegramRequest,
35-
webhook_module.MSTEAMS: newMSTeamsRequest,
36-
webhook_module.FEISHU: newFeishuRequest,
37-
webhook_module.MATRIX: newMatrixRequest,
38-
webhook_module.WECHATWORK: newWechatworkRequest,
39-
webhook_module.PACKAGIST: newPackagistRequest,
30+
type Requester func(context.Context, *webhook_model.Webhook, *webhook_model.HookTask) (req *http.Request, body []byte, err error)
31+
32+
var webhookRequesters = map[webhook_module.HookType]Requester{}
33+
34+
func RegisterWebhookRequester(hookType webhook_module.HookType, requester Requester) {
35+
webhookRequesters[hookType] = requester
4036
}
4137

4238
// IsValidHookTaskType returns true if a webhook registered

‎services/webhook/wechatwork.go

+4
Original file line numberDiff line numberDiff line change
@@ -179,3 +179,7 @@ func newWechatworkRequest(_ context.Context, w *webhook_model.Webhook, t *webhoo
179179
var pc payloadConvertor[WechatworkPayload] = wechatworkConvertor{}
180180
return newJSONRequest(pc, w, t, true)
181181
}
182+
183+
func init() {
184+
RegisterWebhookRequester(webhook_module.WECHATWORK, newWechatworkRequest)
185+
}

‎tests/integration/api_repo_test.go

+10-5
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,15 @@ func TestAPIMirrorSyncNonMirrorRepo(t *testing.T) {
471471
assert.Equal(t, "Repository is not a mirror", errRespJSON["message"])
472472
}
473473

474+
func testAPIOrgCreateRepo(t *testing.T, session *TestSession, orgName, repoName string, status int) {
475+
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteOrganization, auth_model.AccessTokenScopeWriteRepository)
476+
477+
req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/org/%s/repos", orgName), &api.CreateRepoOption{
478+
Name: repoName,
479+
}).AddTokenAuth(token)
480+
MakeRequest(t, req, status)
481+
}
482+
474483
func TestAPIOrgRepoCreate(t *testing.T) {
475484
testCases := []struct {
476485
ctxUserID int64
@@ -488,11 +497,7 @@ func TestAPIOrgRepoCreate(t *testing.T) {
488497
for _, testCase := range testCases {
489498
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: testCase.ctxUserID})
490499
session := loginUser(t, user.Name)
491-
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteOrganization, auth_model.AccessTokenScopeWriteRepository)
492-
req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/org/%s/repos", testCase.orgName), &api.CreateRepoOption{
493-
Name: testCase.repoName,
494-
}).AddTokenAuth(token)
495-
MakeRequest(t, req, testCase.expectedStatus)
500+
testAPIOrgCreateRepo(t, session, testCase.orgName, testCase.repoName, testCase.expectedStatus)
496501
}
497502
}
498503

0 commit comments

Comments
 (0)