From dbf27c30c3022ccb9ad902a9a36f358269dd130c Mon Sep 17 00:00:00 2001 From: Pawel Kadluczka Date: Fri, 30 Dec 2016 11:42:02 -0800 Subject: [PATCH] Introducing HubActivator (#83) Fixes #60 --- .../DefaultHubActivator.cs | 51 ++++++++++++++++ .../HubEndPoint.cs | 60 ++++++++---------- .../IHubActivator.cs | 13 ++++ .../SignalRDependencyInjectionExtensions.cs | 1 + .../DefaultHubActivatorTests.cs | 61 +++++++++++++++++++ 5 files changed, 153 insertions(+), 33 deletions(-) create mode 100644 src/Microsoft.AspNetCore.SignalR/DefaultHubActivator.cs create mode 100644 src/Microsoft.AspNetCore.SignalR/IHubActivator.cs create mode 100644 test/Microsoft.AspNetCore.SignalR.Tests/DefaultHubActivatorTests.cs diff --git a/src/Microsoft.AspNetCore.SignalR/DefaultHubActivator.cs b/src/Microsoft.AspNetCore.SignalR/DefaultHubActivator.cs new file mode 100644 index 0000000000..84687e1f2e --- /dev/null +++ b/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 : IHubActivator + where THub: Hub + { + 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(); + if (hub == null) + { + hub = ActivatorUtilities.CreateInstance(_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(); + } + } + } +} diff --git a/src/Microsoft.AspNetCore.SignalR/HubEndPoint.cs b/src/Microsoft.AspNetCore.SignalR/HubEndPoint.cs index 0d49bc5e2f..3db914b5d6 100644 --- a/src/Microsoft.AspNetCore.SignalR/HubEndPoint.cs +++ b/src/Microsoft.AspNetCore.SignalR/HubEndPoint.cs @@ -19,9 +19,9 @@ namespace Microsoft.AspNetCore.SignalR IHubContext hubContext, InvocationAdapterRegistry registry, ILogger> 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 hub = CreateHub(scope.ServiceProvider, connection, out created); - - await hub.OnConnectedAsync(); - - if (created) + var hubActivator = scope.ServiceProvider.GetRequiredService>(); + var hub = hubActivator.Create(); + try { - 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 hub = CreateHub(scope.ServiceProvider, connection, out created); - - await hub.OnDisconnectedAsync(exception); - - if (created) + var hubActivator = scope.ServiceProvider.GetRequiredService>(); + var hub = hubActivator.Create(); + try { - 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(); - created = false; - - if (hub == null) - { - hub = ActivatorUtilities.CreateInstance(provider); - created = true; - } - hub.Clients = _hubContext.Clients; hub.Context = new HubCallerContext(connection); hub.Groups = new GroupManager(connection, _lifetimeManager); - - return hub; } private void DiscoverHubMethods() @@ -193,11 +186,13 @@ namespace Microsoft.AspNetCore.SignalR using (var scope = _serviceScopeFactory.CreateScope()) { - bool created; - var hub = CreateHub(scope.ServiceProvider, connection, out created); + var hubActivator = scope.ServiceProvider.GetRequiredService>(); + 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; } - - if (created) + finally { - hub.Dispose(); + hubActivator.Release(hub); } } diff --git a/src/Microsoft.AspNetCore.SignalR/IHubActivator.cs b/src/Microsoft.AspNetCore.SignalR/IHubActivator.cs new file mode 100644 index 0000000000..72d42e5574 --- /dev/null +++ b/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 where THub : Hub + { + THub Create(); + void Release(THub hub); + } +} diff --git a/src/Microsoft.AspNetCore.SignalR/SignalRDependencyInjectionExtensions.cs b/src/Microsoft.AspNetCore.SignalR/SignalRDependencyInjectionExtensions.cs index 90f8035c19..0d5b20aee7 100644 --- a/src/Microsoft.AspNetCore.SignalR/SignalRDependencyInjectionExtensions.cs +++ b/src/Microsoft.AspNetCore.SignalR/SignalRDependencyInjectionExtensions.cs @@ -19,6 +19,7 @@ namespace Microsoft.Extensions.DependencyInjection services.AddSingleton, SignalROptionsSetup>(); services.AddSingleton(); services.AddSingleton(); + services.AddScoped(typeof(IHubActivator<,>), typeof(DefaultHubActivator<,>)); services.AddRouting(); return new SignalRBuilder(services); diff --git a/test/Microsoft.AspNetCore.SignalR.Tests/DefaultHubActivatorTests.cs b/test/Microsoft.AspNetCore.SignalR.Tests/DefaultHubActivatorTests.cs new file mode 100644 index 0000000000..cde3c44f51 --- /dev/null +++ b/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, object>(Mock.Of()).Create()); + } + + [Fact] + public void HubCanBeResolvedFromServiceProvider() + { + var hub = Mock.Of>(); + var mockServiceProvider = new Mock(); + mockServiceProvider + .Setup(sp => sp.GetService(typeof(Hub))) + .Returns(hub); + + Assert.Same(hub, + new DefaultHubActivator, object>(mockServiceProvider.Object).Create()); + } + + + [Fact] + public void DisposeNotCalledForHubsResolvedFromServiceProvider() + { + var mockServiceProvider = new Mock(); + mockServiceProvider + .Setup(sp => sp.GetService(typeof(Hub))) + .Returns(() => + { + var m = new Mock>(); + m.Protected().Setup("Dispose", ItExpr.IsAny()); + return m.Object; + }); + + var hubActivator = new DefaultHubActivator, object>(mockServiceProvider.Object); + var hub = hubActivator.Create(); + hubActivator.Release(hub); + Mock.Get(hub).Protected().Verify("Dispose", Times.Never(), ItExpr.IsAny()); + } + + [Fact] + public void CannotReleaseNullHub() + { + Assert.Equal("hub", + Assert.Throws( + () => new DefaultHubActivator, object>(Mock.Of()).Release(null)).ParamName); + } + } +}