Introducing HubActivator (#83)

Fixes #60

src/Microsoft.AspNetCore.SignalR/DefaultHubActivator.cs

@@ -0,0 +1,51 @@
+// 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.Diagnostics;
+using Microsoft.Extensions.DependencyInjection;
+
+namespace Microsoft.AspNetCore.SignalR
+{
+    public class DefaultHubActivator<THub, TClient> : IHubActivator<THub, TClient>
+        where THub: Hub<TClient>
+    {
+        private readonly IServiceProvider _serviceProvider;
+        private bool? _created;
+
+        public DefaultHubActivator(IServiceProvider serviceProvider)
+        {
+            _serviceProvider = serviceProvider;
+        }
+
+        public THub Create()
+        {
+            Debug.Assert(!_created.HasValue, "hub activators must not be reused.");
+
+            _created = false;
+            var hub = _serviceProvider.GetService<THub>();
+            if (hub == null)
+            {
+                hub = ActivatorUtilities.CreateInstance<THub>(_serviceProvider);
+                _created = true;
+            }
+
+            return hub;
+        }
+
+        public void Release(THub hub)
+        {
+            if (hub == null)
+            {
+                throw new ArgumentNullException(nameof(hub));
+            }
+
+            Debug.Assert(_created.HasValue, "hubs must be released with the hub activator they were created");
+
+            if (_created.Value)
+            {
+                hub.Dispose();
+            }
+        }
+    }
+}

src/Microsoft.AspNetCore.SignalR/HubEndPoint.cs

@@ -19,9 +19,9 @@ namespace Microsoft.AspNetCore.SignalR
                            IHubContext<THub> hubContext,
                            InvocationAdapterRegistry registry,
                            ILogger<HubEndPoint<THub>> logger,
-                           IServiceScopeFactory serviceScopeFactory) : base(lifetimeManager, hubContext, registry, logger, serviceScopeFactory)
+                           IServiceScopeFactory serviceScopeFactory)
+            : base(lifetimeManager, hubContext, registry, logger, serviceScopeFactory)
         {
-
         }
     }
 
@@ -63,14 +63,16 @@ namespace Microsoft.AspNetCore.SignalR
 
                 using (var scope = _serviceScopeFactory.CreateScope())
                 {
-                    bool created;
+                    var hubActivator = scope.ServiceProvider.GetRequiredService<IHubActivator<THub, TClient>>();
-                    var hub = CreateHub(scope.ServiceProvider, connection, out created);
+                    var hub = hubActivator.Create();
-
+                    try
-                    await hub.OnConnectedAsync();
-
-                    if (created)
                     {
-                        hub.Dispose();
+                        InitializeHub(hub, connection);
+                        await hub.OnConnectedAsync();
+                    }
+                    finally
+                    {
+                        hubActivator.Release(hub);
                     }
                 }
 
@@ -87,14 +89,16 @@ namespace Microsoft.AspNetCore.SignalR
             {
                 using (var scope = _serviceScopeFactory.CreateScope())
                 {
-                    bool created;
+                    var hubActivator = scope.ServiceProvider.GetRequiredService<IHubActivator<THub, TClient>>();
-                    var hub = CreateHub(scope.ServiceProvider, connection, out created);
+                    var hub = hubActivator.Create();
-
+                    try
-                    await hub.OnDisconnectedAsync(exception);
-
-                    if (created)
                     {
-                        hub.Dispose();
+                        InitializeHub(hub, connection);
+                        await hub.OnDisconnectedAsync(exception);
+                    }
+                    finally
+                    {
+                        hubActivator.Release(hub);
                     }
                 }
 
@@ -145,22 +149,11 @@ namespace Microsoft.AspNetCore.SignalR
             }
         }
 
-        private THub CreateHub(IServiceProvider provider, Connection connection, out bool created)
+        private void InitializeHub(THub hub, Connection connection)
         {
-            var hub = provider.GetService<THub>();
-            created = false;
-
-            if (hub == null)
-            {
-                hub = ActivatorUtilities.CreateInstance<THub>(provider);
-                created = true;
-            }
-
             hub.Clients = _hubContext.Clients;
             hub.Context = new HubCallerContext(connection);
             hub.Groups = new GroupManager<THub>(connection, _lifetimeManager);
-
-            return hub;
         }
 
         private void DiscoverHubMethods()
