Add logging by default to HubConnectionBuilder (#1867)

- Use the AddLogging extension method by default in the HubConnection
- Removed WithConsoleLogger extension methods
- Removed WithLoggerFactory extension method (moved to test only)
- Added WithLogger that uses the new the new ILoggerBuilder

samples/ClientSample/HubSample.cs

@@ -32,7 +32,11 @@ namespace ClientSample
             Console.WriteLine("Connecting to {0}", endpoint);
             var connection = new HubConnectionBuilder()
                 .WithEndPoint(endpoint)
-                .WithConsoleLogger(LogLevel.Information)
+                .WithLogging(logging =>
+                {
+                    logging.AddConsole();
+                    logging.SetMinimumLevel(LogLevel.Trace);
+                })
                 .Build();
 
             try

src/Microsoft.AspNetCore.SignalR.Client.Core/HubConnectionBuilder.cs

@@ -22,8 +22,8 @@ namespace Microsoft.AspNetCore.SignalR.Client
         public HubConnectionBuilder()
         {
             Services = new ServiceCollection();
-            Services.AddSingleton<ILoggerFactory>(NullLoggerFactory.Instance);
             Services.AddSingleton<HubConnection>();
+            Services.AddLogging();
             this.AddJsonProtocol();
         }
 

src/Microsoft.AspNetCore.SignalR.Client.Core/HubConnectionBuilderExtensions.cs

@@ -24,22 +24,10 @@ namespace Microsoft.AspNetCore.SignalR.Client
             return hubConnectionBuilder;
         }
 
-        public static IHubConnectionBuilder WithLoggerFactory(this IHubConnectionBuilder hubConnectionBuilder, ILoggerFactory loggerFactory)
+        public static IHubConnectionBuilder WithLogging(this IHubConnectionBuilder hubConnectionBuilder, Action<ILoggingBuilder> configureLogging)
         {
-            if (loggerFactory == null)
+            hubConnectionBuilder.Services.AddLogging(configureLogging);
-            {
-                throw new ArgumentNullException(nameof(loggerFactory));
-            }
-
-            hubConnectionBuilder.Services.AddSingleton(loggerFactory);
             return hubConnectionBuilder;
         }
-
-        public static IHubConnectionBuilder WithLogger(this IHubConnectionBuilder hubConnectionBuilder, Action<ILoggerFactory> configureLogging)
-        {
-            var loggerFactory = new LoggerFactory();
-            configureLogging(loggerFactory);
-            return hubConnectionBuilder.WithLoggerFactory(loggerFactory);
-        }
     }
 }

src/Microsoft.AspNetCore.SignalR.Client/HubConnectionBuilderExtensions.cs

@@ -1,27 +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.Logging;
-
-namespace Microsoft.AspNetCore.SignalR.Client
-{
-    public static class HubConnectionBuilderExtensions
-    {
-        public static IHubConnectionBuilder WithConsoleLogger(this IHubConnectionBuilder hubConnectionBuilder)
-        {
-            return hubConnectionBuilder.WithLogger(loggerFactory =>
-            {
-                loggerFactory.AddConsole();
-            });
-        }
-
-        public static IHubConnectionBuilder WithConsoleLogger(this IHubConnectionBuilder hubConnectionBuilder, LogLevel logLevel)
-        {
-            return hubConnectionBuilder.WithLogger(loggerFactory =>
-            {
-                loggerFactory.AddConsole(logLevel);
-            });
-        }
-    }
-}

test/Microsoft.AspNetCore.SignalR.Client.Tests/HubConnectionBuilderExtensionsTests.cs

@@ -57,18 +57,13 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
         }
 
         [Fact]
-        public void WithConsoleLoggerAddsLogger()
+        public void DefaultLoggerFactoryExists()
         {
-            var loggingFactory = Mock.Of<ILoggerFactory>();
-
             var connectionBuilder = new HubConnectionBuilder();
-            connectionBuilder.WithLoggerFactory(loggingFactory);
-
             var serviceProvider = connectionBuilder.Services.BuildServiceProvider();
 
-            var resolvedLoggingFactory = serviceProvider.GetService<ILoggerFactory>();
+            var loggerFactory = serviceProvider.GetService<ILoggerFactory>();
-
+            Assert.NotNull(loggerFactory);
-            Assert.Same(resolvedLoggingFactory, loggingFactory);
         }
 
         [Fact]

test/Microsoft.AspNetCore.SignalR.Client.Tests/HubConnectionBuilderTests.cs

@@ -22,13 +22,6 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
             Assert.Equal("Cannot create HubConnection instance. A connection was not configured.", ex.Message);
         }
 
-        [Fact]
-        public void WithLoggerFactoryThrowsForNullLoggerFactory()
-        {
-            Assert.Equal("loggerFactory",
-                Assert.Throws<ArgumentNullException>(() => new HubConnectionBuilder().WithLoggerFactory(null)).ParamName);
-        }
-
         [Fact]
         public void AddJsonProtocolSetsHubProtocolToJsonWithDefaultOptions()
         {

test/Microsoft.AspNetCore.SignalR.Tests.Utils/HubConnectionBuilderTestExtensions.cs

@@ -0,0 +1,25 @@
+// 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;
+
+namespace Microsoft.AspNetCore.SignalR.Client
+{
+    public static class HubConnectionBuilderTestExtensions
+    {
+        // Tests want to override the built in LoggerFactory, it internally calls AddLogging
+        // https://github.com/aspnet/Logging/blob/671af986ec3b46dc81e28e4a6c37a9d0ee283c65/src/Microsoft.Extensions.Logging.Testing/AssemblyTestLog.cs#L130
+        public static IHubConnectionBuilder WithLoggerFactory(this IHubConnectionBuilder hubConnectionBuilder, ILoggerFactory loggerFactory)
+        {
+            if (loggerFactory == null)
+            {
+                throw new ArgumentNullException(nameof(loggerFactory));
+            }
+
+            hubConnectionBuilder.Services.AddSingleton(loggerFactory);
+            return hubConnectionBuilder;
+        }
+    }
+}

test/Microsoft.AspNetCore.SignalR.Tests.Utils/Microsoft.AspNetCore.SignalR.Tests.Utils.csproj

@@ -12,6 +12,7 @@
   <ItemGroup>
     <ProjectReference Include="..\..\src\Microsoft.AspNetCore.SignalR.Common\Microsoft.AspNetCore.SignalR.Common.csproj" />
     <ProjectReference Include="..\..\src\Microsoft.AspNetCore.SignalR.Core\Microsoft.AspNetCore.SignalR.Core.csproj" />
+    <ProjectReference Include="..\..\src\Microsoft.AspNetCore.SignalR.Client.Core\Microsoft.AspNetCore.SignalR.Client.Core.csproj" />
     <ProjectReference Include="..\..\src\Microsoft.AspNetCore.SignalR.Protocols.MsgPack\Microsoft.AspNetCore.SignalR.Protocols.MsgPack.csproj" />
   </ItemGroup>