LEGLINK-961: Measure eval handle fhir resourceswith identicalds (#1839)

* LEGLINK-961: Handle contained FHIR resources with identical IDs across types

BlobStorageService.normalize collected the MeasureReport's contained
resources into a map keyed by ID part alone via Collectors.toMap, which
throws IllegalStateException when two resources of different types share
an ID part (e.g. Condition/A and Observation/A) - legal in FHIR since IDs
are only unique within a type. This dead-lettered the patient's report.

Replace the single map with a type-qualified index (ResourceType/idPart)
used for typed reference resolution, plus a secondary id-part-only index
that's consulted only for untyped "#id" contained references, and only
resolved when unambiguous. Ambiguous untyped references are left
unresolved and logged with a warning rather than guessing.

Claude-Session: https://claude.ai/code/session_014XemaT8rQoX1w55yZavZbV

* LEGLINK-961: Sanitize the ambiguous-reference warning arguments

idPart comes from a Reference in externally-sourced FHIR data, so it is
attacker-influenced: a control character in it - a newline in particular -
could break out of the log line and forge subsequent entries. Pass it
through LogUtils.sanitize, which maps printable 32-255 to themselves and
replaces everything else with a space.

candidateTypes is built from ResourceType enum names and cannot carry
control characters, so sanitizing it is not load-bearing; done anyway so
both string arguments are treated uniformly and the next person editing
this line does not have to work out which one is safe.

candidates.size() is left numeric - it is an int, not attacker input, and
sanitizing would only stringify it.

No behavioural change: the printable characters in these values pass
through unchanged, so BlobStorageServiceTest's assertion on the logged
message still matches.

Tests: measureeval 88, shared 7, 0 failures.

Claude-Session: https://claude.ai/code/session_01AZubeEYCKfAmDCcgVTr18G

Author: arianamihailescu

Java/measureeval/src/main/java/com/lantanagroup/link/measureeval/services/BlobStorageService.java

@@ -10,6 +10,8 @@
 import com.lantanagroup.link.shared.entities.ReportScheduleModel;
 import com.lantanagroup.link.shared.exceptions.ValidationException;
 import com.lantanagroup.link.shared.services.ReportClient;
+import com.lantanagroup.link.shared.utils.LogUtils;
+import org.hl7.fhir.instance.model.api.IIdType;
 import org.hl7.fhir.r4.model.IdType;
 import org.hl7.fhir.r4.model.MeasureReport;
 import org.hl7.fhir.r4.model.Reference;
@@ -19,10 +21,10 @@
 
 import java.net.URI;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.UUID;
-import java.util.function.Function;
 import java.util.stream.Collectors;
 
 public class BlobStorageService {
@@ -114,9 +116,18 @@ private List<Resource> normalize(MeasureReport measureReport) {
         }
 
         List<Resource> contained = measureReport.getContained();
-        Map<String, Resource> containedByIdPart = contained.stream().collect(Collectors.toMap(
-                resource -> stripHash(resource.getIdPart()),
-                Function.identity()));
+
+        // Contained resource IDs are only guaranteed unique within a resource type (e.g. Condition/A and
+        // Observation/A can legally coexist), so the primary lookup is type-qualified. A secondary,
+        // id-part-only index is kept to resolve untyped "#id" references, but only when unambiguous.
+        Map<String, Resource> containedByTypedId = new HashMap<>();
+        Map<String, List<Resource>> containedByIdPart = new HashMap<>();
+        for (Resource resource : contained) {
+            String idPart = stripHash(resource.getIdPart());
+            containedByTypedId.put(resource.getResourceType().name() + "/" + idPart, resource);
+            containedByIdPart.computeIfAbsent(idPart, key -> new ArrayList<>()).add(resource);
+        }
+
         measureReport.setContained(null);
         measureReport.setEvaluatedResource(null);
         for (Resource resource : contained) {
@@ -127,8 +138,26 @@ private List<Resource> normalize(MeasureReport measureReport) {
         }
         for (Reference reference :
                 fhirContext.newTerser().getAllPopulatedChildElementsOfType(measureReport, Reference.class)) {
-            String idPart = stripHash(reference.getReferenceElement().getIdPart());
-            Resource resource = containedByIdPart.get(idPart);
+            IIdType referenceElement = reference.getReferenceElement();
+            String idPart = stripHash(referenceElement.getIdPart());
+            String resourceType = referenceElement.getResourceType();
+
+            Resource resource;
+            if (resourceType != null) {
+                resource = containedByTypedId.get(resourceType + "/" + idPart);
+            } else {
+                List<Resource> candidates = containedByIdPart.getOrDefault(idPart, List.of());
+                if (candidates.size() > 1) {
+                    String candidateTypes = candidates.stream()
+                            .map(candidate -> candidate.getResourceType().name())
+                            .collect(Collectors.joining(", "));
+                    logger.warn("Contained reference '#{}' is ambiguous between {} resources ({}); leaving reference unresolved",
+                            LogUtils.sanitize(idPart), candidates.size(), LogUtils.sanitize(candidateTypes));
+                    continue;
+                }
+                resource = candidates.isEmpty() ? null : candidates.get(0);
+            }
+
             if (resource == null) {
                 continue;
             }

Java/measureeval/src/test/java/com/lantanagroup/link/measureeval/services/BlobStorageServiceTest.java

@@ -1,19 +1,28 @@
 package com.lantanagroup.link.measureeval.services;
 
 import ca.uhn.fhir.context.FhirContext;
+import ch.qos.logback.classic.Level;
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.LoggerContext;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.read.ListAppender;
 import com.azure.core.util.BinaryData;
 import com.azure.storage.blob.BlobClient;
 import com.azure.storage.blob.BlobContainerClient;
 import com.lantanagroup.link.measureeval.entities.PatientReportingEvaluationStatus;
 import com.lantanagroup.link.shared.entities.ReportScheduleModel;
 import com.lantanagroup.link.shared.exceptions.ValidationException;
 import com.lantanagroup.link.shared.services.ReportClient;
+import org.hl7.fhir.r4.model.Condition;
 import org.hl7.fhir.r4.model.MeasureReport;
+import org.hl7.fhir.r4.model.Observation;
 import org.hl7.fhir.r4.model.Patient;
 import org.hl7.fhir.r4.model.Reference;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.mockito.ArgumentCaptor;
+import org.slf4j.LoggerFactory;
 
 import java.util.List;
 
@@ -142,6 +151,99 @@ void testStorePatientInBlobStorageWithContainedResources() {
         assertTrue(content.contains("MeasureReport/mr1"));
     }
 
+    @Test
+    void testStorePatientInBlobStorageWithDuplicateIdPartAcrossTypes() {
+        PatientReportingEvaluationStatus status = new PatientReportingEvaluationStatus();
+        status.setPatientId("patient1");
+        PatientReportingEvaluationStatus.Report report = new PatientReportingEvaluationStatus.Report();
+        report.setReportTrackingId("track1");
+        report.setReportType("type1");
+        status.setReports(List.of(report));
+
+        ReportScheduleModel schedule = new ReportScheduleModel();
+        schedule.setPayloadRootUri("https://storage.com/test-container/root/");
+        when(reportClient.getReportSchedule("track1")).thenReturn(schedule);
+
+        MeasureReport measureReport = new MeasureReport();
+        measureReport.setId("mr1");
+
+        // Two contained resources of different types sharing the same ID part.
+        // Previously this threw IllegalStateException from Collectors.toMap.
+        Condition condition = new Condition();
+        condition.setId("#LCR-A");
+        measureReport.addContained(condition);
+
+        Observation observation = new Observation();
+        observation.setId("#LCR-A");
+        measureReport.addContained(observation);
+
+        blobStorageService.storePatientInBlobStorage(status, report, measureReport);
+
+        ArgumentCaptor<BinaryData> contentCaptor = ArgumentCaptor.forClass(BinaryData.class);
+        verify(blobClient).upload(contentCaptor.capture(), eq(true));
+        String content = contentCaptor.getValue().toString();
+
+        assertTrue(content.contains("Condition/A"));
+        assertTrue(content.contains("Observation/A"));
+    }
+
+    @Test
+    void testStorePatientInBlobStorageWithAmbiguousUntypedReferenceIsSkippedAndLogged() {
+        Logger blobStorageServiceLogger =
+                (Logger) LoggerFactory.getLogger(BlobStorageService.class);
+        ListAppender<ILoggingEvent> logAppender = new ListAppender<>();
+        logAppender.setContext((LoggerContext) LoggerFactory.getILoggerFactory());
+        logAppender.start();
+        blobStorageServiceLogger.addAppender(logAppender);
+
+        try {
+            PatientReportingEvaluationStatus status = new PatientReportingEvaluationStatus();
+            status.setPatientId("patient1");
+            PatientReportingEvaluationStatus.Report report = new PatientReportingEvaluationStatus.Report();
+            report.setReportTrackingId("track1");
+            report.setReportType("type1");
+            status.setReports(List.of(report));
+
+            ReportScheduleModel schedule = new ReportScheduleModel();
+            schedule.setPayloadRootUri("https://storage.com/test-container/root/");
+            when(reportClient.getReportSchedule("track1")).thenReturn(schedule);
+
+            MeasureReport measureReport = new MeasureReport();
+            measureReport.setId("mr1");
+
+            Condition condition = new Condition();
+            condition.setId("#LCR-A");
+            measureReport.addContained(condition);
+
+            Observation observation = new Observation();
+            observation.setId("#LCR-A");
+            measureReport.addContained(observation);
+
+            // Untyped contained reference to the ambiguous ID part - cannot be resolved deterministically.
+            measureReport.setSubject(new Reference("#LCR-A"));
+
+            blobStorageService.storePatientInBlobStorage(status, report, measureReport);
+
+            ArgumentCaptor<BinaryData> contentCaptor = ArgumentCaptor.forClass(BinaryData.class);
+            verify(blobClient).upload(contentCaptor.capture(), eq(true));
+            String content = contentCaptor.getValue().toString();
+
+            // First line is the "MeasureReport/<id>" marker, second is the MeasureReport JSON itself.
+            String measureReportJson = content.split("\n")[1];
+            MeasureReport parsed = fhirContext.newJsonParser().parseResource(MeasureReport.class, measureReportJson);
+
+            // The ambiguous reference is left unresolved rather than guessing which resource it means.
+            assertEquals("#LCR-A", parsed.getSubject().getReference());
+
+            boolean warningLogged = logAppender.list.stream()
+                    .anyMatch(event -> event.getLevel() == Level.WARN
+                            && event.getFormattedMessage().contains("LCR-A"));
+            assertTrue(warningLogged, "Expected a warning to be logged for the ambiguous contained reference");
+        } finally {
+            blobStorageServiceLogger.detachAppender(logAppender);
+        }
+    }
+
     @Test
     void testStorePatientInBlobStoragePayloadUriNull() {
         PatientReportingEvaluationStatus status = new PatientReportingEvaluationStatus();