Skip to content

Commit 94eed94

Browse files
MikeAtPinnacleMikeAtPinnacle
committed
LEGLINK-887: Terminology FHIR endpoints return Problem Details on error
FhirController answered every error with a bare string body and text/plain, so a rejected request carried no type, title, status or traceId. LEGLINK-886 corrected the status code for a request naming no value set, but the payload was still a plain string; this makes the body match the RFC 9457 format ConfigController has returned since LEGLINK-591. - Adds TerminologyProblem plus BadRequestProblem, NotFoundProblem and InternalServerErrorProblem helpers, and routes all 13 error returns across the six actions through them (7 BadRequest, 3 NotFound, 3 raw 500). - traceId comes from the existing AddTerminologyProblemDetails customization. - detail is prose, so the helper appends a terminating period. Exception messages stay fragments because they are also read from logs and asserted in unit tests. - 500 responses no longer place the exception message in the body; the customization substitutes a generic detail so internal state is not exposed to the caller. Testing: 73 unit tests pass in UnitTests.Terminology, one new covering LEGLINK-887's reported request (empty valueUri in the POST body). Verified against the local docker-compose stack that an empty valueUri in the body, an empty url query parameter, a markup-only display and an unknown ValueSet id all return application/problem+json carrying type, title, status, detail and a W3C traceId, and that the $validate-code success and failure payloads are unchanged.
1 parent 3c957dd commit 94eed94

2 files changed

Lines changed: 87 additions & 21 deletions

File tree

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

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using LantanaGroup.Link.Terminology.Application.Models;
44
using LantanaGroup.Link.Terminology.Controllers;
55
using LantanaGroup.Link.Terminology.Services;
6+
using Microsoft.AspNetCore.Http;
67
using Microsoft.AspNetCore.Mvc;
78
using Microsoft.Extensions.Logging;
89
using Moq;
@@ -53,6 +54,26 @@ private static Parameters AssertOkParameters(ActionResult<Parameters> result)
5354
return Assert.IsType<Parameters>(okResult.Value);
5455
}
5556

57+
/// <summary>
58+
/// Asserts that the action produced an RFC 9457 Problem Details 400 carrying <paramref name="expectedDetail"/>.
59+
/// </summary>
60+
/// <remarks>
61+
/// No <see cref="HttpContext"/> is wired up, so the controller's <c>ProblemDetailsFactory</c> is null and
62+
/// <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).
64+
/// </remarks>
65+
private static void AssertBadRequestProblem(ActionResult<Parameters> result, string expectedDetail)
66+
{
67+
var objectResult = Assert.IsType<ObjectResult>(result.Result);
68+
Assert.Equal(StatusCodes.Status400BadRequest, objectResult.StatusCode);
69+
70+
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);
74+
Assert.Equal(expectedDetail, problem.Detail);
75+
}
76+
5677
[Fact]
5778
public void ValidateCodeInValueSet_WithDisplayContainingAmpersand_ReturnsTrue()
5879
{
@@ -99,8 +120,7 @@ public void ValidateCodeInValueSet_WithMarkupOnlyDisplay_ReturnsBadRequest()
99120

100121
// Assert - the display sanitizes away to nothing; passing the empty value on would skip the
101122
// display check and answer result=true, so the request is rejected instead
102-
var badRequest = Assert.IsType<BadRequestObjectResult>(result.Result);
103-
Assert.Equal("Invalid value supplied for 'display'", badRequest.Value);
123+
AssertBadRequestProblem(result, "Invalid value supplied for 'display'.");
104124
}
105125

106126
[Fact]
@@ -116,8 +136,7 @@ public void ValidateCodeInCodeSystem_WithMarkupOnlyDisplay_ReturnsBadRequest()
116136
CodeSystemUrl, null, LoincCode, "<script>alert('x')</script>", null);
117137

