Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: change alert parts calculations #1084

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 67 additions & 0 deletions senders/calc_message_parts.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,70 @@ func CalculateMessagePartsLength(maxChars, descLen, eventsLen int) (descNewLen i
}
return maxChars/2 - 10, maxChars / 2
}

const (
// messagePartsCount is the number of parts in alert, which will be recalculated.
messagePartsCount = 3
)

// CalculateMessagePartsBetweenTagsDescEvents calculates and returns the length of tags, description and events string
// in order to fit the max chars limit.
func CalculateMessagePartsBetweenTagsDescEvents(maxChars, tagsLen, descLen, eventsLen int) (tagsNewLen int, descNewLen int, eventsNewLen int) { // nolint
if maxChars <= 0 {
return 0, 0, 0
}

if tagsLen+descLen+eventsLen <= maxChars {
return tagsLen, descLen, eventsLen
}

fairMaxLen := maxChars / messagePartsCount

switch {
case firstIsGreaterThanGivenLenAndOthersLessOrEqual(fairMaxLen, tagsLen, descLen, eventsLen):
// give free space to tags
tagsNewLen = maxChars - descLen - eventsLen

return min(tagsNewLen, tagsLen), descLen, eventsLen
case firstIsGreaterThanGivenLenAndOthersLessOrEqual(fairMaxLen, descLen, tagsLen, eventsLen):
// give free space to description
descNewLen = maxChars - tagsLen - eventsLen

return tagsLen, min(descNewLen, descLen), eventsLen
case firstIsGreaterThanGivenLenAndOthersLessOrEqual(fairMaxLen, eventsLen, tagsLen, descLen):
// give free space to events
eventsNewLen = maxChars - tagsLen - descLen

return tagsLen, descLen, min(eventsNewLen, eventsLen)
case firstAndSecondIsGreaterThanGivenLenAndOtherLessOrEqual(fairMaxLen, tagsLen, descLen, eventsLen):
// description is more important than tags
tagsNewLen = fairMaxLen
descNewLen = maxChars - tagsNewLen - eventsLen

return tagsNewLen, min(descNewLen, descLen), eventsLen
case firstAndSecondIsGreaterThanGivenLenAndOtherLessOrEqual(fairMaxLen, tagsLen, eventsLen, descLen):
// events are more important than tags
tagsNewLen = fairMaxLen
eventsNewLen = maxChars - tagsNewLen - descLen

return tagsNewLen, descLen, min(eventsNewLen, eventsLen)
case firstAndSecondIsGreaterThanGivenLenAndOtherLessOrEqual(fairMaxLen, descLen, eventsLen, tagsLen):
// split free space from tags fairly between description and events
spaceFromTags := fairMaxLen - tagsLen
descNewLen = fairMaxLen + spaceFromTags/2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spaceFromTags/2

2 в константу - по коду ниже она тож имеется

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

поправил

eventsNewLen = fairMaxLen + spaceFromTags/2

return tagsLen, min(descNewLen, descLen), min(eventsNewLen, eventsLen)
default:
// all 3 blocks have length greater than maxChars/3, so split space fairly
return fairMaxLen, fairMaxLen, fairMaxLen
}
}

func firstIsGreaterThanGivenLenAndOthersLessOrEqual(givenLen, first, second, third int) bool {
Copy link
Member

@almostinf almostinf Sep 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут как и в константах я не понимаю смысл оборачивать в функцию с названием просто идентичным тому
, что происходит внутри и таким большим названием :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

с first, second, third стало еще только запутанее

return first > givenLen && second <= givenLen && third <= givenLen
}

func firstAndSecondIsGreaterThanGivenLenAndOtherLessOrEqual(givenLen, first, second, third int) bool {
return first > givenLen && second > givenLen && third <= givenLen
}
177 changes: 177 additions & 0 deletions senders/calc_message_parts_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package senders

import (
"fmt"
"testing"

. "github.com/smartystreets/goconvey/convey"
Expand Down Expand Up @@ -33,3 +34,179 @@ func TestCalculateMessagePartsLength(t *testing.T) {
})
})
}

