Resolves #742

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()

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
 )

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=

test.yml

@@ -0,0 +1,4 @@
+aa: bb
+aaa: 10s
+bar: foo
+foo: bar

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{}
+	// Make sure not to run twice this routine
-	initWG.Add(1)
+	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()
+		defer watcherGroup.Done()
-		if err != nil {
+		for event := range watcher.Events {
-			log.Fatal(err)
+			currentConfigFile, _ := filepath.EvalSymlinks(filename)
-		}
+			// we only care about the config file with the following cases:
-		defer watcher.Close()
+			// 1 - if the config file was modified or created
-		// we have to watch the entire directory to pick up renames/atomic saves in a cross-platform way
+			// 2 - if the real path to the config file changed (eg: k8s ConfigMap replacement)
-		filename, err := v.getConfigFile()
+			const writeOrCreateMask = fsnotify.Write | fsnotify.Create
-		if err != nil {
+			if (filepath.Clean(event.Name) == configFile &&
-			log.Printf("error: %v\n", err)
+				event.Op&writeOrCreateMask != 0) ||
-			initWG.Done()
+				(currentConfigFile != "" && currentConfigFile != realConfigFile) {
-			return
+				realConfigFile = currentConfigFile
-		}
+				err := v.ReadInConfig()
-
+				if err != nil {
-		configFile := filepath.Clean(filename)
+					log.Printf("error reading config file: %v\n", err)
-		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
 				}
+				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...
+	// Process watcher errors
-		eventsWG.Wait() // now, wait for event loop to end in this go-routine...
+	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

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)
+		done := make(chan struct{})
-		wg := sync.WaitGroup{}
-		wg.Add(1)
 		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) {