Skip to content

Commit 7635e5d

Browse files
LEGLINK-888: ValueSet $validate-code rejects a blank "system" with a 400
An empty "system" was folded into the absent case by string.IsNullOrEmpty and answered by searching every code system in the value set, so TestRail 10992's request returned 200 result=true. An absent system is legitimate FHIR and keeps that meaning; an empty string is not valid for any FHIR primitive, so the request is malformed and is now rejected rather than reinterpreted into a broader question than the caller asked. - Adds NormalizeSystem and validates every client-supplied system up front in ValidateCodeInValueSet: the query parameter, the body "system" parameter, coding.system and each codeableConcept.coding.system. Validating before the merges matters because they treat an empty string as "not supplied" and would overwrite a blank before anything could see it, and it keeps the verdict for a malformed codeableConcept independent of which coding happens to match first. - Whitespace-only is treated as blank, matching the FHIR rule that a primitive carries at least one character of non-whitespace content. - The literal "null" and "undefined" are treated as an omitted parameter. This is deliberate leniency for clients that interpolate an unset variable into a request rather than FHIR behavior, so it is pinned by tests. - The string.IsNullOrEmpty check in ValidateCodeInCodeGroup is left alone: it is shared with ValidateCodeInCodeSystem, which passes the code group's own Url there, and that is cache content rather than client input. - Registers PreserveEmptyStringMetadataProvider. MVC converts an empty query value to null before an action runs, so "?system=" reached the action indistinguishable from an omitted parameter and returned 200 regardless of the validation above. MvcOptions exposes no setting for this and DisplayFormatAttribute cannot target a parameter, so a display metadata provider is the supported route. Body binding is unaffected; FhirModelBinder deserializes with System.Text.Json and never consults MVC metadata. - Adds FhirControllerHttpTests, which drives the four QA requests (TestRail 10992, 11015, 11342, 11343) over real HTTP through the configured binding pipeline. Calling the action directly cannot reach the query-string case: a direct call passing string.Empty exercises a state real traffic cannot produce. Testing: 97 unit tests pass in UnitTests.Terminology, up from 77, 20 of them new; the full .NET unit suite passes at 1128. Confirmed the metadata provider is load-bearing by removing it and observing only the "?system=" test fail. Verified against the local docker-compose stack: before the change all four QA requests returned 200 result=true; after rebuilding the image each returns application/problem+json carrying type, title, status, the expected detail and a W3C traceId. An omitted system and a valid system are unchanged at 200; "?system=null" now searches all systems rather than being looked up as a system URL, so it returns result=true where it previously returned result=false.
1 parent 908f46f commit 7635e5d

6 files changed

Lines changed: 532 additions & 9 deletions

File tree

