From 79380c209d78c258948f3b861be97bbf9c7d3974 Mon Sep 17 00:00:00 2001 From: oliverpool Date: Tue, 30 Apr 2024 09:29:44 +0200 Subject: [PATCH 1/2] test: webhook fix branch filter tests --- models/fixtures/webhook.yml | 3 +- services/webhook/webhook_test.go | 65 +++++++++++++++++++++----------- 2 files changed, 45 insertions(+), 23 deletions(-) diff --git a/models/fixtures/webhook.yml b/models/fixtures/webhook.yml index f0b64cd57..cab5c5aca 100644 --- a/models/fixtures/webhook.yml +++ b/models/fixtures/webhook.yml @@ -29,8 +29,9 @@ - id: 4 repo_id: 2 + type: gitea url: http://www.example.com/url4 http_method: POST content_type: 1 # json - events: '{"push_only":true,"branch_filter":"{master,feature*}"}' + events: '{"send_everything":true,"branch_filter":"{master,feature*}"}' is_active: false diff --git a/services/webhook/webhook_test.go b/services/webhook/webhook_test.go index 2436b5a4d..f8b66d46f 100644 --- a/services/webhook/webhook_test.go +++ b/services/webhook/webhook_test.go @@ -4,6 +4,7 @@ package webhook import ( + "fmt" "testing" "code.gitea.io/gitea/models/db" @@ -41,38 +42,58 @@ func TestPrepareWebhooks(t *testing.T) { } } +func eventType(p api.Payloader) webhook_module.HookEventType { + switch p.(type) { + case *api.CreatePayload: + return webhook_module.HookEventCreate + case *api.DeletePayload: + return webhook_module.HookEventDelete + case *api.PushPayload: + return webhook_module.HookEventPush + } + panic(fmt.Sprintf("no event type for payload %T", p)) +} + func TestPrepareWebhooksBranchFilterMatch(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) - activateWebhook(t, 4) + // branch_filter: {master,feature*} + w := unittest.AssertExistsAndLoadBean(t, &webhook_model.Webhook{ID: 4}) + activateWebhook(t, w.ID) - hookTasks := []*webhook_model.HookTask{ - {HookID: 4, EventType: webhook_module.HookEventPush}, - } - for _, hookTask := range hookTasks { - unittest.AssertNotExistsBean(t, hookTask) - } - // this test also ensures that * doesn't handle / in any special way (like shell would) - assert.NoError(t, PrepareWebhooks(db.DefaultContext, EventSource{Repository: repo}, webhook_module.HookEventPush, &api.PushPayload{Ref: "refs/heads/feature/7791", Commits: []*api.PayloadCommit{{}}})) - for _, hookTask := range hookTasks { - unittest.AssertExistsAndLoadBean(t, hookTask) + for _, p := range []api.Payloader{ + &api.PushPayload{Ref: "refs/heads/feature/7791"}, + &api.CreatePayload{Ref: "refs/heads/feature/7791"}, // branch creation + &api.DeletePayload{Ref: "refs/heads/feature/7791"}, // branch deletion + } { + t.Run(fmt.Sprintf("%T", p), func(t *testing.T) { + db.DeleteBeans(db.DefaultContext, webhook_model.HookTask{HookID: w.ID}) + typ := eventType(p) + assert.NoError(t, PrepareWebhook(db.DefaultContext, w, typ, p)) + unittest.AssertExistsAndLoadBean(t, &webhook_model.HookTask{ + HookID: w.ID, + EventType: typ, + }) + }) } } func TestPrepareWebhooksBranchFilterNoMatch(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) - hookTasks := []*webhook_model.HookTask{ - {HookID: 4, EventType: webhook_module.HookEventPush}, - } - for _, hookTask := range hookTasks { - unittest.AssertNotExistsBean(t, hookTask) - } - assert.NoError(t, PrepareWebhooks(db.DefaultContext, EventSource{Repository: repo}, webhook_module.HookEventPush, &api.PushPayload{Ref: "refs/heads/fix_weird_bug"})) + // branch_filter: {master,feature*} + w := unittest.AssertExistsAndLoadBean(t, &webhook_model.Webhook{ID: 4}) + activateWebhook(t, w.ID) - for _, hookTask := range hookTasks { - unittest.AssertNotExistsBean(t, hookTask) + for _, p := range []api.Payloader{ + &api.PushPayload{Ref: "refs/heads/fix_weird_bug"}, + &api.CreatePayload{Ref: "refs/heads/fix_weird_bug"}, // branch creation + &api.DeletePayload{Ref: "refs/heads/fix_weird_bug"}, // branch deletion + } { + t.Run(fmt.Sprintf("%T", p), func(t *testing.T) { + db.DeleteBeans(db.DefaultContext, webhook_model.HookTask{HookID: w.ID}) + assert.NoError(t, PrepareWebhook(db.DefaultContext, w, eventType(p), p)) + unittest.AssertNotExistsBean(t, &webhook_model.HookTask{HookID: w.ID}) + }) } } From df06904f4ac65dea87534dd33db7554b5373b44e Mon Sep 17 00:00:00 2001 From: oliverpool Date: Tue, 30 Apr 2024 09:30:29 +0200 Subject: [PATCH 2/2] webhook: fix getPayloadBranch --- services/webhook/webhook.go | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/services/webhook/webhook.go b/services/webhook/webhook.go index cf4f2fdfd..1366ea8e8 100644 --- a/services/webhook/webhook.go +++ b/services/webhook/webhook.go @@ -82,19 +82,17 @@ var hookQueue *queue.WorkerPoolQueue[int64] // getPayloadBranch returns branch for hook event, if applicable. func getPayloadBranch(p api.Payloader) string { + var ref string switch pp := p.(type) { case *api.CreatePayload: - if pp.RefType == "branch" { - return pp.Ref - } + ref = pp.Ref case *api.DeletePayload: - if pp.RefType == "branch" { - return pp.Ref - } + ref = pp.Ref case *api.PushPayload: - if strings.HasPrefix(pp.Ref, git.BranchPrefix) { - return pp.Ref[len(git.BranchPrefix):] - } + ref = pp.Ref + } + if strings.HasPrefix(ref, git.BranchPrefix) { + return ref[len(git.BranchPrefix):] } return "" }