Merge pull request #105 from MV10/multiple-value-exception

Multiple value exception, fixes #103

Author: nblumhardt

src/Serilog.Settings.Configuration/Settings/Configuration/ConfigurationReader.cs

@@ -221,6 +221,16 @@ internal ILookup<string, Dictionary<string, IConfigurationArgumentValue>> GetMet
             IConfigurationArgumentValue GetArgumentValue(IConfigurationSection argumentSection)
             {
                 IConfigurationArgumentValue argumentValue;
+
+                // Reject configurations where an element has both scalar and complex
+                // values as a result of reading multiple configuration sources.
+                if (argumentSection.Value != null && argumentSection.GetChildren().Any())
+                    throw new InvalidOperationException(
+                        $"The value for the argument '{argumentSection.Path}' is assigned different value " +
+                        "types in more than one configuration source. Ensure all configurations consistently " +
+                        "use either a scalar (int, string, boolean) or a complex (array, section, list, " +
+                        "POCO, etc.) type for this argument value.");
+
                 if (argumentSection.Value != null)
                 {
                     argumentValue = new StringArgumentValue(() => argumentSection.Value, argumentSection.GetReloadToken);

test/Serilog.Settings.Configuration.Tests/ConfigurationSettingsTests.cs

@@ -12,9 +12,12 @@ namespace Serilog.Settings.Configuration.Tests
 {
     public class ConfigurationSettingsTests
     {
-        static LoggerConfiguration ConfigFromJson(string jsonString)
+        static LoggerConfiguration ConfigFromJson(string jsonString, string secondJsonSource = null)
         {
-            var config = new ConfigurationBuilder().AddJsonString(jsonString).Build();
+            var builder = new ConfigurationBuilder().AddJsonString(jsonString);
+            if (secondJsonSource != null)
+                builder.AddJsonString(secondJsonSource);
+            var config = builder.Build();
             return new LoggerConfiguration()
                 .ReadFrom.Configuration(config);
         }
@@ -31,7 +34,7 @@ public void PropertyEnrichmentIsApplied()
                     }
                 }
             }";
-            
+
             var log = ConfigFromJson(json)
                 .WriteTo.Sink(new DelegatingSink(e => evt = e))
                 .CreateLogger();
@@ -114,7 +117,7 @@ public void AuditSinksAreConfigured()
 
             var log = ConfigFromJson(json)
                 .CreateLogger();
-            
+
             DummyRollingFileSink.Emitted.Clear();
             DummyRollingFileAuditSink.Emitted.Clear();
 
@@ -227,7 +230,7 @@ public void LoggingLevelSwitchWithInvalidNameThrowsFormatException()
                     ""LevelSwitches"": {""switchNameNotStartingWithDollar"" : ""Warning"" }
                 }
             }";
-            
+
             var ex = Assert.Throws<FormatException>(() => ConfigFromJson(json));
 
             Assert.Contains("\"switchNameNotStartingWithDollar\"", ex.Message);
@@ -271,7 +274,7 @@ public void SettingMinimumLevelControlledByToAnUndeclaredSwitchThrows()
                     }
                 }
             }";
-            
+
             var ex = Assert.Throws<InvalidOperationException>(() =>
                 ConfigFromJson(json)
                     .CreateLogger());
@@ -332,7 +335,7 @@ public void ReferencingAnUndeclaredSwitchInSinkThrows()
                     }]      
                 }
             }";
-            
+
             var ex = Assert.Throws<InvalidOperationException>(() =>
                 ConfigFromJson(json)
                     .CreateLogger());
@@ -544,8 +547,8 @@ public void WriteToSubLoggerWithLevelSwitchIsSupported()
                     }
                 }]        
             }
-            }";            
-             
+            }";
+
             var log = ConfigFromJson(json)
             .CreateLogger();
 
@@ -556,5 +559,43 @@ public void WriteToSubLoggerWithLevelSwitchIsSupported()
 
             Assert.Equal(1, DummyRollingFileSink.Emitted.Count);
         }
+
+        [Trait("Bugfix", "#103")]
+        [Fact]
+        public void InconsistentComplexVsScalarArgumentValuesThrowsIOE()
+        {
+            var jsonDiscreteValue = @"{
+                ""Serilog"": {            
+                    ""Using"": [""TestDummies""],
+                    ""WriteTo"": [{
+                        ""Name"": ""DummyRollingFile"",
+                        ""Args"": {""pathFormat"" : ""C:\\""}
+                    }]        
+                }
+            }";
+
+            var jsonComplexValue = @"{
+                ""Serilog"": {            
+                    ""Using"": [""TestDummies""],
+                    ""WriteTo"": [{
+                        ""Name"": ""DummyRollingFile"",
+                        ""Args"": {""pathFormat"" : { ""foo"" : ""bar"" } }
+                    }]        
+                }
+            }";
+
+            // These will combine into a ConfigurationSection object that has both
+            // Value == "C:\" and GetChildren() == List<string>. No configuration
+            // extension matching this exists (in theory an "object" argument could
+            // accept either value). ConfigurationReader should throw as soon as
+            // the multiple values are recognized; it will never attempt to locate
+            // a matching argument.
+
+            var ex = Assert.Throws<InvalidOperationException>(()
+                => ConfigFromJson(jsonDiscreteValue, jsonComplexValue));
+
+            Assert.Contains("The value for the argument", ex.Message);
+            Assert.Contains("'Serilog:WriteTo:0:Args:pathFormat'", ex.Message);
+        }
     }
 }