From fc501f85e2714b0517c74c9e3c6651de4f807483 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20B=C3=A9richon?= Date: Sun, 2 Feb 2020 12:52:58 +0100 Subject: [PATCH] Resolves #742 --- README.md | 10 ++-- go.mod | 1 + go.sum | 2 + test.yml | 4 ++ viper.go | 140 +++++++++++++++++++++++++++----------------------- viper_test.go | 128 +++++++++++++++++++++++++++++++++++++++++---- 6 files changed, 208 insertions(+), 77 deletions(-) create mode 100644 test.yml diff --git a/README.md b/README.md index a574677..7fc8d90 100644 --- a/README.md +++ b/README.md @@ -164,11 +164,15 @@ Optionally you can provide a function for Viper to run each time a change occurs ```go viper.WatchConfig() +defer CancelWatchConfig() viper.OnConfigChange(func(e fsnotify.Event) { fmt.Println("Config file changed:", e.Name) }) ``` +If you wish to stop watching the configPaths, simply call viper.CancelWatchConfig(). +Note: This might be necessary if your tests involve trying out various config files. + ### Reading Config from io.Reader Viper predefines many configuration sources such as files, environment @@ -435,7 +439,7 @@ err := viper.ReadRemoteConfig() ``` #### Consul -You need to set a key to Consul key/value storage with JSON value containing your desired config. +You need to set a key to Consul key/value storage with JSON value containing your desired config. For example, create a Consul key/value store key `MY_CONSUL_KEY` with value: ```json @@ -728,14 +732,14 @@ Viper uses [github.com/mitchellh/mapstructure](https://github.com/mitchellh/maps ### Marshalling to string -You may need to marshal all the settings held in viper into a string rather than write them to a file. +You may need to marshal all the settings held in viper into a string rather than write them to a file. You can use your favorite format's marshaller with the config returned by `AllSettings()`. ```go import ( yaml "gopkg.in/yaml.v2" // ... -) +) func yamlStringSettings() string { c := viper.AllSettings() diff --git a/go.mod b/go.mod index 0e358cb..5ef695b 100644 --- a/go.mod +++ b/go.mod @@ -43,6 +43,7 @@ require ( golang.org/x/net v0.0.0-20190522155817-f3200d17e092 // indirect golang.org/x/time v0.0.0-20190308202827-9d24e82272b4 // indirect google.golang.org/grpc v1.21.0 // indirect + gopkg.in/fsnotify.v1 v1.4.7 gopkg.in/ini.v1 v1.51.0 gopkg.in/yaml.v2 v2.2.4 ) diff --git a/go.sum b/go.sum index d75aee2..225d6d2 100644 --- a/go.sum +++ b/go.sum @@ -182,6 +182,8 @@ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/fsnotify.v1 v1.4.7 h1:xOHLXZwVvI9hhs+cLKq5+I5onOuwQLhQwiu63xxlHs4= +gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys= gopkg.in/ini.v1 v1.51.0 h1:AQvPpx3LzTDM0AjnIRlVFwFFGC+npRopjZxLJj6gdno= gopkg.in/ini.v1 v1.51.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= gopkg.in/resty.v1 v1.12.0/go.mod h1:mDo4pnntr5jdWRML875a/NmxYqAlA73dVijT2AXvQQo= diff --git a/test.yml b/test.yml new file mode 100644 index 0000000..bbea780 --- /dev/null +++ b/test.yml @@ -0,0 +1,4 @@ +aa: bb +aaa: 10s +bar: foo +foo: bar diff --git a/viper.go b/viper.go index a8f84b5..15e1d65 100644 --- a/viper.go +++ b/viper.go @@ -213,7 +213,8 @@ type Viper struct { // This will only be used if the configuration read is a properties file. properties *properties.Properties - onConfigChange func(fsnotify.Event) + onConfigChange func(fsnotify.Event) + cancelWatchConfig func() } // New returns an initialized Viper instance. @@ -330,80 +331,87 @@ var SupportedExts = []string{"json", "toml", "yaml", "yml", "properties", "props // SupportedRemoteProviders are universally supported remote providers. var SupportedRemoteProviders = []string{"etcd", "consul"} +// OnConfigChange is used to set the handler to run for catching the event +// when the configFile have been externally modified by the filesystem. func OnConfigChange(run func(in fsnotify.Event)) { v.OnConfigChange(run) } func (v *Viper) OnConfigChange(run func(in fsnotify.Event)) { v.onConfigChange = run } +// CancelWatchConfig finishes the current configFile watch if it runs. +// When this function returns the configFile changes are not more looked after. +func CancelWatchConfig() { v.cancelWatchConfig() } +func (v *Viper) CancelWatchConfig() { + if v.cancelWatchConfig != nil { + v.cancelWatchConfig() + v.cancelWatchConfig = nil + } +} + +// WatchConfig watches and notifies changes on the file configFile. func WatchConfig() { v.WatchConfig() } - func (v *Viper) WatchConfig() { - initWG := sync.WaitGroup{} - initWG.Add(1) + // Make sure not to run twice this routine + v.CancelWatchConfig() + + // we have to watch the entire directory to pick up renames/atomic saves in a cross-platform way + filename, err := v.getConfigFile() + if err != nil { + log.Printf("error: %v\n", err) + return + } + + configFile := filepath.Clean(filename) + configDir, _ := filepath.Split(configFile) + realConfigFile, _ := filepath.EvalSymlinks(filename) + + watcher, err := fsnotify.NewWatcher() + if err != nil { + log.Fatal(err) + } + watcher.Add(configDir) + + // cancellation and waiting point + var watcherGroup sync.WaitGroup + v.cancelWatchConfig = func() { + watcher.Close() + watcherGroup.Wait() + } + // Process watcher events + watcherGroup.Add(1) go func() { - watcher, err := fsnotify.NewWatcher() - if err != nil { - log.Fatal(err) - } - defer watcher.Close() - // we have to watch the entire directory to pick up renames/atomic saves in a cross-platform way - filename, err := v.getConfigFile() - if err != nil { - log.Printf("error: %v\n", err) - initWG.Done() - return - } - - configFile := filepath.Clean(filename) - configDir, _ := filepath.Split(configFile) - realConfigFile, _ := filepath.EvalSymlinks(filename) - - eventsWG := sync.WaitGroup{} - eventsWG.Add(1) - go func() { - for { - select { - case event, ok := <-watcher.Events: - if !ok { // 'Events' channel is closed - eventsWG.Done() - return - } - currentConfigFile, _ := filepath.EvalSymlinks(filename) - // we only care about the config file with the following cases: - // 1 - if the config file was modified or created - // 2 - if the real path to the config file changed (eg: k8s ConfigMap replacement) - const writeOrCreateMask = fsnotify.Write | fsnotify.Create - if (filepath.Clean(event.Name) == configFile && - event.Op&writeOrCreateMask != 0) || - (currentConfigFile != "" && currentConfigFile != realConfigFile) { - realConfigFile = currentConfigFile - err := v.ReadInConfig() - if err != nil { - log.Printf("error reading config file: %v\n", err) - } - if v.onConfigChange != nil { - v.onConfigChange(event) - } - } else if filepath.Clean(event.Name) == configFile && - event.Op&fsnotify.Remove&fsnotify.Remove != 0 { - eventsWG.Done() - return - } - - case err, ok := <-watcher.Errors: - if ok { // 'Errors' channel is not closed - log.Printf("watcher error: %v\n", err) - } - eventsWG.Done() - return + defer watcherGroup.Done() + for event := range watcher.Events { + currentConfigFile, _ := filepath.EvalSymlinks(filename) + // we only care about the config file with the following cases: + // 1 - if the config file was modified or created + // 2 - if the real path to the config file changed (eg: k8s ConfigMap replacement) + const writeOrCreateMask = fsnotify.Write | fsnotify.Create + if (filepath.Clean(event.Name) == configFile && + event.Op&writeOrCreateMask != 0) || + (currentConfigFile != "" && currentConfigFile != realConfigFile) { + realConfigFile = currentConfigFile + err := v.ReadInConfig() + if err != nil { + log.Printf("error reading config file: %v\n", err) } + if v.onConfigChange != nil { + v.onConfigChange(event) + } + } else if filepath.Clean(event.Name) == configFile && + event.Op&fsnotify.Remove&fsnotify.Remove != 0 { + return } - }() - watcher.Add(configDir) - initWG.Done() // done initializing the watch in this go routine, so the parent routine can move on... - eventsWG.Wait() // now, wait for event loop to end in this go-routine... + } + }() + // Process watcher errors + watcherGroup.Add(1) + go func() { + defer watcherGroup.Done() + for err := range watcher.Errors { + log.Printf("watcher error: %v\n", err) + } }() - initWG.Wait() // make sure that the go routine above fully ended before returning } // SetConfigFile explicitly defines the path, name and extension of the config file. @@ -1379,6 +1387,10 @@ func (v *Viper) MergeConfigMap(cfg map[string]interface{}) error { // WriteConfig writes the current configuration to a file. func WriteConfig() error { return v.WriteConfig() } func (v *Viper) WriteConfig() error { + if v.cancelWatchConfig != nil { + v.cancelWatchConfig() + defer v.WatchConfig() + } filename, err := v.getConfigFile() if err != nil { return err diff --git a/viper_test.go b/viper_test.go index f9dc34a..ac5f46a 100644 --- a/viper_test.go +++ b/viper_test.go @@ -18,7 +18,6 @@ import ( "runtime" "sort" "strings" - "sync" "testing" "time" @@ -1992,17 +1991,15 @@ func TestWatchFile(t *testing.T) { defer cleanup() _, err := os.Stat(configFile) require.NoError(t, err) - t.Logf("test config file: %s\n", configFile) - wg := sync.WaitGroup{} - wg.Add(1) + done := make(chan struct{}) v.OnConfigChange(func(in fsnotify.Event) { t.Logf("config file changed") - wg.Done() + close(done) }) v.WatchConfig() // when overwriting the file and waiting for the custom change notification handler to be triggered err = ioutil.WriteFile(configFile, []byte("foo: baz\n"), 0640) - wg.Wait() + <-done // then the config value should have changed require.Nil(t, err) assert.Equal(t, "baz", v.Get("foo")) @@ -2015,13 +2012,12 @@ func TestWatchFile(t *testing.T) { } v, watchDir, _, _ := newViperWithSymlinkedConfigFile(t) // defer cleanup() - wg := sync.WaitGroup{} v.WatchConfig() + done := make(chan struct{}) v.OnConfigChange(func(in fsnotify.Event) { t.Logf("config file changed") - wg.Done() + close(done) }) - wg.Add(1) // when link to another `config.yaml` file dataDir2 := path.Join(watchDir, "data2") err := os.Mkdir(dataDir2, 0777) @@ -2032,11 +2028,123 @@ func TestWatchFile(t *testing.T) { // change the symlink using the `ln -sfn` command err = exec.Command("ln", "-sfn", dataDir2, path.Join(watchDir, "data")).Run() require.Nil(t, err) - wg.Wait() + <-done // then require.Nil(t, err) assert.Equal(t, "baz", v.Get("foo")) }) + + t.Run("file content changed after cancel", func(t *testing.T) { + // given a `config.yaml` file being watched + v, configFile, cleanup := newViperWithConfigFile(t) + defer cleanup() + _, err := os.Stat(configFile) + require.NoError(t, err) + + // first run through with watching enabled + done := make(chan struct{}) + v.OnConfigChange(func(in fsnotify.Event) { + done <- struct{}{} + }) + v.WatchConfig() + // when overwriting the file and waiting for the custom change notification handler to be triggered + err = ioutil.WriteFile(configFile, []byte("foo: baz\n"), 0640) + <-done + // then the config value should have changed + require.Nil(t, err) + assert.Equal(t, "baz", v.Get("foo")) + + // cancel and wait for the canceling to finish. + v.OnConfigChange(func(in fsnotify.Event) { + t.Error("CancelWatchConfig did not prevent second change from being seen.") + }) + v.CancelWatchConfig() + + // use another viper as a signal to wait this invisible write + v2 := New() + v2.SetConfigFile(configFile) + v2.WatchConfig() + v2.OnConfigChange(func(in fsnotify.Event) { + close(done) + }) + err = ioutil.WriteFile(configFile, []byte("foo: quz\n"), 0640) + <-done + // the config value should still be the same. + require.Nil(t, err) + assert.Equal(t, "baz", v.Get("foo"), "CancelWatchConfig did not prevent second change from being seen.") + }) + + t.Run("do not watchConfig during writeConfig", func(t *testing.T) { + // given a `config.yaml` file being watched + v, configFile, cleanup := newViperWithConfigFile(t) + defer cleanup() + _, err := os.Stat(configFile) + require.NoError(t, err) + + // first run through with watching enabled + done := make(chan struct{}) + v.OnConfigChange(func(in fsnotify.Event) { + close(done) + }) + v.WatchConfig() + // when overwriting the file and waiting for the custom change notification handler to be triggered + err = ioutil.WriteFile(configFile, []byte("foo: baz\nbar: foo\n"), 0640) + <-done + // then the config value should have changed + require.Nil(t, err) + assert.Equal(t, "baz", v.Get("foo")) + + v.Set("foo", "bar") + v.WriteConfig() + v.Set("baz", "foo") + v.WriteConfig() + v.Set("aa", "bb") + v.WriteConfig() + v.Set("aaa", "10s") + v.WriteConfig() + + f, err := ioutil.ReadFile(configFile) + assert.NoError(t, err) + assert.Equal(t, string([]byte("aa: bb\naaa: 10s\nbar: foo\nbaz: foo\nfoo: bar\n")), string(f)) + }) + t.Run("still watchConfig after writeConfig", func(t *testing.T) { + // given a `config.yaml` file being watched + v, configFile, cleanup := newViperWithConfigFile(t) + defer cleanup() + _, err := os.Stat(configFile) + require.NoError(t, err) + + // first run through with watching enabled + done := make(chan struct{}) + v.OnConfigChange(func(in fsnotify.Event) { + done <- struct{}{} + }) + v.WatchConfig() + // when overwriting the file and waiting for the custom change notification handler to be triggered + err = ioutil.WriteFile(configFile, []byte("foo: baz\nbar: foo\n"), 0640) + <-done + // then the config value should have changed + require.Nil(t, err) + assert.Equal(t, "baz", v.Get("foo")) + + v.Set("foo", "bar") + v.Set("baz", "foo") + v.Set("aa", "bb") + v.Set("aaa", "10s") + v.WriteConfig() + + f, err := ioutil.ReadFile(configFile) + assert.NoError(t, err) + assert.Equal(t, string([]byte("aa: bb\naaa: 10s\nbar: foo\nbaz: foo\nfoo: bar\n")), string(f)) + // Check still watching after write + v.OnConfigChange(func(in fsnotify.Event) { + close(done) + }) + err = ioutil.WriteFile(configFile, []byte("foo: bar\n"), 0640) + require.Nil(t, err) + <-done // then the config value should have changed + assert.Equal(t, "bar", v.Get("foo")) + }) } func TestUnmarshal_DotSeparatorBackwardCompatibility(t *testing.T) {