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.

Sign up for GitHub

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

Jump to bottom

Write a hint for empty arrays/dicts into customvar_flat #601

Merged
merged 2 commits into from Jul 25, 2023
Merged

Write a hint for empty arrays/dicts into customvar_flat #601

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fa0a712
Flatten empty custom vars of type `array` & `map` correctly
yhabteab Jun 16, 2023
23ac591
Add empty custom vars test cases
yhabteab Jun 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 20 additions & 3 deletions pkg/flatten/flatten.go
View file
Open in desktop
@@ -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
Open in desktop
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
Open in desktop
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
Open in desktop
@@ -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
Open in desktop
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
Open in desktop
@@ -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
Open in desktop
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)
}
yhabteab marked this conversation as resolved.
Show resolved Hide resolved
}

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}
}