From 3a19b6e0d99b8272ed919058bfbba07730296dad Mon Sep 17 00:00:00 2001
From: javaducky <javaducky@gmail.com>
Date: Tue, 3 Dec 2019 19:14:08 -0600
Subject: [PATCH] Review updates to utilize afero for file checks and updated
 checks on unit tests

---
 viper.go      | 15 ++++-----------
 viper_test.go | 32 +++++++-------------------------
 2 files changed, 11 insertions(+), 36 deletions(-)

diff --git a/viper.go b/viper.go
index 6167aff..a475e95 100644
--- a/viper.go
+++ b/viper.go
@@ -23,6 +23,7 @@ import (
 	"bytes"
 	"encoding/csv"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"log"
@@ -123,14 +124,6 @@ func (faee ConfigFileAlreadyExistsError) Error() string {
 	return fmt.Sprintf("Config File %q Already Exists", string(faee))
 }
 
-// MissingConfigurationError denotes a required configuration setting has not been provided.
-type MissingConfigurationError string
-
-// Error returns the formatted error when a required configuration element has not been provided.
-func (mce MissingConfigurationError) Error() string {
-	return fmt.Sprintf("Missing Configuration for %q", string(mce))
-}
-
 // A DecoderConfigOption can be passed to viper.Unmarshal to configure
 // mapstructure.DecoderConfig options
 type DecoderConfigOption func(*mapstructure.DecoderConfig)
@@ -1354,7 +1347,7 @@ func (v *Viper) WriteConfig() error {
 func SafeWriteConfig() error { return v.SafeWriteConfig() }
 func (v *Viper) SafeWriteConfig() error {
 	if len(v.configPaths) < 1 {
-		return MissingConfigurationError("configPath")
+		return errors.New("Missing configuration for 'configPath'")
 	}
 	return v.SafeWriteConfigAs(filepath.Join(v.configPaths[0], v.configName+"."+v.configType))
 }
@@ -1368,8 +1361,8 @@ func (v *Viper) WriteConfigAs(filename string) error {
 // SafeWriteConfigAs writes current configuration to a given filename if it does not exist.
 func SafeWriteConfigAs(filename string) error { return v.SafeWriteConfigAs(filename) }
 func (v *Viper) SafeWriteConfigAs(filename string) error {
-	handle, err := v.fs.Stat(filename)
-	if handle != nil && err == nil {
+	alreadyExists, err := afero.Exists(v.fs, filename)
+	if alreadyExists && err == nil {
 		return ConfigFileAlreadyExistsError(filename)
 	}
 	return v.writeConfig(filename, false)
diff --git a/viper_test.go b/viper_test.go
index 7651787..85c2d02 100644
--- a/viper_test.go
+++ b/viper_test.go
@@ -1391,17 +1391,10 @@ func TestSafeWriteConfig(t *testing.T) {
 	v.AddConfigPath("/test")
 	v.SetConfigName("c")
 	v.SetConfigType("yaml")
-	err := v.ReadConfig(bytes.NewBuffer(yamlExample))
-	if err != nil {
-		t.Fatal(err)
-	}
-	if err = v.SafeWriteConfig(); err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, v.ReadConfig(bytes.NewBuffer(yamlExample)))
+	require.NoError(t, v.SafeWriteConfig())
 	read, err := afero.ReadFile(fs, "/test/c.yaml")
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 	assert.Equal(t, yamlWriteExpected, read)
 }
 
@@ -1411,12 +1404,7 @@ func TestSafeWriteConfigWithMissingConfigPath(t *testing.T) {
 	v.SetFs(fs)
 	v.SetConfigName("c")
 	v.SetConfigType("yaml")
-	err := v.SafeWriteConfig()
-	if err == nil {
-		t.Fatal("Expected exception")
-	}
-	_, ok := err.(MissingConfigurationError)
-	assert.True(t, ok, "Expected MissingConfigurationError")
+	require.EqualError(t, v.SafeWriteConfig(), "Missing configuration for 'configPath'")
 }
 
 func TestSafeWriteConfigWithExistingFile(t *testing.T) {
@@ -1428,9 +1416,7 @@ func TestSafeWriteConfigWithExistingFile(t *testing.T) {
 	v.SetConfigName("c")
 	v.SetConfigType("yaml")
 	err := v.SafeWriteConfig()
-	if err == nil {
-		t.Fatal("Expected exception")
-	}
+	require.Error(t, err)
 	_, ok := err.(ConfigFileAlreadyExistsError)
 	assert.True(t, ok, "Expected ConfigFileAlreadyExistsError")
 }
@@ -1443,9 +1429,7 @@ func TestSafeWriteAsConfig(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err = v.SafeWriteConfigAs("/test/c.yaml"); err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, v.SafeWriteConfigAs("/test/c.yaml"))
 	if _, err = afero.ReadFile(fs, "/test/c.yaml"); err != nil {
 		t.Fatal(err)
 	}
@@ -1457,9 +1441,7 @@ func TestSafeWriteConfigAsWithExistingFile(t *testing.T) {
 	fs.Create("/test/c.yaml")
 	v.SetFs(fs)
 	err := v.SafeWriteConfigAs("/test/c.yaml")
-	if err == nil {
-		t.Fatal("Expected exception")
-	}
+	require.Error(t, err)
 	_, ok := err.(ConfigFileAlreadyExistsError)
 	assert.True(t, ok, "Expected ConfigFileAlreadyExistsError")
 }