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,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,39 +141,94 @@ func TestWebhookPayloadOptimization(t *testing.T) {
 			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
+	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)
 }