Skip to content

Commit dc53ec3

Browse files
tsimbalartsimbalar
authored
Merge pull request #144 from tsimbalar/bugfix-143-confgsection
Do not access "Serilog" sub-section when calling ReadFrom.ConfigSection / properly populate `IConfiguration` in configuration methods
2 parents f919606 + 6353a04 commit dc53ec3

File tree

5 files changed

+193
-15
lines changed

5 files changed

+193
-15
lines changed

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

+15-3
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public ConfigurationReader(IConfiguration configuration, DependencyContext depen
3535

3636
// Generally the initial call should use IConfiguration rather than IConfigurationSection, otherwise
3737
// IConfiguration parameters in the target methods will not be populated.
38-
ConfigurationReader(IConfigurationSection configSection, DependencyContext dependencyContext)
38+
public ConfigurationReader(IConfigurationSection configSection, DependencyContext dependencyContext)
3939
{
4040
_section = configSection ?? throw new ArgumentNullException(nameof(configSection));
4141
_dependencyContext = dependencyContext;
@@ -331,7 +331,15 @@ static void CallConfigurationMethods(ILookup<string, Dictionary<string, IConfigu
331331
select directive.Key == null ? p.DefaultValue : directive.Value.ConvertTo(p.ParameterType, declaredLevelSwitches)).ToList();
332332

333333
var parm = methodInfo.GetParameters().FirstOrDefault(i => i.ParameterType == typeof(IConfiguration));
334-
if (parm != null) call[parm.Position - 1] = _configuration;
334+
if (parm != null)
335+
{
336+
if (_configuration is null)
337+
{
338+
throw new InvalidOperationException("Trying to invoke a configuration method accepting a `IConfiguration` argument. " +
339+
$"This is not supported when only a `IConfigSection` has been provided. (method '{methodInfo}')");
340+
}
341+
call[parm.Position - 1] = _configuration;
342+
}
335343

336344
call.Insert(0, receiver);
337345

@@ -348,7 +356,11 @@ internal static MethodInfo SelectConfigurationMethod(IEnumerable<MethodInfo> can
348356
return candidateMethods
349357
.Where(m => m.Name == name &&
350358
m.GetParameters().Skip(1)
351-
.All(p => p.HasDefaultValue || suppliedArgumentValues.Any(s => s.Key.Equals(p.Name, StringComparison.OrdinalIgnoreCase))))
359+
.All(p => p.HasDefaultValue
360+
|| suppliedArgumentValues.Any(s => s.Key.Equals(p.Name, StringComparison.OrdinalIgnoreCase))
361+
// parameters of type IConfiguration are implicitly populated with provided Configuration
362+
|| p.ParameterType == typeof(IConfiguration)
363+
))
352364
.OrderByDescending(m =>
353365
{
354366
var matchingArgs = m.GetParameters().Where(p => suppliedArgumentValues.Any(s => s.Key.Equals(p.Name, StringComparison.OrdinalIgnoreCase))).ToList();

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

+52-8
Original file line numberDiff line numberDiff line change
@@ -419,32 +419,76 @@ public void LoggingLevelSwitchCanBeUsedForMinimumLevelOverrides()
419419
}
420420

421421
[Fact]
422+
423+
[Trait("BugFix", "https://github.com/serilog/serilog-settings-configuration/issues/142")]
422424
public void SinkWithIConfigurationArguments()
423425
{
424426
var json = @"{
425427
""Serilog"": {
426428
""Using"": [""TestDummies""],
427429
""WriteTo"": [{
428-
""Name"": ""DummyRollingFile"",
429-
""Args"": {""pathFormat"" : ""C:\\"",
430-
""configurationSection"" : { ""foo"" : ""bar"" } }
430+
""Name"": ""DummyWithConfiguration"",
431+
""Args"": {}
431432
}]
432433
}
433434
}";
434435

435-
// IConfiguration and IConfigurationSection arguments do not have
436-
// default values so they will throw if they are not populated
437-
436+
DummyConfigurationSink.Reset();
438437
var log = ConfigFromJson(json)
439438
.CreateLogger();
440439

441-
DummyRollingFileSink.Reset();
440+
log.Write(Some.InformationEvent());
441+
442+
Assert.NotNull(DummyConfigurationSink.Configuration);
443+
}
444+
445+
[Fact]
446+
[Trait("BugFix", "https://github.com/serilog/serilog-settings-configuration/issues/142")]
447+
public void SinkWithOptionalIConfigurationArguments()
448+
{
449+
var json = @"{
450+
""Serilog"": {
451+
""Using"": [""TestDummies""],
452+
""WriteTo"": [{
453+
""Name"": ""DummyWithOptionalConfiguration"",
454+
""Args"": {}
455+
}]
456+
}
457+
}";
458+
459+
DummyConfigurationSink.Reset();
460+
var log = ConfigFromJson(json)
461+
.CreateLogger();
442462

443463
log.Write(Some.InformationEvent());
444464

445-
Assert.Equal(1, DummyRollingFileSink.Emitted.Count);
465+
Assert.NotNull(DummyConfigurationSink.Configuration);
466+
}
467+
468+
[Fact]
469+
public void SinkWithIConfigSectionArguments()
470+
{
471+
var json = @"{
472+
""Serilog"": {
473+
""Using"": [""TestDummies""],
474+
""WriteTo"": [{
475+
""Name"": ""DummyWithConfigSection"",
476+
""Args"": {""configurationSection"" : { ""foo"" : ""bar"" } }
477+
}]
478+
}
479+
}";
480+
481+
DummyConfigurationSink.Reset();
482+
var log = ConfigFromJson(json)
483+
.CreateLogger();
484+
485+
log.Write(Some.InformationEvent());
486+
487+
Assert.NotNull(DummyConfigurationSink.ConfigSection);
488+
Assert.Equal("bar", DummyConfigurationSink.ConfigSection["foo"]);
446489
}
447490

