Skip to content

Commit

Permalink
Pass a IReadOnlyDictionary to KeyValuePairSettings
Browse files Browse the repository at this point in the history
Refactored KeyValuePairSettings to accept a IReadOnlyDictionary<string, string> instead of IEnumerable<KeyValuePair<string, string>> in order to reveal the fact that key-value settings are not lazily consumed. 

Added tests on edge cases : duplicate keys, irrelevant settings.
  • Loading branch information
Thibaud DESODT authored and tsimbalar committed Nov 8, 2017
1 parent 9f27375 commit 670a5ff
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 13 deletions.
6 changes: 5 additions & 1 deletion src/Serilog/Configuration/LoggerSettingsConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;
using Serilog.Settings.KeyValuePairs;

namespace Serilog.Configuration
Expand Down Expand Up @@ -52,7 +54,9 @@ public LoggerConfiguration Settings(ILoggerSettings settings)
public LoggerConfiguration KeyValuePairs(IEnumerable<KeyValuePair<string, string>> settings)
{
if (settings == null) throw new ArgumentNullException(nameof(settings));
return Settings(new KeyValuePairSettings(settings));

var settingsDictionary = new ReadOnlyDictionary<string, string>(settings.ToDictionary(x => x.Key, x => x.Value));
return Settings(new KeyValuePairSettings(settingsDictionary));
}
}
}
26 changes: 16 additions & 10 deletions src/Serilog/Settings/KeyValuePairs/KeyValuePairSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,21 +73,18 @@ 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 = ExtractDirectives(_settings);

var declaredLevelSwitches = ParseNamedLevelSwitchDeclarationDirectives(directives);

Expand Down Expand Up @@ -164,12 +161,21 @@ where matchCallables.IsMatch(wt.Key)
}
}

internal static IReadOnlyDictionary<string, string> ExtractDirectives(IReadOnlyDictionary<string, string> settings)
{
var directives = settings
.Where(kvp => _supportedDirectives.Any(kvp.Key.StartsWith))
.ToDictionary(kvp => kvp.Key, kvp => kvp.Value);

return new ReadOnlyDictionary<string, string>(directives);
}

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

Expand Down Expand Up @@ -255,7 +261,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 };

Expand Down
33 changes: 31 additions & 2 deletions test/Serilog.Tests/Settings/KeyValuePairSettingsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,35 @@ namespace Serilog.Tests.Settings
{
public class KeyValuePairSettingsTests
{
[Fact]
public void DuplicateKeysCauseArgumentException()
{
Action action = () => new LoggerConfiguration()
.ReadFrom.KeyValuePairs(new List<KeyValuePair<string, string>>
{
new KeyValuePair<string, string>("setting", "Value"),
new KeyValuePair<string, string>("setting", "SameSettingOtherValue"),
});

var ex = Assert.ThrowsAny<ArgumentException>(action);
Assert.NotNull(ex);
Assert.Contains("An item with the same key has already been added.", ex.Message);
}

[Fact]
public void IrrelevantKeysAreIgnored()
{
var irrelevantSettings = new Dictionary<string, string>()
{
["whatever:foo:bar"] = "willBeIgnored",
["irrelevant"] = "willBeIgnored",
};

var directives = KeyValuePairSettings.ExtractDirectives(irrelevantSettings);

Assert.False(directives.Any());
}

[Fact]
public void FindsConfigurationAssemblies()
{
Expand Down Expand Up @@ -181,7 +210,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);
Expand Down Expand Up @@ -301,7 +330,7 @@ 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");

Expand Down

0 comments on commit 670a5ff

Please sign in to comment.