Skip to content

Commit

Permalink
resolve comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@xingsy97
xingsy97 committed Nov 14, 2024
1 parent 2152f07 commit 47b8960
Show file tree
Hide file tree
Showing 13 changed files with 60 additions and 66 deletions.
4 changes: 3 additions & 1 deletion src/Microsoft.Azure.SignalR.Common/Auth/AuthUtility.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")}";
}
}
1 change: 0 additions & 1 deletion src/Microsoft.Azure.SignalR.Common/Interfaces/IClientConnectionManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.Azure.SignalR.Common/ServiceConnections/ServiceConnectionBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ protected set

public string ConnectionId { get; }

protected HubServiceEndpoint HubEndpoint { get; }
public HubServiceEndpoint HubEndpoint { get; }

protected ILogger Logger { get; }

Expand Down
8 changes: 4 additions & 4 deletions src/Microsoft.Azure.SignalR/HubHost/HeartBeat.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -52,7 +52,7 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
{
(connection as ClientConnectionContext).TickHeartbeat();
}
_cultureInfoManager.Cleanup();
_cultureFeatureManager.Cleanup();
}
}
}
Expand Down
13 changes: 8 additions & 5 deletions src/Microsoft.Azure.SignalR/HubHost/NegotiateHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ internal class NegotiateHandler<THub> where THub : Hub
#if NET6_0_OR_GREATER
private readonly HttpConnectionDispatcherOptions _dispatcherOptions;
#endif
private readonly ICultureFeatureManager _cultureInfoManager;
private readonly ICultureFeatureManager _cultureFeatureManager;

public NegotiateHandler(
IOptions<HubOptions> globalHubOptions,
Expand All @@ -53,8 +53,8 @@ public NegotiateHandler(
#if NET6_0_OR_GREATER
EndpointDataSource endpointDataSource,
#endif
ILogger<NegotiateHandler<THub>> logger,
ICultureFeatureManager cultureInfoManager)
ICultureFeatureManager cultureFeatureManager,
ILogger<NegotiateHandler<THub>> logger)
{
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
_endpointManager = endpointManager ?? throw new ArgumentNullException(nameof(endpointManager));
Expand All @@ -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<NegotiationResponse> Process(HttpContext context)
Expand All @@ -97,7 +97,10 @@ public async Task<NegotiationResponse> Process(HttpContext context)
clientRequestId
);

_cultureInfoManager.TryAddCultureFeature(clientRequestId, cultureFeature);
if (_blazorDetector.IsBlazor(_hubName))
{
_cultureFeatureManager.TryAddCultureFeature(clientRequestId, cultureFeature);
}

return new NegotiationResponse
{
Expand Down
24 changes: 17 additions & 7 deletions src/Microsoft.Azure.SignalR/ServerConnections/ServiceConnection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<HttpContext> ConfigureContext { get; set; }

Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -157,11 +161,17 @@ protected override Task OnClientConnectedAsync(OpenConnectionMessage message)

connection.Features.Set<IConnectionMigrationFeature>(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<IRequestCultureFeature>(cultureFeature);

if (message.Headers.TryGetValue(Constants.AsrsMigrateFrom, out var from))
{
Expand Down
13 changes: 9 additions & 4 deletions src/Microsoft.Azure.SignalR/ServerConnections/ServiceConnectionFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -48,7 +50,8 @@ public ServiceConnectionFactory(
IServiceEventHandler serviceEventHandler,
IClientInvocationManager clientInvocationManager,
IHubProtocolResolver hubProtocolResolver,
ICultureFeatureManager cultureInfoManager)
IBlazorDetector blazorDetector,
ICultureFeatureManager cultureFeatureManager)
{
_serviceProtocol = serviceProtocol;
_clientConnectionManager = clientConnectionManager;
Expand All @@ -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)
Expand All @@ -79,7 +83,8 @@ public virtual IServiceConnection Create(HubServiceEndpoint endpoint, IServiceMe
_serviceEventHandler,
_clientInvocationManager,
_hubProtocolResolver,
_cultureInfoManager,
_blazorDetector,
_cultureFeatureManager,
type,
ShutdownMode,
allowStatefulReconnects: AllowStatefulReconnects
Expand Down
1 change: 1 addition & 0 deletions test/Microsoft.Azure.SignalR.IntegrationTests/Infrastructure/MockServiceConnectionFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public MockServiceConnectionFactory(
null,
clientInvocationManager,
hubProtocolResolver,
null,
null)
{
_mockService = mockService;
Expand Down
1 change: 1 addition & 0 deletions test/Microsoft.Azure.SignalR.Tests/Infrastructure/ServiceConnectionProxy.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ public IServiceConnection Create(HubServiceEndpoint endpoint,
ClientInvocationManager,
new DefaultHubProtocolResolver(new[] { new JsonHubProtocol() }, NullLogger<DefaultHubProtocolResolver>.Instance),
null,
null,
type,
allowStatefulReconnects: AllowStatefulReconnects);
ServiceConnections.TryAdd(connectionId, connection);
Expand Down
3 changes: 3 additions & 0 deletions test/Microsoft.Azure.SignalR.Tests/MultiEndpointServiceConnectionContainerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1657,6 +1657,7 @@ public async Task ServiceConnectionContainerScopeWithPingUpdateTest()
null,
clientInvocationManager,
hubProtocolResolver,
null,
null);

var connection2 = new ServiceConnection(protocol,
Expand All @@ -1672,6 +1673,7 @@ public async Task ServiceConnectionContainerScopeWithPingUpdateTest()
null,
clientInvocationManager,
hubProtocolResolver,
null,
null);

var connection22 = new ServiceConnection(protocol,
Expand All @@ -1687,6 +1689,7 @@ public async Task ServiceConnectionContainerScopeWithPingUpdateTest()
null,
clientInvocationManager,
hubProtocolResolver,
null,
null);

var router = new TestEndpointRouter();
Expand Down
40 changes: 0 additions & 40 deletions test/Microsoft.Azure.SignalR.Tests/NegotiateHandlerFacts.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -435,46 +435,6 @@ public async Task TestNegotiateHandlerWithMultipleEndpointsAndCustomRouter()
await Assert.ThrowsAsync<InvalidOperationException>(() => 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<IConfiguration>(config)
.BuildServiceProvider();

var features = new FeatureCollection();
var requestFeature = new HttpRequestFeature
{
Path = "/user/path/negotiate/",
QueryString = "?endpoint=chosen"
};
features.Set<IHttpRequestFeature>(requestFeature);
var customCulture = new RequestCulture("ar-SA", "en-US");
features.Set<IRequestCultureFeature>(
new RequestCultureFeature(customCulture,
new AcceptLanguageHeaderRequestCultureProvider()));

var httpContext = new DefaultHttpContext(features);

var handler = serviceProvider.GetRequiredService<NegotiateHandler<Hub>>();
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)]
Expand Down
15 changes: 12 additions & 3 deletions test/Microsoft.Azure.SignalR.Tests/ServiceConnectionTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<DefaultHubProtocolResolver>.Instance), null);
new DefaultHubProtocolResolver(new[] { hubProtocol }, NullLogger<DefaultHubProtocolResolver>.Instance), null, null);