func TestCalculateMessagePartsBetweenTagsDescEvents(t *testing.T) {
Convey("Message parts calculating test (for tags, desc, events)", t, func() {
type given struct {
maxChars int
tagsLen int
descLen int
eventsLen int
}

type expected struct {
tagsLen int
descLen int
eventsLen int
}

type testcase struct {
given given
expected expected
description string
}

cases := []testcase{
{
description: "with maxChars < 0",
given: given{
maxChars: -1,
tagsLen: 10,
descLen: 10,
eventsLen: 10,
},
expected: expected{
tagsLen: 0,
descLen: 0,
eventsLen: 0,
},
},
{
description: "with tagsLen + descLen + eventsLen <= maxChars",
given: given{
maxChars: 100,
tagsLen: 20,
descLen: 50,
eventsLen: 30,
},
expected: expected{
tagsLen: 20,
descLen: 50,
eventsLen: 30,
},
},
{
description: "with tagsLen > maxChars/3, descLen and eventsLen <= maxChars/3",
given: given{
maxChars: 100,
tagsLen: 50,
descLen: 30,
eventsLen: 30,
},
expected: expected{
tagsLen: 40,
descLen: 30,
eventsLen: 30,
},
},
{
description: "with descLen > maxChars/3, tagsLen and eventsLen <= maxChars/3",
given: given{
maxChars: 100,
tagsLen: 30,
descLen: 50,
eventsLen: 31,
},
expected: expected{
tagsLen: 30,
descLen: 39,
eventsLen: 31,
},
},
{
description: "with eventsLen > maxChars/3, tagsLen and descLen <= maxChars/3",
given: given{
maxChars: 100,
tagsLen: 33,
descLen: 33,
eventsLen: 61,
},
expected: expected{
tagsLen: 33,
descLen: 33,
eventsLen: 34,
},
},
{
description: "with tagsLen and descLen > maxChars/3, eventsLen <= maxChars/3",
given: given{
maxChars: 100,
tagsLen: 55,
descLen: 46,
eventsLen: 31,
},
expected: expected{
tagsLen: 33,
descLen: 36,
eventsLen: 31,
},
},
{
description: "with tagsLen and eventsLen > maxChars/3, descLen <= maxChars/3",
given: given{
maxChars: 100,
tagsLen: 55,
descLen: 33,
eventsLen: 100,
},
expected: expected{
tagsLen: 33,
descLen: 33,
eventsLen: 34,
},
},
{
description: "with descLen and eventsLen > maxChars/3, tagsLen <= maxChars/3",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

название не соответствует given

given: given{
maxChars: 100,
tagsLen: 55,
descLen: 33,
eventsLen: 100,
},
expected: expected{
tagsLen: 33,
descLen: 33,
eventsLen: 34,
},
},
{
description: "with tagsLen, descLen and eventsLen > maxChars/3",
given: given{
maxChars: 100,
tagsLen: 55,
descLen: 40,
eventsLen: 100,
},
expected: expected{
tagsLen: 33,
descLen: 33,
eventsLen: 33,
},
},
{
description: "with tagsLen, descLen > maxChars/3, eventsLen <= maxChars/3 and maxChars - maxChars/3 - eventsLen > descLen",
given: given{
maxChars: 100,
tagsLen: 100,
descLen: 34,
eventsLen: 20,
},
expected: expected{
tagsLen: 33,
descLen: 34,
eventsLen: 20,
},
},
}

for i, c := range cases {
Convey(fmt.Sprintf("case %d: %s", i+1, c.description), func() {
tagsNewLen, descNewLen, eventsNewLen := CalculateMessagePartsBetweenTagsDescEvents(c.given.maxChars, c.given.tagsLen, c.given.descLen, c.given.eventsLen)

So(tagsNewLen, ShouldResemble, c.expected.tagsLen)
So(descNewLen, ShouldResemble, c.expected.descLen)
So(eventsNewLen, ShouldResemble, c.expected.eventsLen)
})
}
})
}
44 changes: 44 additions & 0 deletions senders/msgformat/defaults.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package msgformat

import "unicode/utf8"

// DefaultDescriptionCutter cuts description, so len(newDesc) <= maxSize. Ensure that len(desc) >= maxSize and
// maxSize >= len("...\n").
func DefaultDescriptionCutter(desc string, maxSize int) string {
suffix := "...\n"
return desc[:maxSize-len(suffix)] + suffix
}

// DefaultTagsLimiter cuts and formats tags to fit maxSize. There will be no tag parts, for example:
//
// if we have
//
// tags = []string{"tag1", "tag2}
// maxSize = 8
//
// so call DefaultTagsLimiter(tags, maxSize) will return " [tag1]".
func DefaultTagsLimiter(tags []string, maxSize int) string {
kissken marked this conversation as resolved.
Show resolved Hide resolved
tagsStr := " "
lenTagsStr := utf8.RuneCountInString(tagsStr)

for i := range tags {
lenTag := utf8.RuneCountInString(tags[i]) + 2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

чтобы не ломать голову почему тут +2 лучше вынести tagsStr повыше и считать длину от него


if lenTagsStr+lenTag > maxSize {
break
}

tagsStr += "[" + tags[i] + "]"
lenTagsStr += lenTag

if lenTagsStr == maxSize {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

лишняя проверка как будто, предыдущая отсекает этот кейс на следующей итерации

break
}
}

if tagsStr == " " {
return ""
}

return tagsStr
}
43 changes: 43 additions & 0 deletions senders/msgformat/defaults_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package msgformat

import (
"testing"

. "github.com/smartystreets/goconvey/convey"
)

func TestDefaultTagsLimiter(t *testing.T) {
Convey("Test default tags limiter", t, func() {
tags := []string{"tag1", "tag2"}

Convey("with maxSize < 0", func() {
tagsStr := DefaultTagsLimiter(tags, -1)

So(tagsStr, ShouldResemble, "")
})

Convey("with maxSize > total characters in tags string", func() {
tagsStr := DefaultTagsLimiter(tags, 30)

So(tagsStr, ShouldResemble, " [tag1][tag2]")
})

Convey("with maxSize not enough for all tags", func() {
tagsStr := DefaultTagsLimiter(tags, 8)

So(tagsStr, ShouldResemble, " [tag1]")
})

Convey("with one long tag > maxSize", func() {
tagsStr := DefaultTagsLimiter([]string{"long_tag"}, 4)

So(tagsStr, ShouldResemble, "")
})

Convey("with no tags", func() {
tagsStr := DefaultTagsLimiter([]string{}, 0)

So(tagsStr, ShouldResemble, "")
})
})
}
Loading
Loading