From fb16a6b8d52881f46d9e7ca3c01cf3abc238436f Mon Sep 17 00:00:00 2001 From: Vlad Didenko Date: Wed, 27 May 2015 15:30:04 -0500 Subject: [PATCH 1/6] spf13/viper#73 Tests to document current behavior and expose the bug --- viper_test.go | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/viper_test.go b/viper_test.go index 7ad0245..d513e25 100644 --- a/viper_test.go +++ b/viper_test.go @@ -8,7 +8,9 @@ package viper import ( "bytes" "fmt" + "io/ioutil" "os" + "path" "sort" "strings" "testing" @@ -124,6 +126,47 @@ func initTOML() { marshalReader(r, v.config) } +// make directories for testing +func initDirs(t *testing.T) (string, string, func()) { + + var ( + testDirs = []string{`a a`, `b`, `c\c`, `D:`} + config = `improbable` + ) + + root, err := ioutil.TempDir("", "") + + cleanup := true + defer func() { + if cleanup { + os.Chdir("..") + os.RemoveAll(root) + } + }() + + assert.Nil(t, err) + + err = os.Chdir(root) + assert.Nil(t, err) + + err = ioutil.WriteFile(path.Join(root, config+".toml"), []byte("key = \"root\"\n"), 0640) + assert.Nil(t, err) + + for _, dir := range testDirs { + err = os.Mkdir(dir, 0750) + assert.Nil(t, err) + + err = ioutil.WriteFile(path.Join(dir, config+".toml"), []byte("key = \"value is "+dir+"\"\n"), 0640) + assert.Nil(t, err) + } + + cleanup = false + return root, config, func() { + os.Chdir("..") + os.RemoveAll(root) + } +} + //stubs for PFlag Values type stringValue string @@ -551,3 +594,59 @@ func TestReadBufConfig(t *testing.T) { assert.Equal(t, map[interface{}]interface{}{"jacket": "leather", "trousers": "denim"}, v.Get("clothing")) assert.Equal(t, 35, v.Get("age")) } + +func TestCWDSearch(t *testing.T) { + + _, config, cleanup := initDirs(t) + defer cleanup() + + v := New() + v.SetConfigName(config) + v.SetDefault(`key`, `default`) + + err := v.ReadInConfig() + assert.Nil(t, err) + + assert.Equal(t, `root`, v.GetString(`key`)) +} + +func TestDirsSearch(t *testing.T) { + + root, config, cleanup := initDirs(t) + defer cleanup() + + v := New() + v.SetConfigName(config) + v.SetDefault(`key`, `default`) + + entries, err := ioutil.ReadDir(root) + for _, e := range entries { + if e.IsDir() { + v.AddConfigPath(e.Name()) + } + } + + err = v.ReadInConfig() + assert.Nil(t, err) + + assert.Equal(t, `value is `+path.Base(v.configPaths[0]), v.GetString(`key`)) +} + +func TestWrongDirsSearchNotFoundOK(t *testing.T) { + + _, config, cleanup := initDirs(t) + defer cleanup() + + v := New() + v.SetConfigName(config) + v.SetDefault(`key`, `default`) + + v.AddConfigPath(`whattayoutalkingbout`) + v.AddConfigPath(`thispathaintthere`) + + err := v.ReadInConfig() + assert.Nil(t, err) + + // Should not see the value "root" which comes from config in CWD + assert.Equal(t, `default`, v.GetString(`key`)) +} From 033e966e68dffc844835105e17b8dbbb13df65a3 Mon Sep 17 00:00:00 2001 From: Vlad Didenko Date: Wed, 27 May 2015 16:03:21 -0500 Subject: [PATCH 2/6] spf13/viper#73 More specific test to document current behavior --- viper_test.go | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/viper_test.go b/viper_test.go index d513e25..554ff7a 100644 --- a/viper_test.go +++ b/viper_test.go @@ -11,6 +11,7 @@ import ( "io/ioutil" "os" "path" + "reflect" "sort" "strings" "testing" @@ -632,7 +633,7 @@ func TestDirsSearch(t *testing.T) { assert.Equal(t, `value is `+path.Base(v.configPaths[0]), v.GetString(`key`)) } -func TestWrongDirsSearchNotFoundOK(t *testing.T) { +func TestWrongDirsSearchNotFoundHasCWDConfig(t *testing.T) { _, config, cleanup := initDirs(t) defer cleanup() @@ -650,3 +651,25 @@ func TestWrongDirsSearchNotFoundOK(t *testing.T) { // Should not see the value "root" which comes from config in CWD assert.Equal(t, `default`, v.GetString(`key`)) } + +func TestWrongDirsSearchNotFoundNoCWDConfig(t *testing.T) { + + _, config, cleanup := initDirs(t) + defer cleanup() + + os.Remove(config + ".toml") + + v := New() + v.SetConfigName(config) + v.SetDefault(`key`, `default`) + + v.AddConfigPath(`whattayoutalkingbout`) + v.AddConfigPath(`thispathaintthere`) + + err := v.ReadInConfig() + assert.Equal(t, reflect.TypeOf(UnsupportedConfigError("")), reflect.TypeOf(err)) + + // Even though config did not load and the error might have + // been ignored by the client, the default still loads + assert.Equal(t, `default`, v.GetString(`key`)) +} From c53a6dbc69a5ffb884e8129ab409094248581251 Mon Sep 17 00:00:00 2001 From: Vlad Didenko Date: Wed, 27 May 2015 16:29:08 -0500 Subject: [PATCH 3/6] spf13/viper#73 Fix when no config found in specified directories No config in specified directories now behave the same regardless if the config file present in CWD or not --- viper.go | 37 +++++++++++++++++++++++++++++++++---- viper_test.go | 3 ++- 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/viper.go b/viper.go index 11f3a77..ff80206 100644 --- a/viper.go +++ b/viper.go @@ -72,6 +72,16 @@ func (rce RemoteConfigError) Error() string { return fmt.Sprintf("Remote Configurations Error: %s", string(rce)) } +// Denotes failing to find configuration file. +type ConfigFileNotFoundError struct { + name, locations string +} + +// Returns the formatted configuration error. +func (fnfe ConfigFileNotFoundError) Error() string { + return fmt.Sprintf("Config File %q Not Found in %q", fnfe.name, fnfe.locations) +} + // Viper is a prioritized configuration registry. It // maintains a set of configuration sources, fetches // values to populate those, and provides them according @@ -955,9 +965,22 @@ func (v *Viper) searchInPath(in string) (filename string) { return "" } -// search all configPaths for any config file. -// Returns the first path that exists (and is a config file) +// Choose where to look for a config file: either +// in provided directories or in the working directory func (v *Viper) findConfigFile() (string, error) { + + if len(v.configPaths) > 0 { + return v.findConfigInPaths() + } else { + return v.findConfigInCWD() + } + +} + +// Search all configPaths for any config file. +// Returns the first path that exists (and has a config file) +func (v *Viper) findConfigInPaths() (string, error) { + jww.INFO.Println("Searching for config in ", v.configPaths) for _, cp := range v.configPaths { @@ -966,14 +989,20 @@ func (v *Viper) findConfigFile() (string, error) { return file, nil } } + return "", ConfigFileNotFoundError{v.configName, fmt.Sprintf("%s", v.configPaths)} +} + +// Search the current working directory for any config file. +func (v *Viper) findConfigInCWD() (string, error) { - // try the current working directory wd, _ := os.Getwd() + jww.INFO.Println("Searching for config in ", wd) + file := v.searchInPath(wd) if file != "" { return file, nil } - return "", fmt.Errorf("config file not found in: %s", v.configPaths) + return "", ConfigFileNotFoundError{v.configName, wd} } // Prints all configuration registries for debugging diff --git a/viper_test.go b/viper_test.go index 554ff7a..2379fcd 100644 --- a/viper_test.go +++ b/viper_test.go @@ -646,7 +646,7 @@ func TestWrongDirsSearchNotFoundHasCWDConfig(t *testing.T) { v.AddConfigPath(`thispathaintthere`) err := v.ReadInConfig() - assert.Nil(t, err) + assert.Equal(t, reflect.TypeOf(UnsupportedConfigError("")), reflect.TypeOf(err)) // Should not see the value "root" which comes from config in CWD assert.Equal(t, `default`, v.GetString(`key`)) @@ -657,6 +657,7 @@ func TestWrongDirsSearchNotFoundNoCWDConfig(t *testing.T) { _, config, cleanup := initDirs(t) defer cleanup() + // Remove the config file in CWD os.Remove(config + ".toml") v := New() From 5fa6a974f222e7a7229b5d414b95cfa584fde5ec Mon Sep 17 00:00:00 2001 From: Vlad Didenko Date: Wed, 27 May 2015 16:35:34 -0500 Subject: [PATCH 4/6] spf13/viper#73 Test to document no config in CWD when expected --- viper_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/viper_test.go b/viper_test.go index 2379fcd..60d2f22 100644 --- a/viper_test.go +++ b/viper_test.go @@ -611,6 +611,24 @@ func TestCWDSearch(t *testing.T) { assert.Equal(t, `root`, v.GetString(`key`)) } +func TestCWDSearchNoConfig(t *testing.T) { + + _, config, cleanup := initDirs(t) + defer cleanup() + + // Remove the config file in CWD + os.Remove(config + ".toml") + + v := New() + v.SetConfigName(config) + v.SetDefault(`key`, `default`) + + err := v.ReadInConfig() + assert.Equal(t, reflect.TypeOf(UnsupportedConfigError("")), reflect.TypeOf(err)) + + assert.Equal(t, `default`, v.GetString(`key`)) +} + func TestDirsSearch(t *testing.T) { root, config, cleanup := initDirs(t) From 1241d2e04b9d3a9d345bf62dd3f60ca40c7dfa0a Mon Sep 17 00:00:00 2001 From: Vlad Didenko Date: Tue, 9 Jun 2015 02:02:36 -0500 Subject: [PATCH 5/6] Fixes spf/viper#83 --- viper.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/viper.go b/viper.go index f46787c..f23de29 100644 --- a/viper.go +++ b/viper.go @@ -529,11 +529,11 @@ func (v *Viper) BindPFlag(key string, flag *pflag.Flag) (err error) { switch flag.Value.Type() { case "int", "int8", "int16", "int32", "int64": - SetDefault(key, cast.ToInt(flag.Value.String())) + v.SetDefault(key, cast.ToInt(flag.Value.String())) case "bool": - SetDefault(key, cast.ToBool(flag.Value.String())) + v.SetDefault(key, cast.ToBool(flag.Value.String())) default: - SetDefault(key, flag.Value.String()) + v.SetDefault(key, flag.Value.String()) } return nil } From ad82755e41a287b1bd99eb88665c904cc52e188d Mon Sep 17 00:00:00 2001 From: Vlad Didenko Date: Tue, 9 Jun 2015 02:46:08 -0500 Subject: [PATCH 6/6] Fixes spf13/viper#68 --- util.go | 21 ++++++++++++++++----- viper.go | 22 +++++++++++----------- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/util.go b/util.go index 7904b1a..454aca2 100644 --- a/util.go +++ b/util.go @@ -28,6 +28,16 @@ import ( "gopkg.in/yaml.v2" ) +// Denotes failing to parse configuration file. +type ConfigFileParseError struct { + err error +} + +// Returns the formatted configuration error. +func (pe ConfigFileParseError) Error() string { + return fmt.Sprintf("While parsing config: %s", pe.err.Error()) +} + func insensitiviseMap(m map[string]interface{}) { for key, val := range m { lower := strings.ToLower(key) @@ -119,31 +129,31 @@ func findCWD() (string, error) { return path, nil } -func marshallConfigReader(in io.Reader, c map[string]interface{}, configType string) { +func marshallConfigReader(in io.Reader, c map[string]interface{}, configType string) error { buf := new(bytes.Buffer) buf.ReadFrom(in) switch strings.ToLower(configType) { case "yaml", "yml": if err := yaml.Unmarshal(buf.Bytes(), &c); err != nil { - jww.ERROR.Fatalf("Error parsing config: %s", err) + return ConfigFileParseError{err} } case "json": if err := json.Unmarshal(buf.Bytes(), &c); err != nil { - jww.ERROR.Fatalf("Error parsing config: %s", err) + return ConfigFileParseError{err} } case "toml": if _, err := toml.Decode(buf.String(), &c); err != nil { - jww.ERROR.Fatalf("Error parsing config: %s", err) + return ConfigFileParseError{err} } case "properties", "props", "prop": var p *properties.Properties var err error if p, err = properties.Load(buf.Bytes(), properties.UTF8); err != nil { - jww.ERROR.Fatalf("Error parsing config: %s", err) + return ConfigFileParseError{err} } for _, key := range p.Keys() { value, _ := p.Get(key) @@ -152,6 +162,7 @@ func marshallConfigReader(in io.Reader, c map[string]interface{}, configType str } insensitiviseMap(c) + return nil } func safeMul(a, b uint) uint { diff --git a/viper.go b/viper.go index f23de29..b46bc33 100644 --- a/viper.go +++ b/viper.go @@ -745,22 +745,19 @@ func (v *Viper) ReadInConfig() error { v.config = make(map[string]interface{}) - v.marshalReader(bytes.NewReader(file), v.config) - return nil + return v.marshalReader(bytes.NewReader(file), v.config) } func ReadConfig(in io.Reader) error { return v.ReadConfig(in) } func (v *Viper) ReadConfig(in io.Reader) error { v.config = make(map[string]interface{}) - v.marshalReader(in, v.config) - return nil + return v.marshalReader(in, v.config) } // func ReadBufConfig(buf *bytes.Buffer) error { return v.ReadBufConfig(buf) } // func (v *Viper) ReadBufConfig(buf *bytes.Buffer) error { // v.config = make(map[string]interface{}) -// v.marshalReader(buf, v.config) -// return nil +// return v.marshalReader(buf, v.config) // } // Attempts to get configuration from a remote source @@ -785,9 +782,12 @@ func (v *Viper) WatchRemoteConfig() error { // Marshall a Reader into a map // Should probably be an unexported function -func marshalReader(in io.Reader, c map[string]interface{}) { v.marshalReader(in, c) } -func (v *Viper) marshalReader(in io.Reader, c map[string]interface{}) { - marshallConfigReader(in, c, v.getConfigType()) +func marshalReader(in io.Reader, c map[string]interface{}) error { + return v.marshalReader(in, c) +} + +func (v *Viper) marshalReader(in io.Reader, c map[string]interface{}) error { + return marshallConfigReader(in, c, v.getConfigType()) } func (v *Viper) insensitiviseMaps() { @@ -820,7 +820,7 @@ func (v *Viper) getRemoteConfig(provider *defaultRemoteProvider) (map[string]int if err != nil { return nil, err } - v.marshalReader(reader, v.kvstore) + err = v.marshalReader(reader, v.kvstore) return v.kvstore, err } @@ -842,7 +842,7 @@ func (v *Viper) watchRemoteConfig(provider *defaultRemoteProvider) (map[string]i if err != nil { return nil, err } - v.marshalReader(reader, v.kvstore) + err = v.marshalReader(reader, v.kvstore) return v.kvstore, err }