Fix config handling with empty config entries after merge
authorBjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>
Sun, 27 Jun 2021 11:24:49 +0000 (13:24 +0200)
committerBjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>
Sun, 27 Jun 2021 13:01:56 +0000 (15:01 +0200)
Fixes #8701

common/maps/params.go
common/maps/params_test.go
config/defaultConfigProvider.go
config/defaultConfigProvider_test.go

index e5a1bd07db17fcc17ebd74c07a125a188e3c0cf6..c14026df7bef3315f7b743d51588ee0fbb7c8a17 100644 (file)
@@ -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
-
                }
 
        }
index 8859bb86bc6f767eeeaa3fc495c4e5b8931fd25e..5c799aae188e61141b7004ad8cbe550062c94c28 100644 (file)
@@ -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)
+}
index a5e2d09fdd0d50b112c4684dddd04ae62e43fcf2..80353664e413f3c93bf37dc3829bee559351288c 100644 (file)
@@ -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))
                }
index 6752ab2e551c3328d3e2f4af604ce39bb359d850..7ab8c049a5cf9c08ffe5acf4810da8c30428c6c8 100644 (file)
@@ -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()