forgejo/models/webhook/webhook_test.go

414 lines
14 KiB
Go
Raw Normal View History

// Copyright 2017 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package webhook
import (
"testing"
"time"
"forgejo.org/models/db"
"forgejo.org/models/unittest"
"forgejo.org/modules/json"
"forgejo.org/modules/optional"
"forgejo.org/modules/timeutil"
webhook_module "forgejo.org/modules/webhook"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestHookContentType_Name(t *testing.T) {
assert.Equal(t, "json", ContentTypeJSON.Name())
assert.Equal(t, "form", ContentTypeForm.Name())
}
func TestIsValidHookContentType(t *testing.T) {
assert.True(t, IsValidHookContentType("json"))
assert.True(t, IsValidHookContentType("form"))
assert.False(t, IsValidHookContentType("invalid"))
}
func TestWebhook_History(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
webhook := unittest.AssertExistsAndLoadBean(t, &Webhook{ID: 1})
tasks, err := webhook.History(db.DefaultContext, 0)
require.NoError(t, err)
Store webhook event in database (#29145) Refactor the webhook logic, to have the type-dependent processing happen only in one place. --- 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. - 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) (cherry picked from commit 26653b196bd1d15c532af41f60351596dd4330bd) Conflicts: services/webhook/deliver_test.go trivial context conflict
2024-03-07 23:18:38 +01:00
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})
tasks, err = webhook.History(db.DefaultContext, 0)
require.NoError(t, err)
assert.Empty(t, tasks)
}
func TestWebhook_UpdateEvent(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
webhook := unittest.AssertExistsAndLoadBean(t, &Webhook{ID: 1})
hookEvent := &webhook_module.HookEvent{
PushOnly: true,
SendEverything: false,
ChooseEvents: false,
HookEvents: webhook_module.HookEvents{
Create: false,
Push: true,
PullRequest: false,
},
}
webhook.HookEvent = hookEvent
require.NoError(t, webhook.UpdateEvent())
assert.NotEmpty(t, webhook.Events)
actualHookEvent := &webhook_module.HookEvent{}
require.NoError(t, json.Unmarshal([]byte(webhook.Events), actualHookEvent))
assert.Equal(t, *hookEvent, *actualHookEvent)
}
func TestWebhook_EventsArray(t *testing.T) {
assert.Equal(t, []string{
"create", "delete", "fork", "push",
"issues", "issue_assign", "issue_label", "issue_milestone", "issue_comment",
"pull_request", "pull_request_assign", "pull_request_label", "pull_request_milestone",
"pull_request_comment", "pull_request_review_approved", "pull_request_review_rejected",
Webhook for Wiki changes (#20219) Add support for triggering webhook notifications on wiki changes. This PR contains frontend and backend for webhook notifications on wiki actions (create a new page, rename a page, edit a page and delete a page). The frontend got a new checkbox under the Custom Event -> Repository Events section. There is only one checkbox for create/edit/rename/delete actions, because it makes no sense to separate it and others like releases or packages follow the same schema. ![image](https://user-images.githubusercontent.com/121972/177018803-26851196-831f-4fde-9a4c-9e639b0e0d6b.png) The actions itself are separated, so that different notifications will be executed (with the "action" field). All the webhook receivers implement the new interface method (Wiki) and the corresponding tests. When implementing this, I encounter a little bug on editing a wiki page. Creating and editing a wiki page is technically the same action and will be handled by the ```updateWikiPage``` function. But the function need to know if it is a new wiki page or just a change. This distinction is done by the ```action``` parameter, but this will not be sent by the frontend (on form submit). This PR will fix this by adding the ```action``` parameter with the values ```_new``` or ```_edit```, which will be used by the ```updateWikiPage``` function. I've done integration tests with matrix and gitea (http). ![image](https://user-images.githubusercontent.com/121972/177018795-eb5cdc01-9ba3-483e-a6b7-ed0e313a71fb.png) Fix #16457 Signed-off-by: Aaron Fischer <mail@aaron-fischer.net>
2022-09-04 21:54:23 +02:00
"pull_request_review_comment", "pull_request_sync", "wiki", "repository", "release",
Actions Failure, Succes, Recover Webhooks (#7508) Implement Actions Success, Failure and Recover webhooks for Forgejo, Gitea, Gogs, Slack, Discord, DingTalk, Telegram, Microsoft Teams, Feishu / Lark Suite, Matrix, WeCom (Wechat Work), Packagist. Some of these webhooks have not been manually tested. Implement settings for these new webhooks. ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [x] in their respective `*_test.go` for unit tests. - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [ ] I do not want this change to show in the release notes. - [x] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title. <!--start release-notes-assistant--> ## Release notes <!--URL:https://codeberg.org/forgejo/forgejo--> - Features - [PR](https://codeberg.org/forgejo/forgejo/pulls/7508): <!--number 7508 --><!--line 0 --><!--description QWN0aW9ucyBGYWlsdXJlLCBTdWNjZXMsIFJlY292ZXIgV2ViaG9va3M=-->Actions Failure, Succes, Recover Webhooks<!--description--> <!--end release-notes-assistant--> Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7508 Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org> Co-authored-by: christopher-besch <mail@chris-besch.com> Co-committed-by: christopher-besch <mail@chris-besch.com>
2025-06-03 14:29:19 +02:00
"package", "pull_request_review_request", "action_run_failure",
"action_run_recover", "action_run_success",
},
(&Webhook{
HookEvent: &webhook_module.HookEvent{SendEverything: true},
}).EventsArray(),
)
assert.Equal(t, []string{"push"},
(&Webhook{
HookEvent: &webhook_module.HookEvent{PushOnly: true},
}).EventsArray(),
)
}
func TestCreateWebhook(t *testing.T) {
Actions Failure, Succes, Recover Webhooks (#7508) Implement Actions Success, Failure and Recover webhooks for Forgejo, Gitea, Gogs, Slack, Discord, DingTalk, Telegram, Microsoft Teams, Feishu / Lark Suite, Matrix, WeCom (Wechat Work), Packagist. Some of these webhooks have not been manually tested. Implement settings for these new webhooks. ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [x] in their respective `*_test.go` for unit tests. - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [ ] I do not want this change to show in the release notes. - [x] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title. <!--start release-notes-assistant--> ## Release notes <!--URL:https://codeberg.org/forgejo/forgejo--> - Features - [PR](https://codeberg.org/forgejo/forgejo/pulls/7508): <!--number 7508 --><!--line 0 --><!--description QWN0aW9ucyBGYWlsdXJlLCBTdWNjZXMsIFJlY292ZXIgV2ViaG9va3M=-->Actions Failure, Succes, Recover Webhooks<!--description--> <!--end release-notes-assistant--> Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7508 Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org> Co-authored-by: christopher-besch <mail@chris-besch.com> Co-committed-by: christopher-besch <mail@chris-besch.com>
2025-06-03 14:29:19 +02:00
t.Run("Some chosen events 1", func(t *testing.T) {
hook := &Webhook{
RepoID: 3,
URL: "https://www.example.com/unit_test",
ContentType: ContentTypeJSON,
Events: `{"push_only":false,"send_everything":false,"choose_events":true,"events":{"create":false,"push":true,"pull_request":true}}`,
}
unittest.AssertNotExistsBean(t, hook)
require.NoError(t, CreateWebhook(db.DefaultContext, hook))
hookFromDb := unittest.AssertExistsAndLoadBean(t, hook)
assert.Equal(t, []string{
string(webhook_module.HookEventPush),
string(webhook_module.HookEventPullRequest),
}, hookFromDb.EventsArray())
})
t.Run("Some chosen events 2", func(t *testing.T) {
hook := &Webhook{
RepoID: 3,
URL: "https://www.example.com/unit_test",
ContentType: ContentTypeJSON,
Events: `{"push_only":false,"send_everything":false,"choose_events":true,"events":{"action_run_recover":false,"action_run_success":true}}`,
}
unittest.AssertNotExistsBean(t, hook)
require.NoError(t, CreateWebhook(db.DefaultContext, hook))
hookFromDb := unittest.AssertExistsAndLoadBean(t, hook)
assert.Equal(t, []string{string(webhook_module.HookEventActionRunSuccess)}, hookFromDb.EventsArray())
})
t.Run("All events", func(t *testing.T) {
hook := &Webhook{
RepoID: 3,
URL: "https://www.example.com/unit_test",
ContentType: ContentTypeJSON,
Events: `{"push_only":false,"send_everything":false,"choose_events":true,"events":{"create":true,"delete":true,"fork":true,"issues":true,"issue_assign":true,"issue_label":true,"issue_milestone":true,"issue_comment":true,"push":true,"pull_request":true,"pull_request_assign":true,"pull_request_label":true,"pull_request_milestone":true,"pull_request_comment":true,"pull_request_review":true,"pull_request_sync":true,"pull_request_review_request":true,"wiki":true,"repository":true,"release":true,"package":true,"action_run_failure":true,"action_run_recover":true,"action_run_success":true}}`,
}
unittest.AssertNotExistsBean(t, hook)
require.NoError(t, CreateWebhook(db.DefaultContext, hook))
hookFromDb := unittest.AssertExistsAndLoadBean(t, hook)
assert.Equal(t, []string{
string(webhook_module.HookEventCreate),
string(webhook_module.HookEventDelete),
string(webhook_module.HookEventFork),
string(webhook_module.HookEventPush),
string(webhook_module.HookEventIssues),
string(webhook_module.HookEventIssueAssign),
string(webhook_module.HookEventIssueLabel),
string(webhook_module.HookEventIssueMilestone),
string(webhook_module.HookEventIssueComment),
string(webhook_module.HookEventPullRequest),
string(webhook_module.HookEventPullRequestAssign),
string(webhook_module.HookEventPullRequestLabel),
string(webhook_module.HookEventPullRequestMilestone),
string(webhook_module.HookEventPullRequestComment),
string(webhook_module.HookEventPullRequestReviewApproved),
string(webhook_module.HookEventPullRequestReviewRejected),
string(webhook_module.HookEventPullRequestReviewComment),
string(webhook_module.HookEventPullRequestSync),
string(webhook_module.HookEventWiki),
string(webhook_module.HookEventRepository),
string(webhook_module.HookEventRelease),
string(webhook_module.HookEventPackage),
string(webhook_module.HookEventPullRequestReviewRequest),
// these aren't webhook event types
// string(webhook_module.HookEventSchedule),
// string(webhook_module.HookEventWorkflowDispatch),
string(webhook_module.HookEventActionRunFailure),
string(webhook_module.HookEventActionRunRecover),
string(webhook_module.HookEventActionRunSuccess),
},
hookFromDb.EventsArray())
})
}
func TestGetWebhookByRepoID(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
hook, err := GetWebhookByRepoID(db.DefaultContext, 1, 1)
require.NoError(t, err)
assert.Equal(t, int64(1), hook.ID)
_, err = GetWebhookByRepoID(db.DefaultContext, unittest.NonexistentID, unittest.NonexistentID)
require.Error(t, err)
assert.True(t, IsErrWebhookNotExist(err))
}
func TestGetWebhookByOwnerID(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
hook, err := GetWebhookByOwnerID(db.DefaultContext, 3, 3)
require.NoError(t, err)
assert.Equal(t, int64(3), hook.ID)
_, err = GetWebhookByOwnerID(db.DefaultContext, unittest.NonexistentID, unittest.NonexistentID)
require.Error(t, err)
assert.True(t, IsErrWebhookNotExist(err))
}
func TestGetActiveWebhooksByRepoID(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
2024-03-19 11:59:48 +01:00
activateWebhook(t, 1)
hooks, err := db.Find[Webhook](db.DefaultContext, ListWebhookOptions{RepoID: 1, IsActive: optional.Some(true)})
require.NoError(t, err)
if assert.Len(t, hooks, 1) {
assert.Equal(t, int64(1), hooks[0].ID)
assert.True(t, hooks[0].IsActive)
}
}
func TestGetWebhooksByRepoID(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
hooks, err := db.Find[Webhook](db.DefaultContext, ListWebhookOptions{RepoID: 1})
require.NoError(t, err)
if assert.Len(t, hooks, 2) {
assert.Equal(t, int64(1), hooks[0].ID)
assert.Equal(t, int64(2), hooks[1].ID)
}
}
func TestGetActiveWebhooksByOwnerID(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
2024-03-19 11:59:48 +01:00
activateWebhook(t, 3)
hooks, err := db.Find[Webhook](db.DefaultContext, ListWebhookOptions{OwnerID: 3, IsActive: optional.Some(true)})
require.NoError(t, err)
if assert.Len(t, hooks, 1) {
assert.Equal(t, int64(3), hooks[0].ID)
assert.True(t, hooks[0].IsActive)
}
}
2024-03-19 11:59:48 +01:00
func activateWebhook(t *testing.T, hookID int64) {
t.Helper()
updated, err := db.GetEngine(db.DefaultContext).ID(hookID).Cols("is_active").Update(Webhook{IsActive: true})
assert.Equal(t, int64(1), updated)
require.NoError(t, err)
2024-03-19 11:59:48 +01:00
}
func TestGetWebhooksByOwnerID(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
2024-03-19 11:59:48 +01:00
activateWebhook(t, 3)
hooks, err := db.Find[Webhook](db.DefaultContext, ListWebhookOptions{OwnerID: 3})
require.NoError(t, err)
if assert.Len(t, hooks, 1) {
assert.Equal(t, int64(3), hooks[0].ID)
assert.True(t, hooks[0].IsActive)
}
}
func TestUpdateWebhook(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
hook := unittest.AssertExistsAndLoadBean(t, &Webhook{ID: 2})
hook.IsActive = true
hook.ContentType = ContentTypeForm
unittest.AssertNotExistsBean(t, hook)
require.NoError(t, UpdateWebhook(db.DefaultContext, hook))
unittest.AssertExistsAndLoadBean(t, hook)
}
func TestDeleteWebhookByRepoID(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
unittest.AssertExistsAndLoadBean(t, &Webhook{ID: 2, RepoID: 1})
require.NoError(t, DeleteWebhookByRepoID(db.DefaultContext, 1, 2))
unittest.AssertNotExistsBean(t, &Webhook{ID: 2, RepoID: 1})
err := DeleteWebhookByRepoID(db.DefaultContext, unittest.NonexistentID, unittest.NonexistentID)
require.Error(t, err)
assert.True(t, IsErrWebhookNotExist(err))
}
func TestDeleteWebhookByOwnerID(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
unittest.AssertExistsAndLoadBean(t, &Webhook{ID: 3, OwnerID: 3})
require.NoError(t, DeleteWebhookByOwnerID(db.DefaultContext, 3, 3))
unittest.AssertNotExistsBean(t, &Webhook{ID: 3, OwnerID: 3})
err := DeleteWebhookByOwnerID(db.DefaultContext, unittest.NonexistentID, unittest.NonexistentID)
require.Error(t, err)
assert.True(t, IsErrWebhookNotExist(err))
}
func TestHookTasks(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
hookTasks, err := HookTasks(db.DefaultContext, 1, 1)
require.NoError(t, err)
Store webhook event in database (#29145) Refactor the webhook logic, to have the type-dependent processing happen only in one place. --- 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. - 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) (cherry picked from commit 26653b196bd1d15c532af41f60351596dd4330bd) Conflicts: services/webhook/deliver_test.go trivial context conflict
2024-03-07 23:18:38 +01:00
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)
require.NoError(t, err)
assert.Empty(t, hookTasks)
}
func TestCreateHookTask(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
hookTask := &HookTask{
Store webhook event in database (#29145) Refactor the webhook logic, to have the type-dependent processing happen only in one place. --- 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. - 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) (cherry picked from commit 26653b196bd1d15c532af41f60351596dd4330bd) Conflicts: services/webhook/deliver_test.go trivial context conflict
2024-03-07 23:18:38 +01:00
HookID: 3,
PayloadVersion: 2,
}
unittest.AssertNotExistsBean(t, hookTask)
_, err := CreateHookTask(db.DefaultContext, hookTask)
require.NoError(t, err)
unittest.AssertExistsAndLoadBean(t, hookTask)
}
func TestUpdateHookTask(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
hook := unittest.AssertExistsAndLoadBean(t, &HookTask{ID: 1})
hook.PayloadContent = "new payload content"
hook.IsDelivered = true
unittest.AssertNotExistsBean(t, hook)
require.NoError(t, UpdateHookTask(db.DefaultContext, hook))
unittest.AssertExistsAndLoadBean(t, hook)
}
func TestCleanupHookTaskTable_PerWebhook_DeletesDelivered(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
hookTask := &HookTask{
Store webhook event in database (#29145) Refactor the webhook logic, to have the type-dependent processing happen only in one place. --- 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. - 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) (cherry picked from commit 26653b196bd1d15c532af41f60351596dd4330bd) Conflicts: services/webhook/deliver_test.go trivial context conflict
2024-03-07 23:18:38 +01:00
HookID: 3,
IsDelivered: true,
Delivered: timeutil.TimeStampNanoNow(),
PayloadVersion: 2,
}
unittest.AssertNotExistsBean(t, hookTask)
_, err := CreateHookTask(db.DefaultContext, hookTask)
require.NoError(t, err)
unittest.AssertExistsAndLoadBean(t, hookTask)
require.NoError(t, CleanupHookTaskTable(t.Context(), PerWebhook, 168*time.Hour, 0))
unittest.AssertNotExistsBean(t, hookTask)
}
func TestCleanupHookTaskTable_PerWebhook_LeavesUndelivered(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
hookTask := &HookTask{
Store webhook event in database (#29145) Refactor the webhook logic, to have the type-dependent processing happen only in one place. --- 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. - 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) (cherry picked from commit 26653b196bd1d15c532af41f60351596dd4330bd) Conflicts: services/webhook/deliver_test.go trivial context conflict
2024-03-07 23:18:38 +01:00
HookID: 4,
IsDelivered: false,
PayloadVersion: 2,
}
unittest.AssertNotExistsBean(t, hookTask)
_, err := CreateHookTask(db.DefaultContext, hookTask)
require.NoError(t, err)
unittest.AssertExistsAndLoadBean(t, hookTask)
require.NoError(t, CleanupHookTaskTable(t.Context(), PerWebhook, 168*time.Hour, 0))
unittest.AssertExistsAndLoadBean(t, hookTask)
}
func TestCleanupHookTaskTable_PerWebhook_LeavesMostRecentTask(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
hookTask := &HookTask{
Store webhook event in database (#29145) Refactor the webhook logic, to have the type-dependent processing happen only in one place. --- 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. - 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) (cherry picked from commit 26653b196bd1d15c532af41f60351596dd4330bd) Conflicts: services/webhook/deliver_test.go trivial context conflict
2024-03-07 23:18:38 +01:00
HookID: 4,
IsDelivered: true,
Delivered: timeutil.TimeStampNanoNow(),
PayloadVersion: 2,
}
unittest.AssertNotExistsBean(t, hookTask)
_, err := CreateHookTask(db.DefaultContext, hookTask)
require.NoError(t, err)
unittest.AssertExistsAndLoadBean(t, hookTask)
require.NoError(t, CleanupHookTaskTable(t.Context(), PerWebhook, 168*time.Hour, 1))
unittest.AssertExistsAndLoadBean(t, hookTask)
}
func TestCleanupHookTaskTable_OlderThan_DeletesDelivered(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
hookTask := &HookTask{
Store webhook event in database (#29145) Refactor the webhook logic, to have the type-dependent processing happen only in one place. --- 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. - 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) (cherry picked from commit 26653b196bd1d15c532af41f60351596dd4330bd) Conflicts: services/webhook/deliver_test.go trivial context conflict
2024-03-07 23:18:38 +01:00
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)
require.NoError(t, err)
unittest.AssertExistsAndLoadBean(t, hookTask)
require.NoError(t, CleanupHookTaskTable(t.Context(), OlderThan, 168*time.Hour, 0))
unittest.AssertNotExistsBean(t, hookTask)
}
func TestCleanupHookTaskTable_OlderThan_LeavesUndelivered(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
hookTask := &HookTask{
Store webhook event in database (#29145) Refactor the webhook logic, to have the type-dependent processing happen only in one place. --- 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. - 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) (cherry picked from commit 26653b196bd1d15c532af41f60351596dd4330bd) Conflicts: services/webhook/deliver_test.go trivial context conflict
2024-03-07 23:18:38 +01:00
HookID: 4,
IsDelivered: false,
PayloadVersion: 2,
}
unittest.AssertNotExistsBean(t, hookTask)
_, err := CreateHookTask(db.DefaultContext, hookTask)
require.NoError(t, err)
unittest.AssertExistsAndLoadBean(t, hookTask)
require.NoError(t, CleanupHookTaskTable(t.Context(), OlderThan, 168*time.Hour, 0))
unittest.AssertExistsAndLoadBean(t, hookTask)
}
func TestCleanupHookTaskTable_OlderThan_LeavesTaskEarlierThanAgeToDelete(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
hookTask := &HookTask{
Store webhook event in database (#29145) Refactor the webhook logic, to have the type-dependent processing happen only in one place. --- 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. - 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) (cherry picked from commit 26653b196bd1d15c532af41f60351596dd4330bd) Conflicts: services/webhook/deliver_test.go trivial context conflict
2024-03-07 23:18:38 +01:00
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)
require.NoError(t, err)
unittest.AssertExistsAndLoadBean(t, hookTask)
require.NoError(t, CleanupHookTaskTable(t.Context(), OlderThan, 168*time.Hour, 0))
unittest.AssertExistsAndLoadBean(t, hookTask)
}