Skip to content

Commit

Permalink
Merge pull request #601 from Icinga/flatten-empty-custom-vars-correctly
Browse files Browse the repository at this point in the history 
Write a hint for empty arrays/dicts into `customvar_flat`
  • Loading branch information
@julianbrost
julianbrost authored Jul 25, 2023
2 parents 536c808 + 23ac591 commit 68d26a6
Show file tree
Hide file tree
Showing 7 changed files with 118 additions and 71 deletions.
23 changes: 20 additions & 3 deletions pkg/flatten/flatten.go
View file
Original file line number Diff line number Diff line change
@@ -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}}
}
}

Expand Down
15 changes: 6 additions & 9 deletions pkg/icingadb/v1/customvar.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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{
Expand All @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion schema/mysql/schema.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),

Expand Down
1 change: 1 addition & 0 deletions schema/mysql/upgrades/1.2.0.sql
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE customvar_flat MODIFY COLUMN flatvalue text DEFAULT NULL;
2 changes: 1 addition & 1 deletion schema/pgsql/schema.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
Expand Down
1 change: 1 addition & 0 deletions schema/pgsql/upgrades/1.2.0.sql
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE customvar_flat ALTER COLUMN flatvalue DROP NOT NULL;
145 changes: 88 additions & 57 deletions tests/object_sync_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package icingadb_test
import (
"bytes"
"context"
"database/sql"
_ "embed"
"fmt"
"github.com/go-redis/redis/v8"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -1246,34 +1247,36 @@ 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",
zap.Bool("flat", flat),
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)
}
}

Expand All @@ -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": {},
},
})

Expand Down Expand Up @@ -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`),
},
})

Expand All @@ -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{}{
Expand All @@ -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}
}

0 comments on commit 68d26a6

Please sign in to comment.