Skip to content

Commit

Permalink
utils.IterateOrderedMap for plugin.FormatMessage
Browse files Browse the repository at this point in the history
The "Tags" and "Extra Tags" printed in the plugin.FormatMessage were
directly read from their map, thus having no order. This resulted in the
same NotificationRequest being represented by different messages due to
the unordered map.

This change was the result of investigating Go's new rangefunc
experiment[0]. The utilization of this novel language feature - which can
also be indirectly used in the absence of `GOEXPERIMENT=rangefunc` - ensures
that the map is traversed in key order.

Closes #177.

[0]: https://go.dev/wiki/RangefuncExperiment
  • Loading branch information
oxzi authored and julianbrost committed Apr 25, 2024
1 parent 4190d09 commit e371553
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 4 deletions.
26 changes: 26 additions & 0 deletions internal/utils/utils.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package utils

import (
"cmp"
"context"
"database/sql"
"fmt"
Expand All @@ -10,6 +11,7 @@ import (
"github.com/icinga/icingadb/pkg/utils"
"github.com/jmoiron/sqlx"
"github.com/pkg/errors"
"slices"
"strings"
)

Expand Down Expand Up @@ -157,3 +159,27 @@ func RemoveNils[T any](slice []*T) []*T {
return ptr == nil
})
}

// IterateOrderedMap implements iter.Seq2 to iterate over a map in the key's order.
//
// This function returns a func yielding key-value-pairs from a given map in the order of their keys, if their type
// is cmp.Ordered.
//
// Please note that currently - being at Go 1.22 - rangefuncs are still an experimental feature and cannot be directly
// used unless compiled with `GOEXPERIMENT=rangefunc`. However, they can still be invoked normally.
// https://go.dev/wiki/RangefuncExperiment
func IterateOrderedMap[K cmp.Ordered, V any](m map[K]V) func(func(K, V) bool) {
keys := make([]K, 0, len(m))
for key := range m {
keys = append(keys, key)
}
slices.Sort(keys)

return func(yield func(K, V) bool) {
for _, key := range keys {
if !yield(key, m[key]) {
return
}
}
}
}
49 changes: 49 additions & 0 deletions internal/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,52 @@ func TestRemoveNils(t *testing.T) {
})
}
}

func TestIterateOrderedMap(t *testing.T) {
tests := []struct {
name string
in map[int]string
outKeys []int
}{
{"empty", map[int]string{}, nil},
{"single", map[int]string{1: "foo"}, []int{1}},
{"few-numbers", map[int]string{1: "a", 2: "b", 3: "c"}, []int{1, 2, 3}},
{
"1k-numbers",
func() map[int]string {
m := make(map[int]string)
for i := 0; i < 1000; i++ {
m[i] = "foo"
}
return m
}(),
func() []int {
keys := make([]int, 1000)
for i := 0; i < 1000; i++ {
keys[i] = i
}
return keys
}(),
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var outKeys []int

// Either run with GOEXPERIMENT=rangefunc or wait for rangefuncs to land in the next Go release.
// for k, _ := range IterateOrderedMap(tt.in) {
// outKeys = append(outKeys, k)
// }

// In the meantime, it can be invoked as follows.
IterateOrderedMap(tt.in)(func(k int, v string) bool {
assert.Equal(t, tt.in[k], v)
outKeys = append(outKeys, k)
return true
})

assert.Equal(t, tt.outKeys, outKeys)
})
}
}
11 changes: 7 additions & 4 deletions pkg/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"errors"
"fmt"
"github.com/icinga/icinga-notifications/internal/utils"
"github.com/icinga/icinga-notifications/pkg/rpc"
"github.com/icinga/icingadb/pkg/types"
"io"
Expand Down Expand Up @@ -201,15 +202,17 @@ func FormatMessage(writer io.Writer, req *NotificationRequest) {
}
_, _ = fmt.Fprintf(writer, "Object: %s\n\n", req.Object.Url)
_, _ = writer.Write([]byte("Tags:\n"))
for k, v := range req.Object.Tags {
utils.IterateOrderedMap(req.Object.Tags)(func(k, v string) bool {
_, _ = fmt.Fprintf(writer, "%s: %s\n", k, v)
}
return true
})

if len(req.Object.ExtraTags) > 0 {
_, _ = writer.Write([]byte("\nExtra Tags:\n"))
for k, v := range req.Object.ExtraTags {
utils.IterateOrderedMap(req.Object.ExtraTags)(func(k, v string) bool {
_, _ = fmt.Fprintf(writer, "%s: %s\n", k, v)
}
return true
})
}

_, _ = fmt.Fprintf(writer, "\nIncident: %s", req.Incident.Url)
Expand Down

0 comments on commit e371553

Please sign in to comment.