commit 19aa95fc7f4cd58dcc8a8ff075762cfc86d41dc3
parent 923dd9d1c1f649142f3f377109318b07e0f44d5d
Author: Bjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>
Date: Sun, 27 Jun 2021 13:24:49 +0200
Fix config handling with empty config entries after merge
Fixes #8701
Diffstat:
4 files changed, 58 insertions(+), 5 deletions(-)
diff --git a/common/maps/params.go b/common/maps/params.go
@@ -52,6 +52,24 @@ func (p Params) Set(pp Params) {
}
}
+// IsZero returns true if p is considered empty.
+func (p Params) IsZero() bool {
+ if p == nil || len(p) == 0 {
+ return true
+ }
+
+ if len(p) > 1 {
+ return false
+ }
+
+ for k, _ := range p {
+ return k == mergeStrategyKey
+ }
+
+ return false
+
+}
+
// Merge transfers values from pp to p for new keys.
// This is done recursively.
func (p Params) Merge(pp Params) {
@@ -82,12 +100,9 @@ func (p Params) merge(ps ParamsMergeStrategy, pp Params) {
if pv, ok := v.(Params); ok {
vvv.merge(ms, pv)
}
-
}
-
} else if !noUpdate {
p[k] = v
-
}
}
diff --git a/common/maps/params_test.go b/common/maps/params_test.go
@@ -156,3 +156,15 @@ func TestParamsSetAndMerge(t *testing.T) {
})
}
+
+func TestParamsIsZero(t *testing.T) {
+ c := qt.New(t)
+
+ var nilParams Params
+
+ c.Assert(Params{}.IsZero(), qt.IsTrue)
+ c.Assert(nilParams.IsZero(), qt.IsTrue)
+ c.Assert(Params{"foo": "bar"}.IsZero(), qt.IsFalse)
+ c.Assert(Params{"_merge": "foo", "foo": "bar"}.IsZero(), qt.IsFalse)
+ c.Assert(Params{"_merge": "foo"}.IsZero(), qt.IsTrue)
+}
diff --git a/config/defaultConfigProvider.go b/config/defaultConfigProvider.go
@@ -214,6 +214,7 @@ func (c *defaultConfigProvider) Merge(k string, v interface{}) {
if p, ok := maps.ToParamsAndPrepare(v); ok {
// As there may be keys in p not in root, we need to handle
// those as a special case.
+ var keysToDelete []string
for kk, vv := range p {
if pp, ok := vv.(maps.Params); ok {
if pppi, ok := c.root[kk]; ok {
@@ -261,14 +262,19 @@ func (c *defaultConfigProvider) Merge(k string, v interface{}) {
strategy := c.determineMergeStrategy(KeyParams{Key: "", Params: c.root}, KeyParams{Key: kk, Params: np})
np.SetDefaultMergeStrategy(strategy)
np.Merge(pp)
- if len(np) > 0 {
- c.root[kk] = np
+ c.root[kk] = np
+ if np.IsZero() {
+ // Just keep it until merge is done.
+ keysToDelete = append(keysToDelete, kk)
}
}
}
}
// Merge the rest.
c.root.Merge(p)
+ for _, k := range keysToDelete {
+ delete(c.root, k)
+ }
} else {
panic(fmt.Sprintf("unsupported type %T received in Merge", v))
}
diff --git a/config/defaultConfigProvider_test.go b/config/defaultConfigProvider_test.go
@@ -283,6 +283,26 @@ func TestDefaultConfigProvider(t *testing.T) {
})
+ // Issue #8701
+ c.Run("Prevent _merge only maps", func(c *qt.C) {
+ cfg := New()
+
+ cfg.Set("", map[string]interface{}{
+ "B": "bv",
+ })
+
+ cfg.Merge("", map[string]interface{}{
+ "c": map[string]interface{}{
+ "_merge": "shallow",
+ "d": "dv2",
+ },
+ })
+
+ c.Assert(cfg.Get(""), qt.DeepEquals, maps.Params{
+ "b": "bv",
+ })
+ })
+
c.Run("IsSet", func(c *qt.C) {
cfg := New()