Merge pull request #601 from Icinga/flatten-empty-custom-vars-correctly

Write a hint for empty arrays/dicts into `customvar_flat`
This commit is contained in:
Julian Brost 2023-07-25 15:28:07 +02:00 committed by GitHub
commit 68d26a6873
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 118 additions and 71 deletions

View file

@ -1,26 +1,43 @@
package flatten
import (
"database/sql"
"fmt"
"github.com/icinga/icingadb/pkg/types"
"strconv"
)
// Flatten creates flat, one-dimensional maps from arbitrarily nested values, e.g. JSON.
func Flatten(value interface{}, prefix string) map[string]interface{} {
func Flatten(value interface{}, prefix string) map[string]types.String {
var flatten func(string, interface{})
flattened := make(map[string]interface{})
flattened := make(map[string]types.String)
flatten = func(key string, value interface{}) {
switch value := value.(type) {
case map[string]interface{}:
if len(value) == 0 {
flattened[key] = types.String{}
break
}
for k, v := range value {
flatten(key+"."+k, v)
}
case []interface{}:
if len(value) == 0 {
flattened[key] = types.String{}
break
}
for i, v := range value {
flatten(key+"["+strconv.Itoa(i)+"]", v)
}
default:
flattened[key] = value
val := "null"
if value != nil {
val = fmt.Sprintf("%v", value)
}
flattened[key] = types.String{NullString: sql.NullString{String: val, Valid: true}}
}
}

View file

@ -2,7 +2,6 @@ package v1
import (
"context"
"fmt"
"github.com/icinga/icingadb/internal"
"github.com/icinga/icingadb/pkg/com"
"github.com/icinga/icingadb/pkg/contracts"
@ -25,7 +24,7 @@ type CustomvarFlat struct {
CustomvarMeta `json:",inline"`
Flatname string `json:"flatname"`
FlatnameChecksum types.Binary `json:"flatname_checksum"`
Flatvalue string `json:"flatvalue"`
Flatvalue types.String `json:"flatvalue"`
}
func NewCustomvar() contracts.Entity {
@ -117,11 +116,9 @@ func flattenCustomvars(ctx context.Context, g *errgroup.Group, cvs <-chan contra
flattened := flatten.Flatten(value, customvar.Name)
for flatname, flatvalue := range flattened {
var fv string
if flatvalue == nil {
fv = "null"
} else {
fv = fmt.Sprintf("%v", flatvalue)
var fv interface{}
if flatvalue.Valid {
fv = flatvalue.String
}
select {
@ -131,7 +128,7 @@ func flattenCustomvars(ctx context.Context, g *errgroup.Group, cvs <-chan contra
IdMeta: IdMeta{
// TODO(el): Schema comment is wrong.
// Without customvar.Id we would produce duplicate keys here.
Id: utils.Checksum(objectpacker.MustPackSlice(customvar.EnvironmentId, customvar.Id, flatname, flatvalue)),
Id: utils.Checksum(objectpacker.MustPackSlice(customvar.EnvironmentId, customvar.Id, flatname, fv)),
},
},
EnvironmentMeta: EnvironmentMeta{
@ -141,7 +138,7 @@ func flattenCustomvars(ctx context.Context, g *errgroup.Group, cvs <-chan contra
},
Flatname: flatname,
FlatnameChecksum: utils.Checksum(flatname),
Flatvalue: fv,
Flatvalue: flatvalue,
}:
case <-ctx.Done():
return ctx.Err()

View file

@ -983,7 +983,7 @@ CREATE TABLE customvar_flat (
flatname_checksum binary(20) NOT NULL COMMENT 'sha1(flatname after conversion)',
flatname varchar(512) COLLATE utf8mb4_unicode_ci NOT NULL COMMENT 'Path converted with `.` and `[ ]`',
flatvalue text NOT NULL,
flatvalue text DEFAULT NULL,
PRIMARY KEY (id),

View file

@ -0,0 +1 @@
ALTER TABLE customvar_flat MODIFY COLUMN flatvalue text DEFAULT NULL;

View file

@ -1553,7 +1553,7 @@ CREATE TABLE customvar_flat (
flatname_checksum bytea20 NOT NULL,
flatname citext NOT NULL,
flatvalue text NOT NULL,
flatvalue text DEFAULT NULL,
CONSTRAINT pk_customvar_flat PRIMARY KEY (id)
);

View file

@ -0,0 +1 @@
ALTER TABLE customvar_flat ALTER COLUMN flatvalue DROP NOT NULL;

View file

@ -3,6 +3,7 @@ package icingadb_test
import (
"bytes"
"context"
"database/sql"
_ "embed"
"fmt"
"github.com/go-redis/redis/v8"
@ -1194,9 +1195,9 @@ func newString(s string) *string {
}
type CustomVarTestData struct {
Value interface{} // Value to put into config or API
Vars map[string]string // Expected values in customvar table
VarsFlat map[string]string // Expected values in customvar_flat table
Value interface{} // Value to put into config or API
Vars map[string]string // Expected values in customvar table
VarsFlat map[string]sql.NullString // Expected values in customvar_flat table
}
func (c *CustomVarTestData) Icinga2ConfigValue() string {
@ -1246,20 +1247,22 @@ func (c *CustomVarTestData) verify(t require.TestingT, logger *zap.Logger, db *s
require.NoError(t, err, "querying customvars")
defer func() { _ = rows.Close() }()
expectedSrc := c.Vars
if flat {
expectedSrc = c.VarsFlat
}
// copy map to remove items while reading from the database
expected := make(map[string]string)
for k, v := range expectedSrc {
expected[k] = v
expected := make(map[string]sql.NullString)
if flat {
for k, v := range c.VarsFlat {
expected[k] = v
}
} else {
for k, v := range c.Vars {
expected[k] = toDBString(v)
}
}
for rows.Next() {
var cvName, cvValue string
err := rows.Scan(&cvName, &cvValue)
var cvName string
var cvValue sql.NullString
err = rows.Scan(&cvName, &cvValue)
require.NoError(t, err, "scanning query row")
logger.Debug("custom var from database",
@ -1267,13 +1270,13 @@ func (c *CustomVarTestData) verify(t require.TestingT, logger *zap.Logger, db *s
zap.String("object-type", typeName),
zap.Any("object-name", name),
zap.String("custom-var-name", cvName),
zap.String("custom-var-value", cvValue))
zap.String("custom-var-value", cvValue.String))
if cvExpected, ok := expected[cvName]; ok {
assert.Equalf(t, cvExpected, cvValue, "custom var %q", cvName)
delete(expected, cvName)
} else if !ok {
assert.Failf(t, "unexpected custom var", "%q: %q", cvName, cvValue)
assert.Failf(t, "unexpected custom var", "%q: %q", cvName, cvValue.String)
}
}
@ -1299,9 +1302,33 @@ func makeCustomVarTestData(t *testing.T) []*CustomVarTestData {
id + "-hello": `"` + id + ` world"`,
id + "-foo": `"` + id + ` bar"`,
},
VarsFlat: map[string]string{
id + "-hello": id + " world",
id + "-foo": id + " bar",
VarsFlat: map[string]sql.NullString{
id + "-hello": toDBString(id + " world"),
id + "-foo": toDBString(id + " bar"),
},
})
// empty custom vars of type array and dictionaries
data = append(data, &CustomVarTestData{
Value: map[string]interface{}{
id + "-empty-list": []interface{}{},
id + "-empty-nested-list": []interface{}{[]interface{}{}},
id + "-empty-dict": map[string]interface{}{},
id + "-empty-nested-dict": map[string]interface{}{
"some-key": map[string]interface{}{},
},
},
Vars: map[string]string{
id + "-empty-list": `[]`,
id + "-empty-nested-list": `[[]]`,
id + "-empty-dict": `{}`,
id + "-empty-nested-dict": `{"some-key":{}}`,
},
VarsFlat: map[string]sql.NullString{
id + "-empty-list": {},
id + "-empty-nested-list[0]": {},
id + "-empty-dict": {},
id + "-empty-nested-dict.some-key": {},
},
})
@ -1342,29 +1369,29 @@ func makeCustomVarTestData(t *testing.T) []*CustomVarTestData {
id + "-nested-dict": `{"array":["answer?",42],"dict":{"another-key":"another-value","yet-another-key":4711},"top-level-entry":"good morning"}`,
id + "-nested-array": `[[1,2,3],{"contains-a-map":"yes","really?":true},-42]`,
},
VarsFlat: map[string]string{
id + "-array[0]": `foo`,
id + "-array[1]": `23`,
id + "-array[2]": `bar`,
id + "-dict.some-key": `some-value`,
id + "-dict.other-key": `other-value`,
id + "-string": `hello icinga`,
id + "-int": `-1`,
id + "-float": `13.37`,
id + "-true": `true`,
id + "-false": `false`,
id + "-null": `null`,
id + "-nested-dict.dict.another-key": `another-value`,
id + "-nested-dict.dict.yet-another-key": `4711`,
id + "-nested-dict.array[0]": `answer?`,
id + "-nested-dict.array[1]": `42`,
id + "-nested-dict.top-level-entry": `good morning`,
id + "-nested-array[0][0]": `1`,
id + "-nested-array[0][1]": `2`,
id + "-nested-array[0][2]": `3`,
id + "-nested-array[1].contains-a-map": `yes`,
id + "-nested-array[1].really?": `true`,
id + "-nested-array[2]": `-42`,
VarsFlat: map[string]sql.NullString{
id + "-array[0]": toDBString(`foo`),
id + "-array[1]": toDBString(`23`),
id + "-array[2]": toDBString(`bar`),
id + "-dict.some-key": toDBString(`some-value`),
id + "-dict.other-key": toDBString(`other-value`),
id + "-string": toDBString(`hello icinga`),
id + "-int": toDBString(`-1`),
id + "-float": toDBString(`13.37`),
id + "-true": toDBString(`true`),
id + "-false": toDBString(`false`),
id + "-null": toDBString(`null`),
id + "-nested-dict.dict.another-key": toDBString(`another-value`),
id + "-nested-dict.dict.yet-another-key": toDBString(`4711`),
id + "-nested-dict.array[0]": toDBString(`answer?`),
id + "-nested-dict.array[1]": toDBString(`42`),
id + "-nested-dict.top-level-entry": toDBString(`good morning`),
id + "-nested-array[0][0]": toDBString(`1`),
id + "-nested-array[0][1]": toDBString(`2`),
id + "-nested-array[0][2]": toDBString(`3`),
id + "-nested-array[1].contains-a-map": toDBString(`yes`),
id + "-nested-array[1].really?": toDBString(`true`),
id + "-nested-array[2]": toDBString(`-42`),
},
})
@ -1380,14 +1407,14 @@ func makeCustomVarTestData(t *testing.T) []*CustomVarTestData {
"b": `["bar",42,-13.37]`,
"c": `{"a":true,"b":false,"c":null}`,
},
VarsFlat: map[string]string{
"a": "foo",
"b[0]": `bar`,
"b[1]": `42`,
"b[2]": `-13.37`,
"c.a": `true`,
"c.b": `false`,
"c.c": `null`,
VarsFlat: map[string]sql.NullString{
"a": toDBString("foo"),
"b[0]": toDBString(`bar`),
"b[1]": toDBString(`42`),
"b[2]": toDBString(`-13.37`),
"c.a": toDBString(`true`),
"c.b": toDBString(`false`),
"c.c": toDBString(`null`),
},
}, &CustomVarTestData{
Value: map[string]interface{}{
@ -1400,16 +1427,20 @@ func makeCustomVarTestData(t *testing.T) []*CustomVarTestData {
"b": `[true,false,null]`,
"c": `{"a":"foo","b":"bar","c":42}`,
},
VarsFlat: map[string]string{
"a": `-13.37`,
"b[0]": `true`,
"b[1]": `false`,
"b[2]": `null`,
"c.a": "foo",
"c.b": `bar`,
"c.c": `42`,
VarsFlat: map[string]sql.NullString{
"a": toDBString(`-13.37`),
"b[0]": toDBString(`true`),
"b[1]": toDBString(`false`),
"b[2]": toDBString(`null`),
"c.a": toDBString("foo"),
"c.b": toDBString(`bar`),
"c.c": toDBString(`42`),
},
})
return data
}
func toDBString(str string) sql.NullString {
return sql.NullString{String: str, Valid: true}
}