Merge pull request #2960 from IBM/issue-2946

issue #2946 - add conditional reference value and entry index (again)

fhir-server-test/src/test/java/com/ibm/fhir/server/test/ConditionalReferenceTest.java

@@ -107,6 +107,7 @@ public void testBundleTransactionInvalidConditionalReferenceNoQueryParameters()
         assertEquals(outcome.getIssue().get(0).getCode(), IssueType.INVALID);
         assertTrue(outcome.getIssue().get(0).getDetails().getText().getValue().startsWith("Invalid conditional reference:") &&
                 outcome.getIssue().get(0).getDetails().getText().getValue().endsWith("no query parameters found"));
+        assertEquals(outcome.getIssue().get(0).getExpression().get(0).getValue(), "Bundle.entry[0]");
     }
 
     @Test(dependsOnMethods = { "testCreatePatients" })
@@ -136,6 +137,7 @@ public void testBundleTransactionInvalidConditionalReferenceResultParameter() th
         assertEquals(outcome.getIssue().get(0).getCode(), IssueType.INVALID);
         assertTrue(outcome.getIssue().get(0).getDetails().getText().getValue().startsWith("Invalid conditional reference:") &&
                 outcome.getIssue().get(0).getDetails().getText().getValue().endsWith("only filtering parameters are allowed"));
+        assertEquals(outcome.getIssue().get(0).getExpression().get(0).getValue(), "Bundle.entry[0]");
     }
 
     @Test(dependsOnMethods = { "testCreatePatients" })
@@ -163,9 +165,9 @@ public void testBundleTransactionConditionalReferenceNoResult() throws Exception
 
         OperationOutcome outcome = response.readEntity(OperationOutcome.class);
         assertEquals(outcome.getIssue().get(0).getCode(), IssueType.NOT_FOUND);
-
         assertTrue(outcome.getIssue().get(0).getDetails().getText().getValue().startsWith("Error resolving conditional reference:") &&
                 outcome.getIssue().get(0).getDetails().getText().getValue().endsWith("returned no results"));
+        assertEquals(outcome.getIssue().get(0).getExpression().get(0).getValue(), "Bundle.entry[0]");
     }
 
     @Test(dependsOnMethods = { "testCreatePatients" })
@@ -176,7 +178,7 @@ public void testBundleTransactionConditionalReferenceMultipleMatches() throws Ex
         entry = entry.toBuilder()
                 .resource(entry.getResource().as(Observation.class).toBuilder()
                     .subject(Reference.builder()
-                        .reference(string("Patient?family=Doe&given=John"))
+                        .reference(string("Patient?family:exact=Doe&given:exact=John"))
                         .build())
                     .build())
                 .build();
@@ -195,6 +197,7 @@ public void testBundleTransactionConditionalReferenceMultipleMatches() throws Ex
         assertEquals(outcome.getIssue().get(0).getCode(), IssueType.MULTIPLE_MATCHES);
         assertTrue(outcome.getIssue().get(0).getDetails().getText().getValue().startsWith("Error resolving conditional reference:") &&
                 outcome.getIssue().get(0).getDetails().getText().getValue().endsWith("returned multiple results"));
