kodev-org
/
vikunja
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(webhook): order matching webhooks by id for deterministic fan-out 

Previously WebhookListener.Handle fetched matching webhooks with
Find(&ws) without an explicit ORDER BY, so iteration order depended on
the DB driver. Add ORDER BY id ASC so the fan-out order is stable for
both project- and user-level webhooks, and update the sibling-blocking
regression test to insert webhooks with explicit ids so its ordering
assumption is robust to autoincrement state.
This commit is contained in:
kolaente 2026-04-09 11:10:24 +02:00 committed by kolaente
parent 85a3b3e469
commit ca7f82a5e5
2 changed files with 9 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
pkg/e2etests/webhook_multi_test.go
View File

@ -69,15 +69,18 @@ func TestWebhookFailingSiblingDoesNotBlockOthers(t *testing.T) {
defer bad.Close() defer bad.Close()
// Clean slate + insert two webhooks listening on task.updated for project 1. // Clean slate + insert two webhooks listening on task.updated for project 1.
// Insert bad FIRST so it has the lower id and would be iterated before good. // Use explicit ids so the bad webhook (id=10) is strictly ordered before
// Delete the fixture webhook id=1 (example.com target) first so it // the good one (id=11) once WebhookListener.Handle applies its ORDER BY id.
// does not pollute this test with unrelated delivery failures. // This makes the test independent of auto-increment state and DB insert
// ordering. Delete the fixture webhook id=1 (example.com target) first so
// it does not pollute this test with unrelated delivery failures.
require.NoError(t, db.LoadFixtures()) require.NoError(t, db.LoadFixtures())
s := db.NewSession() s := db.NewSession()
defer s.Close() defer s.Close()
_, err = s.Where("id = ?", 1).Delete(&models.Webhook{}) _, err = s.Where("id = ?", 1).Delete(&models.Webhook{})
require.NoError(t, err) require.NoError(t, err)
_, err = s.Insert(&models.Webhook{ _, err = s.Insert(&models.Webhook{
ID: 10,
TargetURL: bad.URL, TargetURL: bad.URL,
Events: []string{"task.updated"}, Events: []string{"task.updated"},
ProjectID: 1, ProjectID: 1,
@ -85,6 +88,7 @@ func TestWebhookFailingSiblingDoesNotBlockOthers(t *testing.T) {
}) })
require.NoError(t, err) require.NoError(t, err)
_, err = s.Insert(&models.Webhook{ _, err = s.Insert(&models.Webhook{
ID: 11,
TargetURL: good.URL, TargetURL: good.URL,
Events: []string{"task.updated"}, Events: []string{"task.updated"},
ProjectID: 1, ProjectID: 1,

2
pkg/models/listeners.go
View File

@ -1121,6 +1121,7 @@ func (wl *WebhookListener) Handle(msg *message.Message) (err error) {
ws := []*Webhook{} ws := []*Webhook{}
err = s.In("project_id", projectIDs). err = s.In("project_id", projectIDs).
OrderBy("id ASC").
Find(&ws) Find(&ws)
if err != nil { if err != nil {
return err return err
@ -1142,6 +1143,7 @@ func (wl *WebhookListener) Handle(msg *message.Message) (err error) {
if userID > 0 { if userID > 0 {
userWebhooks := []*Webhook{} userWebhooks := []*Webhook{}
err = s.Where("user_id = ? AND (project_id IS NULL OR project_id = 0)", userID). err = s.Where("user_id = ? AND (project_id IS NULL OR project_id = 0)", userID).
OrderBy("id ASC").
Find(&userWebhooks) Find(&userWebhooks)
if err != nil { if err != nil {
return err return err