Run users options last (#15121)

Author: BrennanConroy

src/SignalR/perf/Microbenchmarks/DefaultHubDispatcherBenchmark.cs

@@ -37,8 +37,7 @@ public void GlobalSetup()
             _dispatcher = new DefaultHubDispatcher<TestHub>(
                 serviceScopeFactory,
                 new HubContext<TestHub>(new DefaultHubLifetimeManager<TestHub>(NullLogger<DefaultHubLifetimeManager<TestHub>>.Instance)),
-                Options.Create(new HubOptions<TestHub>()),
-                Options.Create(new HubOptions()),
+                enableDetailedErrors: false,
                 new Logger<DefaultHubDispatcher<TestHub>>(NullLoggerFactory.Instance));
 
             var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default);

src/SignalR/server/Core/src/HubConnectionHandler.cs

@@ -59,14 +59,22 @@ IServiceScopeFactory serviceScopeFactory
             _logger = loggerFactory.CreateLogger<HubConnectionHandler<THub>>();
             _userIdProvider = userIdProvider;
 
-            _enableDetailedErrors = _hubOptions.EnableDetailedErrors ?? _globalHubOptions.EnableDetailedErrors ?? false;
-            _maximumMessageSize = _hubOptions.MaximumReceiveMessageSize ?? _globalHubOptions.MaximumReceiveMessageSize;
+            _enableDetailedErrors = false;
+            if (_hubOptions.UserHasSetValues)
+            {
+                _maximumMessageSize = _hubOptions.MaximumReceiveMessageSize;
+                _enableDetailedErrors = _hubOptions.EnableDetailedErrors ?? _enableDetailedErrors;
+            }
+            else
+            {
+                _maximumMessageSize = _globalHubOptions.MaximumReceiveMessageSize;
+                _enableDetailedErrors = _globalHubOptions.EnableDetailedErrors ?? _enableDetailedErrors;
+            }
 
             _dispatcher = new DefaultHubDispatcher<THub>(
                 serviceScopeFactory,
                 new HubContext<THub>(lifetimeManager),
-                hubOptions,
-                globalHubOptions,
+                _enableDetailedErrors,
                 new Logger<DefaultHubDispatcher<THub>>(loggerFactory));
         }
 

src/SignalR/server/Core/src/HubOptionsSetup`T.cs

@@ -23,6 +23,12 @@ public void Configure(HubOptions<THub> options)
             }
             options.KeepAliveInterval = _hubOptions.KeepAliveInterval;
             options.HandshakeTimeout = _hubOptions.HandshakeTimeout;
+            options.ClientTimeoutInterval = _hubOptions.ClientTimeoutInterval;
+            options.EnableDetailedErrors = _hubOptions.EnableDetailedErrors;
+            options.MaximumReceiveMessageSize = _hubOptions.MaximumReceiveMessageSize;
+            options.StreamBufferCapacity = _hubOptions.StreamBufferCapacity;
+
+            options.UserHasSetValues = true;
         }
     }
 }

src/SignalR/server/Core/src/HubOptions`T.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// 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.
 
 namespace Microsoft.AspNetCore.SignalR
@@ -9,5 +9,6 @@ namespace Microsoft.AspNetCore.SignalR
     /// <typeparam name="THub">The hub type to configure.</typeparam>
     public class HubOptions<THub> : HubOptions where THub : Hub
     {
+        internal bool UserHasSetValues { get; set; }
     }
 }

src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs

@@ -28,12 +28,11 @@ internal partial class DefaultHubDispatcher<THub> : HubDispatcher<THub> where TH
         private readonly ILogger<HubDispatcher<THub>> _logger;
         private readonly bool _enableDetailedErrors;
 
-        public DefaultHubDispatcher(IServiceScopeFactory serviceScopeFactory, IHubContext<THub> hubContext, IOptions<HubOptions<THub>> hubOptions,
-            IOptions<HubOptions> globalHubOptions, ILogger<DefaultHubDispatcher<THub>> logger)
+        public DefaultHubDispatcher(IServiceScopeFactory serviceScopeFactory, IHubContext<THub> hubContext, bool enableDetailedErrors, ILogger<DefaultHubDispatcher<THub>> logger)
         {
             _serviceScopeFactory = serviceScopeFactory;
             _hubContext = hubContext;
-            _enableDetailedErrors = hubOptions.Value.EnableDetailedErrors ?? globalHubOptions.Value.EnableDetailedErrors ?? false;
+            _enableDetailedErrors = enableDetailedErrors;
             _logger = logger;
             DiscoverHubMethods();
         }

src/SignalR/server/SignalR/src/SignalRDependencyInjectionExtensions.cs

@@ -4,7 +4,6 @@
 using System;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.SignalR;
-using Microsoft.AspNetCore.SignalR.Internal;
 using Microsoft.Extensions.DependencyInjection.Extensions;
 using Microsoft.Extensions.Options;
 
@@ -67,8 +66,10 @@ public static ISignalRServerBuilder AddSignalR(this IServiceCollection services,
                 throw new ArgumentNullException(nameof(services));
             }
 
-            return services.Configure(configure)
-                .AddSignalR();
+            var signalrBuilder = services.AddSignalR();
+            // Setup users settings after we've setup ours
+            services.Configure(configure);
+            return signalrBuilder;
         }
     }
 }

