Skip to content

Commit da51714

Browse files
LEGLINK-887: Cover the NotFound and 500 Problem Details contracts
Follow-up to review feedback on #1779: AssertBadRequestProblem was the only assertion helper, so NotFoundProblem and InternalServerErrorProblem shipped untested. - Generalises the helper to AssertProblem(result, status, title, type, detail) and keeps AssertBadRequestProblem as a wrapper, leaving the existing call sites unchanged. - Adds 404 coverage via GetValueSetById with an id the cache does not hold, and 500 coverage via a code group cached under the ValueSet type whose resource is a CodeSystem. - Exercises the configured CustomizeProblemDetails callback directly, with no host and no HTTP call, to assert a 5xx detail is replaced by the generic message and a traceId is added. A paired client-error test pins the scrubbing to 5xx so an over-broad change cannot silently erase the actionable 4xx detail this change exists to deliver. The controller-level 500 test asserts status, title and type only: ProblemDetailsFactory is null in unit tests, so ControllerBase.Problem builds a plain ProblemDetails and the scrubbing belongs to the customization test. Testing: 77 unit tests pass in UnitTests.Terminology, up from 73. Test project only; no production code changed.
1 parent 94eed94 commit da51714

1 file changed

Lines changed: 109 additions & 8 deletions

File tree

DotNet/ServiceTests/UnitTests/Terminology/Controllers/FhirControllerTests.cs

Lines changed: 109 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
using Hl7.Fhir.Model;
2+
using LantanaGroup.Link.Terminology.Application.Extensions;
23
using LantanaGroup.Link.Terminology.Application.Interfaces;
34
using LantanaGroup.Link.Terminology.Application.Models;
45
using LantanaGroup.Link.Terminology.Controllers;
56
using LantanaGroup.Link.Terminology.Services;
7+
using Microsoft.AspNetCore.Hosting;
68
using Microsoft.AspNetCore.Http;
79
using Microsoft.AspNetCore.Mvc;
10+
using Microsoft.Extensions.DependencyInjection;
11+
using Microsoft.Extensions.Hosting;
812
using Microsoft.Extensions.Logging;
13+
using Microsoft.Extensions.Options;
914
using Moq;
1015
using Xunit;
1116
using Code = LantanaGroup.Link.Terminology.Application.Models.Code;
@@ -55,25 +60,32 @@ private static Parameters AssertOkParameters(ActionResult<Parameters> result)
5560
}
5661

5762
/// <summary>
58-
/// Asserts that the action produced an RFC 9457 Problem Details 400 carrying <paramref name="expectedDetail"/>.
63+
/// Asserts that the action produced an RFC 9457 Problem Details result with the given status, title,
64+
/// type and detail.
5965
/// </summary>
6066
/// <remarks>
6167
/// No <see cref="HttpContext"/> is wired up, so the controller's <c>ProblemDetailsFactory</c> is null and
6268
/// <c>ControllerBase.Problem</c> builds a plain <see cref="ProblemDetails"/> from its arguments. The runtime
63-
/// <c>traceId</c> is injected by the app's configured factory and is out of scope here (see ConfigControllerTests).
69+
/// <c>traceId</c> extension and the scrubbing of 5xx detail are applied by the configured customization,
70+
/// which is covered separately below (see ConfigControllerTests for the same note).
6471
/// </remarks>
65-
private static void AssertBadRequestProblem(ActionResult<Parameters> result, string expectedDetail)
72+
private static void AssertProblem(
73+
ActionResult? result, int expectedStatus, string expectedTitle, string expectedType, string expectedDetail)
6674
{
67-
var objectResult = Assert.IsType<ObjectResult>(result.Result);
68-
Assert.Equal(StatusCodes.Status400BadRequest, objectResult.StatusCode);
75+
var objectResult = Assert.IsType<ObjectResult>(result);
76+
Assert.Equal(expectedStatus, objectResult.StatusCode);
6977

7078
var problem = Assert.IsType<ProblemDetails>(objectResult.Value);
71-
Assert.Equal("Bad Request", problem.Title);
72-
Assert.Equal(StatusCodes.Status400BadRequest, problem.Status);
73-
Assert.Equal("https://tools.ietf.org/html/rfc9110#section-15.5.1", problem.Type);
79+
Assert.Equal(expectedTitle, problem.Title);
80+
Assert.Equal(expectedStatus, problem.Status);
81+
Assert.Equal(expectedType, problem.Type);
7482
Assert.Equal(expectedDetail, problem.Detail);
7583
}
7684

