repo: fix manu bugs in go-git config

Michael Muré created

Change summary

repository/config_testing.go | 97 +++++++++++++++++++++++++++++--------
repository/gogit_config.go   | 90 ++++++++++++++++++++++------------
2 files changed, 133 insertions(+), 54 deletions(-)

Detailed changes

repository/config_testing.go 🔗

@@ -2,62 +2,115 @@ package repository
 
 import (
 	"testing"
+	"time"
 
-	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func testConfig(t *testing.T, config Config) {
+	// string
 	err := config.StoreString("section.key", "value")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	val, err := config.ReadString("section.key")
-	assert.NoError(t, err)
-	assert.Equal(t, "value", val)
+	require.NoError(t, err)
+	require.Equal(t, "value", val)
 
-	err = config.StoreString("section.true", "true")
-	assert.NoError(t, err)
+	// bool
+	err = config.StoreBool("section.true", true)
+	require.NoError(t, err)
 
 	val2, err := config.ReadBool("section.true")
-	assert.NoError(t, err)
-	assert.Equal(t, true, val2)
+	require.NoError(t, err)
+	require.Equal(t, true, val2)
 
+	// timestamp
+	err = config.StoreTimestamp("section.time", time.Unix(1234, 0))
+	require.NoError(t, err)
+
+	val3, err := config.ReadTimestamp("section.time")
+	require.NoError(t, err)
+	require.Equal(t, time.Unix(1234, 0), val3)
+
+	// ReadAll
 	configs, err := config.ReadAll("section")
-	assert.NoError(t, err)
-	assert.Equal(t, map[string]string{
+	require.NoError(t, err)
+	require.Equal(t, map[string]string{
 		"section.key":  "value",
 		"section.true": "true",
+		"section.time": "1234",
 	}, configs)
 
+	// RemoveAll
 	err = config.RemoveAll("section.true")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	configs, err = config.ReadAll("section")
-	assert.NoError(t, err)
-	assert.Equal(t, map[string]string{
-		"section.key": "value",
+	require.NoError(t, err)
+	require.Equal(t, map[string]string{
+		"section.key":  "value",
+		"section.time": "1234",
 	}, configs)
 
 	_, err = config.ReadBool("section.true")
-	assert.Equal(t, ErrNoConfigEntry, err)
+	require.Equal(t, ErrNoConfigEntry, err)
 
 	err = config.RemoveAll("section.nonexistingkey")
-	assert.Error(t, err)
+	require.Error(t, err)
 
 	err = config.RemoveAll("section.key")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	_, err = config.ReadString("section.key")
-	assert.Equal(t, ErrNoConfigEntry, err)
+	require.Equal(t, ErrNoConfigEntry, err)
 
 	err = config.RemoveAll("nonexistingsection")
-	assert.Error(t, err)
+	require.Error(t, err)
+
+	err = config.RemoveAll("section.time")
+	require.NoError(t, err)
 
 	err = config.RemoveAll("section")
-	assert.Error(t, err)
+	require.Error(t, err)
 
 	_, err = config.ReadString("section.key")
-	assert.Error(t, err)
+	require.Error(t, err)
 
 	err = config.RemoveAll("section.key")
-	assert.Error(t, err)
+	require.Error(t, err)
+
+	// section + subsections
+	require.NoError(t, config.StoreString("section.opt1", "foo"))
+	require.NoError(t, config.StoreString("section.opt2", "foo2"))
+	require.NoError(t, config.StoreString("section.subsection.opt1", "foo3"))
+	require.NoError(t, config.StoreString("section.subsection.opt2", "foo4"))
+	require.NoError(t, config.StoreString("section.subsection.subsection.opt1", "foo5"))
+	require.NoError(t, config.StoreString("section.subsection.subsection.opt2", "foo6"))
+
+	all, err := config.ReadAll("section")
+	require.NoError(t, err)
+	require.Equal(t, map[string]string{
+		"section.opt1":                       "foo",
+		"section.opt2":                       "foo2",
+		"section.subsection.opt1":            "foo3",
+		"section.subsection.opt2":            "foo4",
+		"section.subsection.subsection.opt1": "foo5",
+		"section.subsection.subsection.opt2": "foo6",
+	}, all)
+
+	all, err = config.ReadAll("section.subsection")
+	require.NoError(t, err)
+	require.Equal(t, map[string]string{
+		"section.subsection.opt1":            "foo3",
+		"section.subsection.opt2":            "foo4",
+		"section.subsection.subsection.opt1": "foo5",
+		"section.subsection.subsection.opt2": "foo6",
+	}, all)
+
+	all, err = config.ReadAll("section.subsection.subsection")
+	require.NoError(t, err)
+	require.Equal(t, map[string]string{
+		"section.subsection.subsection.opt1": "foo5",
+		"section.subsection.subsection.opt2": "foo6",
+	}, all)
 }

repository/gogit_config.go 🔗

@@ -7,7 +7,6 @@ import (
 	"time"
 
 	gogit "github.com/go-git/go-git/v5"
-	"github.com/go-git/go-git/v5/plumbing/format/config"
 )
 
 var _ Config = &goGitConfig{}
@@ -35,7 +34,7 @@ func (ggc *goGitConfig) StoreString(key, value string) error {
 		cfg.Raw.Section(split[0]).SetOption(split[1], value)
 	default:
 		section := split[0]
-		subsection := strings.Join(split[1:len(split)-2], ".")
+		subsection := strings.Join(split[1:len(split)-1], ".")
 		option := split[len(split)-1]
 		cfg.Raw.Section(section).Subsection(subsection).SetOption(option, value)
 	}
@@ -58,33 +57,52 @@ func (ggc *goGitConfig) ReadAll(keyPrefix string) (map[string]string, error) {
 	}
 
 	split := strings.Split(keyPrefix, ".")
-
-	var opts config.Options
+	result := make(map[string]string)
 
 	switch {
-	case len(split) < 1:
-		return nil, fmt.Errorf("invalid key prefix")
+	case keyPrefix == "":
+		for _, section := range cfg.Raw.Sections {
+			for _, option := range section.Options {
+				result[fmt.Sprintf("%s.%s", section.Name, option.Key)] = option.Value
+			}
+			for _, subsection := range section.Subsections {
+				for _, option := range subsection.Options {
+					result[fmt.Sprintf("%s.%s.%s", section.Name, subsection.Name, option.Key)] = option.Value
+				}
+			}
+		}
 	case len(split) == 1:
-		opts = cfg.Raw.Section(split[0]).Options
+		if !cfg.Raw.HasSection(split[0]) {
+			return nil, fmt.Errorf("invalid section")
+		}
+		section := cfg.Raw.Section(split[0])
+		for _, option := range section.Options {
+			result[fmt.Sprintf("%s.%s", section.Name, option.Key)] = option.Value
+		}
+		for _, subsection := range section.Subsections {
+			for _, option := range subsection.Options {
+				result[fmt.Sprintf("%s.%s.%s", section.Name, subsection.Name, option.Key)] = option.Value
+			}
+		}
 	default:
-		section := split[0]
-		subsection := strings.Join(split[1:len(split)-1], ".")
-		opts = cfg.Raw.Section(section).Subsection(subsection).Options
+		if !cfg.Raw.HasSection(split[0]) {
+			return nil, fmt.Errorf("invalid section")
+		}
+		section := cfg.Raw.Section(split[0])
+		rest := strings.Join(split[1:], ".")
+		for _, subsection := range section.Subsections {
+			if strings.HasPrefix(subsection.Name, rest) {
+				for _, option := range subsection.Options {
+					result[fmt.Sprintf("%s.%s.%s", section.Name, subsection.Name, option.Key)] = option.Value
+				}
+			}
+		}
 	}
 
-	if len(opts) == 0 {
+	if len(result) == 0 {
 		return nil, fmt.Errorf("invalid section")
 	}
 
-	if keyPrefix[len(keyPrefix)-1:] != "." {
-		keyPrefix += "."
-	}
-
-	result := make(map[string]string, len(opts))
-	for _, opt := range opts {
-		result[keyPrefix+opt.Key] = opt.Value
-	}
-
 	return result, nil
 }
 
@@ -159,26 +177,34 @@ func (ggc *goGitConfig) RemoveAll(keyPrefix string) error {
 	split := strings.Split(keyPrefix, ".")
 
 	switch {
-	case len(split) < 1:
-		return fmt.Errorf("invalid key prefix")
+	case keyPrefix == "":
+		cfg.Raw.Sections = nil
+		// warning: this does not actually remove everything as go-git config hold
+		// some entries in multiple places (cfg.User ...)
 	case len(split) == 1:
-		if len(cfg.Raw.Section(split[0]).Options) > 0 {
+		if cfg.Raw.HasSection(split[0]) {
 			cfg.Raw.RemoveSection(split[0])
 		} else {
 			return fmt.Errorf("invalid key prefix")
 		}
 	default:
-		section := split[0]
+		if !cfg.Raw.HasSection(split[0]) {
+			return fmt.Errorf("invalid key prefix")
+		}
+		section := cfg.Raw.Section(split[0])
 		rest := strings.Join(split[1:], ".")
 
-		if cfg.Raw.Section(section).HasSubsection(rest) {
-			cfg.Raw.RemoveSubsection(section, rest)
-		} else {
-			if cfg.Raw.Section(section).HasOption(rest) {
-				cfg.Raw.Section(section).RemoveOption(rest)
-			} else {
-				return fmt.Errorf("invalid key prefix")
-			}
+		ok := false
+		if section.HasSubsection(rest) {
+			section.RemoveSubsection(rest)
+			ok = true
+		}
+		if section.HasOption(rest) {
+			section.RemoveOption(rest)
+			ok = true
+		}
+		if !ok {
+			return fmt.Errorf("invalid key prefix")
 		}
 	}