From 21015714ddbe673ec7e542c44d8b2b2e9edaf822 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed <10833894+tarekgh@users.noreply.github.com> Date: Tue, 13 Aug 2024 08:42:44 -0700 Subject: [PATCH] Fix logging formatting (#106283) * Fix Logging formatting with multiple collections * Feedback --- .../src/LogValuesFormatter.cs | 26 +++++++------------ .../ConsoleLoggerTest.cs | 25 ++++++++++++++++++ 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/src/LogValuesFormatter.cs b/src/libraries/Microsoft.Extensions.Logging.Abstractions/src/LogValuesFormatter.cs index 69707c14c3f2d4..09fe16792d9b62 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/src/LogValuesFormatter.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/src/LogValuesFormatter.cs @@ -186,32 +186,26 @@ internal string Format() #if NET8_0_OR_GREATER internal string Format(TArg0 arg0) { - object? arg0String = null; return - !TryFormatArgumentIfNullOrEnumerable(arg0, ref arg0String) ? + !TryFormatArgumentIfNullOrEnumerable(arg0, out object? arg0String) ? string.Format(CultureInfo.InvariantCulture, _format, arg0) : string.Format(CultureInfo.InvariantCulture, _format, arg0String); } internal string Format(TArg0 arg0, TArg1 arg1) { - object? arg0String = null, arg1String = null; return - !TryFormatArgumentIfNullOrEnumerable(arg0, ref arg0String) && - !TryFormatArgumentIfNullOrEnumerable(arg1, ref arg1String) ? - string.Format(CultureInfo.InvariantCulture, _format, arg0, arg1) : - string.Format(CultureInfo.InvariantCulture, _format, arg0String ?? arg0, arg1String ?? arg1); + TryFormatArgumentIfNullOrEnumerable(arg0, out object? arg0String) | TryFormatArgumentIfNullOrEnumerable(arg1, out object? arg1String) ? + string.Format(CultureInfo.InvariantCulture, _format, arg0String ?? arg0, arg1String ?? arg1) : + string.Format(CultureInfo.InvariantCulture, _format, arg0, arg1); } internal string Format(TArg0 arg0, TArg1 arg1, TArg2 arg2) { - object? arg0String = null, arg1String = null, arg2String = null; return - !TryFormatArgumentIfNullOrEnumerable(arg0, ref arg0String) && - !TryFormatArgumentIfNullOrEnumerable(arg1, ref arg1String) && - !TryFormatArgumentIfNullOrEnumerable(arg2, ref arg2String) ? - string.Format(CultureInfo.InvariantCulture, _format, arg0, arg1, arg2) : - string.Format(CultureInfo.InvariantCulture, _format, arg0String ?? arg0, arg1String ?? arg1, arg2String ?? arg2); + TryFormatArgumentIfNullOrEnumerable(arg0, out object? arg0String) | TryFormatArgumentIfNullOrEnumerable(arg1, out object? arg1String) | TryFormatArgumentIfNullOrEnumerable(arg2, out object? arg2String) ? + string.Format(CultureInfo.InvariantCulture, _format, arg0String ?? arg0, arg1String ?? arg1, arg2String ?? arg2): + string.Format(CultureInfo.InvariantCulture, _format, arg0, arg1, arg2); } #else internal string Format(object? arg0) => @@ -253,11 +247,10 @@ internal string Format(object? arg0, object? arg1, object? arg2) => private static object FormatArgument(object? value) { - object? stringValue = null; - return TryFormatArgumentIfNullOrEnumerable(value, ref stringValue) ? stringValue : value!; + return TryFormatArgumentIfNullOrEnumerable(value, out object? stringValue) ? stringValue : value!; } - private static bool TryFormatArgumentIfNullOrEnumerable(T? value, [NotNullWhen(true)] ref object? stringValue) + private static bool TryFormatArgumentIfNullOrEnumerable(T? value, [NotNullWhen(true)] out object? stringValue) { if (value == null) { @@ -284,6 +277,7 @@ private static bool TryFormatArgumentIfNullOrEnumerable(T? value, [NotNullWhe return true; } + stringValue = null; return false; } } 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 900d667f8e198a..6747e98ebf322f 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 @@ -1409,6 +1409,31 @@ public void ConsoleLoggerOptions_IncludeScopes_IsReadFromLoggingConfiguration() Assert.True(formatter.FormatterOptions.IncludeScopes); } + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] + public void LogMultipleArrays() + { + // Arrange + using var t = SetUp(); + var logger = t.Logger; + var sink = t.Sink; + + var define1 = LoggerMessage.Define(LogLevel.Information, new EventId(), "Log: {Array1} and {Array2}"); + var define2 = LoggerMessage.Define(LogLevel.Information, new EventId(), "Log {Number}: {Array1} and {Array2}"); + + // Act + define1(logger, ["a", "b", "c"], ["d", "e", "f"], null); + define2(logger, 30, ["a", "b", "c"], ["d", "e", "f"], null); + + var expectedMessage1 = $"{CreateHeader(ConsoleLoggerFormat.Default)}{Environment.NewLine}{_paddingString}Log: a, b, c and d, e, f{Environment.NewLine}"; + var expectedMessage2 = $"{CreateHeader(ConsoleLoggerFormat.Default)}{Environment.NewLine}{_paddingString}Log 30: a, b, c and d, e, f{Environment.NewLine}"; + + Assert.Equal(4, sink.Writes.Count); + Assert.Equal("info", sink.Writes[0].Message); + Assert.Equal(expectedMessage1, sink.Writes[1].Message); + Assert.Equal("info", sink.Writes[2].Message); + Assert.Equal(expectedMessage2, sink.Writes[3].Message); + } + public static TheoryData FormatsAndLevels { get