From 4a9d408fe0bbf4c563546e35d2be7ade4e920c4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Tue, 22 Jun 2021 09:53:37 +0200 Subject: [PATCH] config: Fix merge of config with map[string]string values. Fixes #8679 --- common/maps/maps.go | 9 ++++ common/maps/maps_test.go | 14 +++-- common/maps/params.go | 12 ++++- config/compositeConfig.go | 4 ++ config/configProvider.go | 1 + config/defaultConfigProvider.go | 20 ++++--- config/defaultConfigProvider_test.go | 79 ++++++++++++++++++++++++++++ hugolib/config.go | 10 ++-- hugolib/content_render_hooks_test.go | 1 + 9 files changed, 133 insertions(+), 17 deletions(-) diff --git a/common/maps/maps.go b/common/maps/maps.go index 5fb0790092f..79fcc23d000 100644 --- a/common/maps/maps.go +++ b/common/maps/maps.go @@ -49,6 +49,15 @@ func ToParamsAndPrepare(in interface{}) (Params, bool) { return m, true } +// MustToParamsAndPrepare calls ToParamsAndPrepare and panics if it fails. +func MustToParamsAndPrepare(in interface{}) Params { + if p, ok := ToParamsAndPrepare(in); ok { + return p + } else { + panic(fmt.Sprintf("cannot convert %T to maps.Params", in)) + } +} + // ToStringMap converts in to map[string]interface{}. func ToStringMap(in interface{}) map[string]interface{} { m, _ := ToStringMapE(in) diff --git a/common/maps/maps_test.go b/common/maps/maps_test.go index dbe97a15aef..ba3c2508757 100644 --- a/common/maps/maps_test.go +++ b/common/maps/maps_test.go @@ -21,10 +21,10 @@ import ( qt "github.com/frankban/quicktest" ) -func TestToLower(t *testing.T) { +func TestPrepareParams(t *testing.T) { tests := []struct { - input map[string]interface{} - expected map[string]interface{} + input Params + expected Params }{ { map[string]interface{}{ @@ -47,6 +47,9 @@ func TestToLower(t *testing.T) { "gHi": map[string]interface{}{ "J": 25, }, + "jKl": map[string]string{ + "M": "26", + }, }, Params{ "abc": 32, @@ -60,13 +63,16 @@ func TestToLower(t *testing.T) { "ghi": Params{ "j": 25, }, + "jkl": Params{ + "m": "26", + }, }, }, } for i, test := range tests { t.Run(fmt.Sprint(i), func(t *testing.T) { - // ToLower modifies input. + // PrepareParams modifies input. PrepareParams(test.input) if !reflect.DeepEqual(test.expected, test.input) { t.Errorf("[%d] Expected\n%#v, got\n%#v\n", i, test.expected, test.input) diff --git a/common/maps/params.go b/common/maps/params.go index 7e94d593b59..e5a1bd07db1 100644 --- a/common/maps/params.go +++ b/common/maps/params.go @@ -226,7 +226,7 @@ func toMergeStrategy(v interface{}) ParamsMergeStrategy { // PrepareParams // * makes all the keys in the given map lower cased and will do so // * This will modify the map given. -// * Any nested map[interface{}]interface{} will be converted to Params. +// * Any nested map[interface{}]interface{}, map[string]interface{},map[string]string will be converted to Params. // * Any _merge value will be converted to proper type and value. func PrepareParams(m Params) { for k, v := range m { @@ -236,7 +236,7 @@ func PrepareParams(m Params) { v = toMergeStrategy(v) retyped = true } else { - switch v.(type) { + switch vv := v.(type) { case map[interface{}]interface{}: var p Params = cast.ToStringMap(v) v = p @@ -247,6 +247,14 @@ func PrepareParams(m Params) { v = p PrepareParams(p) retyped = true + case map[string]string: + p := make(Params) + for k, v := range vv { + p[k] = v + } + v = p + PrepareParams(p) + retyped = true } } diff --git a/config/compositeConfig.go b/config/compositeConfig.go index c68419533f5..92bb165b7c3 100644 --- a/config/compositeConfig.go +++ b/config/compositeConfig.go @@ -104,6 +104,10 @@ func (c *compositeConfig) Set(key string, value interface{}) { c.layer.Set(key, value) } +func (c *compositeConfig) SetDefaults(params maps.Params) { + c.layer.SetDefaults(params) +} + func (c *compositeConfig) WalkParams(walkFn func(params ...KeyParams) bool) { panic("not supported") } diff --git a/config/configProvider.go b/config/configProvider.go index 92206ca9e3e..481524c3b2b 100644 --- a/config/configProvider.go +++ b/config/configProvider.go @@ -30,6 +30,7 @@ type Provider interface { Get(key string) interface{} Set(key string, value interface{}) Merge(key string, value interface{}) + SetDefaults(params maps.Params) SetDefaultMergeStrategy() WalkParams(walkFn func(params ...KeyParams) bool) IsSet(key string) bool diff --git a/config/defaultConfigProvider.go b/config/defaultConfigProvider.go index d9c9db7f1dd..fd32c08a692 100644 --- a/config/defaultConfigProvider.go +++ b/config/defaultConfigProvider.go @@ -163,10 +163,9 @@ func (c *defaultConfigProvider) Set(k string, v interface{}) { } switch vv := v.(type) { - case map[string]interface{}: - var p maps.Params = vv + case map[string]interface{}, map[interface{}]interface{}, map[string]string: + p := maps.MustToParamsAndPrepare(vv) v = p - maps.PrepareParams(p) } key, m := c.getNestedKeyAndMap(k, true) @@ -183,6 +182,16 @@ func (c *defaultConfigProvider) Set(k string, v interface{}) { m[key] = v } +// SetDefaults will set values from params if not already set. +func (c *defaultConfigProvider) SetDefaults(params maps.Params) { + maps.PrepareParams(params) + for k, v := range params { + if _, found := c.root[k]; !found { + c.root[k] = v + } + } +} + func (c *defaultConfigProvider) Merge(k string, v interface{}) { c.mu.Lock() defer c.mu.Unlock() @@ -226,10 +235,9 @@ func (c *defaultConfigProvider) Merge(k string, v interface{}) { } switch vv := v.(type) { - case map[string]interface{}: - var p maps.Params = vv + case map[string]interface{}, map[interface{}]interface{}, map[string]string: + p := maps.MustToParamsAndPrepare(vv) v = p - maps.PrepareParams(p) } key, m := c.getNestedKeyAndMap(k, true) diff --git a/config/defaultConfigProvider_test.go b/config/defaultConfigProvider_test.go index 834165d9602..6752ab2e551 100644 --- a/config/defaultConfigProvider_test.go +++ b/config/defaultConfigProvider_test.go @@ -204,6 +204,85 @@ func TestDefaultConfigProvider(t *testing.T) { }) }) + // Issue #8679 + c.Run("Merge typed maps", func(c *qt.C) { + + for _, left := range []interface{}{ + map[string]string{ + "c": "cv1", + }, + map[string]interface{}{ + "c": "cv1", + }, + map[interface{}]interface{}{ + "c": "cv1", + }, + } { + cfg := New() + + cfg.Set("", map[string]interface{}{ + "b": left, + }) + + cfg.Merge("", maps.Params{ + "b": maps.Params{ + "c": "cv2", + "d": "dv2", + }, + }) + + c.Assert(cfg.Get(""), qt.DeepEquals, maps.Params{ + "b": maps.Params{ + "c": "cv1", + "d": "dv2", + }, + }) + } + + for _, left := range []interface{}{ + map[string]string{ + "b": "bv1", + }, + map[string]interface{}{ + "b": "bv1", + }, + map[interface{}]interface{}{ + "b": "bv1", + }, + } { + + for _, right := range []interface{}{ + map[string]string{ + "b": "bv2", + "c": "cv2", + }, + map[string]interface{}{ + "b": "bv2", + "c": "cv2", + }, + map[interface{}]interface{}{ + "b": "bv2", + "c": "cv2", + }, + } { + cfg := New() + + cfg.Set("a", left) + + cfg.Merge("a", right) + + c.Assert(cfg.Get(""), qt.DeepEquals, maps.Params{ + "a": maps.Params{ + "b": "bv1", + "c": "cv2", + }, + }) + } + + } + + }) + c.Run("IsSet", func(c *qt.C) { cfg := New() diff --git a/hugolib/config.go b/hugolib/config.go index cad8451990d..90ac7eb0172 100644 --- a/hugolib/config.go +++ b/hugolib/config.go @@ -64,10 +64,6 @@ func LoadConfig(d ConfigSourceDescriptor, doWithConfig ...func(cfg config.Provid l := configLoader{ConfigSourceDescriptor: d, cfg: config.New()} - if err := l.applyConfigDefaults(); err != nil { - return l.cfg, configFiles, err - } - for _, name := range d.configFilenames() { var filename string filename, err := l.loadConfig(name) @@ -78,6 +74,10 @@ func LoadConfig(d ConfigSourceDescriptor, doWithConfig ...func(cfg config.Provid } } + if err := l.applyConfigDefaults(); err != nil { + return l.cfg, configFiles, err + } + if d.AbsConfigDir != "" { dcfg, dirnames, err := config.LoadConfigFromDir(l.Fs, d.AbsConfigDir, l.Environment) if err == nil { @@ -298,7 +298,7 @@ func (l configLoader) applyConfigDefaults() error { "enableInlineShortcodes": false, } - l.cfg.Merge("", defaultSettings) + l.cfg.SetDefaults(defaultSettings) return nil } diff --git a/hugolib/content_render_hooks_test.go b/hugolib/content_render_hooks_test.go index 97916830529..1d7a4f8e310 100644 --- a/hugolib/content_render_hooks_test.go +++ b/hugolib/content_render_hooks_test.go @@ -56,6 +56,7 @@ title: P1 b.AssertFileContent("public/p1/index.html", `Link First Link|PARTIAL1_EDITED PARTIAL2_EDITEDEND`) } + func TestRenderHooks(t *testing.T) { config := ` baseURL="https://example.org"