Merge pull request serilog#1051 from tsimbalar/kvpsettings-readonlydict

Handle duplicate key-value pair settings - last-in wins

src/Serilog/Configuration/LoggerSettingsConfiguration.cs

@@ -14,6 +14,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using Serilog.Settings.KeyValuePairs;
 
 namespace Serilog.Configuration
@@ -49,9 +50,20 @@ public LoggerConfiguration Settings(ILoggerSettings settings)
         /// </summary>
         /// <param name="settings">A list of key-value pairs describing logger settings.</param>
         /// <returns>Configuration object allowing method chaining.</returns>
+        /// <remarks>In case of duplicate keys, the last value for the key is kept and the previous ones are ignored.</remarks>
         public LoggerConfiguration KeyValuePairs(IEnumerable<KeyValuePair<string, string>> settings)
         {
             if (settings == null) throw new ArgumentNullException(nameof(settings));
+            var uniqueSettings = new Dictionary<string, string>();
+            foreach (var kvp in settings)
+            {
+                uniqueSettings[kvp.Key] = kvp.Value;
+            }
+            return KeyValuePairs(uniqueSettings);
+        }
+
+        LoggerConfiguration KeyValuePairs(IReadOnlyDictionary<string, string> settings)
+        {
             return Settings(new KeyValuePairSettings(settings));
         }
     }

src/Serilog/Settings/KeyValuePairs/KeyValuePairSettings.cs

@@ -14,7 +14,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Collections.ObjectModel;
 using System.Linq;
 using System.Reflection;
 using System.Text.RegularExpressions;
@@ -73,21 +72,20 @@ class KeyValuePairSettings : ILoggerSettings
             [typeof(LoggerFilterConfiguration)] = lc => lc.Filter
         };
 
-        readonly Dictionary<string, string> _settings;
+        readonly IReadOnlyDictionary<string, string> _settings;
 
-        public KeyValuePairSettings(IEnumerable<KeyValuePair<string, string>> settings)
+        public KeyValuePairSettings(IReadOnlyDictionary<string, string> settings)
         {
-            if (settings == null) throw new ArgumentNullException(nameof(settings));
-            _settings = settings.ToDictionary(s => s.Key, s => s.Value);
+            _settings = settings ?? throw new ArgumentNullException(nameof(settings));
         }
 
         public void Configure(LoggerConfiguration loggerConfiguration)
         {
             if (loggerConfiguration == null) throw new ArgumentNullException(nameof(loggerConfiguration));
 
-            var directives = _settings.Keys
-                .Where(k => _supportedDirectives.Any(k.StartsWith))
-                .ToDictionary(k => k, k => _settings[k]);
+            var directives = _settings
+                .Where(kvp => _supportedDirectives.Any(kvp.Key.StartsWith))
+                .ToDictionary(kvp => kvp.Key, kvp => kvp.Value);
 
             var declaredLevelSwitches = ParseNamedLevelSwitchDeclarationDirectives(directives);
 
@@ -169,7 +167,7 @@ internal static bool IsValidSwitchName(string input)
             return Regex.IsMatch(input, LevelSwitchNameRegex);
         }
 
-        static IReadOnlyDictionary<string, LoggingLevelSwitch> ParseNamedLevelSwitchDeclarationDirectives(Dictionary<string, string> directives)
+        static IReadOnlyDictionary<string, LoggingLevelSwitch> ParseNamedLevelSwitchDeclarationDirectives(IReadOnlyDictionary<string, string> directives)
         {
             var matchLevelSwitchDeclarations = new Regex(LevelSwitchDeclarationDirectiveRegex);
 
@@ -204,7 +202,7 @@ where matchLevelSwitchDeclarations.IsMatch(wt.Key)
                 }
                 namedSwitches.Add(switchName, newSwitch);
             }
-            return new ReadOnlyDictionary<string, LoggingLevelSwitch>(namedSwitches);
+            return namedSwitches;
         }
 
         static LoggingLevelSwitch LookUpSwitchByName(string switchName, IReadOnlyDictionary<string, LoggingLevelSwitch> declaredLevelSwitches)
@@ -255,7 +253,7 @@ internal static MethodInfo SelectConfigurationMethod(IEnumerable<MethodInfo> can
                 .FirstOrDefault();
         }
 
-        internal static IEnumerable<Assembly> LoadConfigurationAssemblies(Dictionary<string, string> directives)
+        internal static IEnumerable<Assembly> LoadConfigurationAssemblies(IReadOnlyDictionary<string, string> directives)
         {
             var configurationAssemblies = new List<Assembly> { typeof(ILogger).GetTypeInfo().Assembly };
 

test/Serilog.Tests/Settings/KeyValuePairSettingsTests.cs

@@ -15,6 +15,26 @@ namespace Serilog.Tests.Settings
 {
     public class KeyValuePairSettingsTests
     {
+        [Fact]
+        public void LastValueIsTakenWhenKeysAreDuplicate()
+        {
+            LogEvent evt = null;
+            var log = new LoggerConfiguration()
+                .ReadFrom.KeyValuePairs(new List<KeyValuePair<string, string>>
+                {
+                    new KeyValuePair<string, string>("enrich:with-property:App", "InitialValue"),
+                    new KeyValuePair<string, string>("enrich:with-property:App", "OverridenValue"),
+                    new KeyValuePair<string, string>("enrich:with-property:App", "FinalValue")
+                })
+                .WriteTo.Sink(new DelegatingSink(e => evt = e))
+                .CreateLogger();
+
+            log.Information("Has a test property");
+
+            Assert.NotNull(evt);
+            Assert.Equal("FinalValue", evt.Properties["App"].LiteralValue());
+        }
+
         [Fact]
         public void FindsConfigurationAssemblies()
         {
@@ -181,7 +201,7 @@ public void LoggingLevelSwitchWithInvalidNameThrowsFormatException()
                 ["level-switch:switchNameNotStartingWithDollar"] = "Warning",
             };
 
-            var ex = Assert.Throws<FormatException>(() =>  new LoggerConfiguration()
+            var ex = Assert.Throws<FormatException>(() => new LoggerConfiguration()
                 .ReadFrom.KeyValuePairs(settings));
 
             Assert.Contains("\"switchNameNotStartingWithDollar\"", ex.Message);
@@ -301,19 +321,19 @@ public void LoggingLevelSwitchCanBeUsedForMinimumLevelOverrides()
                 .CreateLogger();
 
             var systemLogger = log.ForContext(Constants.SourceContextPropertyName, "System.Bar");
-            
+
             log.Write(Some.InformationEvent());
             Assert.False(evt is null, "Minimul level is Debug. It should log Information messages");
 
             evt = null;
-
+            // ReSharper disable HeuristicUnreachableCode
             systemLogger.Write(Some.InformationEvent());
             Assert.True(evt is null, "LoggingLevelSwitch initial level was Warning for logger System.*. It should not log Information messages for SourceContext System.Bar");
 
             systemLogger.Write(Some.WarningEvent());
             Assert.False(evt is null, "LoggingLevelSwitch initial level was Warning for logger System.*. It should log Warning messages for SourceContext System.Bar");
 
-            // ReSharper disable HeuristicUnreachableCode
+
             evt = null;
             var controlSwitch = DummyWithLevelSwitchSink.ControlLevelSwitch;