85+
private static void AssertBadRequestProblem(ActionResult<Parameters> result, string expectedDetail) =>
86+
AssertProblem(result.Result, StatusCodes.Status400BadRequest, "Bad Request",
87+
"https://tools.ietf.org/html/rfc9110#section-15.5.1", expectedDetail);
88+
7789
[Fact]
7890
public void ValidateCodeInValueSet_WithDisplayContainingAmpersand_ReturnsTrue()
7991
{
@@ -173,4 +185,93 @@ public void ValidateCodeInValueSet_WithEmptyValueUriInBody_ReturnsBadRequest()
173185
// Assert
174186
AssertBadRequestProblem(result, "No id or url parameter specified.");
175187
}
188+
189+
[Fact]
190+
public void GetValueSetById_WhenValueSetNotLoaded_ReturnsNotFoundProblem()
191+
{
192+
// Arrange - the cache has no value set under this id
193+
_mockCacheService
194+
.Setup(x => x.GetCodeGroupById(CodeGroup.CodeGroupTypes.ValueSet, "missing-vs", It.IsAny<string>()))
195+
.Returns((CodeGroup?)null);
196+
197+
// Act
198+
var result = _controller.GetValueSetById("missing-vs");
199+
200+
// Assert
201+
AssertProblem(result.Result, StatusCodes.Status404NotFound, "Not Found",
202+
"https://tools.ietf.org/html/rfc9110#section-15.5.5", "Value set not found with ID missing-vs.");
203+
}
204+
205+
[Fact]
206+
public void GetValueSets_WhenCachedResourceIsNotAValueSet_ReturnsInternalServerErrorProblem()
207+
{
208+
// Arrange - a code group cached under the ValueSet type whose resource is a CodeSystem
209+
var mismatched = BuildCodeGroup(CodeGroup.CodeGroupTypes.ValueSet, CodeSystemUrl);
210+
mismatched.Resource = new CodeSystem { Id = "not-a-value-set", Url = CodeSystemUrl };
211+
212+
_mockCacheService
213+
.Setup(x => x.GetCodeGroup(CodeGroup.CodeGroupTypes.ValueSet, ValueSetUrl, It.IsAny<string>()))
214+
.Returns(mismatched);
215+
216+
// Act
217+
var result = _controller.GetValueSets(ValueSetUrl, null);
218+
219+
// Assert - the controller sets the 5xx contract; the customization scrubs the detail at runtime
220+
AssertProblem(result.Result, StatusCodes.Status500InternalServerError, "Internal Server Error",
221+
"https://tools.ietf.org/html/rfc9110#section-15.6.1", "Code group found is not a ValueSet.");
222+
}
223+
224+
/// <summary>
225+
/// Builds the <c>CustomizeProblemDetails</c> callback the service registers at startup, so the
226+
/// runtime-only behaviour can be exercised without standing up a host or issuing an HTTP request.
227+
/// </summary>
228+
private static Action<ProblemDetailsContext> GetConfiguredCustomization()
229+
{
230+
var environment = new Mock<IWebHostEnvironment>();
231+
environment.SetupGet(e => e.EnvironmentName).Returns(Environments.Production);
232+
233+
var options = new ServiceCollection()
234+
.AddTerminologyProblemDetails(environment.Object)
235+
.BuildServiceProvider()
236+
.GetRequiredService<IOptions<ProblemDetailsOptions>>();
237+
238+
return Assert.IsType<Action<ProblemDetailsContext>>(options.Value.CustomizeProblemDetails);
239+
}
240+
241+
private static ProblemDetailsContext BuildContext(int status, string detail) => new()
242+
{
243+
HttpContext = new DefaultHttpContext(),
244+
ProblemDetails = new ProblemDetails { Status = status, Detail = detail }
245+
};
246+
247+
[Fact]
248+
public void ProblemDetailsCustomization_ForServerError_ReplacesRawExceptionDetail()
249+
{
250+
// Arrange - the raw message a 500 would otherwise carry out of the controller
251+
var context = BuildContext(StatusCodes.Status500InternalServerError, "Value set could not be copied.");
252+
253+
// Act
254+
GetConfiguredCustomization()(context);
255+
256+
// Assert - internal state is replaced by a generic message, and a traceId is added to correlate
257+
Assert.Equal(
258+
"An error occurred in our API. Please use the trace id when requesting assistance.",
259+
context.ProblemDetails.Detail);
260+
Assert.DoesNotContain("Value set could not be copied", context.ProblemDetails.Detail);
261+
Assert.True(context.ProblemDetails.Extensions.ContainsKey("traceId"));
262+
}
263+
264+
[Fact]
265+
public void ProblemDetailsCustomization_ForClientError_PreservesDetail()
266+
{
267+
// Arrange - scrubbing must be limited to 5xx; a 4xx detail is actionable and must survive
268+
var context = BuildContext(StatusCodes.Status404NotFound, "Value set not found with ID missing-vs.");
269+
270+
// Act
271+
GetConfiguredCustomization()(context);
272+
273+
// Assert
274+
Assert.Equal("Value set not found with ID missing-vs.", context.ProblemDetails.Detail);
275+
Assert.True(context.ProblemDetails.Extensions.ContainsKey("traceId"));
276+
}
176277
}

0 commit comments

Comments
 (0)