Store webhook event in database (#29145)

Refactor the webhook logic, to have the type-dependent processing happen
only in one place.

---

## Current webhook flow

1. An event happens
2. It is pre-processed (depending on the webhook type) and its body is
added to a task queue
3. When the task is processed, some more logic (depending on the webhook
type as well) is applied to make an HTTP request

This means that webhook-type dependant logic is needed in step 2 and 3.
This is cumbersome and brittle to maintain.

Updated webhook flow with this PR:
1. An event happens
2. It is stored as-is and added to a task queue
3. When the task is processed, the event is processed (depending on the
webhook type) to make an HTTP request

So the only webhook-type dependent logic happens in one place (step 3)
which should be much more robust.

## Consequences of the refactor

- the raw event must be stored in the hooktask (until now, the
pre-processed body was stored)
- to ensure that previous hooktasks are correctly sent, a
`payload_version` is added (version 1: the body has already been
pre-process / version 2: the body is the raw event)

So future webhook additions will only have to deal with creating an
http.Request based on the raw event (no need to adjust the code in
multiple places, like currently).

Moreover since this processing happens when fetching from the task
queue, it ensures that the queuing of new events (upon a `git push` for
instance) does not get slowed down by a slow webhook.

As a concrete example, the PR #19307 for custom webhooks, should be
substantially smaller:
- no need to change `services/webhook/deliver.go` 
- minimal change in `services/webhook/webhook.go` (add the new webhook
to the map)
- no need to change all the individual webhook files (since with this
refactor the `*webhook_model.Webhook` is provided as argument)

models/fixtures/hook_task.yml

@@ -3,3 +3,35 @@
   hook_id: 1
   uuid: uuid1
   is_delivered: true
+  is_succeed: false
+  request_content: >
+    {
+      "url": "/matrix-delivered",
+      "http_method":"PUT",
+      "headers": {
+        "X-Head": "42"
+      },
+      "body": "{}"
+    }
+
+-
+  id: 2
+  hook_id: 1
+  uuid: uuid2
+  is_delivered: false
+
+-
+  id: 3
+  hook_id: 1
+  uuid: uuid3
+  is_delivered: true
+  is_succeed: true
+  payload_content: '{"key":"value"}' # legacy task, payload saved in payload_content (and not in request_content)
+  request_content: >
+    {
+      "url": "/matrix-success",
+      "http_method":"PUT",
+      "headers": {
+        "X-Head": "42"
+      }
+    }

models/migrations/migrations.go

@@ -564,6 +564,8 @@ var migrations = []Migration{
 	NewMigration("Add user_blocking table", v1_22.AddUserBlockingTable),
 	// v289 -> v290
 	NewMigration("Add default_wiki_branch to repository table", v1_22.AddDefaultWikiBranch),
+	// v290 -> v291
+	NewMigration("Add PayloadVersion to HookTask", v1_22.AddPayloadVersionToHookTaskTable),
 }
 
 // GetCurrentDBVersion returns the current db version

models/migrations/v1_22/v290.go

@@ -0,0 +1,17 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package v1_22 //nolint
+
+import (
+	"xorm.io/xorm"
+)
+
+type HookTask struct {
+	PayloadVersion int `xorm:"DEFAULT 1"`
+}
+
+func AddPayloadVersionToHookTaskTable(x *xorm.Engine) error {
+	// create missing column
+	return x.Sync(new(HookTask))
+}

models/webhook/hooktask.go

@@ -5,13 +5,13 @@ package webhook
 
 import (
 	"context"
+	"errors"
 	"time"
 
 	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/modules/json"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
-	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/timeutil"
 	webhook_module "code.gitea.io/gitea/modules/webhook"
 
@@ -31,6 +31,7 @@ type HookRequest struct {
 	URL        string            `json:"url"`
 	HTTPMethod string            `json:"http_method"`
 	Headers    map[string]string `json:"headers"`
+	Body       string            `json:"body"`
 }
 
 // HookResponse represents hook task response information.
@@ -45,11 +46,15 @@ type HookTask struct {
 	ID             int64  `xorm:"pk autoincr"`
 	HookID         int64  `xorm:"index"`
 	UUID           string `xorm:"unique"`
-	api.Payloader  `xorm:"-"`
 	PayloadContent string `xorm:"LONGTEXT"`
-	EventType      webhook_module.HookEventType
-	IsDelivered    bool
-	Delivered      timeutil.TimeStampNano
+	// PayloadVersion number to allow for smooth version upgrades:
+	//  - PayloadVersion 1: PayloadContent contains the JSON as sent to the URL
+	//  - PayloadVersion 2: PayloadContent contains the original event
+	PayloadVersion int `xorm:"DEFAULT 1"`
+
+	EventType   webhook_module.HookEventType
+	IsDelivered bool
+	Delivered   timeutil.TimeStampNano
 
 	// History info.
 	IsSucceed       bool
@@ -115,16 +120,12 @@ func HookTasks(ctx context.Context, hookID int64, page int) ([]*HookTask, error)
 // it handles conversion from Payload to PayloadContent.
 func CreateHookTask(ctx context.Context, t *HookTask) (*HookTask, error) {
 	t.UUID = gouuid.New().String()
-	if t.Payloader != nil {
-		data, err := t.Payloader.JSONPayload()
-		if err != nil {
-			return nil, err
-		}
-		t.PayloadContent = string(data)
-	}
 	if t.Delivered == 0 {
 		t.Delivered = timeutil.TimeStampNanoNow()
 	}
+	if t.PayloadVersion == 0 {
+		return nil, errors.New("missing HookTask.PayloadVersion")
+	}
 	return t, db.Insert(ctx, t)
 }
 
@@ -165,6 +166,7 @@ func ReplayHookTask(ctx context.Context, hookID int64, uuid string) (*HookTask,
 		HookID:         task.HookID,
 		PayloadContent: task.PayloadContent,
 		EventType:      task.EventType,
+		PayloadVersion: task.PayloadVersion,
 	})
 }
 

models/webhook/webhook_test.go

@@ -12,7 +12,6 @@ import (
 	"code.gitea.io/gitea/models/unittest"
 	"code.gitea.io/gitea/modules/json"
 	"code.gitea.io/gitea/modules/optional"
-	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/timeutil"
 	webhook_module "code.gitea.io/gitea/modules/webhook"
 
@@ -35,8 +34,10 @@ func TestWebhook_History(t *testing.T) {
 	webhook := unittest.AssertExistsAndLoadBean(t, &Webhook{ID: 1})
 	tasks, err := webhook.History(db.DefaultContext, 0)
 	assert.NoError(t, err)
-	if assert.Len(t, tasks, 1) {
-		assert.Equal(t, int64(1), tasks[0].ID)
+	if assert.Len(t, tasks, 3) {
+		assert.Equal(t, int64(3), tasks[0].ID)
+		assert.Equal(t, int64(2), tasks[1].ID)
+		assert.Equal(t, int64(1), tasks[2].ID)
 	}
 
 	webhook = unittest.AssertExistsAndLoadBean(t, &Webhook{ID: 2})
@@ -197,8 +198,10 @@ func TestHookTasks(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	hookTasks, err := HookTasks(db.DefaultContext, 1, 1)
 	assert.NoError(t, err)
-	if assert.Len(t, hookTasks, 1) {
-		assert.Equal(t, int64(1), hookTasks[0].ID)
+	if assert.Len(t, hookTasks, 3) {
+		assert.Equal(t, int64(3), hookTasks[0].ID)
+		assert.Equal(t, int64(2), hookTasks[1].ID)
+		assert.Equal(t, int64(1), hookTasks[2].ID)
 	}
 
 	hookTasks, err = HookTasks(db.DefaultContext, unittest.NonexistentID, 1)
@@ -209,8 +212,8 @@ func TestHookTasks(t *testing.T) {
 func TestCreateHookTask(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	hookTask := &HookTask{
-		HookID:    3,
-		Payloader: &api.PushPayload{},
+		HookID:         3,
+		PayloadVersion: 2,
 	}
 	unittest.AssertNotExistsBean(t, hookTask)
 	_, err := CreateHookTask(db.DefaultContext, hookTask)
@@ -232,10 +235,10 @@ func TestUpdateHookTask(t *testing.T) {
 func TestCleanupHookTaskTable_PerWebhook_DeletesDelivered(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	hookTask := &HookTask{
-		HookID:      3,
-		Payloader:   &api.PushPayload{},
-		IsDelivered: true,
-		Delivered:   timeutil.TimeStampNanoNow(),
+		HookID:         3,
+		IsDelivered:    true,
+		Delivered:      timeutil.TimeStampNanoNow(),
+		PayloadVersion: 2,
 	}
 	unittest.AssertNotExistsBean(t, hookTask)
 	_, err := CreateHookTask(db.DefaultContext, hookTask)
@@ -249,9 +252,9 @@ func TestCleanupHookTaskTable_PerWebhook_DeletesDelivered(t *testing.T) {
 func TestCleanupHookTaskTable_PerWebhook_LeavesUndelivered(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	hookTask := &HookTask{
-		HookID:      4,
-		Payloader:   &api.PushPayload{},
-		IsDelivered: false,
+		HookID:         4,
+		IsDelivered:    false,
+		PayloadVersion: 2,
 	}
 	unittest.AssertNotExistsBean(t, hookTask)
 	_, err := CreateHookTask(db.DefaultContext, hookTask)
@@ -265,10 +268,10 @@ func TestCleanupHookTaskTable_PerWebhook_LeavesUndelivered(t *testing.T) {
 func TestCleanupHookTaskTable_PerWebhook_LeavesMostRecentTask(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	hookTask := &HookTask{
-		HookID:      4,
-		Payloader:   &api.PushPayload{},
-		IsDelivered: true,
-		Delivered:   timeutil.TimeStampNanoNow(),
+		HookID:         4,
+		IsDelivered:    true,
+		Delivered:      timeutil.TimeStampNanoNow(),
+		PayloadVersion: 2,
 	}
 	unittest.AssertNotExistsBean(t, hookTask)
 	_, err := CreateHookTask(db.DefaultContext, hookTask)
@@ -282,10 +285,10 @@ func TestCleanupHookTaskTable_PerWebhook_LeavesMostRecentTask(t *testing.T) {
 func TestCleanupHookTaskTable_OlderThan_DeletesDelivered(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	hookTask := &HookTask{
-		HookID:      3,
-		Payloader:   &api.PushPayload{},
-		IsDelivered: true,
-		Delivered:   timeutil.TimeStampNano(time.Now().AddDate(0, 0, -8).UnixNano()),
+		HookID:         3,
+		IsDelivered:    true,
+		Delivered:      timeutil.TimeStampNano(time.Now().AddDate(0, 0, -8).UnixNano()),
+		PayloadVersion: 2,
 	}
 	unittest.AssertNotExistsBean(t, hookTask)
 	_, err := CreateHookTask(db.DefaultContext, hookTask)
@@ -299,9 +302,9 @@ func TestCleanupHookTaskTable_OlderThan_DeletesDelivered(t *testing.T) {
 func TestCleanupHookTaskTable_OlderThan_LeavesUndelivered(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	hookTask := &HookTask{
-		HookID:      4,
-		Payloader:   &api.PushPayload{},
-		IsDelivered: false,
+		HookID:         4,
+		IsDelivered:    false,
+		PayloadVersion: 2,
 	}
 	unittest.AssertNotExistsBean(t, hookTask)
 	_, err := CreateHookTask(db.DefaultContext, hookTask)
@@ -315,10 +318,10 @@ func TestCleanupHookTaskTable_OlderThan_LeavesUndelivered(t *testing.T) {
 func TestCleanupHookTaskTable_OlderThan_LeavesTaskEarlierThanAgeToDelete(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	hookTask := &HookTask{
-		HookID:      4,
-		Payloader:   &api.PushPayload{},
-		IsDelivered: true,
-		Delivered:   timeutil.TimeStampNano(time.Now().AddDate(0, 0, -6).UnixNano()),
+		HookID:         4,
+		IsDelivered:    true,
+		Delivered:      timeutil.TimeStampNano(time.Now().AddDate(0, 0, -6).UnixNano()),
+		PayloadVersion: 2,
 	}
 	unittest.AssertNotExistsBean(t, hookTask)
 	_, err := CreateHookTask(db.DefaultContext, hookTask)

services/webhook/deliver.go

@@ -32,36 +32,17 @@ import (
 	"github.com/gobwas/glob"
 )
 
-// Deliver deliver hook task
-func Deliver(ctx context.Context, t *webhook_model.HookTask) error {
-	w, err := webhook_model.GetWebhookByID(ctx, t.HookID)
-	if err != nil {
-		return err
-	}
-
-	defer func() {
-		err := recover()
-		if err == nil {
-			return
-		}
-		// There was a panic whilst delivering a hook...
-		log.Error("PANIC whilst trying to deliver webhook task[%d] to webhook %s Panic: %v\nStacktrace: %s", t.ID, w.URL, err, log.Stack(2))
-	}()
-
-	t.IsDelivered = true
-
-	var req *http.Request
-
+func newDefaultRequest(ctx context.Context, w *webhook_model.Webhook, t *webhook_model.HookTask) (req *http.Request, body []byte, err error) {
 	switch w.HTTPMethod {
 	case "":
-		log.Info("HTTP Method for webhook %s empty, setting to POST as default", w.URL)
+		log.Info("HTTP Method for %s webhook %s [ID: %d] is not set, defaulting to POST", w.Type, w.URL, w.ID)
 		fallthrough
 	case http.MethodPost:
 		switch w.ContentType {
 		case webhook_model.ContentTypeJSON:
 			req, err = http.NewRequest("POST", w.URL, strings.NewReader(t.PayloadContent))
 			if err != nil {
-				return err
+				return nil, nil, err
 			}
 
 			req.Header.Set("Content-Type", "application/json")
@@ -72,50 +53,58 @@ func Deliver(ctx context.Context, t *webhook_model.HookTask) error {
 
 			req, err = http.NewRequest("POST", w.URL, strings.NewReader(forms.Encode()))
 			if err != nil {
-				return err
+				return nil, nil, err
 			}
 
 			req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+		default:
+			return nil, nil, fmt.Errorf("invalid content type: %v", w.ContentType)
 		}
 	case http.MethodGet:
 		u, err := url.Parse(w.URL)
 		if err != nil {
-			return fmt.Errorf("unable to deliver webhook task[%d] as cannot parse webhook url %s: %w", t.ID, w.URL, err)
+			return nil, nil, fmt.Errorf("invalid URL: %w", err)
 		}
 		vals := u.Query()
 		vals["payload"] = []string{t.PayloadContent}
 		u.RawQuery = vals.Encode()
 		req, err = http.NewRequest("GET", u.String(), nil)
 		if err != nil {
-			return fmt.Errorf("unable to deliver webhook task[%d] as unable to create HTTP request for webhook url %s: %w", t.ID, w.URL, err)
+			return nil, nil, err
 		}
 	case http.MethodPut:
 		switch w.Type {
-		case webhook_module.MATRIX:
+		case webhook_module.MATRIX: // used when t.Version == 1
 			txnID, err := getMatrixTxnID([]byte(t.PayloadContent))
 			if err != nil {
-				return err
+				return nil, nil, err
 			}
 			url := fmt.Sprintf("%s/%s", w.URL, url.PathEscape(txnID))
 			req, err = http.NewRequest("PUT", url, strings.NewReader(t.PayloadContent))
 			if err != nil {
-				return fmt.Errorf("unable to deliver webhook task[%d] as cannot create matrix request for webhook url %s: %w", t.ID, w.URL, err)
+				return nil, nil, err
 			}
 		default:
-			return fmt.Errorf("invalid http method for webhook task[%d] in webhook %s: %v", t.ID, w.URL, w.HTTPMethod)
+			return nil, nil, fmt.Errorf("invalid http method: %v", w.HTTPMethod)
 		}
 	default:
-		return fmt.Errorf("invalid http method for webhook task[%d] in webhook %s: %v", t.ID, w.URL, w.HTTPMethod)
+		return nil, nil, fmt.Errorf("invalid http method: %v", w.HTTPMethod)
 	}
 
+	body = []byte(t.PayloadContent)
+	return req, body, addDefaultHeaders(req, []byte(w.Secret), t, body)
+}
+
+func addDefaultHeaders(req *http.Request, secret []byte, t *webhook_model.HookTask, payloadContent []byte) error {
 	var signatureSHA1 string
 	var signatureSHA256 string
-	if len(w.Secret) > 0 {
-		sig1 := hmac.New(sha1.New, []byte(w.Secret))
-		sig256 := hmac.New(sha256.New, []byte(w.Secret))
-		_, err = io.MultiWriter(sig1, sig256).Write([]byte(t.PayloadContent))
+	if len(secret) > 0 {
+		sig1 := hmac.New(sha1.New, secret)
+		sig256 := hmac.New(sha256.New, secret)
+		_, err := io.MultiWriter(sig1, sig256).Write(payloadContent)
 		if err != nil {
-			log.Error("prepareWebhooks.sigWrite: %v", err)
+			// this error should never happen, since the hashes are writing to []byte and always return a nil error.
+			return fmt.Errorf("prepareWebhooks.sigWrite: %w", err)
 		}
 		signatureSHA1 = hex.EncodeToString(sig1.Sum(nil))
 		signatureSHA256 = hex.EncodeToString(sig256.Sum(nil))
@@ -136,15 +125,36 @@ func Deliver(ctx context.Context, t *webhook_model.HookTask) error {
 	req.Header["X-GitHub-Delivery"] = []string{t.UUID}
 	req.Header["X-GitHub-Event"] = []string{event}
 	req.Header["X-GitHub-Event-Type"] = []string{eventType}
+	return nil
+}
 
-	// Add Authorization Header
-	authorization, err := w.HeaderAuthorization()
+// Deliver creates the [http.Request] (depending on the webhook type), sends it
+// and records the status and response.
+func Deliver(ctx context.Context, t *webhook_model.HookTask) error {
+	w, err := webhook_model.GetWebhookByID(ctx, t.HookID)
 	if err != nil {
-		log.Error("Webhook could not get Authorization header [%d]: %v", w.ID, err)
 		return err
 	}
-	if authorization != "" {
-		req.Header["Authorization"] = []string{authorization}
+
+	defer func() {
+		err := recover()
+		if err == nil {
+			return
+		}
+		// There was a panic whilst delivering a hook...
+		log.Error("PANIC whilst trying to deliver webhook task[%d] to webhook %s Panic: %v\nStacktrace: %s", t.ID, w.URL, err, log.Stack(2))
+	}()
+
+	t.IsDelivered = true
+
+	newRequest := webhookRequesters[w.Type]
+	if t.PayloadVersion == 1 || newRequest == nil {
+		newRequest = newDefaultRequest
+	}
+
+	req, body, err := newRequest(ctx, w, t)
+	if err != nil {
+		return fmt.Errorf("cannot create http request for webhook %s[%d %s]: %w", w.Type, w.ID, w.URL, err)
 	}
 
 	// Record delivery information.
@@ -152,11 +162,22 @@ func Deliver(ctx context.Context, t *webhook_model.HookTask) error {
 		URL:        req.URL.String(),
 		HTTPMethod: req.Method,
 		Headers:    map[string]string{},
+		Body:       string(body),
 	}
 	for k, vals := range req.Header {
 		t.RequestInfo.Headers[k] = strings.Join(vals, ",")
 	}
 
+	// Add Authorization Header
+	authorization, err := w.HeaderAuthorization()
+	if err != nil {
+		return fmt.Errorf("cannot get Authorization header for webhook %s[%d %s]: %w", w.Type, w.ID, w.URL, err)
+	}
+	if authorization != "" {
+		req.Header.Set("Authorization", authorization)
+		t.RequestInfo.Headers["Authorization"] = "******"
+	}
+
 	t.ResponseInfo = &webhook_model.HookResponse{
 		Headers: map[string]string{},
 	}

services/webhook/deliver_test.go

@@ -5,9 +5,11 @@ package webhook
 
 import (
 	"context"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"strings"
 	"testing"
 	"time"
 
@@ -16,7 +18,6 @@ import (
 	webhook_model "code.gitea.io/gitea/models/webhook"
 	"code.gitea.io/gitea/modules/hostmatcher"
 	"code.gitea.io/gitea/modules/setting"
-	api "code.gitea.io/gitea/modules/structs"
 	webhook_module "code.gitea.io/gitea/modules/webhook"
 
 	"github.com/stretchr/testify/assert"
@@ -107,13 +108,15 @@ func TestWebhookDeliverAuthorizationHeader(t *testing.T) {
 	assert.NoError(t, webhook_model.CreateWebhook(db.DefaultContext, hook))
 	db.GetEngine(db.DefaultContext).NoAutoTime().DB().Logger.ShowSQL(true)
 
-	hookTask := &webhook_model.HookTask{HookID: hook.ID, EventType: webhook_module.HookEventPush, Payloader: &api.PushPayload{}}
+	hookTask := &webhook_model.HookTask{
+		HookID:         hook.ID,
+		EventType:      webhook_module.HookEventPush,
+		PayloadVersion: 2,
+	}
 
 	hookTask, err = webhook_model.CreateHookTask(db.DefaultContext, hookTask)
 	assert.NoError(t, err)
-	if !assert.NotNil(t, hookTask) {
-		return
-	}
+	assert.NotNil(t, hookTask)
 
 	assert.NoError(t, Deliver(context.Background(), hookTask))
 	select {
@@ -123,4 +126,193 @@ func TestWebhookDeliverAuthorizationHeader(t *testing.T) {
 	}
 
 	assert.True(t, hookTask.IsSucceed)
+	assert.Equal(t, "******", hookTask.RequestInfo.Headers["Authorization"])
+}
+
+func TestWebhookDeliverHookTask(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	done := make(chan struct{}, 1)
+	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		assert.Equal(t, "PUT", r.Method)
+		switch r.URL.Path {
+		case "/webhook/66d222a5d6349e1311f551e50722d837e30fce98":
+			// Version 1
+			assert.Equal(t, "push", r.Header.Get("X-GitHub-Event"))
+			assert.Equal(t, "", r.Header.Get("Content-Type"))
+			body, err := io.ReadAll(r.Body)
+			assert.NoError(t, err)
+			assert.Equal(t, `{"data": 42}`, string(body))
+
+		case "/webhook/6db5dc1e282529a8c162c7fe93dd2667494eeb51":
+			// Version 2
+			assert.Equal(t, "push", r.Header.Get("X-GitHub-Event"))
+			assert.Equal(t, "application/json", r.Header.Get("Content-Type"))
+			body, err := io.ReadAll(r.Body)
+			assert.NoError(t, err)
+			assert.Len(t, body, 2147)
+
+		default:
+			w.WriteHeader(404)
+			t.Fatalf("unexpected url path %s", r.URL.Path)
+			return
+		}
+		w.WriteHeader(200)
+		done <- struct{}{}
+	}))
+	t.Cleanup(s.Close)
+
+	hook := &webhook_model.Webhook{
+		RepoID:      3,
+		IsActive:    true,
+		Type:        webhook_module.MATRIX,
+		URL:         s.URL + "/webhook",
+		HTTPMethod:  "PUT",
+		ContentType: webhook_model.ContentTypeJSON,
+		Meta:        `{"message_type":0}`, // text
+	}
+	assert.NoError(t, webhook_model.CreateWebhook(db.DefaultContext, hook))
+
+	t.Run("Version 1", func(t *testing.T) {
+		hookTask := &webhook_model.HookTask{
+			HookID:         hook.ID,
+			EventType:      webhook_module.HookEventPush,
+			PayloadContent: `{"data": 42}`,
+			PayloadVersion: 1,
+		}
+
+		hookTask, err := webhook_model.CreateHookTask(db.DefaultContext, hookTask)
+		assert.NoError(t, err)
+		assert.NotNil(t, hookTask)
+
+		assert.NoError(t, Deliver(context.Background(), hookTask))
+		select {
+		case <-done:
+		case <-time.After(5 * time.Second):
+			t.Fatal("waited to long for request to happen")
+		}
+
+		assert.True(t, hookTask.IsSucceed)
+	})
+
+	t.Run("Version 2", func(t *testing.T) {
+		p := pushTestPayload()
+		data, err := p.JSONPayload()
+		assert.NoError(t, err)
+
+		hookTask := &webhook_model.HookTask{
+			HookID:         hook.ID,
+			EventType:      webhook_module.HookEventPush,
+			PayloadContent: string(data),
+			PayloadVersion: 2,
+		}
+
+		hookTask, err = webhook_model.CreateHookTask(db.DefaultContext, hookTask)
+		assert.NoError(t, err)
+		assert.NotNil(t, hookTask)
+
+		assert.NoError(t, Deliver(context.Background(), hookTask))
+		select {
+		case <-done:
+		case <-time.After(5 * time.Second):
+			t.Fatal("waited to long for request to happen")
+		}
+
+		assert.True(t, hookTask.IsSucceed)
+	})
+}
+
+func TestWebhookDeliverSpecificTypes(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	type hookCase struct {
+		gotBody chan []byte
+	}
+
+	cases := map[string]hookCase{
+		webhook_module.SLACK: {
+			gotBody: make(chan []byte, 1),
+		},
+		webhook_module.DISCORD: {
+			gotBody: make(chan []byte, 1),
+		},
+		webhook_module.DINGTALK: {
+			gotBody: make(chan []byte, 1),
+		},
+		webhook_module.TELEGRAM: {
+			gotBody: make(chan []byte, 1),
+		},
+		webhook_module.MSTEAMS: {
+			gotBody: make(chan []byte, 1),
+		},
+		webhook_module.FEISHU: {
+			gotBody: make(chan []byte, 1),
+		},
+		webhook_module.MATRIX: {
+			gotBody: make(chan []byte, 1),
+		},
+		webhook_module.WECHATWORK: {
+			gotBody: make(chan []byte, 1),
+		},
+		webhook_module.PACKAGIST: {
+			gotBody: make(chan []byte, 1),
+		},
+	}
+
+	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		assert.Equal(t, "application/json", r.Header.Get("Content-Type"), r.URL.Path)
+
+		typ := strings.Split(r.URL.Path, "/")[1] // take first segment (after skipping leading slash)
+		hc := cases[typ]
+		require.NotNil(t, hc.gotBody, r.URL.Path)
+		body, err := io.ReadAll(r.Body)
+		assert.NoError(t, err)
+		w.WriteHeader(200)
+		hc.gotBody <- body
+	}))
+	t.Cleanup(s.Close)
+
+	p := pushTestPayload()
+	data, err := p.JSONPayload()
+	assert.NoError(t, err)
+
+	for typ, hc := range cases {
+		typ := typ
+		hc := hc
+		t.Run(typ, func(t *testing.T) {
+			t.Parallel()
+			hook := &webhook_model.Webhook{
+				RepoID:      3,
+				IsActive:    true,
+				Type:        typ,
+				URL:         s.URL + "/" + typ,
+				HTTPMethod:  "POST",
+				ContentType: 0, // set to 0 so that falling back to default request fails with "invalid content type"
+				Meta:        "{}",
+			}
+			assert.NoError(t, webhook_model.CreateWebhook(db.DefaultContext, hook))
+
+			hookTask := &webhook_model.HookTask{
+				HookID:         hook.ID,
+				EventType:      webhook_module.HookEventPush,
+				PayloadContent: string(data),
+				PayloadVersion: 2,
+			}
+
+			hookTask, err := webhook_model.CreateHookTask(db.DefaultContext, hookTask)
+			assert.NoError(t, err)
+			assert.NotNil(t, hookTask)
+
+			assert.NoError(t, Deliver(context.Background(), hookTask))
+			select {
+			case gotBody := <-hc.gotBody:
+				assert.NotEqual(t, string(data), string(gotBody), "request body must be different from the event payload")
+				assert.Equal(t, hookTask.RequestInfo.Body, string(gotBody), "request body was not saved")
+			case <-time.After(5 * time.Second):
+				t.Fatal("waited to long for request to happen")
+			}
+
+			assert.True(t, hookTask.IsSucceed)
+		})
+	}
 }

services/webhook/dingtalk.go

@@ -4,12 +4,14 @@
 package webhook
 
 import (
+	"context"
 	"fmt"
+	"net/http"
 	"net/url"
 	"strings"
 
+	webhook_model "code.gitea.io/gitea/models/webhook"
 	"code.gitea.io/gitea/modules/git"
-	"code.gitea.io/gitea/modules/json"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/util"
 	webhook_module "code.gitea.io/gitea/modules/webhook"
@@ -22,19 +24,8 @@ type (
 	DingtalkPayload dingtalk.Payload
 )
 
-var _ PayloadConvertor = &DingtalkPayload{}
-
-// JSONPayload Marshals the DingtalkPayload to json
-func (d *DingtalkPayload) JSONPayload() ([]byte, error) {
-	data, err := json.MarshalIndent(d, "", "  ")
-	if err != nil {
-		return []byte{}, err
-	}
-	return data, nil
-}
-
 // Create implements PayloadConvertor Create method
-func (d *DingtalkPayload) Create(p *api.CreatePayload) (api.Payloader, error) {
+func (dc dingtalkConvertor) Create(p *api.CreatePayload) (DingtalkPayload, error) {
 	// created tag/branch
 	refName := git.RefName(p.Ref).ShortName()
 	title := fmt.Sprintf("[%s] %s %s created", p.Repo.FullName, p.RefType, refName)
@@ -43,7 +34,7 @@ func (d *DingtalkPayload) Create(p *api.CreatePayload) (api.Payloader, error) {
 }
 
 // Delete implements PayloadConvertor Delete method
-func (d *DingtalkPayload) Delete(p *api.DeletePayload) (api.Payloader, error) {
+func (dc dingtalkConvertor) Delete(p *api.DeletePayload) (DingtalkPayload, error) {
 	// created tag/branch
 	refName := git.RefName(p.Ref).ShortName()
 	title := fmt.Sprintf("[%s] %s %s deleted", p.Repo.FullName, p.RefType, refName)
@@ -52,14 +43,14 @@ func (d *DingtalkPayload) Delete(p *api.DeletePayload) (api.Payloader, error) {
 }
 
 // Fork implements PayloadConvertor Fork method
-func (d *DingtalkPayload) Fork(p *api.ForkPayload) (api.Payloader, error) {
+func (dc dingtalkConvertor) Fork(p *api.ForkPayload) (DingtalkPayload, error) {
 	title := fmt.Sprintf("%s is forked to %s", p.Forkee.FullName, p.Repo.FullName)
 
 	return createDingtalkPayload(title, title, fmt.Sprintf("view forked repo %s", p.Repo.FullName), p.Repo.HTMLURL), nil
 }
 
 // Push implements PayloadConvertor Push method
-func (d *DingtalkPayload) Push(p *api.PushPayload) (api.Payloader, error) {
+func (dc dingtalkConvertor) Push(p *api.PushPayload) (DingtalkPayload, error) {
 	var (
 		branchName = git.RefName(p.Ref).ShortName()
 		commitDesc string
@@ -100,14 +91,14 @@ func (d *DingtalkPayload) Push(p *api.PushPayload) (api.Payloader, error) {
 }
 
 // Issue implements PayloadConvertor Issue method
-func (d *DingtalkPayload) Issue(p *api.IssuePayload) (api.Payloader, error) {
+func (dc dingtalkConvertor) Issue(p *api.IssuePayload) (DingtalkPayload, error) {
 	text, issueTitle, attachmentText, _ := getIssuesPayloadInfo(p, noneLinkFormatter, true)
 
 	return createDingtalkPayload(issueTitle, text+"\r\n\r\n"+attachmentText, "view issue", p.Issue.HTMLURL), nil
 }
 
 // Wiki implements PayloadConvertor Wiki method
-func (d *DingtalkPayload) Wiki(p *api.WikiPayload) (api.Payloader, error) {
+func (dc dingtalkConvertor) Wiki(p *api.WikiPayload) (DingtalkPayload, error) {
 	text, _, _ := getWikiPayloadInfo(p, noneLinkFormatter, true)
 	url := p.Repository.HTMLURL + "/wiki/" + url.PathEscape(p.Page)
 
@@ -115,27 +106,27 @@ func (d *DingtalkPayload) Wiki(p *api.WikiPayload) (api.Payloader, error) {
 }
 
 // IssueComment implements PayloadConvertor IssueComment method
-func (d *DingtalkPayload) IssueComment(p *api.IssueCommentPayload) (api.Payloader, error) {
+func (dc dingtalkConvertor) IssueComment(p *api.IssueCommentPayload) (DingtalkPayload, error) {
 	text, issueTitle, _ := getIssueCommentPayloadInfo(p, noneLinkFormatter, true)
 
 	return createDingtalkPayload(issueTitle, text+"\r\n\r\n"+p.Comment.Body, "view issue comment", p.Comment.HTMLURL), nil
 }
 
 // PullRequest implements PayloadConvertor PullRequest method
-func (d *DingtalkPayload) PullRequest(p *api.PullRequestPayload) (api.Payloader, error) {
+func (dc dingtalkConvertor) PullRequest(p *api.PullRequestPayload) (DingtalkPayload, error) {
 	text, issueTitle, attachmentText, _ := getPullRequestPayloadInfo(p, noneLinkFormatter, true)
 
 	return createDingtalkPayload(issueTitle, text+"\r\n\r\n"+attachmentText, "view pull request", p.PullRequest.HTMLURL), nil
 }
 
 // Review implements PayloadConvertor Review method
-func (d *DingtalkPayload) Review(p *api.PullRequestPayload, event webhook_module.HookEventType) (api.Payloader, error) {
+func (dc dingtalkConvertor) Review(p *api.PullRequestPayload, event webhook_module.HookEventType) (DingtalkPayload, error) {
 	var text, title string
 	switch p.Action {
 	case api.HookIssueReviewed:
 		action, err := parseHookPullRequestEventType(event)
 		if err != nil {
-			return nil, err
+			return DingtalkPayload{}, err
 		}
 
 		title = fmt.Sprintf("[%s] Pull request review %s : #%d %s", p.Repository.FullName, action, p.Index, p.PullRequest.Title)
@@ -146,14 +137,14 @@ func (d *DingtalkPayload) Review(p *api.PullRequestPayload, event webhook_module
 }
 
 // Repository implements PayloadConvertor Repository method
-func (d *DingtalkPayload) Repository(p *api.RepositoryPayload) (api.Payloader, error) {
+func (dc dingtalkConvertor) Repository(p *api.RepositoryPayload) (DingtalkPayload, error) {
 	switch p.Action {
 	case api.HookRepoCreated:
 		title := fmt.Sprintf("[%s] Repository created", p.Repository.FullName)
 		return createDingtalkPayload(title, title, "view repository", p.Repository.HTMLURL), nil
 	case api.HookRepoDeleted:
 		title := fmt.Sprintf("[%s] Repository deleted", p.Repository.FullName)
-		return &DingtalkPayload{
+		return DingtalkPayload{
 			MsgType: "text",
 			Text: struct {
 				Content string `json:"content"`
@@ -163,24 +154,24 @@ func (d *DingtalkPayload) Repository(p *api.RepositoryPayload) (api.Payloader, e
 		}, nil
 	}
 
-	return nil, nil
+	return DingtalkPayload{}, nil
 }
 
 // Release implements PayloadConvertor Release method
-func (d *DingtalkPayload) Release(p *api.ReleasePayload) (api.Payloader, error) {
+func (dc dingtalkConvertor) Release(p *api.ReleasePayload) (DingtalkPayload, error) {
 	text, _ := getReleasePayloadInfo(p, noneLinkFormatter, true)
 
 	return createDingtalkPayload(text, text, "view release", p.Release.HTMLURL), nil
 }
 
-func (d *DingtalkPayload) Package(p *api.PackagePayload) (api.Payloader, error) {
+func (dc dingtalkConvertor) Package(p *api.PackagePayload) (DingtalkPayload, error) {
 	text, _ := getPackagePayloadInfo(p, noneLinkFormatter, true)
 
 	return createDingtalkPayload(text, text, "view package", p.Package.HTMLURL), nil
 }
 
-func createDingtalkPayload(title, text, singleTitle, singleURL string) *DingtalkPayload {
-	return &DingtalkPayload{
+func createDingtalkPayload(title, text, singleTitle, singleURL string) DingtalkPayload {
+	return DingtalkPayload{
 		MsgType: "actionCard",
 		ActionCard: dingtalk.ActionCard{
 			Text:        strings.TrimSpace(text),
@@ -195,7 +186,10 @@ func createDingtalkPayload(title, text, singleTitle, singleURL string) *Dingtalk
 	}
 }
 
-// GetDingtalkPayload converts a ding talk webhook into a DingtalkPayload
-func GetDingtalkPayload(p api.Payloader, event webhook_module.HookEventType, _ string) (api.Payloader, error) {
-	return convertPayloader(new(DingtalkPayload), p, event)
+type dingtalkConvertor struct{}
+
+var _ payloadConvertor[DingtalkPayload] = dingtalkConvertor{}
+
+func newDingtalkRequest(ctx context.Context, w *webhook_model.Webhook, t *webhook_model.HookTask) (*http.Request, []byte, error) {
+	return newJSONRequest(dingtalkConvertor{}, w, t, true)
 }

services/webhook/discord.go

@@ -4,8 +4,10 @@
 package webhook
 
 import (
+	"context"
 	"errors"
 	"fmt"
+	"net/http"
 	"net/url"
 	"strconv"
 	"strings"
@@ -98,19 +100,8 @@ var (
 	redColor         = color("ff3232")
 )
 
-// JSONPayload Marshals the DiscordPayload to json
-func (d *DiscordPayload) JSONPayload() ([]byte, error) {
-	data, err := json.MarshalIndent(d, "", "  ")
-	if err != nil {
-		return []byte{}, err
-	}
-	return data, nil
-}
-
-var _ PayloadConvertor = &DiscordPayload{}
-
 // Create implements PayloadConvertor Create method
-func (d *DiscordPayload) Create(p *api.CreatePayload) (api.Payloader, error) {
+func (d discordConvertor) Create(p *api.CreatePayload) (DiscordPayload, error) {
 	// created tag/branch
 	refName := git.RefName(p.Ref).ShortName()
 	title := fmt.Sprintf("[%s] %s %s created", p.Repo.FullName, p.RefType, refName)
@@ -119,7 +110,7 @@ func (d *DiscordPayload) Create(p *api.CreatePayload) (api.Payloader, error) {
 }
 
 // Delete implements PayloadConvertor Delete method
-func (d *DiscordPayload) Delete(p *api.DeletePayload) (api.Payloader, error) {
+func (d discordConvertor) Delete(p *api.DeletePayload) (DiscordPayload, error) {
 	// deleted tag/branch
 	refName := git.RefName(p.Ref).ShortName()
 	title := fmt.Sprintf("[%s] %s %s deleted", p.Repo.FullName, p.RefType, refName)
@@ -128,14 +119,14 @@ func (d *DiscordPayload) Delete(p *api.DeletePayload) (api.Payloader, error) {
 }
 
 // Fork implements PayloadConvertor Fork method
-func (d *DiscordPayload) Fork(p *api.ForkPayload) (api.Payloader, error) {
+func (d discordConvertor) Fork(p *api.ForkPayload) (DiscordPayload, error) {
 	title := fmt.Sprintf("%s is forked to %s", p.Forkee.FullName, p.Repo.FullName)
 
 	return d.createPayload(p.Sender, title, "", p.Repo.HTMLURL, greenColor), nil
 }
 
 // Push implements PayloadConvertor Push method
-func (d *DiscordPayload) Push(p *api.PushPayload) (api.Payloader, error) {
+func (d discordConvertor) Push(p *api.PushPayload) (DiscordPayload, error) {
 	var (
 		branchName = git.RefName(p.Ref).ShortName()
 		commitDesc string
@@ -170,35 +161,35 @@ func (d *DiscordPayload) Push(p *api.PushPayload) (api.Payloader, error) {
 }
 
 // Issue implements PayloadConvertor Issue method
-func (d *DiscordPayload) Issue(p *api.IssuePayload) (api.Payloader, error) {
+func (d discordConvertor) Issue(p *api.IssuePayload) (DiscordPayload, error) {
 	title, _, text, color := getIssuesPayloadInfo(p, noneLinkFormatter, false)
 
 	return d.createPayload(p.Sender, title, text, p.Issue.HTMLURL, color), nil
 }
 
 // IssueComment implements PayloadConvertor IssueComment method
-func (d *DiscordPayload) IssueComment(p *api.IssueCommentPayload) (api.Payloader, error) {
+func (d discordConvertor) IssueComment(p *api.IssueCommentPayload) (DiscordPayload, error) {
 	title, _, color := getIssueCommentPayloadInfo(p, noneLinkFormatter, false)
 
 	return d.createPayload(p.Sender, title, p.Comment.Body, p.Comment.HTMLURL, color), nil
 }
 
 // PullRequest implements PayloadConvertor PullRequest method
-func (d *DiscordPayload) PullRequest(p *api.PullRequestPayload) (api.Payloader, error) {
+func (d discordConvertor) PullRequest(p *api.PullRequestPayload) (DiscordPayload, error) {
 	title, _, text, color := getPullRequestPayloadInfo(p, noneLinkFormatter, false)
 
 	return d.createPayload(p.Sender, title, text, p.PullRequest.HTMLURL, color), nil
 }
 
 // Review implements PayloadConvertor Review method
-func (d *DiscordPayload) Review(p *api.PullRequestPayload, event webhook_module.HookEventType) (api.Payloader, error) {
+func (d discordConvertor) Review(p *api.PullRequestPayload, event webhook_module.HookEventType) (DiscordPayload, error) {
 	var text, title string
 	var color int
 	switch p.Action {
 	case api.HookIssueReviewed:
 		action, err := parseHookPullRequestEventType(event)
 		if err != nil {
-			return nil, err
+			return DiscordPayload{}, err
 		}
 
 		title = fmt.Sprintf("[%s] Pull request review %s: #%d %s", p.Repository.FullName, action, p.Index, p.PullRequest.Title)
@@ -220,7 +211,7 @@ func (d *DiscordPayload) Review(p *api.PullRequestPayload, event webhook_module.
 }
 
 // Repository implements PayloadConvertor Repository method
-func (d *DiscordPayload) Repository(p *api.RepositoryPayload) (api.Payloader, error) {
+func (d discordConvertor) Repository(p *api.RepositoryPayload) (DiscordPayload, error) {
 	var title, url string
 	var color int
 	switch p.Action {
@@ -237,7 +228,7 @@ func (d *DiscordPayload) Repository(p *api.RepositoryPayload) (api.Payloader, er
 }
 
 // Wiki implements PayloadConvertor Wiki method
-func (d *DiscordPayload) Wiki(p *api.WikiPayload) (api.Payloader, error) {
+func (d discordConvertor) Wiki(p *api.WikiPayload) (DiscordPayload, error) {
 	text, color, _ := getWikiPayloadInfo(p, noneLinkFormatter, false)
 	htmlLink := p.Repository.HTMLURL + "/wiki/" + url.PathEscape(p.Page)
 
@@ -250,30 +241,35 @@ func (d *DiscordPayload) Wiki(p *api.WikiPayload) (api.Payloader, error) {
 }
 
 // Release implements PayloadConvertor Release method
-func (d *DiscordPayload) Release(p *api.ReleasePayload) (api.Payloader, error) {
+func (d discordConvertor) Release(p *api.ReleasePayload) (DiscordPayload, error) {
 	text, color := getReleasePayloadInfo(p, noneLinkFormatter, false)
 
 	return d.createPayload(p.Sender, text, p.Release.Note, p.Release.HTMLURL, color), nil
 }
 
-func (d *DiscordPayload) Package(p *api.PackagePayload) (api.Payloader, error) {
+func (d discordConvertor) Package(p *api.PackagePayload) (DiscordPayload, error) {
 	text, color := getPackagePayloadInfo(p, noneLinkFormatter, false)
 
 	return d.createPayload(p.Sender, text, "", p.Package.HTMLURL, color), nil
 }
 
-// GetDiscordPayload converts a discord webhook into a DiscordPayload
-func GetDiscordPayload(p api.Payloader, event webhook_module.HookEventType, meta string) (api.Payloader, error) {
-	s := new(DiscordPayload)
+type discordConvertor struct {
+	Username  string
+	AvatarURL string
+}
 
-	discord := &DiscordMeta{}
-	if err := json.Unmarshal([]byte(meta), &discord); err != nil {
-		return s, errors.New("GetDiscordPayload meta json:" + err.Error())
+var _ payloadConvertor[DiscordPayload] = discordConvertor{}
+
+func newDiscordRequest(ctx context.Context, w *webhook_model.Webhook, t *webhook_model.HookTask) (*http.Request, []byte, error) {
+	meta := &DiscordMeta{}
+	if err := json.Unmarshal([]byte(w.Meta), meta); err != nil {
+		return nil, nil, fmt.Errorf("newDiscordRequest meta json: %w", err)
 	}
-	s.Username = discord.Username
-	s.AvatarURL = discord.IconURL
-
-	return convertPayloader(s, p, event)
+	sc := discordConvertor{
+		Username:  meta.Username,
+		AvatarURL: meta.IconURL,
+	}
+	return newJSONRequest(sc, w, t, true)
 }
 
 func parseHookPullRequestEventType(event webhook_module.HookEventType) (string, error) {
@@ -291,8 +287,8 @@ func parseHookPullRequestEventType(event webhook_module.HookEventType) (string,
 	}
 }
 
-func (d *DiscordPayload) createPayload(s *api.User, title, text, url string, color int) *DiscordPayload {
-	return &DiscordPayload{
+func (d discordConvertor) createPayload(s *api.User, title, text, url string, color int) DiscordPayload {
+	return DiscordPayload{
 		Username:  d.Username,
 		AvatarURL: d.AvatarURL,
 		Embeds: []DiscordEmbed{

services/webhook/feishu.go

@@ -4,11 +4,13 @@
 package webhook
 
 import (
+	"context"
 	"fmt"
+	"net/http"
 	"strings"
 
+	webhook_model "code.gitea.io/gitea/models/webhook"
 	"code.gitea.io/gitea/modules/git"
-	"code.gitea.io/gitea/modules/json"
 	api "code.gitea.io/gitea/modules/structs"
 	webhook_module "code.gitea.io/gitea/modules/webhook"
 )
@@ -23,8 +25,8 @@ type (
 	}
 )
 
-func newFeishuTextPayload(text string) *FeishuPayload {
-	return &FeishuPayload{
+func newFeishuTextPayload(text string) FeishuPayload {
+	return FeishuPayload{
 		MsgType: "text",
 		Content: struct {
 			Text string `json:"text"`
@@ -34,19 +36,8 @@ func newFeishuTextPayload(text string) *FeishuPayload {
 	}
 }
 
-// JSONPayload Marshals the FeishuPayload to json
-func (f *FeishuPayload) JSONPayload() ([]byte, error) {
-	data, err := json.MarshalIndent(f, "", "  ")
-	if err != nil {
-		return []byte{}, err
-	}
-	return data, nil
-}
-
-var _ PayloadConvertor = &FeishuPayload{}
-
 // Create implements PayloadConvertor Create method
-func (f *FeishuPayload) Create(p *api.CreatePayload) (api.Payloader, error) {
+func (fc feishuConvertor) Create(p *api.CreatePayload) (FeishuPayload, error) {
 	// created tag/branch
 	refName := git.RefName(p.Ref).ShortName()
 	text := fmt.Sprintf("[%s] %s %s created", p.Repo.FullName, p.RefType, refName)
@@ -55,7 +46,7 @@ func (f *FeishuPayload) Create(p *api.CreatePayload) (api.Payloader, error) {
 }
 
 // Delete implements PayloadConvertor Delete method
-func (f *FeishuPayload) Delete(p *api.DeletePayload) (api.Payloader, error) {
+func (fc feishuConvertor) Delete(p *api.DeletePayload) (FeishuPayload, error) {
 	// created tag/branch
 	refName := git.RefName(p.Ref).ShortName()
 	text := fmt.Sprintf("[%s] %s %s deleted", p.Repo.FullName, p.RefType, refName)
@@ -64,14 +55,14 @@ func (f *FeishuPayload) Delete(p *api.DeletePayload) (api.Payloader, error) {
 }
 
 // Fork implements PayloadConvertor Fork method
-func (f *FeishuPayload) Fork(p *api.ForkPayload) (api.Payloader, error) {
+func (fc feishuConvertor) Fork(p *api.ForkPayload) (FeishuPayload, error) {
 	text := fmt.Sprintf("%s is forked to %s", p.Forkee.FullName, p.Repo.FullName)
 
 	return newFeishuTextPayload(text), nil
 }
 
 // Push implements PayloadConvertor Push method
-func (f *FeishuPayload) Push(p *api.PushPayload) (api.Payloader, error) {
+func (fc feishuConvertor) Push(p *api.PushPayload) (FeishuPayload, error) {
 	var (
 		branchName = git.RefName(p.Ref).ShortName()
 		commitDesc string
@@ -96,48 +87,40 @@ func (f *FeishuPayload) Push(p *api.PushPayload) (api.Payloader, error) {
 }
 
 // Issue implements PayloadConvertor Issue method
-func (f *FeishuPayload) Issue(p *api.IssuePayload) (api.Payloader, error) {
+func (fc feishuConvertor) Issue(p *api.IssuePayload) (FeishuPayload, error) {
 	title, link, by, operator, result, assignees := getIssuesInfo(p)
-	var res api.Payloader
 	if assignees != "" {
 		if p.Action == api.HookIssueAssigned || p.Action == api.HookIssueUnassigned || p.Action == api.HookIssueMilestoned {
-			res = newFeishuTextPayload(fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n%s\n\n%s", title, link, by, operator, result, assignees, p.Issue.Body))
-		} else {
-			res = newFeishuTextPayload(fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n\n%s", title, link, by, operator, assignees, p.Issue.Body))
+			return newFeishuTextPayload(fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n%s\n\n%s", title, link, by, operator, result, assignees, p.Issue.Body)), nil
 		}
-	} else {
-		res = newFeishuTextPayload(fmt.Sprintf("%s\n%s\n%s\n%s\n\n%s", title, link, by, operator, p.Issue.Body))
+		return newFeishuTextPayload(fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n\n%s", title, link, by, operator, assignees, p.Issue.Body)), nil
 	}
-	return res, nil
+	return newFeishuTextPayload(fmt.Sprintf("%s\n%s\n%s\n%s\n\n%s", title, link, by, operator, p.Issue.Body)), nil
 }
 
 // IssueComment implements PayloadConvertor IssueComment method
-func (f *FeishuPayload) IssueComment(p *api.IssueCommentPayload) (api.Payloader, error) {
+func (fc feishuConvertor) IssueComment(p *api.IssueCommentPayload) (FeishuPayload, error) {
 	title, link, by, operator := getIssuesCommentInfo(p)
 	return newFeishuTextPayload(fmt.Sprintf("%s\n%s\n%s\n%s\n\n%s", title, link, by, operator, p.Comment.Body)), nil
 }
 
 // PullRequest implements PayloadConvertor PullRequest method
-func (f *FeishuPayload) PullRequest(p *api.PullRequestPayload) (api.Payloader, error) {
+func (fc feishuConvertor) PullRequest(p *api.PullRequestPayload) (FeishuPayload, error) {
 	title, link, by, operator, result, assignees := getPullRequestInfo(p)
-	var res api.Payloader
 	if assignees != "" {
 		if p.Action == api.HookIssueAssigned || p.Action == api.HookIssueUnassigned || p.Action == api.HookIssueMilestoned {
-			res = newFeishuTextPayload(fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n%s\n\n%s", title, link, by, operator, result, assignees, p.PullRequest.Body))
-		} else {
-			res = newFeishuTextPayload(fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n\n%s", title, link, by, operator, assignees, p.PullRequest.Body))
+			return newFeishuTextPayload(fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n%s\n\n%s", title, link, by, operator, result, assignees, p.PullRequest.Body)), nil
 		}
-	} else {
-		res = newFeishuTextPayload(fmt.Sprintf("%s\n%s\n%s\n%s\n\n%s", title, link, by, operator, p.PullRequest.Body))
+		return newFeishuTextPayload(fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n\n%s", title, link, by, operator, assignees, p.PullRequest.Body)), nil
 	}
-	return res, nil
+	return newFeishuTextPayload(fmt.Sprintf("%s\n%s\n%s\n%s\n\n%s", title, link, by, operator, p.PullRequest.Body)), nil
 }
 
 // Review implements PayloadConvertor Review method
-func (f *FeishuPayload) Review(p *api.PullRequestPayload, event webhook_module.HookEventType) (api.Payloader, error) {
+func (fc feishuConvertor) Review(p *api.PullRequestPayload, event webhook_module.HookEventType) (FeishuPayload, error) {
 	action, err := parseHookPullRequestEventType(event)
 	if err != nil {
-		return nil, err
+		return FeishuPayload{}, err
 	}
 
 	title := fmt.Sprintf("[%s] Pull request review %s : #%d %s", p.Repository.FullName, action, p.Index, p.PullRequest.Title)
@@ -147,7 +130,7 @@ func (f *FeishuPayload) Review(p *api.PullRequestPayload, event webhook_module.H
 }
 
 // Repository implements PayloadConvertor Repository method
-func (f *FeishuPayload) Repository(p *api.RepositoryPayload) (api.Payloader, error) {
+func (fc feishuConvertor) Repository(p *api.RepositoryPayload) (FeishuPayload, error) {
 	var text string
 	switch p.Action {
 	case api.HookRepoCreated:
@@ -158,30 +141,33 @@ func (f *FeishuPayload) Repository(p *api.RepositoryPayload) (api.Payloader, err
 		return newFeishuTextPayload(text), nil
 	}
 
-	return nil, nil
+	return FeishuPayload{}, nil
 }
 
 // Wiki implements PayloadConvertor Wiki method
-func (f *FeishuPayload) Wiki(p *api.WikiPayload) (api.Payloader, error) {
+func (fc feishuConvertor) Wiki(p *api.WikiPayload) (FeishuPayload, error) {
 	text, _, _ := getWikiPayloadInfo(p, noneLinkFormatter, true)
 
 	return newFeishuTextPayload(text), nil
 }
 
 // Release implements PayloadConvertor Release method
-func (f *FeishuPayload) Release(p *api.ReleasePayload) (api.Payloader, error) {
+func (fc feishuConvertor) Release(p *api.ReleasePayload) (FeishuPayload, error) {
 	text, _ := getReleasePayloadInfo(p, noneLinkFormatter, true)
 
 	return newFeishuTextPayload(text), nil
 }
 
-func (f *FeishuPayload) Package(p *api.PackagePayload) (api.Payloader, error) {
+func (fc feishuConvertor) Package(p *api.PackagePayload) (FeishuPayload, error) {
 	text, _ := getPackagePayloadInfo(p, noneLinkFormatter, true)
 
 	return newFeishuTextPayload(text), nil
 }
 
-// GetFeishuPayload converts a ding talk webhook into a FeishuPayload
-func GetFeishuPayload(p api.Payloader, event webhook_module.HookEventType, _ string) (api.Payloader, error) {
-	return convertPayloader(new(FeishuPayload), p, event)
+type feishuConvertor struct{}
+
+var _ payloadConvertor[FeishuPayload] = feishuConvertor{}
+
+func newFeishuRequest(ctx context.Context, w *webhook_model.Webhook, t *webhook_model.HookTask) (*http.Request, []byte, error) {
+	return newJSONRequest(feishuConvertor{}, w, t, true)
 }

services/webhook/feishu_test.go

@@ -4,8 +4,11 @@
 package webhook
 
 import (
+	"context"
 	"testing"
 
+	webhook_model "code.gitea.io/gitea/models/webhook"
+	"code.gitea.io/gitea/modules/json"
 	api "code.gitea.io/gitea/modules/structs"
 	webhook_module "code.gitea.io/gitea/modules/webhook"
 
@@ -14,199 +17,177 @@ import (
 )
 
 func TestFeishuPayload(t *testing.T) {
+	fc := feishuConvertor{}
 	t.Run("Create", func(t *testing.T) {
 		p := createTestPayload()
 
-		d := new(FeishuPayload)
-		pl, err := d.Create(p)
+		pl, err := fc.Create(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &FeishuPayload{}, pl)
 
-		assert.Equal(t, `[test/repo] branch test created`, pl.(*FeishuPayload).Content.Text)
+		assert.Equal(t, `[test/repo] branch test created`, pl.Content.Text)
 	})
 
 	t.Run("Delete", func(t *testing.T) {
 		p := deleteTestPayload()
 
-		d := new(FeishuPayload)
-		pl, err := d.Delete(p)
+		pl, err := fc.Delete(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &FeishuPayload{}, pl)
 
-		assert.Equal(t, `[test/repo] branch test deleted`, pl.(*FeishuPayload).Content.Text)
+		assert.Equal(t, `[test/repo] branch test deleted`, pl.Content.Text)
 	})
 
 	t.Run("Fork", func(t *testing.T) {
 		p := forkTestPayload()
 
-		d := new(FeishuPayload)
-		pl, err := d.Fork(p)
+		pl, err := fc.Fork(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &FeishuPayload{}, pl)
 
-		assert.Equal(t, `test/repo2 is forked to test/repo`, pl.(*FeishuPayload).Content.Text)
+		assert.Equal(t, `test/repo2 is forked to test/repo`, pl.Content.Text)
 	})
 
 	t.Run("Push", func(t *testing.T) {
 		p := pushTestPayload()
 
-		d := new(FeishuPayload)
-		pl, err := d.Push(p)
+		pl, err := fc.Push(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &FeishuPayload{}, pl)
 
-		assert.Equal(t, "[test/repo:test] \r\n[2020558](http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778) commit message - user1\r\n[2020558](http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778) commit message - user1", pl.(*FeishuPayload).Content.Text)
+		assert.Equal(t, "[test/repo:test] \r\n[2020558](http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778) commit message - user1\r\n[2020558](http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778) commit message - user1", pl.Content.Text)
 	})
 
 	t.Run("Issue", func(t *testing.T) {
 		p := issueTestPayload()
 
-		d := new(FeishuPayload)
 		p.Action = api.HookIssueOpened
-		pl, err := d.Issue(p)
+		pl, err := fc.Issue(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &FeishuPayload{}, pl)
 
-		assert.Equal(t, "[Issue-test/repo #2]: opened\ncrash\nhttp://localhost:3000/test/repo/issues/2\nIssue by user1\nOperator: user1\nAssignees: user1\n\nissue body", pl.(*FeishuPayload).Content.Text)
+		assert.Equal(t, "[Issue-test/repo #2]: opened\ncrash\nhttp://localhost:3000/test/repo/issues/2\nIssue by user1\nOperator: user1\nAssignees: user1\n\nissue body", pl.Content.Text)
 
 		p.Action = api.HookIssueClosed
-		pl, err = d.Issue(p)
+		pl, err = fc.Issue(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &FeishuPayload{}, pl)
 
-		assert.Equal(t, "[Issue-test/repo #2]: closed\ncrash\nhttp://localhost:3000/test/repo/issues/2\nIssue by user1\nOperator: user1\nAssignees: user1\n\nissue body", pl.(*FeishuPayload).Content.Text)
+		assert.Equal(t, "[Issue-test/repo #2]: closed\ncrash\nhttp://localhost:3000/test/repo/issues/2\nIssue by user1\nOperator: user1\nAssignees: user1\n\nissue body", pl.Content.Text)
 	})
 
 	t.Run("IssueComment", func(t *testing.T) {
 		p := issueCommentTestPayload()
 
-		d := new(FeishuPayload)
-		pl, err := d.IssueComment(p)
+		pl, err := fc.IssueComment(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &FeishuPayload{}, pl)
 
-		assert.Equal(t, "[Comment-test/repo #2]: created\ncrash\nhttp://localhost:3000/test/repo/issues/2\nIssue by user1\nOperator: user1\n\nmore info needed", pl.(*FeishuPayload).Content.Text)
+		assert.Equal(t, "[Comment-test/repo #2]: created\ncrash\nhttp://localhost:3000/test/repo/issues/2\nIssue by user1\nOperator: user1\n\nmore info needed", pl.Content.Text)
 	})
 
 	t.Run("PullRequest", func(t *testing.T) {
 		p := pullRequestTestPayload()
 
-		d := new(FeishuPayload)
-		pl, err := d.PullRequest(p)
+		pl, err := fc.PullRequest(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &FeishuPayload{}, pl)
 
-		assert.Equal(t, "[PullRequest-test/repo #12]: opened\nFix bug\nhttp://localhost:3000/test/repo/pulls/12\nPullRequest by user1\nOperator: user1\nAssignees: user1\n\nfixes bug #2", pl.(*FeishuPayload).Content.Text)
+		assert.Equal(t, "[PullRequest-test/repo #12]: opened\nFix bug\nhttp://localhost:3000/test/repo/pulls/12\nPullRequest by user1\nOperator: user1\nAssignees: user1\n\nfixes bug #2", pl.Content.Text)
 	})
 
 	t.Run("PullRequestComment", func(t *testing.T) {
 		p := pullRequestCommentTestPayload()
 
-		d := new(FeishuPayload)
-		pl, err := d.IssueComment(p)
+		pl, err := fc.IssueComment(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &FeishuPayload{}, pl)
 
-		assert.Equal(t, "[Comment-test/repo #12]: created\nFix bug\nhttp://localhost:3000/test/repo/pulls/12\nPullRequest by user1\nOperator: user1\n\nchanges requested", pl.(*FeishuPayload).Content.Text)
+		assert.Equal(t, "[Comment-test/repo #12]: created\nFix bug\nhttp://localhost:3000/test/repo/pulls/12\nPullRequest by user1\nOperator: user1\n\nchanges requested", pl.Content.Text)
 	})
 
 	t.Run("Review", func(t *testing.T) {
 		p := pullRequestTestPayload()
 		p.Action = api.HookIssueReviewed
 
-		d := new(FeishuPayload)
-		pl, err := d.Review(p, webhook_module.HookEventPullRequestReviewApproved)
+		pl, err := fc.Review(p, webhook_module.HookEventPullRequestReviewApproved)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &FeishuPayload{}, pl)
 
-		assert.Equal(t, "[test/repo] Pull request review approved : #12 Fix bug\r\n\r\ngood job", pl.(*FeishuPayload).Content.Text)
+		assert.Equal(t, "[test/repo] Pull request review approved : #12 Fix bug\r\n\r\ngood job", pl.Content.Text)
 	})
 
 	t.Run("Repository", func(t *testing.T) {
 		p := repositoryTestPayload()
 
-		d := new(FeishuPayload)
-		pl, err := d.Repository(p)
+		pl, err := fc.Repository(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &FeishuPayload{}, pl)
 
-		assert.Equal(t, "[test/repo] Repository created", pl.(*FeishuPayload).Content.Text)
+		assert.Equal(t, "[test/repo] Repository created", pl.Content.Text)
 	})
 
 	t.Run("Package", func(t *testing.T) {
 		p := packageTestPayload()
 
-		d := new(FeishuPayload)
-		pl, err := d.Package(p)
+		pl, err := fc.Package(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &FeishuPayload{}, pl)
 
-		assert.Equal(t, "Package created: GiteaContainer:latest by user1", pl.(*FeishuPayload).Content.Text)
+		assert.Equal(t, "Package created: GiteaContainer:latest by user1", pl.Content.Text)
 	})
 
 	t.Run("Wiki", func(t *testing.T) {
 		p := wikiTestPayload()
 
-		d := new(FeishuPayload)
 		p.Action = api.HookWikiCreated
-		pl, err := d.Wiki(p)
+		pl, err := fc.Wiki(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &FeishuPayload{}, pl)
 
-		assert.Equal(t, "[test/repo] New wiki page 'index' (Wiki change comment) by user1", pl.(*FeishuPayload).Content.Text)
+		assert.Equal(t, "[test/repo] New wiki page 'index' (Wiki change comment) by user1", pl.Content.Text)
 
 		p.Action = api.HookWikiEdited
-		pl, err = d.Wiki(p)
+		pl, err = fc.Wiki(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &FeishuPayload{}, pl)
 
-		assert.Equal(t, "[test/repo] Wiki page 'index' edited (Wiki change comment) by user1", pl.(*FeishuPayload).Content.Text)
+		assert.Equal(t, "[test/repo] Wiki page 'index' edited (Wiki change comment) by user1", pl.Content.Text)
 
 		p.Action = api.HookWikiDeleted
-		pl, err = d.Wiki(p)
+		pl, err = fc.Wiki(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &FeishuPayload{}, pl)
 
-		assert.Equal(t, "[test/repo] Wiki page 'index' deleted by user1", pl.(*FeishuPayload).Content.Text)
+		assert.Equal(t, "[test/repo] Wiki page 'index' deleted by user1", pl.Content.Text)
 	})
 
 	t.Run("Release", func(t *testing.T) {
 		p := pullReleaseTestPayload()
 
-		d := new(FeishuPayload)
-		pl, err := d.Release(p)
+		pl, err := fc.Release(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &FeishuPayload{}, pl)
 
-		assert.Equal(t, "[test/repo] Release created: v1.0 by user1", pl.(*FeishuPayload).Content.Text)
+		assert.Equal(t, "[test/repo] Release created: v1.0 by user1", pl.Content.Text)
 	})
 }
 
 func TestFeishuJSONPayload(t *testing.T) {
 	p := pushTestPayload()
-
-	pl, err := new(FeishuPayload).Push(p)
+	data, err := p.JSONPayload()
 	require.NoError(t, err)
-	require.NotNil(t, pl)
-	require.IsType(t, &FeishuPayload{}, pl)
 
-	json, err := pl.JSONPayload()
+	hook := &webhook_model.Webhook{
+		RepoID:     3,
+		IsActive:   true,
+		Type:       webhook_module.FEISHU,
+		URL:        "https://feishu.example.com/",
+		Meta:       `{}`,
+		HTTPMethod: "POST",
+	}
+	task := &webhook_model.HookTask{
+		HookID:         hook.ID,
+		EventType:      webhook_module.HookEventPush,
+		PayloadContent: string(data),
+		PayloadVersion: 2,
+	}
+
+	req, reqBody, err := newFeishuRequest(context.Background(), hook, task)
+	require.NotNil(t, req)
+	require.NotNil(t, reqBody)
 	require.NoError(t, err)
-	assert.NotEmpty(t, json)
+
+	assert.Equal(t, "POST", req.Method)
+	assert.Equal(t, "https://feishu.example.com/", req.URL.String())
+	assert.Equal(t, "sha256=", req.Header.Get("X-Hub-Signature-256"))
+	assert.Equal(t, "application/json", req.Header.Get("Content-Type"))
+	var body FeishuPayload
+	err = json.NewDecoder(req.Body).Decode(&body)
+	assert.NoError(t, err)
+	assert.Equal(t, "[test/repo:test] \r\n[2020558](http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778) commit message - user1\r\n[2020558](http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778) commit message - user1", body.Content.Text)
 }

services/webhook/msteams.go

@@ -4,12 +4,14 @@
 package webhook
 
 import (
+	"context"
 	"fmt"
+	"net/http"
 	"net/url"
 	"strings"
 
+	webhook_model "code.gitea.io/gitea/models/webhook"
 	"code.gitea.io/gitea/modules/git"
-	"code.gitea.io/gitea/modules/json"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/util"
 	webhook_module "code.gitea.io/gitea/modules/webhook"
@@ -56,19 +58,8 @@ type (
 	}
 )
 
-// JSONPayload Marshals the MSTeamsPayload to json
-func (m *MSTeamsPayload) JSONPayload() ([]byte, error) {
-	data, err := json.MarshalIndent(m, "", "  ")
-	if err != nil {
-		return []byte{}, err
-	}
-	return data, nil
-}
-
-var _ PayloadConvertor = &MSTeamsPayload{}
-
 // Create implements PayloadConvertor Create method
-func (m *MSTeamsPayload) Create(p *api.CreatePayload) (api.Payloader, error) {
+func (m msteamsConvertor) Create(p *api.CreatePayload) (MSTeamsPayload, error) {
 	// created tag/branch
 	refName := git.RefName(p.Ref).ShortName()
 	title := fmt.Sprintf("[%s] %s %s created", p.Repo.FullName, p.RefType, refName)
@@ -85,7 +76,7 @@ func (m *MSTeamsPayload) Create(p *api.CreatePayload) (api.Payloader, error) {
 }
 
 // Delete implements PayloadConvertor Delete method
-func (m *MSTeamsPayload) Delete(p *api.DeletePayload) (api.Payloader, error) {
+func (m msteamsConvertor) Delete(p *api.DeletePayload) (MSTeamsPayload, error) {
 	// deleted tag/branch
 	refName := git.RefName(p.Ref).ShortName()
 	title := fmt.Sprintf("[%s] %s %s deleted", p.Repo.FullName, p.RefType, refName)
@@ -102,7 +93,7 @@ func (m *MSTeamsPayload) Delete(p *api.DeletePayload) (api.Payloader, error) {
 }
 
 // Fork implements PayloadConvertor Fork method
-func (m *MSTeamsPayload) Fork(p *api.ForkPayload) (api.Payloader, error) {
+func (m msteamsConvertor) Fork(p *api.ForkPayload) (MSTeamsPayload, error) {
 	title := fmt.Sprintf("%s is forked to %s", p.Forkee.FullName, p.Repo.FullName)
 
 	return createMSTeamsPayload(
@@ -117,7 +108,7 @@ func (m *MSTeamsPayload) Fork(p *api.ForkPayload) (api.Payloader, error) {
 }
 
 // Push implements PayloadConvertor Push method
-func (m *MSTeamsPayload) Push(p *api.PushPayload) (api.Payloader, error) {
+func (m msteamsConvertor) Push(p *api.PushPayload) (MSTeamsPayload, error) {
 	var (
 		branchName = git.RefName(p.Ref).ShortName()
 		commitDesc string
@@ -160,7 +151,7 @@ func (m *MSTeamsPayload) Push(p *api.PushPayload) (api.Payloader, error) {
 }
 
 // Issue implements PayloadConvertor Issue method
-func (m *MSTeamsPayload) Issue(p *api.IssuePayload) (api.Payloader, error) {
+func (m msteamsConvertor) Issue(p *api.IssuePayload) (MSTeamsPayload, error) {
 	title, _, attachmentText, color := getIssuesPayloadInfo(p, noneLinkFormatter, false)
 
 	return createMSTeamsPayload(
@@ -175,7 +166,7 @@ func (m *MSTeamsPayload) Issue(p *api.IssuePayload) (api.Payloader, error) {
 }
 
 // IssueComment implements PayloadConvertor IssueComment method
-func (m *MSTeamsPayload) IssueComment(p *api.IssueCommentPayload) (api.Payloader, error) {
+func (m msteamsConvertor) IssueComment(p *api.IssueCommentPayload) (MSTeamsPayload, error) {
 	title, _, color := getIssueCommentPayloadInfo(p, noneLinkFormatter, false)
 
 	return createMSTeamsPayload(
@@ -190,7 +181,7 @@ func (m *MSTeamsPayload) IssueComment(p *api.IssueCommentPayload) (api.Payloader
 }
 
 // PullRequest implements PayloadConvertor PullRequest method
-func (m *MSTeamsPayload) PullRequest(p *api.PullRequestPayload) (api.Payloader, error) {
+func (m msteamsConvertor) PullRequest(p *api.PullRequestPayload) (MSTeamsPayload, error) {
 	title, _, attachmentText, color := getPullRequestPayloadInfo(p, noneLinkFormatter, false)
 
 	return createMSTeamsPayload(
@@ -205,14 +196,14 @@ func (m *MSTeamsPayload) PullRequest(p *api.PullRequestPayload) (api.Payloader,
 }
 
 // Review implements PayloadConvertor Review method
-func (m *MSTeamsPayload) Review(p *api.PullRequestPayload, event webhook_module.HookEventType) (api.Payloader, error) {
+func (m msteamsConvertor) Review(p *api.PullRequestPayload, event webhook_module.HookEventType) (MSTeamsPayload, error) {
 	var text, title string
 	var color int
 	switch p.Action {
 	case api.HookIssueReviewed:
 		action, err := parseHookPullRequestEventType(event)
 		if err != nil {
-			return nil, err
+			return MSTeamsPayload{}, err
 		}
 
 		title = fmt.Sprintf("[%s] Pull request review %s: #%d %s", p.Repository.FullName, action, p.Index, p.PullRequest.Title)
@@ -242,7 +233,7 @@ func (m *MSTeamsPayload) Review(p *api.PullRequestPayload, event webhook_module.
 }
 
 // Repository implements PayloadConvertor Repository method
-func (m *MSTeamsPayload) Repository(p *api.RepositoryPayload) (api.Payloader, error) {
+func (m msteamsConvertor) Repository(p *api.RepositoryPayload) (MSTeamsPayload, error) {
 	var title, url string
 	var color int
 	switch p.Action {
@@ -267,7 +258,7 @@ func (m *MSTeamsPayload) Repository(p *api.RepositoryPayload) (api.Payloader, er
 }
 
 // Wiki implements PayloadConvertor Wiki method
-func (m *MSTeamsPayload) Wiki(p *api.WikiPayload) (api.Payloader, error) {
+func (m msteamsConvertor) Wiki(p *api.WikiPayload) (MSTeamsPayload, error) {
 	title, color, _ := getWikiPayloadInfo(p, noneLinkFormatter, false)
 
 	return createMSTeamsPayload(
@@ -282,7 +273,7 @@ func (m *MSTeamsPayload) Wiki(p *api.WikiPayload) (api.Payloader, error) {
 }
 
 // Release implements PayloadConvertor Release method
-func (m *MSTeamsPayload) Release(p *api.ReleasePayload) (api.Payloader, error) {
+func (m msteamsConvertor) Release(p *api.ReleasePayload) (MSTeamsPayload, error) {
 	title, color := getReleasePayloadInfo(p, noneLinkFormatter, false)
 
 	return createMSTeamsPayload(
@@ -296,7 +287,7 @@ func (m *MSTeamsPayload) Release(p *api.ReleasePayload) (api.Payloader, error) {
 	), nil
 }
 
-func (m *MSTeamsPayload) Package(p *api.PackagePayload) (api.Payloader, error) {
+func (m msteamsConvertor) Package(p *api.PackagePayload) (MSTeamsPayload, error) {
 	title, color := getPackagePayloadInfo(p, noneLinkFormatter, false)
 
 	return createMSTeamsPayload(
@@ -310,12 +301,7 @@ func (m *MSTeamsPayload) Package(p *api.PackagePayload) (api.Payloader, error) {
 	), nil
 }
 
-// GetMSTeamsPayload converts a MSTeams webhook into a MSTeamsPayload
-func GetMSTeamsPayload(p api.Payloader, event webhook_module.HookEventType, _ string) (api.Payloader, error) {
-	return convertPayloader(new(MSTeamsPayload), p, event)
-}
-
-func createMSTeamsPayload(r *api.Repository, s *api.User, title, text, actionTarget string, color int, fact *MSTeamsFact) *MSTeamsPayload {
+func createMSTeamsPayload(r *api.Repository, s *api.User, title, text, actionTarget string, color int, fact *MSTeamsFact) MSTeamsPayload {
 	facts := make([]MSTeamsFact, 0, 2)
 	if r != nil {
 		facts = append(facts, MSTeamsFact{
@@ -327,7 +313,7 @@ func createMSTeamsPayload(r *api.Repository, s *api.User, title, text, actionTar
 		facts = append(facts, *fact)
 	}
 
-	return &MSTeamsPayload{
+	return MSTeamsPayload{
 		Type:       "MessageCard",
 		Context:    "https://schema.org/extensions",
 		ThemeColor: fmt.Sprintf("%x", color),
@@ -356,3 +342,11 @@ func createMSTeamsPayload(r *api.Repository, s *api.User, title, text, actionTar
 		},
 	}
 }
+
+type msteamsConvertor struct{}
+
+var _ payloadConvertor[MSTeamsPayload] = msteamsConvertor{}
+
+func newMSTeamsRequest(ctx context.Context, w *webhook_model.Webhook, t *webhook_model.HookTask) (*http.Request, []byte, error) {
+	return newJSONRequest(msteamsConvertor{}, w, t, true)
+}

services/webhook/packagist.go

@@ -4,7 +4,9 @@
 package webhook
 
 import (
-	"errors"
+	"context"
+	"fmt"
+	"net/http"
 
 	webhook_model "code.gitea.io/gitea/models/webhook"
 	"code.gitea.io/gitea/modules/json"
@@ -38,84 +40,85 @@ func GetPackagistHook(w *webhook_model.Webhook) *PackagistMeta {
 	return s
 }
 
-// JSONPayload Marshals the PackagistPayload to json
-func (f *PackagistPayload) JSONPayload() ([]byte, error) {
-	data, err := json.MarshalIndent(f, "", "  ")
-	if err != nil {
-		return []byte{}, err
-	}
-	return data, nil
-}
-
-var _ PayloadConvertor = &PackagistPayload{}
-
 // Create implements PayloadConvertor Create method
-func (f *PackagistPayload) Create(_ *api.CreatePayload) (api.Payloader, error) {
-	return nil, nil
+func (pc packagistConvertor) Create(_ *api.CreatePayload) (PackagistPayload, error) {
+	return PackagistPayload{}, nil
 }
 
 // Delete implements PayloadConvertor Delete method
-func (f *PackagistPayload) Delete(_ *api.DeletePayload) (api.Payloader, error) {
-	return nil, nil
+func (pc packagistConvertor) Delete(_ *api.DeletePayload) (PackagistPayload, error) {
+	return PackagistPayload{}, nil
 }
 
 // Fork implements PayloadConvertor Fork method
-func (f *PackagistPayload) Fork(_ *api.ForkPayload) (api.Payloader, error) {
-	return nil, nil
+func (pc packagistConvertor) Fork(_ *api.ForkPayload) (PackagistPayload, error) {
+	return PackagistPayload{}, nil
 }
 
 // Push implements PayloadConvertor Push method
-func (f *PackagistPayload) Push(_ *api.PushPayload) (api.Payloader, error) {
-	return f, nil
+// https://packagist.org/about
+func (pc packagistConvertor) Push(_ *api.PushPayload) (PackagistPayload, error) {
+	return PackagistPayload{
+		PackagistRepository: struct {
+			URL string `json:"url"`
+		}{
+			URL: pc.PackageURL,
+		},
+	}, nil
 }
 
 // Issue implements PayloadConvertor Issue method
-func (f *PackagistPayload) Issue(_ *api.IssuePayload) (api.Payloader, error) {
-	return nil, nil
+func (pc packagistConvertor) Issue(_ *api.IssuePayload) (PackagistPayload, error) {
+	return PackagistPayload{}, nil
 }
 
 // IssueComment implements PayloadConvertor IssueComment method
-func (f *PackagistPayload) IssueComment(_ *api.IssueCommentPayload) (api.Payloader, error) {
-	return nil, nil
+func (pc packagistConvertor) IssueComment(_ *api.IssueCommentPayload) (PackagistPayload, error) {
+	return PackagistPayload{}, nil
 }
 
 // PullRequest implements PayloadConvertor PullRequest method
-func (f *PackagistPayload) PullRequest(_ *api.PullRequestPayload) (api.Payloader, error) {
-	return nil, nil
+func (pc packagistConvertor) PullRequest(_ *api.PullRequestPayload) (PackagistPayload, error) {
+	return PackagistPayload{}, nil
 }
 
 // Review implements PayloadConvertor Review method
-func (f *PackagistPayload) Review(_ *api.PullRequestPayload, _ webhook_module.HookEventType) (api.Payloader, error) {
-	return nil, nil
+func (pc packagistConvertor) Review(_ *api.PullRequestPayload, _ webhook_module.HookEventType) (PackagistPayload, error) {
+	return PackagistPayload{}, nil
 }
 
 // Repository implements PayloadConvertor Repository method
-func (f *PackagistPayload) Repository(_ *api.RepositoryPayload) (api.Payloader, error) {
-	return nil, nil
+func (pc packagistConvertor) Repository(_ *api.RepositoryPayload) (PackagistPayload, error) {
+	return PackagistPayload{}, nil
 }
 
 // Wiki implements PayloadConvertor Wiki method
-func (f *PackagistPayload) Wiki(_ *api.WikiPayload) (api.Payloader, error) {
-	return nil, nil
+func (pc packagistConvertor) Wiki(_ *api.WikiPayload) (PackagistPayload, error) {
+	return PackagistPayload{}, nil
 }
 
 // Release implements PayloadConvertor Release method
-func (f *PackagistPayload) Release(_ *api.ReleasePayload) (api.Payloader, error) {
-	return nil, nil
+func (pc packagistConvertor) Release(_ *api.ReleasePayload) (PackagistPayload, error) {
+	return PackagistPayload{}, nil
 }
 
-func (f *PackagistPayload) Package(_ *api.PackagePayload) (api.Payloader, error) {
-	return nil, nil
+func (pc packagistConvertor) Package(_ *api.PackagePayload) (PackagistPayload, error) {
+	return PackagistPayload{}, nil
 }
 
-// GetPackagistPayload converts a packagist webhook into a PackagistPayload
-func GetPackagistPayload(p api.Payloader, event webhook_module.HookEventType, meta string) (api.Payloader, error) {
-	s := new(PackagistPayload)
+type packagistConvertor struct {
+	PackageURL string
+}
 
-	packagist := &PackagistMeta{}
-	if err := json.Unmarshal([]byte(meta), &packagist); err != nil {
-		return s, errors.New("GetPackagistPayload meta json:" + err.Error())
+var _ payloadConvertor[PackagistPayload] = packagistConvertor{}
+
+func newPackagistRequest(ctx context.Context, w *webhook_model.Webhook, t *webhook_model.HookTask) (*http.Request, []byte, error) {
+	meta := &PackagistMeta{}
+	if err := json.Unmarshal([]byte(w.Meta), meta); err != nil {
+		return nil, nil, fmt.Errorf("newpackagistRequest meta json: %w", err)
 	}
-	s.PackagistRepository.URL = packagist.PackageURL
-	return convertPayloader(s, p, event)
+	pc := packagistConvertor{
+		PackageURL: meta.PackageURL,
+	}
+	return newJSONRequest(pc, w, t, true)
 }

services/webhook/packagist_test.go

@@ -4,8 +4,11 @@
 package webhook
 
 import (
+	"context"
 	"testing"
 
+	webhook_model "code.gitea.io/gitea/models/webhook"
+	"code.gitea.io/gitea/modules/json"
 	api "code.gitea.io/gitea/modules/structs"
 	webhook_module "code.gitea.io/gitea/modules/webhook"
 
@@ -14,155 +17,199 @@ import (
 )
 
 func TestPackagistPayload(t *testing.T) {
+	pc := packagistConvertor{
+		PackageURL: "https://packagist.org/packages/example",
+	}
 	t.Run("Create", func(t *testing.T) {
 		p := createTestPayload()
 
-		d := new(PackagistPayload)
-		pl, err := d.Create(p)
+		pl, err := pc.Create(p)
 		require.NoError(t, err)
-		require.Nil(t, pl)
+		require.Equal(t, pl, PackagistPayload{})
 	})
 
 	t.Run("Delete", func(t *testing.T) {
 		p := deleteTestPayload()
 
-		d := new(PackagistPayload)
-		pl, err := d.Delete(p)
+		pl, err := pc.Delete(p)
 		require.NoError(t, err)
-		require.Nil(t, pl)
+		require.Equal(t, pl, PackagistPayload{})
 	})
 
 	t.Run("Fork", func(t *testing.T) {
 		p := forkTestPayload()
 
-		d := new(PackagistPayload)
-		pl, err := d.Fork(p)
+		pl, err := pc.Fork(p)
 		require.NoError(t, err)
-		require.Nil(t, pl)
+		require.Equal(t, pl, PackagistPayload{})
 	})
 
 	t.Run("Push", func(t *testing.T) {
 		p := pushTestPayload()
 
-		d := new(PackagistPayload)
-		d.PackagistRepository.URL = "https://packagist.org/api/update-package?username=THEUSERNAME&apiToken=TOPSECRETAPITOKEN"
-		pl, err := d.Push(p)
+		pl, err := pc.Push(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &PackagistPayload{}, pl)
 
-		assert.Equal(t, "https://packagist.org/api/update-package?username=THEUSERNAME&apiToken=TOPSECRETAPITOKEN", pl.(*PackagistPayload).PackagistRepository.URL)
+		assert.Equal(t, "https://packagist.org/packages/example", pl.PackagistRepository.URL)
 	})
 
 	t.Run("Issue", func(t *testing.T) {
 		p := issueTestPayload()
 
-		d := new(PackagistPayload)
 		p.Action = api.HookIssueOpened
-		pl, err := d.Issue(p)
+		pl, err := pc.Issue(p)
 		require.NoError(t, err)
-		require.Nil(t, pl)
+		require.Equal(t, pl, PackagistPayload{})
 
 		p.Action = api.HookIssueClosed
-		pl, err = d.Issue(p)
+		pl, err = pc.Issue(p)
 		require.NoError(t, err)
-		require.Nil(t, pl)
+		require.Equal(t, pl, PackagistPayload{})
 	})
 
 	t.Run("IssueComment", func(t *testing.T) {
 		p := issueCommentTestPayload()
 
-		d := new(PackagistPayload)
-		pl, err := d.IssueComment(p)
+		pl, err := pc.IssueComment(p)
 		require.NoError(t, err)
-		require.Nil(t, pl)
+		require.Equal(t, pl, PackagistPayload{})
 	})
 
 	t.Run("PullRequest", func(t *testing.T) {
 		p := pullRequestTestPayload()
 
-		d := new(PackagistPayload)
-		pl, err := d.PullRequest(p)
+		pl, err := pc.PullRequest(p)
 		require.NoError(t, err)
-		require.Nil(t, pl)
+		require.Equal(t, pl, PackagistPayload{})
 	})
 
 	t.Run("PullRequestComment", func(t *testing.T) {
 		p := pullRequestCommentTestPayload()
 
-		d := new(PackagistPayload)
-		pl, err := d.IssueComment(p)
+		pl, err := pc.IssueComment(p)
 		require.NoError(t, err)
-		require.Nil(t, pl)
+		require.Equal(t, pl, PackagistPayload{})
 	})
 
 	t.Run("Review", func(t *testing.T) {
 		p := pullRequestTestPayload()
 		p.Action = api.HookIssueReviewed
 
-		d := new(PackagistPayload)
-		pl, err := d.Review(p, webhook_module.HookEventPullRequestReviewApproved)
+		pl, err := pc.Review(p, webhook_module.HookEventPullRequestReviewApproved)
 		require.NoError(t, err)
-		require.Nil(t, pl)
+		require.Equal(t, pl, PackagistPayload{})
 	})
 
 	t.Run("Repository", func(t *testing.T) {
 		p := repositoryTestPayload()
 
-		d := new(PackagistPayload)
-		pl, err := d.Repository(p)
+		pl, err := pc.Repository(p)
 		require.NoError(t, err)
-		require.Nil(t, pl)
+		require.Equal(t, pl, PackagistPayload{})
 	})
 
 	t.Run("Package", func(t *testing.T) {
 		p := packageTestPayload()
 
-		d := new(PackagistPayload)
-		pl, err := d.Package(p)
+		pl, err := pc.Package(p)
 		require.NoError(t, err)
-		require.Nil(t, pl)
+		require.Equal(t, pl, PackagistPayload{})
 	})
 
 	t.Run("Wiki", func(t *testing.T) {
 		p := wikiTestPayload()
 
-		d := new(PackagistPayload)
 		p.Action = api.HookWikiCreated
-		pl, err := d.Wiki(p)
+		pl, err := pc.Wiki(p)
 		require.NoError(t, err)
-		require.Nil(t, pl)
+		require.Equal(t, pl, PackagistPayload{})
 
 		p.Action = api.HookWikiEdited
-		pl, err = d.Wiki(p)
+		pl, err = pc.Wiki(p)
 		require.NoError(t, err)
-		require.Nil(t, pl)
+		require.Equal(t, pl, PackagistPayload{})
 
 		p.Action = api.HookWikiDeleted
-		pl, err = d.Wiki(p)
+		pl, err = pc.Wiki(p)
 		require.NoError(t, err)
-		require.Nil(t, pl)
+		require.Equal(t, pl, PackagistPayload{})
 	})
 
 	t.Run("Release", func(t *testing.T) {
 		p := pullReleaseTestPayload()
 
-		d := new(PackagistPayload)
-		pl, err := d.Release(p)
+		pl, err := pc.Release(p)
 		require.NoError(t, err)
-		require.Nil(t, pl)
+		require.Equal(t, pl, PackagistPayload{})
 	})
 }
 
 func TestPackagistJSONPayload(t *testing.T) {
 	p := pushTestPayload()
-
-	pl, err := new(PackagistPayload).Push(p)
+	data, err := p.JSONPayload()
 	require.NoError(t, err)
-	require.NotNil(t, pl)
-	require.IsType(t, &PackagistPayload{}, pl)
 
-	json, err := pl.JSONPayload()
+	hook := &webhook_model.Webhook{
+		RepoID:     3,
+		IsActive:   true,
+		Type:       webhook_module.PACKAGIST,
+		URL:        "https://packagist.org/api/update-package?username=THEUSERNAME&apiToken=TOPSECRETAPITOKEN",
+		Meta:       `{"package_url":"https://packagist.org/packages/example"}`,
+		HTTPMethod: "POST",
+	}
+	task := &webhook_model.HookTask{
+		HookID:         hook.ID,
+		EventType:      webhook_module.HookEventPush,
+		PayloadContent: string(data),
+		PayloadVersion: 2,
+	}
+
+	req, reqBody, err := newPackagistRequest(context.Background(), hook, task)
+	require.NotNil(t, req)
+	require.NotNil(t, reqBody)
 	require.NoError(t, err)
-	assert.NotEmpty(t, json)
+
+	assert.Equal(t, "POST", req.Method)
+	assert.Equal(t, "https://packagist.org/api/update-package?username=THEUSERNAME&apiToken=TOPSECRETAPITOKEN", req.URL.String())
+	assert.Equal(t, "sha256=", req.Header.Get("X-Hub-Signature-256"))
+	assert.Equal(t, "application/json", req.Header.Get("Content-Type"))
+	var body PackagistPayload
+	err = json.NewDecoder(req.Body).Decode(&body)
+	assert.NoError(t, err)
+	assert.Equal(t, "https://packagist.org/packages/example", body.PackagistRepository.URL)
+}
+
+func TestPackagistEmptyPayload(t *testing.T) {
+	p := createTestPayload()
+	data, err := p.JSONPayload()
+	require.NoError(t, err)
+
+	hook := &webhook_model.Webhook{
+		RepoID:     3,
+		IsActive:   true,
+		Type:       webhook_module.PACKAGIST,
+		URL:        "https://packagist.org/api/update-package?username=THEUSERNAME&apiToken=TOPSECRETAPITOKEN",
+		Meta:       `{"package_url":"https://packagist.org/packages/example"}`,
+		HTTPMethod: "POST",
+	}
+	task := &webhook_model.HookTask{
+		HookID:         hook.ID,
+		EventType:      webhook_module.HookEventCreate,
+		PayloadContent: string(data),
+		PayloadVersion: 2,
+	}
+
+	req, reqBody, err := newPackagistRequest(context.Background(), hook, task)
+	require.NotNil(t, req)
+	require.NotNil(t, reqBody)
+	require.NoError(t, err)
+
+	assert.Equal(t, "POST", req.Method)
+	assert.Equal(t, "https://packagist.org/api/update-package?username=THEUSERNAME&apiToken=TOPSECRETAPITOKEN", req.URL.String())
+	assert.Equal(t, "sha256=", req.Header.Get("X-Hub-Signature-256"))
+	assert.Equal(t, "application/json", req.Header.Get("Content-Type"))
+	var body PackagistPayload
+	err = json.NewDecoder(req.Body).Decode(&body)
+	assert.NoError(t, err)
+	assert.Equal(t, "", body.PackagistRepository.URL)
 }

services/webhook/payloader.go

@@ -4,58 +4,104 @@
 package webhook
 
 import (
+	"bytes"
+	"fmt"
+	"net/http"
+
+	webhook_model "code.gitea.io/gitea/models/webhook"
+	"code.gitea.io/gitea/modules/json"
 	api "code.gitea.io/gitea/modules/structs"
 	webhook_module "code.gitea.io/gitea/modules/webhook"
 )
 
-// PayloadConvertor defines the interface to convert system webhook payload to external payload
-type PayloadConvertor interface {
-	api.Payloader
-	Create(*api.CreatePayload) (api.Payloader, error)
-	Delete(*api.DeletePayload) (api.Payloader, error)
-	Fork(*api.ForkPayload) (api.Payloader, error)
-	Issue(*api.IssuePayload) (api.Payloader, error)
-	IssueComment(*api.IssueCommentPayload) (api.Payloader, error)
-	Push(*api.PushPayload) (api.Payloader, error)
-	PullRequest(*api.PullRequestPayload) (api.Payloader, error)
-	Review(*api.PullRequestPayload, webhook_module.HookEventType) (api.Payloader, error)
-	Repository(*api.RepositoryPayload) (api.Payloader, error)
-	Release(*api.ReleasePayload) (api.Payloader, error)
-	Wiki(*api.WikiPayload) (api.Payloader, error)
-	Package(*api.PackagePayload) (api.Payloader, error)
+// payloadConvertor defines the interface to convert system payload to webhook payload
+type payloadConvertor[T any] interface {
+	Create(*api.CreatePayload) (T, error)
+	Delete(*api.DeletePayload) (T, error)
+	Fork(*api.ForkPayload) (T, error)
+	Issue(*api.IssuePayload) (T, error)
+	IssueComment(*api.IssueCommentPayload) (T, error)
+	Push(*api.PushPayload) (T, error)
+	PullRequest(*api.PullRequestPayload) (T, error)
+	Review(*api.PullRequestPayload, webhook_module.HookEventType) (T, error)
+	Repository(*api.RepositoryPayload) (T, error)
+	Release(*api.ReleasePayload) (T, error)
+	Wiki(*api.WikiPayload) (T, error)
+	Package(*api.PackagePayload) (T, error)
 }
 
-func convertPayloader(s PayloadConvertor, p api.Payloader, event webhook_module.HookEventType) (api.Payloader, error) {
+func convertUnmarshalledJSON[T, P any](convert func(P) (T, error), data []byte) (T, error) {
+	var p P
+	if err := json.Unmarshal(data, &p); err != nil {
+		var t T
+		return t, fmt.Errorf("could not unmarshal payload: %w", err)
+	}
+	return convert(p)
+}
+
+func newPayload[T any](rc payloadConvertor[T], data []byte, event webhook_module.HookEventType) (T, error) {
 	switch event {
 	case webhook_module.HookEventCreate:
-		return s.Create(p.(*api.CreatePayload))
+		return convertUnmarshalledJSON(rc.Create, data)
 	case webhook_module.HookEventDelete:
-		return s.Delete(p.(*api.DeletePayload))
+		return convertUnmarshalledJSON(rc.Delete, data)
 	case webhook_module.HookEventFork:
-		return s.Fork(p.(*api.ForkPayload))
+		return convertUnmarshalledJSON(rc.Fork, data)
 	case webhook_module.HookEventIssues, webhook_module.HookEventIssueAssign, webhook_module.HookEventIssueLabel, webhook_module.HookEventIssueMilestone:
-		return s.Issue(p.(*api.IssuePayload))
+		return convertUnmarshalledJSON(rc.Issue, data)
 	case webhook_module.HookEventIssueComment, webhook_module.HookEventPullRequestComment:
-		pl, ok := p.(*api.IssueCommentPayload)
-		if ok {
-			return s.IssueComment(pl)
-		}
-		return s.PullRequest(p.(*api.PullRequestPayload))
+		// previous code sometimes sent s.PullRequest(p.(*api.PullRequestPayload))
+		// however I couldn't find in notifier.go such a payload with an HookEvent***Comment event
+
+		// History (most recent first):
+		//  - refactored in https://github.com/go-gitea/gitea/pull/12310
+		//  - assertion added in https://github.com/go-gitea/gitea/pull/12046
+		//  - issue raised in https://github.com/go-gitea/gitea/issues/11940#issuecomment-645713996
+		//    > That's because for HookEventPullRequestComment event, some places use IssueCommentPayload and others use PullRequestPayload
+
+		// In modules/actions/workflows.go:183 the type assertion is always payload.(*api.IssueCommentPayload)
+		return convertUnmarshalledJSON(rc.IssueComment, data)
 	case webhook_module.HookEventPush:
-		return s.Push(p.(*api.PushPayload))
+		return convertUnmarshalledJSON(rc.Push, data)
 	case webhook_module.HookEventPullRequest, webhook_module.HookEventPullRequestAssign, webhook_module.HookEventPullRequestLabel,
 		webhook_module.HookEventPullRequestMilestone, webhook_module.HookEventPullRequestSync, webhook_module.HookEventPullRequestReviewRequest:
-		return s.PullRequest(p.(*api.PullRequestPayload))
+		return convertUnmarshalledJSON(rc.PullRequest, data)
 	case webhook_module.HookEventPullRequestReviewApproved, webhook_module.HookEventPullRequestReviewRejected, webhook_module.HookEventPullRequestReviewComment:
-		return s.Review(p.(*api.PullRequestPayload), event)
+		return convertUnmarshalledJSON(func(p *api.PullRequestPayload) (T, error) {
+			return rc.Review(p, event)
+		}, data)
 	case webhook_module.HookEventRepository:
-		return s.Repository(p.(*api.RepositoryPayload))
+		return convertUnmarshalledJSON(rc.Repository, data)
 	case webhook_module.HookEventRelease:
-		return s.Release(p.(*api.ReleasePayload))
+		return convertUnmarshalledJSON(rc.Release, data)
 	case webhook_module.HookEventWiki:
-		return s.Wiki(p.(*api.WikiPayload))
+		return convertUnmarshalledJSON(rc.Wiki, data)
 	case webhook_module.HookEventPackage:
-		return s.Package(p.(*api.PackagePayload))
+		return convertUnmarshalledJSON(rc.Package, data)
 	}
-	return s, nil
+	var t T
+	return t, fmt.Errorf("newPayload unsupported event: %s", event)
+}
+
+func newJSONRequest[T any](pc payloadConvertor[T], w *webhook_model.Webhook, t *webhook_model.HookTask, withDefaultHeaders bool) (*http.Request, []byte, error) {
+	payload, err := newPayload(pc, []byte(t.PayloadContent), t.EventType)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	body, err := json.MarshalIndent(payload, "", "  ")
+	if err != nil {
+		return nil, nil, err
+	}
+
+	req, err := http.NewRequest(w.HTTPMethod, w.URL, bytes.NewReader(body))
+	if err != nil {
+		return nil, nil, err
+	}
+	req.Header.Set("Content-Type", "application/json")
+
+	if withDefaultHeaders {
+		return req, body, addDefaultHeaders(req, []byte(w.Secret), t, body)
+	}
+	return req, body, nil
 }

services/webhook/slack.go

@@ -4,8 +4,9 @@
 package webhook
 
 import (
-	"errors"
+	"context"
 	"fmt"
+	"net/http"
 	"regexp"
 	"strings"
 
@@ -39,7 +40,6 @@ func GetSlackHook(w *webhook_model.Webhook) *SlackMeta {
 type SlackPayload struct {
 	Channel     string            `json:"channel"`
 	Text        string            `json:"text"`
-	Color       string            `json:"-"`
 	Username    string            `json:"username"`
 	IconURL     string            `json:"icon_url"`
 	UnfurlLinks int               `json:"unfurl_links"`
@@ -56,15 +56,6 @@ type SlackAttachment struct {
 	Text      string `json:"text"`
 }
 
-// JSONPayload Marshals the SlackPayload to json
-func (s *SlackPayload) JSONPayload() ([]byte, error) {
-	data, err := json.MarshalIndent(s, "", "  ")
-	if err != nil {
-		return []byte{}, err
-	}
-	return data, nil
-}
-
 // SlackTextFormatter replaces &, <, > with HTML characters
 // see: https://api.slack.com/docs/formatting
 func SlackTextFormatter(s string) string {
@@ -98,10 +89,8 @@ func SlackLinkToRef(repoURL, ref string) string {
 	return SlackLinkFormatter(url, refName)
 }
 
-var _ PayloadConvertor = &SlackPayload{}
-
-// Create implements PayloadConvertor Create method
-func (s *SlackPayload) Create(p *api.CreatePayload) (api.Payloader, error) {
+// Create implements payloadConvertor Create method
+func (s slackConvertor) Create(p *api.CreatePayload) (SlackPayload, error) {
 	repoLink := SlackLinkFormatter(p.Repo.HTMLURL, p.Repo.FullName)
 	refLink := SlackLinkToRef(p.Repo.HTMLURL, p.Ref)
 	text := fmt.Sprintf("[%s:%s] %s created by %s", repoLink, refLink, p.RefType, p.Sender.UserName)
@@ -110,7 +99,7 @@ func (s *SlackPayload) Create(p *api.CreatePayload) (api.Payloader, error) {
 }
 
 // Delete composes Slack payload for delete a branch or tag.
-func (s *SlackPayload) Delete(p *api.DeletePayload) (api.Payloader, error) {
+func (s slackConvertor) Delete(p *api.DeletePayload) (SlackPayload, error) {
 	refName := git.RefName(p.Ref).ShortName()
 	repoLink := SlackLinkFormatter(p.Repo.HTMLURL, p.Repo.FullName)
 	text := fmt.Sprintf("[%s:%s] %s deleted by %s", repoLink, refName, p.RefType, p.Sender.UserName)
@@ -119,7 +108,7 @@ func (s *SlackPayload) Delete(p *api.DeletePayload) (api.Payloader, error) {
 }
 
 // Fork composes Slack payload for forked by a repository.
-func (s *SlackPayload) Fork(p *api.ForkPayload) (api.Payloader, error) {
+func (s slackConvertor) Fork(p *api.ForkPayload) (SlackPayload, error) {
 	baseLink := SlackLinkFormatter(p.Forkee.HTMLURL, p.Forkee.FullName)
 	forkLink := SlackLinkFormatter(p.Repo.HTMLURL, p.Repo.FullName)
 	text := fmt.Sprintf("%s is forked to %s", baseLink, forkLink)
@@ -127,8 +116,8 @@ func (s *SlackPayload) Fork(p *api.ForkPayload) (api.Payloader, error) {
 	return s.createPayload(text, nil), nil
 }
 
-// Issue implements PayloadConvertor Issue method
-func (s *SlackPayload) Issue(p *api.IssuePayload) (api.Payloader, error) {
+// Issue implements payloadConvertor Issue method
+func (s slackConvertor) Issue(p *api.IssuePayload) (SlackPayload, error) {
 	text, issueTitle, attachmentText, color := getIssuesPayloadInfo(p, SlackLinkFormatter, true)
 
 	var attachments []SlackAttachment
@@ -146,8 +135,8 @@ func (s *SlackPayload) Issue(p *api.IssuePayload) (api.Payloader, error) {
 	return s.createPayload(text, attachments), nil
 }
 
-// IssueComment implements PayloadConvertor IssueComment method
-func (s *SlackPayload) IssueComment(p *api.IssueCommentPayload) (api.Payloader, error) {
+// IssueComment implements payloadConvertor IssueComment method
+func (s slackConvertor) IssueComment(p *api.IssueCommentPayload) (SlackPayload, error) {
 	text, issueTitle, color := getIssueCommentPayloadInfo(p, SlackLinkFormatter, true)
 
 	return s.createPayload(text, []SlackAttachment{{
@@ -158,28 +147,28 @@ func (s *SlackPayload) IssueComment(p *api.IssueCommentPayload) (api.Payloader,
 	}}), nil
 }
 
-// Wiki implements PayloadConvertor Wiki method
-func (s *SlackPayload) Wiki(p *api.WikiPayload) (api.Payloader, error) {
+// Wiki implements payloadConvertor Wiki method
+func (s slackConvertor) Wiki(p *api.WikiPayload) (SlackPayload, error) {
 	text, _, _ := getWikiPayloadInfo(p, SlackLinkFormatter, true)
 
 	return s.createPayload(text, nil), nil
 }
 
-// Release implements PayloadConvertor Release method
-func (s *SlackPayload) Release(p *api.ReleasePayload) (api.Payloader, error) {
+// Release implements payloadConvertor Release method
+func (s slackConvertor) Release(p *api.ReleasePayload) (SlackPayload, error) {
 	text, _ := getReleasePayloadInfo(p, SlackLinkFormatter, true)
 
 	return s.createPayload(text, nil), nil
 }
 
-func (s *SlackPayload) Package(p *api.PackagePayload) (api.Payloader, error) {
+func (s slackConvertor) Package(p *api.PackagePayload) (SlackPayload, error) {
 	text, _ := getPackagePayloadInfo(p, SlackLinkFormatter, true)
 
 	return s.createPayload(text, nil), nil
 }
 
-// Push implements PayloadConvertor Push method
-func (s *SlackPayload) Push(p *api.PushPayload) (api.Payloader, error) {
+// Push implements payloadConvertor Push method
+func (s slackConvertor) Push(p *api.PushPayload) (SlackPayload, error) {
 	// n new commits
 	var (
 		commitDesc   string
@@ -219,8 +208,8 @@ func (s *SlackPayload) Push(p *api.PushPayload) (api.Payloader, error) {
 	}}), nil
 }
 
-// PullRequest implements PayloadConvertor PullRequest method
-func (s *SlackPayload) PullRequest(p *api.PullRequestPayload) (api.Payloader, error) {
+// PullRequest implements payloadConvertor PullRequest method
+func (s slackConvertor) PullRequest(p *api.PullRequestPayload) (SlackPayload, error) {
 	text, issueTitle, attachmentText, color := getPullRequestPayloadInfo(p, SlackLinkFormatter, true)
 
 	var attachments []SlackAttachment
@@ -238,8 +227,8 @@ func (s *SlackPayload) PullRequest(p *api.PullRequestPayload) (api.Payloader, er
 	return s.createPayload(text, attachments), nil
 }
 
-// Review implements PayloadConvertor Review method
-func (s *SlackPayload) Review(p *api.PullRequestPayload, event webhook_module.HookEventType) (api.Payloader, error) {
+// Review implements payloadConvertor Review method
+func (s slackConvertor) Review(p *api.PullRequestPayload, event webhook_module.HookEventType) (SlackPayload, error) {
 	senderLink := SlackLinkFormatter(setting.AppURL+p.Sender.UserName, p.Sender.UserName)
 	title := fmt.Sprintf("#%d %s", p.Index, p.PullRequest.Title)
 	titleLink := fmt.Sprintf("%s/pulls/%d", p.Repository.HTMLURL, p.Index)
@@ -250,7 +239,7 @@ func (s *SlackPayload) Review(p *api.PullRequestPayload, event webhook_module.Ho
 	case api.HookIssueReviewed:
 		action, err := parseHookPullRequestEventType(event)
 		if err != nil {
-			return nil, err
+			return SlackPayload{}, err
 		}
 
 		text = fmt.Sprintf("[%s] Pull request review %s: [%s](%s) by %s", repoLink, action, title, titleLink, senderLink)
@@ -259,8 +248,8 @@ func (s *SlackPayload) Review(p *api.PullRequestPayload, event webhook_module.Ho
 	return s.createPayload(text, nil), nil
 }
 
-// Repository implements PayloadConvertor Repository method
-func (s *SlackPayload) Repository(p *api.RepositoryPayload) (api.Payloader, error) {
+// Repository implements payloadConvertor Repository method
+func (s slackConvertor) Repository(p *api.RepositoryPayload) (SlackPayload, error) {
 	senderLink := SlackLinkFormatter(setting.AppURL+p.Sender.UserName, p.Sender.UserName)
 	repoLink := SlackLinkFormatter(p.Repository.HTMLURL, p.Repository.FullName)
 	var text string
@@ -275,8 +264,8 @@ func (s *SlackPayload) Repository(p *api.RepositoryPayload) (api.Payloader, erro
 	return s.createPayload(text, nil), nil
 }
 
-func (s *SlackPayload) createPayload(text string, attachments []SlackAttachment) *SlackPayload {
-	return &SlackPayload{
+func (s slackConvertor) createPayload(text string, attachments []SlackAttachment) SlackPayload {
+	return SlackPayload{
 		Channel:     s.Channel,
 		Text:        text,
 		Username:    s.Username,
@@ -285,21 +274,27 @@ func (s *SlackPayload) createPayload(text string, attachments []SlackAttachment)
 	}
 }
 
-// GetSlackPayload converts a slack webhook into a SlackPayload
-func GetSlackPayload(p api.Payloader, event webhook_module.HookEventType, meta string) (api.Payloader, error) {
-	s := new(SlackPayload)
+type slackConvertor struct {
+	Channel  string
+	Username string
+	IconURL  string
+	Color    string
+}
 
-	slack := &SlackMeta{}
-	if err := json.Unmarshal([]byte(meta), &slack); err != nil {
-		return s, errors.New("GetSlackPayload meta json:" + err.Error())
+var _ payloadConvertor[SlackPayload] = slackConvertor{}
+
+func newSlackRequest(ctx context.Context, w *webhook_model.Webhook, t *webhook_model.HookTask) (*http.Request, []byte, error) {
+	meta := &SlackMeta{}
+	if err := json.Unmarshal([]byte(w.Meta), meta); err != nil {
+		return nil, nil, fmt.Errorf("newSlackRequest meta json: %w", err)
 	}
-
-	s.Channel = slack.Channel
-	s.Username = slack.Username
-	s.IconURL = slack.IconURL
-	s.Color = slack.Color
-
-	return convertPayloader(s, p, event)
+	sc := slackConvertor{
+		Channel:  meta.Channel,
+		Username: meta.Username,
+		IconURL:  meta.IconURL,
+		Color:    meta.Color,
+	}
+	return newJSONRequest(sc, w, t, true)
 }
 
 var slackChannel = regexp.MustCompile(`^#?[a-z0-9_-]{1,80}$`)

services/webhook/slack_test.go

@@ -4,8 +4,11 @@
 package webhook
 
 import (
+	"context"
 	"testing"
 
+	webhook_model "code.gitea.io/gitea/models/webhook"
+	"code.gitea.io/gitea/modules/json"
 	api "code.gitea.io/gitea/modules/structs"
 	webhook_module "code.gitea.io/gitea/modules/webhook"
 
@@ -14,201 +17,180 @@ import (
 )
 
 func TestSlackPayload(t *testing.T) {
+	sc := slackConvertor{}
+
 	t.Run("Create", func(t *testing.T) {
 		p := createTestPayload()
 
-		d := new(SlackPayload)
-		pl, err := d.Create(p)
+		pl, err := sc.Create(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &SlackPayload{}, pl)
 
-		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>:<http://localhost:3000/test/repo/src/branch/test|test>] branch created by user1", pl.(*SlackPayload).Text)
+		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>:<http://localhost:3000/test/repo/src/branch/test|test>] branch created by user1", pl.Text)
 	})
 
 	t.Run("Delete", func(t *testing.T) {
 		p := deleteTestPayload()
 
-		d := new(SlackPayload)
-		pl, err := d.Delete(p)
+		pl, err := sc.Delete(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &SlackPayload{}, pl)
 
-		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>:test] branch deleted by user1", pl.(*SlackPayload).Text)
+		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>:test] branch deleted by user1", pl.Text)
 	})
 
 	t.Run("Fork", func(t *testing.T) {
 		p := forkTestPayload()
 
-		d := new(SlackPayload)
-		pl, err := d.Fork(p)
+		pl, err := sc.Fork(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &SlackPayload{}, pl)
 
-		assert.Equal(t, "<http://localhost:3000/test/repo2|test/repo2> is forked to <http://localhost:3000/test/repo|test/repo>", pl.(*SlackPayload).Text)
+		assert.Equal(t, "<http://localhost:3000/test/repo2|test/repo2> is forked to <http://localhost:3000/test/repo|test/repo>", pl.Text)
 	})
 
 	t.Run("Push", func(t *testing.T) {
 		p := pushTestPayload()
 
-		d := new(SlackPayload)
-		pl, err := d.Push(p)
+		pl, err := sc.Push(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &SlackPayload{}, pl)
 
-		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>:<http://localhost:3000/test/repo/src/branch/test|test>] 2 new commits pushed by user1", pl.(*SlackPayload).Text)
+		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>:<http://localhost:3000/test/repo/src/branch/test|test>] 2 new commits pushed by user1", pl.Text)
 	})
 
 	t.Run("Issue", func(t *testing.T) {
 		p := issueTestPayload()
 
-		d := new(SlackPayload)
 		p.Action = api.HookIssueOpened
-		pl, err := d.Issue(p)
+		pl, err := sc.Issue(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &SlackPayload{}, pl)
 
-		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] Issue opened: <http://localhost:3000/test/repo/issues/2|#2 crash> by <https://try.gitea.io/user1|user1>", pl.(*SlackPayload).Text)
+		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] Issue opened: <http://localhost:3000/test/repo/issues/2|#2 crash> by <https://try.gitea.io/user1|user1>", pl.Text)
 
 		p.Action = api.HookIssueClosed
-		pl, err = d.Issue(p)
+		pl, err = sc.Issue(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &SlackPayload{}, pl)
 
-		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] Issue closed: <http://localhost:3000/test/repo/issues/2|#2 crash> by <https://try.gitea.io/user1|user1>", pl.(*SlackPayload).Text)
+		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] Issue closed: <http://localhost:3000/test/repo/issues/2|#2 crash> by <https://try.gitea.io/user1|user1>", pl.Text)
 	})
 
 	t.Run("IssueComment", func(t *testing.T) {
 		p := issueCommentTestPayload()
 
-		d := new(SlackPayload)
-		pl, err := d.IssueComment(p)
+		pl, err := sc.IssueComment(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &SlackPayload{}, pl)
 
-		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] New comment on issue <http://localhost:3000/test/repo/issues/2|#2 crash> by <https://try.gitea.io/user1|user1>", pl.(*SlackPayload).Text)
+		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] New comment on issue <http://localhost:3000/test/repo/issues/2|#2 crash> by <https://try.gitea.io/user1|user1>", pl.Text)
 	})
 
 	t.Run("PullRequest", func(t *testing.T) {
 		p := pullRequestTestPayload()
 
-		d := new(SlackPayload)
-		pl, err := d.PullRequest(p)
+		pl, err := sc.PullRequest(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &SlackPayload{}, pl)
 
-		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] Pull request opened: <http://localhost:3000/test/repo/pulls/12|#12 Fix bug> by <https://try.gitea.io/user1|user1>", pl.(*SlackPayload).Text)
+		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] Pull request opened: <http://localhost:3000/test/repo/pulls/12|#12 Fix bug> by <https://try.gitea.io/user1|user1>", pl.Text)
 	})
 
 	t.Run("PullRequestComment", func(t *testing.T) {
 		p := pullRequestCommentTestPayload()
 
-		d := new(SlackPayload)
-		pl, err := d.IssueComment(p)
+		pl, err := sc.IssueComment(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &SlackPayload{}, pl)
 
-		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] New comment on pull request <http://localhost:3000/test/repo/pulls/12|#12 Fix bug> by <https://try.gitea.io/user1|user1>", pl.(*SlackPayload).Text)
+		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] New comment on pull request <http://localhost:3000/test/repo/pulls/12|#12 Fix bug> by <https://try.gitea.io/user1|user1>", pl.Text)
 	})
 
 	t.Run("Review", func(t *testing.T) {
 		p := pullRequestTestPayload()
 		p.Action = api.HookIssueReviewed
 
-		d := new(SlackPayload)
-		pl, err := d.Review(p, webhook_module.HookEventPullRequestReviewApproved)
+		pl, err := sc.Review(p, webhook_module.HookEventPullRequestReviewApproved)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &SlackPayload{}, pl)
 
-		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] Pull request review approved: [#12 Fix bug](http://localhost:3000/test/repo/pulls/12) by <https://try.gitea.io/user1|user1>", pl.(*SlackPayload).Text)
+		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] Pull request review approved: [#12 Fix bug](http://localhost:3000/test/repo/pulls/12) by <https://try.gitea.io/user1|user1>", pl.Text)
 	})
 
 	t.Run("Repository", func(t *testing.T) {
 		p := repositoryTestPayload()
 
-		d := new(SlackPayload)
-		pl, err := d.Repository(p)
+		pl, err := sc.Repository(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &SlackPayload{}, pl)
 
-		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] Repository created by <https://try.gitea.io/user1|user1>", pl.(*SlackPayload).Text)
+		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] Repository created by <https://try.gitea.io/user1|user1>", pl.Text)
 	})
 
 	t.Run("Package", func(t *testing.T) {
 		p := packageTestPayload()
 
-		d := new(SlackPayload)
-		pl, err := d.Package(p)
+		pl, err := sc.Package(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &SlackPayload{}, pl)
 
-		assert.Equal(t, "Package created: <http://localhost:3000/user1/-/packages/container/GiteaContainer/latest|GiteaContainer:latest> by <https://try.gitea.io/user1|user1>", pl.(*SlackPayload).Text)
+		assert.Equal(t, "Package created: <http://localhost:3000/user1/-/packages/container/GiteaContainer/latest|GiteaContainer:latest> by <https://try.gitea.io/user1|user1>", pl.Text)
 	})
 
 	t.Run("Wiki", func(t *testing.T) {
 		p := wikiTestPayload()
 
-		d := new(SlackPayload)
 		p.Action = api.HookWikiCreated
-		pl, err := d.Wiki(p)
+		pl, err := sc.Wiki(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &SlackPayload{}, pl)
 
-		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] New wiki page '<http://localhost:3000/test/repo/wiki/index|index>' (Wiki change comment) by <https://try.gitea.io/user1|user1>", pl.(*SlackPayload).Text)
+		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] New wiki page '<http://localhost:3000/test/repo/wiki/index|index>' (Wiki change comment) by <https://try.gitea.io/user1|user1>", pl.Text)
 
 		p.Action = api.HookWikiEdited
-		pl, err = d.Wiki(p)
+		pl, err = sc.Wiki(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &SlackPayload{}, pl)
 
-		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] Wiki page '<http://localhost:3000/test/repo/wiki/index|index>' edited (Wiki change comment) by <https://try.gitea.io/user1|user1>", pl.(*SlackPayload).Text)
+		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] Wiki page '<http://localhost:3000/test/repo/wiki/index|index>' edited (Wiki change comment) by <https://try.gitea.io/user1|user1>", pl.Text)
 
 		p.Action = api.HookWikiDeleted
-		pl, err = d.Wiki(p)
+		pl, err = sc.Wiki(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &SlackPayload{}, pl)
 
-		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] Wiki page '<http://localhost:3000/test/repo/wiki/index|index>' deleted by <https://try.gitea.io/user1|user1>", pl.(*SlackPayload).Text)
+		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] Wiki page '<http://localhost:3000/test/repo/wiki/index|index>' deleted by <https://try.gitea.io/user1|user1>", pl.Text)
 	})
 
 	t.Run("Release", func(t *testing.T) {
 		p := pullReleaseTestPayload()
 
-		d := new(SlackPayload)
-		pl, err := d.Release(p)
+		pl, err := sc.Release(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &SlackPayload{}, pl)
 
-		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] Release created: <http://localhost:3000/test/repo/releases/tag/v1.0|v1.0> by <https://try.gitea.io/user1|user1>", pl.(*SlackPayload).Text)
+		assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] Release created: <http://localhost:3000/test/repo/releases/tag/v1.0|v1.0> by <https://try.gitea.io/user1|user1>", pl.Text)
 	})
 }
 
 func TestSlackJSONPayload(t *testing.T) {
 	p := pushTestPayload()
-
-	pl, err := new(SlackPayload).Push(p)
+	data, err := p.JSONPayload()
 	require.NoError(t, err)
-	require.NotNil(t, pl)
-	require.IsType(t, &SlackPayload{}, pl)
 
-	json, err := pl.JSONPayload()
+	hook := &webhook_model.Webhook{
+		RepoID:     3,
+		IsActive:   true,
+		Type:       webhook_module.SLACK,
+		URL:        "https://slack.example.com/",
+		Meta:       `{}`,
+		HTTPMethod: "POST",
+	}
+	task := &webhook_model.HookTask{
+		HookID:         hook.ID,
+		EventType:      webhook_module.HookEventPush,
+		PayloadContent: string(data),
+		PayloadVersion: 2,
+	}
+
+	req, reqBody, err := newSlackRequest(context.Background(), hook, task)
+	require.NotNil(t, req)
+	require.NotNil(t, reqBody)
 	require.NoError(t, err)
-	assert.NotEmpty(t, json)
+
+	assert.Equal(t, "POST", req.Method)
+	assert.Equal(t, "https://slack.example.com/", req.URL.String())
+	assert.Equal(t, "sha256=", req.Header.Get("X-Hub-Signature-256"))
+	assert.Equal(t, "application/json", req.Header.Get("Content-Type"))
+	var body SlackPayload
+	err = json.NewDecoder(req.Body).Decode(&body)
+	assert.NoError(t, err)
+	assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>:<http://localhost:3000/test/repo/src/branch/test|test>] 2 new commits pushed by user1", body.Text)
 }
 
 func TestIsValidSlackChannel(t *testing.T) {

services/webhook/telegram.go

@@ -4,14 +4,15 @@
 package webhook
 
 import (
+	"context"
 	"fmt"
+	"net/http"
 	"strings"
 
 	webhook_model "code.gitea.io/gitea/models/webhook"
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/json"
 	"code.gitea.io/gitea/modules/log"
-	"code.gitea.io/gitea/modules/markup"
 	api "code.gitea.io/gitea/modules/structs"
 	webhook_module "code.gitea.io/gitea/modules/webhook"
 )
@@ -41,22 +42,8 @@ func GetTelegramHook(w *webhook_model.Webhook) *TelegramMeta {
 	return s
 }
 
-var _ PayloadConvertor = &TelegramPayload{}
-
-// JSONPayload Marshals the TelegramPayload to json
-func (t *TelegramPayload) JSONPayload() ([]byte, error) {
-	t.ParseMode = "HTML"
-	t.DisableWebPreview = true
-	t.Message = markup.Sanitize(t.Message)
-	data, err := json.MarshalIndent(t, "", "  ")
-	if err != nil {
-		return []byte{}, err
-	}
-	return data, nil
-}
-
 // Create implements PayloadConvertor Create method
-func (t *TelegramPayload) Create(p *api.CreatePayload) (api.Payloader, error) {
+func (t telegramConvertor) Create(p *api.CreatePayload) (TelegramPayload, error) {
 	// created tag/branch
 	refName := git.RefName(p.Ref).ShortName()
 	title := fmt.Sprintf(`[<a href="%s">%s</a>] %s <a href="%s">%s</a> created`, p.Repo.HTMLURL, p.Repo.FullName, p.RefType,
@@ -66,7 +53,7 @@ func (t *TelegramPayload) Create(p *api.CreatePayload) (api.Payloader, error) {
 }
 
 // Delete implements PayloadConvertor Delete method
-func (t *TelegramPayload) Delete(p *api.DeletePayload) (api.Payloader, error) {
+func (t telegramConvertor) Delete(p *api.DeletePayload) (TelegramPayload, error) {
 	// created tag/branch
 	refName := git.RefName(p.Ref).ShortName()
 	title := fmt.Sprintf(`[<a href="%s">%s</a>] %s <a href="%s">%s</a> deleted`, p.Repo.HTMLURL, p.Repo.FullName, p.RefType,
@@ -76,14 +63,14 @@ func (t *TelegramPayload) Delete(p *api.DeletePayload) (api.Payloader, error) {
 }
 
 // Fork implements PayloadConvertor Fork method
-func (t *TelegramPayload) Fork(p *api.ForkPayload) (api.Payloader, error) {
+func (t telegramConvertor) Fork(p *api.ForkPayload) (TelegramPayload, error) {
 	title := fmt.Sprintf(`%s is forked to <a href="%s">%s</a>`, p.Forkee.FullName, p.Repo.HTMLURL, p.Repo.FullName)
 
 	return createTelegramPayload(title), nil
 }
 
 // Push implements PayloadConvertor Push method
-func (t *TelegramPayload) Push(p *api.PushPayload) (api.Payloader, error) {
+func (t telegramConvertor) Push(p *api.PushPayload) (TelegramPayload, error) {
 	var (
 		branchName = git.RefName(p.Ref).ShortName()
 		commitDesc string
@@ -121,34 +108,34 @@ func (t *TelegramPayload) Push(p *api.PushPayload) (api.Payloader, error) {
 }
 
 // Issue implements PayloadConvertor Issue method
-func (t *TelegramPayload) Issue(p *api.IssuePayload) (api.Payloader, error) {
+func (t telegramConvertor) Issue(p *api.IssuePayload) (TelegramPayload, error) {
 	text, _, attachmentText, _ := getIssuesPayloadInfo(p, htmlLinkFormatter, true)
 
 	return createTelegramPayload(text + "\n\n" + attachmentText), nil
 }
 
 // IssueComment implements PayloadConvertor IssueComment method
-func (t *TelegramPayload) IssueComment(p *api.IssueCommentPayload) (api.Payloader, error) {
+func (t telegramConvertor) IssueComment(p *api.IssueCommentPayload) (TelegramPayload, error) {
 	text, _, _ := getIssueCommentPayloadInfo(p, htmlLinkFormatter, true)
 
 	return createTelegramPayload(text + "\n" + p.Comment.Body), nil
 }
 
 // PullRequest implements PayloadConvertor PullRequest method
-func (t *TelegramPayload) PullRequest(p *api.PullRequestPayload) (api.Payloader, error) {
+func (t telegramConvertor) PullRequest(p *api.PullRequestPayload) (TelegramPayload, error) {
 	text, _, attachmentText, _ := getPullRequestPayloadInfo(p, htmlLinkFormatter, true)
 
 	return createTelegramPayload(text + "\n" + attachmentText), nil
 }
 
 // Review implements PayloadConvertor Review method
-func (t *TelegramPayload) Review(p *api.PullRequestPayload, event webhook_module.HookEventType) (api.Payloader, error) {
+func (t telegramConvertor) Review(p *api.PullRequestPayload, event webhook_module.HookEventType) (TelegramPayload, error) {
 	var text, attachmentText string
 	switch p.Action {
 	case api.HookIssueReviewed:
 		action, err := parseHookPullRequestEventType(event)
 		if err != nil {
-			return nil, err
+			return TelegramPayload{}, err
 		}
 
 		text = fmt.Sprintf("[%s] Pull request review %s: #%d %s", p.Repository.FullName, action, p.Index, p.PullRequest.Title)
@@ -159,7 +146,7 @@ func (t *TelegramPayload) Review(p *api.PullRequestPayload, event webhook_module
 }
 
 // Repository implements PayloadConvertor Repository method
-func (t *TelegramPayload) Repository(p *api.RepositoryPayload) (api.Payloader, error) {
+func (t telegramConvertor) Repository(p *api.RepositoryPayload) (TelegramPayload, error) {
 	var title string
 	switch p.Action {
 	case api.HookRepoCreated:
@@ -169,36 +156,39 @@ func (t *TelegramPayload) Repository(p *api.RepositoryPayload) (api.Payloader, e
 		title = fmt.Sprintf("[%s] Repository deleted", p.Repository.FullName)
 		return createTelegramPayload(title), nil
 	}
-	return nil, nil
+	return TelegramPayload{}, nil
 }
 
 // Wiki implements PayloadConvertor Wiki method
-func (t *TelegramPayload) Wiki(p *api.WikiPayload) (api.Payloader, error) {
+func (t telegramConvertor) Wiki(p *api.WikiPayload) (TelegramPayload, error) {
 	text, _, _ := getWikiPayloadInfo(p, htmlLinkFormatter, true)
 
 	return createTelegramPayload(text), nil
 }
 
 // Release implements PayloadConvertor Release method
-func (t *TelegramPayload) Release(p *api.ReleasePayload) (api.Payloader, error) {
+func (t telegramConvertor) Release(p *api.ReleasePayload) (TelegramPayload, error) {
 	text, _ := getReleasePayloadInfo(p, htmlLinkFormatter, true)
 
 	return createTelegramPayload(text), nil
 }
 
-func (t *TelegramPayload) Package(p *api.PackagePayload) (api.Payloader, error) {
+func (t telegramConvertor) Package(p *api.PackagePayload) (TelegramPayload, error) {
 	text, _ := getPackagePayloadInfo(p, htmlLinkFormatter, true)
 
 	return createTelegramPayload(text), nil
 }
 
-// GetTelegramPayload converts a telegram webhook into a TelegramPayload
-func GetTelegramPayload(p api.Payloader, event webhook_module.HookEventType, _ string) (api.Payloader, error) {
-	return convertPayloader(new(TelegramPayload), p, event)
-}
-
-func createTelegramPayload(message string) *TelegramPayload {
-	return &TelegramPayload{
+func createTelegramPayload(message string) TelegramPayload {
+	return TelegramPayload{
 		Message: strings.TrimSpace(message),
 	}
 }
+
+type telegramConvertor struct{}
+
+var _ payloadConvertor[TelegramPayload] = telegramConvertor{}
+
+func newTelegramRequest(ctx context.Context, w *webhook_model.Webhook, t *webhook_model.HookTask) (*http.Request, []byte, error) {
+	return newJSONRequest(telegramConvertor{}, w, t, true)
+}

services/webhook/telegram_test.go

@@ -4,8 +4,11 @@
 package webhook
 
 import (
+	"context"
 	"testing"
 
+	webhook_model "code.gitea.io/gitea/models/webhook"
+	"code.gitea.io/gitea/modules/json"
 	api "code.gitea.io/gitea/modules/structs"
 	webhook_module "code.gitea.io/gitea/modules/webhook"
 
@@ -14,199 +17,177 @@ import (
 )
 
 func TestTelegramPayload(t *testing.T) {
+	tc := telegramConvertor{}
 	t.Run("Create", func(t *testing.T) {
 		p := createTestPayload()
 
-		d := new(TelegramPayload)
-		pl, err := d.Create(p)
+		pl, err := tc.Create(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &TelegramPayload{}, pl)
 
-		assert.Equal(t, `[<a href="http://localhost:3000/test/repo">test/repo</a>] branch <a href="http://localhost:3000/test/repo/src/test">test</a> created`, pl.(*TelegramPayload).Message)
+		assert.Equal(t, `[<a href="http://localhost:3000/test/repo">test/repo</a>] branch <a href="http://localhost:3000/test/repo/src/test">test</a> created`, pl.Message)
 	})
 
 	t.Run("Delete", func(t *testing.T) {
 		p := deleteTestPayload()
 
-		d := new(TelegramPayload)
-		pl, err := d.Delete(p)
+		pl, err := tc.Delete(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &TelegramPayload{}, pl)
 
-		assert.Equal(t, `[<a href="http://localhost:3000/test/repo">test/repo</a>] branch <a href="http://localhost:3000/test/repo/src/test">test</a> deleted`, pl.(*TelegramPayload).Message)
+		assert.Equal(t, `[<a href="http://localhost:3000/test/repo">test/repo</a>] branch <a href="http://localhost:3000/test/repo/src/test">test</a> deleted`, pl.Message)
 	})
 
 	t.Run("Fork", func(t *testing.T) {
 		p := forkTestPayload()
 
-		d := new(TelegramPayload)
-		pl, err := d.Fork(p)
+		pl, err := tc.Fork(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &TelegramPayload{}, pl)
 
-		assert.Equal(t, `test/repo2 is forked to <a href="http://localhost:3000/test/repo">test/repo</a>`, pl.(*TelegramPayload).Message)
+		assert.Equal(t, `test/repo2 is forked to <a href="http://localhost:3000/test/repo">test/repo</a>`, pl.Message)
 	})
 
 	t.Run("Push", func(t *testing.T) {
 		p := pushTestPayload()
 
-		d := new(TelegramPayload)
-		pl, err := d.Push(p)
+		pl, err := tc.Push(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &TelegramPayload{}, pl)
 
-		assert.Equal(t, "[<a href=\"http://localhost:3000/test/repo\">test/repo</a>:<a href=\"http://localhost:3000/test/repo/src/test\">test</a>] 2 new commits\n[<a href=\"http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778\">2020558</a>] commit message - user1\n[<a href=\"http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778\">2020558</a>] commit message - user1", pl.(*TelegramPayload).Message)
+		assert.Equal(t, "[<a href=\"http://localhost:3000/test/repo\">test/repo</a>:<a href=\"http://localhost:3000/test/repo/src/test\">test</a>] 2 new commits\n[<a href=\"http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778\">2020558</a>] commit message - user1\n[<a href=\"http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778\">2020558</a>] commit message - user1", pl.Message)
 	})
 
 	t.Run("Issue", func(t *testing.T) {
 		p := issueTestPayload()
 
-		d := new(TelegramPayload)
 		p.Action = api.HookIssueOpened
-		pl, err := d.Issue(p)
+		pl, err := tc.Issue(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &TelegramPayload{}, pl)
 
-		assert.Equal(t, "[<a href=\"http://localhost:3000/test/repo\">test/repo</a>] Issue opened: <a href=\"http://localhost:3000/test/repo/issues/2\">#2 crash</a> by <a href=\"https://try.gitea.io/user1\">user1</a>\n\nissue body", pl.(*TelegramPayload).Message)
+		assert.Equal(t, "[<a href=\"http://localhost:3000/test/repo\">test/repo</a>] Issue opened: <a href=\"http://localhost:3000/test/repo/issues/2\">#2 crash</a> by <a href=\"https://try.gitea.io/user1\">user1</a>\n\nissue body", pl.Message)
 
 		p.Action = api.HookIssueClosed
-		pl, err = d.Issue(p)
+		pl, err = tc.Issue(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &TelegramPayload{}, pl)
 
-		assert.Equal(t, `[<a href="http://localhost:3000/test/repo">test/repo</a>] Issue closed: <a href="http://localhost:3000/test/repo/issues/2">#2 crash</a> by <a href="https://try.gitea.io/user1">user1</a>`, pl.(*TelegramPayload).Message)
+		assert.Equal(t, `[<a href="http://localhost:3000/test/repo">test/repo</a>] Issue closed: <a href="http://localhost:3000/test/repo/issues/2">#2 crash</a> by <a href="https://try.gitea.io/user1">user1</a>`, pl.Message)
 	})
 
 	t.Run("IssueComment", func(t *testing.T) {
 		p := issueCommentTestPayload()
 
-		d := new(TelegramPayload)
-		pl, err := d.IssueComment(p)
+		pl, err := tc.IssueComment(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &TelegramPayload{}, pl)
 
-		assert.Equal(t, "[<a href=\"http://localhost:3000/test/repo\">test/repo</a>] New comment on issue <a href=\"http://localhost:3000/test/repo/issues/2\">#2 crash</a> by <a href=\"https://try.gitea.io/user1\">user1</a>\nmore info needed", pl.(*TelegramPayload).Message)
+		assert.Equal(t, "[<a href=\"http://localhost:3000/test/repo\">test/repo</a>] New comment on issue <a href=\"http://localhost:3000/test/repo/issues/2\">#2 crash</a> by <a href=\"https://try.gitea.io/user1\">user1</a>\nmore info needed", pl.Message)
 	})
 
 	t.Run("PullRequest", func(t *testing.T) {
 		p := pullRequestTestPayload()
 
-		d := new(TelegramPayload)
-		pl, err := d.PullRequest(p)
+		pl, err := tc.PullRequest(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &TelegramPayload{}, pl)
 
-		assert.Equal(t, "[<a href=\"http://localhost:3000/test/repo\">test/repo</a>] Pull request opened: <a href=\"http://localhost:3000/test/repo/pulls/12\">#12 Fix bug</a> by <a href=\"https://try.gitea.io/user1\">user1</a>\nfixes bug #2", pl.(*TelegramPayload).Message)
+		assert.Equal(t, "[<a href=\"http://localhost:3000/test/repo\">test/repo</a>] Pull request opened: <a href=\"http://localhost:3000/test/repo/pulls/12\">#12 Fix bug</a> by <a href=\"https://try.gitea.io/user1\">user1</a>\nfixes bug #2", pl.Message)
 	})
 
 	t.Run("PullRequestComment", func(t *testing.T) {
 		p := pullRequestCommentTestPayload()
 
-		d := new(TelegramPayload)
-		pl, err := d.IssueComment(p)
+		pl, err := tc.IssueComment(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &TelegramPayload{}, pl)
 
-		assert.Equal(t, "[<a href=\"http://localhost:3000/test/repo\">test/repo</a>] New comment on pull request <a href=\"http://localhost:3000/test/repo/pulls/12\">#12 Fix bug</a> by <a href=\"https://try.gitea.io/user1\">user1</a>\nchanges requested", pl.(*TelegramPayload).Message)
+		assert.Equal(t, "[<a href=\"http://localhost:3000/test/repo\">test/repo</a>] New comment on pull request <a href=\"http://localhost:3000/test/repo/pulls/12\">#12 Fix bug</a> by <a href=\"https://try.gitea.io/user1\">user1</a>\nchanges requested", pl.Message)
 	})
 
 	t.Run("Review", func(t *testing.T) {
 		p := pullRequestTestPayload()
 		p.Action = api.HookIssueReviewed
 
-		d := new(TelegramPayload)
-		pl, err := d.Review(p, webhook_module.HookEventPullRequestReviewApproved)
+		pl, err := tc.Review(p, webhook_module.HookEventPullRequestReviewApproved)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &TelegramPayload{}, pl)
 
-		assert.Equal(t, "[test/repo] Pull request review approved: #12 Fix bug\ngood job", pl.(*TelegramPayload).Message)
+		assert.Equal(t, "[test/repo] Pull request review approved: #12 Fix bug\ngood job", pl.Message)
 	})
 
 	t.Run("Repository", func(t *testing.T) {
 		p := repositoryTestPayload()
 
-		d := new(TelegramPayload)
-		pl, err := d.Repository(p)
+		pl, err := tc.Repository(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &TelegramPayload{}, pl)
 
-		assert.Equal(t, `[<a href="http://localhost:3000/test/repo">test/repo</a>] Repository created`, pl.(*TelegramPayload).Message)
+		assert.Equal(t, `[<a href="http://localhost:3000/test/repo">test/repo</a>] Repository created`, pl.Message)
 	})
 
 	t.Run("Package", func(t *testing.T) {
 		p := packageTestPayload()
 
-		d := new(TelegramPayload)
-		pl, err := d.Package(p)
+		pl, err := tc.Package(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &TelegramPayload{}, pl)
 
-		assert.Equal(t, `Package created: <a href="http://localhost:3000/user1/-/packages/container/GiteaContainer/latest">GiteaContainer:latest</a> by <a href="https://try.gitea.io/user1">user1</a>`, pl.(*TelegramPayload).Message)
+		assert.Equal(t, `Package created: <a href="http://localhost:3000/user1/-/packages/container/GiteaContainer/latest">GiteaContainer:latest</a> by <a href="https://try.gitea.io/user1">user1</a>`, pl.Message)
 	})
 
 	t.Run("Wiki", func(t *testing.T) {
 		p := wikiTestPayload()
 
-		d := new(TelegramPayload)
 		p.Action = api.HookWikiCreated
-		pl, err := d.Wiki(p)
+		pl, err := tc.Wiki(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &TelegramPayload{}, pl)
 
-		assert.Equal(t, `[<a href="http://localhost:3000/test/repo">test/repo</a>] New wiki page '<a href="http://localhost:3000/test/repo/wiki/index">index</a>' (Wiki change comment) by <a href="https://try.gitea.io/user1">user1</a>`, pl.(*TelegramPayload).Message)
+		assert.Equal(t, `[<a href="http://localhost:3000/test/repo">test/repo</a>] New wiki page '<a href="http://localhost:3000/test/repo/wiki/index">index</a>' (Wiki change comment) by <a href="https://try.gitea.io/user1">user1</a>`, pl.Message)
 
 		p.Action = api.HookWikiEdited
-		pl, err = d.Wiki(p)
+		pl, err = tc.Wiki(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &TelegramPayload{}, pl)
 
-		assert.Equal(t, `[<a href="http://localhost:3000/test/repo">test/repo</a>] Wiki page '<a href="http://localhost:3000/test/repo/wiki/index">index</a>' edited (Wiki change comment) by <a href="https://try.gitea.io/user1">user1</a>`, pl.(*TelegramPayload).Message)
+		assert.Equal(t, `[<a href="http://localhost:3000/test/repo">test/repo</a>] Wiki page '<a href="http://localhost:3000/test/repo/wiki/index">index</a>' edited (Wiki change comment) by <a href="https://try.gitea.io/user1">user1</a>`, pl.Message)
 
 		p.Action = api.HookWikiDeleted
-		pl, err = d.Wiki(p)
+		pl, err = tc.Wiki(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &TelegramPayload{}, pl)
 
-		assert.Equal(t, `[<a href="http://localhost:3000/test/repo">test/repo</a>] Wiki page '<a href="http://localhost:3000/test/repo/wiki/index">index</a>' deleted by <a href="https://try.gitea.io/user1">user1</a>`, pl.(*TelegramPayload).Message)
+		assert.Equal(t, `[<a href="http://localhost:3000/test/repo">test/repo</a>] Wiki page '<a href="http://localhost:3000/test/repo/wiki/index">index</a>' deleted by <a href="https://try.gitea.io/user1">user1</a>`, pl.Message)
 	})
 
 	t.Run("Release", func(t *testing.T) {
 		p := pullReleaseTestPayload()
 
-		d := new(TelegramPayload)
-		pl, err := d.Release(p)
+		pl, err := tc.Release(p)
 		require.NoError(t, err)
-		require.NotNil(t, pl)
-		require.IsType(t, &TelegramPayload{}, pl)
 
-		assert.Equal(t, `[<a href="http://localhost:3000/test/repo">test/repo</a>] Release created: <a href="http://localhost:3000/test/repo/releases/tag/v1.0">v1.0</a> by <a href="https://try.gitea.io/user1">user1</a>`, pl.(*TelegramPayload).Message)
+		assert.Equal(t, `[<a href="http://localhost:3000/test/repo">test/repo</a>] Release created: <a href="http://localhost:3000/test/repo/releases/tag/v1.0">v1.0</a> by <a href="https://try.gitea.io/user1">user1</a>`, pl.Message)
 	})
 }
 
 func TestTelegramJSONPayload(t *testing.T) {
 	p := pushTestPayload()
-
-	pl, err := new(TelegramPayload).Push(p)
+	data, err := p.JSONPayload()
 	require.NoError(t, err)
-	require.NotNil(t, pl)
-	require.IsType(t, &TelegramPayload{}, pl)
 
-	json, err := pl.JSONPayload()
+	hook := &webhook_model.Webhook{
+		RepoID:     3,
+		IsActive:   true,
+		Type:       webhook_module.TELEGRAM,
+		URL:        "https://telegram.example.com/",
+		Meta:       ``,
+		HTTPMethod: "POST",
+	}
+	task := &webhook_model.HookTask{
+		HookID:         hook.ID,
+		EventType:      webhook_module.HookEventPush,
+		PayloadContent: string(data),
+		PayloadVersion: 2,
+	}
+
+	req, reqBody, err := newTelegramRequest(context.Background(), hook, task)
+	require.NotNil(t, req)
+	require.NotNil(t, reqBody)
 	require.NoError(t, err)
-	assert.NotEmpty(t, json)
+
+	assert.Equal(t, "POST", req.Method)
+	assert.Equal(t, "https://telegram.example.com/", req.URL.String())
+	assert.Equal(t, "sha256=", req.Header.Get("X-Hub-Signature-256"))
+	assert.Equal(t, "application/json", req.Header.Get("Content-Type"))
+	var body TelegramPayload
+	err = json.NewDecoder(req.Body).Decode(&body)
+	assert.NoError(t, err)
+	assert.Equal(t, "[<a href=\"http://localhost:3000/test/repo\">test/repo</a>:<a href=\"http://localhost:3000/test/repo/src/test\">test</a>] 2 new commits\n[<a href=\"http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778\">2020558</a>] commit message - user1\n[<a href=\"http://localhost:3000/test/repo/commit/2020558fe2e34debb818a514715839cabd25e778\">2020558</a>] commit message - user1", body.Message)
 }

services/webhook/webhook.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net/http"
 	"strings"
 
 	"code.gitea.io/gitea/models/db"
@@ -26,48 +27,16 @@ import (
 	"github.com/gobwas/glob"
 )
 
-type webhook struct {
-	name           webhook_module.HookType
-	payloadCreator func(p api.Payloader, event webhook_module.HookEventType, meta string) (api.Payloader, error)
-}
-
-var webhooks = map[webhook_module.HookType]*webhook{
-	webhook_module.SLACK: {
-		name:           webhook_module.SLACK,
-		payloadCreator: GetSlackPayload,
-	},
-	webhook_module.DISCORD: {
-		name:           webhook_module.DISCORD,
-		payloadCreator: GetDiscordPayload,
-	},
-	webhook_module.DINGTALK: {
-		name:           webhook_module.DINGTALK,
-		payloadCreator: GetDingtalkPayload,
-	},
-	webhook_module.TELEGRAM: {
-		name:           webhook_module.TELEGRAM,
-		payloadCreator: GetTelegramPayload,
-	},
-	webhook_module.MSTEAMS: {
-		name:           webhook_module.MSTEAMS,
-		payloadCreator: GetMSTeamsPayload,
-	},
-	webhook_module.FEISHU: {
-		name:           webhook_module.FEISHU,
-		payloadCreator: GetFeishuPayload,
-	},
-	webhook_module.MATRIX: {
-		name:           webhook_module.MATRIX,
-		payloadCreator: GetMatrixPayload,
-	},
-	webhook_module.WECHATWORK: {
-		name:           webhook_module.WECHATWORK,
-		payloadCreator: GetWechatworkPayload,
-	},
-	webhook_module.PACKAGIST: {
-		name:           webhook_module.PACKAGIST,
-		payloadCreator: GetPackagistPayload,
-	},
+var webhookRequesters = map[webhook_module.HookType]func(context.Context, *webhook_model.Webhook, *webhook_model.HookTask) (req *http.Request, body []byte, err error){
+	webhook_module.SLACK:      newSlackRequest,
+	webhook_module.DISCORD:    newDiscordRequest,
+	webhook_module.DINGTALK:   newDingtalkRequest,
+	webhook_module.TELEGRAM:   newTelegramRequest,
+	webhook_module.MSTEAMS:    newMSTeamsRequest,
+	webhook_module.FEISHU:     newFeishuRequest,
+	webhook_module.MATRIX:     newMatrixRequest,
+	webhook_module.WECHATWORK: newWechatworkRequest,
+	webhook_module.PACKAGIST:  newPackagistRequest,
 }
 
 // IsValidHookTaskType returns true if a webhook registered
@@ -75,7 +44,7 @@ func IsValidHookTaskType(name string) bool {
 	if name == webhook_module.GITEA || name == webhook_module.GOGS {
 		return true
 	}
-	_, ok := webhooks[name]
+	_, ok := webhookRequesters[name]
 	return ok
 }
 
@@ -159,7 +128,9 @@ func checkBranch(w *webhook_model.Webhook, branch string) bool {
 	return g.Match(branch)
 }
 
-// PrepareWebhook creates a hook task and enqueues it for processing
+// PrepareWebhook creates a hook task and enqueues it for processing.
+// The payload is saved as-is. The adjustments depending on the webhook type happen
+// right before delivery, in the [Deliver] method.
 func PrepareWebhook(ctx context.Context, w *webhook_model.Webhook, event webhook_module.HookEventType, p api.Payloader) error {
 	// Skip sending if webhooks are disabled.
 	if setting.DisableWebhooks {
@@ -193,25 +164,19 @@ func PrepareWebhook(ctx context.Context, w *webhook_model.Webhook, event webhook
 		}
 	}
 
-	var payloader api.Payloader
-	var err error
-	webhook, ok := webhooks[w.Type]
-	if ok {
-		payloader, err = webhook.payloadCreator(p, event, w.Meta)
-		if err != nil {
-			return fmt.Errorf("create payload for %s[%s]: %w", w.Type, event, err)
-		}
-	} else {
-		payloader = p
+	payload, err := p.JSONPayload()
+	if err != nil {
+		return fmt.Errorf("JSONPayload for %s: %w", event, err)
 	}
 
 	task, err := webhook_model.CreateHookTask(ctx, &webhook_model.HookTask{
-		HookID:    w.ID,
-		Payloader: payloader,
-		EventType: event,
+		HookID:         w.ID,
+		PayloadContent: string(payload),
+		EventType:      event,
+		PayloadVersion: 2,
 	})
 	if err != nil {
-		return fmt.Errorf("CreateHookTask: %w", err)
+		return fmt.Errorf("CreateHookTask for %s: %w", event, err)
 	}
 
 	return enqueueHookTask(task.ID)

services/webhook/webhook_test.go

@@ -77,7 +77,3 @@ func TestPrepareWebhooksBranchFilterNoMatch(t *testing.T) {
 		unittest.AssertNotExistsBean(t, hookTask)
 	}
 }
-
-// TODO TestHookTask_deliver
-
-// TODO TestDeliverHooks

services/webhook/wechatwork.go

@@ -4,11 +4,13 @@
 package webhook
 
 import (
+	"context"
 	"fmt"
+	"net/http"
 	"strings"
 
+	webhook_model "code.gitea.io/gitea/models/webhook"
 	"code.gitea.io/gitea/modules/git"
-	"code.gitea.io/gitea/modules/json"
 	api "code.gitea.io/gitea/modules/structs"
 	webhook_module "code.gitea.io/gitea/modules/webhook"
 )
@@ -28,20 +30,8 @@ type (
 	}
 )
 
-// SetSecret sets the Wechatwork secret
-func (f *WechatworkPayload) SetSecret(_ string) {}
-
-// JSONPayload Marshals the WechatworkPayload to json
-func (f *WechatworkPayload) JSONPayload() ([]byte, error) {
-	data, err := json.MarshalIndent(f, "", "  ")
-	if err != nil {
-		return []byte{}, err
-	}
-	return data, nil
-}
-
-func newWechatworkMarkdownPayload(title string) *WechatworkPayload {
-	return &WechatworkPayload{
+func newWechatworkMarkdownPayload(title string) WechatworkPayload {
+	return WechatworkPayload{
 		Msgtype: "markdown",
 		Markdown: struct {
 			Content string `json:"content"`
@@ -51,10 +41,8 @@ func newWechatworkMarkdownPayload(title string) *WechatworkPayload {
 	}
 }
 
-var _ PayloadConvertor = &WechatworkPayload{}
-
 // Create implements PayloadConvertor Create method
-func (f *WechatworkPayload) Create(p *api.CreatePayload) (api.Payloader, error) {
+func (wc wechatworkConvertor) Create(p *api.CreatePayload) (WechatworkPayload, error) {
 	// created tag/branch
 	refName := git.RefName(p.Ref).ShortName()
 	title := fmt.Sprintf("[%s] %s %s created", p.Repo.FullName, p.RefType, refName)
@@ -63,7 +51,7 @@ func (f *WechatworkPayload) Create(p *api.CreatePayload) (api.Payloader, error)
 }
 
 // Delete implements PayloadConvertor Delete method
-func (f *WechatworkPayload) Delete(p *api.DeletePayload) (api.Payloader, error) {
+func (wc wechatworkConvertor) Delete(p *api.DeletePayload) (WechatworkPayload, error) {
 	// created tag/branch
 	refName := git.RefName(p.Ref).ShortName()
 	title := fmt.Sprintf("[%s] %s %s deleted", p.Repo.FullName, p.RefType, refName)
@@ -72,14 +60,14 @@ func (f *WechatworkPayload) Delete(p *api.DeletePayload) (api.Payloader, error)
 }
 
 // Fork implements PayloadConvertor Fork method
-func (f *WechatworkPayload) Fork(p *api.ForkPayload) (api.Payloader, error) {
+func (wc wechatworkConvertor) Fork(p *api.ForkPayload) (WechatworkPayload, error) {
 	title := fmt.Sprintf("%s is forked to %s", p.Forkee.FullName, p.Repo.FullName)
 
 	return newWechatworkMarkdownPayload(title), nil
 }
 
 // Push implements PayloadConvertor Push method
-func (f *WechatworkPayload) Push(p *api.PushPayload) (api.Payloader, error) {
+func (wc wechatworkConvertor) Push(p *api.PushPayload) (WechatworkPayload, error) {
 	var (
 		branchName = git.RefName(p.Ref).ShortName()
 		commitDesc string
@@ -108,7 +96,7 @@ func (f *WechatworkPayload) Push(p *api.PushPayload) (api.Payloader, error) {
 }
 
 // Issue implements PayloadConvertor Issue method
-func (f *WechatworkPayload) Issue(p *api.IssuePayload) (api.Payloader, error) {
+func (wc wechatworkConvertor) Issue(p *api.IssuePayload) (WechatworkPayload, error) {
 	text, issueTitle, attachmentText, _ := getIssuesPayloadInfo(p, noneLinkFormatter, true)
 	var content string
 	content += fmt.Sprintf(" ><font color=\"info\">%s</font>\n >%s \n ><font color=\"warning\"> %s</font> \n [%s](%s)", text, attachmentText, issueTitle, p.Issue.HTMLURL, p.Issue.HTMLURL)
@@ -117,7 +105,7 @@ func (f *WechatworkPayload) Issue(p *api.IssuePayload) (api.Payloader, error) {
 }
 
 // IssueComment implements PayloadConvertor IssueComment method
-func (f *WechatworkPayload) IssueComment(p *api.IssueCommentPayload) (api.Payloader, error) {
+func (wc wechatworkConvertor) IssueComment(p *api.IssueCommentPayload) (WechatworkPayload, error) {
 	text, issueTitle, _ := getIssueCommentPayloadInfo(p, noneLinkFormatter, true)
 	var content string
 	content += fmt.Sprintf(" ><font color=\"info\">%s</font>\n >%s \n ><font color=\"warning\">%s</font> \n [%s](%s)", text, p.Comment.Body, issueTitle, p.Comment.HTMLURL, p.Comment.HTMLURL)
@@ -126,7 +114,7 @@ func (f *WechatworkPayload) IssueComment(p *api.IssueCommentPayload) (api.Payloa
 }
 
 // PullRequest implements PayloadConvertor PullRequest method
-func (f *WechatworkPayload) PullRequest(p *api.PullRequestPayload) (api.Payloader, error) {
+func (wc wechatworkConvertor) PullRequest(p *api.PullRequestPayload) (WechatworkPayload, error) {
 	text, issueTitle, attachmentText, _ := getPullRequestPayloadInfo(p, noneLinkFormatter, true)
 	pr := fmt.Sprintf("> <font color=\"info\"> %s </font> \r\n > <font color=\"comment\">%s </font> \r\n > <font color=\"comment\">%s </font> \r\n",
 		text, issueTitle, attachmentText)
@@ -135,13 +123,13 @@ func (f *WechatworkPayload) PullRequest(p *api.PullRequestPayload) (api.Payloade
 }
 
 // Review implements PayloadConvertor Review method
-func (f *WechatworkPayload) Review(p *api.PullRequestPayload, event webhook_module.HookEventType) (api.Payloader, error) {
+func (wc wechatworkConvertor) Review(p *api.PullRequestPayload, event webhook_module.HookEventType) (WechatworkPayload, error) {
 	var text, title string
 	switch p.Action {
 	case api.HookIssueReviewed:
 		action, err := parseHookPullRequestEventType(event)
 		if err != nil {
-			return nil, err
+			return WechatworkPayload{}, err
 		}
 		title = fmt.Sprintf("[%s] Pull request review %s : #%d %s", p.Repository.FullName, action, p.Index, p.PullRequest.Title)
 		text = p.Review.Content
@@ -151,7 +139,7 @@ func (f *WechatworkPayload) Review(p *api.PullRequestPayload, event webhook_modu
 }
 
 // Repository implements PayloadConvertor Repository method
-func (f *WechatworkPayload) Repository(p *api.RepositoryPayload) (api.Payloader, error) {
+func (wc wechatworkConvertor) Repository(p *api.RepositoryPayload) (WechatworkPayload, error) {
 	var title string
 	switch p.Action {
 	case api.HookRepoCreated:
@@ -162,30 +150,33 @@ func (f *WechatworkPayload) Repository(p *api.RepositoryPayload) (api.Payloader,
 		return newWechatworkMarkdownPayload(title), nil
 	}
 
-	return nil, nil
+	return WechatworkPayload{}, nil
 }
 
 // Wiki implements PayloadConvertor Wiki method
-func (f *WechatworkPayload) Wiki(p *api.WikiPayload) (api.Payloader, error) {
+func (wc wechatworkConvertor) Wiki(p *api.WikiPayload) (WechatworkPayload, error) {
 	text, _, _ := getWikiPayloadInfo(p, noneLinkFormatter, true)
 
 	return newWechatworkMarkdownPayload(text), nil
 }
 
 // Release implements PayloadConvertor Release method
-func (f *WechatworkPayload) Release(p *api.ReleasePayload) (api.Payloader, error) {
+func (wc wechatworkConvertor) Release(p *api.ReleasePayload) (WechatworkPayload, error) {
 	text, _ := getReleasePayloadInfo(p, noneLinkFormatter, true)
 
 	return newWechatworkMarkdownPayload(text), nil
 }
 
-func (f *WechatworkPayload) Package(p *api.PackagePayload) (api.Payloader, error) {
+func (wc wechatworkConvertor) Package(p *api.PackagePayload) (WechatworkPayload, error) {
 	text, _ := getPackagePayloadInfo(p, noneLinkFormatter, true)
 
 	return newWechatworkMarkdownPayload(text), nil
 }
 
-// GetWechatworkPayload GetWechatworkPayload converts a ding talk webhook into a WechatworkPayload
-func GetWechatworkPayload(p api.Payloader, event webhook_module.HookEventType, _ string) (api.Payloader, error) {
-	return convertPayloader(new(WechatworkPayload), p, event)
+type wechatworkConvertor struct{}
+
+var _ payloadConvertor[WechatworkPayload] = wechatworkConvertor{}
+
+func newWechatworkRequest(ctx context.Context, w *webhook_model.Webhook, t *webhook_model.HookTask) (*http.Request, []byte, error) {
+	return newJSONRequest(wechatworkConvertor{}, w, t, true)
 }

templates/repo/settings/webhook/history.tmpl

@@ -19,6 +19,8 @@
 						<div class="flex-text-inline">
 							{{if .IsSucceed}}
 								<span class="text green">{{svg "octicon-check"}}</span>
+							{{else if not .IsDelivered}}
+								<span class="text orange">{{svg "octicon-stopwatch"}}</span>
 							{{else}}
 								<span class="text red">{{svg "octicon-alert"}}</span>
 							{{end}}
@@ -62,7 +64,7 @@
 {{range $key, $val := .RequestInfo.Headers}}<strong>{{$key}}:</strong> {{$val}}
 {{end}}</pre>
 								<h5>{{ctx.Locale.Tr "repo.settings.webhook.payload"}}</h5>
-								<pre class="webhook-info"><code class="json">{{.PayloadContent}}</code></pre>
+								<pre class="webhook-info"><code class="json">{{or .RequestInfo.Body .PayloadContent}}</code></pre>
 							{{else}}
 								-
 							{{end}}