From 46977b0f013420136926b6b96a5aee4dea1c3018 Mon Sep 17 00:00:00 2001 From: Laura Hausmann Date: Sun, 14 Apr 2024 23:45:18 +0200 Subject: [PATCH 1/2] Fix release published actions not triggering for releases created from existing tags --- routers/api/v1/repo/release.go | 4 ++-- routers/web/repo/release.go | 4 ++-- services/release/release.go | 8 ++++---- services/release/release_test.go | 14 +++++++------- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/routers/api/v1/repo/release.go b/routers/api/v1/repo/release.go index f0f3c0bbc..c6495c3de 100644 --- a/routers/api/v1/repo/release.go +++ b/routers/api/v1/repo/release.go @@ -267,7 +267,7 @@ func CreateRelease(ctx *context.APIContext) { rel.Publisher = ctx.Doer rel.Target = form.Target - if err = release_service.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, nil, nil, nil); err != nil { + if err = release_service.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, nil, nil, nil, true); err != nil { ctx.Error(http.StatusInternalServerError, "UpdateRelease", err) return } @@ -341,7 +341,7 @@ func EditRelease(ctx *context.APIContext) { if form.IsPrerelease != nil { rel.IsPrerelease = *form.IsPrerelease } - if err := release_service.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, nil, nil, nil); err != nil { + if err := release_service.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, nil, nil, nil, false); err != nil { ctx.Error(http.StatusInternalServerError, "UpdateRelease", err) return } diff --git a/routers/web/repo/release.go b/routers/web/repo/release.go index 388fb6cb9..bb1644b89 100644 --- a/routers/web/repo/release.go +++ b/routers/web/repo/release.go @@ -567,7 +567,7 @@ func NewReleasePost(ctx *context.Context) { rel.PublisherID = ctx.Doer.ID rel.IsTag = false - if err = releaseservice.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, attachmentUUIDs, nil, nil); err != nil { + if err = releaseservice.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, attachmentUUIDs, nil, nil, true); err != nil { ctx.Data["Err_TagName"] = true ctx.ServerError("UpdateRelease", err) return @@ -674,7 +674,7 @@ func EditReleasePost(ctx *context.Context) { rel.IsDraft = len(form.Draft) > 0 rel.IsPrerelease = form.Prerelease if err = releaseservice.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, - rel, addAttachmentUUIDs, delAttachmentUUIDs, editAttachments); err != nil { + rel, addAttachmentUUIDs, delAttachmentUUIDs, editAttachments, false); err != nil { ctx.ServerError("UpdateRelease", err) return } diff --git a/services/release/release.go b/services/release/release.go index b387ccb5c..5062af143 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -199,7 +199,7 @@ func CreateNewTag(ctx context.Context, doer *user_model.User, repo *repo_model.R // delAttachmentUUIDs accept a slice of attachments' uuids which will be deleted from the release // editAttachments accept a map of attachment uuid to new attachment name which will be updated with attachments. func UpdateRelease(ctx context.Context, doer *user_model.User, gitRepo *git.Repository, rel *repo_model.Release, - addAttachmentUUIDs, delAttachmentUUIDs []string, editAttachments map[string]string, + addAttachmentUUIDs, delAttachmentUUIDs []string, editAttachments map[string]string, createdFromTag bool, ) error { if rel.ID == 0 { return errors.New("UpdateRelease only accepts an exist release") @@ -292,11 +292,11 @@ func UpdateRelease(ctx context.Context, doer *user_model.User, gitRepo *git.Repo } if !rel.IsDraft { - if !isCreated { - notify_service.UpdateRelease(gitRepo.Ctx, doer, rel) + if createdFromTag || isCreated { + notify_service.NewRelease(gitRepo.Ctx, rel) return nil } - notify_service.NewRelease(gitRepo.Ctx, rel) + notify_service.UpdateRelease(gitRepo.Ctx, doer, rel) } return nil } diff --git a/services/release/release_test.go b/services/release/release_test.go index 3d0681f1e..eac1879f8 100644 --- a/services/release/release_test.go +++ b/services/release/release_test.go @@ -159,7 +159,7 @@ func TestRelease_Update(t *testing.T) { releaseCreatedUnix := release.CreatedUnix time.Sleep(2 * time.Second) // sleep 2 seconds to ensure a different timestamp release.Note = "Changed note" - assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, nil, nil, nil)) + assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, nil, nil, nil, false)) release, err = repo_model.GetReleaseByID(db.DefaultContext, release.ID) assert.NoError(t, err) assert.Equal(t, int64(releaseCreatedUnix), int64(release.CreatedUnix)) @@ -183,7 +183,7 @@ func TestRelease_Update(t *testing.T) { releaseCreatedUnix = release.CreatedUnix time.Sleep(2 * time.Second) // sleep 2 seconds to ensure a different timestamp release.Title = "Changed title" - assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, nil, nil, nil)) + assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, nil, nil, nil, false)) release, err = repo_model.GetReleaseByID(db.DefaultContext, release.ID) assert.NoError(t, err) assert.Less(t, int64(releaseCreatedUnix), int64(release.CreatedUnix)) @@ -208,7 +208,7 @@ func TestRelease_Update(t *testing.T) { time.Sleep(2 * time.Second) // sleep 2 seconds to ensure a different timestamp release.Title = "Changed title" release.Note = "Changed note" - assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, nil, nil, nil)) + assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, nil, nil, nil, false)) release, err = repo_model.GetReleaseByID(db.DefaultContext, release.ID) assert.NoError(t, err) assert.Equal(t, int64(releaseCreatedUnix), int64(release.CreatedUnix)) @@ -233,7 +233,7 @@ func TestRelease_Update(t *testing.T) { release.IsDraft = false tagName := release.TagName - assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, nil, nil, nil)) + assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, nil, nil, nil, false)) release, err = repo_model.GetReleaseByID(db.DefaultContext, release.ID) assert.NoError(t, err) assert.Equal(t, tagName, release.TagName) @@ -247,7 +247,7 @@ func TestRelease_Update(t *testing.T) { }, strings.NewReader(samplePayload), int64(len([]byte(samplePayload)))) assert.NoError(t, err) - assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, []string{attach.UUID}, nil, nil)) + assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, []string{attach.UUID}, nil, nil, false)) assert.NoError(t, repo_model.GetReleaseAttachments(db.DefaultContext, release)) assert.Len(t, release.Attachments, 1) assert.EqualValues(t, attach.UUID, release.Attachments[0].UUID) @@ -257,7 +257,7 @@ func TestRelease_Update(t *testing.T) { // update the attachment name assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, nil, nil, map[string]string{ attach.UUID: "test2.txt", - })) + }, false)) release.Attachments = nil assert.NoError(t, repo_model.GetReleaseAttachments(db.DefaultContext, release)) assert.Len(t, release.Attachments, 1) @@ -266,7 +266,7 @@ func TestRelease_Update(t *testing.T) { assert.EqualValues(t, "test2.txt", release.Attachments[0].Name) // delete the attachment - assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, nil, []string{attach.UUID}, nil)) + assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, nil, []string{attach.UUID}, nil, false)) release.Attachments = nil assert.NoError(t, repo_model.GetReleaseAttachments(db.DefaultContext, release)) assert.Empty(t, release.Attachments) From 8506dbe2e5a96b82b502be7d74b53527a2277df8 Mon Sep 17 00:00:00 2001 From: Laura Hausmann Date: Tue, 16 Apr 2024 18:43:39 +0200 Subject: [PATCH 2/2] Add tests for webhook release events Co-authored-by: oliverpool --- tests/integration/webhook_test.go | 116 ++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/tests/integration/webhook_test.go b/tests/integration/webhook_test.go index e5ecddab3..ec85d12b0 100644 --- a/tests/integration/webhook_test.go +++ b/tests/integration/webhook_test.go @@ -9,10 +9,16 @@ import ( "testing" "code.gitea.io/gitea/models/db" + repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" webhook_model "code.gitea.io/gitea/models/webhook" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/json" webhook_module "code.gitea.io/gitea/modules/webhook" + "code.gitea.io/gitea/services/release" + "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" ) @@ -70,3 +76,113 @@ func TestWebhookPayloadRef(t *testing.T) { assert.Empty(t, expected) }) } + +func TestWebhookReleaseEvents(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + w := unittest.AssertExistsAndLoadBean(t, &webhook_model.Webhook{ + ID: 1, + RepoID: repo.ID, + }) + 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) + + gitRepo, err := gitrepo.OpenRepository(git.DefaultContext, repo) + assert.NoError(t, err) + defer gitRepo.Close() + + t.Run("CreateRelease", func(t *testing.T) { + assert.NoError(t, release.CreateRelease(gitRepo, &repo_model.Release{ + RepoID: repo.ID, + Repo: repo, + PublisherID: user.ID, + Publisher: user, + TagName: "v1.1.1", + Target: "master", + Title: "v1.1.1 is released", + Note: "v1.1.1 is released", + IsDraft: false, + IsPrerelease: false, + IsTag: false, + }, nil, "")) + + // check the newly created hooktasks + hookTasksLenBefore := len(hookTasks) + hookTasks = retrieveHookTasks(t, w.ID, false) + + checkHookTasks(t, map[webhook_module.HookEventType]string{ + webhook_module.HookEventRelease: "published", + webhook_module.HookEventCreate: "", // a tag was created as well + webhook_module.HookEventPush: "", // the tag creation also means a push event + }, hookTasks[:len(hookTasks)-hookTasksLenBefore]) + + t.Run("UpdateRelease", func(t *testing.T) { + rel := unittest.AssertExistsAndLoadBean(t, &repo_model.Release{RepoID: repo.ID, TagName: "v1.1.1"}) + assert.NoError(t, release.UpdateRelease(db.DefaultContext, user, gitRepo, rel, nil, nil, nil, false)) + + // check the newly created hooktasks + hookTasksLenBefore := len(hookTasks) + hookTasks = retrieveHookTasks(t, w.ID, false) + + checkHookTasks(t, map[webhook_module.HookEventType]string{ + webhook_module.HookEventRelease: "updated", + }, hookTasks[:len(hookTasks)-hookTasksLenBefore]) + }) + }) + + t.Run("CreateNewTag", func(t *testing.T) { + assert.NoError(t, release.CreateNewTag(db.DefaultContext, + user, + repo, + "master", + "v1.1.2", + "v1.1.2 is tagged", + )) + + // check the newly created hooktasks + hookTasksLenBefore := len(hookTasks) + hookTasks = retrieveHookTasks(t, w.ID, false) + + checkHookTasks(t, map[webhook_module.HookEventType]string{ + webhook_module.HookEventCreate: "", // tag was created as well + webhook_module.HookEventPush: "", // the tag creation also means a push event + }, hookTasks[:len(hookTasks)-hookTasksLenBefore]) + + t.Run("UpdateRelease", func(t *testing.T) { + rel := unittest.AssertExistsAndLoadBean(t, &repo_model.Release{RepoID: repo.ID, TagName: "v1.1.2"}) + assert.NoError(t, release.UpdateRelease(db.DefaultContext, user, gitRepo, rel, nil, nil, nil, true)) + + // check the newly created hooktasks + hookTasksLenBefore := len(hookTasks) + hookTasks = retrieveHookTasks(t, w.ID, false) + + checkHookTasks(t, map[webhook_module.HookEventType]string{ + webhook_module.HookEventRelease: "published", + }, hookTasks[:len(hookTasks)-hookTasksLenBefore]) + }) + }) +} + +func checkHookTasks(t *testing.T, expectedActions map[webhook_module.HookEventType]string, hookTasks []*webhook_model.HookTask) { + t.Helper() + for _, hookTask := range hookTasks { + expectedAction, ok := expectedActions[hookTask.EventType] + if !ok { + t.Errorf("unexpected (or duplicated) event %q", hookTask.EventType) + } + var payload struct { + Action string `json:"action"` + } + assert.NoError(t, json.Unmarshal([]byte(hookTask.PayloadContent), &payload)) + assert.Equal(t, expectedAction, payload.Action, "unexpected action for %q event", hookTask.EventType) + delete(expectedActions, hookTask.EventType) + } + assert.Empty(t, expectedActions) +}