Skip to content

Commit 7b06836

Browse files
LEGLINK-582: Omit the MeasureReport locator when the MeasureReport has no id (#1755)
resolveMeasureReport derives the ref's id from getIdElement().getIdPart(), which returns null for a MeasureReport carrying no id. The locator guard only checked that the ref itself was non-null, so a non-null ref with a null id reached String.format and emitted Bundle.entry[0].resource.ofType(MeasureReport).where(id = 'null').extension[0] — valid FHIRPath that resolves to nothing. The issue still looked well-formed, so a dead locator would ship in submitted output with nothing logged. The null-ref path warned; the null-id path did not. - Require a non-null id alongside the non-null ref, falling through to the existing omission path. Broaden the warning to "No identifiable MeasureReport", since it now covers both causes. - Rename build_nullMeasureReportId_omitsLocatorButKeepsResultExpressions to build_noMeasureReportRef_...: it passes null for the whole ref, so despite its name it never covered a null id. That misnaming is why the gap survived review. - Add build_measureReportWithNullId_... for the case the old name claimed, and resolveMeasureReport_measureReportWithoutId_yieldsRefWithNullId to pin the source of the null id, so the guard is re-examined rather than silently orphaned if resolution ever starts synthesizing an id. Raised by CodeRabbit on #1753 after it merged, hence the follow-up branch. Testing: mvn -pl validation -am test on JDK 17, 131 tests pass (0 failures, 0 errors); PreQualOperationOutcomeBuilderTest grew 17 -> 19. The new null-id test was confirmed to fail without the guard, reproducing where(id = 'null') exactly, so it pins the fix rather than passing vacuously.
1 parent c664e85 commit 7b06836

2 files changed

Lines changed: 36 additions & 3 deletions

File tree

Java/validation/src/main/java/com/lantanagroup/link/validation/services/PreQualOperationOutcomeBuilder.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,14 @@ public Optional<OperationOutcome> build(List<Result> results, MeasureReportRef m
9696
issue.addExtension(new Extension(PQ_ISSUE_CAT_URL, new CodeType(categoryId)));
9797

9898
if (writeExpressions) {
99-
if (measureReport != null) {
99+
// The id is checked as well as the ref: a MeasureReport carrying no id yields a non-null
100+
// ref with a null id, and formatting that produces where(id = 'null') — valid FHIRPath
101+
// that silently resolves to nothing. Omitting the locator is the honest outcome.
102+
if (measureReport != null && measureReport.id() != null) {
100103
issue.addExpression(
101104
String.format(MEASURE_REPORT_LOCATOR, measureReport.index(), measureReport.id()));
102105
} else {
103-
_logger.warn("No MeasureReport in the bundle; omitting the MeasureReport locator expression");
106+
_logger.warn("No identifiable MeasureReport in the bundle; omitting the MeasureReport locator expression");
104107
}
105108
categoryResults.forEach(r -> issue.addExpression(r.getExpression()));
106109
}

Java/validation/src/test/java/com/lantanagroup/link/validation/services/PreQualOperationOutcomeBuilderTest.java

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ void build_writeExpressionsFalse_omitsAllExpressions() {
199199
}
200200

201201
@Test
202-
void build_nullMeasureReportId_omitsLocatorButKeepsResultExpressions() {
202+
void build_noMeasureReportRef_omitsLocatorButKeepsResultExpressions() {
203203
Result r1 = result("msg", "result-expr", category("inactive_code", false));
204204

205205
OperationOutcome oo = builder.build(List.of(r1), null, true).orElseThrow();
@@ -208,10 +208,40 @@ void build_nullMeasureReportId_omitsLocatorButKeepsResultExpressions() {
208208
assertEquals(List.of("result-expr"), expressions);
209209
}
210210

211+
@Test
212+
void build_measureReportWithNullId_omitsLocatorButKeepsResultExpressions() {
213+
// A MeasureReport with no id resolves to a non-null ref whose id is null. Formatting that would
214+
// emit where(id = 'null') — valid FHIRPath that resolves to nothing, i.e. a silently dead locator
215+
// in submitted output. The locator must be omitted instead.
216+
Result r1 = result("msg", "result-expr", category("inactive_code", false));
217+
PreQualOperationOutcomeBuilder.MeasureReportRef ref =
218+
new PreQualOperationOutcomeBuilder.MeasureReportRef(0, null);
219+
220+
OperationOutcome oo = builder.build(List.of(r1), ref, true).orElseThrow();
221+
222+
List<String> expressions = expressionStrings(oo.getIssueFirstRep());
223+
assertEquals(List.of("result-expr"), expressions);
224+
assertFalse(expressions.stream().anyMatch(e -> e.contains("null")),
225+
"Locator must not be emitted with a null id");
226+
}
227+
211228
// -------------------------------------------------------------------------
212229
// MeasureReport resolution (index + id)
213230
// -------------------------------------------------------------------------
214231

232+
@Test
233+
void resolveMeasureReport_measureReportWithoutId_yieldsRefWithNullId() {
234+
// Pins the source of the null id the builder has to defend against.
235+
Bundle bundle = new Bundle();
236+
bundle.addEntry().setResource(new MeasureReport());
237+
238+
PreQualOperationOutcomeBuilder.MeasureReportRef ref = builder.resolveMeasureReport(bundle);
239+
240+
assertNotNull(ref);
241+
assertEquals(0, ref.index());
242+
assertNull(ref.id());
243+
}
244+
215245
@Test
216246
void resolveMeasureReport_returnsIndexAndIdWhenFirstEntry() {
217247
Bundle bundle = new Bundle();

0 commit comments

Comments
 (0)