refactor webhook payload optimization: switch from bool to limit number

Author: kerwin612

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{

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"`

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)
 }

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 <code>*</code>, events for all branches are reported. See <a href="%[1]s">%[2]s</a> documentation for syntax. Examples: <code>master</code>, <code>{master,release*}</code>.
 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

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 {

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 {

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

services/webhook/webhook_test.go

@@ -95,20 +95,23 @@ 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,
 },
@@ -143,30 +146,43 @@ func TestWebhookPayloadOptimization(t *testing.T) {
 Modified: []string{"file1.txt"},
 }
 
-// Test payload optimization
+// Test payload optimization: should truncate to 1 file per field
 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)
+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)
 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)
 }

templates/repo/settings/webhook/settings.tmpl

@@ -51,18 +51,14 @@
 <div class="field">
 <h4>{{ctx.Locale.Tr "repo.settings.payload_optimization"}}</h4>
 <div class="field">
-<div class="ui checkbox">
-<input name="exclude_files" type="checkbox" {{if .Webhook.ExcludeFiles}}checked{{end}}>
-<label>{{ctx.Locale.Tr "repo.settings.exclude_files"}}</label>
-<span class="help">{{ctx.Locale.Tr "repo.settings.exclude_files_desc"}}</span>
-</div>
+<label>{{ctx.Locale.Tr "repo.settings.exclude_files_limit"}}</label>
+<input name="exclude_files_limit" type="number" min="0" value="{{.Webhook.ExcludeFilesLimit}}" placeholder="0">
+<span class="help">{{ctx.Locale.Tr "repo.settings.exclude_files_limit_desc"}}</span>
 </div>
 <div class="field">
-<div class="ui checkbox">
-<input name="exclude_commits" type="checkbox" {{if .Webhook.ExcludeCommits}}checked{{end}}>
-<label>{{ctx.Locale.Tr "repo.settings.exclude_commits"}}</label>
-<span class="help">{{ctx.Locale.Tr "repo.settings.exclude_commits_desc"}}</span>
-</div>
+<label>{{ctx.Locale.Tr "repo.settings.exclude_commits_limit"}}</label>
+<input name="exclude_commits_limit" type="number" min="0" value="{{.Webhook.ExcludeCommitsLimit}}" placeholder="0">
+<span class="help">{{ctx.Locale.Tr "repo.settings.exclude_commits_limit_desc"}}</span>
 </div>
 </div>