From 583731dc4cd13efca7503dddee85bf2b00a1b97b Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Mon, 13 Feb 2023 18:35:45 -0600 Subject: [PATCH 1/3] Remove ConfigurationBinder usage from Console Logging This allows ConfigurationBinder, and its dependencies like TypeConverter, to be trimmed in an application that uses Console Logging, like an ASP.NET API application. Fix #81931 --- .../src/ConsoleFormatterConfigureOptions.cs | 49 ++++++++++ .../src/ConsoleLoggerConfigureOptions.cs | 96 +++++++++++++++++++ .../src/ConsoleLoggerExtensions.cs | 47 +++++---- .../JsonConsoleFormatterConfigureOptions.cs | 58 +++++++++++ .../SimpleConsoleFormatterConfigureOptions.cs | 44 +++++++++ .../ConsoleLoggerExtensionsTests.cs | 3 +- .../ConsoleLoggerTest.cs | 17 ++++ 7 files changed, 293 insertions(+), 21 deletions(-) create mode 100644 src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleFormatterConfigureOptions.cs create mode 100644 src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerConfigureOptions.cs create mode 100644 src/libraries/Microsoft.Extensions.Logging.Console/src/JsonConsoleFormatterConfigureOptions.cs create mode 100644 src/libraries/Microsoft.Extensions.Logging.Console/src/SimpleConsoleFormatterConfigureOptions.cs diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleFormatterConfigureOptions.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleFormatterConfigureOptions.cs new file mode 100644 index 0000000000000..9f0a796070863 --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleFormatterConfigureOptions.cs @@ -0,0 +1,49 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.Versioning; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging.Configuration; +using Microsoft.Extensions.Logging.Console; +using Microsoft.Extensions.Options; + +namespace Microsoft.Extensions.Logging +{ + /// + /// Configures a ConsoleFormatterOptions object from an IConfiguration. + /// + /// + /// Doesn't use ConfigurationBinder in order to allow ConfigurationBinder, and all its dependencies, + /// to be trimmed. This improves app size and startup. + /// + [UnsupportedOSPlatform("browser")] + internal sealed class ConsoleFormatterConfigureOptions : IConfigureOptions + { + private readonly IConfiguration _configuration; + + public ConsoleFormatterConfigureOptions(ILoggerProviderConfiguration providerConfiguration) + { + _configuration = providerConfiguration.GetFormatterOptionsSection(); + } + + public void Configure(ConsoleFormatterOptions options) => Bind(_configuration, options); + + public static void Bind(IConfiguration configuration, ConsoleFormatterOptions options) + { + if (configuration["IncludeScopes"] is string includeScopes) + { + options.IncludeScopes = bool.Parse(includeScopes); + } + + if (configuration["TimestampFormat"] is string timestampFormat) + { + options.TimestampFormat = timestampFormat; + } + + if (configuration["UseUtcTimestamp"] is string useUtcTimestamp) + { + options.UseUtcTimestamp = bool.Parse(useUtcTimestamp); + } + } + } +} diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerConfigureOptions.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerConfigureOptions.cs new file mode 100644 index 0000000000000..9399e2aadd7a9 --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerConfigureOptions.cs @@ -0,0 +1,96 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Globalization; +using System.Runtime.Versioning; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging.Configuration; +using Microsoft.Extensions.Logging.Console; +using Microsoft.Extensions.Options; + +namespace Microsoft.Extensions.Logging +{ + /// + /// Configures a ConsoleLoggerOptions object from an IConfiguration. + /// + /// + /// Doesn't use ConfigurationBinder in order to allow ConfigurationBinder, and all its dependencies, + /// to be trimmed. This improves app size and startup. + /// + [UnsupportedOSPlatform("browser")] + internal sealed class ConsoleLoggerConfigureOptions : IConfigureOptions + { + private readonly IConfiguration _configuration; + + public ConsoleLoggerConfigureOptions(ILoggerProviderConfiguration providerConfiguration) + { + _configuration = providerConfiguration.Configuration; + } + + public void Configure(ConsoleLoggerOptions options) + { + if (_configuration["DisableColors"] is string disableColors) + { +#pragma warning disable CS0618 // Type or member is obsolete + options.DisableColors = bool.Parse(disableColors); +#pragma warning restore CS0618 // Type or member is obsolete + } + + if (_configuration["Format"] is string format) + { +#pragma warning disable CS0618 // Type or member is obsolete + options.Format = ParseEnum(format); +#pragma warning restore CS0618 // Type or member is obsolete + } + + if (_configuration["FormatterName"] is string formatterName) + { + options.FormatterName = formatterName; + } + + if (_configuration["IncludeScopes"] is string includeScopes) + { +#pragma warning disable CS0618 // Type or member is obsolete + options.IncludeScopes = bool.Parse(includeScopes); +#pragma warning restore CS0618 // Type or member is obsolete + } + + if (_configuration["LogToStandardErrorThreshold"] is string logToStandardErrorThreshold) + { + options.LogToStandardErrorThreshold = ParseEnum(logToStandardErrorThreshold); + } + + if (_configuration["MaxQueueLength"] is string maxQueueLength) + { + options.MaxQueueLength = int.Parse(maxQueueLength, NumberStyles.Integer, NumberFormatInfo.InvariantInfo); + } + + if (_configuration["QueueFullMode"] is string queueFullMode) + { + options.QueueFullMode = ParseEnum(queueFullMode); + } + + if (_configuration["TimestampFormat"] is string timestampFormat) + { +#pragma warning disable CS0618 // Type or member is obsolete + options.TimestampFormat = timestampFormat; +#pragma warning restore CS0618 // Type or member is obsolete + } + + if (_configuration["UseUtcTimestamp"] is string useUtcTimestamp) + { +#pragma warning disable CS0618 // Type or member is obsolete + options.UseUtcTimestamp = bool.Parse(useUtcTimestamp); +#pragma warning restore CS0618 // Type or member is obsolete + } + } + + public static T ParseEnum(string value) where T : struct => +#if NETSTANDARD || NETFRAMEWORK + (T)Enum.Parse(typeof(T), value, ignoreCase: true); +#else + Enum.Parse(value, ignoreCase: true); +#endif + } +} diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerExtensions.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerExtensions.cs index 160e099097f8c..da4ff6bf0c991 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerExtensions.cs @@ -25,22 +25,18 @@ public static class ConsoleLoggerExtensions /// Adds a console logger named 'Console' to the factory. /// /// The to use. - [UnconditionalSuppressMessage("AotAnalysis", "IL3050:RequiresDynamicCode", - Justification = "AddConsoleFormatter and RegisterProviderOptions are only called with Options types which only have simple properties.")] - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", - Justification = "AddConsoleFormatter and RegisterProviderOptions are only dangerous when the Options type cannot be statically analyzed, but that is not the case here. " + - "The DynamicallyAccessedMembers annotations on them will make sure to preserve the right members from the different options objects.")] - [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(JsonWriterOptions))] public static ILoggingBuilder AddConsole(this ILoggingBuilder builder) { builder.AddConfiguration(); - builder.AddConsoleFormatter(); - builder.AddConsoleFormatter(); - builder.AddConsoleFormatter(); + builder.AddConsoleFormatter(); + builder.AddConsoleFormatter(); + builder.AddConsoleFormatter(); builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); - LoggerProviderOptions.RegisterProviderOptions(builder.Services); + + builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton, ConsoleLoggerConfigureOptions>()); + builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton, LoggerProviderOptionsChangeTokenSource>()); return builder; } @@ -135,13 +131,7 @@ private static ILoggingBuilder AddFormatterWithName(this ILoggingBuilder builder where TOptions : ConsoleFormatterOptions where TFormatter : ConsoleFormatter { - builder.AddConfiguration(); - - builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); - builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton, ConsoleLoggerFormatterConfigureOptions>()); - builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton, ConsoleLoggerFormatterOptionsChangeTokenSource>()); - - return builder; + return AddConsoleFormatter>(builder); } /// @@ -161,6 +151,25 @@ private static ILoggingBuilder AddFormatterWithName(this ILoggingBuilder builder builder.Services.Configure(configure); return builder; } + + private static ILoggingBuilder AddConsoleFormatter<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] TFormatter, TOptions, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] TConfigureOptions>(this ILoggingBuilder builder) + where TOptions : ConsoleFormatterOptions + where TFormatter : ConsoleFormatter + where TConfigureOptions : class, IConfigureOptions + { + builder.AddConfiguration(); + + builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); + builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton, TConfigureOptions>()); + builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton, ConsoleLoggerFormatterOptionsChangeTokenSource>()); + + return builder; + } + + internal static IConfiguration GetFormatterOptionsSection(this ILoggerProviderConfiguration providerConfiguration) + { + return providerConfiguration.Configuration.GetSection("FormatterOptions"); + } } [UnsupportedOSPlatform("browser")] @@ -171,7 +180,7 @@ internal sealed class ConsoleLoggerFormatterConfigureOptions providerConfiguration) : - base(providerConfiguration.Configuration.GetSection("FormatterOptions")) + base(providerConfiguration.GetFormatterOptionsSection()) { } } @@ -182,7 +191,7 @@ internal sealed class ConsoleLoggerFormatterOptionsChangeTokenSource providerConfiguration) - : base(providerConfiguration.Configuration.GetSection("FormatterOptions")) + : base(providerConfiguration.GetFormatterOptionsSection()) { } } diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/JsonConsoleFormatterConfigureOptions.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/JsonConsoleFormatterConfigureOptions.cs new file mode 100644 index 0000000000000..a0db12343d22d --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/JsonConsoleFormatterConfigureOptions.cs @@ -0,0 +1,58 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Globalization; +using System.Runtime.Versioning; +using System.Text.Json; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging.Configuration; +using Microsoft.Extensions.Logging.Console; +using Microsoft.Extensions.Options; + +namespace Microsoft.Extensions.Logging +{ + /// + /// Configures a JsonConsoleFormatterOptions object from an IConfiguration. + /// + /// + /// Doesn't use ConfigurationBinder in order to allow ConfigurationBinder, and all its dependencies, + /// to be trimmed. This improves app size and startup. + /// + [UnsupportedOSPlatform("browser")] + internal sealed class JsonConsoleFormatterConfigureOptions : IConfigureOptions + { + private readonly IConfiguration _configuration; + + public JsonConsoleFormatterConfigureOptions(ILoggerProviderConfiguration providerConfiguration) + { + _configuration = providerConfiguration.GetFormatterOptionsSection(); + } + + public void Configure(JsonConsoleFormatterOptions options) + { + ConsoleFormatterConfigureOptions.Bind(_configuration, options); + + if (_configuration.GetSection("JsonWriterOptions") is IConfigurationSection jsonWriterOptionsConfig) + { + JsonWriterOptions jsonWriterOptions = options.JsonWriterOptions; + + if (jsonWriterOptionsConfig["Indented"] is string indented) + { + jsonWriterOptions.Indented = bool.Parse(indented); + } + + if (jsonWriterOptionsConfig["MaxDepth"] is string maxDepth) + { + jsonWriterOptions.MaxDepth = int.Parse(maxDepth, NumberStyles.Integer, NumberFormatInfo.InvariantInfo); + } + + if (jsonWriterOptionsConfig["SkipValidation"] is string skipValidation) + { + jsonWriterOptions.SkipValidation = bool.Parse(skipValidation); + } + + options.JsonWriterOptions = jsonWriterOptions; + } + } + } +} diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/SimpleConsoleFormatterConfigureOptions.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/SimpleConsoleFormatterConfigureOptions.cs new file mode 100644 index 0000000000000..6e0c102182af5 --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/SimpleConsoleFormatterConfigureOptions.cs @@ -0,0 +1,44 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.Versioning; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging.Configuration; +using Microsoft.Extensions.Logging.Console; +using Microsoft.Extensions.Options; + +namespace Microsoft.Extensions.Logging +{ + /// + /// Configures a SimpleConsoleFormatterOptions object from an IConfiguration. + /// + /// + /// Doesn't use ConfigurationBinder in order to allow ConfigurationBinder, and all its dependencies, + /// to be trimmed. This improves app size and startup. + /// + [UnsupportedOSPlatform("browser")] + internal sealed class SimpleConsoleFormatterConfigureOptions : IConfigureOptions + { + private readonly IConfiguration _configuration; + + public SimpleConsoleFormatterConfigureOptions(ILoggerProviderConfiguration providerConfiguration) + { + _configuration = providerConfiguration.GetFormatterOptionsSection(); + } + + public void Configure(SimpleConsoleFormatterOptions options) + { + ConsoleFormatterConfigureOptions.Bind(_configuration, options); + + if (_configuration["ColorBehavior"] is string colorBehavior) + { + options.ColorBehavior = ConsoleLoggerConfigureOptions.ParseEnum(colorBehavior); + } + + if (_configuration["SingleLine"] is string singleLine) + { + options.SingleLine = bool.Parse(singleLine); + } + } + } +} diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerExtensionsTests.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerExtensionsTests.cs index 38771eb258499..99b49170ed8e1 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerExtensionsTests.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerExtensionsTests.cs @@ -374,8 +374,7 @@ public void AddConsole_MaxQueueLengthSetToNegativeOrZero_Throws(int invalidMaxQu ) .BuildServiceProvider(); - // the configuration binder throws TargetInvocationException when setting options property MaxQueueLength throws exception - Assert.Throws(() => serviceProvider.GetRequiredService()); + Assert.Throws(() => serviceProvider.GetRequiredService()); } [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs index 0994980590122..02253daab4b2e 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs @@ -6,6 +6,8 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; +using System.Reflection; +using System.Text.Json; using System.Text.RegularExpressions; using Microsoft.DotNet.RemoteExecutor; using Microsoft.Extensions.Configuration; @@ -1346,6 +1348,21 @@ public void ConsoleLoggerOptions_IncludeScopes_IsReadFromLoggingConfiguration() Assert.True(formatter.FormatterOptions.IncludeScopes); } + [Fact] + public void EnsureConsoleLoggerOptions_ConfigureOptions_SupportsAllProperties() + { + // NOTE: if this test fails, it is because a property was added to one of the following types. + // When adding a new property to one of these types, ensure the corresponding + // IConfigureOptions class is updated for the new property. + + BindingFlags flags = BindingFlags.Public | BindingFlags.Instance; + Assert.Equal(9, typeof(ConsoleLoggerOptions).GetProperties(flags).Length); + Assert.Equal(3, typeof(ConsoleFormatterOptions).GetProperties(flags).Length); + Assert.Equal(5, typeof(SimpleConsoleFormatterOptions).GetProperties(flags).Length); + Assert.Equal(4, typeof(JsonConsoleFormatterOptions).GetProperties(flags).Length); + Assert.Equal(4, typeof(JsonWriterOptions).GetProperties(flags).Length); + } + public static TheoryData FormatsAndLevels { get From c0f5360f8a47c57940f14522aacc46df0aa2de9e Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Wed, 15 Feb 2023 18:29:57 -0600 Subject: [PATCH 2/3] Ensure invalid configuration data throws appropriate exception. --- .../src/ConsoleFormatterConfigureOptions.cs | 8 +- .../src/ConsoleLoggerConfigureOptions.cs | 116 +++++++++++++++--- .../JsonConsoleFormatterConfigureOptions.cs | 13 +- .../src/Resources/Strings.resx | 3 + .../SimpleConsoleFormatterConfigureOptions.cs | 8 +- .../ConsoleLoggerConfigureOptions.cs | 60 +++++++++ .../ConsoleLoggerTest.cs | 15 --- 7 files changed, 173 insertions(+), 50 deletions(-) create mode 100644 src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerConfigureOptions.cs diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleFormatterConfigureOptions.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleFormatterConfigureOptions.cs index 9f0a796070863..106c3c8263e64 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleFormatterConfigureOptions.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleFormatterConfigureOptions.cs @@ -30,9 +30,9 @@ public ConsoleFormatterConfigureOptions(ILoggerProviderConfiguration(format); -#pragma warning restore CS0618 // Type or member is obsolete + if (ParseEnum(_configuration, "Format", out ConsoleLoggerFormat format)) + { + options.Format = format; } +#pragma warning restore CS0618 // Type or member is obsolete if (_configuration["FormatterName"] is string formatterName) { options.FormatterName = formatterName; } - if (_configuration["IncludeScopes"] is string includeScopes) + if (ParseBool(_configuration, "IncludeScopes", out bool includeScopes)) { #pragma warning disable CS0618 // Type or member is obsolete - options.IncludeScopes = bool.Parse(includeScopes); + options.IncludeScopes = includeScopes; #pragma warning restore CS0618 // Type or member is obsolete } - if (_configuration["LogToStandardErrorThreshold"] is string logToStandardErrorThreshold) + if (ParseEnum(_configuration, "LogToStandardErrorThreshold", out LogLevel logToStandardErrorThreshold)) { - options.LogToStandardErrorThreshold = ParseEnum(logToStandardErrorThreshold); + options.LogToStandardErrorThreshold = logToStandardErrorThreshold; } - if (_configuration["MaxQueueLength"] is string maxQueueLength) + if (ParseInt(_configuration, "MaxQueueLength", out int maxQueueLength)) { - options.MaxQueueLength = int.Parse(maxQueueLength, NumberStyles.Integer, NumberFormatInfo.InvariantInfo); + options.MaxQueueLength = maxQueueLength; } - if (_configuration["QueueFullMode"] is string queueFullMode) + if (ParseEnum(_configuration, "QueueFullMode", out ConsoleLoggerQueueFullMode queueFullMode)) { - options.QueueFullMode = ParseEnum(queueFullMode); + options.QueueFullMode = queueFullMode; } if (_configuration["TimestampFormat"] is string timestampFormat) @@ -78,19 +78,95 @@ public void Configure(ConsoleLoggerOptions options) #pragma warning restore CS0618 // Type or member is obsolete } - if (_configuration["UseUtcTimestamp"] is string useUtcTimestamp) + if (ParseBool(_configuration, "UseUtcTimestamp", out bool useUtcTimestamp)) { #pragma warning disable CS0618 // Type or member is obsolete - options.UseUtcTimestamp = bool.Parse(useUtcTimestamp); + options.UseUtcTimestamp = useUtcTimestamp; #pragma warning restore CS0618 // Type or member is obsolete } } - public static T ParseEnum(string value) where T : struct => -#if NETSTANDARD || NETFRAMEWORK - (T)Enum.Parse(typeof(T), value, ignoreCase: true); + /// + /// Parses the configuration value at the specified key into a bool. + /// + /// true if the value was successfully found and parsed. false if the key wasn't found. + /// Thrown when invalid data was found at the specified configuration key. + public static bool ParseBool(IConfiguration configuration, string key, out bool value) + { + if (configuration[key] is string valueString) + { + try + { + value = bool.Parse(valueString); + return true; + } + catch (Exception e) + { + ThrowInvalidConfigurationException(configuration, key, typeof(bool), e); + } + } + + value = default; + return false; + } + + /// + /// Parses the configuration value at the specified key into an enum. + /// + /// true if the value was successfully found and parsed. false if the key wasn't found. + /// Thrown when invalid data was found at the specified configuration key. + public static bool ParseEnum(IConfiguration configuration, string key, out T value) where T : struct + { + if (configuration[key] is string valueString) + { + try + { + value = +#if NETFRAMEWORK || NETSTANDARD2_0 + (T)Enum.Parse(typeof(T), valueString, ignoreCase: true); #else - Enum.Parse(value, ignoreCase: true); + Enum.Parse(valueString, ignoreCase: true); #endif + return true; + } + catch (Exception e) + { + ThrowInvalidConfigurationException(configuration, key, typeof(T), e); + } + } + + value = default; + return false; + } + + /// + /// Parses the configuration value at the specified key into an int. + /// + /// true if the value was successfully found and parsed. false if the key wasn't found. + /// Thrown when invalid data was found at the specified configuration key. + public static bool ParseInt(IConfiguration configuration, string key, out int value) + { + if (configuration[key] is string valueString) + { + try + { + value = int.Parse(valueString, NumberStyles.Integer, NumberFormatInfo.InvariantInfo); + return true; + } + catch (Exception e) + { + ThrowInvalidConfigurationException(configuration, key, typeof(int), e); + } + } + + value = default; + return false; + } + + private static void ThrowInvalidConfigurationException(IConfiguration configuration, string key, Type valueType, Exception innerException) + { + IConfigurationSection section = configuration.GetSection(key); + throw new InvalidOperationException(string.Format(SR.InvalidConfigurationData, section.Path, valueType), innerException); + } } } diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/JsonConsoleFormatterConfigureOptions.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/JsonConsoleFormatterConfigureOptions.cs index a0db12343d22d..acfb948f56a95 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/JsonConsoleFormatterConfigureOptions.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/JsonConsoleFormatterConfigureOptions.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Globalization; using System.Runtime.Versioning; using System.Text.Json; using Microsoft.Extensions.Configuration; @@ -36,19 +35,19 @@ public void Configure(JsonConsoleFormatterOptions options) { JsonWriterOptions jsonWriterOptions = options.JsonWriterOptions; - if (jsonWriterOptionsConfig["Indented"] is string indented) + if (ConsoleLoggerConfigureOptions.ParseBool(jsonWriterOptionsConfig, "Indented", out bool indented)) { - jsonWriterOptions.Indented = bool.Parse(indented); + jsonWriterOptions.Indented = indented; } - if (jsonWriterOptionsConfig["MaxDepth"] is string maxDepth) + if (ConsoleLoggerConfigureOptions.ParseInt(jsonWriterOptionsConfig, "MaxDepth", out int maxDepth)) { - jsonWriterOptions.MaxDepth = int.Parse(maxDepth, NumberStyles.Integer, NumberFormatInfo.InvariantInfo); + jsonWriterOptions.MaxDepth = maxDepth; } - if (jsonWriterOptionsConfig["SkipValidation"] is string skipValidation) + if (ConsoleLoggerConfigureOptions.ParseBool(jsonWriterOptionsConfig, "SkipValidation", out bool skipValidation)) { - jsonWriterOptions.SkipValidation = bool.Parse(skipValidation); + jsonWriterOptions.SkipValidation = skipValidation; } options.JsonWriterOptions = jsonWriterOptions; diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx index c3d2473f20cef..8d925d89baf50 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx @@ -129,4 +129,7 @@ {0} message(s) dropped because of queue size limit. Increase the queue size or decrease logging verbosity to avoid this. You may change `ConsoleLoggerQueueFullMode` to stop dropping messages. + + Failed to convert configuration value at '{0}' to type '{1}'. + \ No newline at end of file diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/SimpleConsoleFormatterConfigureOptions.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/SimpleConsoleFormatterConfigureOptions.cs index 6e0c102182af5..a8f9a6cf88f21 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/SimpleConsoleFormatterConfigureOptions.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/SimpleConsoleFormatterConfigureOptions.cs @@ -30,14 +30,14 @@ public void Configure(SimpleConsoleFormatterOptions options) { ConsoleFormatterConfigureOptions.Bind(_configuration, options); - if (_configuration["ColorBehavior"] is string colorBehavior) + if (ConsoleLoggerConfigureOptions.ParseEnum(_configuration, "ColorBehavior", out LoggerColorBehavior colorBehavior)) { - options.ColorBehavior = ConsoleLoggerConfigureOptions.ParseEnum(colorBehavior); + options.ColorBehavior = colorBehavior; } - if (_configuration["SingleLine"] is string singleLine) + if (ConsoleLoggerConfigureOptions.ParseBool(_configuration, "SingleLine", out bool singleLine)) { - options.SingleLine = bool.Parse(singleLine); + options.SingleLine = singleLine; } } } diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerConfigureOptions.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerConfigureOptions.cs new file mode 100644 index 0000000000000..c30e842d3fc3e --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerConfigureOptions.cs @@ -0,0 +1,60 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Reflection; +using System.Text.Json; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace Microsoft.Extensions.Logging.Console.Test +{ + public class ConsoleLoggerConfigureOptions + { + [Fact] + public void EnsureConsoleLoggerOptions_ConfigureOptions_SupportsAllProperties() + { + // NOTE: if this test fails, it is because a property was added to one of the following types. + // When adding a new property to one of these types, ensure the corresponding + // IConfigureOptions class is updated for the new property. + + BindingFlags flags = BindingFlags.Public | BindingFlags.Instance; + Assert.Equal(9, typeof(ConsoleLoggerOptions).GetProperties(flags).Length); + Assert.Equal(3, typeof(ConsoleFormatterOptions).GetProperties(flags).Length); + Assert.Equal(5, typeof(SimpleConsoleFormatterOptions).GetProperties(flags).Length); + Assert.Equal(4, typeof(JsonConsoleFormatterOptions).GetProperties(flags).Length); + Assert.Equal(4, typeof(JsonWriterOptions).GetProperties(flags).Length); + } + + [Theory] + [InlineData("Console:LogToStandardErrorThreshold", "invalid")] + [InlineData("Console:MaxQueueLength", "notANumber")] + [InlineData("Console:QueueFullMode", "invalid")] + [InlineData("Console:FormatterOptions:IncludeScopes", "not a bool")] + [InlineData("Console:FormatterOptions:UseUtcTimestamp", "not a bool")] + [InlineData("Console:FormatterOptions:ColorBehavior", "not a behavior")] + [InlineData("Console:FormatterOptions:SingleLine", "not a bool")] + [InlineData("Console:FormatterOptions:JsonWriterOptions:Indented", "not a bool")] + [InlineData("Console:FormatterOptions:JsonWriterOptions:MaxDepth", "not an int")] + [InlineData("Console:FormatterOptions:JsonWriterOptions:SkipValidation", "not a bool")] + public void ConsoleLoggerConfigureOptions_InvalidConfigurationData(string key, string value) + { + var configuration = new ConfigurationManager(); + configuration.AddInMemoryCollection(new[] { new KeyValuePair(key, value) }); + + IServiceProvider serviceProvider = new ServiceCollection() + .AddLogging(builder => builder + .AddConfiguration(configuration) + .AddConsole()) + .BuildServiceProvider(); + + InvalidOperationException e = Assert.Throws(() => serviceProvider.GetRequiredService()); + + // "Console:" is stripped off from the config path since that config section is read by the ConsoleLogger, and not part of the Options path. + string configPath = key.Substring("Console:".Length); + Assert.Contains(configPath, e.Message); + } + } +} diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs index 02253daab4b2e..8202204fcf4bb 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs @@ -1348,21 +1348,6 @@ public void ConsoleLoggerOptions_IncludeScopes_IsReadFromLoggingConfiguration() Assert.True(formatter.FormatterOptions.IncludeScopes); } - [Fact] - public void EnsureConsoleLoggerOptions_ConfigureOptions_SupportsAllProperties() - { - // NOTE: if this test fails, it is because a property was added to one of the following types. - // When adding a new property to one of these types, ensure the corresponding - // IConfigureOptions class is updated for the new property. - - BindingFlags flags = BindingFlags.Public | BindingFlags.Instance; - Assert.Equal(9, typeof(ConsoleLoggerOptions).GetProperties(flags).Length); - Assert.Equal(3, typeof(ConsoleFormatterOptions).GetProperties(flags).Length); - Assert.Equal(5, typeof(SimpleConsoleFormatterOptions).GetProperties(flags).Length); - Assert.Equal(4, typeof(JsonConsoleFormatterOptions).GetProperties(flags).Length); - Assert.Equal(4, typeof(JsonWriterOptions).GetProperties(flags).Length); - } - public static TheoryData FormatsAndLevels { get From 3f1c0ea8c47034d42809ee47ceb9586b15fead37 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Wed, 15 Feb 2023 18:33:06 -0600 Subject: [PATCH 3/3] fixup --- .../src/ConsoleLoggerConfigureOptions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerConfigureOptions.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerConfigureOptions.cs index 1c2dc7209e677..c0bd42be6f97b 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerConfigureOptions.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerConfigureOptions.cs @@ -166,7 +166,7 @@ public static bool ParseInt(IConfiguration configuration, string key, out int va private static void ThrowInvalidConfigurationException(IConfiguration configuration, string key, Type valueType, Exception innerException) { IConfigurationSection section = configuration.GetSection(key); - throw new InvalidOperationException(string.Format(SR.InvalidConfigurationData, section.Path, valueType), innerException); + throw new InvalidOperationException(SR.Format(SR.InvalidConfigurationData, section.Path, valueType), innerException); } } }