Merge pull request '[v7.0/forgejo] fix: webhook getPayloadBranch' (#3557) from bp-v7.0/forgejo-df06904-79380c2 into v7.0/forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/3557
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>

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

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" {
+		ref = pp.Ref
-			return pp.Ref
-		}
 	case *api.DeletePayload:
-		if pp.RefType == "branch" {
+		ref = pp.Ref
-			return pp.Ref
-		}
 	case *api.PushPayload:
-		if strings.HasPrefix(pp.Ref, git.BranchPrefix) {
+		ref = pp.Ref
-			return pp.Ref[len(git.BranchPrefix):]
+	}
-		}
+	if strings.HasPrefix(ref, git.BranchPrefix) {
+		return ref[len(git.BranchPrefix):]
 	}
 	return ""
 }

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})
+	// branch_filter: {master,feature*}
-	activateWebhook(t, 4)
+	w := unittest.AssertExistsAndLoadBean(t, &webhook_model.Webhook{ID: 4})
+	activateWebhook(t, w.ID)
 
-	hookTasks := []*webhook_model.HookTask{
+	for _, p := range []api.Payloader{
-		{HookID: 4, EventType: webhook_module.HookEventPush},
+		&api.PushPayload{Ref: "refs/heads/feature/7791"},
-	}
+		&api.CreatePayload{Ref: "refs/heads/feature/7791"}, // branch creation
-	for _, hookTask := range hookTasks {
+		&api.DeletePayload{Ref: "refs/heads/feature/7791"}, // branch deletion
-		unittest.AssertNotExistsBean(t, hookTask)
+	} {
-	}
+		t.Run(fmt.Sprintf("%T", p), func(t *testing.T) {
-	// this test also ensures that * doesn't handle / in any special way (like shell would)
+			db.DeleteBeans(db.DefaultContext, webhook_model.HookTask{HookID: w.ID})
-	assert.NoError(t, PrepareWebhooks(db.DefaultContext, EventSource{Repository: repo}, webhook_module.HookEventPush, &api.PushPayload{Ref: "refs/heads/feature/7791", Commits: []*api.PayloadCommit{{}}}))
+			typ := eventType(p)
-	for _, hookTask := range hookTasks {
+			assert.NoError(t, PrepareWebhook(db.DefaultContext, w, typ, p))
-		unittest.AssertExistsAndLoadBean(t, hookTask)
+			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})
+	// branch_filter: {master,feature*}
-	hookTasks := []*webhook_model.HookTask{
+	w := unittest.AssertExistsAndLoadBean(t, &webhook_model.Webhook{ID: 4})
-		{HookID: 4, EventType: webhook_module.HookEventPush},
+	activateWebhook(t, w.ID)
-	}
-	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"}))
 
-	for _, hookTask := range hookTasks {
+	for _, p := range []api.Payloader{
-		unittest.AssertNotExistsBean(t, hookTask)
+		&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})
+		})
 	}
 }