From 9d2919248bd2cc8025c74aa15d0e37147e5ecf9d Mon Sep 17 00:00:00 2001 From: oliverpool Date: Fri, 5 Apr 2024 11:24:32 +0200 Subject: [PATCH 1/2] [TEST] webhook creation payload ref --- models/webhook/hooktask.go | 2 +- tests/integration/pull_merge_test.go | 1 + tests/integration/webhook_test.go | 72 ++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 tests/integration/webhook_test.go diff --git a/models/webhook/hooktask.go b/models/webhook/hooktask.go index ff3fdbadb..8734feb2e 100644 --- a/models/webhook/hooktask.go +++ b/models/webhook/hooktask.go @@ -106,7 +106,7 @@ func (t *HookTask) simpleMarshalJSON(v any) string { return string(p) } -// HookTasks returns a list of hook tasks by given conditions. +// HookTasks returns a list of hook tasks by given conditions, order by ID desc. func HookTasks(ctx context.Context, hookID int64, page int) ([]*HookTask, error) { tasks := make([]*HookTask, 0, setting.Webhook.PagingNum) return tasks, db.GetEngine(ctx). diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index e613538d0..3bd4f5741 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -83,6 +83,7 @@ func testPullCleanUp(t *testing.T, session *TestSession, user, repo, pullnum str return resp } +// returns the hook tasks, order by ID desc. func retrieveHookTasks(t *testing.T, hookID int64, activateWebhook bool) []*webhook.HookTask { t.Helper() if activateWebhook { diff --git a/tests/integration/webhook_test.go b/tests/integration/webhook_test.go new file mode 100644 index 000000000..e5ecddab3 --- /dev/null +++ b/tests/integration/webhook_test.go @@ -0,0 +1,72 @@ +// Copyright 2024 The Forgejo Authors c/o Codeberg e.V.. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "net/http" + "net/url" + "testing" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/unittest" + webhook_model "code.gitea.io/gitea/models/webhook" + "code.gitea.io/gitea/modules/json" + webhook_module "code.gitea.io/gitea/modules/webhook" + + "github.com/stretchr/testify/assert" +) + +func TestWebhookPayloadRef(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + w := unittest.AssertExistsAndLoadBean(t, &webhook_model.Webhook{ID: 1}) + w.HookEvent = &webhook_module.HookEvent{ + SendEverything: true, + } + assert.NoError(t, w.UpdateEvent()) + assert.NoError(t, webhook_model.UpdateWebhook(db.DefaultContext, w)) + + hookTasks := retrieveHookTasks(t, w.ID, true) + hookTasksLenBefore := len(hookTasks) + + session := loginUser(t, "user2") + // create new branch + csrf := GetCSRF(t, session, "user2/repo1") + req := NewRequestWithValues(t, "POST", "user2/repo1/branches/_new/branch/master", + map[string]string{ + "_csrf": csrf, + "new_branch_name": "arbre", + "create_tag": "false", + }, + ) + session.MakeRequest(t, req, http.StatusSeeOther) + // delete the created branch + req = NewRequestWithValues(t, "POST", "user2/repo1/branches/delete?name=arbre", + map[string]string{ + "_csrf": csrf, + }, + ) + session.MakeRequest(t, req, http.StatusOK) + + // check the newly created hooktasks + hookTasks = retrieveHookTasks(t, w.ID, false) + expected := map[webhook_module.HookEventType]bool{ + webhook_module.HookEventCreate: true, + webhook_module.HookEventPush: true, // the branch creation also creates a push event + webhook_module.HookEventDelete: true, + } + for _, hookTask := range hookTasks[:len(hookTasks)-hookTasksLenBefore] { + if !expected[hookTask.EventType] { + t.Errorf("unexpected (or duplicated) event %q", hookTask.EventType) + } + + var payload struct { + Ref string `json:"ref"` + } + assert.NoError(t, json.Unmarshal([]byte(hookTask.PayloadContent), &payload)) + assert.Equal(t, "refs/heads/arbre", payload.Ref, "unexpected ref for %q event", hookTask.EventType) + delete(expected, hookTask.EventType) + } + assert.Empty(t, expected) + }) +} From 2c85a1417b21ad72a27b6b5b58a0d1bd27839672 Mon Sep 17 00:00:00 2001 From: oliverpool Date: Fri, 5 Apr 2024 11:33:55 +0200 Subject: [PATCH 2/2] [FIX] webhook creation payload ref --- services/webhook/notifier.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/services/webhook/notifier.go b/services/webhook/notifier.go index 1ab14fd6a..03c5bede8 100644 --- a/services/webhook/notifier.go +++ b/services/webhook/notifier.go @@ -748,10 +748,9 @@ func (m *webhookNotifier) PullRequestReviewRequest(ctx context.Context, doer *us func (m *webhookNotifier) CreateRef(ctx context.Context, pusher *user_model.User, repo *repo_model.Repository, refFullName git.RefName, refID string) { apiPusher := convert.ToUser(ctx, pusher, nil) apiRepo := convert.ToRepo(ctx, repo, access_model.Permission{AccessMode: perm.AccessModeNone}) - refName := refFullName.ShortName() if err := PrepareWebhooks(ctx, EventSource{Repository: repo}, webhook_module.HookEventCreate, &api.CreatePayload{ - Ref: refName, // FIXME: should it be a full ref name? + Ref: refFullName.String(), Sha: refID, RefType: refFullName.RefType(), Repo: apiRepo, @@ -785,10 +784,9 @@ func (m *webhookNotifier) PullRequestSynchronized(ctx context.Context, doer *use func (m *webhookNotifier) DeleteRef(ctx context.Context, pusher *user_model.User, repo *repo_model.Repository, refFullName git.RefName) { apiPusher := convert.ToUser(ctx, pusher, nil) apiRepo := convert.ToRepo(ctx, repo, access_model.Permission{AccessMode: perm.AccessModeOwner}) - refName := refFullName.ShortName() if err := PrepareWebhooks(ctx, EventSource{Repository: repo}, webhook_module.HookEventDelete, &api.DeletePayload{ - Ref: refName, // FIXME: should it be a full ref name? + Ref: refFullName.String(), RefType: refFullName.RefType(), PusherType: api.PusherTypeUser, Repo: apiRepo,