491+
448492
[Fact]
449493
public void SinkWithConfigurationBindingArgument()
450494
{

‎test/Serilog.Settings.Configuration.Tests/LoggerConfigurationExtensionsTests.cs

+63-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1-
using System;
1+
using System;
22
using Microsoft.Extensions.Configuration;
3+
using Serilog.Events;
4+
using Serilog.Settings.Configuration.Tests.Support;
35
using Xunit;
46

57
namespace Serilog.Settings.Configuration.Tests
@@ -14,5 +16,65 @@ public void ReadFromConfigurationShouldNotThrowOnEmptyConfiguration()
1416
// should not throw
1517
act();
1618
}
19+
20+
[Fact]
21+
[Trait("BugFix", "https://github.com/serilog/serilog-settings-configuration/issues/143")]
22+
public void ReadFromConfigurationSectionReadsFromAnArbitrarySection()
23+
{
24+
LogEvent evt = null;
25+
26+
var json = @"{
27+
""NotSerilog"": {
28+
""Properties"": {
29+
""App"": ""Test""
30+
}
31+
}
32+
}";
33+
34+
var config = new ConfigurationBuilder()
35+
.AddJsonString(json)
36+
.Build();
37+
38+
var log = new LoggerConfiguration()
39+
.ReadFrom.ConfigurationSection(config.GetSection("NotSerilog"))
40+
.WriteTo.Sink(new DelegatingSink(e => evt = e))
41+
.CreateLogger();
42+
43+
log.Information("Has a test property");
44+
45+
Assert.NotNull(evt);
46+
Assert.Equal("Test", evt.Properties["App"].LiteralValue());
47+
}
48+
49+
[Fact(Skip = "Passes when run alone, but fails when the whole suite is run - to fix")]
50+
[Trait("BugFix", "https://github.com/serilog/serilog-settings-configuration/issues/143")]
51+
public void ReadFromConfigurationSectionThrowsWhenTryingToCallConfigurationMethodWithIConfigurationParam()
52+
{
53+
var json = @"{
54+
""NotSerilog"": {
55+
""Using"": [""TestDummies""],
56+
""WriteTo"": [{
57+
""Name"": ""DummyWithConfiguration"",
58+
""Args"": {""pathFormat"" : ""C:\\"",
59+
""configurationSection"" : { ""foo"" : ""bar"" } }
60+
}]
61+
}
62+
}";
63+
64+
var config = new ConfigurationBuilder()
65+
.AddJsonString(json)
66+
.Build();
67+
68+
var exception = Assert.Throws<InvalidOperationException>(() =>
69+
new LoggerConfiguration()
70+
.ReadFrom.ConfigurationSection(config.GetSection("NotSerilog"))
71+
.CreateLogger());
72+
73+
Assert.Equal("Trying to invoke a configuration method accepting a `IConfiguration` argument. " +
74+
"This is not supported when only a `IConfigSection` has been provided. " +
75+
"(method 'Serilog.LoggerConfiguration DummyWithConfiguration(Serilog.Configuration.LoggerSinkConfiguration, Microsoft.Extensions.Configuration.IConfiguration, Microsoft.Extensions.Configuration.IConfigurationSection, System.String, Serilog.Events.LogEventLevel)')",
76+
exception.Message);
77+
78+
}
1779
}
1880
}

