repo: more config related bug fixes

Michael Muré created 5 years ago

Change summary

identity/identity_user.go  |  27 ++++-----
repository/config.go       |  20 +++++++
repository/gogit.go        |   4 
repository/gogit_config.go | 113 ++++++++++++++++++++++++---------------
4 files changed, 102 insertions(+), 62 deletions(-)

Detailed changes

identity/identity_user.go

@@ -35,24 +35,19 @@ func GetUserIdentity(repo repository.Repo) (*Identity, error) {
 }
 
 func GetUserIdentityId(repo repository.Repo) (entity.Id, error) {
-	configs, err := repo.LocalConfig().ReadAll(identityConfigKey)
-	if err != nil {
-		return entity.UnsetId, err
-	}
-
-	if len(configs) == 0 {
+	val, err := repo.LocalConfig().ReadString(identityConfigKey)
+	if err == repository.ErrNoConfigEntry {
 		return entity.UnsetId, ErrNoIdentitySet
 	}
-
-	if len(configs) > 1 {
+	if err == repository.ErrMultipleConfigEntry {
 		return entity.UnsetId, ErrMultipleIdentitiesSet
 	}
-
-	var id entity.Id
-	for _, val := range configs {
-		id = entity.Id(val)
+	if err != nil {
+		return entity.UnsetId, err
 	}
 
+	var id = entity.Id(val)
+
 	if err := id.Validate(); err != nil {
 		return entity.UnsetId, err
 	}
@@ -62,10 +57,12 @@ func GetUserIdentityId(repo repository.Repo) (entity.Id, error) {
 
 // IsUserIdentitySet say if the user has set his identity
 func IsUserIdentitySet(repo repository.Repo) (bool, error) {
-	configs, err := repo.LocalConfig().ReadAll(identityConfigKey)
+	_, err := repo.LocalConfig().ReadString(identityConfigKey)
+	if err == repository.ErrNoConfigEntry {
+		return false, nil
+	}
 	if err != nil {
 		return false, err
 	}
-
-	return len(configs) == 1, nil
+	return true, nil
 }

repository/config.go

@@ -123,3 +123,23 @@ func (m *mergedConfig) ReadTimestamp(key string) (time.Time, error) {
 	}
 	return m.global.ReadTimestamp(key)
 }
+
+var _ ConfigWrite = &configPanicWriter{}
+
+type configPanicWriter struct{}
+
+func (c configPanicWriter) StoreString(key, value string) error {
+	panic("not implemented")
+}
+
+func (c configPanicWriter) StoreTimestamp(key string, value time.Time) error {
+	panic("not implemented")
+}
+
+func (c configPanicWriter) StoreBool(key string, value bool) error {
+	panic("not implemented")
+}
+
+func (c configPanicWriter) RemoveAll(keyPrefix string) error {
+	panic("not implemented")
+}

repository/gogit.go

@@ -160,7 +160,7 @@ func InitBareGoGitRepo(path string) (*GoGitRepo, error) {
 
 // LocalConfig give access to the repository scoped configuration
 func (repo *GoGitRepo) LocalConfig() Config {
-	return newGoGitConfig(repo.r)
+	return newGoGitLocalConfig(repo.r)
 }
 
 // GlobalConfig give access to the global scoped configuration
@@ -168,7 +168,7 @@ func (repo *GoGitRepo) GlobalConfig() Config {
 	// TODO: replace that with go-git native implementation once it's supported
 	// see: https://github.com/go-git/go-git
 	// see: https://github.com/src-d/go-git/issues/760
-	return newGitConfig(gitCli{repo.path}, true)
+	return newGoGitGlobalConfig(repo.r)
 }
 
 // AnyConfig give access to a merged local/global configuration

repository/gogit_config.go

@@ -7,51 +7,40 @@ import (
 	"time"
 
 	gogit "github.com/go-git/go-git/v5"
+	"github.com/go-git/go-git/v5/config"
 )
 
 var _ Config = &goGitConfig{}
 
 type goGitConfig struct {
-	repo *gogit.Repository
+	ConfigRead
+	ConfigWrite
 }
 
-func newGoGitConfig(repo *gogit.Repository) *goGitConfig {
-	return &goGitConfig{repo: repo}
+func newGoGitLocalConfig(repo *gogit.Repository) *goGitConfig {
+	return &goGitConfig{
+		ConfigRead:  &goGitConfigReader{getConfig: repo.Config},
+		ConfigWrite: &goGitConfigWriter{repo: repo},
+	}
 }
 
-func (ggc *goGitConfig) StoreString(key, value string) error {
-	cfg, err := ggc.repo.Config()
-	if err != nil {
-		return err
+func newGoGitGlobalConfig(repo *gogit.Repository) *goGitConfig {
+	return &goGitConfig{
+		ConfigRead: &goGitConfigReader{getConfig: func() (*config.Config, error) {
+			return config.LoadConfig(config.GlobalScope)
+		}},
+		ConfigWrite: &configPanicWriter{},
 	}
-
-	split := strings.Split(key, ".")
-
-	switch {
-	case len(split) <= 1:
-		return fmt.Errorf("invalid key")
-	case len(split) == 2:
-		cfg.Raw.Section(split[0]).SetOption(split[1], value)
-	default:
-		section := split[0]
-		subsection := strings.Join(split[1:len(split)-1], ".")
-		option := split[len(split)-1]
-		cfg.Raw.Section(section).Subsection(subsection).SetOption(option, value)
-	}
-
-	return ggc.repo.SetConfig(cfg)
 }
 
-func (ggc *goGitConfig) StoreTimestamp(key string, value time.Time) error {
-	return ggc.StoreString(key, strconv.Itoa(int(value.Unix())))
-}
+var _ ConfigRead = &goGitConfigReader{}
 
-func (ggc *goGitConfig) StoreBool(key string, value bool) error {
-	return ggc.StoreString(key, strconv.FormatBool(value))
+type goGitConfigReader struct {
+	getConfig func() (*config.Config, error)
 }
 
-func (ggc *goGitConfig) ReadAll(keyPrefix string) (map[string]string, error) {
-	cfg, err := ggc.repo.Config()
+func (cr *goGitConfigReader) ReadAll(keyPrefix string) (map[string]string, error) {
+	cfg, err := cr.getConfig()
 	if err != nil {
 		return nil, err
 	}
@@ -73,7 +62,7 @@ func (ggc *goGitConfig) ReadAll(keyPrefix string) (map[string]string, error) {
 		}
 	case len(split) == 1:
 		if !cfg.Raw.HasSection(split[0]) {
-			return nil, fmt.Errorf("invalid section")
+			return nil, nil
 		}
 		section := cfg.Raw.Section(split[0])
 		for _, option := range section.Options {
@@ -86,7 +75,7 @@ func (ggc *goGitConfig) ReadAll(keyPrefix string) (map[string]string, error) {
 		}
 	default:
 		if !cfg.Raw.HasSection(split[0]) {
-			return nil, fmt.Errorf("invalid section")
+			return nil, nil
 		}
 		section := cfg.Raw.Section(split[0])
 		rest := strings.Join(split[1:], ".")
@@ -99,15 +88,11 @@ func (ggc *goGitConfig) ReadAll(keyPrefix string) (map[string]string, error) {
 		}
 	}
 
-	if len(result) == 0 {
-		return nil, fmt.Errorf("invalid section")
-	}
-
 	return result, nil
 }
 
-func (ggc *goGitConfig) ReadBool(key string) (bool, error) {
-	val, err := ggc.ReadString(key)
+func (cr *goGitConfigReader) ReadBool(key string) (bool, error) {
+	val, err := cr.ReadString(key)
 	if err != nil {
 		return false, err
 	}
@@ -115,8 +100,8 @@ func (ggc *goGitConfig) ReadBool(key string) (bool, error) {
 	return strconv.ParseBool(val)
 }
 
-func (ggc *goGitConfig) ReadString(key string) (string, error) {
-	cfg, err := ggc.repo.Config()
+func (cr *goGitConfigReader) ReadString(key string) (string, error) {
+	cfg, err := cr.getConfig()
 	if err != nil {
 		return "", err
 	}
@@ -160,16 +145,54 @@ func (ggc *goGitConfig) ReadString(key string) (string, error) {
 	}
 }
 
-func (ggc *goGitConfig) ReadTimestamp(key string) (time.Time, error) {
-	value, err := ggc.ReadString(key)
+func (cr *goGitConfigReader) ReadTimestamp(key string) (time.Time, error) {
+	value, err := cr.ReadString(key)
 	if err != nil {
 		return time.Time{}, err
 	}
 	return ParseTimestamp(value)
 }
 
-func (ggc *goGitConfig) RemoveAll(keyPrefix string) error {
-	cfg, err := ggc.repo.Config()
+var _ ConfigWrite = &goGitConfigWriter{}
+
+// Only works for the local config as go-git only support that
+type goGitConfigWriter struct {
+	repo *gogit.Repository
+}
+
+func (cw *goGitConfigWriter) StoreString(key, value string) error {
+	cfg, err := cw.repo.Config()
+	if err != nil {
+		return err
+	}
+
+	split := strings.Split(key, ".")
+
+	switch {
+	case len(split) <= 1:
+		return fmt.Errorf("invalid key")
+	case len(split) == 2:
+		cfg.Raw.Section(split[0]).SetOption(split[1], value)
+	default:
+		section := split[0]
+		subsection := strings.Join(split[1:len(split)-1], ".")
+		option := split[len(split)-1]
+		cfg.Raw.Section(section).Subsection(subsection).SetOption(option, value)
+	}
+
+	return cw.repo.SetConfig(cfg)
+}
+
+func (cw *goGitConfigWriter) StoreTimestamp(key string, value time.Time) error {
+	return cw.StoreString(key, strconv.Itoa(int(value.Unix())))
+}
+
+func (cw *goGitConfigWriter) StoreBool(key string, value bool) error {
+	return cw.StoreString(key, strconv.FormatBool(value))
+}
+
+func (cw *goGitConfigWriter) RemoveAll(keyPrefix string) error {
+	cfg, err := cw.repo.Config()
 	if err != nil {
 		return err
 	}
@@ -208,5 +231,5 @@ func (ggc *goGitConfig) RemoveAll(keyPrefix string) error {
 		}
 	}
 
-	return ggc.repo.SetConfig(cfg)
+	return cw.repo.SetConfig(cfg)
 }