From dde160df0cefd3713d3bb2e3212c09e65cb4d56b Mon Sep 17 00:00:00 2001 From: Thibaud DESODT Date: Thu, 23 Nov 2017 07:05:30 +0100 Subject: [PATCH] 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; } }