Lines changed: 255 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,255 @@
1+
using System.Net;
2+
using System.Text;
3+
using System.Text.Json;
4+
using LantanaGroup.Link.Terminology.Application.Formatters;
5+
using LantanaGroup.Link.Terminology.Application.Interfaces;
6+
using LantanaGroup.Link.Terminology.Application.Models;
7+
using LantanaGroup.Link.Terminology.Controllers;
8+
using LantanaGroup.Link.Terminology.Services;
9+
using Microsoft.AspNetCore.Builder;
10+
using Microsoft.AspNetCore.Hosting;
11+
using Microsoft.AspNetCore.TestHost;
12+
using Microsoft.Extensions.DependencyInjection;
13+
using Moq;
14+
using Xunit;
15+
using Code = LantanaGroup.Link.Terminology.Application.Models.Code;
16+
using Task = System.Threading.Tasks.Task;
17+
18+
namespace UnitTests.Terminology;
19+
20+
/// <summary>
21+
/// Exercises ValueSet $validate-code over real HTTP, through the same model binding and MVC options
22+
/// <c>Program.cs</c> configures, for the four blank-system requests QA covers in TestRail 10992, 11015,
23+
/// 11342 and 11343 (LEGLINK-888).
24+
/// </summary>
25+
/// <remarks>
26+
/// Calling the action method directly cannot cover these. MVC converts an empty query value to null before
27+
/// an action runs unless <see cref="PreserveEmptyStringMetadataProvider"/> is registered, so a direct call
28+
/// passing <c>string.Empty</c> exercises a state real traffic could not produce and would report a blank
29+
/// <c>?system=</c> as rejected while the deployed service answered 200.
30+
///
31+
/// The MVC options below mirror <c>Program.cs</c> by hand rather than booting the real host, whose startup
32+
/// needs Kafka, the cache and App Configuration. The two must be kept in step: dropping the metadata
33+
/// provider from <c>Program.cs</c> alone would break the deployed service without failing these tests.
34+
/// </remarks>
35+
public class FhirControllerHttpTests
36+
{
37+
private const string ValueSetUrl = "http://hl7.org/fhir/ValueSet/address-type";
38+
private const string CodeSystemUrl = "http://hl7.org/fhir/address-type";
39+
private const string Endpoint = "/api/terminology/fhir/ValueSet/$validate-code";
40+
41+
/// <summary>
42+
/// Stands up the controller behind a real request pipeline, mirroring the MVC configuration in
43+
/// <c>Program.cs</c>. The value set is populated so a request that still fails did so on the system
44+
/// rather than on a failed lookup.
45+
/// </summary>
46+
private static TestServer BuildServer()
47+
{
48+
var cache = new Mock<ICodeGroupCacheService>();
49+
cache.Setup(x => x.GetCodeGroup(CodeGroup.CodeGroupTypes.ValueSet, ValueSetUrl, It.IsAny<string>()))
50+
.Returns(new CodeGroup
51+
{
52+
Id = "address-type",
53+
Type = CodeGroup.CodeGroupTypes.ValueSet,
54+
Url = ValueSetUrl,
55+
Codes = new Dictionary<string, List<Code>>
56+
{
57+
{ CodeSystemUrl, new List<Code> { new() { Value = "postal", Display = "Postal" } } }
58+
}
59+
});
60+
61+
var builder = new WebHostBuilder()
62+
.ConfigureServices(services =>
63+
{
64+
services.AddLogging();
65+
services.AddSingleton(cache.Object);
66+
services.AddSingleton<FhirService>();
67+
services.AddControllers(options =>
68+
{
69+
options.ModelBinderProviders.Insert(0, new FhirModelBinderProvider());
70+
options.OutputFormatters.Insert(0, new FhirOutputFormatter());
71+
options.ModelMetadataDetailsProviders.Add(new PreserveEmptyStringMetadataProvider());
72+
})
73+
.AddApplicationPart(typeof(FhirController).Assembly);
74+
})
75+
.Configure(app =>
76+
{
77+
app.UseRouting();
78+
app.UseEndpoints(endpoints => endpoints.MapControllers());
79+
});
80+
81+
return new TestServer(builder);
82+
}
83+
84+
private static async Task<(HttpStatusCode Status, string Body)> PostAsync(string query, string body)
85+
{
86+
using var server = BuildServer();
87+
using var client = server.CreateClient();
88+
89+
var content = new StringContent(body, Encoding.UTF8, "application/json");
90+
var response = await client.PostAsync($"{Endpoint}{query}", content);
91+
92+
return (response.StatusCode, await response.Content.ReadAsStringAsync());
93+
}
94+
95+
private static void AssertBadRequestDetail(HttpStatusCode status, string body, string expectedDetail)
96+
{
97+
Assert.Equal(HttpStatusCode.BadRequest, status);
98+
99+
using var document = JsonDocument.Parse(body);
100+
var root = document.RootElement;
101+
102+
Assert.Equal("https://tools.ietf.org/html/rfc9110#section-15.5.1", root.GetProperty("type").GetString());
103+
Assert.Equal("Bad Request", root.GetProperty("title").GetString());
104+
Assert.Equal(400, root.GetProperty("status").GetInt32());
105+
Assert.Equal(expectedDetail, root.GetProperty("detail").GetString());
106+
}
107+
108+
/// <summary>TestRail 10992 — blank system inside the body's coding.</summary>
109+
[Fact]
110+
public async Task ValidateCodeInValueSet_BlankSystemInCodingBody_Returns400()
111+
{
112+
const string body = """
113+
{
114+
"resourceType" : "Parameters",
115+
"parameter" : [{
116+
"name": "url",
117+
"valueUri": "http://hl7.org/fhir/ValueSet/address-type"
118+
}, {
119+
"name" : "coding",
120+
"valueCoding": { "code": "postal", "system": "" }
121+
}]
122+
}
123+
""";
124+
125+
var (status, payload) = await PostAsync(string.Empty, body);
126+
127+
AssertBadRequestDetail(status, payload, "The 'coding.system' parameter cannot be blank.");
128+
}
129+
130+
/// <summary>TestRail 11015 — blank system on the query string, valid system in the body.</summary>
131+
[Fact]
132+
public async Task ValidateCodeInValueSet_BlankSystemQueryParameter_Returns400()
133+
{
134+
const string body = """
135+
{
136+
"resourceType" : "Parameters",
137+
"parameter" : [{
138+
"name": "url",
139+
"valueUri": "http://hl7.org/fhir/ValueSet/address-type"
140+
}, {
141+
"name" : "coding",
142+
"valueCoding": { "code": "postal", "system": "http://hl7.org/fhir/address-type" }
143+
}]
144+
}
145+
""";
146+
147+
var (status, payload) = await PostAsync("?system=", body);
148+
149+
AssertBadRequestDetail(status, payload, "The 'system' parameter cannot be blank.");
150+
}
151+
152+
/// <summary>TestRail 11342 — blank system as a top-level body parameter.</summary>
153+
[Fact]
154+
public async Task ValidateCodeInValueSet_BlankSystemParameterInBody_Returns400()
155+
{
156+
const string body = """
157+
{
158+
"resourceType" : "Parameters",
159+
"parameter" : [{
160+
"name": "url",
161+
"valueUri": "http://hl7.org/fhir/ValueSet/address-type"
162+
}, {
163+
"name" : "code",
164+
"valueCode": "postal"
165+
}, {
166+
"name": "system",
167+
"valueUri": ""
168+
}]
169+
}
170+
""";
171+
172+
var (status, payload) = await PostAsync(string.Empty, body);
173+
174+
AssertBadRequestDetail(status, payload, "The 'system' parameter cannot be blank.");
175+
}
176+
177+
/// <summary>TestRail 11343 — blank system inside the body's codeableConcept.</summary>
178+
[Fact]
179+
public async Task ValidateCodeInValueSet_BlankSystemInCodeableConcept_Returns400()
180+
{
181+
const string body = """
182+
{
183+
"resourceType" : "Parameters",
184+
"parameter" : [{
185+
"name": "url",
186+
"valueUri": "http://hl7.org/fhir/ValueSet/address-type"
187+
}, {
188+
"name" : "codeableConcept",
189+
"valueCodeableConcept": {
190+
"coding": [{ "code": "postal", "system": "" }]
191+
}
192+
}]
193+
}
194+
""";
195+
196+
var (status, payload) = await PostAsync(string.Empty, body);
197+
198+
AssertBadRequestDetail(status, payload, "The 'codeableConcept.coding.system' parameter cannot be blank.");
199+
}
200+
201+
/// <summary>
202+
/// An omitted system keeps its FHIR meaning of "search every code system in the value set", so the
203+
/// rejection above must not be reached by simply leaving the parameter out.
204+
/// </summary>
205+
[Fact]
206+
public async Task ValidateCodeInValueSet_AbsentSystem_SearchesAllSystemsAndSucceeds()
207+
{
208+
const string body = """
209+
{
210+
"resourceType" : "Parameters",
211+
"parameter" : [{
212+
"name": "url",
213+
"valueUri": "http://hl7.org/fhir/ValueSet/address-type"
214+
}, {
215+
"name" : "coding",
216+
"valueCoding": { "code": "postal" }
217+
}]
218+
}
219+
""";
220+
221+
var (status, payload) = await PostAsync(string.Empty, body);
222+
223+
Assert.Equal(HttpStatusCode.OK, status);
224+
Assert.Contains("\"result\"", payload);
225+
Assert.Contains("true", payload);
226+
}
227+
228+
/// <summary>
229+
/// Per LEGLINK-888, a client that interpolated an unset variable into the query string is treated as
230+
/// having omitted the parameter. This is deliberate non-FHIR leniency, so it is pinned by a test.
231+
/// </summary>
232+
[Theory]
233+
[InlineData("?system=null")]
234+
[InlineData("?system=undefined")]
235+
public async Task ValidateCodeInValueSet_PlaceholderSystem_TreatedAsAbsent(string query)
236+
{
237+
const string body = """
238+
{
239+
"resourceType" : "Parameters",
240+
"parameter" : [{
241+
"name": "url",
242+
"valueUri": "http://hl7.org/fhir/ValueSet/address-type"
243+
}, {
244+
"name" : "code",
245+
"valueCode": "postal"
246+
}]
247+
}
248+
""";
249+
250+
var (status, payload) = await PostAsync(query, body);
251+
252+
Assert.Equal(HttpStatusCode.OK, status);
253+
Assert.Contains("\"result\"", payload);
254+
}
255+
}

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,58 @@ public void ValidateCodeInValueSet_WithEmptyValueUriInBody_ReturnsBadRequest()
186186
AssertBadRequestProblem(result, "No id or url parameter specified.");
187187
}
188188