@@ -193,11 +186,13 @@ namespace Microsoft.AspNetCore.SignalR
 
                     using (var scope = _serviceScopeFactory.CreateScope())
                     {
-                        bool created;
+                        var hubActivator = scope.ServiceProvider.GetRequiredService<IHubActivator<THub, TClient>>();
-                        var hub = CreateHub(scope.ServiceProvider, connection, out created);
+                        var hub = hubActivator.Create();
 
                         try
                         {
+                            InitializeHub(hub, connection);
+
                             var result = methodInfo.Invoke(hub, invocationDescriptor.Arguments);
                             var resultTask = result as Task;
                             if (resultTask != null)
@@ -224,10 +219,9 @@ namespace Microsoft.AspNetCore.SignalR
                             _logger.LogError(0, ex, "Failed to invoke hub method");
                             invocationResult.Error = ex.Message;
                         }
-
+                        finally
-                        if (created)
                         {
-                            hub.Dispose();
+                            hubActivator.Release(hub);
                         }
                     }
 

src/Microsoft.AspNetCore.SignalR/IHubActivator.cs

@@ -0,0 +1,13 @@
+// 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;
+
+namespace Microsoft.AspNetCore.SignalR
+{
+    public interface IHubActivator<THub, TClient> where THub : Hub<TClient>
+    {
+        THub Create();
+        void Release(THub hub);
+    }
+}

src/Microsoft.AspNetCore.SignalR/SignalRDependencyInjectionExtensions.cs

@@ -19,6 +19,7 @@ namespace Microsoft.Extensions.DependencyInjection
             services.AddSingleton<IConfigureOptions<SignalROptions>, SignalROptionsSetup>();
             services.AddSingleton<JsonNetInvocationAdapter>();
             services.AddSingleton<InvocationAdapterRegistry>();
+            services.AddScoped(typeof(IHubActivator<,>), typeof(DefaultHubActivator<,>));
             services.AddRouting();
 
             return new SignalRBuilder(services);

test/Microsoft.AspNetCore.SignalR.Tests/DefaultHubActivatorTests.cs

@@ -0,0 +1,61 @@
+// 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 Moq;
+using Moq.Protected;
+using Xunit;
+
+namespace Microsoft.AspNetCore.SignalR.Tests
+{
+    public class DefaultHubActivatorTests
+    {
+        [Fact]
+        public void HubCreatedIfNotResolvedFromServiceProvider()
+        {
+            Assert.NotNull(
+                new DefaultHubActivator<Hub<object>, object>(Mock.Of<IServiceProvider>()).Create());
+        }
+
+        [Fact]
+        public void HubCanBeResolvedFromServiceProvider()
+        {
+            var hub = Mock.Of<Hub<object>>();
+            var mockServiceProvider = new Mock<IServiceProvider>();
+            mockServiceProvider
+                .Setup(sp => sp.GetService(typeof(Hub<object>)))
+                .Returns(hub);
+
+            Assert.Same(hub,
+                new DefaultHubActivator<Hub<object>, object>(mockServiceProvider.Object).Create());
+        }
+
+
+        [Fact]
+        public void DisposeNotCalledForHubsResolvedFromServiceProvider()
+        {
+            var mockServiceProvider = new Mock<IServiceProvider>();
+            mockServiceProvider
+                .Setup(sp => sp.GetService(typeof(Hub<object>)))
+                .Returns(() =>
+                {
+                    var m = new Mock<Hub<object>>();
+                    m.Protected().Setup("Dispose", ItExpr.IsAny<bool>());
+                    return m.Object;
+                });
+
+            var hubActivator = new DefaultHubActivator<Hub<object>, object>(mockServiceProvider.Object);
+            var hub = hubActivator.Create();
+            hubActivator.Release(hub);
+            Mock.Get(hub).Protected().Verify("Dispose", Times.Never(), ItExpr.IsAny<bool>());
+        }
+
+        [Fact]
+        public void CannotReleaseNullHub()
+        {
+            Assert.Equal("hub",
+                Assert.Throws<ArgumentNullException>(
+                    () => new DefaultHubActivator<Hub<object>, object>(Mock.Of<IServiceProvider>()).Release(null)).ParamName);
+        }
+    }
+}