+        assertEquals(outcome.getIssue().get(0).getExpression().get(0).getValue(), "Bundle.entry[0]");
     }
 
     private Patient buildPatient() {

fhir-server/src/main/java/com/ibm/fhir/server/rest/FHIRRestInteractionCreate.java

@@ -45,7 +45,9 @@ public FHIRRestInteractionCreate(int entryIndex, FHIRPersistenceEvent event, Ent
 
     @Override
     public void accept(FHIRRestInteractionVisitor visitor) throws Exception {
-        FHIRRestOperationResponse result = visitor.doCreate(getEntryIndex(), getEvent(), getWarnings(), getValidationResponseEntry(), getRequestDescription(), getRequestURL(), getInitialTime(), type, getNewResource(), ifNoneExist, localIdentifier);
+        FHIRRestOperationResponse result = visitor.doCreate(getEntryIndex(), getEvent(), getWarnings(),
+                getValidationResponseEntry(), getRequestDescription(), getRequestURL(), getInitialTime(), type,
+                getNewResource(), ifNoneExist, localIdentifier);
 
         // update the resource so we can use it when called in the next processing phase
         if (result != null && result.getResource() != null) {

fhir-server/src/main/java/com/ibm/fhir/server/rest/FHIRRestInteractionVisitorMeta.java

@@ -9,17 +9,21 @@
 import static com.ibm.fhir.model.type.String.string;
 
 import java.time.ZoneOffset;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.Callable;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.stream.Collectors;
 
 import javax.ws.rs.core.MultivaluedMap;
 import javax.ws.rs.core.Response.Status;
 
+import org.owasp.encoder.Encode;
+
 import com.ibm.fhir.exception.FHIROperationException;
 import com.ibm.fhir.model.patch.FHIRPatch;
 import com.ibm.fhir.model.resource.Bundle;
@@ -104,7 +108,9 @@ public FHIRRestOperationResponse doHistory(int entryIndex, String requestDescrip
     }
 
     @Override
-    public FHIRRestOperationResponse doCreate(int entryIndex, FHIRPersistenceEvent event, List<Issue> warnings, Entry validationResponseEntry, String requestDescription, FHIRUrlParser requestURL, long initialTime, String type, Resource resource, String ifNoneExist, String localIdentifier) throws Exception {
+    public FHIRRestOperationResponse doCreate(int entryIndex, FHIRPersistenceEvent event, List<Issue> warnings,
+            Entry validationResponseEntry, String requestDescription, FHIRUrlParser requestURL, long initialTime,
+            String type, Resource resource, String ifNoneExist, String localIdentifier) throws Exception {
         logStart(entryIndex, requestDescription, requestURL);
 
         // Skip CREATE if validation failed
@@ -152,9 +158,10 @@ public FHIRRestOperationResponse doCreate(int entryIndex, FHIRPersistenceEvent e
     }
 
     @Override
-    public FHIRRestOperationResponse doUpdate(int entryIndex, FHIRPersistenceEvent event, Entry validationResponseEntry, String requestDescription, FHIRUrlParser requestURL, long initialTime,
-        String type, String id, Resource resource, Resource prevResource, String ifMatchValue, String searchQueryString,
-        boolean skippableUpdate, String localIdentifier, List<Issue> warnings, boolean isDeleted, Integer ifNoneMatch) throws Exception {
+    public FHIRRestOperationResponse doUpdate(int entryIndex, FHIRPersistenceEvent event, Entry validationResponseEntry,
+            String requestDescription, FHIRUrlParser requestURL, long initialTime, String type, String id,
+            Resource resource, Resource prevResource, String ifMatchValue, String searchQueryString, boolean skippableUpdate,
+            String localIdentifier, List<Issue> warnings, boolean isDeleted, Integer ifNoneMatch) throws Exception {
         logStart(entryIndex, requestDescription, requestURL);
 
         // Skip UPDATE if validation failed
@@ -329,11 +336,13 @@ private void resolveConditionalReferences(Resource resource) throws Exception {
             int total = bundle.getTotal().getValue();
 
             if (total == 0) {
-                throw buildRestException("Error resolving conditional reference: search returned no results", IssueType.NOT_FOUND);
+                throw buildRestException("Error resolving conditional reference: search '"
+                        + Encode.forHtml(conditionalReference) + "' returned no results", IssueType.NOT_FOUND);
             }
 
             if (total > 1) {
-                throw buildRestException("Error resolving conditional reference: search returned multiple results", IssueType.MULTIPLE_MATCHES);
+                throw buildRestException("Error resolving conditional reference: search '"
+                        + Encode.forHtml(conditionalReference) + "' returned multiple results", IssueType.MULTIPLE_MATCHES);
             }
 
             localRefMap.put(conditionalReference, type + "/" + bundle.getEntry().get(0).getResource().getId());
@@ -384,8 +393,7 @@ private FHIRRestOperationResponse doInteraction(int entryIndex, String requestDe
             return v.call();
         } catch (FHIRPersistenceResourceNotFoundException e) {
             if (failFast) {
-                String msg = "Error while processing request bundle.";
-                throw new FHIRRestBundledRequestException(msg, e).withIssue(e.getIssues());
+                updateIssuesWithEntryIndexAndThrow(entryIndex, e);
             }
 
             // Record the error as an entry in the result bundle
@@ -398,8 +406,7 @@ private FHIRRestOperationResponse doInteraction(int entryIndex, String requestDe
             setEntryComplete(entryIndex, entry, requestDescription, initialTime);
         } catch (FHIRPersistenceResourceDeletedException e) {
             if (failFast) {
-                String msg = "Error while processing request bundle.";
-                throw new FHIRRestBundledRequestException(msg, e).withIssue(e.getIssues());
+                updateIssuesWithEntryIndexAndThrow(entryIndex, e);
             }
 
             Entry entry = Entry.builder()
@@ -411,8 +418,7 @@ private FHIRRestOperationResponse doInteraction(int entryIndex, String requestDe
             setEntryComplete(entryIndex, entry, requestDescription, initialTime);
         } catch (FHIROperationException e) {
             if (failFast) {
-                String msg = "Error while processing request bundle.";
-                throw new FHIRRestBundledRequestException(msg, e).withIssue(e.getIssues());
+                updateIssuesWithEntryIndexAndThrow(entryIndex, e);
             }
 
             Status status;
@@ -433,4 +439,14 @@ private FHIRRestOperationResponse doInteraction(int entryIndex, String requestDe
 
         return null;
     }
+
+    private void updateIssuesWithEntryIndexAndThrow(Integer entryIndex, FHIROperationException cause) throws FHIROperationException {
+        String msg = "Error while processing request bundle on entry " + entryIndex;
+        List<Issue> updatedIssues = cause.getIssues().stream()
+                .map(i -> i.toBuilder().expression(string("Bundle.entry[" + entryIndex + "]")).build())
+                .collect(Collectors.toList());
+        // no need to keep the issues in the cause any more since we've "promoted" them to the wrapped exception
+        cause.withIssue(Collections.emptyList());
+        throw new FHIRRestBundledRequestException(msg, cause).withIssue(updatedIssues);
+    }
 }

fhir-server/src/main/java/com/ibm/fhir/server/util/FHIRRestHelper.java

@@ -1804,7 +1804,8 @@ private String getVersionIdFromETagValue(String ifMatchValue) {
      *            resource on the server, then skip the update; if false, then always attempt the updates specified in the bundle
      * @return a response bundle
      */
-    private Bundle processBundleEntries(Bundle requestBundle, Map<Integer, Entry> validationResponseEntries, boolean skippableUpdates) throws Exception {
+    private Bundle processBundleEntries(Bundle requestBundle, Map<Integer, Entry> validationResponseEntries,
+            boolean skippableUpdates) throws Exception {
         log.entering(this.getClass().getName(), "processBundleEntries");
 
         // Generate a request correlation id for this request bundle.
@@ -1826,7 +1827,8 @@ private Bundle processBundleEntries(Bundle requestBundle, Map<Integer, Entry> va
             // then process in order
             final boolean isTransactionBundle = bundleType == BundleType.Value.TRANSACTION;
             FHIRRestBundleHelper bundleHelper = new FHIRRestBundleHelper(this);
-            List<FHIRRestInteraction> bundleInteractions = bundleHelper.translateBundleEntries(requestBundle, validationResponseEntries, isTransactionBundle, bundleRequestCorrelationId, skippableUpdates);
+            List<FHIRRestInteraction> bundleInteractions = bundleHelper.translateBundleEntries(requestBundle,
+                    validationResponseEntries, isTransactionBundle, bundleRequestCorrelationId, skippableUpdates);
             List<Entry> responseEntries = processBundleInteractions(bundleInteractions, validationResponseEntries, isTransactionBundle);
 
             // Build the response bundle.
@@ -1861,7 +1863,8 @@ private Bundle processBundleEntries(Bundle requestBundle, Map<Integer, Entry> va
      * @return
      * @throws Exception
      */
-    private List<Entry> processBundleInteractions(List<FHIRRestInteraction> bundleInteractions, Map<Integer, Entry> validationResponseEntries, boolean transaction) throws Exception {
+    private List<Entry> processBundleInteractions(List<FHIRRestInteraction> bundleInteractions,
+            Map<Integer, Entry> validationResponseEntries, boolean transaction) throws Exception {
         assert(bundleInteractions.size() > 0);
         Entry[] responseEntries = new Entry[bundleInteractions.size()];
         Map<String, String> localRefMap = new HashMap<>();
@@ -2621,7 +2624,7 @@ public List<Issue> validateResource(Resource resource) throws FHIROperationExcep
                     atLeastOneProfiles.addAll(defaultAtLeastOneProfiles);
                 }
             }
-            
+
             // Build the list of 'atLeastOne' profiles that didn't specify a version
             for (String profile : atLeastOneProfiles) {
                 if (!profile.contains("|")) {
@@ -2654,11 +2657,11 @@ public List<Issue> validateResource(Resource resource) throws FHIROperationExcep
                     notAllowedProfilesWithoutVersion.add(profile);
                 }
             }
-            
+
             if (log.isLoggable(Level.FINER)) {
                 log.finer("Not allowed profile list: " + notAllowedProfiles);
             }
-            
+
             // Get the 'allowUnknown' property
             Boolean resourceSpecificAllowUnknown =
                     FHIRConfigHelper.getBooleanProperty(resourceSpecificProfileConfigPath.toString() +
@@ -2668,12 +2671,12 @@ public List<Issue> validateResource(Resource resource) throws FHIROperationExcep
             } else {
                 allowUnknown = FHIRConfigHelper.getBooleanProperty(defaultProfileConfigPath.toString() +
                     FHIRConfiguration.PROPERTY_FIELD_RESOURCES_PROFILES_ALLOW_UNKNOWN, Boolean.TRUE);
-            }            
-            
+            }
+
             if (log.isLoggable(Level.FINER)) {
                 log.finer("Allow unknown: " + allowUnknown);
             }
-            
+
             // Get the 'defaultVersions' entries
             PropertyGroup resourceSpecificDefaultVersionsGroup =
                     FHIRConfigHelper.getPropertyGroup(resourceSpecificProfileConfigPath.toString() +
@@ -2694,7 +2697,7 @@ public List<Issue> validateResource(Resource resource) throws FHIROperationExcep
                     }
                 }
             }
-            
+
             if (log.isLoggable(Level.FINER)) {
                 log.finer("Default profile versions: [");
                 for (String profile : defaultVersions.keySet()) {
@@ -2778,7 +2781,7 @@ public List<Issue> validateResource(Resource resource) throws FHIROperationExcep
                     }
                 }
             }
-            
+
             // Check if a profile is required but no valid profile asserted
             if (!atLeastOneProfiles.isEmpty() && !validProfileFound) {
                 issues.add(buildOperationOutcomeIssue(IssueSeverity.ERROR, IssueType.BUSINESS_RULE,
@@ -2797,13 +2800,13 @@ public List<Issue> validateResource(Resource resource) throws FHIROperationExcep
                 resourceToValidate = resource.toBuilder().meta(metaCopy).build();
             }
         }
-        
+
         try {
             issues = validator.validate(resourceToValidate);
         } catch (FHIRValidationException e) {
             throw new FHIROperationException("Error validating resource.", e);
         }
-        
+
         return issues;
     }