From be25df670e6bd0b7e2f765ca2013428e50a99722 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sun, 19 Jul 2026 14:41:12 +0200 Subject: [PATCH] feat(431): TTL-cache health-check results; ?refresh=true forces a fresh run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GET /api/v1/health re-ran all 14 health checks on every request, 4 of which shell out to ffmpeg/ffprobe via CliWrap — so each poll spawned ~4 subprocesses. The existing HealthCheckSummary cache was write-only. Cache the full result list for 30s inside HealthCheckService keyed on a new "healthcheck.results" entry; a non-forced call returns it on a hit, skipping the checks and the (subscriber-less) summary publish. Add a `bool forceRefresh` first parameter to IHealthCheckService.PerformHealthChecks: the API poll path reads the cache, while startup (RunHealthChecksService) and the troubleshooting support bundle force a fresh run. Refresh surface: GET /api/v1/health gains an optional `[FromQuery] bool refresh` (additive, follows the ?deep= exemplar); the SPA "Refresh health" button calls /api/v1/health?refresh=true, the initial/poll load does not. Tests: HealthCheckService cache-hit vs force-bypass (mutually opposing, non-vacuous), handler+controller refresh-flag threading, SPA refresh URL. Docs: decisions.md 2026-07-19 (#431), api-conventions §2; regenerated v1.json. fixes #431 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Queries/GetAllHealthCheckResultsForApi.cs | 2 +- .../GetAllHealthCheckResultsForApiHandler.cs | 3 +- .../GetAllHealthCheckResultsHandler.cs | 4 +- .../Queries/GetTroubleshootingInfoHandler.cs | 3 +- ErsatzTV.Core/Health/IHealthCheckService.cs | 4 +- .../Health/HealthCheckService.cs | 23 +++- ...AllHealthCheckResultsForApiHandlerTests.cs | 36 ++++-- .../Controllers/HealthControllerTests.cs | 17 ++- .../Health/HealthCheckServiceTests.cs | 107 ++++++++++++++++++ ErsatzTV/Controllers/Api/HealthController.cs | 8 +- .../RunOnce/RunHealthChecksService.cs | 4 +- ErsatzTV/wwwroot/openapi/v1.json | 20 ++++ docs/api-conventions.md | 6 + docs/decisions.md | 32 ++++++ web/src/App.test.tsx | 2 +- web/src/api/dashboard.ts | 13 ++- web/src/screens/DashboardScreen.test.tsx | 9 +- web/src/screens/SettingsScreen.test.tsx | 2 +- 18 files changed, 260 insertions(+), 35 deletions(-) create mode 100644 ErsatzTV.Tests/Infrastructure/Health/HealthCheckServiceTests.cs diff --git a/ErsatzTV.Application/Health/Queries/GetAllHealthCheckResultsForApi.cs b/ErsatzTV.Application/Health/Queries/GetAllHealthCheckResultsForApi.cs index a3062a2f7..0abda0c00 100644 --- a/ErsatzTV.Application/Health/Queries/GetAllHealthCheckResultsForApi.cs +++ b/ErsatzTV.Application/Health/Queries/GetAllHealthCheckResultsForApi.cs @@ -2,4 +2,4 @@ using ErsatzTV.Core.Api.Health; namespace ErsatzTV.Application.Health; -public record GetAllHealthCheckResultsForApi : IRequest>; +public record GetAllHealthCheckResultsForApi(bool Refresh = false) : IRequest>; diff --git a/ErsatzTV.Application/Health/Queries/GetAllHealthCheckResultsForApiHandler.cs b/ErsatzTV.Application/Health/Queries/GetAllHealthCheckResultsForApiHandler.cs index 334d4cd28..5a0d24224 100644 --- a/ErsatzTV.Application/Health/Queries/GetAllHealthCheckResultsForApiHandler.cs +++ b/ErsatzTV.Application/Health/Queries/GetAllHealthCheckResultsForApiHandler.cs @@ -18,7 +18,8 @@ public class GetAllHealthCheckResultsForApiHandler { try { - List results = await _healthCheckService.PerformHealthChecks(cancellationToken); + List results = + await _healthCheckService.PerformHealthChecks(request.Refresh, cancellationToken); return results .Filter(r => r.Status != HealthCheckStatus.NotApplicable) .Map(ProjectToResponseModel) diff --git a/ErsatzTV.Application/Health/Queries/GetAllHealthCheckResultsHandler.cs b/ErsatzTV.Application/Health/Queries/GetAllHealthCheckResultsHandler.cs index 489872a67..dcc76e9b2 100644 --- a/ErsatzTV.Application/Health/Queries/GetAllHealthCheckResultsHandler.cs +++ b/ErsatzTV.Application/Health/Queries/GetAllHealthCheckResultsHandler.cs @@ -1,4 +1,4 @@ -using ErsatzTV.Core.Health; +using ErsatzTV.Core.Health; namespace ErsatzTV.Application.Health; @@ -15,7 +15,7 @@ public class GetAllHealthCheckResultsHandler : IRequestHandler results = await _healthCheckService.PerformHealthChecks(cancellationToken); + List results = await _healthCheckService.PerformHealthChecks(false, cancellationToken); return results.Filter(r => r.Status != HealthCheckStatus.NotApplicable).ToList(); } catch (Exception ex) when (ex is TaskCanceledException or OperationCanceledException) diff --git a/ErsatzTV.Application/Troubleshooting/Queries/GetTroubleshootingInfoHandler.cs b/ErsatzTV.Application/Troubleshooting/Queries/GetTroubleshootingInfoHandler.cs index c9cec698d..7ce02e811 100644 --- a/ErsatzTV.Application/Troubleshooting/Queries/GetTroubleshootingInfoHandler.cs +++ b/ErsatzTV.Application/Troubleshooting/Queries/GetTroubleshootingInfoHandler.cs @@ -45,7 +45,8 @@ public class GetTroubleshootingInfoHandler : IRequestHandler Handle(GetTroubleshootingInfo request, CancellationToken cancellationToken) { - List healthCheckResults = await _healthCheckService.PerformHealthChecks(cancellationToken); + // Support bundle wants current state, so force a fresh run rather than serving the poll cache. + List healthCheckResults = await _healthCheckService.PerformHealthChecks(true, cancellationToken); string version = Assembly.GetEntryAssembly()? .GetCustomAttribute()? diff --git a/ErsatzTV.Core/Health/IHealthCheckService.cs b/ErsatzTV.Core/Health/IHealthCheckService.cs index c9d3dac2a..efe2e94eb 100644 --- a/ErsatzTV.Core/Health/IHealthCheckService.cs +++ b/ErsatzTV.Core/Health/IHealthCheckService.cs @@ -1,7 +1,7 @@ -namespace ErsatzTV.Core.Health; +namespace ErsatzTV.Core.Health; public interface IHealthCheckService { - Task> PerformHealthChecks(CancellationToken cancellationToken); + Task> PerformHealthChecks(bool forceRefresh, CancellationToken cancellationToken); HealthCheckSummary GetHealthCheckSummary(); } diff --git a/ErsatzTV.Infrastructure/Health/HealthCheckService.cs b/ErsatzTV.Infrastructure/Health/HealthCheckService.cs index 3bda2544b..2911c5a43 100644 --- a/ErsatzTV.Infrastructure/Health/HealthCheckService.cs +++ b/ErsatzTV.Infrastructure/Health/HealthCheckService.cs @@ -1,4 +1,4 @@ -using ErsatzTV.Core.Health; +using ErsatzTV.Core.Health; using ErsatzTV.Core.Health.Checks; using MediatR; using Microsoft.Extensions.Caching.Memory; @@ -8,7 +8,14 @@ namespace ErsatzTV.Infrastructure.Health; public class HealthCheckService : IHealthCheckService { - private const string CacheKey = "healthcheck.summary"; + private const string SummaryCacheKey = "healthcheck.summary"; + private const string ResultsCacheKey = "healthcheck.results"; + + // Health checks shell out to ffmpeg/ffprobe (4 of the 14 checks) on every run, so a bare + // GET /api/v1/health spawns ~4 subprocesses per request. Cache the full result list for a + // short window so repeated polls (a status widget, an MCP client, monitoring) reuse it; an + // explicit refresh (forceRefresh) bypasses and repopulates. See docs/decisions.md 2026-07-19 (#431). + private static readonly TimeSpan CacheTtl = TimeSpan.FromSeconds(30); private readonly List _checks; // ReSharper disable SuggestBaseTypeForParameterInConstructor private readonly IMemoryCache _memoryCache; @@ -56,8 +63,13 @@ public class HealthCheckService : IHealthCheckService ]; } - public async Task> PerformHealthChecks(CancellationToken cancellationToken) + public async Task> PerformHealthChecks(bool forceRefresh, CancellationToken cancellationToken) { + if (!forceRefresh && _memoryCache.TryGetValue(ResultsCacheKey, out List cached) && cached is not null) + { + return cached; + } + List result = await _checks.Map(c => { var failedResult = new HealthCheckResult( @@ -75,7 +87,8 @@ public class HealthCheckService : IHealthCheckService result.Count(x => x.Status is HealthCheckStatus.Warning), result.Count(x => x.Status is HealthCheckStatus.Fail)); - _memoryCache.Set(CacheKey, summary); + _memoryCache.Set(ResultsCacheKey, result, CacheTtl); + _memoryCache.Set(SummaryCacheKey, summary); await _mediator.Publish(summary, cancellationToken); @@ -83,7 +96,7 @@ public class HealthCheckService : IHealthCheckService } public HealthCheckSummary GetHealthCheckSummary() => - _memoryCache.Get(CacheKey) ?? new HealthCheckSummary(0, 0); + _memoryCache.Get(SummaryCacheKey) ?? new HealthCheckSummary(0, 0); private HealthCheckResult LogAndReturn(Exception ex, HealthCheckResult failedResult) { diff --git a/ErsatzTV.Tests/Application/Health/GetAllHealthCheckResultsForApiHandlerTests.cs b/ErsatzTV.Tests/Application/Health/GetAllHealthCheckResultsForApiHandlerTests.cs index e3176ceab..b11fc4fd0 100644 --- a/ErsatzTV.Tests/Application/Health/GetAllHealthCheckResultsForApiHandlerTests.cs +++ b/ErsatzTV.Tests/Application/Health/GetAllHealthCheckResultsForApiHandlerTests.cs @@ -32,7 +32,7 @@ public class GetAllHealthCheckResultsForApiHandlerTests new("Info Check", HealthCheckStatus.Info, "fyi", "info", Option.None) }; - _healthCheckService.PerformHealthChecks(Arg.Any()).Returns(results); + _healthCheckService.PerformHealthChecks(Arg.Any(), Arg.Any()).Returns(results); List response = await _handler.Handle(new GetAllHealthCheckResultsForApi(), CancellationToken.None); @@ -50,7 +50,7 @@ public class GetAllHealthCheckResultsForApiHandlerTests new("NA Check", HealthCheckStatus.NotApplicable, "skip", "skip", Option.None) }; - _healthCheckService.PerformHealthChecks(Arg.Any()).Returns(results); + _healthCheckService.PerformHealthChecks(Arg.Any(), Arg.Any()).Returns(results); List response = await _handler.Handle(new GetAllHealthCheckResultsForApi(), CancellationToken.None); @@ -72,7 +72,7 @@ public class GetAllHealthCheckResultsForApiHandlerTests Option.Some(HealthCheckLink.ExternalDoc("https://example.com/docs"))) }; - _healthCheckService.PerformHealthChecks(Arg.Any()).Returns(results); + _healthCheckService.PerformHealthChecks(Arg.Any(), Arg.Any()).Returns(results); List response = await _handler.Handle(new GetAllHealthCheckResultsForApi(), CancellationToken.None); @@ -99,7 +99,7 @@ public class GetAllHealthCheckResultsForApiHandlerTests Option.Some(HealthCheckLink.AppRoute("/app/trash"))) }; - _healthCheckService.PerformHealthChecks(Arg.Any()).Returns(results); + _healthCheckService.PerformHealthChecks(Arg.Any(), Arg.Any()).Returns(results); List response = await _handler.Handle(new GetAllHealthCheckResultsForApi(), CancellationToken.None); @@ -118,7 +118,7 @@ public class GetAllHealthCheckResultsForApiHandlerTests new("No Link Check", HealthCheckStatus.Pass, "detail", "brief", Option.None) }; - _healthCheckService.PerformHealthChecks(Arg.Any()).Returns(results); + _healthCheckService.PerformHealthChecks(Arg.Any(), Arg.Any()).Returns(results); List response = await _handler.Handle(new GetAllHealthCheckResultsForApi(), CancellationToken.None); @@ -135,7 +135,7 @@ public class GetAllHealthCheckResultsForApiHandlerTests new("Blank Brief", HealthCheckStatus.Pass, "detail", string.Empty, Option.None) }; - _healthCheckService.PerformHealthChecks(Arg.Any()).Returns(results); + _healthCheckService.PerformHealthChecks(Arg.Any(), Arg.Any()).Returns(results); List response = await _handler.Handle(new GetAllHealthCheckResultsForApi(), CancellationToken.None); @@ -146,7 +146,7 @@ public class GetAllHealthCheckResultsForApiHandlerTests [Test] public async Task Should_Return_Empty_List_On_Cancellation() { - _healthCheckService.PerformHealthChecks(Arg.Any()) + _healthCheckService.PerformHealthChecks(Arg.Any(), Arg.Any()) .Returns>>(_ => throw new TaskCanceledException()); List response = @@ -154,4 +154,26 @@ public class GetAllHealthCheckResultsForApiHandlerTests response.ShouldBeEmpty(); } + + [Test] + public async Task Should_Not_Force_Refresh_By_Default() + { + _healthCheckService.PerformHealthChecks(Arg.Any(), Arg.Any()) + .Returns(new List()); + + await _handler.Handle(new GetAllHealthCheckResultsForApi(), CancellationToken.None); + + await _healthCheckService.Received(1).PerformHealthChecks(false, Arg.Any()); + } + + [Test] + public async Task Should_Force_Refresh_When_Requested() + { + _healthCheckService.PerformHealthChecks(Arg.Any(), Arg.Any()) + .Returns(new List()); + + await _handler.Handle(new GetAllHealthCheckResultsForApi(Refresh: true), CancellationToken.None); + + await _healthCheckService.Received(1).PerformHealthChecks(true, Arg.Any()); + } } diff --git a/ErsatzTV.Tests/Controllers/HealthControllerTests.cs b/ErsatzTV.Tests/Controllers/HealthControllerTests.cs index 867a365aa..063288ff8 100644 --- a/ErsatzTV.Tests/Controllers/HealthControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/HealthControllerTests.cs @@ -53,7 +53,7 @@ public class HealthControllerTests _mediator.Send(Arg.Any(), Arg.Any()) .Returns(expected); - List result = await _controller.GetAll(CancellationToken.None); + List result = await _controller.GetAll(false, CancellationToken.None); result.ShouldBe(expected); } @@ -64,8 +64,21 @@ public class HealthControllerTests _mediator.Send(Arg.Any(), Arg.Any()) .Returns([]); - List result = await _controller.GetAll(CancellationToken.None); + List result = await _controller.GetAll(false, CancellationToken.None); result.ShouldBeEmpty(); } + + [Test] + public async Task GetAll_Should_Forward_Refresh_Flag_To_Query() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns([]); + + await _controller.GetAll(true, CancellationToken.None); + + await _mediator.Received(1).Send( + Arg.Is(q => q.Refresh), + Arg.Any()); + } } diff --git a/ErsatzTV.Tests/Infrastructure/Health/HealthCheckServiceTests.cs b/ErsatzTV.Tests/Infrastructure/Health/HealthCheckServiceTests.cs new file mode 100644 index 000000000..5787c0b2c --- /dev/null +++ b/ErsatzTV.Tests/Infrastructure/Health/HealthCheckServiceTests.cs @@ -0,0 +1,107 @@ +using ErsatzTV.Core.Health; +using ErsatzTV.Core.Health.Checks; +using ErsatzTV.Infrastructure.Health; +using LanguageExt; +using MediatR; +using Microsoft.Extensions.Caching.Memory; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using NUnit.Framework; +using Shouldly; + +namespace ErsatzTV.Tests.Infrastructure.Health; + +[TestFixture] +public class HealthCheckServiceTests +{ + private IFFmpegVersionHealthCheck _representativeCheck = null!; + private IMemoryCache _memoryCache = null!; + private IMediator _mediator = null!; + private HealthCheckService _service = null!; + + [SetUp] + public void SetUp() + { + _memoryCache = new MemoryCache(new MemoryCacheOptions()); + _mediator = Substitute.For(); + _representativeCheck = PassCheck(); + + // All 14 checks run together on a single PerformHealthChecks call, so the representative + // check's invocation count equals the number of actual (non-cached) runs. + _service = new HealthCheckService( + PassCheck(), + _representativeCheck, + PassCheck(), + PassCheck(), + PassCheck(), + PassCheck(), + PassCheck(), + PassCheck(), + PassCheck(), + PassCheck(), + PassCheck(), + PassCheck(), + PassCheck(), + PassCheck(), + _memoryCache, + _mediator, + NullLogger.Instance); + } + + [TearDown] + public void TearDown() => (_memoryCache as MemoryCache)?.Dispose(); + + [Test] + public async Task PerformHealthChecks_Should_Serve_Cache_Within_Window() + { + await _service.PerformHealthChecks(false, CancellationToken.None); + await _service.PerformHealthChecks(false, CancellationToken.None); + + // Second call is served from cache: checks are not re-run, summary is not re-published. + await _representativeCheck.Received(1).Check(Arg.Any()); + await _mediator.Received(1).Publish(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task PerformHealthChecks_Should_Return_Cached_Instance_Within_Window() + { + List first = await _service.PerformHealthChecks(false, CancellationToken.None); + List second = await _service.PerformHealthChecks(false, CancellationToken.None); + + second.ShouldBeSameAs(first); + } + + [Test] + public async Task PerformHealthChecks_Should_Bypass_Cache_On_Force_Refresh() + { + await _service.PerformHealthChecks(false, CancellationToken.None); + await _service.PerformHealthChecks(true, CancellationToken.None); + + // Force refresh re-runs the checks and re-publishes, regardless of the warm cache. + await _representativeCheck.Received(2).Check(Arg.Any()); + await _mediator.Received(2).Publish(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task Force_Refresh_Should_Repopulate_Cache_For_Later_Reads() + { + // A forced run should warm the cache so a following non-forced read is a hit. + await _service.PerformHealthChecks(true, CancellationToken.None); + await _service.PerformHealthChecks(false, CancellationToken.None); + + await _representativeCheck.Received(1).Check(Arg.Any()); + } + + private static T PassCheck() where T : class, IHealthCheck + { + var check = Substitute.For(); + check.Check(Arg.Any()) + .Returns(_ => new HealthCheckResult( + typeof(T).Name, + HealthCheckStatus.Pass, + "ok", + "ok", + Option.None)); + return check; + } +} diff --git a/ErsatzTV/Controllers/Api/HealthController.cs b/ErsatzTV/Controllers/Api/HealthController.cs index bb70d1e16..f5ec2cb0e 100644 --- a/ErsatzTV/Controllers/Api/HealthController.cs +++ b/ErsatzTV/Controllers/Api/HealthController.cs @@ -11,8 +11,12 @@ public class HealthController(IMediator mediator) : ControllerBase [HttpGet("/api/v1/health", Name = "GetHealthChecks")] [Tags("Health")] [EndpointSummary("Get health check results")] + [EndpointDescription( + "Results are cached briefly; pass refresh=true to force a fresh run (re-executes ffmpeg-backed checks).")] [EndpointGroupName("general")] [ProducesResponseType(typeof(List), StatusCodes.Status200OK)] - public async Task> GetAll(CancellationToken cancellationToken) => - await mediator.Send(new GetAllHealthCheckResultsForApi(), cancellationToken); + public async Task> GetAll( + [FromQuery] bool refresh, + CancellationToken cancellationToken) => + await mediator.Send(new GetAllHealthCheckResultsForApi(refresh), cancellationToken); } diff --git a/ErsatzTV/Services/RunOnce/RunHealthChecksService.cs b/ErsatzTV/Services/RunOnce/RunHealthChecksService.cs index 5df80f779..3778cd859 100644 --- a/ErsatzTV/Services/RunOnce/RunHealthChecksService.cs +++ b/ErsatzTV/Services/RunOnce/RunHealthChecksService.cs @@ -1,4 +1,4 @@ -using ErsatzTV.Core; +using ErsatzTV.Core; using ErsatzTV.Core.Health; namespace ErsatzTV.Services.RunOnce; @@ -30,6 +30,6 @@ public class RunHealthChecksService(IServiceScopeFactory serviceScopeFactory, Sy using IServiceScope scope = serviceScopeFactory.CreateScope(); IHealthCheckService healthCheckService = scope.ServiceProvider.GetRequiredService(); - await healthCheckService.PerformHealthChecks(stoppingToken); + await healthCheckService.PerformHealthChecks(true, stoppingToken); } } diff --git a/ErsatzTV/wwwroot/openapi/v1.json b/ErsatzTV/wwwroot/openapi/v1.json index 8ad8375e3..b168e4324 100644 --- a/ErsatzTV/wwwroot/openapi/v1.json +++ b/ErsatzTV/wwwroot/openapi/v1.json @@ -7829,7 +7829,17 @@ "Health" ], "summary": "Get health check results", + "description": "Results are cached briefly; pass refresh=true to force a fresh run (re-executes ffmpeg-backed checks).", "operationId": "GetHealthChecks", + "parameters": [ + { + "name": "refresh", + "in": "query", + "schema": { + "type": "boolean" + } + } + ], "responses": { "200": { "description": "OK", @@ -7869,6 +7879,16 @@ } } } + }, + "400": { + "description": "Request validation failed (model binding or FluentValidation).", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ValidationProblemDetails" + } + } + } } }, "security": [ diff --git a/docs/api-conventions.md b/docs/api-conventions.md index be1f57efe..85ae19d5c 100644 --- a/docs/api-conventions.md +++ b/docs/api-conventions.md @@ -96,6 +96,12 @@ Exemplars: the action; filter server-side only when it has a value. Exemplar: `?fillerKind=` on `GET /api/v1/filler-presets` (`GetAllFillerPresetsForApi(FillerKind? FillerKind = null)`). An invalid enum value is rejected by model binding (400) — no handler-side guard needed. +- **Optional bool query param (flag / cache-bust)**: bind `[FromQuery] bool name` (absent → `false`) and + thread it into the query record with a defaulted parameter so existing callers are unaffected. Exemplars: + `?deep=` on `POST /api/v1/libraries/{id}/scan` (§3b), and `?refresh=` on `GET /api/v1/health` + (`GetAllHealthCheckResultsForApi(bool Refresh = false)`) which forces a fresh run past the service's TTL + result cache — the cached poll path is the default, the flag is the explicit opt-out (see `decisions.md` + 2026-07-19, #431). - **Evolving a frozen DTO: deprecate-in-place, add the richer field, never remove.** `/api/v1` is frozen-additive (#286), so when a response field's shape needs to grow, keep the old member populated (mark it deprecated in an XML/`//` comment) and add the replacement alongside. Exemplar: diff --git a/docs/decisions.md b/docs/decisions.md index 74ccb02c0..4c3b10e03 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -100,6 +100,7 @@ in-file entries. - [2026-07-18 — CI build-once was measured and rejected; keep the #420 tree-skip](#2026-07-18--ci-build-once-was-measured-and-rejected-keep-the-420-tree-skip) - [2026-07-18 — Never-scanned `LastScan` surfaces as null at the API boundary, not the 0001-01-01 MinValue sentinel (#409)](#2026-07-18--never-scanned-lastscan-surfaces-as-null-at-the-api-boundary-not-the-0001-01-01-minvalue-sentinel-409) - [2026-07-19 — WeightedShuffle SPA: weights edited on the multi-collection, order offered only on classic MultiCollection schedule items; fair-share is a reset not a mode (#404)](#2026-07-19--weightedshuffle-spa-weights-edited-on-the-multi-collection-order-offered-only-on-classic-multicollection-schedule-items-fair-share-is-a-reset-not-a-mode-404) +- [2026-07-19 — Health-check results are TTL-cached; `?refresh=true` forces a fresh run (#431)](#2026-07-19--health-check-results-are-ttl-cached-refreshtrue-forces-a-fresh-run-431) --- @@ -1982,3 +1983,34 @@ is purely SPA (+ docs). weight field is held as a string in the editor draft so it edits smoothly, clamped to the API validator's 1..1000 on blur and again at save — an out-of-range value never reaches the server as a raw 400. `Input` gained `min`/`max`/`inputMode`/`onBlur` passthroughs for this (reusable by #425's weight UI). + +## 2026-07-19 — Health-check results are TTL-cached; `?refresh=true` forces a fresh run (#431) + +`HealthCheckService.PerformHealthChecks` re-ran all 14 checks on **every** call, four of which shell out to +`ffmpeg`/`ffprobe` via CliWrap — so a bare `GET /api/v1/health` spawned ~4 subprocesses per request. The +existing `HealthCheckSummary` cache was **write-only** (populated + published, never read back to short-circuit +a re-run). Harmless while the SPA Dashboard health panel refreshes on-demand only, but a real cost the moment +anything *polls* health (a status widget, an MCP client, monitoring). Split out of #164 as the orthogonal +performance half. + +- **A short TTL cache of the full result list lives inside `HealthCheckService`.** A `_memoryCache` entry + (`"healthcheck.results"`, `TimeSpan.FromSeconds(30)`) holds the last `List`; a non-forced + call returns it directly on a hit, skipping both the 14 checks and the summary `Publish`. Chosen over + "make the existing summary cache read-through" because the API returns the full per-check list, not the + 2-int summary — the summary entry (`"healthcheck.summary"`, read by `GetHealthCheckSummary`) is kept as-is + (un-expiring) so its fallback behavior is unchanged. +- **`PerformHealthChecks` gained a `bool forceRefresh` first parameter** (interface signature change; one + implementer, 3 live callers). `forceRefresh: true` bypasses the cache and repopulates it. +- **The refresh surface is an optional `?refresh=` query param on the existing GET**, following the + `?deep=` bool-query-param exemplar (`api-conventions.md` §1/§3b) — additive, backward-compatible, no new + endpoint. `[FromQuery] bool refresh` → `GetAllHealthCheckResultsForApi(Refresh)` → `PerformHealthChecks(request.Refresh, …)`. + The SPA "Refresh health" button calls `/api/v1/health?refresh=true`; the initial/poll load calls the bare + path (cached). A separate `POST …/refresh` endpoint was rejected as unnecessary surface for a read. +- **Who forces vs. who reads the cache:** the API GET poll path reads the cache; the **startup** + `RunHealthChecksService` and the **troubleshooting** support bundle force a fresh run (both want current + state — startup is a cold cache anyway, and a diagnostic bundle should reflect *now*, not a ≤30s-old poll). + The legacy `GetAllHealthCheckResults` handler is dead (no senders) and reads the cache. +- **Thundering-herd on a cold cache was left out of scope** (no request-coalescing lock): polling is sequential + per client and the TTL collapses steady-state load, so at most a handful of exactly-simultaneous cold callers + re-run — a once-per-30s edge, not the repeated per-request cost the issue targets. Recorded here so a later + reviewer doesn't read the absence of a `SemaphoreSlim` as an oversight. diff --git a/web/src/App.test.tsx b/web/src/App.test.tsx index e0baae7bf..378aef032 100644 --- a/web/src/App.test.tsx +++ b/web/src/App.test.tsx @@ -1403,7 +1403,7 @@ function mockDashboardApi({ return Promise.resolve(jsonResponse(playoutDetails ?? playout({ id: Number(path.split('/').at(-1)) }))); } - if (path === '/api/v1/health') { + if (path.startsWith('/api/v1/health')) { return Promise.resolve(jsonResponse(health)); } diff --git a/web/src/api/dashboard.ts b/web/src/api/dashboard.ts index be87d87cb..4b5208102 100644 --- a/web/src/api/dashboard.ts +++ b/web/src/api/dashboard.ts @@ -47,8 +47,9 @@ export async function getDashboardData(): Promise { return { channels, channelStates, mediaSources, playouts }; } -export function getDashboardHealth(): Promise { - return request('/api/v1/health'); +export function getDashboardHealth(refresh = false): Promise { + // Health results are cached server-side; the on-demand "Refresh health" action forces a fresh run. + return request(refresh ? '/api/v1/health?refresh=true' : '/api/v1/health'); } export function getDashboardVersion(): Promise { @@ -102,8 +103,8 @@ export function useDashboardHealthQuery(): DashboardHealthQueryState { }; }, []); - const loadHealth = useCallback(() => { - getDashboardHealth() + const loadHealth = useCallback((refresh: boolean) => { + getDashboardHealth(refresh) .then((checks) => { if (activeRef.current) { setState({ checks, error: null, status: 'success' }); @@ -118,11 +119,11 @@ export function useDashboardHealthQuery(): DashboardHealthQueryState { const refresh = useCallback(() => { setState({ checks: null, error: null, status: 'loading' }); - loadHealth(); + loadHealth(true); }, [loadHealth]); useEffect(() => { - loadHealth(); + loadHealth(false); }, [loadHealth]); if (state.status === 'success') { diff --git a/web/src/screens/DashboardScreen.test.tsx b/web/src/screens/DashboardScreen.test.tsx index bfc8f48f0..20c0fe592 100644 --- a/web/src/screens/DashboardScreen.test.tsx +++ b/web/src/screens/DashboardScreen.test.tsx @@ -228,11 +228,16 @@ describe('DashboardScreen', () => { expect(await screen.findByText('All checks passed')).toBeInTheDocument(); expect(fetchCount('/api/v1/health')).toBe(1); + // Initial load uses the cached (non-forced) endpoint. + expect(window.fetch).toHaveBeenCalledWith('/api/v1/health', expect.any(Object)); + expect(window.fetch).not.toHaveBeenCalledWith('/api/v1/health?refresh=true', expect.any(Object)); fireEvent.click(screen.getByRole('button', { name: 'Refresh health' })); expect(await screen.findByText('All checks passed')).toBeInTheDocument(); expect(fetchCount('/api/v1/health')).toBe(2); + // The explicit "Refresh health" action forces a fresh server-side run. + expect(window.fetch).toHaveBeenCalledWith('/api/v1/health?refresh=true', expect.any(Object)); }); it('shows the API error detail when dashboard loading fails', async () => { @@ -297,7 +302,7 @@ function mockDashboardApi({ return Promise.resolve(jsonResponse(playouts)); } - if (path === '/api/v1/health') { + if (path.startsWith('/api/v1/health')) { return Promise.resolve(jsonResponse(health)); } @@ -313,5 +318,5 @@ function jsonResponse(body: unknown, status = 200): Response { } function fetchCount(path: string): number { - return vi.mocked(window.fetch).mock.calls.filter(([input]) => input.toString() === path).length; + return vi.mocked(window.fetch).mock.calls.filter(([input]) => input.toString().startsWith(path)).length; } diff --git a/web/src/screens/SettingsScreen.test.tsx b/web/src/screens/SettingsScreen.test.tsx index f6357be63..df50fd582 100644 --- a/web/src/screens/SettingsScreen.test.tsx +++ b/web/src/screens/SettingsScreen.test.tsx @@ -655,7 +655,7 @@ function mockSettingsApi({ return Promise.resolve(jsonResponse({ apiVersion: 3, appVersion: '26.4.0' })); } - if (path === '/api/v1/health') { + if (path.startsWith('/api/v1/health')) { return Promise.resolve(jsonResponse([])); }