118138
// Assert
119-
var badRequest = Assert.IsType<BadRequestObjectResult>(result.Result);
120-
Assert.Equal("Invalid value supplied for 'display'", badRequest.Value);
139+
AssertBadRequestProblem(result, "Invalid value supplied for 'display'.");
121140
}
122141

123142
[Fact]
@@ -127,8 +146,7 @@ public void ValidateCodeInValueSet_WithNoUrlOrId_ReturnsBadRequest()
127146
var result = _controller.ValidateCodeInValueSet(null, null, null, LoincCode, null, null);
128147

129148
// Assert
130-
var badRequest = Assert.IsType<BadRequestObjectResult>(result.Result);
131-
Assert.Equal("No id or url parameter specified", badRequest.Value);
149+
AssertBadRequestProblem(result, "No id or url parameter specified.");
132150
}
133151

134152
[Fact]
@@ -138,7 +156,21 @@ public void ValidateCodeInValueSet_WithEmptyUrl_ReturnsBadRequest()
138156
var result = _controller.ValidateCodeInValueSet(string.Empty, null, null, LoincCode, null, null);
139157

140158
// Assert
141-
var badRequest = Assert.IsType<BadRequestObjectResult>(result.Result);
142-
Assert.Equal("No id or url parameter specified", badRequest.Value);
159+
AssertBadRequestProblem(result, "No id or url parameter specified.");
160+
}
161+
162+
[Fact]
163+
public void ValidateCodeInValueSet_WithEmptyValueUriInBody_ReturnsBadRequest()
164+
{
165+
// Arrange - LEGLINK-887's reported request: the url arrives in the POST body as an empty valueUri
166+
var parameters = new Parameters();
167+
parameters.Add("url", new FhirUri(string.Empty));
168+
parameters.Add("code", new FhirString(LoincCode));
169+
170+
// Act
171+
var result = _controller.ValidateCodeInValueSet(null, null, null, null, null, parameters);
172+
173+
// Assert
174+
AssertBadRequestProblem(result, "No id or url parameter specified.");
143175
}
144176
}

‎DotNet/Terminology/Controllers/FhirController.cs‎

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,40 @@ public class FhirController(FhirService fhirService) : Controller
5454
return sanitized;
5555
}
5656

57+
/// <summary>
58+
/// Builds an RFC 9457 Problem Details result for a failed terminology request, matching the shape
59+
/// <see cref="ConfigController"/> already returns. The <c>traceId</c> extension is added by the
60+
/// service-wide customization in <c>TerminologyProblemDetailsExtensions</c>.
61+
/// </summary>
62+
/// <remarks>
63+
/// Exception messages are written as fragments ("Value set not found with ID x") because they are
64+
/// also read from logs and asserted in unit tests. <c>detail</c> is prose, so a terminating period
65+
/// is added here rather than baked into every message at the throw site.
66+
/// </remarks>
67+
private ObjectResult TerminologyProblem(HttpStatusCode statusCode, string title, string type, string detail)
68+
{
69+
var sentence = detail.EndsWith('.') || detail.EndsWith('?') || detail.EndsWith('!')
70+
? detail
71+
: detail + ".";
72+
73+
return Problem(detail: sentence, statusCode: (int)statusCode, title: title, type: type);
74+
}
75+
76+
/// <summary>Client input failed validation. RFC 9110 section 15.5.1.</summary>
77+
private ObjectResult BadRequestProblem(string detail) => TerminologyProblem(
78+
HttpStatusCode.BadRequest, "Bad Request", "https://tools.ietf.org/html/rfc9110#section-15.5.1", detail);
79+
80+
/// <summary>The requested terminology resource is not loaded. RFC 9110 section 15.5.5.</summary>
81+
private ObjectResult NotFoundProblem(string detail) => TerminologyProblem(
82+
HttpStatusCode.NotFound, "Not Found", "https://tools.ietf.org/html/rfc9110#section-15.5.5", detail);
83+
84+
/// <summary>
85+
/// A loaded code group could not be used as requested. RFC 9110 section 15.6.1. The customization
86+
/// replaces <c>detail</c> with a generic message so internal state is not exposed to the caller.
87+
/// </summary>
88+
private ObjectResult InternalServerErrorProblem(string detail) => TerminologyProblem(
89+
HttpStatusCode.InternalServerError, "Internal Server Error", "https://tools.ietf.org/html/rfc9110#section-15.6.1", detail);
90+
5791
#region Value Sets
5892

