diff --git a/.github/workflows/sonarcloud.yml b/.github/workflows/sonarcloud.yml index a010ee2..960a941 100644 --- a/.github/workflows/sonarcloud.yml +++ b/.github/workflows/sonarcloud.yml @@ -50,7 +50,7 @@ jobs: /k:"rrusson_DarkClippy" \ /o:"rrusson" \ /d:sonar.host.url="https://sonarcloud.io" \ - /d:sonar.token="${{ SONAR_TOKEN }}" \ + /d:sonar.token="${SONAR_TOKEN}" \ /d:sonar.cs.opencover.reportsPaths="**/TestResults/**/coverage.opencover.xml" - name: Restore diff --git a/ClippyWeb.Tests/ChatClientFactoryTests.cs b/ClippyWeb.Tests/ChatClientFactoryTests.cs new file mode 100644 index 0000000..7352df7 --- /dev/null +++ b/ClippyWeb.Tests/ChatClientFactoryTests.cs @@ -0,0 +1,80 @@ +using Microsoft.Extensions.Caching.Memory; + +using SemanticKernelHelper; + +using SharedInterfaces; + +namespace ClippyWeb.Tests +{ + [TestClass] + public class ChatClientFactoryTests + { + private const string TestApiUrl = "http://localhost:11434/v1"; + private const string TestModel = "test-model"; + private const string TestApiKey = "test-api-key"; + private IMemoryCache _cache = null!; + + [TestInitialize] + public void TestInitialize() + { + _cache = new MemoryCache(new MemoryCacheOptions()); + } + + [TestCleanup] + public void TestCleanup() + { + _cache?.Dispose(); + } + + [TestMethod] + public void IfSessionKeyIsProvidedThenClientIsReturned() + { + var factory = new ChatClientFactory(TestApiUrl, TestModel, TestApiKey, _cache); + + var client = factory.GetOrCreateClient("test-session"); + + Assert.IsNotNull(client); + } + + [TestMethod] + public void IfSameSessionKeyIsUsedThenSameClientIsReturned() + { + var factory = new ChatClientFactory(TestApiUrl, TestModel, TestApiKey, _cache); + + var client1 = factory.GetOrCreateClient("test-session"); + var client2 = factory.GetOrCreateClient("test-session"); + + Assert.AreSame(client1, client2); + } + + [TestMethod] + public void IfDifferentSessionKeysAreUsedThenDifferentClientsAreReturned() + { + var factory = new ChatClientFactory(TestApiUrl, TestModel, TestApiKey, _cache); + + var client1 = factory.GetOrCreateClient("session-1"); + var client2 = factory.GetOrCreateClient("session-2"); + + Assert.AreNotSame(client1, client2); + } + + [TestMethod] + public void IfMultipleSessionsAreConcurrentThenFactoryIsThreadSafe() + { + var factory = new ChatClientFactory(TestApiUrl, TestModel, TestApiKey, _cache); + var clients = new List(); + var lockObj = new object(); + + Parallel.For(0, 10, i => + { + var client = factory.GetOrCreateClient($"session-{i % 3}"); + lock (lockObj) + { + clients.Add(client); + } + }); + + Assert.AreEqual(10, clients.Count); + } + } +} diff --git a/ClippyWeb.Tests/ClippyWeb.Tests.csproj b/ClippyWeb.Tests/ClippyWeb.Tests.csproj index 641b79d..2b430a0 100644 --- a/ClippyWeb.Tests/ClippyWeb.Tests.csproj +++ b/ClippyWeb.Tests/ClippyWeb.Tests.csproj @@ -21,12 +21,8 @@ + - - - - - diff --git a/ClippyWeb.Tests/Controllers/ChatControllerTests.cs b/ClippyWeb.Tests/Controllers/ChatControllerTests.cs index 96e48c7..ab6c243 100644 --- a/ClippyWeb.Tests/Controllers/ChatControllerTests.cs +++ b/ClippyWeb.Tests/Controllers/ChatControllerTests.cs @@ -1,11 +1,13 @@ +using System.Net; +using System.Net.Sockets; + +using ClippyWeb.Controllers; + using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Configuration; -using System.Net; -using System.Net.Sockets; -using ClippyWeb.Controllers; using SharedInterfaces; namespace ClippyWeb.Tests.Controllers @@ -17,6 +19,7 @@ namespace ClippyWeb.Tests.Controllers public class ChatControllerTests { private Mock _mockChatClient = null!; + private Mock _mockChatClientFactory = null!; private IMemoryCache _memoryCache = null!; private Mock _mockConfiguration = null!; private ChatController _sut = null!; @@ -25,10 +28,12 @@ public class ChatControllerTests public void TestInitialize() { _mockChatClient = new Mock(); + _mockChatClientFactory = new Mock(); + _mockChatClientFactory.Setup(f => f.GetOrCreateClient(It.IsAny())).Returns(_mockChatClient.Object); _memoryCache = new MemoryCache(new MemoryCacheOptions()); _mockConfiguration = new Mock(); - _sut = new ChatController(_mockChatClient.Object, _memoryCache, _mockConfiguration.Object); + _sut = new ChatController(_mockChatClientFactory.Object, _memoryCache, _mockConfiguration.Object); SetupHttpContext(); } diff --git a/ClippyWeb.Tests/GlobalUsings.cs b/ClippyWeb.Tests/GlobalUsings.cs index 0ab807c..5fc4745 100644 --- a/ClippyWeb.Tests/GlobalUsings.cs +++ b/ClippyWeb.Tests/GlobalUsings.cs @@ -1,2 +1,3 @@ global using Microsoft.VisualStudio.TestTools.UnitTesting; + global using Moq; diff --git a/ClippyWeb.Tests/Pages/ErrorModelTests.cs b/ClippyWeb.Tests/Pages/ErrorModelTests.cs index 8585d92..0ba4a12 100644 --- a/ClippyWeb.Tests/Pages/ErrorModelTests.cs +++ b/ClippyWeb.Tests/Pages/ErrorModelTests.cs @@ -1,12 +1,11 @@ -using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Mvc; -using Microsoft.AspNetCore.Mvc.RazorPages; -using Microsoft.Extensions.Configuration; - using System.Diagnostics; using ClippyWeb.Pages; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.RazorPages; +using Microsoft.Extensions.Configuration; + namespace ClippyWeb.Tests.Pages { /// diff --git a/ClippyWeb.Tests/SemanticKernelClientTests.cs b/ClippyWeb.Tests/SemanticKernelClientTests.cs new file mode 100644 index 0000000..966b554 --- /dev/null +++ b/ClippyWeb.Tests/SemanticKernelClientTests.cs @@ -0,0 +1,94 @@ +using SemanticKernelHelper; + +namespace ClippyWeb.Tests +{ + [TestClass] + public class SemanticKernelClientTests + { + private const string TestApiUrl = "http://localhost:11434/v1"; + private const string TestModel = "test-model"; + private const string TestApiKey = "test-api-key"; + + [TestMethod] + public void IfApiKeyIsProvidedThenClientIsCreated() + { + var client = new SemanticKernelClient(TestApiUrl, TestModel, TestApiKey); + + Assert.IsNotNull(client); + } + + [TestMethod] + public void IfApiKeyIsNullThenClientIsCreatedWithEmptyKey() + { + var client = new SemanticKernelClient(TestApiUrl, TestModel, null); + + Assert.IsNotNull(client); + } + + [TestMethod] + public void IfApiKeyIsNotProvidedThenClientIsCreatedWithEmptyKey() + { + var client = new SemanticKernelClient(TestApiUrl, TestModel); + + Assert.IsNotNull(client); + } + + [TestMethod] + public async Task IfEmptyMessageThenReturnsDefaultResponse() + { + var client = new SemanticKernelClient(TestApiUrl, TestModel, TestApiKey); + + var response = await client.GetChatResponseAsync(""); + + Assert.AreEqual("You say something?", response); + } + + [TestMethod] + public async Task IfWhitespaceMessageThenReturnsDefaultResponse() + { + var client = new SemanticKernelClient(TestApiUrl, TestModel, TestApiKey); + + var response = await client.GetChatResponseAsync(" "); + + Assert.AreEqual("You say something?", response); + } + + [TestMethod] + public async Task IfNullMessageThenReturnsDefaultResponse() + { + var client = new SemanticKernelClient(TestApiUrl, TestModel, TestApiKey); + + var response = await client.GetChatResponseAsync(null!); + + Assert.AreEqual("You say something?", response); + } + + [TestMethod] + [ExpectedException(typeof(ArgumentNullException))] + public void IfApiUrlIsNullThenThrowsArgumentNullException() + { + _ = new SemanticKernelClient(null!, TestModel, TestApiKey); + } + + [TestMethod] + [ExpectedException(typeof(ArgumentNullException))] + public void IfModelIsNullThenThrowsArgumentNullException() + { + _ = new SemanticKernelClient(TestApiUrl, null!, TestApiKey); + } + + [TestMethod] + [ExpectedException(typeof(UriFormatException))] + public void IfApiUrlIsInvalidThenThrowsUriFormatException() + { + _ = new SemanticKernelClient("not-a-valid-url", TestModel, TestApiKey); + } + + [TestMethod] + [ExpectedException(typeof(UriFormatException))] + public void IfApiUrlIsNotHttpOrHttpsThenThrowsUriFormatException() + { + _ = new SemanticKernelClient("ftp://localhost:11434", TestModel, TestApiKey); + } + } +} diff --git a/ClippyWeb.Tests/Util/ConnectionValidatorTests.cs b/ClippyWeb.Tests/Util/ConnectionValidatorTests.cs index 71a654e..d55c38a 100644 --- a/ClippyWeb.Tests/Util/ConnectionValidatorTests.cs +++ b/ClippyWeb.Tests/Util/ConnectionValidatorTests.cs @@ -1,6 +1,3 @@ -using System.Net; -using System.Net.Sockets; - using ClippyWeb.Util; using Microsoft.Extensions.Configuration; diff --git a/ClippyWeb.Tests/Util/TcpClientFactoryTests.cs b/ClippyWeb.Tests/Util/TcpClientFactoryTests.cs new file mode 100644 index 0000000..326257c --- /dev/null +++ b/ClippyWeb.Tests/Util/TcpClientFactoryTests.cs @@ -0,0 +1,23 @@ +namespace ClippyWeb.Util.UnitTests +{ + /// + /// Unit tests for the class. + /// + [TestClass] + public class TcpClientFactoryTests + { + /// + /// Tests that the Dispose method calls Dispose(bool) with true parameter. + /// + [TestMethod] + public void CreateWorks() + { + // Act + var result = new TcpClientFactory().Create(); + + // Assert + Assert.IsNotNull(result); + Assert.IsInstanceOfType(result, typeof(TcpClientWrapper)); + } + } +} diff --git a/ClippyWeb.Tests/Util/TcpClientWrapperTests.cs b/ClippyWeb.Tests/Util/TcpClientWrapperTests.cs new file mode 100644 index 0000000..e33fb2b --- /dev/null +++ b/ClippyWeb.Tests/Util/TcpClientWrapperTests.cs @@ -0,0 +1,439 @@ +using System.Net.Sockets; + +namespace ClippyWeb.Util.UnitTests +{ + /// + /// Unit tests for the class. + /// + [TestClass] + public class TcpClientWrapperTests + { + /// + /// Tests that the Dispose method calls Dispose(bool) with true parameter. + /// + [TestMethod] + public void Dispose_WhenCalled_CallsDisposeWithTrue() + { + // Arrange + var disposeTracker = new DisposableTestWrapper(); + + // Act + disposeTracker.Dispose(); + + // Assert + Assert.IsTrue(disposeTracker.DisposeCalled, "Dispose(bool) should have been called"); + Assert.IsTrue(disposeTracker.DisposingParameter, "Dispose(bool) should have been called with true"); + } + + /// + /// Tests that the Dispose method does not throw exceptions when called. + /// + [TestMethod] + public void Dispose_WhenCalled_DoesNotThrow() + { + // Arrange + var sut = new global::ClippyWeb.Util.TcpClientWrapper(); + + // Act & Assert + sut.Dispose(); + } + + /// + /// Tests that the Dispose method can be called multiple times without throwing exceptions (idempotency). + /// + [TestMethod] + public void Dispose_WhenCalledMultipleTimes_DoesNotThrow() + { + // Arrange + var sut = new global::ClippyWeb.Util.TcpClientWrapper(); + + // Act & Assert + sut.Dispose(); + sut.Dispose(); + sut.Dispose(); + } + + /// + /// Helper class to track Dispose(bool) calls for testing purposes. + /// + private sealed class DisposableTestWrapper : global::ClippyWeb.Util.TcpClientWrapper + { + public bool DisposeCalled { get; private set; } + public bool DisposingParameter { get; private set; } + + protected override void Dispose(bool disposing) + { + DisposeCalled = true; + DisposingParameter = disposing; + base.Dispose(disposing); + } + } + + /// + /// Tests that the Connected property returns false when the TcpClient is not connected. + /// Input: A newly instantiated TcpClientWrapper with no active connection. + /// Expected: Connected property should return false. + /// + [TestMethod] + public void Connected_WhenNotConnected_ReturnsFalse() + { + // Arrange + using var wrapper = new TcpClientWrapper(); + + // Act + var result = wrapper.Connected; + + // Assert + Assert.IsFalse(result); + } + + /// + /// Tests that the Connected property can be accessed multiple times without throwing. + /// Input: A newly instantiated TcpClientWrapper accessed multiple times. + /// Expected: Connected property should consistently return false without exceptions. + /// + [TestMethod] + public void Connected_MultipleAccesses_ReturnsConsistentValue() + { + // Arrange + using var wrapper = new TcpClientWrapper(); + + // Act + var firstAccess = wrapper.Connected; + var secondAccess = wrapper.Connected; + var thirdAccess = wrapper.Connected; + + // Assert + Assert.IsFalse(firstAccess); + Assert.IsFalse(secondAccess); + Assert.IsFalse(thirdAccess); + } + + /// + /// Tests that the Connected property does not throw an exception when accessed. + /// Input: A newly instantiated TcpClientWrapper. + /// Expected: No exception should be thrown when accessing Connected property. + /// + [TestMethod] + public void Connected_WhenAccessed_DoesNotThrowException() + { + // Arrange + using var wrapper = new TcpClientWrapper(); + + // Act & Assert + try + { + var _ = wrapper.Connected; + Assert.IsTrue(true); + } + catch (Exception ex) + { + Assert.Fail($"Expected no exception, but got {ex.GetType().Name}: {ex.Message}"); + } + } + + /// + /// Tests that the parameterless constructor creates an instance successfully without throwing exceptions. + /// Input: No parameters. + /// Expected: Instance is created and not null. + /// + [TestMethod] + public void TcpClientWrapper_Constructor_CreatesInstanceSuccessfully() + { + // Arrange & Act + var sut = new TcpClientWrapper(); + + // Assert + Assert.IsNotNull(sut); + } + + /// + /// Tests that the constructor properly initializes the internal TcpClient instance. + /// This is verified indirectly through the Connected property. + /// Input: No parameters. + /// Expected: Connected property is accessible and returns false for a newly created, unconnected TcpClient. + /// + [TestMethod] + public void TcpClientWrapper_Constructor_InitializesInternalTcpClient() + { + // Arrange & Act + var sut = new TcpClientWrapper(); + + // Assert + Assert.IsFalse(sut.Connected); + } + + /// + /// Tests that Dispose(bool) with disposing=true properly disposes the internal TcpClient. + /// Verifies that the internal TcpClient is disposed by checking the Connected property behavior. + /// + [TestMethod] + public void Dispose_DisposingTrue_DisposesInternalTcpClient() + { + // Arrange + var wrapper = new TestableTcpClientWrapper(); + + // Act + wrapper.PublicDispose(true); + + // Assert + // After disposal, accessing Connected should either return false or throw ObjectDisposedException + // This verifies the internal TcpClient was actually disposed + try + { + var connected = wrapper.Connected; + // If no exception, verify it returns false after disposal + Assert.IsFalse(connected); + } + catch (ObjectDisposedException) + { + // This is also acceptable behavior after disposal + Assert.IsTrue(true); + } + } + + /// + /// Tests that Dispose(bool) with disposing=false does not dispose the internal TcpClient. + /// Verifies that the method executes without throwing exceptions. + /// + [TestMethod] + public void Dispose_DisposingFalse_DoesNotDispose() + { + // Arrange + var wrapper = new TestableTcpClientWrapper(); + + // Act + wrapper.PublicDispose(false); + + // Assert + // The method should complete without exceptions + // Connected property should still be accessible (though likely false since not connected) + var connected = wrapper.Connected; + Assert.IsFalse(connected); + } + + /// + /// Tests that Dispose(bool) with disposing=true handles null TcpClient gracefully. + /// Verifies that calling Dispose multiple times does not throw exceptions. + /// + [TestMethod] + public void Dispose_CalledMultipleTimes_DoesNotThrow() + { + // Arrange + var wrapper = new TestableTcpClientWrapper(); + + // Act + wrapper.PublicDispose(true); + wrapper.PublicDispose(true); + + // Assert + // Multiple dispose calls should not throw exceptions + Assert.IsTrue(true); + } + + /// + /// Helper class to expose the protected Dispose(bool) method for testing + /// + private class TestableTcpClientWrapper : global::ClippyWeb.Util.TcpClientWrapper + { + /// + /// Exposes the protected Dispose(bool) method as public for testing + /// + /// True to dispose managed resources + public void PublicDispose(bool disposing) + { + Dispose(disposing); + } + } + + /// + /// Tests that ConnectAsync throws ArgumentNullException when host parameter is null. + /// + [TestMethod] + public async Task ConnectAsync_NullHost_ThrowsArgumentNullException() + { + // Arrange + var sut = new TcpClientWrapper(); + string? host = null; + int port = 80; + var cancellationToken = CancellationToken.None; + + // Act & Assert + await Assert.ThrowsExceptionAsync(async () => + { + await sut.ConnectAsync(host!, port, cancellationToken); + }); + } + + /// + /// Tests that ConnectAsync throws ArgumentOutOfRangeException when port is negative. + /// + [TestMethod] + public async Task ConnectAsync_NegativePort_ThrowsArgumentOutOfRangeException() + { + // Arrange + var sut = new TcpClientWrapper(); + string host = "localhost"; + int port = -1; + var cancellationToken = CancellationToken.None; + + // Act & Assert + await Assert.ThrowsExceptionAsync(async () => + { + await sut.ConnectAsync(host, port, cancellationToken); + }); + } + + /// + /// Tests that ConnectAsync throws ArgumentOutOfRangeException when port is zero. + /// + [TestMethod] + public async Task ConnectAsync_ZeroPort_ThrowsArgumentOutOfRangeException() + { + // Arrange + var sut = new TcpClientWrapper(); + string host = "localhost"; + int port = 0; + var cancellationToken = CancellationToken.None; + + // Act & Assert + await Assert.ThrowsExceptionAsync(async () => + { + await sut.ConnectAsync(host, port, cancellationToken); + }); + } + + /// + /// Tests that ConnectAsync throws ArgumentOutOfRangeException when port exceeds maximum valid value. + /// + [TestMethod] + public async Task ConnectAsync_PortAboveMaximum_ThrowsArgumentOutOfRangeException() + { + // Arrange + var sut = new TcpClientWrapper(); + string host = "localhost"; + int port = 65536; + var cancellationToken = CancellationToken.None; + + // Act & Assert + await Assert.ThrowsExceptionAsync(async () => + { + await sut.ConnectAsync(host, port, cancellationToken); + }); + } + + /// + /// Tests that ConnectAsync throws ArgumentOutOfRangeException when port is int.MaxValue. + /// + [TestMethod] + public async Task ConnectAsync_PortMaxValue_ThrowsArgumentOutOfRangeException() + { + // Arrange + var sut = new TcpClientWrapper(); + string host = "localhost"; + int port = int.MaxValue; + var cancellationToken = CancellationToken.None; + + // Act & Assert + await Assert.ThrowsExceptionAsync(async () => + { + await sut.ConnectAsync(host, port, cancellationToken); + }); + } + + /// + /// Tests that ConnectAsync throws ArgumentOutOfRangeException when port is int.MinValue. + /// + [TestMethod] + public async Task ConnectAsync_PortMinValue_ThrowsArgumentOutOfRangeException() + { + // Arrange + var sut = new TcpClientWrapper(); + string host = "localhost"; + int port = int.MinValue; + var cancellationToken = CancellationToken.None; + + // Act & Assert + await Assert.ThrowsExceptionAsync(async () => + { + await sut.ConnectAsync(host, port, cancellationToken); + }); + } + + /// + /// Tests that ConnectAsync respects cancellation when token is already cancelled. + /// + [TestMethod] + public async Task ConnectAsync_CancelledToken_ThrowsOperationCanceledException() + { + // Arrange + var sut = new TcpClientWrapper(); + string host = "localhost"; + int port = 80; + var cts = new CancellationTokenSource(); + cts.Cancel(); + + // Act & Assert + await Assert.ThrowsExceptionAsync(async () => + { + await sut.ConnectAsync(host, port, cts.Token); + }); + } + + /// + /// Tests that ConnectAsync throws ArgumentNullException when host is empty string. + /// Note: The actual behavior depends on the underlying TcpClient implementation. + /// + [TestMethod] + public async Task ConnectAsync_EmptyHost_ThrowsArgumentException() + { + // Arrange + var sut = new TcpClientWrapper(); + string host = ""; + int port = 80; + var cancellationToken = CancellationToken.None; + + // Act & Assert + // Empty host should throw an exception from the underlying TcpClient + await Assert.ThrowsExceptionAsync(async () => + { + await sut.ConnectAsync(host, port, cancellationToken); + }); + } + + /// + /// Tests that ConnectAsync with valid parameters initiates connection attempt. + /// Note: This test validates that the method can be called without immediate exceptions. + /// Actual connection success depends on network availability and is not tested here. + /// The method will likely throw SocketException if no service is listening. + /// + [TestMethod] + public async Task ConnectAsync_ValidParameters_InitiatesConnection() + { + // Arrange + var sut = new TcpClientWrapper(); + string host = "localhost"; + int port = 1; // Valid port number (though unlikely to have a service listening) + using var cts = new CancellationTokenSource(100); // Short timeout to prevent hanging + + // Act & Assert + // This test verifies the method accepts valid parameters and attempts connection. + // It will likely throw SocketException due to no listener, which is expected behavior. + // We're primarily testing that parameter validation passes. + try + { + await sut.ConnectAsync(host, port, cts.Token); + } + catch (SocketException) + { + // Expected when no service is listening + Assert.IsTrue(true); + } + catch (OperationCanceledException) + { + // Expected if timeout occurs before connection fails + Assert.IsTrue(true); + } + } + } +} \ No newline at end of file diff --git a/ClippyWeb/Controllers/ChatController.cs b/ClippyWeb/Controllers/ChatController.cs index 07a9d45..811cffd 100644 --- a/ClippyWeb/Controllers/ChatController.cs +++ b/ClippyWeb/Controllers/ChatController.cs @@ -1,14 +1,14 @@ +using System.Net.Sockets; + using MarkdownSharp; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Caching.Memory; -using SharedInterfaces; - -using System.Net.Sockets; - using Serilog; +using SharedInterfaces; + namespace ClippyWeb.Controllers { [ApiController] @@ -16,14 +16,14 @@ namespace ClippyWeb.Controllers public class ChatController : ControllerBase { private const string RequestInProgressKey = nameof(RequestInProgressKey); - private readonly IChatClient _chatClient; + private readonly IChatClientFactory _chatClientFactory; private readonly Markdown _markdownConverter = new(); private readonly IMemoryCache _cache; private readonly IConfiguration _configuration; - public ChatController(IChatClient chatClient, IMemoryCache cache, IConfiguration configuration) + public ChatController(IChatClientFactory chatClientFactory, IMemoryCache cache, IConfiguration configuration) { - _chatClient = chatClient; + _chatClientFactory = chatClientFactory; _cache = cache; _configuration = configuration; Log.Information("ChatController initialized"); @@ -46,7 +46,8 @@ public async Task Post([FromBody] string question) try { - var response = await _chatClient.GetChatResponseAsync(question); + IChatClient chatClient = _chatClientFactory.GetOrCreateClient(ipAddress); + var response = await chatClient.GetChatResponseAsync(question); if (response == null) { diff --git a/ClippyWeb/Pages/About.cshtml.cs b/ClippyWeb/Pages/About.cshtml.cs index a90b9a0..72a8b8e 100644 --- a/ClippyWeb/Pages/About.cshtml.cs +++ b/ClippyWeb/Pages/About.cshtml.cs @@ -1,10 +1,8 @@ -using Microsoft.AspNetCore.Mvc; -using Microsoft.AspNetCore.Mvc.RazorPages; +using Microsoft.AspNetCore.Mvc.RazorPages; namespace ClippyWeb.Pages { public class AboutModel : PageModel { - public AboutModel() { } } } diff --git a/ClippyWeb/Pages/Error.cshtml.cs b/ClippyWeb/Pages/Error.cshtml.cs index f906b55..0d7e629 100644 --- a/ClippyWeb/Pages/Error.cshtml.cs +++ b/ClippyWeb/Pages/Error.cshtml.cs @@ -11,10 +11,11 @@ namespace ClippyWeb.Pages [IgnoreAntiforgeryToken] public class ErrorModel : PageModel { + private readonly IConfiguration _configuration; + public string? RequestId { get; set; } - public bool ShowRequestId => !string.IsNullOrEmpty(RequestId); - private readonly IConfiguration _configuration; + public bool ShowRequestId => !string.IsNullOrEmpty(RequestId); public ErrorModel(IConfiguration configuration) { diff --git a/ClippyWeb/Pages/Index.cshtml.cs b/ClippyWeb/Pages/Index.cshtml.cs index dcd9166..3f033e7 100644 --- a/ClippyWeb/Pages/Index.cshtml.cs +++ b/ClippyWeb/Pages/Index.cshtml.cs @@ -1,10 +1,8 @@ -using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.RazorPages; namespace ClippyWeb.Pages { public class IndexModel : PageModel { - public IndexModel() { } } } diff --git a/ClippyWeb/Pages/Privacy.cshtml.cs b/ClippyWeb/Pages/Privacy.cshtml.cs index c661614..2967a00 100644 --- a/ClippyWeb/Pages/Privacy.cshtml.cs +++ b/ClippyWeb/Pages/Privacy.cshtml.cs @@ -1,10 +1,8 @@ -using Microsoft.AspNetCore.Mvc; -using Microsoft.AspNetCore.Mvc.RazorPages; +using Microsoft.AspNetCore.Mvc.RazorPages; namespace ClippyWeb.Pages { public class PrivacyModel : PageModel { - public PrivacyModel() { } } } diff --git a/ClippyWeb/Program.cs b/ClippyWeb/Program.cs index 71229ce..8811c42 100644 --- a/ClippyWeb/Program.cs +++ b/ClippyWeb/Program.cs @@ -1,14 +1,15 @@ -using System.Net.NetworkInformation; +using System.Diagnostics.CodeAnalysis; using ClippyWeb.Util; -using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Caching.Memory; using Serilog; using Serilog.Events; using SharedInterfaces; + namespace ClippyWeb { public static class Program @@ -82,7 +83,7 @@ private static async Task SetupLlmService(WebApplicationBuilder builder) var validator = scope.ServiceProvider.GetRequiredService(); await validator.ValidateConnectionAsync(builder.Configuration).ConfigureAwait(false); - builder.Services.AddSingleton(provider => + builder.Services.AddSingleton(provider => { string? serviceUrl = builder.Configuration["ServiceUrl"]; if (string.IsNullOrEmpty(serviceUrl)) @@ -96,9 +97,12 @@ private static async Task SetupLlmService(WebApplicationBuilder builder) throw new InvalidOperationException("Please supply a config value for Model."); } + string? apiKey = builder.Configuration["ApiKey"]; + Log.Information("DarkClippy: Connecting to LLM service at: {ServiceUrl} with model: {Model}", serviceUrl, model); - return new SemanticKernelHelper.SemanticKernelClient(serviceUrl, model); + var cache = provider.GetRequiredService(); + return new SemanticKernelHelper.ChatClientFactory(serviceUrl, model, apiKey ?? string.Empty, cache); }); } @@ -109,6 +113,7 @@ private static async Task SetupLlmService(WebApplicationBuilder builder) /// Thrown if the logging directory path setting is missing from the configuration. /// This method sets up Serilog to log to both the console and a rolling file in the specified directory. /// The log file is rotated daily and limited in size and retention. Logging levels for Microsoft and ASP.NET Core components are set to warning or higher. + [ExcludeFromCodeCoverage] private static void SetupLogging(ConfigurationManager configuration) { string logPath = configuration["LogPath"] ?? throw new System.Configuration.ConfigurationErrorsException("Logging directory path setting missing from appsettings."); diff --git a/ClippyWeb/Util/ConnectionValidator.cs b/ClippyWeb/Util/ConnectionValidator.cs index 58d1962..40996ff 100644 --- a/ClippyWeb/Util/ConnectionValidator.cs +++ b/ClippyWeb/Util/ConnectionValidator.cs @@ -1,8 +1,5 @@ -using System.Net.Sockets; - -using Microsoft.Extensions.Configuration; - using Serilog; + using SharedInterfaces; // Add this using directive to resolve ITcpClient and ITcpClientFactory namespace ClippyWeb.Util diff --git a/ClippyWeb/Util/TcpClientFactory.cs b/ClippyWeb/Util/TcpClientFactory.cs index a977033..8bdc36c 100644 --- a/ClippyWeb/Util/TcpClientFactory.cs +++ b/ClippyWeb/Util/TcpClientFactory.cs @@ -1,5 +1,4 @@ using SharedInterfaces; -using System.Net.Sockets; namespace ClippyWeb.Util { @@ -7,9 +6,6 @@ namespace ClippyWeb.Util public class TcpClientFactory : ITcpClientFactory { /// - public ITcpClient Create() - { - return new TcpClientWrapper(); - } + public ITcpClient Create() => new TcpClientWrapper(); } } diff --git a/ClippyWeb/Util/TcpClientWrapper.cs b/ClippyWeb/Util/TcpClientWrapper.cs index d5bf7fb..ea9f4cf 100644 --- a/ClippyWeb/Util/TcpClientWrapper.cs +++ b/ClippyWeb/Util/TcpClientWrapper.cs @@ -4,43 +4,43 @@ namespace ClippyWeb.Util { - /// - /// Wrapper for TcpClient to enable testability - /// - public class TcpClientWrapper : ITcpClient - { - private readonly TcpClient _tcpClient; + /// + /// Wrapper for TcpClient to enable testability + /// + public class TcpClientWrapper : ITcpClient + { + private readonly TcpClient _tcpClient; - /// - /// Initializes a new instance of the class - /// - public TcpClientWrapper() - { - _tcpClient = new TcpClient(); - } + /// + /// Initializes a new instance of the class + /// + public TcpClientWrapper() + { + _tcpClient = new TcpClient(); + } - /// - public bool Connected => _tcpClient.Connected; + /// + public bool Connected => _tcpClient.Connected; - /// - public ValueTask ConnectAsync(string host, int port, CancellationToken cancellationToken) - { - return _tcpClient.ConnectAsync(host, port, cancellationToken); - } + /// + public ValueTask ConnectAsync(string host, int port, CancellationToken cancellationToken) + { + return _tcpClient.ConnectAsync(host, port, cancellationToken); + } - /// - public void Dispose() - { - Dispose(true); - GC.SuppressFinalize(this); + /// + public void Dispose() + { + Dispose(true); + GC.SuppressFinalize(this); } - protected virtual void Dispose(bool disposing) - { - if (disposing) - { - _tcpClient?.Dispose(); - } + protected virtual void Dispose(bool disposing) + { + if (disposing) + { + _tcpClient?.Dispose(); + } } } } diff --git a/ClippyWeb/appsettings.json b/ClippyWeb/appsettings.json index 45ccf1c..a1c1f74 100644 --- a/ClippyWeb/appsettings.json +++ b/ClippyWeb/appsettings.json @@ -8,5 +8,6 @@ "AllowedHosts": "*", "LogPath": "C:\\temp\\ClippyWeb\\Logs\\", "Model": "HammerAI/neuraldaredevil-abliterated", - "ServiceUrl": "http://localhost:11434/v1" + "ServiceUrl": "http://localhost:11434/v1", + "ApiKey": "" } diff --git a/SemanticKernelHelper/ChatClientFactory.cs b/SemanticKernelHelper/ChatClientFactory.cs new file mode 100644 index 0000000..a85bdc0 --- /dev/null +++ b/SemanticKernelHelper/ChatClientFactory.cs @@ -0,0 +1,46 @@ +using Microsoft.Extensions.Caching.Memory; + +using SharedInterfaces; + +namespace SemanticKernelHelper +{ + public class ChatClientFactory : IChatClientFactory + { + private readonly string _serviceUrl; + private readonly string _model; + private readonly string _apiKey; + private readonly IMemoryCache _cache; + private readonly object _lock = new(); + + public ChatClientFactory(string serviceUrl, string model, string apiKey, IMemoryCache cache) + { + _serviceUrl = serviceUrl; + _model = model; + _apiKey = apiKey; + _cache = cache; + } + + public IChatClient GetOrCreateClient(string sessionKey) + { + string cacheKey = $"ChatClient_{sessionKey}"; + + lock (_lock) + { + if (_cache.TryGetValue(cacheKey, out var client)) + { + // TryGetValue returns true only when client is not null + return client!; + } + + var newClient = new SemanticKernelClient(_serviceUrl, _model, _apiKey); + var cacheOptions = new MemoryCacheEntryOptions + { + SlidingExpiration = TimeSpan.FromMinutes(30), + Priority = CacheItemPriority.Normal + }; + _cache.Set(cacheKey, newClient, cacheOptions); + return newClient; + } + } + } +} diff --git a/SemanticKernelHelper/SemanticKernelClient.cs b/SemanticKernelHelper/SemanticKernelClient.cs index 5f00559..2b1ebbf 100644 --- a/SemanticKernelHelper/SemanticKernelClient.cs +++ b/SemanticKernelHelper/SemanticKernelClient.cs @@ -1,24 +1,60 @@ -using Microsoft.SemanticKernel; +using System.Text; + +using Microsoft.SemanticKernel; using Microsoft.SemanticKernel.ChatCompletion; using SharedInterfaces; -using System.Text; - namespace SemanticKernelHelper { - public class SemanticKernelClient(string apiUrl, string model) : IChatClient + public class SemanticKernelClient : IChatClient { - private readonly string _apiUrl = apiUrl; - private readonly string _model = model; private readonly ChatHistory _chatHistory = []; + private readonly IChatCompletionService _aiChatService; + private int _exchangeCount; + private readonly object _lock = new(); + private const int MaxExchangesBeforeFatigue = 10; + + /// + /// Initializes a new instance of the SemanticKernelClient with the specified configuration. + /// + /// The base URL of the OpenAI-compatible API endpoint. Must be a valid URL. + /// The identifier of the model to use for chat completion. + /// Optional API key for authentication. Defaults to empty string for local services like Ollama. + /// Thrown when apiUrl or model is null. + /// Thrown when apiUrl is not a valid URL format. + public SemanticKernelClient(string apiUrl, string model, string? apiKey = null) + { + ArgumentNullException.ThrowIfNull(apiUrl); + ArgumentNullException.ThrowIfNull(model); + + if (!Uri.TryCreate(apiUrl, UriKind.Absolute, out var apiUri) || + (apiUri.Scheme != Uri.UriSchemeHttp && apiUri.Scheme != Uri.UriSchemeHttps)) + { + throw new UriFormatException($"The value of {nameof(apiUrl)} is not a valid HTTP or HTTPS URL."); + } + + _exchangeCount = 0; + + Kernel kernel = Kernel.CreateBuilder() + .AddOpenAIChatCompletion( + modelId: model, + endpoint: apiUri, + apiKey: apiKey ?? string.Empty) + .Build(); + + _aiChatService = kernel.GetRequiredService(); + + string systemPrompt = "You are Dark Clippy, a profane talking paperclip. You give damn snarky, sarcastic, impatient, and insulting, short answers to questions. And you swear a lot."; + _chatHistory.Add(new ChatMessageContent(AuthorRole.System, systemPrompt)); + } /// /// Generates a chat response from the AI assistant based on the provided user message, maintaining conversational context. /// /// The user's message to send to Dark Clippy. Cannot be null or empty. /// A string containing Dark Clippy's entire response to the user message - /// The conversation history is preserved across calls to provide contextually relevant responses. + /// The conversation history is preserved across calls to provide contextually relevant responses. This method is thread-safe. public async Task GetChatResponseAsync(string chatMessage) { if (string.IsNullOrWhiteSpace(chatMessage)) @@ -26,32 +62,31 @@ public class SemanticKernelClient(string apiUrl, string model) : IChatClient return "You say something?"; } - // Create a kernel with OpenAI chat completion - Kernel kernel = Kernel.CreateBuilder() - .AddOpenAIChatCompletion( - modelId: _model, - endpoint: new Uri(_apiUrl), - apiKey: "") - .Build(); - - IChatCompletionService aiChatService = kernel.GetRequiredService(); + lock (_lock) + { + _exchangeCount++; - string systemPrompt = "You are Dark Clippy, a profane talking paperclip. You give damn snarky, sarcastic, impatient, and insulting, short answers to questions. And you swear a lot."; - _chatHistory.Add(new ChatMessageContent(AuthorRole.System, systemPrompt)); + if (_exchangeCount > MaxExchangesBeforeFatigue) + { + return "Alright, I'm sick of talking about this shit. Go bother someone else."; + } - // Consider persisting the chat history for more coherent conversations - _chatHistory.Add(new ChatMessageContent(AuthorRole.User, chatMessage)); + _chatHistory.Add(new ChatMessageContent(AuthorRole.User, chatMessage)); + } var responseBuilder = new StringBuilder(); - await foreach (StreamingChatMessageContent item in aiChatService.GetStreamingChatMessageContentsAsync(_chatHistory)) + await foreach (StreamingChatMessageContent item in _aiChatService.GetStreamingChatMessageContentsAsync(_chatHistory).ConfigureAwait(false)) { responseBuilder.Append(item.Content); } string response = responseBuilder.ToString(); - _chatHistory.Add(new ChatMessageContent(AuthorRole.Assistant, response)); + lock (_lock) + { + _chatHistory.Add(new ChatMessageContent(AuthorRole.Assistant, response)); + } return response; } diff --git a/SemanticKernelHelper/SemanticKernelHelper.csproj b/SemanticKernelHelper/SemanticKernelHelper.csproj index 8ea0ffb..38f6014 100644 --- a/SemanticKernelHelper/SemanticKernelHelper.csproj +++ b/SemanticKernelHelper/SemanticKernelHelper.csproj @@ -8,6 +8,7 @@ + diff --git a/SharedInterfaces/IChatClientFactory.cs b/SharedInterfaces/IChatClientFactory.cs new file mode 100644 index 0000000..ed84d3a --- /dev/null +++ b/SharedInterfaces/IChatClientFactory.cs @@ -0,0 +1,7 @@ +namespace SharedInterfaces +{ + public interface IChatClientFactory + { + IChatClient GetOrCreateClient(string sessionKey); + } +} diff --git a/SharedInterfaces/IConnectionValidator.cs b/SharedInterfaces/IConnectionValidator.cs index da08538..f4aba30 100644 --- a/SharedInterfaces/IConnectionValidator.cs +++ b/SharedInterfaces/IConnectionValidator.cs @@ -2,15 +2,15 @@ namespace SharedInterfaces { - /// - /// Validates application connection settings. - /// - public interface IConnectionValidator - { - /// - /// Validates the application's connection settings using configuration. - /// - /// The configuration source containing connection settings. - Task ValidateConnectionAsync(IConfiguration configuration); - } + /// + /// Validates application connection settings. + /// + public interface IConnectionValidator + { + /// + /// Validates the application's connection settings using configuration. + /// + /// The configuration source containing connection settings. + Task ValidateConnectionAsync(IConfiguration configuration); + } } \ No newline at end of file diff --git a/SharedInterfaces/IPingService.cs b/SharedInterfaces/IPingService.cs index bf0a17a..792e816 100644 --- a/SharedInterfaces/IPingService.cs +++ b/SharedInterfaces/IPingService.cs @@ -1,10 +1,10 @@ namespace ClippyWeb.Util { - /// - /// Abstraction for sending network pings. - /// - public interface IPingService - { - Task PingAsync(string host, int timeoutMs); - } + /// + /// Abstraction for sending network pings. + /// + public interface IPingService + { + Task PingAsync(string host, int timeoutMs); + } } \ No newline at end of file diff --git a/SharedInterfaces/ITcpClient.cs b/SharedInterfaces/ITcpClient.cs index 57259a0..0005606 100644 --- a/SharedInterfaces/ITcpClient.cs +++ b/SharedInterfaces/ITcpClient.cs @@ -1,24 +1,22 @@ -using System.Net.Sockets; - namespace SharedInterfaces { - /// - /// Abstraction for TcpClient to enable testability. - /// - public interface ITcpClient : IDisposable - { - /// - /// Gets a value indicating whether the underlying Socket is connected to a remote host. - /// - bool Connected { get; } + /// + /// Abstraction for TcpClient to enable testability. + /// + public interface ITcpClient : IDisposable + { + /// + /// Gets a value indicating whether the underlying Socket is connected to a remote host. + /// + bool Connected { get; } - /// - /// Connects the client to a remote TCP host using the specified host name, port number, and cancellation token as an asynchronous operation. - /// - /// The DNS name of the remote host to which you intend to connect. - /// The port number of the remote host to which you intend to connect. - /// A cancellation token used to propagate notification that this operation should be canceled. - /// A task that represents the asynchronous connection operation. - ValueTask ConnectAsync(string host, int port, CancellationToken cancellationToken); - } + /// + /// Connects the client to a remote TCP host using the specified host name, port number, and cancellation token as an asynchronous operation. + /// + /// The DNS name of the remote host to which you intend to connect. + /// The port number of the remote host to which you intend to connect. + /// A cancellation token used to propagate notification that this operation should be canceled. + /// A task that represents the asynchronous connection operation. + ValueTask ConnectAsync(string host, int port, CancellationToken cancellationToken); + } } diff --git a/SharedInterfaces/ITcpClientFactory.cs b/SharedInterfaces/ITcpClientFactory.cs index 69b5dd4..2570136 100644 --- a/SharedInterfaces/ITcpClientFactory.cs +++ b/SharedInterfaces/ITcpClientFactory.cs @@ -1,16 +1,14 @@ -using System.Net.Sockets; - namespace SharedInterfaces { - /// - /// Factory for creating TcpClient instances. - /// - public interface ITcpClientFactory - { - /// - /// Creates a new instance of . - /// - /// A new instance of . - ITcpClient Create(); - } + /// + /// Factory for creating TcpClient instances. + /// + public interface ITcpClientFactory + { + /// + /// Creates a new instance of . + /// + /// A new instance of . + ITcpClient Create(); + } } \ No newline at end of file diff --git a/TestConsole/Program.cs b/TestConsole/Program.cs index b1dac64..bcae1ff 100644 --- a/TestConsole/Program.cs +++ b/TestConsole/Program.cs @@ -24,7 +24,7 @@ static void Main(string[] args) throw new ConfigurationErrorsException("Please supply a config value for Model."); } - IChatClient semanticClient = new SemanticKernelHelper.SemanticKernelClient(serviceUrl, model); + IChatClient semanticClient = new SemanticKernelHelper.SemanticKernelClient(serviceUrl, model); string? responseX = Task.Run(async () => await semanticClient.GetChatResponseAsync(question)).GetAwaiter().GetResult(); Console.WriteLine("Semantic Kernel sez:" + responseX + Environment.NewLine); } diff --git a/sonar-project.properties b/sonar-project.properties deleted file mode 100644 index 0786632..0000000 --- a/sonar-project.properties +++ /dev/null @@ -1,7 +0,0 @@ -# SonarQube project configuration for DarkClippy -sonar.projectKey=DarkClippy -sonar.projectName=DarkClippy -sonar.projectVersion=1.0 -sonar.sources=. -sonar.exclusions=ClippyWeb/wwwroot/js/clippy/src/**/* -sonar.sourceEncoding=UTF-8