var connectionTask = connection.StartAsync();

Expand Down Expand Up @@ -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<DefaultHubProtocolResolver>.Instance), null);
new DefaultHubProtocolResolver(new[] { hubProtocol }, NullLogger<DefaultHubProtocolResolver>.Instance), null, null);

var connectionTask = connection.StartAsync();

Expand Down Expand Up @@ -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<DefaultHubProtocolResolver>.Instance), null);
new DefaultHubProtocolResolver(new[] { hubProtocol }, NullLogger<DefaultHubProtocolResolver>.Instance), null, null);

var connectionTask = connection.StartAsync();

Expand Down Expand Up @@ -393,6 +393,7 @@ public async Task TestServiceConnectionWithEndlessApplicationTaskNeverEnds()
null,
new DefaultClientInvocationManager(),
hubProtocolResolver,
null,
null);

var connectionTask = connection.StartAsync();
Expand Down Expand Up @@ -462,6 +463,7 @@ public async Task TestClientConnectionOutgoingAbortCanEndLifeTime()
null,
new DefaultClientInvocationManager(),
hubProtocolResolver,
null,
null);

var connectionTask = connection.StartAsync();
Expand Down Expand Up @@ -530,6 +532,7 @@ public async Task TestClientConnectionContextAbortCanSendOutCloseMessage()
null,
new DefaultClientInvocationManager(),
hubProtocolResolver,
null,
null);

var connectionTask = connection.StartAsync();
Expand Down Expand Up @@ -613,6 +616,7 @@ public async Task TestClientConnectionWithDiagnosticClientTagTest()
null,
new DefaultClientInvocationManager(),
defaultHubProtocolResolver,
null,
null);

var connectionTask = connection.StartAsync();
Expand Down Expand Up @@ -690,6 +694,7 @@ public async Task TestClientConnectionShouldSkipHandshakeWhenMigrateIn(string he
null,
new DefaultClientInvocationManager(),
defaultHubProtocolResolver,
null,
null);

var connectionTask = connection.StartAsync();
Expand Down Expand Up @@ -779,6 +784,7 @@ public async Task TestClientConnectionLastWillCanSendOut()
null,
new DefaultClientInvocationManager(),
hubProtocolResolver,
null,
null);

var connectionTask = connection.StartAsync();
Expand Down Expand Up @@ -848,6 +854,7 @@ public async Task TestPartialMessagesShouldFlushCorrectly()
null,
new DefaultClientInvocationManager(),
hubProcotolResolver,
null,
null);

var connectionTask = connection.StartAsync().OrTimeout();
Expand Down Expand Up @@ -947,6 +954,7 @@ public async Task TestPartialMessagesShouldBeRemovedWhenReconnected()
null,
new DefaultClientInvocationManager(),
hubProtocolResolver,
null,
null);

var connectionTask = connection.StartAsync();
Expand Down Expand Up @@ -1028,6 +1036,7 @@ private static ServiceConnection CreateServiceConnection(ServiceProtocol protoco
null,
new DefaultClientInvocationManager(),
new DefaultHubProtocolResolver(new[] { hubProtocol }, NullLogger<DefaultHubProtocolResolver>.Instance),
null,
null);
}

Expand Down
1 change: 1 addition & 0 deletions test/Microsoft.Azure.SignalR.Tests/ServiceMessageTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,7 @@ public TestServiceConnection(TestConnectionContainer container,
clientInvocationManager,
hubProtocolResolver,
null,
null,
connectionType: connectionType,
mode: mode)
{
Expand Down

0 comments on commit 47b8960

Please sign in to comment.