From e67e7a08ca45c6ae99f9e69985f6fc1b14cc3d24 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Fri, 3 Apr 2020 10:22:05 -0700 Subject: [PATCH] Remove WebAssemblyLoggerFactory and refactor logging setup (#20432) **Changes in this PR** - Replaces `WebAssemblyLoggerFactory` with `LoggerFactory` from logging extensions package - Moves WebAssemblyConsoleLogger and PrependMessageLogger to provider model Now that we are using the standard `LoggerFactory` support for config options like `SetMinimumLevel` and `AddProvider` is available. Compared to what is currently in the `blazor-wasm` branch, the changes in this PR add an additional 12 kb to the total compressed size. Addresses #19737 --- .../test/RuntimeDependenciesResolverTest.cs | 2 + .../WebAssembly/src/Hosting/LoggingBuilder.cs | 18 ++++++++ .../src/Hosting/WebAssemblyHostBuilder.cs | 13 +++++- ...t.AspNetCore.Components.WebAssembly.csproj | 1 + .../src/Services/WebAssemblyConsoleLogger.cs | 3 +- .../WebAssemblyConsoleLoggerProvider.cs | 45 +++++++++++++++++++ .../src/Services/WebAssemblyLoggerFactory.cs | 32 ------------- .../Hosting/WebAssemblyHostBuilderTest.cs | 20 ++++++++- .../E2ETest/Tests/WebAssemblyLoggingTest.cs | 3 +- ...ory.cs => PrependMessageLoggerProvider.cs} | 39 +++++++++------- .../test/testassets/BasicTestApp/Program.cs | 12 ++--- 11 files changed, 126 insertions(+), 62 deletions(-) create mode 100644 src/Components/WebAssembly/WebAssembly/src/Hosting/LoggingBuilder.cs create mode 100644 src/Components/WebAssembly/WebAssembly/src/Services/WebAssemblyConsoleLoggerProvider.cs delete mode 100644 src/Components/WebAssembly/WebAssembly/src/Services/WebAssemblyLoggerFactory.cs rename src/Components/test/testassets/BasicTestApp/{PrependMessageLoggerFactory.cs => PrependMessageLoggerProvider.cs} (58%) diff --git a/src/Components/WebAssembly/Build/test/RuntimeDependenciesResolverTest.cs b/src/Components/WebAssembly/Build/test/RuntimeDependenciesResolverTest.cs index 19efce0504..d87b404537 100644 --- a/src/Components/WebAssembly/Build/test/RuntimeDependenciesResolverTest.cs +++ b/src/Components/WebAssembly/Build/test/RuntimeDependenciesResolverTest.cs @@ -56,7 +56,9 @@ namespace Microsoft.AspNetCore.Components.WebAssembly.Build "Microsoft.Extensions.FileProviders.Abstractions.dll", "Microsoft.Extensions.FileProviders.Physical.dll", "Microsoft.Extensions.FileSystemGlobbing.dll", + "Microsoft.Extensions.Logging.dll", "Microsoft.Extensions.Logging.Abstractions.dll", + "Microsoft.Extensions.Options.dll", "Microsoft.Extensions.Primitives.dll", "Microsoft.JSInterop.dll", "Microsoft.JSInterop.WebAssembly.dll", diff --git a/src/Components/WebAssembly/WebAssembly/src/Hosting/LoggingBuilder.cs b/src/Components/WebAssembly/WebAssembly/src/Hosting/LoggingBuilder.cs new file mode 100644 index 0000000000..66c2190451 --- /dev/null +++ b/src/Components/WebAssembly/WebAssembly/src/Hosting/LoggingBuilder.cs @@ -0,0 +1,18 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNetCore.Components.WebAssembly.Hosting +{ + internal class LoggingBuilder : ILoggingBuilder + { + public LoggingBuilder(IServiceCollection services) + { + Services = services; + } + + public IServiceCollection Services { get; } + } +} diff --git a/src/Components/WebAssembly/WebAssembly/src/Hosting/WebAssemblyHostBuilder.cs b/src/Components/WebAssembly/WebAssembly/src/Hosting/WebAssemblyHostBuilder.cs index f5d803c3b8..0a60896f3e 100644 --- a/src/Components/WebAssembly/WebAssembly/src/Hosting/WebAssemblyHostBuilder.cs +++ b/src/Components/WebAssembly/WebAssembly/src/Hosting/WebAssemblyHostBuilder.cs @@ -55,6 +55,9 @@ namespace Microsoft.AspNetCore.Components.WebAssembly.Hosting Configuration = new ConfigurationBuilder(); RootComponents = new RootComponentMappingCollection(); Services = new ServiceCollection(); + Logging = new LoggingBuilder(Services); + + Logging.SetMinimumLevel(LogLevel.Warning); // Retrieve required attributes from JSRuntimeInvoker InitializeNavigationManager(jsRuntimeInvoker); @@ -128,6 +131,11 @@ namespace Microsoft.AspNetCore.Components.WebAssembly.Hosting /// public IWebAssemblyHostEnvironment HostEnvironment { get; } + /// + /// Gets the logging builder for configuring logging services. + /// + public ILoggingBuilder Logging { get; } + /// /// Registers a instance to be used to create the . /// @@ -186,8 +194,9 @@ namespace Microsoft.AspNetCore.Components.WebAssembly.Hosting Services.AddSingleton(DefaultWebAssemblyJSRuntime.Instance); Services.AddSingleton(WebAssemblyNavigationManager.Instance); Services.AddSingleton(WebAssemblyNavigationInterception.Instance); - Services.AddSingleton(); - Services.TryAdd(ServiceDescriptor.Singleton(typeof(ILogger<>), typeof(WebAssemblyConsoleLogger<>))); + Services.AddLogging(builder => { + builder.AddProvider(new WebAssemblyConsoleLoggerProvider(DefaultWebAssemblyJSRuntime.Instance)); + }); } } } diff --git a/src/Components/WebAssembly/WebAssembly/src/Microsoft.AspNetCore.Components.WebAssembly.csproj b/src/Components/WebAssembly/WebAssembly/src/Microsoft.AspNetCore.Components.WebAssembly.csproj index c543a7a11b..8b6c508893 100644 --- a/src/Components/WebAssembly/WebAssembly/src/Microsoft.AspNetCore.Components.WebAssembly.csproj +++ b/src/Components/WebAssembly/WebAssembly/src/Microsoft.AspNetCore.Components.WebAssembly.csproj @@ -10,6 +10,7 @@ + diff --git a/src/Components/WebAssembly/WebAssembly/src/Services/WebAssemblyConsoleLogger.cs b/src/Components/WebAssembly/WebAssembly/src/Services/WebAssemblyConsoleLogger.cs index 51fe7536ea..668aeb7c96 100644 --- a/src/Components/WebAssembly/WebAssembly/src/Services/WebAssemblyConsoleLogger.cs +++ b/src/Components/WebAssembly/WebAssembly/src/Services/WebAssemblyConsoleLogger.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Concurrent; using System.Text; using Microsoft.Extensions.Logging; using Microsoft.JSInterop; @@ -43,7 +44,7 @@ namespace Microsoft.AspNetCore.Components.WebAssembly.Services public bool IsEnabled(LogLevel logLevel) { - return logLevel >= LogLevel.Warning && logLevel != LogLevel.None; + return logLevel != LogLevel.None; } public void Log(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func formatter) diff --git a/src/Components/WebAssembly/WebAssembly/src/Services/WebAssemblyConsoleLoggerProvider.cs b/src/Components/WebAssembly/WebAssembly/src/Services/WebAssemblyConsoleLoggerProvider.cs new file mode 100644 index 0000000000..c79b3f07b0 --- /dev/null +++ b/src/Components/WebAssembly/WebAssembly/src/Services/WebAssemblyConsoleLoggerProvider.cs @@ -0,0 +1,45 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Concurrent; +using Microsoft.Extensions.Logging; +using Microsoft.JSInterop; + +namespace Microsoft.AspNetCore.Components.WebAssembly.Services +{ + /// + /// A provider of instances. + /// + internal class WebAssemblyConsoleLoggerProvider : ILoggerProvider + { + private readonly ConcurrentDictionary> _loggers; + private readonly IJSInProcessRuntime _jsRuntime; + private bool _disposed; + + /// + /// Creates an instance of . + /// + /// The options to create instances with. + public WebAssemblyConsoleLoggerProvider(IJSInProcessRuntime jsRuntime) + { + _loggers = new ConcurrentDictionary>(); + _jsRuntime = jsRuntime; + } + + /// + public ILogger CreateLogger(string name) + { + return _loggers.GetOrAdd(name, loggerName => new WebAssemblyConsoleLogger(name, _jsRuntime)); + } + + /// + public void Dispose() + { + if (!_disposed) + { + _disposed = true; + } + } + } +} diff --git a/src/Components/WebAssembly/WebAssembly/src/Services/WebAssemblyLoggerFactory.cs b/src/Components/WebAssembly/WebAssembly/src/Services/WebAssemblyLoggerFactory.cs deleted file mode 100644 index 400d537fd0..0000000000 --- a/src/Components/WebAssembly/WebAssembly/src/Services/WebAssemblyLoggerFactory.cs +++ /dev/null @@ -1,32 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; -using Microsoft.JSInterop; - -namespace Microsoft.AspNetCore.Components.WebAssembly.Services -{ - internal class WebAssemblyLoggerFactory : ILoggerFactory - { - private readonly IJSInProcessRuntime _jsRuntime; - - public WebAssemblyLoggerFactory(IServiceProvider services) - { - _jsRuntime = (IJSInProcessRuntime)services.GetRequiredService(); - } - - // We might implement this in the future, but it's not required currently - public void AddProvider(ILoggerProvider provider) - => throw new NotSupportedException(); - - public ILogger CreateLogger(string categoryName) - => new WebAssemblyConsoleLogger(categoryName, _jsRuntime); - - public void Dispose() - { - // No-op - } - } -} diff --git a/src/Components/WebAssembly/WebAssembly/test/Hosting/WebAssemblyHostBuilderTest.cs b/src/Components/WebAssembly/WebAssembly/test/Hosting/WebAssemblyHostBuilderTest.cs index e35d0a2955..55c436596a 100644 --- a/src/Components/WebAssembly/WebAssembly/test/Hosting/WebAssemblyHostBuilderTest.cs +++ b/src/Components/WebAssembly/WebAssembly/test/Hosting/WebAssemblyHostBuilderTest.cs @@ -230,12 +230,28 @@ namespace Microsoft.AspNetCore.Components.WebAssembly.Hosting // Arrange & Act var builder = new WebAssemblyHostBuilder(new TestWebAssemblyJSRuntimeInvoker()); - // Assert - Assert.Equal(DefaultServiceTypes.Count, builder.Services.Count); foreach (var type in DefaultServiceTypes) { Assert.Single(builder.Services, d => d.ServiceType == type); } } + + [Fact] + public void Builder_SupportsConfiguringLogging() + { + // Arrange + var builder = new WebAssemblyHostBuilder(new TestWebAssemblyJSRuntimeInvoker()); + var provider = new Mock(); + + // Act + builder.Logging.AddProvider(provider.Object); + var host = builder.Build(); + + // Assert + var loggerProvider = host.Services.GetRequiredService(); + Assert.NotNull(loggerProvider); + Assert.Equal(provider.Object, loggerProvider); + + } } } diff --git a/src/Components/test/E2ETest/Tests/WebAssemblyLoggingTest.cs b/src/Components/test/E2ETest/Tests/WebAssemblyLoggingTest.cs index 0e8822b86b..acb8df650f 100644 --- a/src/Components/test/E2ETest/Tests/WebAssemblyLoggingTest.cs +++ b/src/Components/test/E2ETest/Tests/WebAssemblyLoggingTest.cs @@ -85,10 +85,11 @@ namespace Microsoft.AspNetCore.Components.E2ETest.Tests Browser.FindElement(By.Id("log-none")).Click(); Browser.FindElement(By.Id("log-trace")).Click(); Browser.FindElement(By.Id("log-debug")).Click(); - Browser.FindElement(By.Id("log-information")).Click(); AssertLastLogMessage(LogLevel.Info, "Test log message"); // These severity levels are displayed + Browser.FindElement(By.Id("log-information")).Click(); + AssertLastLogMessage(LogLevel.Info, "[Custom logger] This is a Information message with count=4"); Browser.FindElement(By.Id("log-warning")).Click(); AssertLastLogMessage(LogLevel.Warning, "[Custom logger] This is a Warning message with count=5"); Browser.FindElement(By.Id("log-error")).Click(); diff --git a/src/Components/test/testassets/BasicTestApp/PrependMessageLoggerFactory.cs b/src/Components/test/testassets/BasicTestApp/PrependMessageLoggerProvider.cs similarity index 58% rename from src/Components/test/testassets/BasicTestApp/PrependMessageLoggerFactory.cs rename to src/Components/test/testassets/BasicTestApp/PrependMessageLoggerProvider.cs index 40014d0ca4..e0025ec78c 100644 --- a/src/Components/test/testassets/BasicTestApp/PrependMessageLoggerFactory.cs +++ b/src/Components/test/testassets/BasicTestApp/PrependMessageLoggerProvider.cs @@ -3,33 +3,42 @@ using System; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Configuration; +using Microsoft.AspNetCore.Components.WebAssembly.Services; +using Microsoft.JSInterop; namespace BasicTestApp { - // The goal for this class is to make it possible for E2E tests to observe that a custom - // logger factory can be plugged in and gets used when logging unhandled exceptions. - // However, it's valuable to pass through all calls to the default implementation too - // so that if any defect in the underlying implementation would break tests, we still see it. - - public class PrependMessageLoggerFactory : ILoggerFactory + internal class PrependMessageLoggerProvider : ILoggerProvider { - private readonly string _message; - private readonly ILoggerFactory _underlyingFactory; + ILogger _logger; + string _message; + ILogger _defaultLogger; + private bool _disposed = false; - public PrependMessageLoggerFactory(string message, ILoggerFactory underlyingFactory) + public PrependMessageLoggerProvider(string message, IJSRuntime runtime) { _message = message; - _underlyingFactory = underlyingFactory; + _defaultLogger = new WebAssemblyConsoleLogger(runtime); } - public void AddProvider(ILoggerProvider provider) - => _underlyingFactory.AddProvider(provider); - public ILogger CreateLogger(string categoryName) - => new PrependMessageLogger(_message, _underlyingFactory.CreateLogger(categoryName)); + { + if (_logger == null) + { + _logger = new PrependMessageLogger(_message, _defaultLogger); + } + return _logger; + } public void Dispose() - => _underlyingFactory.Dispose(); + { + if (!_disposed) + { + _logger = null; + } + _disposed = true; + } private class PrependMessageLogger : ILogger { diff --git a/src/Components/test/testassets/BasicTestApp/Program.cs b/src/Components/test/testassets/BasicTestApp/Program.cs index d0b7dadd1e..6a1a24d7f9 100644 --- a/src/Components/test/testassets/BasicTestApp/Program.cs +++ b/src/Components/test/testassets/BasicTestApp/Program.cs @@ -16,6 +16,7 @@ using Microsoft.AspNetCore.Components.WebAssembly.Http; using Microsoft.AspNetCore.Components.WebAssembly.Services; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Microsoft.JSInterop; namespace BasicTestApp { @@ -44,15 +45,8 @@ namespace BasicTestApp policy.RequireAssertion(ctx => ctx.User.Identity.Name?.StartsWith("B") ?? false)); }); - // Replace the default logger with a custom one that wraps it - var originalLoggerDescriptor = builder.Services.Single(d => d.ServiceType == typeof(ILoggerFactory)); - builder.Services.AddSingleton(services => - { - var originalLogger = (ILoggerFactory)Activator.CreateInstance( - originalLoggerDescriptor.ImplementationType, - new object[] { services }); - return new PrependMessageLoggerFactory("Custom logger", originalLogger); - }); + builder.Logging.Services.AddSingleton(s => new PrependMessageLoggerProvider("Custom logger", s.GetService())); + builder.Logging.SetMinimumLevel(LogLevel.Information); var host = builder.Build(); ConfigureCulture(host);