5993
/// <summary>
@@ -75,11 +109,11 @@ public ActionResult<ValueSet> GetValueSetById([FromRoute] string id)
75109
}
76110
catch (ArgumentException ex)
77111
{
78-
return BadRequest(ex.Message);
112+
return BadRequestProblem(ex.Message);
79113
}
80114
catch (KeyNotFoundException ex)
81115
{
82-
return NotFound(ex.Message);
116+
return NotFoundProblem(ex.Message);
83117
}
84118
}
85119

@@ -105,11 +139,11 @@ public ActionResult<Bundle> GetValueSets([FromQuery] string? url,
105139
}
106140
catch (ArgumentException ex)
107141
{
108-
return BadRequest(ex.Message);
142+
return BadRequestProblem(ex.Message);
109143
}
110144
catch (InvalidOperationException ex)
111145
{
112-
return StatusCode(StatusCodes.Status500InternalServerError, ex.Message);
146+
return InternalServerErrorProblem(ex.Message);
113147
}
114148
}
115149

@@ -134,15 +168,15 @@ public ActionResult<ValueSet> ExpandValueSet([FromRoute] string? id, [FromQuery]
134168
}
135169
catch (ArgumentException ex)
136170
{
137-
return BadRequest(ex.Message);
171+
return BadRequestProblem(ex.Message);
138172
}
139173
catch (KeyNotFoundException ex)
140174
{
141-
return NotFound(ex.Message);
175+
return NotFoundProblem(ex.Message);
142176
}
143177
catch (InvalidOperationException ex)
144178
{
145-
return StatusCode(StatusCodes.Status500InternalServerError, ex.Message);
179+
return InternalServerErrorProblem(ex.Message);
146180
}
147181
}
148182

@@ -168,11 +202,11 @@ public ActionResult<CodeSystem> GetCodeSystemById([FromRoute] string id)
168202
}
169203
catch (ArgumentException ex)
170204
{
171-
return BadRequest(ex.Message);
205+
return BadRequestProblem(ex.Message);
172206
}
173207
catch (KeyNotFoundException ex)
174208
{
175-
return NotFound(ex.Message);
209+
return NotFoundProblem(ex.Message);
176210
}
177211
}
178212

@@ -198,11 +232,11 @@ public ActionResult<Bundle> GetCodeSystems([FromQuery] string? url, [FromQuery(N
198232
}
199233
catch (ArgumentException ex)
200234
{
201-
return BadRequest(ex.Message);
235+
return BadRequestProblem(ex.Message);
202236
}
203237
catch (InvalidOperationException ex)
204238
{
205-
return StatusCode(StatusCodes.Status500InternalServerError, ex.Message);
239+
return InternalServerErrorProblem(ex.Message);
206240
}
207241
}
208242

@@ -235,7 +269,7 @@ public ActionResult<Parameters> ValidateCodeInCodeSystem([FromQuery] string? url
235269
}
236270
catch (ArgumentException ex)
237271
{
238-
return BadRequest(ex.Message);
272+
return BadRequestProblem(ex.Message);
239273
}
240274
}
241275

@@ -271,7 +305,7 @@ public ActionResult<Parameters> ValidateCodeInValueSet([FromQuery] string? url,
271305
}
272306
catch (ArgumentException ex)
273307
{
274-
return BadRequest(ex.Message);
308+
return BadRequestProblem(ex.Message);
275309
}
276310
}
277311

0 commit comments

Comments
 (0)