‎test/TestDummies/DummyConfigurationSink.cs

+46
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using Microsoft.Extensions.Configuration;
4+
using Serilog.Core;
5+
using Serilog.Events;
6+
7+
namespace TestDummies
8+
{
9+
public class DummyConfigurationSink : ILogEventSink
10+
{
11+
[ThreadStatic]
12+
static List<LogEvent> _emitted;
13+
14+
[ThreadStatic]
15+
static IConfiguration _configuration;
16+
17+
[ThreadStatic]
18+
static IConfigurationSection _configSection;
19+
20+
public static List<LogEvent> Emitted => _emitted ?? (_emitted = new List<LogEvent>());
21+
22+
public static IConfiguration Configuration => _configuration;
23+
24+
public static IConfigurationSection ConfigSection => _configSection;
25+
26+
27+
public DummyConfigurationSink(IConfiguration configuration, IConfigurationSection configSection)
28+
{
29+
_configuration = configuration;
30+
_configSection = configSection;
31+
}
32+
33+
public void Emit(LogEvent logEvent)
34+
{
35+
Emitted.Add(logEvent);
36+
}
37+
38+
public static void Reset()
39+
{
40+
_emitted = null;
41+
_configuration = null;
42+
_configSection = null;
43+
}
44+
45+
}
46+
}

‎test/TestDummies/DummyLoggerConfigurationExtensions.cs

+17-3
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,28 @@ public static LoggerConfiguration DummyRollingFile(
3737
return loggerSinkConfiguration.Sink(new DummyRollingFileSink(), restrictedToMinimumLevel);
3838
}
3939

40-
public static LoggerConfiguration DummyRollingFile(
40+
public static LoggerConfiguration DummyWithConfiguration(
4141
this LoggerSinkConfiguration loggerSinkConfiguration,
4242
IConfiguration appConfiguration,
43+
LogEventLevel restrictedToMinimumLevel = LevelAlias.Minimum)
44+
{
45+
return loggerSinkConfiguration.Sink(new DummyConfigurationSink(appConfiguration, null), restrictedToMinimumLevel);
46+
}
47+
48+
public static LoggerConfiguration DummyWithOptionalConfiguration(
49+
this LoggerSinkConfiguration loggerSinkConfiguration,
50+
IConfiguration appConfiguration = null,
51+
LogEventLevel restrictedToMinimumLevel = LevelAlias.Minimum)
52+
{
53+
return loggerSinkConfiguration.Sink(new DummyConfigurationSink(appConfiguration, null), restrictedToMinimumLevel);
54+
}
55+
56+
public static LoggerConfiguration DummyWithConfigSection(
57+
this LoggerSinkConfiguration loggerSinkConfiguration,
4358
IConfigurationSection configurationSection,
44-
string pathFormat,
4559
LogEventLevel restrictedToMinimumLevel = LevelAlias.Minimum)
4660
{
47-
return loggerSinkConfiguration.Sink(new DummyRollingFileSink(), restrictedToMinimumLevel);
61+
return loggerSinkConfiguration.Sink(new DummyConfigurationSink(null, configurationSection), restrictedToMinimumLevel);
4862
}
4963

5064
public static LoggerConfiguration DummyRollingFile(

0 commit comments

Comments
 (0)