From 47b8960ae7a9db6636ca959cbe974f0b07b467ac Mon Sep 17 00:00:00 2001 From: xingsy97 <87063252+xingsy97@users.noreply.github.com> Date: Thu, 14 Nov 2024 16:32:20 +0800 Subject: [PATCH] resolve comments --- .../Auth/AuthUtility.cs | 4 +- .../Interfaces/IClientConnectionManager.cs | 1 - .../ServiceConnectionBase.cs | 2 +- .../HubHost/HeartBeat.cs | 8 ++-- .../HubHost/NegotiateHandler.cs | 13 +++--- .../ServerConnections/ServiceConnection.cs | 24 +++++++---- .../ServiceConnectionFactory.cs | 13 ++++-- .../MockServiceConnectionFactory.cs | 1 + .../Infrastructure/ServiceConnectionProxy.cs | 1 + ...EndpointServiceConnectionContainerTests.cs | 3 ++ .../NegotiateHandlerFacts.cs | 40 ------------------- .../ServiceConnectionTests.cs | 15 +++++-- .../ServiceMessageTests.cs | 1 + 13 files changed, 60 insertions(+), 66 deletions(-) diff --git a/src/Microsoft.Azure.SignalR.Common/Auth/AuthUtility.cs b/src/Microsoft.Azure.SignalR.Common/Auth/AuthUtility.cs index 72804c9a0..6523743f6 100644 --- a/src/Microsoft.Azure.SignalR.Common/Auth/AuthUtility.cs +++ b/src/Microsoft.Azure.SignalR.Common/Auth/AuthUtility.cs @@ -67,6 +67,8 @@ public static string GenerateAccessToken(byte[] keyBytes, public static string GenerateRequestId(string traceIdentifier) { - return $"{traceIdentifier}-{Convert.ToBase64String(BitConverter.GetBytes(Stopwatch.GetTimestamp()))}"; + // Before filled into query string, this id will be process by "WebUtility.UrlEncode(...)". So base64 encoding is not needed. + // Use hex to shorten the length. + return $"{traceIdentifier}:{Stopwatch.GetTimestamp().ToString("X")}"; } } diff --git a/src/Microsoft.Azure.SignalR.Common/Interfaces/IClientConnectionManager.cs b/src/Microsoft.Azure.SignalR.Common/Interfaces/IClientConnectionManager.cs index 0337e2005..ff33aee71 100644 --- a/src/Microsoft.Azure.SignalR.Common/Interfaces/IClientConnectionManager.cs +++ b/src/Microsoft.Azure.SignalR.Common/Interfaces/IClientConnectionManager.cs @@ -2,7 +2,6 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Generic; -using System.Globalization; using System.Threading.Tasks; namespace Microsoft.Azure.SignalR; diff --git a/src/Microsoft.Azure.SignalR.Common/ServiceConnections/ServiceConnectionBase.cs b/src/Microsoft.Azure.SignalR.Common/ServiceConnections/ServiceConnectionBase.cs index e5bf683a9..9472e3614 100644 --- a/src/Microsoft.Azure.SignalR.Common/ServiceConnections/ServiceConnectionBase.cs +++ b/src/Microsoft.Azure.SignalR.Common/ServiceConnections/ServiceConnectionBase.cs @@ -99,7 +99,7 @@ protected set public string ConnectionId { get; } - protected HubServiceEndpoint HubEndpoint { get; } + public HubServiceEndpoint HubEndpoint { get; } protected ILogger Logger { get; } diff --git a/src/Microsoft.Azure.SignalR/HubHost/HeartBeat.cs b/src/Microsoft.Azure.SignalR/HubHost/HeartBeat.cs index de218230f..69931153e 100644 --- a/src/Microsoft.Azure.SignalR/HubHost/HeartBeat.cs +++ b/src/Microsoft.Azure.SignalR/HubHost/HeartBeat.cs @@ -13,14 +13,14 @@ internal class HeartBeat : BackgroundService private static readonly TimeSpan HeartbeatTickRate = TimeSpan.FromSeconds(1); private readonly IClientConnectionManager _connectionManager; - private readonly ICultureFeatureManager _cultureInfoManager; + private readonly ICultureFeatureManager _cultureFeatureManager; private readonly TimerAwaitable _nextHeartbeat; - public HeartBeat(IClientConnectionManager connectionManager, ICultureFeatureManager cultureInfoManager) + public HeartBeat(IClientConnectionManager connectionManager, ICultureFeatureManager cultureFeatureManager) { _connectionManager = connectionManager; - _cultureInfoManager = cultureInfoManager; + _cultureFeatureManager = cultureFeatureManager; _nextHeartbeat = new TimerAwaitable(HeartbeatTickRate, HeartbeatTickRate); } @@ -52,7 +52,7 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) { (connection as ClientConnectionContext).TickHeartbeat(); } - _cultureInfoManager.Cleanup(); + _cultureFeatureManager.Cleanup(); } } } diff --git a/src/Microsoft.Azure.SignalR/HubHost/NegotiateHandler.cs b/src/Microsoft.Azure.SignalR/HubHost/NegotiateHandler.cs index cfa9cc352..7b9cea7b3 100644 --- a/src/Microsoft.Azure.SignalR/HubHost/NegotiateHandler.cs +++ b/src/Microsoft.Azure.SignalR/HubHost/NegotiateHandler.cs @@ -38,7 +38,7 @@ internal class NegotiateHandler where THub : Hub #if NET6_0_OR_GREATER private readonly HttpConnectionDispatcherOptions _dispatcherOptions; #endif - private readonly ICultureFeatureManager _cultureInfoManager; + private readonly ICultureFeatureManager _cultureFeatureManager; public NegotiateHandler( IOptions globalHubOptions, @@ -53,8 +53,8 @@ public NegotiateHandler( #if NET6_0_OR_GREATER EndpointDataSource endpointDataSource, #endif - ILogger> logger, - ICultureFeatureManager cultureInfoManager) + ICultureFeatureManager cultureFeatureManager, + ILogger> logger) { _logger = logger ?? throw new ArgumentNullException(nameof(logger)); _endpointManager = endpointManager ?? throw new ArgumentNullException(nameof(endpointManager)); @@ -75,7 +75,7 @@ public NegotiateHandler( #if NET6_0_OR_GREATER _dispatcherOptions = GetDispatcherOptions(endpointDataSource, typeof(THub)); #endif - _cultureInfoManager = cultureInfoManager ?? throw new ArgumentNullException(nameof(cultureInfoManager)); + _cultureFeatureManager = cultureFeatureManager ?? throw new ArgumentNullException(nameof(cultureFeatureManager)); } public async Task Process(HttpContext context) @@ -97,7 +97,10 @@ public async Task Process(HttpContext context) clientRequestId ); - _cultureInfoManager.TryAddCultureFeature(clientRequestId, cultureFeature); + if (_blazorDetector.IsBlazor(_hubName)) + { + _cultureFeatureManager.TryAddCultureFeature(clientRequestId, cultureFeature); + } return new NegotiationResponse { diff --git a/src/Microsoft.Azure.SignalR/ServerConnections/ServiceConnection.cs b/src/Microsoft.Azure.SignalR/ServerConnections/ServiceConnection.cs index 7da4cc496..84075cd99 100644 --- a/src/Microsoft.Azure.SignalR/ServerConnections/ServiceConnection.cs +++ b/src/Microsoft.Azure.SignalR/ServerConnections/ServiceConnection.cs @@ -48,7 +48,9 @@ internal partial class ServiceConnection : ServiceConnectionBase private readonly IHubProtocolResolver _hubProtocolResolver; - private readonly ICultureFeatureManager _cultureInfoManager; + private readonly IBlazorDetector _blazorDetector; + + private readonly ICultureFeatureManager _cultureFeatureManager; public Action ConfigureContext { get; set; } @@ -65,7 +67,8 @@ public ServiceConnection(IServiceProtocol serviceProtocol, IServiceEventHandler serviceEventHandler, IClientInvocationManager clientInvocationManager, IHubProtocolResolver hubProtocolResolver, - ICultureFeatureManager cultureInfoManager, + IBlazorDetector blazorDetector, + ICultureFeatureManager cultureFeatureManager, ServiceConnectionType connectionType = ServiceConnectionType.Default, GracefulShutdownMode mode = GracefulShutdownMode.Off, bool allowStatefulReconnects = false) @@ -87,7 +90,8 @@ public ServiceConnection(IServiceProtocol serviceProtocol, _clientConnectionFactory = clientConnectionFactory; _clientInvocationManager = clientInvocationManager; _hubProtocolResolver = hubProtocolResolver; - _cultureInfoManager = cultureInfoManager; + _blazorDetector = blazorDetector; + _cultureFeatureManager = cultureFeatureManager; } public override bool TryAddClientConnection(IClientConnection connection) @@ -157,11 +161,17 @@ protected override Task OnClientConnectedAsync(OpenConnectionMessage message) connection.Features.Set(null); - if (!_cultureInfoManager.TryRemoveCultureFeature(connection.RequestId, out var cultureFeature)) - { - Log.FailedToApplyCultureInfo(Logger, connection.RequestId); + if (_blazorDetector.IsBlazor(HubEndpoint.Hub)) { + if (_cultureFeatureManager.TryRemoveCultureFeature(connection.RequestId, out var cultureFeature)) + { + CultureInfo.CurrentCulture = cultureFeature.RequestCulture.Culture; + CultureInfo.CurrentUICulture = cultureFeature.RequestCulture.UICulture; + } + else + { + Log.FailedToApplyCultureInfo(Logger, connection.RequestId); + } } - connection.GetHttpContext().Features.Set(cultureFeature); if (message.Headers.TryGetValue(Constants.AsrsMigrateFrom, out var from)) { diff --git a/src/Microsoft.Azure.SignalR/ServerConnections/ServiceConnectionFactory.cs b/src/Microsoft.Azure.SignalR/ServerConnections/ServiceConnectionFactory.cs index 328b3edad..5caf4691f 100644 --- a/src/Microsoft.Azure.SignalR/ServerConnections/ServiceConnectionFactory.cs +++ b/src/Microsoft.Azure.SignalR/ServerConnections/ServiceConnectionFactory.cs @@ -29,7 +29,9 @@ internal class ServiceConnectionFactory : IServiceConnectionFactory private readonly IHubProtocolResolver _hubProtocolResolver; - private readonly ICultureFeatureManager _cultureInfoManager; + private readonly IBlazorDetector _blazorDetector; + + private readonly ICultureFeatureManager _cultureFeatureManager; public GracefulShutdownMode ShutdownMode { get; set; } = GracefulShutdownMode.Off; @@ -48,7 +50,8 @@ public ServiceConnectionFactory( IServiceEventHandler serviceEventHandler, IClientInvocationManager clientInvocationManager, IHubProtocolResolver hubProtocolResolver, - ICultureFeatureManager cultureInfoManager) + IBlazorDetector blazorDetector, + ICultureFeatureManager cultureFeatureManager) { _serviceProtocol = serviceProtocol; _clientConnectionManager = clientConnectionManager; @@ -60,7 +63,8 @@ public ServiceConnectionFactory( _serviceEventHandler = serviceEventHandler; _clientInvocationManager = clientInvocationManager; _hubProtocolResolver = hubProtocolResolver; - _cultureInfoManager = cultureInfoManager; + _blazorDetector = blazorDetector; + _cultureFeatureManager = cultureFeatureManager; } public virtual IServiceConnection Create(HubServiceEndpoint endpoint, IServiceMessageHandler serviceMessageHandler, AckHandler ackHandler, ServiceConnectionType type) @@ -79,7 +83,8 @@ public virtual IServiceConnection Create(HubServiceEndpoint endpoint, IServiceMe _serviceEventHandler, _clientInvocationManager, _hubProtocolResolver, - _cultureInfoManager, + _blazorDetector, + _cultureFeatureManager, type, ShutdownMode, allowStatefulReconnects: AllowStatefulReconnects diff --git a/test/Microsoft.Azure.SignalR.IntegrationTests/Infrastructure/MockServiceConnectionFactory.cs b/test/Microsoft.Azure.SignalR.IntegrationTests/Infrastructure/MockServiceConnectionFactory.cs index f44f8ba33..75c010835 100644 --- a/test/Microsoft.Azure.SignalR.IntegrationTests/Infrastructure/MockServiceConnectionFactory.cs +++ b/test/Microsoft.Azure.SignalR.IntegrationTests/Infrastructure/MockServiceConnectionFactory.cs @@ -34,6 +34,7 @@ public MockServiceConnectionFactory( null, clientInvocationManager, hubProtocolResolver, + null, null) { _mockService = mockService; diff --git a/test/Microsoft.Azure.SignalR.Tests/Infrastructure/ServiceConnectionProxy.cs b/test/Microsoft.Azure.SignalR.Tests/Infrastructure/ServiceConnectionProxy.cs index b273745f6..b7197127d 100644 --- a/test/Microsoft.Azure.SignalR.Tests/Infrastructure/ServiceConnectionProxy.cs +++ b/test/Microsoft.Azure.SignalR.Tests/Infrastructure/ServiceConnectionProxy.cs @@ -121,6 +121,7 @@ public IServiceConnection Create(HubServiceEndpoint endpoint, ClientInvocationManager, new DefaultHubProtocolResolver(new[] { new JsonHubProtocol() }, NullLogger.Instance), null, + null, type, allowStatefulReconnects: AllowStatefulReconnects); ServiceConnections.TryAdd(connectionId, connection); diff --git a/test/Microsoft.Azure.SignalR.Tests/MultiEndpointServiceConnectionContainerTests.cs b/test/Microsoft.Azure.SignalR.Tests/MultiEndpointServiceConnectionContainerTests.cs index dd7912255..f8c8f9143 100644 --- a/test/Microsoft.Azure.SignalR.Tests/MultiEndpointServiceConnectionContainerTests.cs +++ b/test/Microsoft.Azure.SignalR.Tests/MultiEndpointServiceConnectionContainerTests.cs @@ -1657,6 +1657,7 @@ public async Task ServiceConnectionContainerScopeWithPingUpdateTest() null, clientInvocationManager, hubProtocolResolver, + null, null); var connection2 = new ServiceConnection(protocol, @@ -1672,6 +1673,7 @@ public async Task ServiceConnectionContainerScopeWithPingUpdateTest() null, clientInvocationManager, hubProtocolResolver, + null, null); var connection22 = new ServiceConnection(protocol, @@ -1687,6 +1689,7 @@ public async Task ServiceConnectionContainerScopeWithPingUpdateTest() null, clientInvocationManager, hubProtocolResolver, + null, null); var router = new TestEndpointRouter(); diff --git a/test/Microsoft.Azure.SignalR.Tests/NegotiateHandlerFacts.cs b/test/Microsoft.Azure.SignalR.Tests/NegotiateHandlerFacts.cs index b648492bc..899e0f68c 100644 --- a/test/Microsoft.Azure.SignalR.Tests/NegotiateHandlerFacts.cs +++ b/test/Microsoft.Azure.SignalR.Tests/NegotiateHandlerFacts.cs @@ -435,46 +435,6 @@ public async Task TestNegotiateHandlerWithMultipleEndpointsAndCustomRouter() await Assert.ThrowsAsync(() => handler.Process(httpContext)); } - [Fact] - public async Task TestNegotiateHandlerRespectClientRequestCulture() - { - var config = new ConfigurationBuilder().Build(); - var serviceProvider = new ServiceCollection() - .AddSignalR(o => o.EnableDetailedErrors = false) - .AddAzureSignalR( - o => - { - o.ConnectionString = DefaultConnectionString; - o.AccessTokenLifetime = TimeSpan.FromDays(1); - }) - .Services - .AddLogging() - .AddSingleton(config) - .BuildServiceProvider(); - - var features = new FeatureCollection(); - var requestFeature = new HttpRequestFeature - { - Path = "/user/path/negotiate/", - QueryString = "?endpoint=chosen" - }; - features.Set(requestFeature); - var customCulture = new RequestCulture("ar-SA", "en-US"); - features.Set( - new RequestCultureFeature(customCulture, - new AcceptLanguageHeaderRequestCultureProvider())); - - var httpContext = new DefaultHttpContext(features); - - var handler = serviceProvider.GetRequiredService>(); - var negotiateResponse = await handler.Process(httpContext); - - var queryContainsCulture = negotiateResponse.Url.Contains($"{Constants.QueryParameter.RequestCulture}=ar-SA"); - var queryContainsUICulture = negotiateResponse.Url.Contains($"{Constants.QueryParameter.RequestUICulture}=en-US"); - Assert.True(queryContainsCulture); - Assert.True(queryContainsUICulture); - } - [Theory] [InlineData(-10)] [InlineData(0)] diff --git a/test/Microsoft.Azure.SignalR.Tests/ServiceConnectionTests.cs b/test/Microsoft.Azure.SignalR.Tests/ServiceConnectionTests.cs index b89b0c09d..6b5f559c4 100644 --- a/test/Microsoft.Azure.SignalR.Tests/ServiceConnectionTests.cs +++ b/test/Microsoft.Azure.SignalR.Tests/ServiceConnectionTests.cs @@ -198,7 +198,7 @@ public async Task TestServiceConnectionWithNormalApplicationTask() var connection = new ServiceConnection( protocol, ccm, connectionFactory, loggerFactory, handler, ccf, "serverId", Guid.NewGuid().ToString("N"), null, null, null, new DefaultClientInvocationManager(), - new DefaultHubProtocolResolver(new[] { hubProtocol }, NullLogger.Instance), null); + new DefaultHubProtocolResolver(new[] { hubProtocol }, NullLogger.Instance), null, null); var connectionTask = connection.StartAsync(); @@ -258,7 +258,7 @@ public async Task TestServiceConnectionErrorCleansAllClients() var connection = new ServiceConnection( protocol, ccm, connectionFactory, loggerFactory, handler, ccf, "serverId", Guid.NewGuid().ToString("N"), null, null, null, new DefaultClientInvocationManager(), - new DefaultHubProtocolResolver(new[] { hubProtocol }, NullLogger.Instance), null); + new DefaultHubProtocolResolver(new[] { hubProtocol }, NullLogger.Instance), null, null); var connectionTask = connection.StartAsync(); @@ -317,7 +317,7 @@ public async Task TestServiceConnectionWithErrorApplicationTask() var connection = new ServiceConnection( protocol, ccm, connectionFactory, loggerFactory, handler, ccf, "serverId", Guid.NewGuid().ToString("N"), null, null, null, new DefaultClientInvocationManager(), - new DefaultHubProtocolResolver(new[] { hubProtocol }, NullLogger.Instance), null); + new DefaultHubProtocolResolver(new[] { hubProtocol }, NullLogger.Instance), null, null); var connectionTask = connection.StartAsync(); @@ -393,6 +393,7 @@ public async Task TestServiceConnectionWithEndlessApplicationTaskNeverEnds() null, new DefaultClientInvocationManager(), hubProtocolResolver, + null, null); var connectionTask = connection.StartAsync(); @@ -462,6 +463,7 @@ public async Task TestClientConnectionOutgoingAbortCanEndLifeTime() null, new DefaultClientInvocationManager(), hubProtocolResolver, + null, null); var connectionTask = connection.StartAsync(); @@ -530,6 +532,7 @@ public async Task TestClientConnectionContextAbortCanSendOutCloseMessage() null, new DefaultClientInvocationManager(), hubProtocolResolver, + null, null); var connectionTask = connection.StartAsync(); @@ -613,6 +616,7 @@ public async Task TestClientConnectionWithDiagnosticClientTagTest() null, new DefaultClientInvocationManager(), defaultHubProtocolResolver, + null, null); var connectionTask = connection.StartAsync(); @@ -690,6 +694,7 @@ public async Task TestClientConnectionShouldSkipHandshakeWhenMigrateIn(string he null, new DefaultClientInvocationManager(), defaultHubProtocolResolver, + null, null); var connectionTask = connection.StartAsync(); @@ -779,6 +784,7 @@ public async Task TestClientConnectionLastWillCanSendOut() null, new DefaultClientInvocationManager(), hubProtocolResolver, + null, null); var connectionTask = connection.StartAsync(); @@ -848,6 +854,7 @@ public async Task TestPartialMessagesShouldFlushCorrectly() null, new DefaultClientInvocationManager(), hubProcotolResolver, + null, null); var connectionTask = connection.StartAsync().OrTimeout(); @@ -947,6 +954,7 @@ public async Task TestPartialMessagesShouldBeRemovedWhenReconnected() null, new DefaultClientInvocationManager(), hubProtocolResolver, + null, null); var connectionTask = connection.StartAsync(); @@ -1028,6 +1036,7 @@ private static ServiceConnection CreateServiceConnection(ServiceProtocol protoco null, new DefaultClientInvocationManager(), new DefaultHubProtocolResolver(new[] { hubProtocol }, NullLogger.Instance), + null, null); } diff --git a/test/Microsoft.Azure.SignalR.Tests/ServiceMessageTests.cs b/test/Microsoft.Azure.SignalR.Tests/ServiceMessageTests.cs index f6e8101ed..d296e478d 100644 --- a/test/Microsoft.Azure.SignalR.Tests/ServiceMessageTests.cs +++ b/test/Microsoft.Azure.SignalR.Tests/ServiceMessageTests.cs @@ -508,6 +508,7 @@ public TestServiceConnection(TestConnectionContainer container, clientInvocationManager, hubProtocolResolver, null, + null, connectionType: connectionType, mode: mode) {