189+
[Fact]
190+
public void ValidateCodeInValueSet_WithBlankSystemInCodingBody_ReturnsBadRequest()
191+
{
192+
// Arrange - LEGLINK-888's reported request: a resolvable value set and a code that does match,
193+
// with the coding's system blank. The value set is wired up deliberately, so the 400 proves the
194+
// blank is rejected rather than the request merely failing to find anything.
195+
_mockCacheService
196+
.Setup(x => x.GetCodeGroup(CodeGroup.CodeGroupTypes.ValueSet, ValueSetUrl, It.IsAny<string>()))
197+
.Returns(BuildCodeGroup(CodeGroup.CodeGroupTypes.ValueSet, CodeSystemUrl));
198+
199+
var parameters = new Parameters();
200+
parameters.Add("url", new FhirUri(ValueSetUrl));
201+
parameters.Add("coding", new Coding { Code = LoincCode, System = string.Empty });
202+
203+
// Act
204+
var result = _controller.ValidateCodeInValueSet(null, null, null, null, null, parameters);
205+
206+
// Assert - previously this answered 200 result=true by searching every system in the value set
207+
AssertBadRequestProblem(result, "The 'coding.system' parameter cannot be blank.");
208+
}
209+
210+
[Fact]
211+
public void ValidateCodeInValueSet_WithBlankSystemQueryParameter_ReturnsBadRequest()
212+
{
213+
// Arrange
214+
_mockCacheService
215+
.Setup(x => x.GetCodeGroup(CodeGroup.CodeGroupTypes.ValueSet, ValueSetUrl, It.IsAny<string>()))
216+
.Returns(BuildCodeGroup(CodeGroup.CodeGroupTypes.ValueSet, CodeSystemUrl));
217+
218+
// Act - an empty query string value binds as "", not as an absent parameter
219+
var result = _controller.ValidateCodeInValueSet(ValueSetUrl, null, string.Empty, LoincCode, null, null);
220+
221+
// Assert
222+
AssertBadRequestProblem(result, "The 'system' parameter cannot be blank.");
223+
}
224+
225+
[Fact]
226+
public void ValidateCodeInValueSet_WithNullPlaceholderSystem_SearchesAllSystems()
227+
{
228+
// Arrange
229+
_mockCacheService
230+
.Setup(x => x.GetCodeGroup(CodeGroup.CodeGroupTypes.ValueSet, ValueSetUrl, It.IsAny<string>()))
231+
.Returns(BuildCodeGroup(CodeGroup.CodeGroupTypes.ValueSet, CodeSystemUrl));
232+
233+
// Act - a client that interpolated an unset variable sends the literal string "null"
234+
var result = _controller.ValidateCodeInValueSet(ValueSetUrl, null, "null", LoincCode, null, null);
235+
236+
// Assert - treated as if the system had been omitted rather than looked up as a system URL
237+
var parameters = AssertOkParameters(result);
238+
Assert.True(parameters.GetSingleValue<FhirBoolean>("result")?.Value);
239+
}
240+
189241
[Fact]
190242
public void GetValueSetById_WhenValueSetNotLoaded_ReturnsNotFoundProblem()
191243
{

0 commit comments

Comments
 (0)