Skip to content

Commit c0d2a38

Browse files
zjjhotzjjhot
committed
Reapply "Store webhook event in database (go-gitea#29145)"
This reverts commit ec30295.
1 parent 73df3ab commit c0d2a38

28 files changed

+1713
-1545
lines changed

‎models/fixtures/hook_task.yml

+32
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,35 @@
33
hook_id: 1
44
uuid: uuid1
55
is_delivered: true
6+
is_succeed: false
7+
request_content: >
8+
{
9+
"url": "/matrix-delivered",
10+
"http_method":"PUT",
11+
"headers": {
12+
"X-Head": "42"
13+
},
14+
"body": "{}"
15+
}
16+
17+
-
18+
id: 2
19+
hook_id: 1
20+
uuid: uuid2
21+
is_delivered: false
22+
23+
-
24+
id: 3
25+
hook_id: 1
26+
uuid: uuid3
27+
is_delivered: true
28+
is_succeed: true
29+
payload_content: '{"key":"value"}' # legacy task, payload saved in payload_content (and not in request_content)
30+
request_content: >
31+
{
32+
"url": "/matrix-success",
33+
"http_method":"PUT",
34+
"headers": {
35+
"X-Head": "42"
36+
}
37+
}

‎models/migrations/migrations.go

+2
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,8 @@ var migrations = []Migration{
564564
NewMigration("Add user_blocking table", v1_22.AddUserBlockingTable),
565565
// v289 -> v290
566566
NewMigration("Add default_wiki_branch to repository table", v1_22.AddDefaultWikiBranch),
567+
// v290 -> v291
568+
NewMigration("Add PayloadVersion to HookTask", v1_22.AddPayloadVersionToHookTaskTable),
567569
}
568570

569571
// GetCurrentDBVersion returns the current db version

‎models/migrations/v1_22/v290.go

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright 2024 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package v1_22 //nolint
5+
6+
import (
7+
"xorm.io/xorm"
8+
)
9+
10+
type HookTask struct {
11+
PayloadVersion int `xorm:"DEFAULT 1"`
12+
}
13+
14+
func AddPayloadVersionToHookTaskTable(x *xorm.Engine) error {
15+
// create missing column
16+
return x.Sync(new(HookTask))
17+
}

‎models/webhook/hooktask.go

+14-12
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@ package webhook
55

66
import (
77
"context"
8+
"errors"
89
"time"
910

1011
"code.gitea.io/gitea/models/db"
1112
"code.gitea.io/gitea/modules/json"
1213
"code.gitea.io/gitea/modules/log"
1314
"code.gitea.io/gitea/modules/setting"
14-
api "code.gitea.io/gitea/modules/structs"
1515
"code.gitea.io/gitea/modules/timeutil"
1616
webhook_module "code.gitea.io/gitea/modules/webhook"
1717

@@ -31,6 +31,7 @@ type HookRequest struct {
3131
URL string `json:"url"`
3232
HTTPMethod string `json:"http_method"`
3333
Headers map[string]string `json:"headers"`
34+
Body string `json:"body"`
3435
}
3536

3637
// HookResponse represents hook task response information.
@@ -45,11 +46,15 @@ type HookTask struct {
4546
ID int64 `xorm:"pk autoincr"`
4647
HookID int64 `xorm:"index"`
4748
UUID string `xorm:"unique"`
48-
api.Payloader `xorm:"-"`
4949
PayloadContent string `xorm:"LONGTEXT"`
50-
EventType webhook_module.HookEventType
51-
IsDelivered bool
52-
Delivered timeutil.TimeStampNano
50+
// PayloadVersion number to allow for smooth version upgrades:
51+
// - PayloadVersion 1: PayloadContent contains the JSON as sent to the URL
52+
// - PayloadVersion 2: PayloadContent contains the original event
53+
PayloadVersion int `xorm:"DEFAULT 1"`
54+
55+
EventType webhook_module.HookEventType
56+
IsDelivered bool
57+
Delivered timeutil.TimeStampNano
5358

5459
// History info.
5560
IsSucceed bool
@@ -115,16 +120,12 @@ func HookTasks(ctx context.Context, hookID int64, page int) ([]*HookTask, error)
115120
// it handles conversion from Payload to PayloadContent.
116121
func CreateHookTask(ctx context.Context, t *HookTask) (*HookTask, error) {
117122
t.UUID = gouuid.New().String()
118-
if t.Payloader != nil {
119-
data, err := t.Payloader.JSONPayload()
120-
if err != nil {
121-
return nil, err
122-
}
123-
t.PayloadContent = string(data)
124-
}
125123
if t.Delivered == 0 {
126124
t.Delivered = timeutil.TimeStampNanoNow()
127125
}
126+
if t.PayloadVersion == 0 {
127+
return nil, errors.New("missing HookTask.PayloadVersion")
128+
}
128129
return t, db.Insert(ctx, t)
129130
}
130131

@@ -165,6 +166,7 @@ func ReplayHookTask(ctx context.Context, hookID int64, uuid string) (*HookTask,
165166
HookID: task.HookID,
166167
PayloadContent: task.PayloadContent,
167168
EventType: task.EventType,
169+
PayloadVersion: task.PayloadVersion,
168170
})
169171
}
170172

