From cd627866d87a5a7f84dd213652a92e08f5833b87 Mon Sep 17 00:00:00 2001
From: brumhard <fobytobi@googlemail.com>
Date: Tue, 30 Mar 2021 22:19:44 +0200
Subject: [PATCH] Fix override with zero value for readers source

---
 README.md         |  3 +++
 alligotor_test.go | 14 ++++++--------
 readers.go        | 14 +++++++++-----
 3 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/README.md b/README.md
index 63f6dbb..0f6ad96 100644
--- a/README.md
+++ b/README.md
@@ -277,6 +277,9 @@ For sources that only support setting values as strings (like for example enviro
 slice containing the string and it will automatically be converted to the target type if possible. Any other type is
 used directly leading to an error on type mismatch.
 
+> You should not return structs directly since this could lead to errors if some struct properties are set and others
+> are not. This would then overwrite the target with the zero value, which is not intended.
+
 The received fields match directly to the fields in the config. So for example for a config struct like the following:
 
 ```Go
diff --git a/alligotor_test.go b/alligotor_test.go
index a834dcc..0aa36c6 100644
--- a/alligotor_test.go
+++ b/alligotor_test.go
@@ -280,28 +280,27 @@ var _ = Describe("config", func() {
 					})
 					Context("file is set", func() {
 						BeforeEach(func() {
-							jsonBytes := []byte(`{"sleep": "2s","api": {"port": 2, "logLevel": "file"}, "db": {"logLevel": "file"}}`)
+							jsonBytes := []byte(`{"sleep": "2s","api": {"port": 2}, "db": {"logLevel": "file"}}`)
 							Expect(os.WriteFile(path.Join(tempDir, fileBaseName+".json"), jsonBytes, 0600)).To(Succeed())
 						})
-						It("overrides defaults", func() {
+						It("overrides defaults only if set", func() {
 							Expect(c.Get(&testingStruct)).To(Succeed())
 							Expect(testingStruct.Sleep).To(Equal(2 * time.Second))
 							Expect(testingStruct.API.Port).To(Equal(2))
-							Expect(testingStruct.API.LogLevel).To(Equal("file"))
+							Expect(testingStruct.API.LogLevel).To(Equal("info"))
 							Expect(testingStruct.DB.LogLevel).To(Equal("file"))
 						})
 						Context("env is set", func() {
 							BeforeEach(func() {
 								Expect(os.Setenv("SLEEP", "3s")).To(Succeed())
 								Expect(os.Setenv("API_PORT", "3")).To(Succeed())
-								Expect(os.Setenv("API_LOGLEVEL", "env")).To(Succeed())
 								Expect(os.Setenv("DB_LOGLEVEL", "env")).To(Succeed())
 							})
-							It("overrides file", func() {
+							It("overrides file, keeps defaults if not set", func() {
 								Expect(c.Get(&testingStruct)).To(Succeed())
 								Expect(testingStruct.Sleep).To(Equal(3 * time.Second))
 								Expect(testingStruct.API.Port).To(Equal(3))
-								Expect(testingStruct.API.LogLevel).To(Equal("env"))
+								Expect(testingStruct.API.LogLevel).To(Equal("info"))
 								Expect(testingStruct.DB.LogLevel).To(Equal("env"))
 							})
 							Context("flags are set", func() {
@@ -309,7 +308,6 @@ var _ = Describe("config", func() {
 									os.Args = []string{"commandName",
 										"--sleep", "4s",
 										"--api.port", "4",
-										"--api.loglevel", "flag",
 										"--db.loglevel", "flag",
 									}
 								})
@@ -318,7 +316,7 @@ var _ = Describe("config", func() {
 									Expect(testingStruct.Enabled).To(Equal(true))
 									Expect(testingStruct.Sleep).To(Equal(4 * time.Second))
 									Expect(testingStruct.API.Port).To(Equal(4))
-									Expect(testingStruct.API.LogLevel).To(Equal("flag"))
+									Expect(testingStruct.API.LogLevel).To(Equal("info"))
 									Expect(testingStruct.DB.LogLevel).To(Equal("flag"))
 								})
 							})
diff --git a/readers.go b/readers.go
index 088a56c..731341c 100644
--- a/readers.go
+++ b/readers.go
@@ -104,15 +104,19 @@ func readFileMap(f *Field, m *ciMap) (interface{}, error) {
 
 	fieldTypeNew := reflect.New(f.Type())
 
-	if err := mapstructure.Decode(valueForField, fieldTypeNew.Interface()); err != nil {
-		// if theres a type mismatch check if value is a string so maybe it can be parsed
+	if f.Type().Kind() == reflect.Struct {
+		// if it's a struct, it could be assigned with TextUnmarshaler, otherwise return nil
 		if valueString, ok := valueForField.(string); ok {
 			return []byte(valueString), nil
 		}
 
-		// if it's a struct, maybe one of the properties can be assigned nevertheless
-		if f.Type().Kind() == reflect.Struct {
-			return nil, nil
+		return nil, nil
+	}
+
+	if err := mapstructure.Decode(valueForField, fieldTypeNew.Interface()); err != nil {
+		// if theres a type mismatch check if value is a string so maybe it can be parsed
+		if valueString, ok := valueForField.(string); ok {
+			return []byte(valueString), nil
 		}
 
 		return nil, err