From 1f47ccfacd81470e2c33a02bb8d255172aa4ec08 Mon Sep 17 00:00:00 2001 From: Till Faelligen Date: Sat, 11 Jul 2020 13:09:06 +0200 Subject: [PATCH 1/5] Avoid sending "0 new commits" webhook Signed-off-by: Till Faelligen --- modules/webhook/dingtalk.go | 4 ++++ modules/webhook/discord.go | 4 ++++ modules/webhook/feishu.go | 4 ++++ modules/webhook/matrix.go | 4 ++++ modules/webhook/msteams.go | 4 ++++ modules/webhook/slack.go | 4 ++++ modules/webhook/telegram.go | 4 ++++ 7 files changed, 28 insertions(+) diff --git a/modules/webhook/dingtalk.go b/modules/webhook/dingtalk.go index 4e0e52451abb1..a3fda843d5bb1 100644 --- a/modules/webhook/dingtalk.go +++ b/modules/webhook/dingtalk.go @@ -83,6 +83,10 @@ func getDingtalkForkPayload(p *api.ForkPayload) (*DingtalkPayload, error) { } func getDingtalkPushPayload(p *api.PushPayload) (*DingtalkPayload, error) { + if len(p.Commits) == 0 { + return nil, fmt.Errorf("no commits in payload") + } + var ( branchName = git.RefEndName(p.Ref) commitDesc string diff --git a/modules/webhook/discord.go b/modules/webhook/discord.go index 761129d8d9e72..7fcca5f8e1a0d 100644 --- a/modules/webhook/discord.go +++ b/modules/webhook/discord.go @@ -177,6 +177,10 @@ func getDiscordForkPayload(p *api.ForkPayload, meta *DiscordMeta) (*DiscordPaylo } func getDiscordPushPayload(p *api.PushPayload, meta *DiscordMeta) (*DiscordPayload, error) { + if len(p.Commits) == 0 { + return nil, fmt.Errorf("no commits in payload") + } + var ( branchName = git.RefEndName(p.Ref) commitDesc string diff --git a/modules/webhook/feishu.go b/modules/webhook/feishu.go index 4beda9014c54d..dfc335480f45a 100644 --- a/modules/webhook/feishu.go +++ b/modules/webhook/feishu.go @@ -66,6 +66,10 @@ func getFeishuForkPayload(p *api.ForkPayload) (*FeishuPayload, error) { } func getFeishuPushPayload(p *api.PushPayload) (*FeishuPayload, error) { + if len(p.Commits) == 0 { + return nil, fmt.Errorf("no commits in payload") + } + var ( branchName = git.RefEndName(p.Ref) commitDesc string diff --git a/modules/webhook/matrix.go b/modules/webhook/matrix.go index 68c65623f727a..b2bc1968164aa 100644 --- a/modules/webhook/matrix.go +++ b/modules/webhook/matrix.go @@ -145,6 +145,10 @@ func getMatrixReleasePayload(p *api.ReleasePayload, matrix *MatrixMeta) (*Matrix } func getMatrixPushPayload(p *api.PushPayload, matrix *MatrixMeta) (*MatrixPayloadUnsafe, error) { + if len(p.Commits) == 0 { + return nil, fmt.Errorf("no commits in payload") + } + var commitDesc string if len(p.Commits) == 1 { diff --git a/modules/webhook/msteams.go b/modules/webhook/msteams.go index e7ec396f2941c..812c7ce21900e 100644 --- a/modules/webhook/msteams.go +++ b/modules/webhook/msteams.go @@ -196,6 +196,10 @@ func getMSTeamsForkPayload(p *api.ForkPayload) (*MSTeamsPayload, error) { } func getMSTeamsPushPayload(p *api.PushPayload) (*MSTeamsPayload, error) { + if len(p.Commits) == 0 { + return nil, fmt.Errorf("no commits in payload") + } + var ( branchName = git.RefEndName(p.Ref) commitDesc string diff --git a/modules/webhook/slack.go b/modules/webhook/slack.go index 4177bd1250e92..8f42d9debe874 100644 --- a/modules/webhook/slack.go +++ b/modules/webhook/slack.go @@ -189,6 +189,10 @@ func getSlackReleasePayload(p *api.ReleasePayload, slack *SlackMeta) (*SlackPayl } func getSlackPushPayload(p *api.PushPayload, slack *SlackMeta) (*SlackPayload, error) { + if len(p.Commits) == 0 { + return nil, fmt.Errorf("no commits in payload") + } + // n new commits var ( commitDesc string diff --git a/modules/webhook/telegram.go b/modules/webhook/telegram.go index 6d2f804a70237..3c43fa1d4fffb 100644 --- a/modules/webhook/telegram.go +++ b/modules/webhook/telegram.go @@ -86,6 +86,10 @@ func getTelegramForkPayload(p *api.ForkPayload) (*TelegramPayload, error) { } func getTelegramPushPayload(p *api.PushPayload) (*TelegramPayload, error) { + if len(p.Commits) == 0 { + return nil, fmt.Errorf("no commits in payload") + } + var ( branchName = git.RefEndName(p.Ref) commitDesc string From 696355074207f4ac257f5a78ff5291fb8eb8e8d5 Mon Sep 17 00:00:00 2001 From: Till Faelligen Date: Sat, 11 Jul 2020 18:47:52 +0200 Subject: [PATCH 2/5] Revert "Avoid sending "0 new commits" webhook" This reverts commit 1f47ccfacd81470e2c33a02bb8d255172aa4ec08. --- modules/webhook/dingtalk.go | 4 ---- modules/webhook/discord.go | 4 ---- modules/webhook/feishu.go | 4 ---- modules/webhook/matrix.go | 4 ---- modules/webhook/msteams.go | 4 ---- modules/webhook/slack.go | 4 ---- modules/webhook/telegram.go | 4 ---- 7 files changed, 28 deletions(-) diff --git a/modules/webhook/dingtalk.go b/modules/webhook/dingtalk.go index a3fda843d5bb1..4e0e52451abb1 100644 --- a/modules/webhook/dingtalk.go +++ b/modules/webhook/dingtalk.go @@ -83,10 +83,6 @@ func getDingtalkForkPayload(p *api.ForkPayload) (*DingtalkPayload, error) { } func getDingtalkPushPayload(p *api.PushPayload) (*DingtalkPayload, error) { - if len(p.Commits) == 0 { - return nil, fmt.Errorf("no commits in payload") - } - var ( branchName = git.RefEndName(p.Ref) commitDesc string diff --git a/modules/webhook/discord.go b/modules/webhook/discord.go index 7fcca5f8e1a0d..761129d8d9e72 100644 --- a/modules/webhook/discord.go +++ b/modules/webhook/discord.go @@ -177,10 +177,6 @@ func getDiscordForkPayload(p *api.ForkPayload, meta *DiscordMeta) (*DiscordPaylo } func getDiscordPushPayload(p *api.PushPayload, meta *DiscordMeta) (*DiscordPayload, error) { - if len(p.Commits) == 0 { - return nil, fmt.Errorf("no commits in payload") - } - var ( branchName = git.RefEndName(p.Ref) commitDesc string diff --git a/modules/webhook/feishu.go b/modules/webhook/feishu.go index dfc335480f45a..4beda9014c54d 100644 --- a/modules/webhook/feishu.go +++ b/modules/webhook/feishu.go @@ -66,10 +66,6 @@ func getFeishuForkPayload(p *api.ForkPayload) (*FeishuPayload, error) { } func getFeishuPushPayload(p *api.PushPayload) (*FeishuPayload, error) { - if len(p.Commits) == 0 { - return nil, fmt.Errorf("no commits in payload") - } - var ( branchName = git.RefEndName(p.Ref) commitDesc string diff --git a/modules/webhook/matrix.go b/modules/webhook/matrix.go index b2bc1968164aa..68c65623f727a 100644 --- a/modules/webhook/matrix.go +++ b/modules/webhook/matrix.go @@ -145,10 +145,6 @@ func getMatrixReleasePayload(p *api.ReleasePayload, matrix *MatrixMeta) (*Matrix } func getMatrixPushPayload(p *api.PushPayload, matrix *MatrixMeta) (*MatrixPayloadUnsafe, error) { - if len(p.Commits) == 0 { - return nil, fmt.Errorf("no commits in payload") - } - var commitDesc string if len(p.Commits) == 1 { diff --git a/modules/webhook/msteams.go b/modules/webhook/msteams.go index 812c7ce21900e..e7ec396f2941c 100644 --- a/modules/webhook/msteams.go +++ b/modules/webhook/msteams.go @@ -196,10 +196,6 @@ func getMSTeamsForkPayload(p *api.ForkPayload) (*MSTeamsPayload, error) { } func getMSTeamsPushPayload(p *api.PushPayload) (*MSTeamsPayload, error) { - if len(p.Commits) == 0 { - return nil, fmt.Errorf("no commits in payload") - } - var ( branchName = git.RefEndName(p.Ref) commitDesc string diff --git a/modules/webhook/slack.go b/modules/webhook/slack.go index 8f42d9debe874..4177bd1250e92 100644 --- a/modules/webhook/slack.go +++ b/modules/webhook/slack.go @@ -189,10 +189,6 @@ func getSlackReleasePayload(p *api.ReleasePayload, slack *SlackMeta) (*SlackPayl } func getSlackPushPayload(p *api.PushPayload, slack *SlackMeta) (*SlackPayload, error) { - if len(p.Commits) == 0 { - return nil, fmt.Errorf("no commits in payload") - } - // n new commits var ( commitDesc string diff --git a/modules/webhook/telegram.go b/modules/webhook/telegram.go index 3c43fa1d4fffb..6d2f804a70237 100644 --- a/modules/webhook/telegram.go +++ b/modules/webhook/telegram.go @@ -86,10 +86,6 @@ func getTelegramForkPayload(p *api.ForkPayload) (*TelegramPayload, error) { } func getTelegramPushPayload(p *api.PushPayload) (*TelegramPayload, error) { - if len(p.Commits) == 0 { - return nil, fmt.Errorf("no commits in payload") - } - var ( branchName = git.RefEndName(p.Ref) commitDesc string From 6fe78500d67cfea611d2680611eec0cded8dacbd Mon Sep 17 00:00:00 2001 From: Till Faelligen Date: Sat, 11 Jul 2020 19:00:09 +0200 Subject: [PATCH 3/5] Move commit count check to more central place --- modules/webhook/webhook.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/modules/webhook/webhook.go b/modules/webhook/webhook.go index 9748721450351..26c0747fbb1d4 100644 --- a/modules/webhook/webhook.go +++ b/modules/webhook/webhook.go @@ -76,6 +76,15 @@ func prepareWebhook(w *models.Webhook, repo *models.Repository, event models.Hoo } } + pushEvent, ok := p.(*api.PushPayload) + // Avoid sending "0 new commits" to non-integration relevant webhooks (e.g. slack, discord, etc.). + // Integration webhooks (e.g. drone) still receive the required data. + if ok && w.HookTaskType != models.GITEA && w.HookTaskType != models.GOGS { + if len(pushEvent.Commits) == 0 { + return nil + } + } + // If payload has no associated branch (e.g. it's a new tag, issue, etc.), // branch filter has no effect. if branch := getPayloadBranch(p); branch != "" { From 7cfbf06a8ff82b7138a5a02efdcca45625ef0c50 Mon Sep 17 00:00:00 2001 From: Till Faelligen Date: Sun, 12 Jul 2020 16:28:46 +0200 Subject: [PATCH 4/5] Make tests pass --- modules/webhook/webhook_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/webhook/webhook_test.go b/modules/webhook/webhook_test.go index e88e67e9bfefe..10c32a9485be2 100644 --- a/modules/webhook/webhook_test.go +++ b/modules/webhook/webhook_test.go @@ -34,7 +34,7 @@ func TestPrepareWebhooks(t *testing.T) { for _, hookTask := range hookTasks { models.AssertNotExistsBean(t, hookTask) } - assert.NoError(t, PrepareWebhooks(repo, models.HookEventPush, &api.PushPayload{})) + assert.NoError(t, PrepareWebhooks(repo, models.HookEventPush, &api.PushPayload{Commits: []*api.PayloadCommit{{}}})) for _, hookTask := range hookTasks { models.AssertExistsAndLoadBean(t, hookTask) } @@ -51,7 +51,7 @@ func TestPrepareWebhooksBranchFilterMatch(t *testing.T) { models.AssertNotExistsBean(t, hookTask) } // this test also ensures that * doesn't handle / in any special way (like shell would) - assert.NoError(t, PrepareWebhooks(repo, models.HookEventPush, &api.PushPayload{Ref: "refs/heads/feature/7791"})) + assert.NoError(t, PrepareWebhooks(repo, models.HookEventPush, &api.PushPayload{Ref: "refs/heads/feature/7791", Commits: []*api.PayloadCommit{{}}})) for _, hookTask := range hookTasks { models.AssertExistsAndLoadBean(t, hookTask) } From b2aec3a30699fbee091ebd8c0aff6bddce99fa00 Mon Sep 17 00:00:00 2001 From: S7evinK Date: Sun, 19 Jul 2020 08:24:54 +0200 Subject: [PATCH 5/5] Update modules/webhook/webhook.go Co-authored-by: Lauris BH --- modules/webhook/webhook.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/modules/webhook/webhook.go b/modules/webhook/webhook.go index 26c0747fbb1d4..2ef150210e71c 100644 --- a/modules/webhook/webhook.go +++ b/modules/webhook/webhook.go @@ -76,13 +76,12 @@ func prepareWebhook(w *models.Webhook, repo *models.Repository, event models.Hoo } } - pushEvent, ok := p.(*api.PushPayload) // Avoid sending "0 new commits" to non-integration relevant webhooks (e.g. slack, discord, etc.). // Integration webhooks (e.g. drone) still receive the required data. - if ok && w.HookTaskType != models.GITEA && w.HookTaskType != models.GOGS { - if len(pushEvent.Commits) == 0 { - return nil - } + if pushEvent, ok := p.(*api.PushPayload); ok && + w.HookTaskType != models.GITEA && w.HookTaskType != models.GOGS && + len(pushEvent.Commits) == 0 { + return nil } // If payload has no associated branch (e.g. it's a new tag, issue, etc.),