Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[REFACTOR] store webhook event in database #29145

Merged
merged 10 commits into from
Mar 7, 2024
Merged

[REFACTOR] store webhook event in database #29145

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7cd2fdb
[REFACTOR] store webhook event in database
oliverpool Mar 16, 2023
1c19970
remove cron import
oliverpool Feb 12, 2024
ad5f81d
address KNA4CK3R comments
oliverpool Feb 13, 2024
dc35563
orange stopwatch
oliverpool Feb 14, 2024
1e0262c
payload version: default to 1
oliverpool Feb 27, 2024
8168998
Merge remote-tracking branch 'origin/main' into webhook_store_event
oliverpool Feb 27, 2024
1c62324
Update services/webhook/deliver.go
oliverpool Feb 27, 2024
7d1f4f6
Update services/webhook/packagist.go
oliverpool Feb 27, 2024
e2f7e73
rename migration
oliverpool Mar 7, 2024
84c9663
Merge remote-tracking branch 'origin/main' into webhook_store_event
oliverpool Mar 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions models/fixtures/hook_task.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
2 changes: 2 additions & 0 deletions models/migrations/migrations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,8 @@ var migrations = []Migration{
NewMigration("Add PreviousDuration to ActionRun", v1_22.AddPreviousDurationToActionRun),
// v286 -> v287
NewMigration("Add support for SHA256 git repositories", v1_22.AdjustDBForSha256),
// v287 -> v288
NewMigration("Add PayloadVersion to HookTask", v1_22.AddPayloadVersionToHookTaskTable),
}

// GetCurrentDBVersion returns the current db version
Expand Down
17 changes: 17 additions & 0 deletions models/migrations/v1_22/v287.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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))
}
26 changes: 14 additions & 12 deletions models/webhook/hooktask.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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,
})
}

Expand Down
61 changes: 32 additions & 29 deletions models/webhook/webhook_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/unittest"
"code.gitea.io/gitea/modules/json"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/modules/util"
webhook_module "code.gitea.io/gitea/modules/webhook"
Expand All @@ -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})
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down
Loading
Loading