src/SignalR/server/SignalR/test/AddSignalRTests.cs

@@ -1,6 +1,7 @@
 // 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.Generic;
 using System.Threading;
 using System.Threading.Tasks;
@@ -81,6 +82,29 @@ public void HubSpecificOptionsDoNotAffectGlobalHubOptions()
             Assert.Equal(0, serviceProvider.GetRequiredService<IOptions<HubOptions<CustomHub>>>().Value.SupportedProtocols.Count);
         }
 
+        [Fact]
+        public void HubSpecificOptionsHaveSameValuesAsGlobalHubOptions()
+        {
+            var serviceCollection = new ServiceCollection();
+
+            serviceCollection.AddSignalR().AddHubOptions<CustomHub>(options =>
+            {
+            });
+
+            var serviceProvider = serviceCollection.BuildServiceProvider();
+            var hubOptions = serviceProvider.GetRequiredService<IOptions<HubOptions<CustomHub>>>().Value;
+            var globalHubOptions = serviceProvider.GetRequiredService<IOptions<HubOptions>>().Value;
+
+            Assert.Equal(globalHubOptions.MaximumReceiveMessageSize, hubOptions.MaximumReceiveMessageSize);
+            Assert.Equal(globalHubOptions.StreamBufferCapacity, hubOptions.StreamBufferCapacity);
+            Assert.Equal(globalHubOptions.EnableDetailedErrors, hubOptions.EnableDetailedErrors);
+            Assert.Equal(globalHubOptions.KeepAliveInterval, hubOptions.KeepAliveInterval);
+            Assert.Equal(globalHubOptions.HandshakeTimeout, hubOptions.HandshakeTimeout);
+            Assert.Equal(globalHubOptions.SupportedProtocols, hubOptions.SupportedProtocols);
+            Assert.Equal(globalHubOptions.ClientTimeoutInterval, hubOptions.ClientTimeoutInterval);
+            Assert.True(hubOptions.UserHasSetValues);
+        }
+
         [Fact]
         public void StreamBufferCapacityGetSet()
         {
@@ -94,6 +118,36 @@ public void StreamBufferCapacityGetSet()
             var serviceProvider = serviceCollection.BuildServiceProvider();
             Assert.Equal(42, serviceProvider.GetRequiredService<IOptions<HubOptions<CustomHub>>>().Value.StreamBufferCapacity);
         }
+
+        [Fact]
+        public void UserSpecifiedOptionsRunAfterDefaultOptions()
+        {
+            var serviceCollection = new ServiceCollection();
+
+            // null is special when the default options setup runs, so we set to null to verify that our options run after the default
+            // setup runs
+            serviceCollection.AddSignalR(options =>
+            {
+                options.MaximumReceiveMessageSize = null;
+                options.StreamBufferCapacity = null;
+                options.EnableDetailedErrors = null;
+                options.KeepAliveInterval = null;
+                options.HandshakeTimeout = null;
+                options.SupportedProtocols = null;
+                options.ClientTimeoutInterval = TimeSpan.FromSeconds(1);
+            });
+
+            var serviceProvider = serviceCollection.BuildServiceProvider();
+
+            var globalOptions = serviceProvider.GetRequiredService<IOptions<HubOptions>>().Value;
+            Assert.Null(globalOptions.MaximumReceiveMessageSize);
+            Assert.Null(globalOptions.StreamBufferCapacity);
+            Assert.Null(globalOptions.EnableDetailedErrors);
+            Assert.Null(globalOptions.KeepAliveInterval);
+            Assert.Null(globalOptions.HandshakeTimeout);
+            Assert.Null(globalOptions.SupportedProtocols);
+            Assert.Equal(TimeSpan.FromSeconds(1), globalOptions.ClientTimeoutInterval);
+        }
     }
 
     public class CustomHub : Hub

src/SignalR/server/SignalR/test/HubConnectionHandlerTests.cs

@@ -3834,6 +3834,38 @@ public async Task ConnectionCloseCleansUploadStreams()
             }
         }
 
+        [Fact]
+        public async Task SpecificHubOptionForMaximumReceiveMessageSizeIsUsedOverGlobalHubOption()
+        {
+            var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(serviceBuilder =>
+            {
+                serviceBuilder.AddSignalR(o =>
+                {
+                    // ConnectAsync would fail if this value was used
+                    o.MaximumReceiveMessageSize = 1;
+                }).AddHubOptions<MethodHub>(o =>
+                {
+                    // null is treated as both no-limit and not set, this test verifies that we track if the user explicitly sets the value
+                    o.MaximumReceiveMessageSize = null;
+                });
+            });
+            var connectionHandler = serviceProvider.GetService<HubConnectionHandler<MethodHub>>();
+
+            using (StartVerifiableLog())
+            {
+                using var client = new TestClient();
+
+                var connectionHandlerTask = await client.ConnectAsync(connectionHandler);
+
+                // Wait for a connection, or for the endpoint to fail.
+                await client.Connected.OrThrowIfOtherFails(connectionHandlerTask).OrTimeout();
+
+                await client.DisposeAsync().OrTimeout();
+
+                await connectionHandlerTask.OrTimeout();
+            }
+        }
+
         private class CustomHubActivator<THub> : IHubActivator<THub> where THub : Hub
         {
             public int ReleaseCount;