From c851815a2ca69ce20d766b1eb28ef1a72bce50b5 Mon Sep 17 00:00:00 2001 From: Dmitri Shimanski Date: Sun, 25 May 2025 03:32:33 +0300 Subject: [PATCH] Refactor WebSocket routing and error handling logic Replaces `Dictionary` with `ConcurrentDictionary` for thread-safe WebSocket route management and improves error logging with added debug assertions. Also fixes duplicate registrations, enhances dependency injection, updates package references, and adjusts WebSocket attribute structure for better extensibility and usage. --- Examples/Program.cs | 2 + LICENSE | 2 +- README.md | 20 +++++ .../Core/WebSocketManagerTest.cs | 79 +++++++++---------- .../yawaflua.WebSockets.Tests.csproj | 2 + .../Attributes/WebSocketAttribute.cs | 4 +- yawaflua.WebSockets/Core/WebSocketRouter.cs | 76 ++++++++++++++---- .../Models/Abstracts/WebSocketController.cs | 2 +- yawaflua.WebSockets/ServiceBindings.cs | 2 + .../yawaflua.WebSockets.csproj | 3 +- 10 files changed, 131 insertions(+), 61 deletions(-) diff --git a/Examples/Program.cs b/Examples/Program.cs index af36673..717e630 100644 --- a/Examples/Program.cs +++ b/Examples/Program.cs @@ -46,6 +46,8 @@ internal class Startup services.AddScoped(_ => new ConfigurationBuilder() .AddJsonFile("appsettings.json", true) .Build()); + + } public static void Configure(IApplicationBuilder app) diff --git a/LICENSE b/LICENSE index 36deabc..32d7807 100644 --- a/LICENSE +++ b/LICENSE @@ -186,7 +186,7 @@ same "printed page" as the copyright notice for easier identification within third-party archives. - Copyright [2025] [Dmitrii Shimanskii] + Copyright [2025] [Dmitri Shimanski] Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/README.md b/README.md index df5391a..a6a81fa 100644 --- a/README.md +++ b/README.md @@ -129,6 +129,26 @@ services.AddSingleton(new WebSocketConfig() }) ``` +## Work with c=connected users from any point at your code! +```csharp +public class MyCoolService +{ + private IWebSocketManager _manager; + public MyCoolService(IWebSocketManager manager) + { + _manager = manager; + } + + public async Task DoSomething() + { + await _manager.Broadcast(k => k.Path == "/my/cool/endpoint", "Hello!"); + } +} + +// DependencyInjection should provide IWebSocketManager to builder +services.AddSingleton(); +``` + ## Lifecycle Management 1. **Connection** - Automatically handled by middleware diff --git a/yawaflua.WebSockets.Tests/Core/WebSocketManagerTest.cs b/yawaflua.WebSockets.Tests/Core/WebSocketManagerTest.cs index d903c81..abd6b8d 100644 --- a/yawaflua.WebSockets.Tests/Core/WebSocketManagerTest.cs +++ b/yawaflua.WebSockets.Tests/Core/WebSocketManagerTest.cs @@ -1,10 +1,12 @@ using yawaflua.WebSockets.Core; using System; +using System.Diagnostics; using System.Net.WebSockets; using System.Threading; using System.Threading.Tasks; using JetBrains.Annotations; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Moq; @@ -25,16 +27,29 @@ public class WebSocketRouterTests private readonly Mock _serviceProviderMock = new(); private readonly Mock> _loggerMock = new(); private IServiceCollection _services; + private static WebSocketRouter _router; public WebSocketRouterTests() { _services = new ServiceCollection(); + _services.AddSingleton(_ => new ConfigurationBuilder().Build() as IConfiguration); _serviceProviderMock.Setup(k => k.GetService(typeof(IServiceScopeFactory))) .Returns(_services.BuildServiceProvider().CreateScope()); + + _services.AddTransient(); + _services.AddSingleton(_ => _loggerMock.Object); + _services.SettingUpWebSockets(new WebSocketConfig()); + _router ??= _services.BuildServiceProvider().GetService(); } [yawaflua.WebSockets.Attributes.WebSocket("/test")] public class TestHandler : WebSocketController { + [CanBeNull] internal static IConfiguration Configuration { get; set; } + public TestHandler(IConfiguration configuration) + { + Configuration ??= configuration; + Debug.WriteLine("Hi!"); + } [yawaflua.WebSockets.Attributes.WebSocket("/static")] public static Task StaticHandler(IWebSocket ws, HttpContext context) => Task.CompletedTask; @@ -42,23 +57,30 @@ public class WebSocketRouterTests public Task InstanceHandler(IWebSocket ws, HttpContext context) => Task.CompletedTask; } + [Fact] + public void DiscoverHandlers_ShouldRegisterStaticVars() + { + // Assert + Assert.NotNull(TestHandler.Configuration); + } + + [Fact] + public void DiscoverHandlers_ShouldRegisterWebSocketManager() + { + // Assert + Assert.NotNull(_services.BuildServiceProvider().GetService()); + } + [Fact] public void DiscoverHandlers_ShouldRegisterStaticAndInstanceMethods() { - // Arrange - _services.AddTransient(); - _serviceProviderMock.Setup(x => x.GetService(typeof(TestHandler))) - .Returns(new TestHandler()); - // Act - var router = new WebSocketRouter(_serviceProviderMock.Object, _loggerMock.Object); - // Assert Assert.True(WebSocketRouter.Routes.ContainsKey("/test/static")); Assert.True(WebSocketRouter.Routes.ContainsKey("/test/instance")); } [Fact] - public async Task HandleRequest_ShouldAcceptWebSocketAndAddClient() + public async Task HandleRequest_ShouldAcceptWebSocketAndRemoveClientOnClose() { // Arrange var webSocketMock = new Mock(); @@ -69,18 +91,17 @@ public class WebSocketRouterTests .ReturnsAsync(webSocketMock.Object); webSocketManagerMock.Setup(m => m.IsWebSocketRequest) .Returns(true); + contextMock.SetupGet(c => c.Connection.RemoteIpAddress).Returns(new System.Net.IPAddress(new byte[] { 127, 0, 0, 1 })); contextMock.SetupGet(c => c.WebSockets).Returns(webSocketManagerMock.Object); contextMock.SetupGet(c => c.Request.Path).Returns(new PathString("/test/static")); contextMock.Setup(c => c.RequestServices) - .Returns(_serviceProviderMock.Object); - - var router = new WebSocketRouter(_services.BuildServiceProvider(), _loggerMock.Object); + .Returns(_services.BuildServiceProvider()); // Act - await router.HandleRequest(contextMock.Object); + await _router.HandleRequest(contextMock.Object); // Assert - Assert.Single(WebSocketRouter.Clients); + Assert.Empty(WebSocketRouter.Clients); // Because clients should be clean after exit } [Fact] @@ -92,40 +113,18 @@ public class WebSocketRouterTests var webSocketManagerMock = new Mock(); webSocketManagerMock.Setup(m => m.IsWebSocketRequest).Returns(true); + contextMock.SetupGet(c => c.Connection.RemoteIpAddress).Returns(new System.Net.IPAddress(new byte[] { 127, 0, 0, 1 })); contextMock.SetupGet(c => c.WebSockets).Returns(webSocketManagerMock.Object); contextMock.SetupGet(c => c.Request.Path).Returns(new PathString("/unknown")); contextMock.SetupGet(c => c.Response).Returns(responseMock.Object); - var router = new WebSocketRouter(_services.BuildServiceProvider(), _loggerMock.Object); - // Act - await router.HandleRequest(contextMock.Object); + await _router.HandleRequest(contextMock.Object); // Assert responseMock.VerifySet(r => r.StatusCode = 404); } - - [Fact] - public void DiscoverHandlers_ShouldLogErrorOnInvalidHandler() - { - // Arrange - var invalidHandlerType = typeof(InvalidHandler); - _serviceProviderMock.Setup(x => x.GetService(invalidHandlerType)) - .Throws(new InvalidOperationException()); - - // Act - var router = new WebSocketRouter(_serviceProviderMock.Object, _loggerMock.Object); - - // Assert - _loggerMock.Verify( - x => x.Log( - LogLevel.Critical, - It.IsAny(), - It.IsAny(), - It.IsAny(), - (Func)It.IsAny()), - Times.AtLeastOnce); - } + [WebSocket("/invalid")] public class InvalidHandler : WebSocketController @@ -151,10 +150,8 @@ public class WebSocketRouterTests contextMock.SetupGet(c => c.Request.Path).Returns(new PathString("/test/static")); contextMock.Setup(c => c.RequestServices).Returns(_serviceProviderMock.Object); - var router = new WebSocketRouter(_serviceProviderMock.Object, _loggerMock.Object); - // Act - await router.HandleRequest(contextMock.Object); + await _router.HandleRequest(contextMock.Object); await Task.Delay(100); // Allow background task to complete // Assert diff --git a/yawaflua.WebSockets.Tests/yawaflua.WebSockets.Tests.csproj b/yawaflua.WebSockets.Tests/yawaflua.WebSockets.Tests.csproj index 91fabae..d75d631 100644 --- a/yawaflua.WebSockets.Tests/yawaflua.WebSockets.Tests.csproj +++ b/yawaflua.WebSockets.Tests/yawaflua.WebSockets.Tests.csproj @@ -7,6 +7,8 @@ + + diff --git a/yawaflua.WebSockets/Attributes/WebSocketAttribute.cs b/yawaflua.WebSockets/Attributes/WebSocketAttribute.cs index 983b407..3dd01d5 100644 --- a/yawaflua.WebSockets/Attributes/WebSocketAttribute.cs +++ b/yawaflua.WebSockets/Attributes/WebSocketAttribute.cs @@ -21,7 +21,7 @@ namespace yawaflua.WebSockets.Attributes; /// [AttributeUsage(AttributeTargets.Method | AttributeTargets.Class)] [ApiExplorerSettings(IgnoreApi = true)] -public class WebSocketAttribute : RouteAttribute, IRouteTemplateProvider, IApiDescriptionVisibilityProvider +public class WebSocketAttribute : Attribute, IApiDescriptionVisibilityProvider { /// /// Original route template specified in attribute @@ -39,7 +39,7 @@ public class WebSocketAttribute : RouteAttribute, IRouteTemplateProvider, IApiDe /// - Parameters: "/user/{id}" /// - Constraints: "/file/{name:alpha}" /// - Optional: "/feed/{category?}" - public WebSocketAttribute([RouteTemplate]string path) : base(path) + public WebSocketAttribute(string path) { Template = path; Name = path; diff --git a/yawaflua.WebSockets/Core/WebSocketRouter.cs b/yawaflua.WebSockets/Core/WebSocketRouter.cs index 5d7c065..3ee85bb 100644 --- a/yawaflua.WebSockets/Core/WebSocketRouter.cs +++ b/yawaflua.WebSockets/Core/WebSocketRouter.cs @@ -1,4 +1,6 @@ -using System.Diagnostics.CodeAnalysis; +using System.Collections.Concurrent; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Net.WebSockets; using System.Reflection; using System.Text; @@ -15,7 +17,7 @@ namespace yawaflua.WebSockets.Core; [SuppressMessage("ReSharper", "AsyncVoidLambda")] public class WebSocketRouter { - internal static readonly Dictionary> Routes = new(); + internal static readonly ConcurrentDictionary> Routes = new(); internal static readonly List Clients = new(); private readonly IServiceProvider _serviceProvider; private readonly ILogger _logger; @@ -69,6 +71,7 @@ public class WebSocketRouter parameters[1].ParameterType != typeof(HttpContext) || func.ReturnType != typeof(Task)) { + _logger.LogCritical($"Invalid handler signature in {type.Name}.{func.Name}"); throw new InvalidOperationException( $"Invalid handler signature in {type.Name}.{func.Name}"); } @@ -79,15 +82,25 @@ public class WebSocketRouter typeof(Func), func ); - Routes.Add(parentAttributeTemplate, delegateFunc); + if (!Routes.TryAdd(parentAttributeTemplate, delegateFunc)) + { + _logger.LogCritical($"Error registered whilest adds new route: {parentAttributeTemplate}"); + throw new InvalidOperationException( + $"Error registered whilest adds new route: {parentAttributeTemplate}"); + } } else { - Routes.Add(parentAttributeTemplate, async (ws, context) => + if (!Routes.TryAdd(parentAttributeTemplate, async (ws, context) => + { + var instance = context.RequestServices.GetRequiredService(type); + await (Task)func.Invoke(instance, new object[] { ws, context })!; + })) { - var instance = context.RequestServices.GetRequiredService(type); - await (Task)func.Invoke(instance, new object[] { ws, context })!; - }); + _logger.LogCritical($"Error registered whilest adds new route: {parentAttributeTemplate}"); + throw new InvalidOperationException( + $"Error registered whilest adds new route: {parentAttributeTemplate}"); + } } } else @@ -96,21 +109,42 @@ public class WebSocketRouter { var attribute = (WebSocketAttribute)method.GetCustomAttributes(typeof(WebSocketAttribute), false).First(); + + var key = parentAttributeTemplate+attribute.Template; + + if (Routes.ContainsKey(key)) + { + Debug.WriteLine(Routes); + _logger.LogCritical($"Duplicate route error: {key}"); + throw new InvalidOperationException( + $"Duplicate route error: {key}"); + } + if (method.IsStatic) { var delegateFunc = (Func)Delegate.CreateDelegate( typeof(Func), method ); - Routes.Add(parentAttributeTemplate+attribute.Template, delegateFunc); + if (!Routes.TryAdd(key, delegateFunc)) + { + _logger.LogCritical($"Error registered whilest adds new route: {key}"); + throw new InvalidOperationException( + $"Error registered whilest adds new route: {key}"); + } } else { - Routes.Add(parentAttributeTemplate+attribute.Template, async (ws, context) => + if (!Routes.TryAdd(key, async (ws, context) => + { + var instance = context.RequestServices.GetRequiredService(type); + await (Task)method.Invoke(instance, new object[] { ws, context })!; + })) { - var instance = context.RequestServices.GetRequiredService(type); - await (Task)method.Invoke(instance, new object[] { ws, context })!; - }); + _logger.LogCritical($"Error registered whilest adds new route: {key}"); + throw new InvalidOperationException( + $"Error registered whilest adds new route: {key}"); + } } } } @@ -127,8 +161,19 @@ public class WebSocketRouter } catch (Exception ex) { - _logger.LogCritical(message:"Error when parsing attributes from assemblies: ", exception:ex); + _logger.LogCritical("Error when parsing attributes from assemblies: {ex}", ex); + Debug.WriteLine(ex); + Debug.WriteLine(Routes); + throw new Exception("Error when parsing attributes from assemblies", ex); } + +#if DEBUG + _logger.LogDebug("Routes:"); + foreach (var route in Routes) + { + _logger.LogDebug("Key:FuncName => {k}:{f}", route.Key, route.Value.Method.Name); + } +#endif } internal async Task HandleRequest(HttpContext context, CancellationToken cts = default) @@ -179,12 +224,13 @@ public class WebSocketRouter } catch (Exception ex) { - _logger.LogError(message:"Error with handling request: ",exception: ex); + _logger.LogError("Error with handling request: {ex}", ex); await Task.Run(async () => { if (_webSocketConfig?.OnErrorHandler != null) await _webSocketConfig.OnErrorHandler(ex, new WebSocket(webSocket, client, webSocketManager), context); }, cts); + } }, cts); @@ -197,7 +243,7 @@ public class WebSocketRouter } catch (Exception ex) { - _logger.LogError(ex, $"Error when handle request {context.Connection.Id}: "); + _logger.LogError($"Error when handle request {context.Connection.RemoteIpAddress}: {ex}"); if (_webSocketConfig!.OnConnectionErrorHandler != null) await _webSocketConfig.OnConnectionErrorHandler(ex, context); } diff --git a/yawaflua.WebSockets/Models/Abstracts/WebSocketController.cs b/yawaflua.WebSockets/Models/Abstracts/WebSocketController.cs index 7af5ceb..579ea38 100644 --- a/yawaflua.WebSockets/Models/Abstracts/WebSocketController.cs +++ b/yawaflua.WebSockets/Models/Abstracts/WebSocketController.cs @@ -5,7 +5,7 @@ using WebSocketManager = yawaflua.WebSockets.Core.WebSocketManager; namespace yawaflua.WebSockets.Models.Abstracts; -public abstract class WebSocketController : IWebSocketController +public abstract class WebSocketController : IWebSocketController { /// /// WebsocketManager provides work with all clients diff --git a/yawaflua.WebSockets/ServiceBindings.cs b/yawaflua.WebSockets/ServiceBindings.cs index 38d343f..5184ca1 100644 --- a/yawaflua.WebSockets/ServiceBindings.cs +++ b/yawaflua.WebSockets/ServiceBindings.cs @@ -16,6 +16,8 @@ public static class ServiceBindings if (isc.All(k => k.ServiceType != typeof(WebSocketConfig))) isc.AddSingleton(new WebSocketConfig()); isc.AddScoped(); + isc.AddSingleton(); + isc.AddTransient(); isc.AddSingleton(); return isc; } diff --git a/yawaflua.WebSockets/yawaflua.WebSockets.csproj b/yawaflua.WebSockets/yawaflua.WebSockets.csproj index b09d48c..9f1697b 100644 --- a/yawaflua.WebSockets/yawaflua.WebSockets.csproj +++ b/yawaflua.WebSockets/yawaflua.WebSockets.csproj @@ -4,7 +4,7 @@ net6.0;net7.0;net8.0;net9.0 enable enable - 1.0.1 + 1.0.2 yawaflua.WebSockets New AspNet controllers looks like websocket manager Dmitrii Shimanskii @@ -20,6 +20,7 @@ +