‎models/webhook/webhook_test.go

+32-29
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"code.gitea.io/gitea/models/unittest"
1313
"code.gitea.io/gitea/modules/json"
1414
"code.gitea.io/gitea/modules/optional"
15-
api "code.gitea.io/gitea/modules/structs"
1615
"code.gitea.io/gitea/modules/timeutil"
1716
webhook_module "code.gitea.io/gitea/modules/webhook"
1817

@@ -35,8 +34,10 @@ func TestWebhook_History(t *testing.T) {
3534
webhook := unittest.AssertExistsAndLoadBean(t, &Webhook{ID: 1})
3635
tasks, err := webhook.History(db.DefaultContext, 0)
3736
assert.NoError(t, err)
38-
if assert.Len(t, tasks, 1) {
39-
assert.Equal(t, int64(1), tasks[0].ID)
37+
if assert.Len(t, tasks, 3) {
38+
assert.Equal(t, int64(3), tasks[0].ID)
39+
assert.Equal(t, int64(2), tasks[1].ID)
40+
assert.Equal(t, int64(1), tasks[2].ID)
4041
}
4142

4243
webhook = unittest.AssertExistsAndLoadBean(t, &Webhook{ID: 2})
@@ -197,8 +198,10 @@ func TestHookTasks(t *testing.T) {
197198
assert.NoError(t, unittest.PrepareTestDatabase())
198199
hookTasks, err := HookTasks(db.DefaultContext, 1, 1)
199200
assert.NoError(t, err)
200-
if assert.Len(t, hookTasks, 1) {
201-
assert.Equal(t, int64(1), hookTasks[0].ID)
201+
if assert.Len(t, hookTasks, 3) {
202+
assert.Equal(t, int64(3), hookTasks[0].ID)
203+
assert.Equal(t, int64(2), hookTasks[1].ID)
204+
assert.Equal(t, int64(1), hookTasks[2].ID)
202205
}
203206

204207
hookTasks, err = HookTasks(db.DefaultContext, unittest.NonexistentID, 1)
@@ -209,8 +212,8 @@ func TestHookTasks(t *testing.T) {
209212
func TestCreateHookTask(t *testing.T) {
210213
assert.NoError(t, unittest.PrepareTestDatabase())
211214
hookTask := &HookTask{
212-
HookID: 3,
213-
Payloader: &api.PushPayload{},
215+
HookID: 3,
216+
PayloadVersion: 2,
214217
}
215218
unittest.AssertNotExistsBean(t, hookTask)
216219
_, err := CreateHookTask(db.DefaultContext, hookTask)
@@ -232,10 +235,10 @@ func TestUpdateHookTask(t *testing.T) {
232235
func TestCleanupHookTaskTable_PerWebhook_DeletesDelivered(t *testing.T) {
233236
assert.NoError(t, unittest.PrepareTestDatabase())
234237
hookTask := &HookTask{
235-
HookID: 3,
236-
Payloader: &api.PushPayload{},
237-
IsDelivered: true,
238-
Delivered: timeutil.TimeStampNanoNow(),
238+
HookID: 3,
239+
IsDelivered: true,
240+
Delivered: timeutil.TimeStampNanoNow(),
241+
PayloadVersion: 2,
239242
}
240243
unittest.AssertNotExistsBean(t, hookTask)
241244
_, err := CreateHookTask(db.DefaultContext, hookTask)
@@ -249,9 +252,9 @@ func TestCleanupHookTaskTable_PerWebhook_DeletesDelivered(t *testing.T) {
249252
func TestCleanupHookTaskTable_PerWebhook_LeavesUndelivered(t *testing.T) {
250253
assert.NoError(t, unittest.PrepareTestDatabase())
251254
hookTask := &HookTask{
252-
HookID: 4,
253-
Payloader: &api.PushPayload{},
254-
IsDelivered: false,
255+
HookID: 4,
256+
IsDelivered: false,
257+
PayloadVersion: 2,
255258
}
256259
unittest.AssertNotExistsBean(t, hookTask)
257260
_, err := CreateHookTask(db.DefaultContext, hookTask)
@@ -265,10 +268,10 @@ func TestCleanupHookTaskTable_PerWebhook_LeavesUndelivered(t *testing.T) {
265268
func TestCleanupHookTaskTable_PerWebhook_LeavesMostRecentTask(t *testing.T) {
266269
assert.NoError(t, unittest.PrepareTestDatabase())
267270
hookTask := &HookTask{
268-
HookID: 4,
269-
Payloader: &api.PushPayload{},
270-
IsDelivered: true,
271-
Delivered: timeutil.TimeStampNanoNow(),
271+
HookID: 4,
272+
IsDelivered: true,
273+
Delivered: timeutil.TimeStampNanoNow(),
274+
PayloadVersion: 2,
272275
}
273276
unittest.AssertNotExistsBean(t, hookTask)
274277
_, err := CreateHookTask(db.DefaultContext, hookTask)
@@ -282,10 +285,10 @@ func TestCleanupHookTaskTable_PerWebhook_LeavesMostRecentTask(t *testing.T) {
282285
func TestCleanupHookTaskTable_OlderThan_DeletesDelivered(t *testing.T) {
283286
assert.NoError(t, unittest.PrepareTestDatabase())
284287
hookTask := &HookTask{
285-
HookID: 3,
286-
Payloader: &api.PushPayload{},
287-
IsDelivered: true,
288-
Delivered: timeutil.TimeStampNano(time.Now().AddDate(0, 0, -8).UnixNano()),
288+
HookID: 3,
289+
IsDelivered: true,
290+
Delivered: timeutil.TimeStampNano(time.Now().AddDate(0, 0, -8).UnixNano()),
291+
PayloadVersion: 2,
289292
}
290293
unittest.AssertNotExistsBean(t, hookTask)
291294
_, err := CreateHookTask(db.DefaultContext, hookTask)
@@ -299,9 +302,9 @@ func TestCleanupHookTaskTable_OlderThan_DeletesDelivered(t *testing.T) {
299302
func TestCleanupHookTaskTable_OlderThan_LeavesUndelivered(t *testing.T) {
300303
assert.NoError(t, unittest.PrepareTestDatabase())
301304
hookTask := &HookTask{
302-
HookID: 4,
303-
Payloader: &api.PushPayload{},
304-
IsDelivered: false,
305+
HookID: 4,
306+
IsDelivered: false,
307+
PayloadVersion: 2,
305308
}
306309
unittest.AssertNotExistsBean(t, hookTask)
307310
_, err := CreateHookTask(db.DefaultContext, hookTask)
@@ -315,10 +318,10 @@ func TestCleanupHookTaskTable_OlderThan_LeavesUndelivered(t *testing.T) {
315318
func TestCleanupHookTaskTable_OlderThan_LeavesTaskEarlierThanAgeToDelete(t *testing.T) {
316319
assert.NoError(t, unittest.PrepareTestDatabase())
317320
hookTask := &HookTask{
318-
HookID: 4,
319-
Payloader: &api.PushPayload{},
320-
IsDelivered: true,
321-
Delivered: timeutil.TimeStampNano(time.Now().AddDate(0, 0, -6).UnixNano()),
321+
HookID: 4,
322+
IsDelivered: true,
323+
Delivered: timeutil.TimeStampNano(time.Now().AddDate(0, 0, -6).UnixNano()),
324+
PayloadVersion: 2,
322325
}
323326
unittest.AssertNotExistsBean(t, hookTask)
324327
_, err := CreateHookTask(db.DefaultContext, hookTask)

0 commit comments

Comments
 (0)