From a23d1566f617361a884f12568f225e2b4dbec84e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?nils=20ma=CC=8Ase=CC=81n?= Date: Tue, 23 Nov 2021 16:10:11 +0100 Subject: [PATCH] feat: send multiple messages when exceeding limits --- pkg/services/discord/discord.go | 43 ++++++++++++++------- pkg/services/discord/discord_json.go | 14 +------ pkg/services/discord/discord_test.go | 43 ++++++++------------- pkg/util/partition_message.go | 55 ++++++++++++++------------- pkg/util/partition_message_test.go | 56 ++++++++++------------------ 5 files changed, 96 insertions(+), 115 deletions(-) diff --git a/pkg/services/discord/discord.go b/pkg/services/discord/discord.go index 5582b454..200c173c 100644 --- a/pkg/services/discord/discord.go +++ b/pkg/services/discord/discord.go @@ -33,28 +33,36 @@ const ( ) // Send a notification message to discord -func (service *Service) Send(message string, params *types.Params) (err error) { +func (service *Service) Send(message string, params *types.Params) error { + var firstErr error + if service.config.JSON { postURL := CreateAPIURLFromConfig(service.config) - err = doSend([]byte(message), postURL) + firstErr = doSend([]byte(message), postURL) } else { - items, omitted := CreateItemsFromPlain(message, service.config.SplitLines) - err = service.sendItems(items, params, omitted) + batches := CreateItemsFromPlain(message, service.config.SplitLines) + for _, items := range batches { + if err := service.sendItems(items, params); err != nil { + service.Log(err) + if firstErr == nil { + firstErr = err + } + } + } } - if err != nil { - err = fmt.Errorf("failed to send discord notification: %v", err) + if firstErr != nil { + return fmt.Errorf("failed to send discord notification: %v", firstErr) } - - return + return nil } // SendItems sends items with additional meta data and richer appearance func (service *Service) SendItems(items []types.MessageItem, params *types.Params) error { - return service.sendItems(items, params, 0) + return service.sendItems(items, params) } -func (service *Service) sendItems(items []types.MessageItem, params *types.Params, omitted int) error { +func (service *Service) sendItems(items []types.MessageItem, params *types.Params) error { var err error config := *service.config @@ -63,7 +71,7 @@ func (service *Service) sendItems(items []types.MessageItem, params *types.Param } var payload WebhookPayload - payload, err = CreatePayloadFromItems(items, config.Title, config.LevelColors(), omitted) + payload, err = CreatePayloadFromItems(items, config.Title, config.LevelColors()) if err != nil { return err } @@ -82,12 +90,21 @@ func (service *Service) sendItems(items []types.MessageItem, params *types.Param } // CreateItemsFromPlain creates a set of MessageItems that is compatible with Discords webhook payload -func CreateItemsFromPlain(plain string, splitLines bool) (items []types.MessageItem, omitted int) { +func CreateItemsFromPlain(plain string, splitLines bool) (batches [][]types.MessageItem) { if splitLines { return util.MessageItemsFromLines(plain, limits) } - return util.PartitionMessage(plain, limits, maxSearchRunes) + for { + items, omitted := util.PartitionMessage(plain, limits, maxSearchRunes) + batches = append(batches, items) + if omitted == 0 { + break + } + plain = plain[len(plain)-omitted:] + } + + return } // Initialize loads ServiceConfig from configURL and sets logger for this Service diff --git a/pkg/services/discord/discord_json.go b/pkg/services/discord/discord_json.go index 3acda28f..c94274db 100644 --- a/pkg/services/discord/discord_json.go +++ b/pkg/services/discord/discord_json.go @@ -31,19 +31,15 @@ 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) { +func CreatePayloadFromItems(items []types.MessageItem, title string, colors [types.MessageLevelCount]uint) (WebhookPayload, error) { if len(items) < 1 { return WebhookPayload{}, fmt.Errorf("message is empty") } - metaCount := 1 - if omitted < 1 && len(title) < 1 { - metaCount = 0 - } itemCount := util.Min(9, len(items)) - embeds := make([]embedItem, metaCount, itemCount+metaCount) + embeds := make([]embedItem, 0, itemCount) for _, item := range items { @@ -73,12 +69,6 @@ func CreatePayloadFromItems(items []types.MessageItem, title string, colors [typ // 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), - } - } } return WebhookPayload{ diff --git a/pkg/services/discord/discord_test.go b/pkg/services/discord/discord_test.go index 87492e62..f28bdf7d 100644 --- a/pkg/services/discord/discord_test.go +++ b/pkg/services/discord/discord_test.go @@ -119,17 +119,19 @@ var _ = Describe("the discord service", func() { When("given a blank message", func() { When("split lines is enabled", func() { It("should return an error", func() { - items, omitted := CreateItemsFromPlain("", true) + // batches := CreateItemsFromPlain("", true) + items := []types.MessageItem{} Expect(items).To(BeEmpty()) - _, err := CreatePayloadFromItems(items, "title", dummyColors, omitted) + _, err := CreatePayloadFromItems(items, "title", dummyColors) Expect(err).To(HaveOccurred()) }) }) When("split lines is disabled", func() { It("should return an error", func() { - items, omitted := CreateItemsFromPlain("", false) + batches := CreateItemsFromPlain("", false) + items := batches[0] Expect(items).To(BeEmpty()) - _, err := CreatePayloadFromItems(items, "title", dummyColors, omitted) + _, err := CreatePayloadFromItems(items, "title", dummyColors) Expect(err).To(HaveOccurred()) }) }) @@ -140,24 +142,20 @@ var _ = Describe("the discord service", func() { payload, err := buildPayloadFromHundreds(42, false, "Title", dummyColors) Expect(err).ToNot(HaveOccurred()) - meta := payload.Embeds[0] - items := payload.Embeds[1:] + items := payload.Embeds Expect(items).To(HaveLen(3)) Expect(items[0].Content).To(HaveLen(1994)) Expect(items[1].Content).To(HaveLen(1999)) Expect(items[2].Content).To(HaveLen(205)) - - Expect(meta.Footer).To(BeNil()) }) It("omit characters above total max", func() { payload, err := buildPayloadFromHundreds(62, false, "", dummyColors) Expect(err).ToNot(HaveOccurred()) - meta := payload.Embeds[0] - items := payload.Embeds[1:] + items := payload.Embeds Expect(items).To(HaveLen(4)) Expect(items[0].Content).To(HaveLen(1994)) @@ -165,7 +163,7 @@ var _ = Describe("the discord service", func() { Expect(len(items[2].Content)).To(Equal(1999)) Expect(len(items[3].Content)).To(Equal(5)) - Expect(meta.Footer.Text).To(ContainSubstring("200")) + // Expect(meta.Footer.Text).To(ContainSubstring("200")) }) When("no title is supplied and content fits", func() { It("should return a payload without a meta chunk", func() { @@ -176,14 +174,6 @@ var _ = Describe("the discord service", func() { Expect(payload.Embeds[0].Title).To(BeEmpty()) }) }) - When("no title is supplied but content was omitted", func() { - It("should return a payload with a meta chunk", func() { - - payload, err := buildPayloadFromHundreds(62, false, "", dummyColors) - Expect(err).ToNot(HaveOccurred()) - Expect(payload.Embeds[0].Footer).ToNot(BeNil()) - }) - }) When("title is supplied, but content fits", func() { It("should return a payload with a meta chunk", func() { payload, err := buildPayloadFromHundreds(42, false, "Title", dummyColors) @@ -202,15 +192,14 @@ var _ = Describe("the discord service", func() { Level: types.Warning, }, } - payload, err := CreatePayloadFromItems(items, "Title", dummyColors, 0) + payload, err := CreatePayloadFromItems(items, "Title", dummyColors) Expect(err).ToNot(HaveOccurred()) - meta := payload.Embeds[0] - item := payload.Embeds[1] + item := payload.Embeds[0] - Expect(payload.Embeds).To(HaveLen(2)) + Expect(payload.Embeds).To(HaveLen(1)) Expect(item.Footer.Text).To(Equal(types.Warning.String())) - Expect(meta.Title).To(Equal("Title")) + Expect(item.Title).To(Equal("Title")) Expect(item.Color).To(Equal(dummyColors[types.Warning])) }) }) @@ -267,10 +256,10 @@ func buildPayloadFromHundreds(hundreds int, split bool, title string, colors [ty builder.WriteString(hundredChars) } - items, omitted := CreateItemsFromPlain(builder.String(), split) - println("Items:", len(items), "Omitted:", omitted) + batches := CreateItemsFromPlain(builder.String(), split) + items := batches[0] - return CreatePayloadFromItems(items, title, colors, omitted) + return CreatePayloadFromItems(items, title, colors) } func setupResponder(config *Config, code int, body string) { diff --git a/pkg/util/partition_message.go b/pkg/util/partition_message.go index a5d6b5b2..6f47dc23 100644 --- a/pkg/util/partition_message.go +++ b/pkg/util/partition_message.go @@ -68,41 +68,44 @@ func Ellipsis(text string, maxLength int) string { } // MessageItemsFromLines creates a set of MessageItems that is compatible with the supplied limits -func MessageItemsFromLines(plain string, limits t.MessageLimit) (items []t.MessageItem, omitted int) { - omitted = 0 - maxCount := limits.ChunkCount - 1 +func MessageItemsFromLines(plain string, limits t.MessageLimit) (batches [][]t.MessageItem) { + maxCount := limits.ChunkCount lines := strings.Split(plain, "\n") - items = make([]t.MessageItem, 0, Min(maxCount, len(lines))) + batches = make([][]t.MessageItem, 0) + items := make([]t.MessageItem, 0, Min(maxCount, len(lines))) totalLength := 0 - for l, line := range lines { - if l < maxCount && totalLength < limits.TotalChunkSize { - runes := []rune(line) - maxLen := limits.ChunkSize - if totalLength+maxLen > limits.TotalChunkSize { - maxLen = limits.TotalChunkSize - totalLength - } - if len(runes) > maxLen { - // Trim and add ellipsis - runes = runes[:maxLen-len(ellipsis)] - line = string(runes) + ellipsis - } + for _, line := range lines { - if len(runes) < 1 { - continue - } + maxLen := limits.ChunkSize - items = append(items, t.MessageItem{ - Text: line, - }) + if len(items) == maxCount || totalLength+maxLen > limits.TotalChunkSize { + batches = append(batches, items) + items = items[:0] + } - totalLength += len(runes) + runes := []rune(line) + if len(runes) > maxLen { + // Trim and add ellipsis + runes = runes[:maxLen-len(ellipsis)] + line = string(runes) + ellipsis + } - } else { - omitted += len(line) + if len(runes) < 1 { + continue } + + items = append(items, t.MessageItem{ + Text: line, + }) + + totalLength += len(runes) + } + + if len(items) > 0 { + batches = append(batches, items) } - return items, omitted + return batches } diff --git a/pkg/util/partition_message_test.go b/pkg/util/partition_message_test.go index d6b33254..8f678aad 100644 --- a/pkg/util/partition_message_test.go +++ b/pkg/util/partition_message_test.go @@ -1,6 +1,7 @@ package util import ( + "fmt" "strconv" "strings" @@ -106,56 +107,42 @@ var _ = Describe("Partition Message", func() { }) When("splitting by lines", func() { It("should return a payload with chunked messages", func() { - items, omitted := testMessageItemsFromLines(18, limits, 2) + batches := testMessageItemsFromLines(18, limits, 2) + items := batches[0] 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(len(items[0].Text)).To(Equal(200)) - Expect(len(items[8].Text)).To(Equal(200)) + When("the message items exceed the limits", func() { + It("should split items into multiple batches", func() { + batches := testMessageItemsFromLines(21, limits, 2) + + for b, chunks := range batches { + fmt.Fprintf(GinkgoWriter, "Batch #%v: (%v chunks)\n", b, len(chunks)) + for c, chunk := range chunks { + fmt.Fprintf(GinkgoWriter, " - Chunk #%v: (%v runes)\n", c, len(chunk.Text)) + } + } - Expect(omitted).To(Equal(100)) + Expect(len(batches)).To(Equal(2)) + }) }) 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(limits.ChunkSize)) - Expect(len(items[1].Text)).To(Equal(limits.ChunkSize)) - - // Trimmed characters do not count towards the total omitted count - Expect(omitted).To(Equal(0)) - }) - - It("omit characters above total chunk size", func() { - hundreds := 100 - repeat := 20 - items, omitted := testMessageItemsFromLines(hundreds, limits, repeat) + batches := testMessageItemsFromLines(hundreds, limits, repeat) + items := batches[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)) - - maxRunes := hundreds * 100 - expectedOmitted := maxRunes - limits.TotalChunkSize - - Expect(omitted).To(Equal(expectedOmitted)) }) - }) - }) }) const hundredChars = "this string is exactly (to the letter) a hundred characters long which will make the send func error" -func testMessageItemsFromLines(hundreds int, limits types.MessageLimit, repeat int) (items []types.MessageItem, omitted int) { +func testMessageItemsFromLines(hundreds int, limits types.MessageLimit, repeat int) (batches [][]types.MessageItem) { builder := strings.Builder{} @@ -169,12 +156,7 @@ func testMessageItemsFromLines(hundreds int, limits types.MessageLimit, repeat i } } - items, omitted = MessageItemsFromLines(builder.String(), limits) - - maxChunkSize := Min(limits.ChunkSize, repeat*100) - - expectedChunkCount := Min(limits.TotalChunkSize/maxChunkSize, Min(hundreds/repeat, limits.ChunkCount-1)) - Expect(len(items)).To(Equal(expectedChunkCount), "Chunk count") + batches = MessageItemsFromLines(builder.String(), limits) return }