From 47725a13bd8e0af5d0d0961dc44f20258abf6afc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 24 Aug 2025 17:19:26 +0000 Subject: [PATCH] Implement CachedTransportService design with interface separation Co-authored-by: clayschaad <11883505+clayschaad@users.noreply.github.com> --- Server/Controllers/ShiftController.cs | 4 +- Server/Program.cs | 3 +- Services/CachedTransportService.cs | 43 ++++++ Services/ITransportService.cs | 14 ++ Services/TransportService.cs | 33 +---- .../CachedTransportServiceTests.cs | 124 ++++++++++++++++++ .../TransportServiceTests.cs | 62 +-------- 7 files changed, 187 insertions(+), 96 deletions(-) create mode 100644 Services/CachedTransportService.cs create mode 100644 Services/ITransportService.cs create mode 100644 ShiftScheduler.Services.Tests/CachedTransportServiceTests.cs diff --git a/Server/Controllers/ShiftController.cs b/Server/Controllers/ShiftController.cs index 7678ce0..f4617b2 100644 --- a/Server/Controllers/ShiftController.cs +++ b/Server/Controllers/ShiftController.cs @@ -11,14 +11,14 @@ namespace ShiftScheduler.Server.Controllers private readonly ShiftService _shiftService; private readonly IcsExportService _icsService; private readonly PdfExportService _pdfExportService; - private readonly TransportService _transportService; + private readonly ITransportService _transportService; private readonly TransportConfiguration _transportConfig; public ShiftController( ShiftService shiftService, IcsExportService icsService, PdfExportService pdfExportService, - TransportService transportService, + ITransportService transportService, TransportConfiguration transportConfig) { _shiftService = shiftService; diff --git a/Server/Program.cs b/Server/Program.cs index 63d0c2d..89a6aa1 100644 --- a/Server/Program.cs +++ b/Server/Program.cs @@ -15,7 +15,8 @@ builder.Services.AddHttpClient(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); -builder.Services.AddSingleton(); +builder.Services.AddSingleton(); +builder.Services.AddSingleton(); builder.Services.AddControllersWithViews(); builder.Services.AddRazorPages(); diff --git a/Services/CachedTransportService.cs b/Services/CachedTransportService.cs new file mode 100644 index 0000000..36e7d88 --- /dev/null +++ b/Services/CachedTransportService.cs @@ -0,0 +1,43 @@ +using Microsoft.Extensions.Caching.Memory; +using ShiftScheduler.Shared; + +namespace ShiftScheduler.Services +{ + public class CachedTransportService(ITransportApiService transportService, TransportConfiguration config, IMemoryCache cache) : ITransportService + { + public async Task GetConnectionAsync(DateTime shiftStartTime) + { + var searchDate = shiftStartTime.ToString("yyyy-MM-dd"); + var searchTime = shiftStartTime.AddMinutes(config.MaxLateArrivalMinutes).ToString("HH:mm"); + + // Generate cache key based on request parameters + var cacheKey = GenerateCacheKey(searchDate, searchTime); + + // Try to get from cache first + if (cache.TryGetValue(cacheKey, out TransportConnection? cachedConnection) && cachedConnection != null) + { + return cachedConnection; + } + + // Not in cache, call the underlying transport service + var connection = await transportService.GetConnectionAsync(shiftStartTime); + + // Cache the connection result if valid + if (connection != null) + { + var cacheOptions = new MemoryCacheEntryOptions + { + AbsoluteExpirationRelativeToNow = TimeSpan.FromDays(config.CacheDurationDays) + }; + cache.Set(cacheKey, connection, cacheOptions); + } + + return connection; + } + + private string GenerateCacheKey(string searchDate, string searchTime) + { + return $"transport_{config.StartStation}_{config.EndStation}_{searchDate}_{searchTime}"; + } + } +} \ No newline at end of file diff --git a/Services/ITransportService.cs b/Services/ITransportService.cs new file mode 100644 index 0000000..ee3d593 --- /dev/null +++ b/Services/ITransportService.cs @@ -0,0 +1,14 @@ +using ShiftScheduler.Shared; + +namespace ShiftScheduler.Services +{ + public interface ITransportService + { + Task GetConnectionAsync(DateTime shiftStartTime); + } + + public interface ITransportApiService + { + Task GetConnectionAsync(DateTime shiftStartTime); + } +} \ No newline at end of file diff --git a/Services/TransportService.cs b/Services/TransportService.cs index 2358350..4047020 100644 --- a/Services/TransportService.cs +++ b/Services/TransportService.cs @@ -1,10 +1,9 @@ using System.Text.Json; -using Microsoft.Extensions.Caching.Memory; using ShiftScheduler.Shared; namespace ShiftScheduler.Services { - public class TransportService(HttpClient httpClient, TransportConfiguration config, IMemoryCache cache) + public class TransportService(HttpClient httpClient, TransportConfiguration config) : ITransportApiService { public async Task GetConnectionAsync(DateTime shiftStartTime) { @@ -15,16 +14,6 @@ namespace ShiftScheduler.Services // and request more connections to cover the full range var searchTime = shiftStartTime.AddMinutes(config.MaxLateArrivalMinutes).ToString("HH:mm"); - // Generate cache key based on request parameters - var cacheKey = GenerateCacheKey(searchDate, searchTime); - - // Try to get from cache first - if (cache.TryGetValue(cacheKey, out TransportApiResponse? cachedResponse) && cachedResponse != null) - { - return ProcessApiResponse(cachedResponse, shiftStartTime); - } - - // Not in cache, make API call var url = $"{config.ApiBaseUrl}/connections?from={Uri.EscapeDataString(config.StartStation)}&to={Uri.EscapeDataString(config.EndStation)}&date={searchDate}&time={searchTime}&isArrivalTime=1&limit=5"; var response = await httpClient.GetStringAsync(url); var apiResponse = JsonSerializer.Deserialize(response, new JsonSerializerOptions @@ -32,26 +21,6 @@ namespace ShiftScheduler.Services PropertyNameCaseInsensitive = true }); - // Cache the response if valid - if (apiResponse != null) - { - var cacheOptions = new MemoryCacheEntryOptions - { - AbsoluteExpirationRelativeToNow = TimeSpan.FromDays(config.CacheDurationDays) - }; - cache.Set(cacheKey, apiResponse, cacheOptions); - } - - return ProcessApiResponse(apiResponse, shiftStartTime); - } - - private string GenerateCacheKey(string searchDate, string searchTime) - { - return $"transport_{config.StartStation}_{config.EndStation}_{searchDate}_{searchTime}"; - } - - private TransportConnection? ProcessApiResponse(TransportApiResponse? apiResponse, DateTime shiftStartTime) - { if (apiResponse?.Connections.Count > 0) { var allConnections = apiResponse.Connections.Select(MapToTransportConnection).ToList(); diff --git a/ShiftScheduler.Services.Tests/CachedTransportServiceTests.cs b/ShiftScheduler.Services.Tests/CachedTransportServiceTests.cs new file mode 100644 index 0000000..aee4fd2 --- /dev/null +++ b/ShiftScheduler.Services.Tests/CachedTransportServiceTests.cs @@ -0,0 +1,124 @@ +using Microsoft.Extensions.Caching.Memory; +using Moq; +using ShiftScheduler.Services; +using ShiftScheduler.Shared; +using Shouldly; + +namespace ShiftScheduler.Services.Tests; + +public class CachedTransportServiceTests +{ + private readonly Mock _transportServiceMock; + private readonly IMemoryCache _memoryCache; + private readonly CachedTransportService _cachedTransportService; + private readonly TransportConfiguration _config; + + public CachedTransportServiceTests() + { + _transportServiceMock = new Mock(); + _memoryCache = new MemoryCache(new MemoryCacheOptions()); + + _config = new TransportConfiguration + { + StartStation = "Zurich HB", + EndStation = "Bern", + ApiBaseUrl = "https://transport.opendata.ch/v1", + SafetyBufferMinutes = 30, + MinBreakMinutes = 60, + MaxEarlyArrivalMinutes = 60, + MaxLateArrivalMinutes = 15, + CacheDurationDays = 1 + }; + + _cachedTransportService = new CachedTransportService(_transportServiceMock.Object, _config, _memoryCache); + } + + [Fact] + public async Task GetConnectionAsync_WithValidConnection_ShouldCacheResult() + { + // Arrange + var shiftStartTime = new DateTime(2023, 12, 15, 8, 0, 0); + var connection = new TransportConnection + { + DepartureTime = "2023-12-15T06:45:00", + ArrivalTime = "2023-12-15T07:30:00", + Duration = "00:45:00", + Platform = "5" + }; + + _transportServiceMock + .Setup(x => x.GetConnectionAsync(shiftStartTime)) + .ReturnsAsync(connection); + + // Act - First call should hit the transport service + var result1 = await _cachedTransportService.GetConnectionAsync(shiftStartTime); + + // Act - Second call should use cache + var result2 = await _cachedTransportService.GetConnectionAsync(shiftStartTime); + + // Assert + result1.ShouldNotBeNull(); + result2.ShouldNotBeNull(); + result1.ArrivalTime.ShouldBe(result2.ArrivalTime); + result1.DepartureTime.ShouldBe(result2.DepartureTime); + + // Verify that transport service was called only once + _transportServiceMock.Verify(x => x.GetConnectionAsync(shiftStartTime), Times.Once); + } + + [Fact] + public async Task GetConnectionAsync_WithNullConnection_ShouldNotCache() + { + // Arrange + var shiftStartTime = new DateTime(2023, 12, 15, 8, 0, 0); + + _transportServiceMock + .Setup(x => x.GetConnectionAsync(shiftStartTime)) + .ReturnsAsync((TransportConnection?)null); + + // Act - First call + var result1 = await _cachedTransportService.GetConnectionAsync(shiftStartTime); + + // Act - Second call should call transport service again since null wasn't cached + var result2 = await _cachedTransportService.GetConnectionAsync(shiftStartTime); + + // Assert + result1.ShouldBeNull(); + result2.ShouldBeNull(); + + // Verify that transport service was called twice (no caching for null) + _transportServiceMock.Verify(x => x.GetConnectionAsync(shiftStartTime), Times.Exactly(2)); + } + + [Fact] + public async Task GetConnectionAsync_WithDifferentDates_ShouldCreateSeparateCacheEntries() + { + // Arrange + var shiftStartTime1 = new DateTime(2023, 12, 15, 8, 0, 0); + var shiftStartTime2 = new DateTime(2023, 12, 16, 8, 0, 0); + + var connection1 = new TransportConnection { ArrivalTime = "2023-12-15T07:30:00" }; + var connection2 = new TransportConnection { ArrivalTime = "2023-12-16T07:30:00" }; + + _transportServiceMock + .Setup(x => x.GetConnectionAsync(shiftStartTime1)) + .ReturnsAsync(connection1); + + _transportServiceMock + .Setup(x => x.GetConnectionAsync(shiftStartTime2)) + .ReturnsAsync(connection2); + + // Act - Different dates should result in different cache keys + var result1 = await _cachedTransportService.GetConnectionAsync(shiftStartTime1); + var result2 = await _cachedTransportService.GetConnectionAsync(shiftStartTime2); + + // Assert + result1.ShouldNotBeNull(); + result2.ShouldNotBeNull(); + result1.ArrivalTime.ShouldBe("2023-12-15T07:30:00"); + result2.ArrivalTime.ShouldBe("2023-12-16T07:30:00"); + + // Verify that transport service was called twice (different cache keys) + _transportServiceMock.Verify(x => x.GetConnectionAsync(It.IsAny()), Times.Exactly(2)); + } +} \ No newline at end of file diff --git a/ShiftScheduler.Services.Tests/TransportServiceTests.cs b/ShiftScheduler.Services.Tests/TransportServiceTests.cs index a1e30b0..23ff77a 100644 --- a/ShiftScheduler.Services.Tests/TransportServiceTests.cs +++ b/ShiftScheduler.Services.Tests/TransportServiceTests.cs @@ -1,6 +1,5 @@ using System.Net; using System.Text.Json; -using Microsoft.Extensions.Caching.Memory; using Moq; using Moq.Protected; using ShiftScheduler.Services; @@ -13,14 +12,12 @@ public class TransportServiceTests { private readonly Mock _httpMessageHandlerMock; private readonly HttpClient _httpClient; - private readonly IMemoryCache _memoryCache; private readonly TransportService _transportService; public TransportServiceTests() { _httpMessageHandlerMock = new Mock(); _httpClient = new HttpClient(_httpMessageHandlerMock.Object); - _memoryCache = new MemoryCache(new MemoryCacheOptions()); var config = new TransportConfiguration { @@ -34,7 +31,7 @@ public class TransportServiceTests CacheDurationDays = 1 }; - _transportService = new TransportService(_httpClient, config, _memoryCache); + _transportService = new TransportService(_httpClient, config); } [Fact] @@ -119,63 +116,6 @@ public class TransportServiceTests result.ArrivalTime.ShouldBe("2023-12-15T07:25:00"); } - [Fact] - public async Task GetConnectionAsync_WithCaching_ShouldCacheApiResponse() - { - // Arrange - var shiftStartTime = new DateTime(2023, 12, 15, 8, 0, 0); - var apiResponse = CreateValidApiResponse(); - var jsonResponse = JsonSerializer.Serialize(apiResponse); - - SetupHttpMockResponse(HttpStatusCode.OK, jsonResponse); - - // Act - First call should hit the API - var result1 = await _transportService.GetConnectionAsync(shiftStartTime); - - // Act - Second call should use cache - var result2 = await _transportService.GetConnectionAsync(shiftStartTime); - - // Assert - result1.ShouldNotBeNull(); - result2.ShouldNotBeNull(); - result1.ArrivalTime.ShouldBe(result2.ArrivalTime); - result1.DepartureTime.ShouldBe(result2.DepartureTime); - - // Verify that HTTP call was made only once - _httpMessageHandlerMock.Protected().Verify( - "SendAsync", - Times.Once(), - ItExpr.IsAny(), - ItExpr.IsAny()); - } - - [Fact] - public async Task GetConnectionAsync_WithDifferentDates_ShouldMakeSeparateApiCalls() - { - // Arrange - var shiftStartTime1 = new DateTime(2023, 12, 15, 8, 0, 0); - var shiftStartTime2 = new DateTime(2023, 12, 16, 8, 0, 0); - var apiResponse = CreateValidApiResponse(); - var jsonResponse = JsonSerializer.Serialize(apiResponse); - - SetupHttpMockResponse(HttpStatusCode.OK, jsonResponse); - - // Act - Different dates should result in different cache keys - var result1 = await _transportService.GetConnectionAsync(shiftStartTime1); - var result2 = await _transportService.GetConnectionAsync(shiftStartTime2); - - // Assert - result1.ShouldNotBeNull(); - result2.ShouldNotBeNull(); - - // Verify that HTTP calls were made twice (different cache keys) - _httpMessageHandlerMock.Protected().Verify( - "SendAsync", - Times.Exactly(2), - ItExpr.IsAny(), - ItExpr.IsAny()); - } - private void SetupHttpMockResponse(HttpStatusCode statusCode, string content) { _httpMessageHandlerMock.Protected()