diff --git a/pkg/services/discord/discord.go b/pkg/services/discord/discord.go index 385bda24..5582b454 100644 --- a/pkg/services/discord/discord.go +++ b/pkg/services/discord/discord.go @@ -4,12 +4,13 @@ import ( "bytes" "encoding/json" "fmt" + "net/http" + "net/url" + "github.com/containrrr/shoutrrr/pkg/format" "github.com/containrrr/shoutrrr/pkg/services/standard" "github.com/containrrr/shoutrrr/pkg/types" "github.com/containrrr/shoutrrr/pkg/util" - "net/http" - "net/url" ) // Service providing Discord as a notification service @@ -32,15 +33,20 @@ const ( ) // Send a notification message to discord -func (service *Service) Send(message string, params *types.Params) error { - +func (service *Service) Send(message string, params *types.Params) (err error) { if service.config.JSON { postURL := CreateAPIURLFromConfig(service.config) - return doSend([]byte(message), postURL) + err = doSend([]byte(message), postURL) + } else { + items, omitted := CreateItemsFromPlain(message, service.config.SplitLines) + err = service.sendItems(items, params, omitted) } - items, omitted := CreateItemsFromPlain(message, service.config.SplitLines) - return service.sendItems(items, params, omitted) + if err != nil { + err = fmt.Errorf("failed to send discord notification: %v", err) + } + + return } // SendItems sends items with additional meta data and richer appearance @@ -121,9 +127,5 @@ func doSend(payload []byte, postURL string) error { err = fmt.Errorf("response status code %s", res.Status) } - if err != nil { - return fmt.Errorf("failed to send discord notification: %v", err) - } - - return nil + return err } diff --git a/pkg/services/discord/discord_json.go b/pkg/services/discord/discord_json.go index 471de794..3acda28f 100644 --- a/pkg/services/discord/discord_json.go +++ b/pkg/services/discord/discord_json.go @@ -2,16 +2,17 @@ package discord import ( "fmt" + "time" + "github.com/containrrr/shoutrrr/pkg/types" "github.com/containrrr/shoutrrr/pkg/util" - "time" ) // WebhookPayload is the webhook endpoint payload type WebhookPayload struct { - Embeds []embedItem `json:"embeds"` - Username string `json:"username,omitempty"` - AvatarURL string `json:"avatar_url,omitempty"` + Embeds []embedItem `json:"embeds"` + Username string `json:"username,omitempty"` + AvatarURL string `json:"avatar_url,omitempty"` } // JSON is the actual notification payload @@ -32,6 +33,10 @@ type embedFooter struct { // CreatePayloadFromItems creates a JSON payload to be sent to the discord webhook API func CreatePayloadFromItems(items []types.MessageItem, title string, colors [types.MessageLevelCount]uint, omitted int) (WebhookPayload, error) { + if len(items) < 1 { + return WebhookPayload{}, fmt.Errorf("message is empty") + } + metaCount := 1 if omitted < 1 && len(title) < 1 { metaCount = 0 @@ -65,10 +70,14 @@ func CreatePayloadFromItems(items []types.MessageItem, title string, colors [typ embeds = append(embeds, ei) } - embeds[0].Title = title - if omitted > 0 { - embeds[0].Footer = &embedFooter{ - Text: fmt.Sprintf("... (%v character(s) where omitted)", omitted), + // This should not happen, but it's better to leave the index check before dereferencing the array + if len(embeds) > 0 { + embeds[0].Title = title + + if omitted > 0 { + embeds[0].Footer = &embedFooter{ + Text: fmt.Sprintf("... (%v character(s) were omitted)", omitted), + } } } diff --git a/pkg/services/discord/discord_test.go b/pkg/services/discord/discord_test.go index e4385762..87492e62 100644 --- a/pkg/services/discord/discord_test.go +++ b/pkg/services/discord/discord_test.go @@ -1,6 +1,7 @@ package discord_test import ( + "fmt" "log" "time" @@ -115,12 +116,24 @@ var _ = Describe("the discord service", func() { }) }) Describe("creating a json payload", func() { - //When("given a blank message", func() { - // It("should return an error", func() { - // _, err := CreatePayloadFromItems("", false) - // Expect(err).To(HaveOccurred()) - // }) - //}) + When("given a blank message", func() { + When("split lines is enabled", func() { + It("should return an error", func() { + items, omitted := CreateItemsFromPlain("", true) + Expect(items).To(BeEmpty()) + _, err := CreatePayloadFromItems(items, "title", dummyColors, omitted) + Expect(err).To(HaveOccurred()) + }) + }) + When("split lines is disabled", func() { + It("should return an error", func() { + items, omitted := CreateItemsFromPlain("", false) + Expect(items).To(BeEmpty()) + _, err := CreatePayloadFromItems(items, "title", dummyColors, omitted) + Expect(err).To(HaveOccurred()) + }) + }) + }) When("given a message that exceeds the max length", func() { It("should return a payload with chunked messages", func() { @@ -204,29 +217,45 @@ var _ = Describe("the discord service", func() { }) Describe("sending the payload", func() { - var err error + var dummyConfig = Config{ + WebhookID: "1", + Token: "dummyToken", + } + var service Service BeforeEach(func() { httpmock.Activate() + service = Service{} + if err := service.Initialize(dummyConfig.GetURL(), logger); err != nil { + panic(fmt.Errorf("service initialization failed: %v", err)) + } }) AfterEach(func() { httpmock.DeactivateAndReset() }) It("should not report an error if the server accepts the payload", func() { - config := Config{ - WebhookID: "1", - Token: "dummyToken", - } - serviceURL := config.GetURL() - service := Service{} - err = service.Initialize(serviceURL, logger) - Expect(err).NotTo(HaveOccurred()) + setupResponder(&dummyConfig, 204, "") - setupResponder(&config, 204, "") - - err = service.Send("Message", nil) - Expect(err).NotTo(HaveOccurred()) + Expect(service.Send("Message", nil)).To(Succeed()) + }) + It("should report an error if the server response is not OK", func() { + setupResponder(&dummyConfig, 400, "") + Expect(service.Initialize(dummyConfig.GetURL(), logger)).To(Succeed()) + Expect(service.Send("Message", nil)).NotTo(Succeed()) + }) + It("should report an error if the message is empty", func() { + setupResponder(&dummyConfig, 204, "") + Expect(service.Initialize(dummyConfig.GetURL(), logger)).To(Succeed()) + Expect(service.Send("", nil)).NotTo(Succeed()) + }) + When("using a custom json payload", func() { + It("should report an error if the server response is not OK", func() { + config := dummyConfig + config.JSON = true + setupResponder(&config, 400, "") + Expect(service.Initialize(config.GetURL(), logger)).To(Succeed()) + Expect(service.Send("Message", nil)).NotTo(Succeed()) + }) }) - }) }) diff --git a/pkg/util/partition_message.go b/pkg/util/partition_message.go index 95899b0f..14e85d3e 100644 --- a/pkg/util/partition_message.go +++ b/pkg/util/partition_message.go @@ -18,11 +18,17 @@ func PartitionMessage(input string, limits t.MessageLimit, distance int) (items maxTotal := Min(len(runes), limits.TotalChunkSize) maxCount := limits.ChunkCount - 1 + if len(input) == 0 { + // If the message is empty, return an empty array + omitted = 0 + return + } + for i := 0; i < maxCount; i++ { - // If no suitable split point is found, use the chunkSize - chunkEnd := chunkOffset + limits.ChunkSize - // ... and start next chunk directly after this one - nextChunkStart := chunkEnd + // If no suitable split point is found, start next chunk at chunkSize from chunk start + nextChunkStart := chunkOffset + limits.ChunkSize + // ... and set the chunk end to the rune before the next chunk + chunkEnd := nextChunkStart - 1 if chunkEnd > maxTotal { // The chunk is smaller than the limit, no need to search chunkEnd = maxTotal diff --git a/pkg/util/partition_message_test.go b/pkg/util/partition_message_test.go index e3201616..81d4b1a7 100644 --- a/pkg/util/partition_message_test.go +++ b/pkg/util/partition_message_test.go @@ -1,85 +1,101 @@ package util import ( + "strings" + "github.com/containrrr/shoutrrr/pkg/types" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "strings" ) var _ = Describe("Partition Message", func() { - Describe("creating a json payload", func() { - limits := types.MessageLimit{ - ChunkSize: 2000, - TotalChunkSize: 6000, - ChunkCount: 10, - } - When("given a message that exceeds the max length", func() { - When("not splitting by lines", func() { - It("should return a payload with chunked messages", func() { + limits := types.MessageLimit{ + ChunkSize: 2000, + TotalChunkSize: 6000, + ChunkCount: 10, + } + When("given a message that exceeds the max length", func() { + When("not splitting by lines", func() { + It("should return a payload with chunked messages", func() { - items, _ := testPartitionMessage(42, limits, 100) + items, _ := testPartitionMessage(42, limits, 100) - Expect(len(items[0].Text)).To(Equal(1994)) - Expect(len(items[1].Text)).To(Equal(1999)) - Expect(len(items[2].Text)).To(Equal(205)) - }) - It("omit characters above total max", func() { - items, _ := testPartitionMessage(62, limits, 100) + Expect(len(items[0].Text)).To(Equal(1994)) + Expect(len(items[1].Text)).To(Equal(1999)) + Expect(len(items[2].Text)).To(Equal(205)) + }) + It("omit characters above total max", func() { + items, _ := testPartitionMessage(62, limits, 100) - Expect(len(items[0].Text)).To(Equal(1994)) - Expect(len(items[1].Text)).To(Equal(1999)) - Expect(len(items[2].Text)).To(Equal(1999)) - Expect(len(items[3].Text)).To(Equal(5)) + Expect(len(items[0].Text)).To(Equal(1994)) + Expect(len(items[1].Text)).To(Equal(1999)) + Expect(len(items[2].Text)).To(Equal(1999)) + Expect(len(items[3].Text)).To(Equal(5)) + }) + It("should handle messages with a size modulus of chunksize", func() { + items, _ := testPartitionMessage(20, limits, 100) + Expect(len(items[0].Text)).To(Equal(1994)) + Expect(len(items[1].Text)).To(Equal(5)) + + items, _ = testPartitionMessage(40, limits, 100) + Expect(len(items[0].Text)).To(Equal(1994)) + Expect(len(items[1].Text)).To(Equal(1999)) + Expect(len(items[2].Text)).To(Equal(5)) + }) + When("the message is empty", func() { + It("should return no items", func() { + items, _ := testPartitionMessage(0, limits, 100) + Expect(items).To(BeEmpty()) }) }) - When("splitting by lines", func() { - It("should return a payload with chunked messages", func() { - items, omitted := testMessageItemsFromLines(18, limits, 2) - Expect(len(items[0].Text)).To(Equal(200)) - Expect(len(items[8].Text)).To(Equal(200)) + }) + When("splitting by lines", func() { + It("should return a payload with chunked messages", func() { + items, omitted := testMessageItemsFromLines(18, limits, 2) - Expect(omitted).To(Equal(0)) - }) - It("omit characters above total max", func() { - items, omitted := testMessageItemsFromLines(19, limits, 2) + Expect(len(items[0].Text)).To(Equal(200)) + Expect(len(items[8].Text)).To(Equal(200)) - Expect(len(items[0].Text)).To(Equal(200)) - Expect(len(items[8].Text)).To(Equal(200)) + Expect(omitted).To(Equal(0)) + }) + It("omit characters above total max", func() { + items, omitted := testMessageItemsFromLines(19, limits, 2) - Expect(omitted).To(Equal(100)) - }) - It("should trim characters above chunk size", func() { - hundreds := 42 - repeat := 21 - items, omitted := testMessageItemsFromLines(hundreds, limits, repeat) + Expect(len(items[0].Text)).To(Equal(200)) + Expect(len(items[8].Text)).To(Equal(200)) - Expect(len(items[0].Text)).To(Equal(limits.ChunkSize)) - Expect(len(items[1].Text)).To(Equal(limits.ChunkSize)) + Expect(omitted).To(Equal(100)) + }) + It("should trim characters above chunk size", func() { + hundreds := 42 + repeat := 21 + items, omitted := testMessageItemsFromLines(hundreds, limits, repeat) - // Trimmed characters do not count towards the total omitted count - Expect(omitted).To(Equal(0)) - }) + Expect(len(items[0].Text)).To(Equal(limits.ChunkSize)) + Expect(len(items[1].Text)).To(Equal(limits.ChunkSize)) - It("omit characters above total chunk size", func() { - hundreds := 100 - repeat := 20 - items, omitted := testMessageItemsFromLines(hundreds, limits, repeat) + // Trimmed characters do not count towards the total omitted count + Expect(omitted).To(Equal(0)) + }) - Expect(len(items[0].Text)).To(Equal(limits.ChunkSize)) - Expect(len(items[1].Text)).To(Equal(limits.ChunkSize)) - Expect(len(items[2].Text)).To(Equal(limits.ChunkSize)) + It("omit characters above total chunk size", func() { + hundreds := 100 + repeat := 20 + items, omitted := testMessageItemsFromLines(hundreds, limits, repeat) - maxRunes := hundreds * 100 - expectedOmitted := maxRunes - limits.TotalChunkSize + Expect(len(items[0].Text)).To(Equal(limits.ChunkSize)) + Expect(len(items[1].Text)).To(Equal(limits.ChunkSize)) + Expect(len(items[2].Text)).To(Equal(limits.ChunkSize)) - Expect(omitted).To(Equal(expectedOmitted)) - }) + maxRunes := hundreds * 100 + expectedOmitted := maxRunes - limits.TotalChunkSize + Expect(omitted).To(Equal(expectedOmitted)) }) }) + }) })