From a20dce5651d3fada10b67e366b7676b8818c8531 Mon Sep 17 00:00:00 2001 From: Thibaud DESODT Date: Fri, 17 Nov 2017 07:26:38 +0100 Subject: [PATCH 1/4] Add support for reference to public static properties in settings as a value for parameters, using the syntax `NameSpace.To.ConcreteType::StaticProperty, AssemblyName` --- .../LoggerSettingsConfiguration.cs | 1 - .../KeyValuePairs/SettingValueConversions.cs | 46 +++++++++ .../Settings/KeyValuePairSettingsTests.cs | 38 ++++++++ .../Settings/SettingValueConversionsTests.cs | 95 +++++++++++++++++++ test/Serilog.Tests/Support/ClassHierarchy.cs | 1 - .../Support/CustomConsoleTheme.cs | 8 ++ .../Support/StaticAccessorClasses.cs | 35 +++++++ test/TestDummies/Console/DummyConsoleSink.cs | 24 +++++ .../Console/Themes/ConcreteConsoleTheme.cs | 6 ++ .../Console/Themes/ConsoleTheme.cs | 7 ++ .../Console/Themes/ConsoleThemes.cs | 7 ++ .../Console/Themes/EmptyConsoleTheme.cs | 6 ++ .../DummyLoggerConfigurationExtensions.cs | 10 ++ 13 files changed, 282 insertions(+), 2 deletions(-) create mode 100644 test/Serilog.Tests/Support/CustomConsoleTheme.cs create mode 100644 test/Serilog.Tests/Support/StaticAccessorClasses.cs create mode 100644 test/TestDummies/Console/DummyConsoleSink.cs create mode 100644 test/TestDummies/Console/Themes/ConcreteConsoleTheme.cs create mode 100644 test/TestDummies/Console/Themes/ConsoleTheme.cs create mode 100644 test/TestDummies/Console/Themes/ConsoleThemes.cs create mode 100644 test/TestDummies/Console/Themes/EmptyConsoleTheme.cs diff --git a/src/Serilog/Configuration/LoggerSettingsConfiguration.cs b/src/Serilog/Configuration/LoggerSettingsConfiguration.cs index 6acbf70ac..fc63f4cfe 100644 --- a/src/Serilog/Configuration/LoggerSettingsConfiguration.cs +++ b/src/Serilog/Configuration/LoggerSettingsConfiguration.cs @@ -14,7 +14,6 @@ using System; using System.Collections.Generic; -using System.Linq; using Serilog.Settings.KeyValuePairs; namespace Serilog.Configuration diff --git a/src/Serilog/Settings/KeyValuePairs/SettingValueConversions.cs b/src/Serilog/Settings/KeyValuePairs/SettingValueConversions.cs index 662fe60d4..f1069af67 100644 --- a/src/Serilog/Settings/KeyValuePairs/SettingValueConversions.cs +++ b/src/Serilog/Settings/KeyValuePairs/SettingValueConversions.cs @@ -16,11 +16,17 @@ using System.Collections.Generic; using System.Linq; using System.Reflection; +using System.Text.RegularExpressions; namespace Serilog.Settings.KeyValuePairs { class SettingValueConversions { + // should match "The.NameSpace.TypeName::MemberName" optionnally followed by + // usual assembly qualifiers like : + // ", MyAssembly, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089" + static Regex StaticMemberAccessorRegex = new Regex("^(?[^:]+)::(?[A-Za-z][A-Za-z0-9]*)(?[^:]*)$"); + static Dictionary> ExtendedTypeConversions = new Dictionary> { { typeof(Uri), s => new Uri(s) }, @@ -29,6 +35,22 @@ class SettingValueConversions public static object ConvertToType(string value, Type toType) { + if (TryParseStaticMemberAccessor(value, out var accessorTypeName, out var memberName)) + { + var accessorType = Type.GetType(accessorTypeName, throwOnError: true); + var publicStaticPropertyInfo = accessorType.GetTypeInfo().DeclaredProperties + .Where(x => x.Name == memberName) + .Where(x => x.GetMethod != null) + .Where(x => x.GetMethod.IsPublic) + .FirstOrDefault(x => x.GetMethod.IsStatic); + + if (publicStaticPropertyInfo == null) + { + throw new InvalidOperationException($"Could not find public static property `{memberName}` on type `{accessorTypeName}`"); + } + return publicStaticPropertyInfo.GetValue(null); // static property, no instance to pass + } + var toTypeInfo = toType.GetTypeInfo(); if (toTypeInfo.IsGenericType && toType.GetGenericTypeDefinition() == typeof(Nullable<>)) { @@ -72,5 +94,29 @@ public static object ConvertToType(string value, Type toType) return Convert.ChangeType(value, toType); } + + internal static bool TryParseStaticMemberAccessor(string input, out string accessorTypeName, out string memberName) + { + if (input == null) + { + accessorTypeName = null; + memberName = null; + return false; + } + if (StaticMemberAccessorRegex.IsMatch(input)) + { + var match = StaticMemberAccessorRegex.Match(input); + var shortAccessorTypeName = match.Groups["shortTypeName"].Value; + var rawMemberName = match.Groups["memberName"].Value; + var extraQualifiers = match.Groups["typeNameExtraQualifiers"].Value; + + memberName = rawMemberName.Trim(); + accessorTypeName = shortAccessorTypeName.Trim() + extraQualifiers.TrimEnd(); + return true; + } + accessorTypeName = null; + memberName = null; + return false; + } } } diff --git a/test/Serilog.Tests/Settings/KeyValuePairSettingsTests.cs b/test/Serilog.Tests/Settings/KeyValuePairSettingsTests.cs index 275f82b82..fcceea943 100644 --- a/test/Serilog.Tests/Settings/KeyValuePairSettingsTests.cs +++ b/test/Serilog.Tests/Settings/KeyValuePairSettingsTests.cs @@ -10,6 +10,8 @@ using Serilog.Configuration; using Serilog.Core; using Serilog.Formatting; +using TestDummies.Console; +using TestDummies.Console.Themes; namespace Serilog.Tests.Settings { @@ -343,5 +345,41 @@ public void LoggingLevelSwitchCanBeUsedForMinimumLevelOverrides() // ReSharper restore HeuristicUnreachableCode } + [Fact] + public void SinksWithAbstractParamsAreConfiguredWithTypeName() + { + var settings = new Dictionary + { + ["using:TestDummies"] = typeof(DummyLoggerConfigurationExtensions).GetTypeInfo().Assembly.FullName, + ["write-to:DummyConsole.theme"] = "Serilog.Tests.Support.CustomConsoleTheme, Serilog.Tests" + }; + + DummyConsoleSink.Theme = null; + + new LoggerConfiguration() + .ReadFrom.KeyValuePairs(settings) + .CreateLogger(); + + Assert.NotNull(DummyConsoleSink.Theme); + Assert.IsType(DummyConsoleSink.Theme); + } + + [Fact] + public void SinksAreConfiguredWithStaticMember() + { + var settings = new Dictionary + { + ["using:TestDummies"] = typeof(DummyLoggerConfigurationExtensions).GetTypeInfo().Assembly.FullName, + ["write-to:DummyConsole.theme"] = "TestDummies.Console.Themes.ConsoleThemes::Theme1, TestDummies" + }; + + DummyConsoleSink.Theme = null; + + new LoggerConfiguration() + .ReadFrom.KeyValuePairs(settings) + .CreateLogger(); + + Assert.Equal(ConsoleThemes.Theme1, DummyConsoleSink.Theme); + } } } diff --git a/test/Serilog.Tests/Settings/SettingValueConversionsTests.cs b/test/Serilog.Tests/Settings/SettingValueConversionsTests.cs index 586ba748a..c03273ff6 100644 --- a/test/Serilog.Tests/Settings/SettingValueConversionsTests.cs +++ b/test/Serilog.Tests/Settings/SettingValueConversionsTests.cs @@ -82,5 +82,100 @@ public void TimeSpanValuesCanBeParsed(string input, int expDays, int expHours, i Assert.IsType(actual); Assert.Equal(expectedTimeSpan, actual); } + + [Theory] + [InlineData("My.NameSpace.Class+InnerClass::Member", + "My.NameSpace.Class+InnerClass", "Member")] + [InlineData(" TrimMe.NameSpace.Class::NeedsTrimming ", + "TrimMe.NameSpace.Class", "NeedsTrimming")] + [InlineData("My.NameSpace.Class::Member", + "My.NameSpace.Class", "Member")] + [InlineData("My.NameSpace.Class::Member, MyAssembly", + "My.NameSpace.Class, MyAssembly", "Member")] + [InlineData("My.NameSpace.Class::Member, MyAssembly, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089", + "My.NameSpace.Class, MyAssembly, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089", "Member")] + [InlineData("Just a random string with :: in it", + null, null)] + [InlineData("Its::a::trapWithColonsAppearingTwice", + null, null)] + [InlineData("ThereIsNoMemberHere::", + null, null)] + [InlineData(null, + null, null)] + [InlineData(" " , + null, null)] + public void TryParseStaticMemberAccessorReturnsExpectedResults(string input, string expectedAccessorType, string expectedPropertyName) + { + var actual = SettingValueConversions.TryParseStaticMemberAccessor(input, + out var actualAccessorType, + out var actualMemberName); + + if (expectedAccessorType == null) + { + Assert.False(actual, $"Should not parse {input}"); + } + else + { + Assert.True(actual, $"should successfully parse {input}"); + Assert.Equal(expectedAccessorType, actualAccessorType); + Assert.Equal(expectedPropertyName, actualMemberName); + } + } + + [Theory] + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::StringProperty, Serilog.Tests", typeof(string), "StringProperty")] + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::IntProperty, Serilog.Tests", typeof(int), 42)] + public void StaticMembersAccessorsCanBeUsedForSImpleTypes(string input, Type targetType, object expectedValue) + { + var actual = SettingValueConversions.ConvertToType(input, targetType); + + Assert.IsAssignableFrom(targetType, actual); + Assert.Equal(expectedValue, actual); + } + + [Theory] + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::InterfaceProperty, Serilog.Tests", typeof(IAmAnInterface))] + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::AbstractProperty, Serilog.Tests", typeof(AnAbstractClass))] + public void StaticMembersAccessorsCanBeUsedForReferenceTypes(string input, Type targetType) + { + var actual = SettingValueConversions.ConvertToType(input, targetType); + + Assert.IsAssignableFrom(targetType, actual); + Assert.Equal(ConcreteImpl.Instance, actual); + } + + [Theory] + // unknown type + [InlineData("Namespace.ThisIsNotAKnownType::StringProperty, Serilog.Tests", typeof(string))] + // good type name, but wrong namespace + [InlineData("Random.Namespace.ClassWithStaticAccessors::StringProperty, Serilog.Tests", typeof(string))] + // good full type name, but missing or wrong assembly + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::StringProperty", typeof(string))] + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::StringProperty, TestDummies", typeof(string))] + public void StaticAccessorOnUnknownTypeThrowsTypeLoadException(string input, Type targetType) + { + Assert.Throws(() => + SettingValueConversions.ConvertToType(input, targetType) + ); + } + + [Theory] + // unknown member + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::UnknownMember, Serilog.Tests", typeof(string))] + // static property exists but it's private + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::PrivateStringProperty, Serilog.Tests", typeof(string))] + // public member exists but it's a field + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::StringField, Serilog.Tests", typeof(string))] + // public property exists but it's not static + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::InstanceStringProperty, Serilog.Tests", typeof(string))] + public void StaticAccessorWithInvalidMemberThrowsInvalidOperationException(string input, Type targetType) + { + var exception = Assert.Throws(() => + SettingValueConversions.ConvertToType(input, targetType) + ); + + Assert.Contains("Could not find public static property ", exception.Message); + Assert.Contains("on type `Serilog.Tests.Support.ClassWithStaticAccessors, Serilog.Tests`", exception.Message); + } } } diff --git a/test/Serilog.Tests/Support/ClassHierarchy.cs b/test/Serilog.Tests/Support/ClassHierarchy.cs index 554fcf94a..597317bb3 100644 --- a/test/Serilog.Tests/Support/ClassHierarchy.cs +++ b/test/Serilog.Tests/Support/ClassHierarchy.cs @@ -1,6 +1,5 @@ namespace Serilog.Tests.Support { - public abstract class DummyAbstractClass { } diff --git a/test/Serilog.Tests/Support/CustomConsoleTheme.cs b/test/Serilog.Tests/Support/CustomConsoleTheme.cs new file mode 100644 index 000000000..345425b9d --- /dev/null +++ b/test/Serilog.Tests/Support/CustomConsoleTheme.cs @@ -0,0 +1,8 @@ +using TestDummies.Console.Themes; + +namespace Serilog.Tests.Support +{ + class CustomConsoleTheme : ConsoleTheme + { + } +} diff --git a/test/Serilog.Tests/Support/StaticAccessorClasses.cs b/test/Serilog.Tests/Support/StaticAccessorClasses.cs new file mode 100644 index 000000000..e5f788c40 --- /dev/null +++ b/test/Serilog.Tests/Support/StaticAccessorClasses.cs @@ -0,0 +1,35 @@ + +namespace Serilog.Tests.Support +{ + public interface IAmAnInterface + { + + } + + public abstract class AnAbstractClass + { + + } + + class ConcreteImpl : AnAbstractClass, IAmAnInterface + { + ConcreteImpl() + { + + } + + public static ConcreteImpl Instance { get; } = new ConcreteImpl(); + } + + public class ClassWithStaticAccessors + { + public static string StringProperty => nameof(StringProperty); + public static int IntProperty => 42; + public static IAmAnInterface InterfaceProperty => ConcreteImpl.Instance; + public static AnAbstractClass AbstractProperty => ConcreteImpl.Instance; + // ReSharper disable once UnusedMember.Local + static string PrivateStringProperty => nameof(PrivateStringProperty); + public string InstanceStringProperty => nameof(InstanceStringProperty); + public static string StringField = nameof(StringField); + } +} diff --git a/test/TestDummies/Console/DummyConsoleSink.cs b/test/TestDummies/Console/DummyConsoleSink.cs new file mode 100644 index 000000000..571e73fc8 --- /dev/null +++ b/test/TestDummies/Console/DummyConsoleSink.cs @@ -0,0 +1,24 @@ +using System; +using Serilog.Core; +using Serilog.Events; +using TestDummies.Console.Themes; + +namespace TestDummies.Console +{ + public class DummyConsoleSink : ILogEventSink + { + public DummyConsoleSink(ConsoleTheme theme = null) + { + Theme = theme ?? ConsoleTheme.None; + } + + [ThreadStatic] + public static ConsoleTheme Theme; + + public void Emit(LogEvent logEvent) + { + } + } + +} + diff --git a/test/TestDummies/Console/Themes/ConcreteConsoleTheme.cs b/test/TestDummies/Console/Themes/ConcreteConsoleTheme.cs new file mode 100644 index 000000000..8b3b041b2 --- /dev/null +++ b/test/TestDummies/Console/Themes/ConcreteConsoleTheme.cs @@ -0,0 +1,6 @@ +namespace TestDummies.Console.Themes +{ + class ConcreteConsoleTheme : ConsoleTheme + { + } +} diff --git a/test/TestDummies/Console/Themes/ConsoleTheme.cs b/test/TestDummies/Console/Themes/ConsoleTheme.cs new file mode 100644 index 000000000..1c5aaf5da --- /dev/null +++ b/test/TestDummies/Console/Themes/ConsoleTheme.cs @@ -0,0 +1,7 @@ +namespace TestDummies.Console.Themes +{ + public abstract class ConsoleTheme + { + public static ConsoleTheme None { get; } = new EmptyConsoleTheme(); + } +} diff --git a/test/TestDummies/Console/Themes/ConsoleThemes.cs b/test/TestDummies/Console/Themes/ConsoleThemes.cs new file mode 100644 index 000000000..7bb414cb8 --- /dev/null +++ b/test/TestDummies/Console/Themes/ConsoleThemes.cs @@ -0,0 +1,7 @@ +namespace TestDummies.Console.Themes +{ + public static class ConsoleThemes + { + public static ConsoleTheme Theme1 { get; } = new ConcreteConsoleTheme(); + } +} diff --git a/test/TestDummies/Console/Themes/EmptyConsoleTheme.cs b/test/TestDummies/Console/Themes/EmptyConsoleTheme.cs new file mode 100644 index 000000000..100e89e87 --- /dev/null +++ b/test/TestDummies/Console/Themes/EmptyConsoleTheme.cs @@ -0,0 +1,6 @@ +namespace TestDummies.Console.Themes +{ + class EmptyConsoleTheme : ConsoleTheme + { + } +} diff --git a/test/TestDummies/DummyLoggerConfigurationExtensions.cs b/test/TestDummies/DummyLoggerConfigurationExtensions.cs index 2fb795c0c..7c728cc94 100644 --- a/test/TestDummies/DummyLoggerConfigurationExtensions.cs +++ b/test/TestDummies/DummyLoggerConfigurationExtensions.cs @@ -4,6 +4,8 @@ using Serilog.Formatting; using Serilog.Configuration; using Serilog.Core; +using TestDummies.Console; +using TestDummies.Console.Themes; namespace TestDummies { @@ -51,6 +53,14 @@ public static LoggerConfiguration DummyWithLevelSwitch( return loggerSinkConfiguration.Sink(new DummyWithLevelSwitchSink(controlLevelSwitch), restrictedToMinimumLevel); } + public static LoggerConfiguration DummyConsole( + this LoggerSinkConfiguration loggerSinkConfiguration, + LogEventLevel restrictedToMinimumLevel = LevelAlias.Minimum, + ConsoleTheme theme = null) + { + return loggerSinkConfiguration.Sink(new DummyConsoleSink(theme), restrictedToMinimumLevel); + } + public static LoggerConfiguration Dummy( this LoggerSinkConfiguration loggerSinkConfiguration, Action wrappedSinkAction) From e81439ae1b5661cad1b099c361d8230a7bda56e9 Mon Sep 17 00:00:00 2001 From: Thibaud DESODT Date: Tue, 21 Nov 2017 17:51:04 +0100 Subject: [PATCH 2/4] Extend the class::static handling to also support public static fields --- .../KeyValuePairs/SettingValueConversions.cs | 18 +++++++++++++++--- .../Settings/KeyValuePairSettingsTests.cs | 2 +- .../Settings/SettingValueConversionsTests.cs | 12 +++++++++--- .../Support/StaticAccessorClasses.cs | 12 +++++++++++- 4 files changed, 36 insertions(+), 8 deletions(-) diff --git a/src/Serilog/Settings/KeyValuePairs/SettingValueConversions.cs b/src/Serilog/Settings/KeyValuePairs/SettingValueConversions.cs index f1069af67..230f1943c 100644 --- a/src/Serilog/Settings/KeyValuePairs/SettingValueConversions.cs +++ b/src/Serilog/Settings/KeyValuePairs/SettingValueConversions.cs @@ -44,11 +44,23 @@ public static object ConvertToType(string value, Type toType) .Where(x => x.GetMethod.IsPublic) .FirstOrDefault(x => x.GetMethod.IsStatic); - if (publicStaticPropertyInfo == null) + if (publicStaticPropertyInfo != null) { - throw new InvalidOperationException($"Could not find public static property `{memberName}` on type `{accessorTypeName}`"); + return publicStaticPropertyInfo.GetValue(null); // static property, no instance to pass } - return publicStaticPropertyInfo.GetValue(null); // static property, no instance to pass + + // no property ? look for a field + var publicStaticFieldInfo = accessorType.GetTypeInfo().DeclaredFields + .Where(x => x.Name == memberName) + .Where(x => x.IsPublic) + .FirstOrDefault(x => x.IsStatic); + + if (publicStaticFieldInfo != null) + { + return publicStaticFieldInfo.GetValue(null); // static field, no instance to pass + } + + throw new InvalidOperationException($"Could not find a public static property or field with name `{memberName}` on type `{accessorTypeName}`"); } var toTypeInfo = toType.GetTypeInfo(); diff --git a/test/Serilog.Tests/Settings/KeyValuePairSettingsTests.cs b/test/Serilog.Tests/Settings/KeyValuePairSettingsTests.cs index fcceea943..52502daa9 100644 --- a/test/Serilog.Tests/Settings/KeyValuePairSettingsTests.cs +++ b/test/Serilog.Tests/Settings/KeyValuePairSettingsTests.cs @@ -325,7 +325,7 @@ public void LoggingLevelSwitchCanBeUsedForMinimumLevelOverrides() 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"); + Assert.False(evt is null, "Minimum level is Debug. It should log Information messages"); evt = null; // ReSharper disable HeuristicUnreachableCode diff --git a/test/Serilog.Tests/Settings/SettingValueConversionsTests.cs b/test/Serilog.Tests/Settings/SettingValueConversionsTests.cs index c03273ff6..c0a33a474 100644 --- a/test/Serilog.Tests/Settings/SettingValueConversionsTests.cs +++ b/test/Serilog.Tests/Settings/SettingValueConversionsTests.cs @@ -125,6 +125,8 @@ public void TryParseStaticMemberAccessorReturnsExpectedResults(string input, str [Theory] [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::StringProperty, Serilog.Tests", typeof(string), "StringProperty")] [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::IntProperty, Serilog.Tests", typeof(int), 42)] + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::StringField, Serilog.Tests", typeof(string), "StringField")] + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::IntField, Serilog.Tests", typeof(int), 666)] public void StaticMembersAccessorsCanBeUsedForSImpleTypes(string input, Type targetType, object expectedValue) { var actual = SettingValueConversions.ConvertToType(input, targetType); @@ -136,6 +138,8 @@ public void StaticMembersAccessorsCanBeUsedForSImpleTypes(string input, Type tar [Theory] [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::InterfaceProperty, Serilog.Tests", typeof(IAmAnInterface))] [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::AbstractProperty, Serilog.Tests", typeof(AnAbstractClass))] + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::InterfaceField, Serilog.Tests", typeof(IAmAnInterface))] + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::AbstractField, Serilog.Tests", typeof(AnAbstractClass))] public void StaticMembersAccessorsCanBeUsedForReferenceTypes(string input, Type targetType) { var actual = SettingValueConversions.ConvertToType(input, targetType); @@ -164,17 +168,19 @@ public void StaticAccessorOnUnknownTypeThrowsTypeLoadException(string input, Typ [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::UnknownMember, Serilog.Tests", typeof(string))] // static property exists but it's private [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::PrivateStringProperty, Serilog.Tests", typeof(string))] - // public member exists but it's a field - [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::StringField, Serilog.Tests", typeof(string))] + // static field exists but it's private + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::PrivateStringField, Serilog.Tests", typeof(string))] // public property exists but it's not static [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::InstanceStringProperty, Serilog.Tests", typeof(string))] + // public field exists but it's not static + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::InstanceStringField, Serilog.Tests", typeof(string))] public void StaticAccessorWithInvalidMemberThrowsInvalidOperationException(string input, Type targetType) { var exception = Assert.Throws(() => SettingValueConversions.ConvertToType(input, targetType) ); - Assert.Contains("Could not find public static property ", exception.Message); + Assert.Contains("Could not find a public static property or field ", exception.Message); Assert.Contains("on type `Serilog.Tests.Support.ClassWithStaticAccessors, Serilog.Tests`", exception.Message); } } diff --git a/test/Serilog.Tests/Support/StaticAccessorClasses.cs b/test/Serilog.Tests/Support/StaticAccessorClasses.cs index e5f788c40..057886dca 100644 --- a/test/Serilog.Tests/Support/StaticAccessorClasses.cs +++ b/test/Serilog.Tests/Support/StaticAccessorClasses.cs @@ -27,9 +27,19 @@ public class ClassWithStaticAccessors public static int IntProperty => 42; public static IAmAnInterface InterfaceProperty => ConcreteImpl.Instance; public static AnAbstractClass AbstractProperty => ConcreteImpl.Instance; + + public static string StringField = nameof(StringField); + public static int IntField = 666; + public static IAmAnInterface InterfaceField = ConcreteImpl.Instance; + public static AnAbstractClass AbstractField = ConcreteImpl.Instance; + // ReSharper disable once UnusedMember.Local static string PrivateStringProperty => nameof(PrivateStringProperty); +#pragma warning disable 169 + static string PrivateStringField = nameof(PrivateStringField); +#pragma warning restore 169 public string InstanceStringProperty => nameof(InstanceStringProperty); - public static string StringField = nameof(StringField); + public string InstanceStringField = nameof(InstanceStringField); + } } From dde160df0cefd3713d3bb2e3212c09e65cb4d56b Mon Sep 17 00:00:00 2001 From: Thibaud DESODT Date: Thu, 23 Nov 2017 07:05:30 +0100 Subject: [PATCH 3/4] Only support static member accessors for interfaces and abstract types --- .../KeyValuePairs/SettingValueConversions.cs | 61 ++++++++++--------- .../Settings/SettingValueConversionsTests.cs | 38 +++++------- .../Support/StaticAccessorClasses.cs | 12 ++-- 3 files changed, 51 insertions(+), 60 deletions(-) diff --git a/src/Serilog/Settings/KeyValuePairs/SettingValueConversions.cs b/src/Serilog/Settings/KeyValuePairs/SettingValueConversions.cs index 230f1943c..b6a5300ca 100644 --- a/src/Serilog/Settings/KeyValuePairs/SettingValueConversions.cs +++ b/src/Serilog/Settings/KeyValuePairs/SettingValueConversions.cs @@ -35,34 +35,6 @@ class SettingValueConversions public static object ConvertToType(string value, Type toType) { - if (TryParseStaticMemberAccessor(value, out var accessorTypeName, out var memberName)) - { - var accessorType = Type.GetType(accessorTypeName, throwOnError: true); - var publicStaticPropertyInfo = accessorType.GetTypeInfo().DeclaredProperties - .Where(x => x.Name == memberName) - .Where(x => x.GetMethod != null) - .Where(x => x.GetMethod.IsPublic) - .FirstOrDefault(x => x.GetMethod.IsStatic); - - if (publicStaticPropertyInfo != null) - { - return publicStaticPropertyInfo.GetValue(null); // static property, no instance to pass - } - - // no property ? look for a field - var publicStaticFieldInfo = accessorType.GetTypeInfo().DeclaredFields - .Where(x => x.Name == memberName) - .Where(x => x.IsPublic) - .FirstOrDefault(x => x.IsStatic); - - if (publicStaticFieldInfo != null) - { - return publicStaticFieldInfo.GetValue(null); // static field, no instance to pass - } - - throw new InvalidOperationException($"Could not find a public static property or field with name `{memberName}` on type `{accessorTypeName}`"); - } - var toTypeInfo = toType.GetTypeInfo(); if (toTypeInfo.IsGenericType && toType.GetGenericTypeDefinition() == typeof(Nullable<>)) { @@ -87,6 +59,39 @@ public static object ConvertToType(string value, Type toType) if ((toTypeInfo.IsInterface || toTypeInfo.IsAbstract) && !string.IsNullOrWhiteSpace(value)) { + // check if value looks like a static property or field directive + // like "Namespace.TypeName::StaticProperty, AssemblyName" + if (TryParseStaticMemberAccessor(value, out var accessorTypeName, out var memberName)) + { + var accessorType = Type.GetType(accessorTypeName, throwOnError: true); + // is there a public static property with that name ? + var publicStaticPropertyInfo = accessorType.GetTypeInfo().DeclaredProperties + .Where(x => x.Name == memberName) + .Where(x => x.GetMethod != null) + .Where(x => x.GetMethod.IsPublic) + .FirstOrDefault(x => x.GetMethod.IsStatic); + + if (publicStaticPropertyInfo != null) + { + return publicStaticPropertyInfo.GetValue(null); // static property, no instance to pass + } + + // no property ? look for a public static field + var publicStaticFieldInfo = accessorType.GetTypeInfo().DeclaredFields + .Where(x => x.Name == memberName) + .Where(x => x.IsPublic) + .FirstOrDefault(x => x.IsStatic); + + if (publicStaticFieldInfo != null) + { + return publicStaticFieldInfo.GetValue(null); // static field, no instance to pass + } + + throw new InvalidOperationException($"Could not find a public static property or field with name `{memberName}` on type `{accessorTypeName}`"); + } + + // maybe it's the assembly-qualified type name of a concrete implementation + // with a default constructor var type = Type.GetType(value.Trim(), throwOnError: false); if (type != null) { diff --git a/test/Serilog.Tests/Settings/SettingValueConversionsTests.cs b/test/Serilog.Tests/Settings/SettingValueConversionsTests.cs index c0a33a474..89e3c66e7 100644 --- a/test/Serilog.Tests/Settings/SettingValueConversionsTests.cs +++ b/test/Serilog.Tests/Settings/SettingValueConversionsTests.cs @@ -99,10 +99,13 @@ public void TimeSpanValuesCanBeParsed(string input, int expDays, int expHours, i [InlineData("Its::a::trapWithColonsAppearingTwice", null, null)] [InlineData("ThereIsNoMemberHere::", - null, null)] + null, null)] [InlineData(null, - null, null)] + null, null)] [InlineData(" " , + null, null)] + // a full-qualified type name should not be considered a static member accessor + [InlineData("My.NameSpace.Class, MyAssembly, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089", null, null)] public void TryParseStaticMemberAccessorReturnsExpectedResults(string input, string expectedAccessorType, string expectedPropertyName) { @@ -122,19 +125,6 @@ public void TryParseStaticMemberAccessorReturnsExpectedResults(string input, str } } - [Theory] - [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::StringProperty, Serilog.Tests", typeof(string), "StringProperty")] - [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::IntProperty, Serilog.Tests", typeof(int), 42)] - [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::StringField, Serilog.Tests", typeof(string), "StringField")] - [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::IntField, Serilog.Tests", typeof(int), 666)] - public void StaticMembersAccessorsCanBeUsedForSImpleTypes(string input, Type targetType, object expectedValue) - { - var actual = SettingValueConversions.ConvertToType(input, targetType); - - Assert.IsAssignableFrom(targetType, actual); - Assert.Equal(expectedValue, actual); - } - [Theory] [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::InterfaceProperty, Serilog.Tests", typeof(IAmAnInterface))] [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::AbstractProperty, Serilog.Tests", typeof(AnAbstractClass))] @@ -150,12 +140,12 @@ public void StaticMembersAccessorsCanBeUsedForReferenceTypes(string input, Type [Theory] // unknown type - [InlineData("Namespace.ThisIsNotAKnownType::StringProperty, Serilog.Tests", typeof(string))] + [InlineData("Namespace.ThisIsNotAKnownType::InterfaceProperty, Serilog.Tests", typeof(IAmAnInterface))] // good type name, but wrong namespace - [InlineData("Random.Namespace.ClassWithStaticAccessors::StringProperty, Serilog.Tests", typeof(string))] + [InlineData("Random.Namespace.ClassWithStaticAccessors::InterfaceProperty, Serilog.Tests", typeof(IAmAnInterface))] // good full type name, but missing or wrong assembly - [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::StringProperty", typeof(string))] - [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::StringProperty, TestDummies", typeof(string))] + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::InterfaceProperty", typeof(IAmAnInterface))] + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::InterfaceProperty, TestDummies", typeof(IAmAnInterface))] public void StaticAccessorOnUnknownTypeThrowsTypeLoadException(string input, Type targetType) { Assert.Throws(() => @@ -165,15 +155,15 @@ public void StaticAccessorOnUnknownTypeThrowsTypeLoadException(string input, Typ [Theory] // unknown member - [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::UnknownMember, Serilog.Tests", typeof(string))] + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::UnknownMember, Serilog.Tests", typeof(IAmAnInterface))] // static property exists but it's private - [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::PrivateStringProperty, Serilog.Tests", typeof(string))] + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::PrivateInterfaceProperty, Serilog.Tests", typeof(IAmAnInterface))] // static field exists but it's private - [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::PrivateStringField, Serilog.Tests", typeof(string))] + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::PrivateInterfaceField, Serilog.Tests", typeof(IAmAnInterface))] // public property exists but it's not static - [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::InstanceStringProperty, Serilog.Tests", typeof(string))] + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::InstanceInterfaceProperty, Serilog.Tests", typeof(IAmAnInterface))] // public field exists but it's not static - [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::InstanceStringField, Serilog.Tests", typeof(string))] + [InlineData("Serilog.Tests.Support.ClassWithStaticAccessors::InstanceInterfaceField, Serilog.Tests", typeof(IAmAnInterface))] public void StaticAccessorWithInvalidMemberThrowsInvalidOperationException(string input, Type targetType) { var exception = Assert.Throws(() => diff --git a/test/Serilog.Tests/Support/StaticAccessorClasses.cs b/test/Serilog.Tests/Support/StaticAccessorClasses.cs index 057886dca..26b98d06e 100644 --- a/test/Serilog.Tests/Support/StaticAccessorClasses.cs +++ b/test/Serilog.Tests/Support/StaticAccessorClasses.cs @@ -23,23 +23,19 @@ class ConcreteImpl : AnAbstractClass, IAmAnInterface public class ClassWithStaticAccessors { - public static string StringProperty => nameof(StringProperty); - public static int IntProperty => 42; public static IAmAnInterface InterfaceProperty => ConcreteImpl.Instance; public static AnAbstractClass AbstractProperty => ConcreteImpl.Instance; - public static string StringField = nameof(StringField); - public static int IntField = 666; public static IAmAnInterface InterfaceField = ConcreteImpl.Instance; public static AnAbstractClass AbstractField = ConcreteImpl.Instance; // ReSharper disable once UnusedMember.Local - static string PrivateStringProperty => nameof(PrivateStringProperty); + static IAmAnInterface PrivateInterfaceProperty => ConcreteImpl.Instance; #pragma warning disable 169 - static string PrivateStringField = nameof(PrivateStringField); + static IAmAnInterface PrivateInterfaceField = ConcreteImpl.Instance; #pragma warning restore 169 - public string InstanceStringProperty => nameof(InstanceStringProperty); - public string InstanceStringField = nameof(InstanceStringField); + public IAmAnInterface InstanceInterfaceProperty => ConcreteImpl.Instance; + public IAmAnInterface InstanceInterfaceField = ConcreteImpl.Instance; } } From e91e377753bdd873b82defc2879d25ab4f8ad448 Mon Sep 17 00:00:00 2001 From: Thibaud DESODT Date: Thu, 23 Nov 2017 07:14:12 +0100 Subject: [PATCH 4/4] Fix wrong class hierarchy in Abstract class test cases --- test/Serilog.Tests/Support/ClassHierarchy.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Serilog.Tests/Support/ClassHierarchy.cs b/test/Serilog.Tests/Support/ClassHierarchy.cs index 597317bb3..34dae7f00 100644 --- a/test/Serilog.Tests/Support/ClassHierarchy.cs +++ b/test/Serilog.Tests/Support/ClassHierarchy.cs @@ -4,7 +4,7 @@ public abstract class DummyAbstractClass { } - public class DummyConcreteClassWithDefaultConstructor + public class DummyConcreteClassWithDefaultConstructor : DummyAbstractClass { // ReSharper disable once UnusedParameter.Local public DummyConcreteClassWithDefaultConstructor(string param = "") @@ -12,7 +12,7 @@ public DummyConcreteClassWithDefaultConstructor(string param = "") } } - public class DummyConcreteClassWithoutDefaultConstructor + public class DummyConcreteClassWithoutDefaultConstructor : DummyAbstractClass { // ReSharper disable once UnusedParameter.Local public DummyConcreteClassWithoutDefaultConstructor(string param)