Merge pull request #422 from serilog/dev

8.0.1 Release

Author: nblumhardt

.gitignore

@@ -201,4 +201,3 @@ FakesAssemblies/
 project.lock.json
 
 artifacts/
-/test/TestApp-*

Build.ps1

@@ -30,6 +30,9 @@ if($LASTEXITCODE -ne 0) { throw 'pack failed' }
 
 Write-Output "build: Testing"
 
-& dotnet test test\Serilog.Settings.Configuration.Tests\Serilog.Settings.Configuration.Tests.csproj
+# Dotnet test doesn't run separate TargetFrameworks in parallel: https://github.com/dotnet/sdk/issues/19147
+# Workaround: use `dotnet test` on dlls directly in order to pass the `--parallel` option to vstest.
+# The _reported_ runtime is wrong but the _actual_ used runtime is correct, see https://github.com/microsoft/vstest/issues/2037#issuecomment-720549173
+& dotnet test test\Serilog.Settings.Configuration.Tests\bin\Release\*\Serilog.Settings.Configuration.Tests.dll --parallel
 
-if($LASTEXITCODE -ne 0) { throw 'unit tests failed' }
+if($LASTEXITCODE -ne 0) { throw 'unit tests failed' }

README.md

@@ -2,7 +2,7 @@
 
 A Serilog settings provider that reads from [Microsoft.Extensions.Configuration](https://docs.microsoft.com/en-us/aspnet/core/fundamentals/configuration/?view=aspnetcore-3.1) sources, including .NET Core's `appsettings.json` file.
 
-By default, configuration is read from the `Serilog` section.
+By default, configuration is read from the `Serilog` section that should be at the **top level** of the configuration file.
 
 ```json
 {
@@ -328,13 +328,13 @@ Destructuring means extracting pieces of information from an object and create p
   {
     "Name": "With",
     "Args": {
-      "policy": "policy": "MySecondNamespace.SecondDestructuringPolicy, MySecondAssembly"
+      "policy": "MySecondNamespace.SecondDestructuringPolicy, MySecondAssembly"
     }
   },
    {
     "Name": "With",
     "Args": {
-      "policy": "policy": "MyThirdNamespace.ThirdDestructuringPolicy, MyThirdAssembly"
+      "policy": "MyThirdNamespace.ThirdDestructuringPolicy, MyThirdAssembly"
     }
   },
 ],

global.json

@@ -1,5 +1,7 @@
 {
   "sdk": {
-    "version": "8.0.100"
+    "version": "8.0.100",
+    "allowPrerelease": false,
+    "rollForward": "latestFeature"
   }
 }

src/Serilog.Settings.Configuration/Serilog.Settings.Configuration.csproj

@@ -3,7 +3,7 @@
   <PropertyGroup>
     <Description>Microsoft.Extensions.Configuration (appsettings.json) support for Serilog.</Description>
     <!-- This must match the major and minor components of the referenced Microsoft.Extensions.Logging package. -->
-    <VersionPrefix>8.0.0</VersionPrefix>
+    <VersionPrefix>8.0.1</VersionPrefix>
     <Authors>Serilog Contributors</Authors>
     <!-- These must match the Dependencies tab in https://www.nuget.org/packages/microsoft.settings.configuration at
         the target version. -->

src/Serilog.Settings.Configuration/Settings/Configuration/ObjectArgumentValue.cs

@@ -1,5 +1,4 @@
 using System.Diagnostics.CodeAnalysis;
-using System.Linq.Expressions;
 using System.Reflection;
 
 using Microsoft.Extensions.Configuration;
@@ -46,13 +45,16 @@ public ObjectArgumentValue(IConfigurationSection section, IReadOnlyCollection<As
         if (toType.IsArray)
             return CreateArray();
 
+        // Only try to call ctor when type is explicitly specified in _section
+        if (TryCallCtorExplicit(_section, resolutionContext, out var ctorResult))
+            return ctorResult;
+
         if (IsContainer(toType, out var elementType) && TryCreateContainer(out var container))
             return container;
 
-        if (TryBuildCtorExpression(_section, toType, resolutionContext, out var ctorExpression))
-        {
-            return Expression.Lambda<Func<object>>(ctorExpression).Compile().Invoke();
-        }
+        // Without a type explicitly specified, attempt to call ctor of toType
+        if (TryCallCtorImplicit(_section, toType, resolutionContext, out ctorResult))
+            return ctorResult;
 
         // MS Config binding can work with a limited set of primitive types and collections
         return _section.Get(toType);
@@ -76,33 +78,41 @@ bool TryCreateContainer([NotNullWhen(true)] out object? result)
         {
             result = null;
 
-            if (toType.GetConstructor(Type.EmptyTypes) == null)
-                return false;
-
-            // https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/object-and-collection-initializers#collection-initializers
-            var addMethod = toType.GetMethods().FirstOrDefault(m => !m.IsStatic && m.Name == "Add" && m.GetParameters().Length == 1 && m.GetParameters()[0].ParameterType == elementType);
-            if (addMethod == null)
-                return false;
+            if (IsConstructableDictionary(toType, elementType, out var concreteType, out var keyType, out var valueType, out var addMethod))
+            {
+                result = Activator.CreateInstance(concreteType) ?? throw new InvalidOperationException($"Activator.CreateInstance returned null for {concreteType}");
 
-            var configurationElements = _section.GetChildren().ToArray();
-            result = Activator.CreateInstance(toType) ?? throw new InvalidOperationException($"Activator.CreateInstance returned null for {toType}");
+                foreach (var section in _section.GetChildren())
+                {
+                    var argumentValue = ConfigurationReader.GetArgumentValue(section, _configurationAssemblies);
+                    var key = new StringArgumentValue(section.Key).ConvertTo(keyType, resolutionContext);
+                    var value = argumentValue.ConvertTo(valueType, resolutionContext);
+                    addMethod.Invoke(result, new[] { key, value });
+                }
+                return true;
+            }
+            else if (IsConstructableContainer(toType, elementType, out concreteType, out addMethod))
+            {
+                result = Activator.CreateInstance(concreteType) ?? throw new InvalidOperationException($"Activator.CreateInstance returned null for {concreteType}");
 
-            for (int i = 0; i < configurationElements.Length; ++i)
+                foreach (var section in _section.GetChildren())
+                {
+                    var argumentValue = ConfigurationReader.GetArgumentValue(section, _configurationAssemblies);
+                    var value = argumentValue.ConvertTo(elementType, resolutionContext);
+                    addMethod.Invoke(result, new[] { value });
+                }
+                return true;
+            }
+            else
             {
-                var argumentValue = ConfigurationReader.GetArgumentValue(configurationElements[i], _configurationAssemblies);
-                var value = argumentValue.ConvertTo(elementType, resolutionContext);
-                addMethod.Invoke(result, new[] { value });
+                return false;
             }
-
-            return true;
         }
     }
 
-    internal static bool TryBuildCtorExpression(
-        IConfigurationSection section, Type parameterType, ResolutionContext resolutionContext, [NotNullWhen(true)] out NewExpression? ctorExpression)
+    bool TryCallCtorExplicit(
+        IConfigurationSection section, ResolutionContext resolutionContext, [NotNullWhen(true)] out object? value)
     {
-        ctorExpression = null;
-
         var typeDirective = section.GetValue<string>("$type") switch
         {
             not null => "$type",
@@ -116,21 +126,39 @@ internal static bool TryBuildCtorExpression(
         var type = typeDirective switch
         {
             not null => Type.GetType(section.GetValue<string>(typeDirective)!, throwOnError: false),
-            null => parameterType,
+            null => null,
         };
 
         if (type is null or { IsAbstract: true })
         {
+            value = null;
             return false;
         }
+        else
+        {
+            var suppliedArguments = section.GetChildren().Where(s => s.Key != typeDirective)
+                .ToDictionary(s => s.Key, StringComparer.OrdinalIgnoreCase);
+            return TryCallCtor(type, suppliedArguments, resolutionContext, out value);
+        }
 
-        var suppliedArguments = section.GetChildren().Where(s => s.Key != typeDirective)
+    }
+
+    bool TryCallCtorImplicit(
+        IConfigurationSection section, Type parameterType, ResolutionContext resolutionContext, out object? value)
+    {
+        var suppliedArguments = section.GetChildren()
             .ToDictionary(s => s.Key, StringComparer.OrdinalIgnoreCase);
+        return TryCallCtor(parameterType, suppliedArguments, resolutionContext, out value);
+    }
+
+    bool TryCallCtor(Type type, Dictionary<string, IConfigurationSection> suppliedArguments, ResolutionContext resolutionContext, [NotNullWhen(true)] out object? value)
+    {
+        value = null;
 
         if (suppliedArguments.Count == 0 &&
             type.GetConstructor(Type.EmptyTypes) is ConstructorInfo parameterlessCtor)
         {
-            ctorExpression = Expression.New(parameterlessCtor);
+            value = parameterlessCtor.Invoke([]);
             return true;
         }
 
@@ -163,76 +191,126 @@ where gr.All(z => z.argumentBindResult.success)
             return false;
         }
 
-        var ctorArguments = new List<Expression>();
-        foreach (var argumentValue in ctor.ArgumentValues)
+        var ctorArguments = new object?[ctor.ArgumentValues.Count];
+        for (var i = 0; i < ctor.ArgumentValues.Count; i++)
         {
-            if (TryBindToCtorArgument(argumentValue.Value, argumentValue.Type, resolutionContext, out var argumentExpression))
+            var argument = ctor.ArgumentValues[i];
+            var valueValue = argument.Value;
+            if (valueValue is IConfigurationSection s)
             {
-                ctorArguments.Add(argumentExpression);
-            }
-            else
-            {
-                return false;
+                var argumentValue = ConfigurationReader.GetArgumentValue(s, _configurationAssemblies);
+                valueValue = argumentValue.ConvertTo(argument.Type, resolutionContext);
             }
+            ctorArguments[i] = valueValue;
         }
 
-        ctorExpression = Expression.New(ctor.ConstructorInfo, ctorArguments);
+        value = ctor.ConstructorInfo.Invoke(ctorArguments);
         return true;
+    }
 
-        static bool TryBindToCtorArgument(object value, Type type, ResolutionContext resolutionContext, [NotNullWhen(true)] out Expression? argumentExpression)
+    static bool IsContainer(Type type, [NotNullWhen(true)] out Type? elementType)
+    {
+        elementType = null;
+        if (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(IEnumerable<>))
         {
-            argumentExpression = null;
-
-            if (value is IConfigurationSection s)
+            elementType = type.GetGenericArguments()[0];
+            return true;
+        }
+        foreach (var iface in type.GetInterfaces())
+        {
+            if (iface.IsGenericType)
             {
-                if (s.Value is string argValue)
+                if (iface.GetGenericTypeDefinition() == typeof(IEnumerable<>))
                 {
-                    var stringArgumentValue = new StringArgumentValue(argValue);
-                    try
-                    {
-                        argumentExpression = Expression.Constant(
-                            stringArgumentValue.ConvertTo(type, resolutionContext),
-                            type);
-
-                        return true;
-                    }
-                    catch (Exception)
-                    {
-                        return false;
-                    }
+                    elementType = iface.GetGenericArguments()[0];
+                    return true;
                 }
-                else if (s.GetChildren().Any())
-                {
-                    if (TryBuildCtorExpression(s, type, resolutionContext, out var ctorExpression))
-                    {
-                        argumentExpression = ctorExpression;
-                        return true;
-                    }
+            }
+        }
 
-                    return false;
+        return false;
+    }
+
+    static bool IsConstructableDictionary(Type type, Type elementType, [NotNullWhen(true)] out Type? concreteType, [NotNullWhen(true)] out Type? keyType, [NotNullWhen(true)] out Type? valueType, [NotNullWhen(true)] out MethodInfo? addMethod)
+    {
+        concreteType = null;
+        keyType = null;
+        valueType = null;
+        addMethod = null;
+        if (!elementType.IsGenericType || elementType.GetGenericTypeDefinition() != typeof(KeyValuePair<,>))
+        {
+            return false;
+        }
+        var argumentTypes = elementType.GetGenericArguments();
+        keyType = argumentTypes[0];
+        valueType = argumentTypes[1];
+        if (type.IsAbstract)
+        {
+            concreteType = typeof(Dictionary<,>).MakeGenericType(argumentTypes);
+            if (!type.IsAssignableFrom(concreteType))
+            {
+                return false;
+            }
+        }
+        else
+        {
+            concreteType = type;
+        }
+        if (concreteType.GetConstructor(Type.EmptyTypes) == null)
+        {
+            return false;
+        }
+        foreach (var method in concreteType.GetMethods())
+        {
+            if (!method.IsStatic && method.Name == "Add")
+            {
+                var parameters = method.GetParameters();
+                if (parameters.Length == 2 && parameters[0].ParameterType == keyType && parameters[1].ParameterType == valueType)
+                {
+                    addMethod = method;
+                    return true;
                 }
             }
-
-            argumentExpression = Expression.Constant(value, type);
-            return true;
         }
+        return false;
     }
 
-    static bool IsContainer(Type type, [NotNullWhen(true)] out Type? elementType)
+    static bool IsConstructableContainer(Type type, Type elementType, [NotNullWhen(true)] out Type? concreteType, [NotNullWhen(true)] out MethodInfo? addMethod)
     {
-        elementType = null;
-        foreach (var iface in type.GetInterfaces())
+        addMethod = null;
+        if (type.IsAbstract)
         {
-            if (iface.IsGenericType)
+            concreteType = typeof(List<>).MakeGenericType(elementType);
+            if (!type.IsAssignableFrom(concreteType))
             {
-                if (iface.GetGenericTypeDefinition() == typeof(IEnumerable<>))
+                concreteType = typeof(HashSet<>).MakeGenericType(elementType);
+                if (!type.IsAssignableFrom(concreteType))
                 {
-                    elementType = iface.GetGenericArguments()[0];
+                    concreteType = null;
+                    return false;
+                }
+            }
+        }
+        else
+        {
+            concreteType = type;
+        }
+        if (concreteType.GetConstructor(Type.EmptyTypes) == null)
+        {
+            return false;
+        }
+        foreach (var method in concreteType.GetMethods())
+        {
+            if (!method.IsStatic && method.Name == "Add")
+            {
+                var parameters = method.GetParameters();
+                if (parameters.Length == 1 && parameters[0].ParameterType == elementType)
+                {
+                    addMethod = method;
                     return true;
                 }
             }
         }
-
         return false;
     }
 }