From 29407f637be45ec7ffece7096f8503dc37eba973 Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 3 Jul 2026 21:14:42 +0200 Subject: [PATCH] fix(api): align health endpoint with API conventions (#108) Fix the WIP health-check API slice to match established patterns: - HealthController: add Name="GetHealthChecks" route name and move [EndpointGroupName("general")] to method level, matching FillerPresetController/FFmpegProfileController exactly (the precedent for parameterless 200-only GET actions). - HealthCheckResponseModel: enable #nullable for the file and mark Link as string? since the mapper can emit null when HealthCheckResult.Link is None. - Mapper: fix a real bug - LanguageExt's Option.Match throws ResultIsNullException.ResultIsNull if either branch returns null (by design, to catch accidental nulls). The WIP's `Link.Match(l => l.Link, () => null)` crashed on every health check without a Link. Switch to MatchUnsafe, the LanguageExt- sanctioned way to intentionally produce a nullable result from Option. - HealthControllerTests: add the idiomatic-route-assertion test (route template + Name) and empty-list case, matching FillerPresetControllerTests. Verified: GetAllHealthCheckResultsForApiHandler already matches the existing GetAllHealthCheckResultsHandler's cancellation handling (both swallow TaskCanceledException/OperationCanceledException), so no change was needed there. Confirmed no OpenApi contract test enumerates all endpoints for error-response metadata (it's an explicit TestCase allowlist), so the new GET needed no new entries. Co-Authored-By: Claude Fable 5 --- ErsatzTV.Application/Health/Mapper.cs | 2 +- .../Api/Health/HealthCheckResponseModel.cs | 3 ++- .../Controllers/HealthControllerTests.cs | 25 +++++++++++++++++++ ErsatzTV/Controllers/Api/HealthController.cs | 4 +-- 4 files changed, 30 insertions(+), 4 deletions(-) diff --git a/ErsatzTV.Application/Health/Mapper.cs b/ErsatzTV.Application/Health/Mapper.cs index 50c5fa61a..bc5bd56d8 100644 --- a/ErsatzTV.Application/Health/Mapper.cs +++ b/ErsatzTV.Application/Health/Mapper.cs @@ -10,7 +10,7 @@ internal static class Mapper result.Title, GetStatus(result.Status), result.Message, - result.Link.Match(l => l.Link, () => null)); + result.Link.MatchUnsafe(l => l.Link, () => null)); private static string GetStatus(HealthCheckStatus status) => status switch diff --git a/ErsatzTV.Core/Api/Health/HealthCheckResponseModel.cs b/ErsatzTV.Core/Api/Health/HealthCheckResponseModel.cs index 37be2a34c..39cf3ae39 100644 --- a/ErsatzTV.Core/Api/Health/HealthCheckResponseModel.cs +++ b/ErsatzTV.Core/Api/Health/HealthCheckResponseModel.cs @@ -1,7 +1,8 @@ +#nullable enable namespace ErsatzTV.Core.Api.Health; public record HealthCheckResponseModel( string Title, string Status, string Detail, - string Link); + string? Link); diff --git a/ErsatzTV.Tests/Controllers/HealthControllerTests.cs b/ErsatzTV.Tests/Controllers/HealthControllerTests.cs index 113732a89..ca8cbfc2c 100644 --- a/ErsatzTV.Tests/Controllers/HealthControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/HealthControllerTests.cs @@ -1,7 +1,9 @@ +using System.Reflection; using ErsatzTV.Application.Health; using ErsatzTV.Controllers.Api; using ErsatzTV.Core.Api.Health; using MediatR; +using Microsoft.AspNetCore.Mvc.Routing; using NSubstitute; using NUnit.Framework; using Shouldly; @@ -21,6 +23,18 @@ public class HealthControllerTests _controller = new HealthController(_mediator); } + [Test] + public void Controller_Should_Expose_Idiomatic_Rest_Route() + { + MethodInfo action = typeof(HealthController).GetMethod(nameof(HealthController.GetAll)) + ?? throw new AssertionException("Missing action GetAll"); + + HttpMethodAttribute attribute = action.GetCustomAttributes(inherit: true).Single(); + attribute.HttpMethods.ShouldContain("GET"); + attribute.Template.ShouldBe("/api/health"); + attribute.Name.ShouldBe("GetHealthChecks"); + } + [Test] public async Task GetAll_Should_Return_Results_From_Mediator() { @@ -37,4 +51,15 @@ public class HealthControllerTests result.ShouldBe(expected); } + + [Test] + public async Task GetAll_Should_Return_Empty_List_When_None_Exist() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns([]); + + List result = await _controller.GetAll(CancellationToken.None); + + result.ShouldBeEmpty(); + } } diff --git a/ErsatzTV/Controllers/Api/HealthController.cs b/ErsatzTV/Controllers/Api/HealthController.cs index 3c865fd49..9398c1e03 100644 --- a/ErsatzTV/Controllers/Api/HealthController.cs +++ b/ErsatzTV/Controllers/Api/HealthController.cs @@ -6,12 +6,12 @@ using Microsoft.AspNetCore.Mvc; namespace ErsatzTV.Controllers.Api; [ApiController] -[EndpointGroupName("general")] public class HealthController(IMediator mediator) : ControllerBase { - [HttpGet("/api/health")] + [HttpGet("/api/health", Name = "GetHealthChecks")] [Tags("Health")] [EndpointSummary("Get health check results")] + [EndpointGroupName("general")] [ProducesResponseType(typeof(List), StatusCodes.Status200OK)] public async Task> GetAll(CancellationToken cancellationToken) => await mediator.Send(new GetAllHealthCheckResultsForApi(), cancellationToken);