From 1915361b42257cb47c45e3f6b84df98d65c8f0a7 Mon Sep 17 00:00:00 2001 From: kerwin612 Date: Sat, 19 Jul 2025 22:11:11 +0800 Subject: [PATCH 1/6] Add webhook payload size optimization options --- models/migrations/migrations.go | 1 + models/migrations/v1_24/v321.go | 23 ++++++ models/webhook/webhook.go | 4 + models/webhook/webhook_test.go | 42 ++++++++++ options/locale/locale_en-US.ini | 5 ++ routers/web/repo/setting/webhook.go | 4 + services/forms/repo_form.go | 3 + services/webhook/notifier.go | 60 ++++++++++++++ services/webhook/webhook_test.go | 79 +++++++++++++++++++ templates/repo/settings/webhook/settings.tmpl | 19 +++++ 10 files changed, 240 insertions(+) create mode 100644 models/migrations/v1_24/v321.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 176372486e8f6..525e667668923 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -382,6 +382,7 @@ func prepareMigrationTasks() []*migration { newMigration(318, "Add anonymous_access_mode for repo_unit", v1_24.AddRepoUnitAnonymousAccessMode), newMigration(319, "Add ExclusiveOrder to Label table", v1_24.AddExclusiveOrderColumnToLabelTable), newMigration(320, "Migrate two_factor_policy to login_source table", v1_24.MigrateSkipTwoFactor), + newMigration(321, "Add webhook payload optimization columns", v1_24.AddWebhookPayloadOptimizationColumns), } return preparedMigrations } diff --git a/models/migrations/v1_24/v321.go b/models/migrations/v1_24/v321.go new file mode 100644 index 0000000000000..e88146789e219 --- /dev/null +++ b/models/migrations/v1_24/v321.go @@ -0,0 +1,23 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_24 + +import ( + "xorm.io/xorm" +) + +func AddWebhookPayloadOptimizationColumns(x *xorm.Engine) error { + type Webhook struct { + ExcludeFiles bool `xorm:"exclude_files NOT NULL DEFAULT false"` + ExcludeCommits bool `xorm:"exclude_commits NOT NULL DEFAULT false"` + } + _, err := x.SyncWithOptions( + xorm.SyncOptions{ + IgnoreConstrains: true, + IgnoreIndices: true, + }, + new(Webhook), + ) + return err +} diff --git a/models/webhook/webhook.go b/models/webhook/webhook.go index b234d9ffee519..117447de90f52 100644 --- a/models/webhook/webhook.go +++ b/models/webhook/webhook.go @@ -139,6 +139,10 @@ type Webhook struct { // HeaderAuthorizationEncrypted should be accessed using HeaderAuthorization() and SetHeaderAuthorization() HeaderAuthorizationEncrypted string `xorm:"TEXT"` + // Payload size optimization options + ExcludeFiles bool `xorm:"exclude_files"` // Exclude file changes from commit payloads + ExcludeCommits bool `xorm:"exclude_commits"` // Exclude commits and head_commit from push payloads + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` } diff --git a/models/webhook/webhook_test.go b/models/webhook/webhook_test.go index edad8fc996c14..95ec4d6d95106 100644 --- a/models/webhook/webhook_test.go +++ b/models/webhook/webhook_test.go @@ -330,3 +330,45 @@ func TestCleanupHookTaskTable_OlderThan_LeavesTaskEarlierThanAgeToDelete(t *test assert.NoError(t, CleanupHookTaskTable(t.Context(), OlderThan, 168*time.Hour, 0)) unittest.AssertExistsAndLoadBean(t, hookTask) } + +func TestWebhookPayloadOptimization(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + webhook := &Webhook{ + RepoID: 1, + URL: "http://example.com/webhook", + HTTPMethod: "POST", + ContentType: ContentTypeJSON, + Secret: "secret", + IsActive: true, + Type: webhook_module.GITEA, + ExcludeFiles: true, + ExcludeCommits: false, + HookEvent: &webhook_module.HookEvent{ + PushOnly: true, + }, + } + + // Test creating webhook with payload optimization options + err := CreateWebhook(db.DefaultContext, webhook) + assert.NoError(t, err) + assert.NotZero(t, webhook.ID) + + // Test retrieving webhook and checking payload optimization options + retrievedWebhook, err := GetWebhookByID(db.DefaultContext, webhook.ID) + assert.NoError(t, err) + assert.True(t, retrievedWebhook.ExcludeFiles) + assert.False(t, retrievedWebhook.ExcludeCommits) + + // Test updating webhook with different payload optimization options + retrievedWebhook.ExcludeFiles = false + retrievedWebhook.ExcludeCommits = true + err = UpdateWebhook(db.DefaultContext, retrievedWebhook) + assert.NoError(t, err) + + // Verify the update + updatedWebhook, err := GetWebhookByID(db.DefaultContext, webhook.ID) + assert.NoError(t, err) + assert.False(t, updatedWebhook.ExcludeFiles) + assert.True(t, updatedWebhook.ExcludeCommits) +} diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index c1a3d37037fe3..b5063b03ceb8c 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2424,6 +2424,11 @@ settings.event_package = Package settings.event_package_desc = Package created or deleted in a repository. settings.branch_filter = Branch filter settings.branch_filter_desc = Branch whitelist for push, branch creation and branch deletion events, specified as glob pattern. If empty or *, events for all branches are reported. See %[2]s documentation for syntax. Examples: master, {master,release*}. +settings.payload_optimization = Payload Size Optimization +settings.exclude_files = Exclude file changes +settings.exclude_files_desc = Remove file information (added, removed, modified files) from commit payloads to reduce webhook size. +settings.exclude_commits = Exclude commits +settings.exclude_commits_desc = Remove commits and head_commit from push payloads, keeping only before, after and total_commits fields. settings.authorization_header = Authorization Header settings.authorization_header_desc = Will be included as authorization header for requests when present. Examples: %s. settings.active = Active diff --git a/routers/web/repo/setting/webhook.go b/routers/web/repo/setting/webhook.go index f107449749364..ce2bce04db0cd 100644 --- a/routers/web/repo/setting/webhook.go +++ b/routers/web/repo/setting/webhook.go @@ -243,6 +243,8 @@ func createWebhook(ctx *context.Context, params webhookParams) { Meta: string(meta), OwnerID: orCtx.OwnerID, IsSystemWebhook: orCtx.IsSystemWebhook, + ExcludeFiles: params.WebhookForm.ExcludeFiles, + ExcludeCommits: params.WebhookForm.ExcludeCommits, } err = w.SetHeaderAuthorization(params.WebhookForm.AuthorizationHeader) if err != nil { @@ -294,6 +296,8 @@ func editWebhook(ctx *context.Context, params webhookParams) { w.IsActive = params.WebhookForm.Active w.HTTPMethod = params.HTTPMethod w.Meta = string(meta) + w.ExcludeFiles = params.WebhookForm.ExcludeFiles + w.ExcludeCommits = params.WebhookForm.ExcludeCommits err = w.SetHeaderAuthorization(params.WebhookForm.AuthorizationHeader) if err != nil { diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index cb267f891ccb7..cf933f67a9b0e 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -239,6 +239,9 @@ type WebhookForm struct { BranchFilter string `binding:"GlobPattern"` AuthorizationHeader string Secret string + // Payload size optimization options + ExcludeFiles bool // Exclude file changes from commit payloads + ExcludeCommits bool // Exclude commits and head_commit from push payloads } // PushOnly if the hook will be triggered when push diff --git a/services/webhook/notifier.go b/services/webhook/notifier.go index 672abd5c95d0e..5f54beba672e9 100644 --- a/services/webhook/notifier.go +++ b/services/webhook/notifier.go @@ -7,6 +7,7 @@ import ( "context" actions_model "code.gitea.io/gitea/models/actions" + "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/models/organization" @@ -15,10 +16,12 @@ import ( access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" 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/httplib" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" @@ -640,6 +643,57 @@ func (m *webhookNotifier) IssueChangeMilestone(ctx context.Context, doer *user_m } } +// applyWebhookPayloadOptimizations applies payload size optimizations based on webhook configurations +func (m *webhookNotifier) applyWebhookPayloadOptimizations(ctx context.Context, repo *repo_model.Repository, apiCommits []*api.PayloadCommit, apiHeadCommit *api.PayloadCommit) ([]*api.PayloadCommit, *api.PayloadCommit) { + // Get webhooks for this repository to check their configuration + webhooks, err := db.Find[webhook_model.Webhook](ctx, webhook_model.ListWebhookOptions{ + RepoID: repo.ID, + IsActive: optional.Some(true), + }) + if err != nil { + log.Error("Failed to get webhooks for repository %d: %v", repo.ID, err) + // Continue with default behavior if we can't get webhooks + return apiCommits, apiHeadCommit + } + + // Check if any webhook has payload optimization options enabled + hasExcludeFiles := false + hasExcludeCommits := false + for _, webhook := range webhooks { + if webhook.HasEvent(webhook_module.HookEventPush) { + if webhook.ExcludeFiles { + hasExcludeFiles = true + } + if webhook.ExcludeCommits { + hasExcludeCommits = true + } + } + } + + // Apply payload optimizations based on webhook configurations + if hasExcludeFiles { + // Remove file information from commits + for _, commit := range apiCommits { + commit.Added = nil + commit.Removed = nil + commit.Modified = nil + } + if apiHeadCommit != nil { + apiHeadCommit.Added = nil + apiHeadCommit.Removed = nil + apiHeadCommit.Modified = nil + } + } + + if hasExcludeCommits { + // Exclude commits and head_commit from payload + apiCommits = nil + apiHeadCommit = nil + } + + return apiCommits, apiHeadCommit +} + func (m *webhookNotifier) PushCommits(ctx context.Context, pusher *user_model.User, repo *repo_model.Repository, opts *repository.PushUpdateOptions, commits *repository.PushCommits) { apiPusher := convert.ToUser(ctx, pusher, nil) apiCommits, apiHeadCommit, err := commits.ToAPIPayloadCommits(ctx, repo) @@ -648,6 +702,9 @@ func (m *webhookNotifier) PushCommits(ctx context.Context, pusher *user_model.Us return } + // Apply payload optimizations + apiCommits, apiHeadCommit = m.applyWebhookPayloadOptimizations(ctx, repo, apiCommits, apiHeadCommit) + if err := PrepareWebhooks(ctx, EventSource{Repository: repo}, webhook_module.HookEventPush, &api.PushPayload{ Ref: opts.RefFullName.String(), Before: opts.OldCommitID, @@ -887,6 +944,9 @@ func (m *webhookNotifier) SyncPushCommits(ctx context.Context, pusher *user_mode return } + // Apply payload optimizations + apiCommits, apiHeadCommit = m.applyWebhookPayloadOptimizations(ctx, repo, apiCommits, apiHeadCommit) + if err := PrepareWebhooks(ctx, EventSource{Repository: repo}, webhook_module.HookEventPush, &api.PushPayload{ Ref: opts.RefFullName.String(), Before: opts.OldCommitID, diff --git a/services/webhook/webhook_test.go b/services/webhook/webhook_test.go index 5a805347e38a7..00dfe540df168 100644 --- a/services/webhook/webhook_test.go +++ b/services/webhook/webhook_test.go @@ -91,3 +91,82 @@ func TestWebhookUserMail(t *testing.T) { assert.Equal(t, user.GetPlaceholderEmail(), convert.ToUser(db.DefaultContext, user, nil).Email) assert.Equal(t, user.Email, convert.ToUser(db.DefaultContext, user, user).Email) } + +func TestWebhookPayloadOptimization(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + // Create a test repository + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + + // Create a webhook with payload optimization enabled + webhook := &webhook_model.Webhook{ + RepoID: repo.ID, + URL: "http://example.com/webhook", + HTTPMethod: "POST", + ContentType: webhook_model.ContentTypeJSON, + Secret: "secret", + IsActive: true, + Type: webhook_module.GITEA, + ExcludeFiles: true, + ExcludeCommits: false, + HookEvent: &webhook_module.HookEvent{ + PushOnly: true, + }, + } + + err := webhook_model.CreateWebhook(db.DefaultContext, webhook) + assert.NoError(t, err) + + // Create test commits with file information + apiCommits := []*api.PayloadCommit{ + { + ID: "abc123", + Message: "Test commit", + Added: []string{"file1.txt", "file2.txt"}, + Removed: []string{"oldfile.txt"}, + Modified: []string{"modified.txt"}, + }, + { + ID: "def456", + Message: "Another commit", + Added: []string{"file3.txt"}, + Removed: []string{}, + Modified: []string{"file1.txt"}, + }, + } + + apiHeadCommit := &api.PayloadCommit{ + ID: "def456", + Message: "Another commit", + Added: []string{"file3.txt"}, + Removed: []string{}, + Modified: []string{"file1.txt"}, + } + + // Test payload optimization + notifier := &webhookNotifier{} + optimizedCommits, optimizedHeadCommit := notifier.applyWebhookPayloadOptimizations(db.DefaultContext, repo, apiCommits, apiHeadCommit) + + // Verify that file information was removed when ExcludeFiles is true + assert.Nil(t, optimizedCommits[0].Added) + assert.Nil(t, optimizedCommits[0].Removed) + assert.Nil(t, optimizedCommits[0].Modified) + assert.Nil(t, optimizedCommits[1].Added) + assert.Nil(t, optimizedCommits[1].Removed) + assert.Nil(t, optimizedCommits[1].Modified) + assert.Nil(t, optimizedHeadCommit.Added) + assert.Nil(t, optimizedHeadCommit.Removed) + assert.Nil(t, optimizedHeadCommit.Modified) + + // Test with ExcludeCommits enabled + webhook.ExcludeFiles = false + webhook.ExcludeCommits = true + err = webhook_model.UpdateWebhook(db.DefaultContext, webhook) + assert.NoError(t, err) + + optimizedCommits, optimizedHeadCommit = notifier.applyWebhookPayloadOptimizations(db.DefaultContext, repo, apiCommits, apiHeadCommit) + + // Verify that commits and head_commit were excluded + assert.Nil(t, optimizedCommits) + assert.Nil(t, optimizedHeadCommit) +} diff --git a/templates/repo/settings/webhook/settings.tmpl b/templates/repo/settings/webhook/settings.tmpl index a8ad1d6c9e5cf..c390246a71095 100644 --- a/templates/repo/settings/webhook/settings.tmpl +++ b/templates/repo/settings/webhook/settings.tmpl @@ -47,6 +47,25 @@ {{ctx.Locale.Tr "repo.settings.branch_filter_desc" "https://pkg.go.dev/github.com/gobwas/glob#Compile" "github.com/gobwas/glob"}} + +
+

{{ctx.Locale.Tr "repo.settings.payload_optimization"}}

+
+
+ + + {{ctx.Locale.Tr "repo.settings.exclude_files_desc"}} +
+
+
+
+ + + {{ctx.Locale.Tr "repo.settings.exclude_commits_desc"}} +
+
+
+

{{ctx.Locale.Tr "repo.settings.event_desc"}}

From 5472d66d16c6f1cdbfbe02cb56858d83c2867352 Mon Sep 17 00:00:00 2001 From: kerwin612 Date: Wed, 23 Jul 2025 14:41:59 +0800 Subject: [PATCH 2/6] refactor webhook payload optimization: switch from bool to limit number --- models/migrations/v1_24/v321.go | 4 +- models/webhook/webhook.go | 4 +- models/webhook/webhook_test.go | 30 ++-- options/locale/locale_en-US.ini | 8 +- routers/web/repo/setting/webhook.go | 30 ++-- services/forms/repo_form.go | 6 +- services/webhook/notifier.go | 45 +++--- services/webhook/webhook_test.go | 134 +++++++++++++----- templates/repo/settings/webhook/settings.tmpl | 16 +-- 9 files changed, 172 insertions(+), 105 deletions(-) diff --git a/models/migrations/v1_24/v321.go b/models/migrations/v1_24/v321.go index e88146789e219..748ee6b0149d0 100644 --- a/models/migrations/v1_24/v321.go +++ b/models/migrations/v1_24/v321.go @@ -9,8 +9,8 @@ import ( func AddWebhookPayloadOptimizationColumns(x *xorm.Engine) error { type Webhook struct { - ExcludeFiles bool `xorm:"exclude_files NOT NULL DEFAULT false"` - ExcludeCommits bool `xorm:"exclude_commits NOT NULL DEFAULT false"` + ExcludeFilesLimit int `xorm:"exclude_files_limit NOT NULL DEFAULT 0"` + ExcludeCommitsLimit int `xorm:"exclude_commits_limit NOT NULL DEFAULT 0"` } _, err := x.SyncWithOptions( xorm.SyncOptions{ diff --git a/models/webhook/webhook.go b/models/webhook/webhook.go index 117447de90f52..989e140dc42a0 100644 --- a/models/webhook/webhook.go +++ b/models/webhook/webhook.go @@ -140,8 +140,8 @@ type Webhook struct { HeaderAuthorizationEncrypted string `xorm:"TEXT"` // Payload size optimization options - ExcludeFiles bool `xorm:"exclude_files"` // Exclude file changes from commit payloads - ExcludeCommits bool `xorm:"exclude_commits"` // Exclude commits and head_commit from push payloads + ExcludeFilesLimit int `xorm:"exclude_files_limit"` // Limit number of file changes in commit payloads, 0 means unlimited + ExcludeCommitsLimit int `xorm:"exclude_commits_limit"` // Limit number of commits in push payloads, 0 means unlimited CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` diff --git a/models/webhook/webhook_test.go b/models/webhook/webhook_test.go index 95ec4d6d95106..db1150772e5cd 100644 --- a/models/webhook/webhook_test.go +++ b/models/webhook/webhook_test.go @@ -335,15 +335,15 @@ func TestWebhookPayloadOptimization(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) webhook := &Webhook{ - RepoID: 1, - URL: "http://example.com/webhook", - HTTPMethod: "POST", - ContentType: ContentTypeJSON, - Secret: "secret", - IsActive: true, - Type: webhook_module.GITEA, - ExcludeFiles: true, - ExcludeCommits: false, + RepoID: 1, + URL: "http://example.com/webhook", + HTTPMethod: "POST", + ContentType: ContentTypeJSON, + Secret: "secret", + IsActive: true, + Type: webhook_module.GITEA, + ExcludeFilesLimit: 1, + ExcludeCommitsLimit: 0, HookEvent: &webhook_module.HookEvent{ PushOnly: true, }, @@ -357,18 +357,18 @@ func TestWebhookPayloadOptimization(t *testing.T) { // Test retrieving webhook and checking payload optimization options retrievedWebhook, err := GetWebhookByID(db.DefaultContext, webhook.ID) assert.NoError(t, err) - assert.True(t, retrievedWebhook.ExcludeFiles) - assert.False(t, retrievedWebhook.ExcludeCommits) + assert.Equal(t, 1, retrievedWebhook.ExcludeFilesLimit) + assert.Equal(t, 0, retrievedWebhook.ExcludeCommitsLimit) // Test updating webhook with different payload optimization options - retrievedWebhook.ExcludeFiles = false - retrievedWebhook.ExcludeCommits = true + retrievedWebhook.ExcludeFilesLimit = 0 + retrievedWebhook.ExcludeCommitsLimit = 2 err = UpdateWebhook(db.DefaultContext, retrievedWebhook) assert.NoError(t, err) // Verify the update updatedWebhook, err := GetWebhookByID(db.DefaultContext, webhook.ID) assert.NoError(t, err) - assert.False(t, updatedWebhook.ExcludeFiles) - assert.True(t, updatedWebhook.ExcludeCommits) + assert.Equal(t, 0, updatedWebhook.ExcludeFilesLimit) + assert.Equal(t, 2, updatedWebhook.ExcludeCommitsLimit) } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index b5063b03ceb8c..8199126c9a647 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2425,10 +2425,10 @@ settings.event_package_desc = Package created or deleted in a repository. settings.branch_filter = Branch filter settings.branch_filter_desc = Branch whitelist for push, branch creation and branch deletion events, specified as glob pattern. If empty or *, events for all branches are reported. See %[2]s documentation for syntax. Examples: master, {master,release*}. settings.payload_optimization = Payload Size Optimization -settings.exclude_files = Exclude file changes -settings.exclude_files_desc = Remove file information (added, removed, modified files) from commit payloads to reduce webhook size. -settings.exclude_commits = Exclude commits -settings.exclude_commits_desc = Remove commits and head_commit from push payloads, keeping only before, after and total_commits fields. +settings.exclude_files_limit = Limit file changes +settings.exclude_files_limit_desc = Limit the number of file changes (added, removed, modified) included in each commit payload. 0 means unlimited. +settings.exclude_commits_limit = Limit commits +settings.exclude_commits_limit_desc = Limit the number of commits included in push payloads. 0 means unlimited. settings.authorization_header = Authorization Header settings.authorization_header_desc = Will be included as authorization header for requests when present. Examples: %s. settings.active = Active diff --git a/routers/web/repo/setting/webhook.go b/routers/web/repo/setting/webhook.go index ce2bce04db0cd..7b6d69674d9c7 100644 --- a/routers/web/repo/setting/webhook.go +++ b/routers/web/repo/setting/webhook.go @@ -232,19 +232,19 @@ func createWebhook(ctx *context.Context, params webhookParams) { } w := &webhook.Webhook{ - RepoID: orCtx.RepoID, - URL: params.URL, - HTTPMethod: params.HTTPMethod, - ContentType: params.ContentType, - Secret: params.WebhookForm.Secret, - HookEvent: ParseHookEvent(params.WebhookForm), - IsActive: params.WebhookForm.Active, - Type: params.Type, - Meta: string(meta), - OwnerID: orCtx.OwnerID, - IsSystemWebhook: orCtx.IsSystemWebhook, - ExcludeFiles: params.WebhookForm.ExcludeFiles, - ExcludeCommits: params.WebhookForm.ExcludeCommits, + RepoID: orCtx.RepoID, + URL: params.URL, + HTTPMethod: params.HTTPMethod, + ContentType: params.ContentType, + Secret: params.WebhookForm.Secret, + HookEvent: ParseHookEvent(params.WebhookForm), + IsActive: params.WebhookForm.Active, + Type: params.Type, + Meta: string(meta), + OwnerID: orCtx.OwnerID, + IsSystemWebhook: orCtx.IsSystemWebhook, + ExcludeFilesLimit: params.WebhookForm.ExcludeFilesLimit, + ExcludeCommitsLimit: params.WebhookForm.ExcludeCommitsLimit, } err = w.SetHeaderAuthorization(params.WebhookForm.AuthorizationHeader) if err != nil { @@ -296,8 +296,8 @@ func editWebhook(ctx *context.Context, params webhookParams) { w.IsActive = params.WebhookForm.Active w.HTTPMethod = params.HTTPMethod w.Meta = string(meta) - w.ExcludeFiles = params.WebhookForm.ExcludeFiles - w.ExcludeCommits = params.WebhookForm.ExcludeCommits + w.ExcludeFilesLimit = params.WebhookForm.ExcludeFilesLimit + w.ExcludeCommitsLimit = params.WebhookForm.ExcludeCommitsLimit err = w.SetHeaderAuthorization(params.WebhookForm.AuthorizationHeader) if err != nil { diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index cf933f67a9b0e..e9c58bae6c7aa 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -240,8 +240,8 @@ type WebhookForm struct { AuthorizationHeader string Secret string // Payload size optimization options - ExcludeFiles bool // Exclude file changes from commit payloads - ExcludeCommits bool // Exclude commits and head_commit from push payloads + ExcludeFilesLimit int // Limit number of file changes in commit payloads, 0 means unlimited + ExcludeCommitsLimit int // Limit number of commits in push payloads, 0 means unlimited } // PushOnly if the hook will be triggered when push @@ -625,7 +625,7 @@ type UpdateAllowEditsForm struct { // | _// __ \| | _/ __ \\__ \ / ___// __ \ // | | \ ___/| |_\ ___/ / __ \_\___ \\ ___/ // |____|_ /\___ >____/\___ >____ /____ >\___ > -// \/ \/ \/ \/ \/ \/ +// \/ \/ \/ \/ \/ \/ // NewReleaseForm form for creating release type NewReleaseForm struct { diff --git a/services/webhook/notifier.go b/services/webhook/notifier.go index 5f54beba672e9..5c3fc4996b1b4 100644 --- a/services/webhook/notifier.go +++ b/services/webhook/notifier.go @@ -657,38 +657,47 @@ func (m *webhookNotifier) applyWebhookPayloadOptimizations(ctx context.Context, } // Check if any webhook has payload optimization options enabled - hasExcludeFiles := false - hasExcludeCommits := false + hasFilesLimit := 0 + hasCommitsLimit := 0 for _, webhook := range webhooks { if webhook.HasEvent(webhook_module.HookEventPush) { - if webhook.ExcludeFiles { - hasExcludeFiles = true + if webhook.ExcludeFilesLimit > 0 && (hasFilesLimit == 0 || webhook.ExcludeFilesLimit < hasFilesLimit) { + hasFilesLimit = webhook.ExcludeFilesLimit } - if webhook.ExcludeCommits { - hasExcludeCommits = true + if webhook.ExcludeCommitsLimit > 0 && (hasCommitsLimit == 0 || webhook.ExcludeCommitsLimit < hasCommitsLimit) { + hasCommitsLimit = webhook.ExcludeCommitsLimit } } } // Apply payload optimizations based on webhook configurations - if hasExcludeFiles { - // Remove file information from commits + if hasFilesLimit > 0 { for _, commit := range apiCommits { - commit.Added = nil - commit.Removed = nil - commit.Modified = nil + if commit.Added != nil && len(commit.Added) > hasFilesLimit { + commit.Added = commit.Added[:hasFilesLimit] + } + if commit.Removed != nil && len(commit.Removed) > hasFilesLimit { + commit.Removed = commit.Removed[:hasFilesLimit] + } + if commit.Modified != nil && len(commit.Modified) > hasFilesLimit { + commit.Modified = commit.Modified[:hasFilesLimit] + } } if apiHeadCommit != nil { - apiHeadCommit.Added = nil - apiHeadCommit.Removed = nil - apiHeadCommit.Modified = nil + if apiHeadCommit.Added != nil && len(apiHeadCommit.Added) > hasFilesLimit { + apiHeadCommit.Added = apiHeadCommit.Added[:hasFilesLimit] + } + if apiHeadCommit.Removed != nil && len(apiHeadCommit.Removed) > hasFilesLimit { + apiHeadCommit.Removed = apiHeadCommit.Removed[:hasFilesLimit] + } + if apiHeadCommit.Modified != nil && len(apiHeadCommit.Modified) > hasFilesLimit { + apiHeadCommit.Modified = apiHeadCommit.Modified[:hasFilesLimit] + } } } - if hasExcludeCommits { - // Exclude commits and head_commit from payload - apiCommits = nil - apiHeadCommit = nil + if hasCommitsLimit > 0 && len(apiCommits) > hasCommitsLimit { + apiCommits = apiCommits[:hasCommitsLimit] } return apiCommits, apiHeadCommit diff --git a/services/webhook/webhook_test.go b/services/webhook/webhook_test.go index 00dfe540df168..bc9501cbcfa8d 100644 --- a/services/webhook/webhook_test.go +++ b/services/webhook/webhook_test.go @@ -95,29 +95,36 @@ func TestWebhookUserMail(t *testing.T) { func TestWebhookPayloadOptimization(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) + var optimizedCommits []*api.PayloadCommit + var optimizedHeadCommit *api.PayloadCommit + // Create a test repository repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - // Create a webhook with payload optimization enabled + // Create a webhook with file limit = 1 webhook := &webhook_model.Webhook{ - RepoID: repo.ID, - URL: "http://example.com/webhook", - HTTPMethod: "POST", - ContentType: webhook_model.ContentTypeJSON, - Secret: "secret", - IsActive: true, - Type: webhook_module.GITEA, - ExcludeFiles: true, - ExcludeCommits: false, + RepoID: repo.ID, + URL: "http://example.com/webhook", + HTTPMethod: "POST", + ContentType: webhook_model.ContentTypeJSON, + Secret: "secret", + IsActive: true, + Type: webhook_module.GITEA, + ExcludeFilesLimit: 1, + ExcludeCommitsLimit: 0, HookEvent: &webhook_module.HookEvent{ PushOnly: true, }, } - err := webhook_model.CreateWebhook(db.DefaultContext, webhook) + err := webhook.UpdateEvent() + assert.NoError(t, err) + err = webhook_model.CreateWebhook(db.DefaultContext, webhook) assert.NoError(t, err) + assert.NotZero(t, webhook.ID) - // Create test commits with file information + // Test payload optimization: should truncate to 1 file per field + notifier := &webhookNotifier{} apiCommits := []*api.PayloadCommit{ { ID: "abc123", @@ -134,7 +141,6 @@ func TestWebhookPayloadOptimization(t *testing.T) { Modified: []string{"file1.txt"}, }, } - apiHeadCommit := &api.PayloadCommit{ ID: "def456", Message: "Another commit", @@ -142,31 +148,87 @@ func TestWebhookPayloadOptimization(t *testing.T) { Removed: []string{}, Modified: []string{"file1.txt"}, } - - // Test payload optimization - notifier := &webhookNotifier{} - optimizedCommits, optimizedHeadCommit := notifier.applyWebhookPayloadOptimizations(db.DefaultContext, repo, apiCommits, apiHeadCommit) - - // Verify that file information was removed when ExcludeFiles is true - assert.Nil(t, optimizedCommits[0].Added) - assert.Nil(t, optimizedCommits[0].Removed) - assert.Nil(t, optimizedCommits[0].Modified) - assert.Nil(t, optimizedCommits[1].Added) - assert.Nil(t, optimizedCommits[1].Removed) - assert.Nil(t, optimizedCommits[1].Modified) - assert.Nil(t, optimizedHeadCommit.Added) - assert.Nil(t, optimizedHeadCommit.Removed) - assert.Nil(t, optimizedHeadCommit.Modified) - - // Test with ExcludeCommits enabled - webhook.ExcludeFiles = false - webhook.ExcludeCommits = true + optimizedCommits, _ = notifier.applyWebhookPayloadOptimizations(db.DefaultContext, repo, apiCommits, apiHeadCommit) + assert.Equal(t, []string{"file1.txt"}, optimizedCommits[0].Added) + assert.Equal(t, []string{"oldfile.txt"}, optimizedCommits[0].Removed) + assert.Equal(t, []string{"modified.txt"}, optimizedCommits[0].Modified) + assert.Equal(t, []string{"file3.txt"}, optimizedCommits[1].Added) + assert.Equal(t, []string{}, optimizedCommits[1].Removed) + assert.Equal(t, []string{"file1.txt"}, optimizedCommits[1].Modified) + + _, optimizedHeadCommit = notifier.applyWebhookPayloadOptimizations(db.DefaultContext, repo, apiCommits, apiHeadCommit) + assert.Equal(t, []string{"file3.txt"}, optimizedHeadCommit.Added) + assert.Equal(t, []string{}, optimizedHeadCommit.Removed) + assert.Equal(t, []string{"file1.txt"}, optimizedHeadCommit.Modified) + + // Test with commit limit = 1 + webhook.ExcludeFilesLimit = 0 + webhook.ExcludeCommitsLimit = 1 err = webhook_model.UpdateWebhook(db.DefaultContext, webhook) assert.NoError(t, err) + apiCommits = []*api.PayloadCommit{ + { + ID: "abc123", + Message: "Test commit", + Added: []string{"file1.txt", "file2.txt"}, + Removed: []string{"oldfile.txt"}, + Modified: []string{"modified.txt"}, + }, + { + ID: "def456", + Message: "Another commit", + Added: []string{"file3.txt"}, + Removed: []string{}, + Modified: []string{"file1.txt"}, + }, + } + apiHeadCommit = &api.PayloadCommit{ + ID: "def456", + Message: "Another commit", + Added: []string{"file3.txt"}, + Removed: []string{}, + Modified: []string{"file1.txt"}, + } + optimizedCommits, _ = notifier.applyWebhookPayloadOptimizations(db.DefaultContext, repo, apiCommits, apiHeadCommit) + assert.Len(t, optimizedCommits, 1) + assert.Equal(t, "abc123", optimizedCommits[0].ID) + // Test with no limits (0 means unlimited) + webhook.ExcludeFilesLimit = 0 + webhook.ExcludeCommitsLimit = 0 + err = webhook_model.UpdateWebhook(db.DefaultContext, webhook) + assert.NoError(t, err) + apiCommits = []*api.PayloadCommit{ + { + ID: "abc123", + Message: "Test commit", + Added: []string{"file1.txt", "file2.txt"}, + Removed: []string{"oldfile.txt"}, + Modified: []string{"modified.txt"}, + }, + { + ID: "def456", + Message: "Another commit", + Added: []string{"file3.txt"}, + Removed: []string{}, + Modified: []string{"file1.txt"}, + }, + } + apiHeadCommit = &api.PayloadCommit{ + ID: "def456", + Message: "Another commit", + Added: []string{"file3.txt"}, + Removed: []string{}, + Modified: []string{"file1.txt"}, + } optimizedCommits, optimizedHeadCommit = notifier.applyWebhookPayloadOptimizations(db.DefaultContext, repo, apiCommits, apiHeadCommit) - - // Verify that commits and head_commit were excluded - assert.Nil(t, optimizedCommits) - assert.Nil(t, optimizedHeadCommit) + assert.Equal(t, []string{"file1.txt", "file2.txt"}, optimizedCommits[0].Added) + assert.Equal(t, []string{"oldfile.txt"}, optimizedCommits[0].Removed) + assert.Equal(t, []string{"modified.txt"}, optimizedCommits[0].Modified) + assert.Equal(t, []string{"file3.txt"}, optimizedCommits[1].Added) + assert.Equal(t, []string{}, optimizedCommits[1].Removed) + assert.Equal(t, []string{"file1.txt"}, optimizedCommits[1].Modified) + assert.Equal(t, []string{"file3.txt"}, optimizedHeadCommit.Added) + assert.Equal(t, []string{}, optimizedHeadCommit.Removed) + assert.Equal(t, []string{"file1.txt"}, optimizedHeadCommit.Modified) } diff --git a/templates/repo/settings/webhook/settings.tmpl b/templates/repo/settings/webhook/settings.tmpl index c390246a71095..ef93f34f1b1a2 100644 --- a/templates/repo/settings/webhook/settings.tmpl +++ b/templates/repo/settings/webhook/settings.tmpl @@ -51,18 +51,14 @@

{{ctx.Locale.Tr "repo.settings.payload_optimization"}}

-
- - - {{ctx.Locale.Tr "repo.settings.exclude_files_desc"}} -
+ + + {{ctx.Locale.Tr "repo.settings.exclude_files_limit_desc"}}
-
- - - {{ctx.Locale.Tr "repo.settings.exclude_commits_desc"}} -
+ + + {{ctx.Locale.Tr "repo.settings.exclude_commits_limit_desc"}}
From 9682610d9adbe6a9d4d4951e24cfd7c0c6af16ec Mon Sep 17 00:00:00 2001 From: kerwin612 Date: Thu, 24 Jul 2025 21:46:10 +0800 Subject: [PATCH 3/6] fix --- .../{v1_24/v321.go => v1_25/v322.go} | 6 +- models/webhook/webhook.go | 4 +- options/locale/locale_en-US.ini | 6 +- services/forms/repo_form.go | 4 +- services/webhook/notifier.go | 67 ++++++++++---- services/webhook/webhook_test.go | 87 +++++++++++-------- templates/repo/settings/webhook/settings.tmpl | 4 +- 7 files changed, 110 insertions(+), 68 deletions(-) rename models/migrations/{v1_24/v321.go => v1_25/v322.go} (92%) diff --git a/models/migrations/v1_24/v321.go b/models/migrations/v1_25/v322.go similarity index 92% rename from models/migrations/v1_24/v321.go rename to models/migrations/v1_25/v322.go index 748ee6b0149d0..b1888e7c681fa 100644 --- a/models/migrations/v1_24/v321.go +++ b/models/migrations/v1_25/v322.go @@ -1,7 +1,7 @@ // Copyright 2025 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -package v1_24 +package v1_25 import ( "xorm.io/xorm" @@ -9,8 +9,8 @@ import ( func AddWebhookPayloadOptimizationColumns(x *xorm.Engine) error { type Webhook struct { - ExcludeFilesLimit int `xorm:"exclude_files_limit NOT NULL DEFAULT 0"` - ExcludeCommitsLimit int `xorm:"exclude_commits_limit NOT NULL DEFAULT 0"` + ExcludeFilesLimit int `xorm:"exclude_files_limit NOT NULL DEFAULT -1"` + ExcludeCommitsLimit int `xorm:"exclude_commits_limit NOT NULL DEFAULT -1"` } _, err := x.SyncWithOptions( xorm.SyncOptions{ diff --git a/models/webhook/webhook.go b/models/webhook/webhook.go index 03bd411f1e39a..e78a81b7dfd6a 100644 --- a/models/webhook/webhook.go +++ b/models/webhook/webhook.go @@ -140,8 +140,8 @@ type Webhook struct { HeaderAuthorizationEncrypted string `xorm:"TEXT"` // Payload size optimization options - ExcludeFilesLimit int `xorm:"exclude_files_limit"` // Limit number of file changes in commit payloads, 0 means unlimited - ExcludeCommitsLimit int `xorm:"exclude_commits_limit"` // Limit number of commits in push payloads, 0 means unlimited + ExcludeFilesLimit int `xorm:"exclude_files_limit"` // -1: do not trim, 0: trim all (none kept), >0: keep N file changes in commit payloads + ExcludeCommitsLimit int `xorm:"exclude_commits_limit"` // -1: do not trim, 0: trim all (none kept), >0: keep N commits in push payloads CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 3c296c7a8b459..7fb88791be09b 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2426,9 +2426,9 @@ settings.branch_filter = Branch filter settings.branch_filter_desc = Branch whitelist for push, branch creation and branch deletion events, specified as glob pattern. If empty or *, events for all branches are reported. See %[2]s documentation for syntax. Examples: master, {master,release*}. settings.payload_optimization = Payload Size Optimization settings.exclude_files_limit = Limit file changes -settings.exclude_files_limit_desc = Limit the number of file changes (added, removed, modified) included in each commit payload. 0 means unlimited. +settings.exclude_files_limit_desc = -1: do not trim, 0: trim all (none kept), >0: keep N file changes settings.exclude_commits_limit = Limit commits -settings.exclude_commits_limit_desc = Limit the number of commits included in push payloads. 0 means unlimited. +settings.exclude_commits_limit_desc = -1: do not trim, 0: trim all (none kept), >0: keep N commits settings.authorization_header = Authorization Header settings.authorization_header_desc = Will be included as authorization header for requests when present. Examples: %s. settings.active = Active @@ -3287,7 +3287,7 @@ auths.tip.github = Register a new OAuth application on %s auths.tip.gitlab_new = Register a new application on %s auths.tip.google_plus = Obtain OAuth2 client credentials from the Google API console at %s auths.tip.openid_connect = Use the OpenID Connect Discovery URL "https://{server}/.well-known/openid-configuration" to specify the endpoints -auths.tip.twitter = Go to %s, create an application and ensure that the “Allow this application to be used to Sign in with Twitter” option is enabled +auths.tip.twitter = Go to %s, create an application and ensure that the "Allow this application to be used to Sign in with Twitter" option is enabled auths.tip.discord = Register a new application on %s auths.tip.gitea = Register a new OAuth2 application. Guide can be found at %s auths.tip.yandex = Create a new application at %s. Select following permissions from the "Yandex.Passport API" section: "Access to email address", "Access to user avatar" and "Access to username, first name and surname, gender" diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index e9c58bae6c7aa..7d2cc58300ce3 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -240,8 +240,8 @@ type WebhookForm struct { AuthorizationHeader string Secret string // Payload size optimization options - ExcludeFilesLimit int // Limit number of file changes in commit payloads, 0 means unlimited - ExcludeCommitsLimit int // Limit number of commits in push payloads, 0 means unlimited + ExcludeFilesLimit int // -1: do not trim, 0: trim all (none kept), >0: keep N file changes in commit payloads + ExcludeCommitsLimit int // -1: do not trim, 0: trim all (none kept), >0: keep N commits in push payloads } // PushOnly if the hook will be triggered when push diff --git a/services/webhook/notifier.go b/services/webhook/notifier.go index 5c3fc4996b1b4..16f9844be5808 100644 --- a/services/webhook/notifier.go +++ b/services/webhook/notifier.go @@ -657,47 +657,76 @@ func (m *webhookNotifier) applyWebhookPayloadOptimizations(ctx context.Context, } // Check if any webhook has payload optimization options enabled - hasFilesLimit := 0 - hasCommitsLimit := 0 + hasFilesLimit := -1 + hasCommitsLimit := -1 for _, webhook := range webhooks { if webhook.HasEvent(webhook_module.HookEventPush) { - if webhook.ExcludeFilesLimit > 0 && (hasFilesLimit == 0 || webhook.ExcludeFilesLimit < hasFilesLimit) { + if webhook.ExcludeFilesLimit >= 0 && (hasFilesLimit == -1 || webhook.ExcludeFilesLimit < hasFilesLimit) { hasFilesLimit = webhook.ExcludeFilesLimit } - if webhook.ExcludeCommitsLimit > 0 && (hasCommitsLimit == 0 || webhook.ExcludeCommitsLimit < hasCommitsLimit) { + if webhook.ExcludeCommitsLimit >= 0 && (hasCommitsLimit == -1 || webhook.ExcludeCommitsLimit < hasCommitsLimit) { hasCommitsLimit = webhook.ExcludeCommitsLimit } } } // Apply payload optimizations based on webhook configurations - if hasFilesLimit > 0 { + // -1 not trim, 0 trim all (none kept), >0 trim to N commits + if hasFilesLimit != -1 { for _, commit := range apiCommits { - if commit.Added != nil && len(commit.Added) > hasFilesLimit { - commit.Added = commit.Added[:hasFilesLimit] + if commit.Added != nil { + if hasFilesLimit == 0 { + commit.Added = nil + } else if hasFilesLimit > 0 && len(commit.Added) > hasFilesLimit { + commit.Added = commit.Added[:hasFilesLimit] + } } - if commit.Removed != nil && len(commit.Removed) > hasFilesLimit { - commit.Removed = commit.Removed[:hasFilesLimit] + if commit.Removed != nil { + if hasFilesLimit == 0 { + commit.Removed = nil + } else if hasFilesLimit > 0 && len(commit.Removed) > hasFilesLimit { + commit.Removed = commit.Removed[:hasFilesLimit] + } } - if commit.Modified != nil && len(commit.Modified) > hasFilesLimit { - commit.Modified = commit.Modified[:hasFilesLimit] + if commit.Modified != nil { + if hasFilesLimit == 0 { + commit.Modified = nil + } else if hasFilesLimit > 0 && len(commit.Modified) > hasFilesLimit { + commit.Modified = commit.Modified[:hasFilesLimit] + } } } if apiHeadCommit != nil { - if apiHeadCommit.Added != nil && len(apiHeadCommit.Added) > hasFilesLimit { - apiHeadCommit.Added = apiHeadCommit.Added[:hasFilesLimit] + if apiHeadCommit.Added != nil { + if hasFilesLimit == 0 { + apiHeadCommit.Added = nil + } else if hasFilesLimit > 0 && len(apiHeadCommit.Added) > hasFilesLimit { + apiHeadCommit.Added = apiHeadCommit.Added[:hasFilesLimit] + } } - if apiHeadCommit.Removed != nil && len(apiHeadCommit.Removed) > hasFilesLimit { - apiHeadCommit.Removed = apiHeadCommit.Removed[:hasFilesLimit] + if apiHeadCommit.Removed != nil { + if hasFilesLimit == 0 { + apiHeadCommit.Removed = nil + } else if hasFilesLimit > 0 && len(apiHeadCommit.Removed) > hasFilesLimit { + apiHeadCommit.Removed = apiHeadCommit.Removed[:hasFilesLimit] + } } - if apiHeadCommit.Modified != nil && len(apiHeadCommit.Modified) > hasFilesLimit { - apiHeadCommit.Modified = apiHeadCommit.Modified[:hasFilesLimit] + if apiHeadCommit.Modified != nil { + if hasFilesLimit == 0 { + apiHeadCommit.Modified = nil + } else if hasFilesLimit > 0 && len(apiHeadCommit.Modified) > hasFilesLimit { + apiHeadCommit.Modified = apiHeadCommit.Modified[:hasFilesLimit] + } } } } - if hasCommitsLimit > 0 && len(apiCommits) > hasCommitsLimit { - apiCommits = apiCommits[:hasCommitsLimit] + if hasCommitsLimit != -1 { + if hasCommitsLimit == 0 { + apiCommits = nil + } else if hasCommitsLimit > 0 && len(apiCommits) > hasCommitsLimit { + apiCommits = apiCommits[:hasCommitsLimit] + } } return apiCommits, apiHeadCommit diff --git a/services/webhook/webhook_test.go b/services/webhook/webhook_test.go index bc9501cbcfa8d..dd5497c62c7d4 100644 --- a/services/webhook/webhook_test.go +++ b/services/webhook/webhook_test.go @@ -98,10 +98,17 @@ func TestWebhookPayloadOptimization(t *testing.T) { var optimizedCommits []*api.PayloadCommit var optimizedHeadCommit *api.PayloadCommit - // Create a test repository repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - // Create a webhook with file limit = 1 + // Clean up all webhooks for this repo to avoid interference + webhooks, err := db.Find[webhook_model.Webhook](db.DefaultContext, webhook_model.ListWebhookOptions{RepoID: repo.ID}) + assert.NoError(t, err) + for _, wh := range webhooks { + err = webhook_model.DeleteWebhookByID(db.DefaultContext, wh.ID) + assert.NoError(t, err) + } + + // Case: -1 (no trimming) webhook := &webhook_model.Webhook{ RepoID: repo.ID, URL: "http://example.com/webhook", @@ -110,21 +117,19 @@ func TestWebhookPayloadOptimization(t *testing.T) { Secret: "secret", IsActive: true, Type: webhook_module.GITEA, - ExcludeFilesLimit: 1, - ExcludeCommitsLimit: 0, + ExcludeFilesLimit: -1, + ExcludeCommitsLimit: -1, HookEvent: &webhook_module.HookEvent{ PushOnly: true, }, } - err := webhook.UpdateEvent() + err = webhook.UpdateEvent() assert.NoError(t, err) err = webhook_model.CreateWebhook(db.DefaultContext, webhook) assert.NoError(t, err) assert.NotZero(t, webhook.ID) - // Test payload optimization: should truncate to 1 file per field - notifier := &webhookNotifier{} apiCommits := []*api.PayloadCommit{ { ID: "abc123", @@ -148,22 +153,24 @@ func TestWebhookPayloadOptimization(t *testing.T) { Removed: []string{}, Modified: []string{"file1.txt"}, } - optimizedCommits, _ = notifier.applyWebhookPayloadOptimizations(db.DefaultContext, repo, apiCommits, apiHeadCommit) - assert.Equal(t, []string{"file1.txt"}, optimizedCommits[0].Added) - assert.Equal(t, []string{"oldfile.txt"}, optimizedCommits[0].Removed) - assert.Equal(t, []string{"modified.txt"}, optimizedCommits[0].Modified) - assert.Equal(t, []string{"file3.txt"}, optimizedCommits[1].Added) - assert.Equal(t, []string{}, optimizedCommits[1].Removed) - assert.Equal(t, []string{"file1.txt"}, optimizedCommits[1].Modified) - - _, optimizedHeadCommit = notifier.applyWebhookPayloadOptimizations(db.DefaultContext, repo, apiCommits, apiHeadCommit) - assert.Equal(t, []string{"file3.txt"}, optimizedHeadCommit.Added) - assert.Equal(t, []string{}, optimizedHeadCommit.Removed) - assert.Equal(t, []string{"file1.txt"}, optimizedHeadCommit.Modified) + optimizedCommits, optimizedHeadCommit = (&webhookNotifier{}).applyWebhookPayloadOptimizations(db.DefaultContext, repo, apiCommits, apiHeadCommit) + if assert.NotNil(t, optimizedCommits) && len(optimizedCommits) == 2 { + assert.Equal(t, []string{"file1.txt", "file2.txt"}, optimizedCommits[0].Added) + assert.Equal(t, []string{"oldfile.txt"}, optimizedCommits[0].Removed) + assert.Equal(t, []string{"modified.txt"}, optimizedCommits[0].Modified) + assert.Equal(t, []string{"file3.txt"}, optimizedCommits[1].Added) + assert.Equal(t, []string{}, optimizedCommits[1].Removed) + assert.Equal(t, []string{"file1.txt"}, optimizedCommits[1].Modified) + } + if assert.NotNil(t, optimizedHeadCommit) { + assert.Equal(t, []string{"file3.txt"}, optimizedHeadCommit.Added) + assert.Equal(t, []string{}, optimizedHeadCommit.Removed) + assert.Equal(t, []string{"file1.txt"}, optimizedHeadCommit.Modified) + } - // Test with commit limit = 1 + // Case: 0 (keep nothing) webhook.ExcludeFilesLimit = 0 - webhook.ExcludeCommitsLimit = 1 + webhook.ExcludeCommitsLimit = 0 err = webhook_model.UpdateWebhook(db.DefaultContext, webhook) assert.NoError(t, err) apiCommits = []*api.PayloadCommit{ @@ -189,13 +196,17 @@ func TestWebhookPayloadOptimization(t *testing.T) { Removed: []string{}, Modified: []string{"file1.txt"}, } - optimizedCommits, _ = notifier.applyWebhookPayloadOptimizations(db.DefaultContext, repo, apiCommits, apiHeadCommit) - assert.Len(t, optimizedCommits, 1) - assert.Equal(t, "abc123", optimizedCommits[0].ID) + optimizedCommits, optimizedHeadCommit = (&webhookNotifier{}).applyWebhookPayloadOptimizations(db.DefaultContext, repo, apiCommits, apiHeadCommit) + assert.Nil(t, optimizedCommits) + if assert.NotNil(t, optimizedHeadCommit) { + assert.Nil(t, optimizedHeadCommit.Added) + assert.Nil(t, optimizedHeadCommit.Removed) + assert.Nil(t, optimizedHeadCommit.Modified) + } - // Test with no limits (0 means unlimited) - webhook.ExcludeFilesLimit = 0 - webhook.ExcludeCommitsLimit = 0 + // Case: 1 (keep only 1) + webhook.ExcludeFilesLimit = 1 + webhook.ExcludeCommitsLimit = 1 err = webhook_model.UpdateWebhook(db.DefaultContext, webhook) assert.NoError(t, err) apiCommits = []*api.PayloadCommit{ @@ -221,14 +232,16 @@ func TestWebhookPayloadOptimization(t *testing.T) { Removed: []string{}, Modified: []string{"file1.txt"}, } - optimizedCommits, optimizedHeadCommit = notifier.applyWebhookPayloadOptimizations(db.DefaultContext, repo, apiCommits, apiHeadCommit) - assert.Equal(t, []string{"file1.txt", "file2.txt"}, optimizedCommits[0].Added) - assert.Equal(t, []string{"oldfile.txt"}, optimizedCommits[0].Removed) - assert.Equal(t, []string{"modified.txt"}, optimizedCommits[0].Modified) - assert.Equal(t, []string{"file3.txt"}, optimizedCommits[1].Added) - assert.Equal(t, []string{}, optimizedCommits[1].Removed) - assert.Equal(t, []string{"file1.txt"}, optimizedCommits[1].Modified) - assert.Equal(t, []string{"file3.txt"}, optimizedHeadCommit.Added) - assert.Equal(t, []string{}, optimizedHeadCommit.Removed) - assert.Equal(t, []string{"file1.txt"}, optimizedHeadCommit.Modified) + optimizedCommits, optimizedHeadCommit = (&webhookNotifier{}).applyWebhookPayloadOptimizations(db.DefaultContext, repo, apiCommits, apiHeadCommit) + if assert.NotNil(t, optimizedCommits) && len(optimizedCommits) == 1 { + assert.Equal(t, "abc123", optimizedCommits[0].ID) + assert.Equal(t, []string{"file1.txt"}, optimizedCommits[0].Added) + assert.Equal(t, []string{"oldfile.txt"}, optimizedCommits[0].Removed) + assert.Equal(t, []string{"modified.txt"}, optimizedCommits[0].Modified) + } + if assert.NotNil(t, optimizedHeadCommit) { + assert.Equal(t, []string{"file3.txt"}, optimizedHeadCommit.Added) + assert.Equal(t, []string{}, optimizedHeadCommit.Removed) + assert.Equal(t, []string{"file1.txt"}, optimizedHeadCommit.Modified) + } } diff --git a/templates/repo/settings/webhook/settings.tmpl b/templates/repo/settings/webhook/settings.tmpl index ef93f34f1b1a2..0f423c7963e1a 100644 --- a/templates/repo/settings/webhook/settings.tmpl +++ b/templates/repo/settings/webhook/settings.tmpl @@ -52,12 +52,12 @@

{{ctx.Locale.Tr "repo.settings.payload_optimization"}}

- + {{ctx.Locale.Tr "repo.settings.exclude_files_limit_desc"}}
- + {{ctx.Locale.Tr "repo.settings.exclude_commits_limit_desc"}}
From 544e44502498a6f0df7880c3f373cb1408c96d60 Mon Sep 17 00:00:00 2001 From: kerwin612 Date: Wed, 30 Jul 2025 12:16:15 +0800 Subject: [PATCH 4/6] refactor: swap webhook payload optimization logic (-1/0 values) --- models/migrations/v1_25/v322.go | 4 +-- models/webhook/webhook.go | 4 +-- options/locale/locale_en-US.ini | 4 +-- services/forms/repo_form.go | 4 +-- services/webhook/notifier.go | 28 +++++++++---------- services/webhook/webhook_test.go | 12 ++++---- templates/repo/settings/webhook/settings.tmpl | 4 +-- 7 files changed, 30 insertions(+), 30 deletions(-) diff --git a/models/migrations/v1_25/v322.go b/models/migrations/v1_25/v322.go index b1888e7c681fa..430d52750fac4 100644 --- a/models/migrations/v1_25/v322.go +++ b/models/migrations/v1_25/v322.go @@ -9,8 +9,8 @@ import ( func AddWebhookPayloadOptimizationColumns(x *xorm.Engine) error { type Webhook struct { - ExcludeFilesLimit int `xorm:"exclude_files_limit NOT NULL DEFAULT -1"` - ExcludeCommitsLimit int `xorm:"exclude_commits_limit NOT NULL DEFAULT -1"` + ExcludeFilesLimit int `xorm:"exclude_files_limit NOT NULL DEFAULT 0"` + ExcludeCommitsLimit int `xorm:"exclude_commits_limit NOT NULL DEFAULT 0"` } _, err := x.SyncWithOptions( xorm.SyncOptions{ diff --git a/models/webhook/webhook.go b/models/webhook/webhook.go index e78a81b7dfd6a..e4a1f675000a6 100644 --- a/models/webhook/webhook.go +++ b/models/webhook/webhook.go @@ -140,8 +140,8 @@ type Webhook struct { HeaderAuthorizationEncrypted string `xorm:"TEXT"` // Payload size optimization options - ExcludeFilesLimit int `xorm:"exclude_files_limit"` // -1: do not trim, 0: trim all (none kept), >0: keep N file changes in commit payloads - ExcludeCommitsLimit int `xorm:"exclude_commits_limit"` // -1: do not trim, 0: trim all (none kept), >0: keep N commits in push payloads + ExcludeFilesLimit int `xorm:"exclude_files_limit"` // -1: trim all (none kept), 0: do not trim, >0: keep N file changes in commit payloads + ExcludeCommitsLimit int `xorm:"exclude_commits_limit"` // -1: trim all (none kept), 0: do not trim, >0: keep N commits in push payloads CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 7fb88791be09b..a6b3e57164885 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2426,9 +2426,9 @@ settings.branch_filter = Branch filter settings.branch_filter_desc = Branch whitelist for push, branch creation and branch deletion events, specified as glob pattern. If empty or *, events for all branches are reported. See %[2]s documentation for syntax. Examples: master, {master,release*}. settings.payload_optimization = Payload Size Optimization settings.exclude_files_limit = Limit file changes -settings.exclude_files_limit_desc = -1: do not trim, 0: trim all (none kept), >0: keep N file changes +settings.exclude_files_limit_desc = -1: trim all (none kept), 0: do not trim, >0: keep N file changes settings.exclude_commits_limit = Limit commits -settings.exclude_commits_limit_desc = -1: do not trim, 0: trim all (none kept), >0: keep N commits +settings.exclude_commits_limit_desc = -1: trim all (none kept), 0: do not trim, >0: keep N commits settings.authorization_header = Authorization Header settings.authorization_header_desc = Will be included as authorization header for requests when present. Examples: %s. settings.active = Active diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 7d2cc58300ce3..fe96e261506aa 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -240,8 +240,8 @@ type WebhookForm struct { AuthorizationHeader string Secret string // Payload size optimization options - ExcludeFilesLimit int // -1: do not trim, 0: trim all (none kept), >0: keep N file changes in commit payloads - ExcludeCommitsLimit int // -1: do not trim, 0: trim all (none kept), >0: keep N commits in push payloads + ExcludeFilesLimit int // -1: trim all (none kept), 0: do not trim, >0: keep N file changes in commit payloads + ExcludeCommitsLimit int // -1: trim all (none kept), 0: do not trim, >0: keep N commits in push payloads } // PushOnly if the hook will be triggered when push diff --git a/services/webhook/notifier.go b/services/webhook/notifier.go index 16f9844be5808..af9d3e17a791d 100644 --- a/services/webhook/notifier.go +++ b/services/webhook/notifier.go @@ -657,39 +657,39 @@ func (m *webhookNotifier) applyWebhookPayloadOptimizations(ctx context.Context, } // Check if any webhook has payload optimization options enabled - hasFilesLimit := -1 - hasCommitsLimit := -1 + hasFilesLimit := 0 + hasCommitsLimit := 0 for _, webhook := range webhooks { if webhook.HasEvent(webhook_module.HookEventPush) { - if webhook.ExcludeFilesLimit >= 0 && (hasFilesLimit == -1 || webhook.ExcludeFilesLimit < hasFilesLimit) { + if webhook.ExcludeFilesLimit != 0 && (hasFilesLimit == 0 || webhook.ExcludeFilesLimit < hasFilesLimit) { hasFilesLimit = webhook.ExcludeFilesLimit } - if webhook.ExcludeCommitsLimit >= 0 && (hasCommitsLimit == -1 || webhook.ExcludeCommitsLimit < hasCommitsLimit) { + if webhook.ExcludeCommitsLimit != 0 && (hasCommitsLimit == 0 || webhook.ExcludeCommitsLimit < hasCommitsLimit) { hasCommitsLimit = webhook.ExcludeCommitsLimit } } } // Apply payload optimizations based on webhook configurations - // -1 not trim, 0 trim all (none kept), >0 trim to N commits - if hasFilesLimit != -1 { + // -1 trim all (none kept), 0 do not trim, >0 trim to N commits + if hasFilesLimit != 0 { for _, commit := range apiCommits { if commit.Added != nil { - if hasFilesLimit == 0 { + if hasFilesLimit == -1 { commit.Added = nil } else if hasFilesLimit > 0 && len(commit.Added) > hasFilesLimit { commit.Added = commit.Added[:hasFilesLimit] } } if commit.Removed != nil { - if hasFilesLimit == 0 { + if hasFilesLimit == -1 { commit.Removed = nil } else if hasFilesLimit > 0 && len(commit.Removed) > hasFilesLimit { commit.Removed = commit.Removed[:hasFilesLimit] } } if commit.Modified != nil { - if hasFilesLimit == 0 { + if hasFilesLimit == -1 { commit.Modified = nil } else if hasFilesLimit > 0 && len(commit.Modified) > hasFilesLimit { commit.Modified = commit.Modified[:hasFilesLimit] @@ -698,21 +698,21 @@ func (m *webhookNotifier) applyWebhookPayloadOptimizations(ctx context.Context, } if apiHeadCommit != nil { if apiHeadCommit.Added != nil { - if hasFilesLimit == 0 { + if hasFilesLimit == -1 { apiHeadCommit.Added = nil } else if hasFilesLimit > 0 && len(apiHeadCommit.Added) > hasFilesLimit { apiHeadCommit.Added = apiHeadCommit.Added[:hasFilesLimit] } } if apiHeadCommit.Removed != nil { - if hasFilesLimit == 0 { + if hasFilesLimit == -1 { apiHeadCommit.Removed = nil } else if hasFilesLimit > 0 && len(apiHeadCommit.Removed) > hasFilesLimit { apiHeadCommit.Removed = apiHeadCommit.Removed[:hasFilesLimit] } } if apiHeadCommit.Modified != nil { - if hasFilesLimit == 0 { + if hasFilesLimit == -1 { apiHeadCommit.Modified = nil } else if hasFilesLimit > 0 && len(apiHeadCommit.Modified) > hasFilesLimit { apiHeadCommit.Modified = apiHeadCommit.Modified[:hasFilesLimit] @@ -721,8 +721,8 @@ func (m *webhookNotifier) applyWebhookPayloadOptimizations(ctx context.Context, } } - if hasCommitsLimit != -1 { - if hasCommitsLimit == 0 { + if hasCommitsLimit != 0 { + if hasCommitsLimit == -1 { apiCommits = nil } else if hasCommitsLimit > 0 && len(apiCommits) > hasCommitsLimit { apiCommits = apiCommits[:hasCommitsLimit] diff --git a/services/webhook/webhook_test.go b/services/webhook/webhook_test.go index dd5497c62c7d4..35ebc0dc06b85 100644 --- a/services/webhook/webhook_test.go +++ b/services/webhook/webhook_test.go @@ -108,7 +108,7 @@ func TestWebhookPayloadOptimization(t *testing.T) { assert.NoError(t, err) } - // Case: -1 (no trimming) + // Case: 0 (no trimming) webhook := &webhook_model.Webhook{ RepoID: repo.ID, URL: "http://example.com/webhook", @@ -117,8 +117,8 @@ func TestWebhookPayloadOptimization(t *testing.T) { Secret: "secret", IsActive: true, Type: webhook_module.GITEA, - ExcludeFilesLimit: -1, - ExcludeCommitsLimit: -1, + ExcludeFilesLimit: 0, + ExcludeCommitsLimit: 0, HookEvent: &webhook_module.HookEvent{ PushOnly: true, }, @@ -168,9 +168,9 @@ func TestWebhookPayloadOptimization(t *testing.T) { assert.Equal(t, []string{"file1.txt"}, optimizedHeadCommit.Modified) } - // Case: 0 (keep nothing) - webhook.ExcludeFilesLimit = 0 - webhook.ExcludeCommitsLimit = 0 + // Case: -1 (trim all) + webhook.ExcludeFilesLimit = -1 + webhook.ExcludeCommitsLimit = -1 err = webhook_model.UpdateWebhook(db.DefaultContext, webhook) assert.NoError(t, err) apiCommits = []*api.PayloadCommit{ diff --git a/templates/repo/settings/webhook/settings.tmpl b/templates/repo/settings/webhook/settings.tmpl index 0f423c7963e1a..1dd5d99a5b84f 100644 --- a/templates/repo/settings/webhook/settings.tmpl +++ b/templates/repo/settings/webhook/settings.tmpl @@ -52,12 +52,12 @@

{{ctx.Locale.Tr "repo.settings.payload_optimization"}}

- + {{ctx.Locale.Tr "repo.settings.exclude_files_limit_desc"}}
- + {{ctx.Locale.Tr "repo.settings.exclude_commits_limit_desc"}}
From 6e07c6f3384f52decdbb7f2a96607ce4c36d1382 Mon Sep 17 00:00:00 2001 From: kerwin612 Date: Wed, 30 Jul 2025 20:48:10 +0800 Subject: [PATCH 5/6] add webhook payload optimization API support --- modules/structs/hook.go | 11 ++++- routers/api/v1/utils/hook.go | 14 ++++++- services/webhook/general.go | 2 + tests/integration/repo_webhook_test.go | 56 ++++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 3 deletions(-) diff --git a/modules/structs/hook.go b/modules/structs/hook.go index ac779a5740748..c9270de09a8bb 100644 --- a/modules/structs/hook.go +++ b/modules/structs/hook.go @@ -25,6 +25,9 @@ type Hook struct { Events []string `json:"events"` AuthorizationHeader string `json:"authorization_header"` Active bool `json:"active"` + // Payload size optimization options + ExcludeFilesLimit int `json:"exclude_files_limit"` // -1: trim all (none kept), 0: do not trim, >0: keep N file changes + ExcludeCommitsLimit int `json:"exclude_commits_limit"` // -1: trim all (none kept), 0: do not trim, >0: keep N commits // swagger:strfmt date-time Updated time.Time `json:"updated_at"` // swagger:strfmt date-time @@ -48,6 +51,9 @@ type CreateHookOption struct { Events []string `json:"events"` BranchFilter string `json:"branch_filter" binding:"GlobPattern"` AuthorizationHeader string `json:"authorization_header"` + // Payload size optimization options + ExcludeFilesLimit int `json:"exclude_files_limit"` // -1: trim all (none kept), 0: do not trim, >0: keep N file changes + ExcludeCommitsLimit int `json:"exclude_commits_limit"` // -1: trim all (none kept), 0: do not trim, >0: keep N commits // default: false Active bool `json:"active"` } @@ -58,7 +64,10 @@ type EditHookOption struct { Events []string `json:"events"` BranchFilter string `json:"branch_filter" binding:"GlobPattern"` AuthorizationHeader string `json:"authorization_header"` - Active *bool `json:"active"` + // Payload size optimization options + ExcludeFilesLimit *int `json:"exclude_files_limit"` // -1: trim all (none kept), 0: do not trim, >0: keep N file changes + ExcludeCommitsLimit *int `json:"exclude_commits_limit"` // -1: trim all (none kept), 0: do not trim, >0: keep N commits + Active *bool `json:"active"` } // Payloader payload is some part of one hook diff --git a/routers/api/v1/utils/hook.go b/routers/api/v1/utils/hook.go index 6f598f14c8b2b..cfbe142b690a6 100644 --- a/routers/api/v1/utils/hook.go +++ b/routers/api/v1/utils/hook.go @@ -224,8 +224,10 @@ func addHook(ctx *context.APIContext, form *api.CreateHookOption, ownerID, repoI HookEvents: updateHookEvents(form.Events), BranchFilter: form.BranchFilter, }, - IsActive: form.Active, - Type: form.Type, + IsActive: form.Active, + Type: form.Type, + ExcludeFilesLimit: form.ExcludeFilesLimit, + ExcludeCommitsLimit: form.ExcludeCommitsLimit, } err := w.SetHeaderAuthorization(form.AuthorizationHeader) if err != nil { @@ -391,6 +393,14 @@ func editHook(ctx *context.APIContext, form *api.EditHookOption, w *webhook.Webh w.IsActive = *form.Active } + if form.ExcludeFilesLimit != nil { + w.ExcludeFilesLimit = *form.ExcludeFilesLimit + } + + if form.ExcludeCommitsLimit != nil { + w.ExcludeCommitsLimit = *form.ExcludeCommitsLimit + } + if err := webhook.UpdateWebhook(ctx, w); err != nil { ctx.APIErrorInternal(err) return false diff --git a/services/webhook/general.go b/services/webhook/general.go index be457e46f5f7a..08c03b4885517 100644 --- a/services/webhook/general.go +++ b/services/webhook/general.go @@ -417,6 +417,8 @@ func ToHook(repoLink string, w *webhook_model.Webhook) (*api.Hook, error) { Config: config, Events: w.EventsArray(), AuthorizationHeader: authorizationHeader, + ExcludeFilesLimit: w.ExcludeFilesLimit, + ExcludeCommitsLimit: w.ExcludeCommitsLimit, Updated: w.UpdatedUnix.AsTime(), Created: w.CreatedUnix.AsTime(), BranchFilter: w.BranchFilter, diff --git a/tests/integration/repo_webhook_test.go b/tests/integration/repo_webhook_test.go index 1da7bc9d3c80f..004e063ee6501 100644 --- a/tests/integration/repo_webhook_test.go +++ b/tests/integration/repo_webhook_test.go @@ -1222,3 +1222,59 @@ jobs: assert.Equal(t, "user2/repo1", webhookData.payloads[i].Repo.FullName) } } + +func Test_WebhookPayloadOptimizationAPI(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + session := loginUser(t, "user2") + + // Test creating webhook with payload optimization options via API + createHookOption := map[string]any{ + "type": "gitea", + "config": map[string]string{ + "url": "http://example.com/webhook", + "content_type": "json", + }, + "events": []string{"push"}, + "exclude_files_limit": 5, + "exclude_commits_limit": 10, + "active": true, + } + + req := NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo1/hooks", createHookOption) + resp := session.MakeRequest(t, req, http.StatusCreated) + + var hook api.Hook + DecodeJSON(t, resp, &hook) + + // Verify the webhook was created with correct payload optimization settings + assert.Equal(t, 5, hook.ExcludeFilesLimit) + assert.Equal(t, 10, hook.ExcludeCommitsLimit) + + // Test updating webhook with different payload optimization options + editHookOption := map[string]any{ + "exclude_files_limit": -1, + "exclude_commits_limit": 0, + } + + req = NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/repos/user2/repo1/hooks/%d", hook.ID), editHookOption) + resp = session.MakeRequest(t, req, http.StatusOK) + + var updatedHook api.Hook + DecodeJSON(t, resp, &updatedHook) + + // Verify the webhook was updated with correct payload optimization settings + assert.Equal(t, -1, updatedHook.ExcludeFilesLimit) + assert.Equal(t, 0, updatedHook.ExcludeCommitsLimit) + + // Test getting webhook to verify the settings are persisted + req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/user2/repo1/hooks/%d", hook.ID)) + resp = session.MakeRequest(t, req, http.StatusOK) + + var retrievedHook api.Hook + DecodeJSON(t, resp, &retrievedHook) + + // Verify the webhook settings are correctly retrieved + assert.Equal(t, -1, retrievedHook.ExcludeFilesLimit) + assert.Equal(t, 0, retrievedHook.ExcludeCommitsLimit) + }) +} From b718c54e7c4965b6ff2a44f9562182260c0d890f Mon Sep 17 00:00:00 2001 From: kerwin612 Date: Mon, 4 Aug 2025 14:32:22 +0800 Subject: [PATCH 6/6] refactor: replace webhook payload optimization with JSON-based configuration --- models/migrations/migrations.go | 2 +- models/migrations/v1_25/v322.go | 3 +- models/webhook/webhook.go | 95 ++++++++++- models/webhook/webhook_test.go | 70 ++++---- modules/structs/hook.go | 13 +- options/locale/locale_en-US.ini | 10 +- routers/api/v1/utils/hook.go | 100 ++++++++++-- routers/web/repo/setting/webhook.go | 59 +++++-- services/forms/repo_form.go | 8 +- services/webhook/general.go | 16 +- services/webhook/notifier.go | 149 +++++++++++------- services/webhook/webhook_test.go | 74 +++++---- templates/repo/settings/webhook/settings.tmpl | 41 ++++- templates/swagger/v1_json.tmpl | 18 +++ tests/integration/repo_webhook_test.go | 62 ++++++-- 15 files changed, 530 insertions(+), 190 deletions(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index bdc122888808d..a6b90c0dacc87 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -385,7 +385,7 @@ func prepareMigrationTasks() []*migration { newMigration(320, "Migrate two_factor_policy to login_source table", v1_24.MigrateSkipTwoFactor), // Gitea 1.24.0 ends at database version 321 newMigration(321, "Use LONGTEXT for some columns and fix review_state.updated_files column", v1_25.UseLongTextInSomeColumnsAndFixBugs), - newMigration(322, "Add webhook payload optimization columns", v1_25.AddWebhookPayloadOptimizationColumns), + newMigration(322, "Add webhook payload optimization JSON field", v1_25.AddWebhookPayloadOptimizationColumns), } return preparedMigrations } diff --git a/models/migrations/v1_25/v322.go b/models/migrations/v1_25/v322.go index 430d52750fac4..fc0b3f3ff5136 100644 --- a/models/migrations/v1_25/v322.go +++ b/models/migrations/v1_25/v322.go @@ -9,8 +9,7 @@ import ( func AddWebhookPayloadOptimizationColumns(x *xorm.Engine) error { type Webhook struct { - ExcludeFilesLimit int `xorm:"exclude_files_limit NOT NULL DEFAULT 0"` - ExcludeCommitsLimit int `xorm:"exclude_commits_limit NOT NULL DEFAULT 0"` + PayloadOptimization string `xorm:"payload_optimization TEXT"` } _, err := x.SyncWithOptions( xorm.SyncOptions{ diff --git a/models/webhook/webhook.go b/models/webhook/webhook.go index e4a1f675000a6..d88b1f4842312 100644 --- a/models/webhook/webhook.go +++ b/models/webhook/webhook.go @@ -22,6 +22,26 @@ import ( "xorm.io/builder" ) +// PayloadOptimizationConfig represents the configuration for webhook payload optimization +type PayloadOptimizationConfig struct { + Files *PayloadOptimizationItem `json:"files,omitempty"` // Files optimization config + Commits *PayloadOptimizationItem `json:"commits,omitempty"` // Commits optimization config +} + +// PayloadOptimizationItem represents a single optimization item configuration +type PayloadOptimizationItem struct { + Enable bool `json:"enable"` // Whether to enable optimization for this item + Limit int `json:"limit"` // 0: trim all (none kept), >0: keep N items (forward order), <0: keep N items (reverse order) +} + +// DefaultPayloadOptimizationConfig returns the default payload optimization configuration +func DefaultPayloadOptimizationConfig() *PayloadOptimizationConfig { + return &PayloadOptimizationConfig{ + Files: &PayloadOptimizationItem{Enable: false, Limit: 0}, + Commits: &PayloadOptimizationItem{Enable: false, Limit: 0}, + } +} + // ErrWebhookNotExist represents a "WebhookNotExist" kind of error. type ErrWebhookNotExist struct { ID int64 @@ -140,8 +160,7 @@ type Webhook struct { HeaderAuthorizationEncrypted string `xorm:"TEXT"` // Payload size optimization options - ExcludeFilesLimit int `xorm:"exclude_files_limit"` // -1: trim all (none kept), 0: do not trim, >0: keep N file changes in commit payloads - ExcludeCommitsLimit int `xorm:"exclude_commits_limit"` // -1: trim all (none kept), 0: do not trim, >0: keep N commits in push payloads + PayloadOptimization string `xorm:"payload_optimization TEXT"` // JSON: {"enable": bool, "limit": int} CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` @@ -350,3 +369,75 @@ func DeleteWebhookByOwnerID(ctx context.Context, ownerID, id int64) error { } return DeleteWebhookByID(ctx, id) } + +// GetPayloadOptimizationConfig returns the payload optimization configuration +func (w *Webhook) GetPayloadOptimizationConfig() *PayloadOptimizationConfig { + if w.PayloadOptimization == "" { + return DefaultPayloadOptimizationConfig() + } + + var config PayloadOptimizationConfig + if err := json.Unmarshal([]byte(w.PayloadOptimization), &config); err != nil { + log.Error("Failed to unmarshal payload optimization config: %v", err) + return DefaultPayloadOptimizationConfig() + } + + return &config +} + +// SetPayloadOptimizationConfig sets the payload optimization configuration +func (w *Webhook) SetPayloadOptimizationConfig(config *PayloadOptimizationConfig) error { + if config == nil { + config = DefaultPayloadOptimizationConfig() + } + + data, err := json.Marshal(config) + if err != nil { + return fmt.Errorf("failed to marshal payload optimization config: %w", err) + } + + w.PayloadOptimization = string(data) + return nil +} + +// IsPayloadOptimizationEnabled returns whether payload optimization is enabled +func (w *Webhook) IsPayloadOptimizationEnabled() bool { + config := w.GetPayloadOptimizationConfig() + return config.Files.Enable || config.Commits.Enable +} + +// GetPayloadOptimizationLimit returns the payload optimization limit +func (w *Webhook) GetPayloadOptimizationLimit() int { + config := w.GetPayloadOptimizationConfig() + if config.Files.Enable { + return config.Files.Limit + } + if config.Commits.Enable { + return config.Commits.Limit + } + return 0 +} + +// IsFilesOptimizationEnabled returns whether files optimization is enabled +func (w *Webhook) IsFilesOptimizationEnabled() bool { + config := w.GetPayloadOptimizationConfig() + return config.Files.Enable +} + +// GetFilesOptimizationLimit returns the files optimization limit +func (w *Webhook) GetFilesOptimizationLimit() int { + config := w.GetPayloadOptimizationConfig() + return config.Files.Limit +} + +// IsCommitsOptimizationEnabled returns whether commits optimization is enabled +func (w *Webhook) IsCommitsOptimizationEnabled() bool { + config := w.GetPayloadOptimizationConfig() + return config.Commits.Enable +} + +// GetCommitsOptimizationLimit returns the commits optimization limit +func (w *Webhook) GetCommitsOptimizationLimit() int { + config := w.GetPayloadOptimizationConfig() + return config.Commits.Limit +} diff --git a/models/webhook/webhook_test.go b/models/webhook/webhook_test.go index db1150772e5cd..7dcfb6de6e454 100644 --- a/models/webhook/webhook_test.go +++ b/models/webhook/webhook_test.go @@ -332,43 +332,39 @@ func TestCleanupHookTaskTable_OlderThan_LeavesTaskEarlierThanAgeToDelete(t *test } func TestWebhookPayloadOptimization(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - webhook := &Webhook{ - RepoID: 1, - URL: "http://example.com/webhook", - HTTPMethod: "POST", - ContentType: ContentTypeJSON, - Secret: "secret", - IsActive: true, - Type: webhook_module.GITEA, - ExcludeFilesLimit: 1, - ExcludeCommitsLimit: 0, - HookEvent: &webhook_module.HookEvent{ - PushOnly: true, + webhook := &Webhook{} + + // Test default configuration + config := webhook.GetPayloadOptimizationConfig() + assert.False(t, config.Files.Enable) + assert.Equal(t, 0, config.Files.Limit) + assert.False(t, config.Commits.Enable) + assert.Equal(t, 0, config.Commits.Limit) + + // Test setting configuration + newConfig := &PayloadOptimizationConfig{ + Files: &PayloadOptimizationItem{ + Enable: true, + Limit: 5, + }, + Commits: &PayloadOptimizationItem{ + Enable: true, + Limit: -3, }, } - - // Test creating webhook with payload optimization options - err := CreateWebhook(db.DefaultContext, webhook) - assert.NoError(t, err) - assert.NotZero(t, webhook.ID) - - // Test retrieving webhook and checking payload optimization options - retrievedWebhook, err := GetWebhookByID(db.DefaultContext, webhook.ID) - assert.NoError(t, err) - assert.Equal(t, 1, retrievedWebhook.ExcludeFilesLimit) - assert.Equal(t, 0, retrievedWebhook.ExcludeCommitsLimit) - - // Test updating webhook with different payload optimization options - retrievedWebhook.ExcludeFilesLimit = 0 - retrievedWebhook.ExcludeCommitsLimit = 2 - err = UpdateWebhook(db.DefaultContext, retrievedWebhook) - assert.NoError(t, err) - - // Verify the update - updatedWebhook, err := GetWebhookByID(db.DefaultContext, webhook.ID) - assert.NoError(t, err) - assert.Equal(t, 0, updatedWebhook.ExcludeFilesLimit) - assert.Equal(t, 2, updatedWebhook.ExcludeCommitsLimit) + webhook.SetPayloadOptimizationConfig(newConfig) + + // Test getting configuration + config = webhook.GetPayloadOptimizationConfig() + assert.True(t, config.Files.Enable) + assert.Equal(t, 5, config.Files.Limit) + assert.True(t, config.Commits.Enable) + assert.Equal(t, -3, config.Commits.Limit) + + // Test individual methods + assert.True(t, webhook.IsFilesOptimizationEnabled()) + assert.Equal(t, 5, webhook.GetFilesOptimizationLimit()) + assert.True(t, webhook.IsCommitsOptimizationEnabled()) + assert.Equal(t, -3, webhook.GetCommitsOptimizationLimit()) + assert.True(t, webhook.IsPayloadOptimizationEnabled()) } diff --git a/modules/structs/hook.go b/modules/structs/hook.go index c9270de09a8bb..55e795ac88d95 100644 --- a/modules/structs/hook.go +++ b/modules/structs/hook.go @@ -25,9 +25,8 @@ type Hook struct { Events []string `json:"events"` AuthorizationHeader string `json:"authorization_header"` Active bool `json:"active"` - // Payload size optimization options - ExcludeFilesLimit int `json:"exclude_files_limit"` // -1: trim all (none kept), 0: do not trim, >0: keep N file changes - ExcludeCommitsLimit int `json:"exclude_commits_limit"` // -1: trim all (none kept), 0: do not trim, >0: keep N commits + // PayloadOptimization configuration for webhook payload optimization + PayloadOptimization map[string]any `json:"payload_optimization"` // swagger:strfmt date-time Updated time.Time `json:"updated_at"` // swagger:strfmt date-time @@ -52,8 +51,7 @@ type CreateHookOption struct { BranchFilter string `json:"branch_filter" binding:"GlobPattern"` AuthorizationHeader string `json:"authorization_header"` // Payload size optimization options - ExcludeFilesLimit int `json:"exclude_files_limit"` // -1: trim all (none kept), 0: do not trim, >0: keep N file changes - ExcludeCommitsLimit int `json:"exclude_commits_limit"` // -1: trim all (none kept), 0: do not trim, >0: keep N commits + PayloadOptimization map[string]any `json:"payload_optimization"` // {"enable": bool, "limit": int} // default: false Active bool `json:"active"` } @@ -65,9 +63,8 @@ type EditHookOption struct { BranchFilter string `json:"branch_filter" binding:"GlobPattern"` AuthorizationHeader string `json:"authorization_header"` // Payload size optimization options - ExcludeFilesLimit *int `json:"exclude_files_limit"` // -1: trim all (none kept), 0: do not trim, >0: keep N file changes - ExcludeCommitsLimit *int `json:"exclude_commits_limit"` // -1: trim all (none kept), 0: do not trim, >0: keep N commits - Active *bool `json:"active"` + PayloadOptimization *map[string]any `json:"payload_optimization"` // {"enable": bool, "limit": int} + Active *bool `json:"active"` } // Payloader payload is some part of one hook diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 607cf688bd3fc..2cd563aefef7a 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2425,10 +2425,12 @@ settings.event_package_desc = Package created or deleted in a repository. settings.branch_filter = Branch filter settings.branch_filter_desc = Branch whitelist for push, branch creation and branch deletion events, specified as glob pattern. If empty or *, events for all branches are reported. See %[2]s documentation for syntax. Examples: master, {master,release*}. settings.payload_optimization = Payload Size Optimization -settings.exclude_files_limit = Limit file changes -settings.exclude_files_limit_desc = -1: trim all (none kept), 0: do not trim, >0: keep N file changes -settings.exclude_commits_limit = Limit commits -settings.exclude_commits_limit_desc = -1: trim all (none kept), 0: do not trim, >0: keep N commits +settings.payload_optimization_files = Files +settings.payload_optimization_commits = Commits +settings.payload_optimization_enable = Enable optimization +settings.payload_optimization_enable_desc = Enable payload size optimization for this item +settings.payload_optimization_limit = Limit +settings.payload_optimization_limit_desc = 0: trim all (none kept), >0: keep N items (forward order), <0: keep N items (reverse order) settings.authorization_header = Authorization Header settings.authorization_header_desc = Will be included as authorization header for requests when present. Examples: %s. settings.active = Active diff --git a/routers/api/v1/utils/hook.go b/routers/api/v1/utils/hook.go index cfbe142b690a6..90ea87d560dc1 100644 --- a/routers/api/v1/utils/hook.go +++ b/routers/api/v1/utils/hook.go @@ -21,6 +21,37 @@ import ( webhook_service "code.gitea.io/gitea/services/webhook" ) +// getBoolFromMap extracts a boolean value from a map with a default fallback +// +//nolint:unparam // defaultValue is needed for generic helper function +func getBoolFromMap(m map[string]any, defaultValue bool) bool { + if val, ok := m["enable"]; ok { + if boolVal, ok := val.(bool); ok { + return boolVal + } + } + return defaultValue +} + +// getIntFromMap extracts an integer value from a map with a default fallback +// +//nolint:unparam // defaultValue is needed for generic helper function +func getIntFromMap(m map[string]any, defaultValue int) int { + if val, ok := m["limit"]; ok { + switch v := val.(type) { + case int: + return v + case float64: + return int(v) + case string: + if intVal, err := strconv.Atoi(v); err == nil { + return intVal + } + } + } + return defaultValue +} + // ListOwnerHooks lists the webhooks of the provided owner func ListOwnerHooks(ctx *context.APIContext, owner *user_model.User) { opts := &webhook.ListWebhookOptions{ @@ -224,11 +255,40 @@ func addHook(ctx *context.APIContext, form *api.CreateHookOption, ownerID, repoI HookEvents: updateHookEvents(form.Events), BranchFilter: form.BranchFilter, }, - IsActive: form.Active, - Type: form.Type, - ExcludeFilesLimit: form.ExcludeFilesLimit, - ExcludeCommitsLimit: form.ExcludeCommitsLimit, + IsActive: form.Active, + Type: form.Type, + } + + // Set payload optimization config + if form.PayloadOptimization != nil { + payloadOptConfig := &webhook.PayloadOptimizationConfig{} + + // Parse files config + if filesConfig, ok := form.PayloadOptimization["files"].(map[string]any); ok { + payloadOptConfig.Files = &webhook.PayloadOptimizationItem{ + Enable: getBoolFromMap(filesConfig, false), + Limit: getIntFromMap(filesConfig, 0), + } + } else { + payloadOptConfig.Files = &webhook.PayloadOptimizationItem{Enable: false, Limit: 0} + } + + // Parse commits config + if commitsConfig, ok := form.PayloadOptimization["commits"].(map[string]any); ok { + payloadOptConfig.Commits = &webhook.PayloadOptimizationItem{ + Enable: getBoolFromMap(commitsConfig, false), + Limit: getIntFromMap(commitsConfig, 0), + } + } else { + payloadOptConfig.Commits = &webhook.PayloadOptimizationItem{Enable: false, Limit: 0} + } + + if err := w.SetPayloadOptimizationConfig(payloadOptConfig); err != nil { + ctx.APIErrorInternal(err) + return nil, false + } } + err := w.SetHeaderAuthorization(form.AuthorizationHeader) if err != nil { ctx.APIErrorInternal(err) @@ -393,12 +453,34 @@ func editHook(ctx *context.APIContext, form *api.EditHookOption, w *webhook.Webh w.IsActive = *form.Active } - if form.ExcludeFilesLimit != nil { - w.ExcludeFilesLimit = *form.ExcludeFilesLimit - } + // Update payload optimization config + if form.PayloadOptimization != nil { + payloadOptConfig := &webhook.PayloadOptimizationConfig{} + + // Parse files config + if filesConfig, ok := (*form.PayloadOptimization)["files"].(map[string]any); ok { + payloadOptConfig.Files = &webhook.PayloadOptimizationItem{ + Enable: getBoolFromMap(filesConfig, false), + Limit: getIntFromMap(filesConfig, 0), + } + } else { + payloadOptConfig.Files = &webhook.PayloadOptimizationItem{Enable: false, Limit: 0} + } - if form.ExcludeCommitsLimit != nil { - w.ExcludeCommitsLimit = *form.ExcludeCommitsLimit + // Parse commits config + if commitsConfig, ok := (*form.PayloadOptimization)["commits"].(map[string]any); ok { + payloadOptConfig.Commits = &webhook.PayloadOptimizationItem{ + Enable: getBoolFromMap(commitsConfig, false), + Limit: getIntFromMap(commitsConfig, 0), + } + } else { + payloadOptConfig.Commits = &webhook.PayloadOptimizationItem{Enable: false, Limit: 0} + } + + if err := w.SetPayloadOptimizationConfig(payloadOptConfig); err != nil { + ctx.APIErrorInternal(err) + return false + } } if err := webhook.UpdateWebhook(ctx, w); err != nil { diff --git a/routers/web/repo/setting/webhook.go b/routers/web/repo/setting/webhook.go index 7b6d69674d9c7..111a582881297 100644 --- a/routers/web/repo/setting/webhook.go +++ b/routers/web/repo/setting/webhook.go @@ -232,20 +232,35 @@ func createWebhook(ctx *context.Context, params webhookParams) { } w := &webhook.Webhook{ - RepoID: orCtx.RepoID, - URL: params.URL, - HTTPMethod: params.HTTPMethod, - ContentType: params.ContentType, - Secret: params.WebhookForm.Secret, - HookEvent: ParseHookEvent(params.WebhookForm), - IsActive: params.WebhookForm.Active, - Type: params.Type, - Meta: string(meta), - OwnerID: orCtx.OwnerID, - IsSystemWebhook: orCtx.IsSystemWebhook, - ExcludeFilesLimit: params.WebhookForm.ExcludeFilesLimit, - ExcludeCommitsLimit: params.WebhookForm.ExcludeCommitsLimit, + RepoID: orCtx.RepoID, + URL: params.URL, + HTTPMethod: params.HTTPMethod, + ContentType: params.ContentType, + Secret: params.WebhookForm.Secret, + HookEvent: ParseHookEvent(params.WebhookForm), + IsActive: params.WebhookForm.Active, + Type: params.Type, + Meta: string(meta), + OwnerID: orCtx.OwnerID, + IsSystemWebhook: orCtx.IsSystemWebhook, + } + + // Set payload optimization config + payloadOptConfig := &webhook.PayloadOptimizationConfig{ + Files: &webhook.PayloadOptimizationItem{ + Enable: params.WebhookForm.PayloadOptimizationFilesEnable, + Limit: params.WebhookForm.PayloadOptimizationFilesLimit, + }, + Commits: &webhook.PayloadOptimizationItem{ + Enable: params.WebhookForm.PayloadOptimizationCommitsEnable, + Limit: params.WebhookForm.PayloadOptimizationCommitsLimit, + }, + } + if err := w.SetPayloadOptimizationConfig(payloadOptConfig); err != nil { + ctx.ServerError("SetPayloadOptimizationConfig", err) + return } + err = w.SetHeaderAuthorization(params.WebhookForm.AuthorizationHeader) if err != nil { ctx.ServerError("SetHeaderAuthorization", err) @@ -296,8 +311,22 @@ func editWebhook(ctx *context.Context, params webhookParams) { w.IsActive = params.WebhookForm.Active w.HTTPMethod = params.HTTPMethod w.Meta = string(meta) - w.ExcludeFilesLimit = params.WebhookForm.ExcludeFilesLimit - w.ExcludeCommitsLimit = params.WebhookForm.ExcludeCommitsLimit + + // Set payload optimization config + payloadOptConfig := &webhook.PayloadOptimizationConfig{ + Files: &webhook.PayloadOptimizationItem{ + Enable: params.WebhookForm.PayloadOptimizationFilesEnable, + Limit: params.WebhookForm.PayloadOptimizationFilesLimit, + }, + Commits: &webhook.PayloadOptimizationItem{ + Enable: params.WebhookForm.PayloadOptimizationCommitsEnable, + Limit: params.WebhookForm.PayloadOptimizationCommitsLimit, + }, + } + if err := w.SetPayloadOptimizationConfig(payloadOptConfig); err != nil { + ctx.ServerError("SetPayloadOptimizationConfig", err) + return + } err = w.SetHeaderAuthorization(params.WebhookForm.AuthorizationHeader) if err != nil { diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index fe96e261506aa..57e7c98db5b4c 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -239,9 +239,11 @@ type WebhookForm struct { BranchFilter string `binding:"GlobPattern"` AuthorizationHeader string Secret string - // Payload size optimization options - ExcludeFilesLimit int // -1: trim all (none kept), 0: do not trim, >0: keep N file changes in commit payloads - ExcludeCommitsLimit int // -1: trim all (none kept), 0: do not trim, >0: keep N commits in push payloads + // Payload optimization settings + PayloadOptimizationFilesEnable bool `form:"payload_optimization_files_enable"` + PayloadOptimizationFilesLimit int `form:"payload_optimization_files_limit"` + PayloadOptimizationCommitsEnable bool `form:"payload_optimization_commits_enable"` + PayloadOptimizationCommitsLimit int `form:"payload_optimization_commits_limit"` } // PushOnly if the hook will be triggered when push diff --git a/services/webhook/general.go b/services/webhook/general.go index 08c03b4885517..a326f7ea71579 100644 --- a/services/webhook/general.go +++ b/services/webhook/general.go @@ -409,6 +409,19 @@ func ToHook(repoLink string, w *webhook_model.Webhook) (*api.Hook, error) { return nil, err } + // Convert payload optimization config to map + payloadOptConfig := w.GetPayloadOptimizationConfig() + payloadOptimization := map[string]any{ + "files": map[string]any{ + "enable": payloadOptConfig.Files.Enable, + "limit": payloadOptConfig.Files.Limit, + }, + "commits": map[string]any{ + "enable": payloadOptConfig.Commits.Enable, + "limit": payloadOptConfig.Commits.Limit, + }, + } + return &api.Hook{ ID: w.ID, Type: w.Type, @@ -417,8 +430,7 @@ func ToHook(repoLink string, w *webhook_model.Webhook) (*api.Hook, error) { Config: config, Events: w.EventsArray(), AuthorizationHeader: authorizationHeader, - ExcludeFilesLimit: w.ExcludeFilesLimit, - ExcludeCommitsLimit: w.ExcludeCommitsLimit, + PayloadOptimization: payloadOptimization, Updated: w.UpdatedUnix.AsTime(), Created: w.CreatedUnix.AsTime(), BranchFilter: w.BranchFilter, diff --git a/services/webhook/notifier.go b/services/webhook/notifier.go index af9d3e17a791d..4939cd06a06fa 100644 --- a/services/webhook/notifier.go +++ b/services/webhook/notifier.go @@ -643,9 +643,9 @@ func (m *webhookNotifier) IssueChangeMilestone(ctx context.Context, doer *user_m } } -// applyWebhookPayloadOptimizations applies payload size optimizations based on webhook configurations +// applyWebhookPayloadOptimizations applies payload optimizations based on webhook configurations func (m *webhookNotifier) applyWebhookPayloadOptimizations(ctx context.Context, repo *repo_model.Repository, apiCommits []*api.PayloadCommit, apiHeadCommit *api.PayloadCommit) ([]*api.PayloadCommit, *api.PayloadCommit) { - // Get webhooks for this repository to check their configuration + // Get all webhooks for this repository webhooks, err := db.Find[webhook_model.Webhook](ctx, webhook_model.ListWebhookOptions{ RepoID: repo.ID, IsActive: optional.Some(true), @@ -657,75 +657,118 @@ func (m *webhookNotifier) applyWebhookPayloadOptimizations(ctx context.Context, } // Check if any webhook has payload optimization options enabled - hasFilesLimit := 0 - hasCommitsLimit := 0 + var filesLimit, commitsLimit int + hasFilesLimit := false + hasCommitsLimit := false + optimizationEnabled := false + for _, webhook := range webhooks { if webhook.HasEvent(webhook_module.HookEventPush) { - if webhook.ExcludeFilesLimit != 0 && (hasFilesLimit == 0 || webhook.ExcludeFilesLimit < hasFilesLimit) { - hasFilesLimit = webhook.ExcludeFilesLimit + config := webhook.GetPayloadOptimizationConfig() + + // Check files optimization + if config.Files.Enable { + optimizationEnabled = true + if !hasFilesLimit || config.Files.Limit < filesLimit { + filesLimit = config.Files.Limit + hasFilesLimit = true + } } - if webhook.ExcludeCommitsLimit != 0 && (hasCommitsLimit == 0 || webhook.ExcludeCommitsLimit < hasCommitsLimit) { - hasCommitsLimit = webhook.ExcludeCommitsLimit + + // Check commits optimization + if config.Commits.Enable { + optimizationEnabled = true + if !hasCommitsLimit || config.Commits.Limit < commitsLimit { + commitsLimit = config.Commits.Limit + hasCommitsLimit = true + } } } } // Apply payload optimizations based on webhook configurations - // -1 trim all (none kept), 0 do not trim, >0 trim to N commits - if hasFilesLimit != 0 { - for _, commit := range apiCommits { - if commit.Added != nil { - if hasFilesLimit == -1 { - commit.Added = nil - } else if hasFilesLimit > 0 && len(commit.Added) > hasFilesLimit { - commit.Added = commit.Added[:hasFilesLimit] + // 0: trim all (none kept), >0: trim to N items (forward order), <0: trim to N items (reverse order) + if optimizationEnabled { + // Apply files optimization to all commits + if hasFilesLimit { + for _, commit := range apiCommits { + if commit.Added != nil { + if filesLimit == 0 { + commit.Added = nil + } else if filesLimit > 0 && len(commit.Added) > filesLimit { + commit.Added = commit.Added[:filesLimit] + } else if filesLimit < 0 && len(commit.Added) > -filesLimit { + // Reverse order: keep the last N items + commit.Added = commit.Added[len(commit.Added)+filesLimit:] + } } - } - if commit.Removed != nil { - if hasFilesLimit == -1 { - commit.Removed = nil - } else if hasFilesLimit > 0 && len(commit.Removed) > hasFilesLimit { - commit.Removed = commit.Removed[:hasFilesLimit] + if commit.Removed != nil { + if filesLimit == 0 { + commit.Removed = nil + } else if filesLimit > 0 && len(commit.Removed) > filesLimit { + commit.Removed = commit.Removed[:filesLimit] + } else if filesLimit < 0 && len(commit.Removed) > -filesLimit { + // Reverse order: keep the last N items + commit.Removed = commit.Removed[len(commit.Removed)+filesLimit:] + } } - } - if commit.Modified != nil { - if hasFilesLimit == -1 { - commit.Modified = nil - } else if hasFilesLimit > 0 && len(commit.Modified) > hasFilesLimit { - commit.Modified = commit.Modified[:hasFilesLimit] + if commit.Modified != nil { + if filesLimit == 0 { + commit.Modified = nil + } else if filesLimit > 0 && len(commit.Modified) > filesLimit { + commit.Modified = commit.Modified[:filesLimit] + } else if filesLimit < 0 && len(commit.Modified) > -filesLimit { + // Reverse order: keep the last N items + commit.Modified = commit.Modified[len(commit.Modified)+filesLimit:] + } } } - } - if apiHeadCommit != nil { - if apiHeadCommit.Added != nil { - if hasFilesLimit == -1 { - apiHeadCommit.Added = nil - } else if hasFilesLimit > 0 && len(apiHeadCommit.Added) > hasFilesLimit { - apiHeadCommit.Added = apiHeadCommit.Added[:hasFilesLimit] + + // Apply files optimization to head commit + if apiHeadCommit != nil { + if apiHeadCommit.Added != nil { + if filesLimit == 0 { + apiHeadCommit.Added = nil + } else if filesLimit > 0 && len(apiHeadCommit.Added) > filesLimit { + apiHeadCommit.Added = apiHeadCommit.Added[:filesLimit] + } else if filesLimit < 0 && len(apiHeadCommit.Added) > -filesLimit { + // Reverse order: keep the last N items + apiHeadCommit.Added = apiHeadCommit.Added[len(apiHeadCommit.Added)+filesLimit:] + } } - } - if apiHeadCommit.Removed != nil { - if hasFilesLimit == -1 { - apiHeadCommit.Removed = nil - } else if hasFilesLimit > 0 && len(apiHeadCommit.Removed) > hasFilesLimit { - apiHeadCommit.Removed = apiHeadCommit.Removed[:hasFilesLimit] + if apiHeadCommit.Removed != nil { + if filesLimit == 0 { + apiHeadCommit.Removed = nil + } else if filesLimit > 0 && len(apiHeadCommit.Removed) > filesLimit { + apiHeadCommit.Removed = apiHeadCommit.Removed[:filesLimit] + } else if filesLimit < 0 && len(apiHeadCommit.Removed) > -filesLimit { + // Reverse order: keep the last N items + apiHeadCommit.Removed = apiHeadCommit.Removed[len(apiHeadCommit.Removed)+filesLimit:] + } } - } - if apiHeadCommit.Modified != nil { - if hasFilesLimit == -1 { - apiHeadCommit.Modified = nil - } else if hasFilesLimit > 0 && len(apiHeadCommit.Modified) > hasFilesLimit { - apiHeadCommit.Modified = apiHeadCommit.Modified[:hasFilesLimit] + if apiHeadCommit.Modified != nil { + if filesLimit == 0 { + apiHeadCommit.Modified = nil + } else if filesLimit > 0 && len(apiHeadCommit.Modified) > filesLimit { + apiHeadCommit.Modified = apiHeadCommit.Modified[:filesLimit] + } else if filesLimit < 0 && len(apiHeadCommit.Modified) > -filesLimit { + // Reverse order: keep the last N items + apiHeadCommit.Modified = apiHeadCommit.Modified[len(apiHeadCommit.Modified)+filesLimit:] + } } } } - } - if hasCommitsLimit != 0 { - if hasCommitsLimit == -1 { - apiCommits = nil - } else if hasCommitsLimit > 0 && len(apiCommits) > hasCommitsLimit { - apiCommits = apiCommits[:hasCommitsLimit] + // Apply commits optimization + if hasCommitsLimit { + if commitsLimit == 0 { + apiCommits = nil + } else if commitsLimit > 0 && len(apiCommits) > commitsLimit { + apiCommits = apiCommits[:commitsLimit] + } else if commitsLimit < 0 && len(apiCommits) > -commitsLimit { + // Reverse order: keep the last N commits + apiCommits = apiCommits[len(apiCommits)+commitsLimit:] + } } } diff --git a/services/webhook/webhook_test.go b/services/webhook/webhook_test.go index 35ebc0dc06b85..544cef7b61512 100644 --- a/services/webhook/webhook_test.go +++ b/services/webhook/webhook_test.go @@ -95,40 +95,32 @@ func TestWebhookUserMail(t *testing.T) { func TestWebhookPayloadOptimization(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - var optimizedCommits []*api.PayloadCommit - var optimizedHeadCommit *api.PayloadCommit - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - // Clean up all webhooks for this repo to avoid interference - webhooks, err := db.Find[webhook_model.Webhook](db.DefaultContext, webhook_model.ListWebhookOptions{RepoID: repo.ID}) - assert.NoError(t, err) - for _, wh := range webhooks { - err = webhook_model.DeleteWebhookByID(db.DefaultContext, wh.ID) - assert.NoError(t, err) - } - - // Case: 0 (no trimming) + // Create test webhook webhook := &webhook_model.Webhook{ - RepoID: repo.ID, - URL: "http://example.com/webhook", - HTTPMethod: "POST", - ContentType: webhook_model.ContentTypeJSON, - Secret: "secret", - IsActive: true, - Type: webhook_module.GITEA, - ExcludeFilesLimit: 0, - ExcludeCommitsLimit: 0, + RepoID: repo.ID, + URL: "http://example.com/webhook", + HTTPMethod: "POST", + ContentType: webhook_model.ContentTypeJSON, + Secret: "secret", + IsActive: true, + Type: webhook_module.GITEA, HookEvent: &webhook_module.HookEvent{ PushOnly: true, }, } - err = webhook.UpdateEvent() + // Test case 1: No optimization enabled + webhook.SetPayloadOptimizationConfig(&webhook_model.PayloadOptimizationConfig{ + Files: &webhook_model.PayloadOptimizationItem{Enable: false, Limit: 0}, + Commits: &webhook_model.PayloadOptimizationItem{Enable: false, Limit: 0}, + }) + + err := webhook.UpdateEvent() assert.NoError(t, err) err = webhook_model.CreateWebhook(db.DefaultContext, webhook) assert.NoError(t, err) - assert.NotZero(t, webhook.ID) apiCommits := []*api.PayloadCommit{ { @@ -153,7 +145,9 @@ func TestWebhookPayloadOptimization(t *testing.T) { Removed: []string{}, Modified: []string{"file1.txt"}, } - optimizedCommits, optimizedHeadCommit = (&webhookNotifier{}).applyWebhookPayloadOptimizations(db.DefaultContext, repo, apiCommits, apiHeadCommit) + + // Should not modify anything when optimization is disabled + optimizedCommits, optimizedHeadCommit := (&webhookNotifier{}).applyWebhookPayloadOptimizations(db.DefaultContext, repo, apiCommits, apiHeadCommit) if assert.NotNil(t, optimizedCommits) && len(optimizedCommits) == 2 { assert.Equal(t, []string{"file1.txt", "file2.txt"}, optimizedCommits[0].Added) assert.Equal(t, []string{"oldfile.txt"}, optimizedCommits[0].Removed) @@ -168,11 +162,14 @@ func TestWebhookPayloadOptimization(t *testing.T) { assert.Equal(t, []string{"file1.txt"}, optimizedHeadCommit.Modified) } - // Case: -1 (trim all) - webhook.ExcludeFilesLimit = -1 - webhook.ExcludeCommitsLimit = -1 + // Test case 2: Files optimization enabled, limit = 0 (trim all) + webhook.SetPayloadOptimizationConfig(&webhook_model.PayloadOptimizationConfig{ + Files: &webhook_model.PayloadOptimizationItem{Enable: true, Limit: 0}, + Commits: &webhook_model.PayloadOptimizationItem{Enable: false, Limit: 0}, + }) err = webhook_model.UpdateWebhook(db.DefaultContext, webhook) assert.NoError(t, err) + apiCommits = []*api.PayloadCommit{ { ID: "abc123", @@ -196,19 +193,30 @@ func TestWebhookPayloadOptimization(t *testing.T) { Removed: []string{}, Modified: []string{"file1.txt"}, } + optimizedCommits, optimizedHeadCommit = (&webhookNotifier{}).applyWebhookPayloadOptimizations(db.DefaultContext, repo, apiCommits, apiHeadCommit) - assert.Nil(t, optimizedCommits) + if assert.NotNil(t, optimizedCommits) && len(optimizedCommits) == 2 { + assert.Nil(t, optimizedCommits[0].Added) + assert.Nil(t, optimizedCommits[0].Removed) + assert.Nil(t, optimizedCommits[0].Modified) + assert.Nil(t, optimizedCommits[1].Added) + assert.Nil(t, optimizedCommits[1].Removed) + assert.Nil(t, optimizedCommits[1].Modified) + } if assert.NotNil(t, optimizedHeadCommit) { assert.Nil(t, optimizedHeadCommit.Added) assert.Nil(t, optimizedHeadCommit.Removed) assert.Nil(t, optimizedHeadCommit.Modified) } - // Case: 1 (keep only 1) - webhook.ExcludeFilesLimit = 1 - webhook.ExcludeCommitsLimit = 1 + // Test case 3: Commits optimization enabled, limit = 1 (keep first) + webhook.SetPayloadOptimizationConfig(&webhook_model.PayloadOptimizationConfig{ + Files: &webhook_model.PayloadOptimizationItem{Enable: false, Limit: 0}, + Commits: &webhook_model.PayloadOptimizationItem{Enable: true, Limit: 1}, + }) err = webhook_model.UpdateWebhook(db.DefaultContext, webhook) assert.NoError(t, err) + apiCommits = []*api.PayloadCommit{ { ID: "abc123", @@ -232,10 +240,10 @@ func TestWebhookPayloadOptimization(t *testing.T) { Removed: []string{}, Modified: []string{"file1.txt"}, } + optimizedCommits, optimizedHeadCommit = (&webhookNotifier{}).applyWebhookPayloadOptimizations(db.DefaultContext, repo, apiCommits, apiHeadCommit) if assert.NotNil(t, optimizedCommits) && len(optimizedCommits) == 1 { - assert.Equal(t, "abc123", optimizedCommits[0].ID) - assert.Equal(t, []string{"file1.txt"}, optimizedCommits[0].Added) + assert.Equal(t, []string{"file1.txt", "file2.txt"}, optimizedCommits[0].Added) assert.Equal(t, []string{"oldfile.txt"}, optimizedCommits[0].Removed) assert.Equal(t, []string{"modified.txt"}, optimizedCommits[0].Modified) } diff --git a/templates/repo/settings/webhook/settings.tmpl b/templates/repo/settings/webhook/settings.tmpl index 1dd5d99a5b84f..0994df6801512 100644 --- a/templates/repo/settings/webhook/settings.tmpl +++ b/templates/repo/settings/webhook/settings.tmpl @@ -51,17 +51,46 @@

{{ctx.Locale.Tr "repo.settings.payload_optimization"}}

- - - {{ctx.Locale.Tr "repo.settings.exclude_files_limit_desc"}} +
{{ctx.Locale.Tr "repo.settings.payload_optimization_files"}}
+
+
+ + + {{ctx.Locale.Tr "repo.settings.payload_optimization_enable_desc"}} +
+
+
+ + + {{ctx.Locale.Tr "repo.settings.payload_optimization_limit_desc"}} +
- - - {{ctx.Locale.Tr "repo.settings.exclude_commits_limit_desc"}} +
{{ctx.Locale.Tr "repo.settings.payload_optimization_commits"}}
+
+
+ + + {{ctx.Locale.Tr "repo.settings.payload_optimization_enable_desc"}} +
+
+
+ + + {{ctx.Locale.Tr "repo.settings.payload_optimization_limit_desc"}} +
+ +

{{ctx.Locale.Tr "repo.settings.event_desc"}}

diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 35c743dcd4a24..340e08d0f38c2 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -22818,6 +22818,12 @@ }, "x-go-name": "Events" }, + "payload_optimization": { + "description": "Payload size optimization options", + "type": "object", + "additionalProperties": {}, + "x-go-name": "PayloadOptimization" + }, "type": { "type": "string", "enum": [ @@ -23979,6 +23985,12 @@ "type": "string" }, "x-go-name": "Events" + }, + "payload_optimization": { + "description": "Payload size optimization options", + "type": "object", + "additionalProperties": {}, + "x-go-name": "PayloadOptimization" } }, "x-go-package": "code.gitea.io/gitea/modules/structs" @@ -25250,6 +25262,12 @@ "format": "int64", "x-go-name": "ID" }, + "payload_optimization": { + "description": "PayloadOptimization configuration for webhook payload optimization", + "type": "object", + "additionalProperties": {}, + "x-go-name": "PayloadOptimization" + }, "type": { "type": "string", "x-go-name": "Type" diff --git a/tests/integration/repo_webhook_test.go b/tests/integration/repo_webhook_test.go index 004e063ee6501..1dfa4f6f97171 100644 --- a/tests/integration/repo_webhook_test.go +++ b/tests/integration/repo_webhook_test.go @@ -1226,6 +1226,7 @@ jobs: func Test_WebhookPayloadOptimizationAPI(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { session := loginUser(t, "user2") + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeAll) // Test creating webhook with payload optimization options via API createHookOption := map[string]any{ @@ -1234,47 +1235,78 @@ func Test_WebhookPayloadOptimizationAPI(t *testing.T) { "url": "http://example.com/webhook", "content_type": "json", }, - "events": []string{"push"}, - "exclude_files_limit": 5, - "exclude_commits_limit": 10, - "active": true, + "events": []string{"push"}, + "payload_optimization": map[string]any{ + "files": map[string]any{ + "enable": true, + "limit": 2, + }, + "commits": map[string]any{ + "enable": true, + "limit": 1, + }, + }, + "active": true, } - req := NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo1/hooks", createHookOption) + req := NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo1/hooks", createHookOption).AddTokenAuth(token) resp := session.MakeRequest(t, req, http.StatusCreated) var hook api.Hook DecodeJSON(t, resp, &hook) // Verify the webhook was created with correct payload optimization settings - assert.Equal(t, 5, hook.ExcludeFilesLimit) - assert.Equal(t, 10, hook.ExcludeCommitsLimit) + assert.NotNil(t, hook.PayloadOptimization) + filesConfig := hook.PayloadOptimization["files"].(map[string]any) + commitsConfig := hook.PayloadOptimization["commits"].(map[string]any) + assert.Equal(t, true, filesConfig["enable"]) + assert.InEpsilon(t, 2.0, filesConfig["limit"], 0.01) + assert.Equal(t, true, commitsConfig["enable"]) + assert.InEpsilon(t, 1.0, commitsConfig["limit"], 0.01) // Test updating webhook with different payload optimization options editHookOption := map[string]any{ - "exclude_files_limit": -1, - "exclude_commits_limit": 0, + "payload_optimization": map[string]any{ + "files": map[string]any{ + "enable": false, + "limit": 0, + }, + "commits": map[string]any{ + "enable": false, + "limit": 0, + }, + }, } - req = NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/repos/user2/repo1/hooks/%d", hook.ID), editHookOption) + req = NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/repos/user2/repo1/hooks/%d", hook.ID), editHookOption).AddTokenAuth(token) resp = session.MakeRequest(t, req, http.StatusOK) var updatedHook api.Hook DecodeJSON(t, resp, &updatedHook) // Verify the webhook was updated with correct payload optimization settings - assert.Equal(t, -1, updatedHook.ExcludeFilesLimit) - assert.Equal(t, 0, updatedHook.ExcludeCommitsLimit) + assert.NotNil(t, updatedHook.PayloadOptimization) + filesConfig = updatedHook.PayloadOptimization["files"].(map[string]any) + commitsConfig = updatedHook.PayloadOptimization["commits"].(map[string]any) + assert.Equal(t, false, filesConfig["enable"]) + assert.EqualValues(t, 0, filesConfig["limit"]) + assert.Equal(t, false, commitsConfig["enable"]) + assert.EqualValues(t, 0, commitsConfig["limit"]) // Test getting webhook to verify the settings are persisted - req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/user2/repo1/hooks/%d", hook.ID)) + req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/user2/repo1/hooks/%d", hook.ID)).AddTokenAuth(token) resp = session.MakeRequest(t, req, http.StatusOK) var retrievedHook api.Hook DecodeJSON(t, resp, &retrievedHook) // Verify the webhook settings are correctly retrieved - assert.Equal(t, -1, retrievedHook.ExcludeFilesLimit) - assert.Equal(t, 0, retrievedHook.ExcludeCommitsLimit) + assert.NotNil(t, retrievedHook.PayloadOptimization) + filesConfig = retrievedHook.PayloadOptimization["files"].(map[string]any) + commitsConfig = retrievedHook.PayloadOptimization["commits"].(map[string]any) + assert.Equal(t, false, filesConfig["enable"]) + assert.EqualValues(t, 0, filesConfig["limit"]) + assert.Equal(t, false, commitsConfig["enable"]) + assert.EqualValues(t, 0, commitsConfig["limit"]) }) }