diff --git a/core/keyop/keyop.go b/core/keyop/keyop.go index e399e068a..5d2d00930 100644 --- a/core/keyop/keyop.go +++ b/core/keyop/keyop.go @@ -108,11 +108,17 @@ func New(key key.T, op Op, value string, idx int) *T { } } +// ParseOps function processes a list of strings, parses them into operations, +// filters out any invalid operations (based on the IsZero check), +// and returns a list of valid operations. func ParseOps(kws []string) L { - l := make(L, len(kws)) - for i, kw := range kws { + l := make(L, 0, len(kws)) + for _, kw := range kws { op := Parse(kw) - l[i] = *op + if op.IsZero() { + continue + } + l = append(l, *op) } return l } diff --git a/core/keyop/keyop_test.go b/core/keyop/keyop_test.go index a173f5e56..e1d27512c 100644 --- a/core/keyop/keyop_test.go +++ b/core/keyop/keyop_test.go @@ -8,7 +8,7 @@ import ( "github.com/opensvc/om3/util/key" ) -func TestKeyopsDrop(t *testing.T) { +func TestDrop(t *testing.T) { op1 := T{ Key: key.Parse("topology"), Op: Set, @@ -29,7 +29,7 @@ func TestKeyopsDrop(t *testing.T) { assert.Len(t, ops, 1) } -func TestKeyopParse(t *testing.T) { +func TestParse(t *testing.T) { tests := []struct { expr string key key.T @@ -126,3 +126,45 @@ func TestKeyopParse(t *testing.T) { }) } } + +func TestParseOps(t *testing.T) { + cases := []struct { + l []string + expected L + }{ + { + l: []string{"foo=bar"}, + expected: L{ + T{Key: key.T{Section: "DEFAULT", Option: "foo"}, Op: 1, Value: "bar", Index: 0}, + }, + }, + { + l: []string{"foo1=bar1", "must_be_dropped", "foo2=bar2"}, + expected: L{ + T{Key: key.T{Section: "DEFAULT", Option: "foo1"}, Op: 1, Value: "bar1", Index: 0}, + T{Key: key.T{Section: "DEFAULT", Option: "foo2"}, Op: 1, Value: "bar2", Index: 0}, + }, + }, + { + l: []string{"must_be_dropped"}, + expected: L{}, + }, + { + l: []string{""}, + expected: L{}, + }, + { + l: []string{}, + expected: L{}, + }, + { + l: nil, + expected: L{}, + }, + } + for _, tc := range cases { + t.Logf("ParseOps(%q)", tc.l) + ops := ParseOps(tc.l) + assert.Equal(t, tc.expected, ops) + } +} diff --git a/daemon/daemonapi/post_object_config_update.go b/daemon/daemonapi/post_object_config_update.go index 3dfbe7697..f5026e86a 100644 --- a/daemon/daemonapi/post_object_config_update.go +++ b/daemon/daemonapi/post_object_config_update.go @@ -36,7 +36,14 @@ func (a *DaemonAPI) PostObjectConfigUpdate(ctx echo.Context, namespace string, k unsets = key.ParseStrings(*params.Unset) } if params.Delete != nil { - deletes = *params.Delete + for _, section := range *params.Delete { + if section == "" { + // Prevents from accidental remove DEFAULT section. SectionsByName("") + // return "DEFAULT". Use explicit section="DEFAULT" to remove DEFAULT section. + continue + } + deletes = append(deletes, section) + } } if len(sets)+len(unsets)+len(deletes) == 0 { return JSONProblemf(ctx, http.StatusBadRequest, "No valid update requested", "") diff --git a/util/key/key.go b/util/key/key.go index a7d0ff564..9765843b1 100644 --- a/util/key/key.go +++ b/util/key/key.go @@ -20,15 +20,27 @@ func New(section, option string) T { } } +// ParseStrings function processes a list of strings, parses them into keyword, +// filters out any invalid keyword (based on the IsZero check), +// and returns a list of valid keywords. func ParseStrings(l []string) L { kws := make(L, 0) for _, s := range l { + kw := Parse(s) + if kw.IsZero() { + continue + } kws = append(kws, Parse(s)) } return kws } +// Parse function construct key T from the parsed string s. +// On invalid string s the zero key is returned. func Parse(s string) T { + if s == "" || strings.ContainsAny(s, " \t") { + return T{} + } l := strings.SplitN(s, ".", 2) switch len(l) { case 1: @@ -67,3 +79,7 @@ func (t T) String() string { } return t.Section + "." + t.Option } + +func (t T) IsZero() bool { + return (t.Option + t.Section) == "" +} diff --git a/util/key/key_test.go b/util/key/key_test.go index b1c420601..b1f6f2e0f 100644 --- a/util/key/key_test.go +++ b/util/key/key_test.go @@ -7,71 +7,103 @@ import ( ) func TestKey(t *testing.T) { - tests := []struct { - s string - section string - option string - render string - scope string + cases := []struct { + s string + t T + render string + scope string + isZero bool }{ { - s: "topology", - section: "DEFAULT", - option: "topology", - render: "topology", - scope: "", + s: "topology", + t: T{"DEFAULT", "topology"}, + render: "topology", }, { - s: "DEFAULT.topology", - section: "DEFAULT", - option: "topology", - render: "topology", - scope: "", + s: "DEFAULT.topology", + t: T{"DEFAULT", "topology"}, + render: "topology", }, { - s: "topology@nodes", - section: "DEFAULT", - option: "topology@nodes", - render: "topology@nodes", - scope: "nodes", + s: "topology@nodes", + t: T{"DEFAULT", "topology@nodes"}, + render: "topology@nodes", + scope: "nodes", }, { - s: "DEFAULT.topology@nodes", - section: "DEFAULT", - option: "topology@nodes", - render: "topology@nodes", - scope: "nodes", + s: "DEFAULT.topology@nodes", + t: T{"DEFAULT", "topology@nodes"}, + render: "topology@nodes", + scope: "nodes", }, { - s: "fs#1.dev", - section: "fs#1", - option: "dev", - render: "fs#1.dev", - scope: "", + s: "fs#1.dev", + t: T{"fs#1", "dev"}, + render: "fs#1.dev", }, { - s: "data.a.b/C.D", - section: "data", - option: "a.b/C.D", - render: "data.a.b/C.D", - scope: "", + s: "data.a.b/C.D", + t: T{"data", "a.b/C.D"}, + render: "data.a.b/C.D", }, { - s: "container#1", - section: "container#1", - option: "", - render: "container#1", - scope: "", + s: "container#1", + t: T{"container#1", ""}, + render: "container#1", + }, + { + s: ".foo", + t: T{Option: "foo"}, + render: ".foo", + }, + // test cases where Parse must return zero T + { + s: "", + isZero: true, + }, + { + s: ".", + isZero: true, + }, + { + s: " ", + isZero: true, + }, + { + s: "a ", + isZero: true, + }, + { + s: " b", + isZero: true, + }, + { + s: " .foo", + isZero: true, + }, + { + s: " foo.bar", + isZero: true, + }, + { + s: "foo.bar ", + isZero: true, + }, + { + s: "\tfoo.bar", + isZero: true, }, } - for _, test := range tests { - t.Logf("%s", test.s) - k := Parse(test.s) + for _, tc := range cases { + t.Logf("verify after k := Parse(%q) if k == %#v, k.String() == %q, k.Scope() == %q and k.IsZero() == %v", + tc.s, tc.t, tc.render, tc.scope, tc.isZero) + k := Parse(tc.s) render := k.String() - assert.Equal(t, test.render, render) - assert.Equal(t, test.section, k.Section) - assert.Equal(t, test.option, k.Option) - assert.Equal(t, test.scope, k.Scope()) + assert.Equal(t, tc.render, render, "k.String()") + assert.Equal(t, tc.t.Section, k.Section, "k.Section") + assert.Equal(t, tc.t.Option, k.Option, "k.Option") + assert.Equal(t, tc.scope, k.Scope(), "k.Scope()") + assert.Equal(t, tc.isZero, k.IsZero(), "k.IsZero()") } }