From e2fc7695e07062a3b382d4dc74c5a67ec75aac7a Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Wed, 13 Mar 2024 16:01:41 +0100 Subject: [PATCH 1/5] Introduce types.MakeString() --- pkg/types/string.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pkg/types/string.go b/pkg/types/string.go index f8ead450c..ce2a4ac69 100644 --- a/pkg/types/string.go +++ b/pkg/types/string.go @@ -15,6 +15,14 @@ type String struct { sql.NullString } +// MakeString constructs a new non-NULL String from s. +func MakeString(s string) String { + return String{sql.NullString{ + String: s, + Valid: true, + }} +} + // MarshalJSON implements the json.Marshaler interface. // Supports JSON null. func (s String) MarshalJSON() ([]byte, error) { From d36ade1f14fb39b9db546a2277cccad680ab9410 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Mon, 25 Mar 2024 14:49:06 +0100 Subject: [PATCH 2/5] Test Flatten() --- pkg/flatten/flatten_test.go | 44 +++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 pkg/flatten/flatten_test.go diff --git a/pkg/flatten/flatten_test.go b/pkg/flatten/flatten_test.go new file mode 100644 index 000000000..0655550f1 --- /dev/null +++ b/pkg/flatten/flatten_test.go @@ -0,0 +1,44 @@ +package flatten + +import ( + "github.com/icinga/icingadb/pkg/types" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestFlatten(t *testing.T) { + for _, st := range []struct { + name string + prefix string + value any + output map[string]types.String + }{ + {"nil", "a", nil, map[string]types.String{"a": types.MakeString("null")}}, + {"bool", "b", true, map[string]types.String{"b": types.MakeString("true")}}, + {"int", "c", 42, map[string]types.String{"c": types.MakeString("42")}}, + {"float", "d", 77.7, map[string]types.String{"d": types.MakeString("77.7")}}, + {"string", "f", "\x00", map[string]types.String{"f": types.MakeString("\x00")}}, + {"nil_slice", "g", []any(nil), map[string]types.String{"g": {}}}, + {"empty_slice", "h", []any{}, map[string]types.String{"h": {}}}, + {"slice", "i", []any{nil}, map[string]types.String{"i[0]": types.MakeString("null")}}, + {"nil_map", "j", map[string]any(nil), map[string]types.String{"j": {}}}, + {"empty_map", "k", map[string]any{}, map[string]types.String{"k": {}}}, + {"map", "l", map[string]any{" ": nil}, map[string]types.String{"l. ": types.MakeString("null")}}, + {"map_with_slice", "m", map[string]any{"\t": []any{"ä", "ö", "ü"}, "ß": "s"}, map[string]types.String{ + "m.\t[0]": types.MakeString("ä"), + "m.\t[1]": types.MakeString("ö"), + "m.\t[2]": types.MakeString("ü"), + "m.ß": types.MakeString("s"), + }}, + {"slice_with_map", "n", []any{map[string]any{"ä": "a", "ö": "o", "ü": "u"}, "ß"}, map[string]types.String{ + "n[0].ä": types.MakeString("a"), + "n[0].ö": types.MakeString("o"), + "n[0].ü": types.MakeString("u"), + "n[1]": types.MakeString("ß"), + }}, + } { + t.Run(st.name, func(t *testing.T) { + assert.Equal(t, st.output, Flatten(st.value, st.prefix)) + }) + } +} From 10afc562ceecbc4aaed321f94b17532310969497 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Wed, 13 Mar 2024 16:08:02 +0100 Subject: [PATCH 3/5] Use types.MakeString() instead of manual initialization (refactor) --- cmd/icingadb-migrate/convert.go | 25 +++++-------------------- pkg/flatten/flatten.go | 3 +-- pkg/icingadb/ha.go | 7 +------ 3 files changed, 7 insertions(+), 28 deletions(-) diff --git a/cmd/icingadb-migrate/convert.go b/cmd/icingadb-migrate/convert.go index e14746e46..64a6b3498 100644 --- a/cmd/icingadb-migrate/convert.go +++ b/cmd/icingadb-migrate/convert.go @@ -150,19 +150,9 @@ func convertCommentRows( }, AckHistoryUpserter: history.AckHistoryUpserter{ClearTime: clearTime}, SetTime: setTime, - Author: icingadbTypes.String{ - NullString: sql.NullString{ - String: row.AuthorName, - Valid: true, - }, - }, - Comment: icingadbTypes.String{ - NullString: sql.NullString{ - String: row.CommentData, - Valid: true, - }, - }, - ExpireTime: convertTime(row.ExpirationTime, 0), + Author: icingadbTypes.MakeString(row.AuthorName), + Comment: icingadbTypes.MakeString(row.CommentData), + ExpireTime: convertTime(row.ExpirationTime, 0), IsPersistent: icingadbTypes.Bool{ Bool: row.IsPersistent != 0, Valid: true, @@ -656,13 +646,8 @@ func convertNotificationRows( SendTime: ts, State: row.State, PreviousHardState: previousHardState, - Text: icingadbTypes.String{ - NullString: sql.NullString{ - String: text, - Valid: true, - }, - }, - UsersNotified: row.ContactsNotified, + Text: icingadbTypes.MakeString(text), + UsersNotified: row.ContactsNotified, }) allHistory = append(allHistory, &history.HistoryNotification{ diff --git a/pkg/flatten/flatten.go b/pkg/flatten/flatten.go index 94a6e7ebc..7ce8c6a91 100644 --- a/pkg/flatten/flatten.go +++ b/pkg/flatten/flatten.go @@ -1,7 +1,6 @@ package flatten import ( - "database/sql" "fmt" "github.com/icinga/icingadb/pkg/types" "strconv" @@ -37,7 +36,7 @@ func Flatten(value interface{}, prefix string) map[string]types.String { if value != nil { val = fmt.Sprintf("%v", value) } - flattened[key] = types.String{NullString: sql.NullString{String: val, Valid: true}} + flattened[key] = types.MakeString(val) } } diff --git a/pkg/icingadb/ha.go b/pkg/icingadb/ha.go index 74d3b3234..de31f773f 100644 --- a/pkg/icingadb/ha.go +++ b/pkg/icingadb/ha.go @@ -186,12 +186,7 @@ func (h *HA) controller() { EntityWithoutChecksum: v1.EntityWithoutChecksum{IdMeta: v1.IdMeta{ Id: envId, }}, - Name: types.String{ - NullString: sql.NullString{ - String: envId.String(), - Valid: true, - }, - }, + Name: types.MakeString(envId.String()), } h.environmentMu.Unlock() } From 365f97d092b61a798fc18ff83304319e7766e5b5 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Mon, 25 Mar 2024 13:27:51 +0100 Subject: [PATCH 4/5] Flatten(): type-check input only once --- pkg/flatten/flatten.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/pkg/flatten/flatten.go b/pkg/flatten/flatten.go index 7ce8c6a91..127cbfbde 100644 --- a/pkg/flatten/flatten.go +++ b/pkg/flatten/flatten.go @@ -31,12 +31,10 @@ func Flatten(value interface{}, prefix string) map[string]types.String { for i, v := range value { flatten(key+"["+strconv.Itoa(i)+"]", v) } + case nil: + flattened[key] = types.MakeString("null") default: - val := "null" - if value != nil { - val = fmt.Sprintf("%v", value) - } - flattened[key] = types.MakeString(val) + flattened[key] = types.MakeString(fmt.Sprintf("%v", value)) } } From 17b63b214d68e7bad3b5b27382f5457ac01a28e2 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Mon, 25 Mar 2024 13:30:39 +0100 Subject: [PATCH 5/5] Flatten(): render even large numbers as-is, not using scientific notation E.g. 2000000000000000000 (explicitly), not 2e+18 (as with fmt.Sprintf("%v")). --- pkg/flatten/flatten.go | 2 ++ pkg/flatten/flatten_test.go | 1 + 2 files changed, 3 insertions(+) diff --git a/pkg/flatten/flatten.go b/pkg/flatten/flatten.go index 127cbfbde..698eff178 100644 --- a/pkg/flatten/flatten.go +++ b/pkg/flatten/flatten.go @@ -33,6 +33,8 @@ func Flatten(value interface{}, prefix string) map[string]types.String { } case nil: flattened[key] = types.MakeString("null") + case float64: + flattened[key] = types.MakeString(strconv.FormatFloat(value, 'f', -1, 64)) default: flattened[key] = types.MakeString(fmt.Sprintf("%v", value)) } diff --git a/pkg/flatten/flatten_test.go b/pkg/flatten/flatten_test.go index 0655550f1..f84b8d9ec 100644 --- a/pkg/flatten/flatten_test.go +++ b/pkg/flatten/flatten_test.go @@ -17,6 +17,7 @@ func TestFlatten(t *testing.T) { {"bool", "b", true, map[string]types.String{"b": types.MakeString("true")}}, {"int", "c", 42, map[string]types.String{"c": types.MakeString("42")}}, {"float", "d", 77.7, map[string]types.String{"d": types.MakeString("77.7")}}, + {"large_float", "e", 1e23, map[string]types.String{"e": types.MakeString("100000000000000000000000")}}, {"string", "f", "\x00", map[string]types.String{"f": types.MakeString("\x00")}}, {"nil_slice", "g", []any(nil), map[string]types.String{"g": {}}}, {"empty_slice", "h", []any{}, map[string]types.String{"h": {}}},