Skip to content

Commit

Permalink
Merge pull request #1599 from IBM/issue-1594
Browse files Browse the repository at this point in the history 
Issue #1594 - fix overwriting of validate warnings in bundle processing
  • Loading branch information
@prb112
prb112 authored Oct 20, 2020
2 parents 3a12ffc + 7f806a3 commit 3c1b11e
Show file tree
Hide file tree
Showing 4 changed files with 473 additions and 119 deletions.
2 changes: 1 addition & 1 deletion fhir-model/src/main/java/com/ibm/fhir/model/util/FHIRUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public class FHIRUtil {
private static final SecureRandom RANDOM = new SecureRandom();
private static final JsonBuilderFactory BUILDER_FACTORY = Json.createBuilderFactory(null);
private static final Logger log = Logger.getLogger(FHIRUtil.class.getName());
private static final OperationOutcome ALL_OK = OperationOutcome.builder()
public static final OperationOutcome ALL_OK = OperationOutcome.builder()
.issue(Issue.builder()
.severity(IssueSeverity.INFORMATION)
.code(IssueType.INFORMATIONAL)
Expand Down
76 changes: 44 additions & 32 deletions fhir-server/src/main/java/com/ibm/fhir/server/util/FHIRRestHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public class FHIRRestHelper implements FHIRResourceHelpers {
private static final com.ibm.fhir.model.type.String SC_GONE_STRING = string(Integer.toString(SC_GONE));
private static final com.ibm.fhir.model.type.String SC_NOT_FOUND_STRING = string(Integer.toString(SC_NOT_FOUND));
private static final com.ibm.fhir.model.type.String SC_OK_STRING = string(Integer.toString(SC_OK));

public static final DateTimeFormatter PARSER_FORMATTER = new DateTimeFormatterBuilder()
.appendPattern("EEE")
.optionalStart()
Expand Down Expand Up @@ -1190,7 +1190,7 @@ private Bundle validateBundle(Bundle bundle) throws Exception {
List<OperationOutcome.Issue> issueList = new ArrayList<OperationOutcome.Issue>();

List<Bundle.Entry> responseList = new ArrayList<Bundle.Entry>();

Set<String> localIdentifiers = new HashSet<>();

for (Bundle.Entry requestEntry : bundle.getEntry()) {
Expand Down Expand Up @@ -1503,7 +1503,7 @@ private Bundle processBundleEntries(Bundle requestBundle, Bundle responseBundle,

// Build a mapping of local identifiers to external identifiers for local reference resolution.
Map<String, String> localRefMap = buildLocalRefMap(requestBundle, responseBundle);

// Process entries.
responseBundle = processEntriesForMethod(requestBundle, responseBundle, HTTPVerb.DELETE,
txn != null, localRefMap, requestProperties, bundleRequestCorrelationId);
Expand Down Expand Up @@ -1553,7 +1553,7 @@ private Bundle processBundleEntries(Bundle requestBundle, Bundle responseBundle,
* @param httpMethod
* the HTTP method (GET, POST, PUT, etc.) to be processed
* @param failFast
* a boolean value indicating if processing should stop on first failure
* a boolean value indicating if processing should stop on first failure
* @param localRefMap
* the map of local references to external references
* @param bundleRequestProperties
Expand All @@ -1569,7 +1569,7 @@ private Bundle processEntriesForMethod(Bundle requestBundle, Bundle responseBund
Map<String, String> bundleRequestProperties, String bundleRequestCorrelationId)
throws Exception {
log.entering(this.getClass().getName(), "processEntriesForMethod", new Object[] {"httpMethod", httpMethod });

try {
// First, obtain a list of request entry indices for the entries that we'll process.
// This list will contain the indices associated with only the entries for the specified http method.
Expand All @@ -1587,7 +1587,7 @@ private Bundle processEntriesForMethod(Bundle requestBundle, Bundle responseBund
+ entryIndices.toString());
}
}

// Now visit each of the request entries using the list of indices obtained above.
// Use hashmap to store both the index and the accordingly updated response bundle entry.
Map<Integer, Bundle.Entry> responseIndexAndEntries = new HashMap<Integer, Bundle.Entry>();
Expand All @@ -1601,7 +1601,7 @@ private Bundle processEntriesForMethod(Bundle requestBundle, Bundle responseBund
Bundle.Entry.Builder responseEntryBuilder = responseEntry.toBuilder();
StringBuffer requestDescription = new StringBuffer();
long initialTime = System.currentTimeMillis();

try {
FHIRUrlParser requestURL = new FHIRUrlParser(request.getUrl().getValue());

Expand Down Expand Up @@ -1646,7 +1646,7 @@ private Bundle processEntriesForMethod(Bundle requestBundle, Bundle responseBund
String msg = "Error while processing request bundle.";
throw new FHIRRestBundledRequestException(msg).withIssue(e.getIssues());
}

Bundle.Entry.Response.Builder responseBuilder = response.toBuilder();
responseBuilder.status(SC_NOT_FOUND_STRING);
responseIndexAndEntries.put(entryIndex, responseEntryBuilder.resource(FHIRUtil.buildOperationOutcome(e, false)).response(responseBuilder.build()).build());
Expand All @@ -1656,7 +1656,7 @@ private Bundle processEntriesForMethod(Bundle requestBundle, Bundle responseBund
String msg = "Error while processing request bundle.";
throw new FHIRRestBundledRequestException(msg).withIssue(e.getIssues());
}

Bundle.Entry.Response.Builder responseBuilder = response.toBuilder();
responseBuilder.status(SC_GONE_STRING);
responseIndexAndEntries.put(entryIndex, responseEntryBuilder.resource(FHIRUtil.buildOperationOutcome(e, false)).response(responseBuilder.build()).build());
Expand All @@ -1673,7 +1673,7 @@ private Bundle processEntriesForMethod(Bundle requestBundle, Bundle responseBund
} else {
status = IssueTypeToHttpStatusMapper.issueListToStatus(e.getIssues());
}

Bundle.Entry.Response.Builder responseBuilder = response.toBuilder();
responseBuilder.status(string(Integer.toString(status.getStatusCode())));
responseIndexAndEntries.put(entryIndex, responseEntryBuilder.resource(FHIRUtil.buildOperationOutcome(e, false)).response(responseBuilder.build()).build());
Expand All @@ -1699,7 +1699,7 @@ private Bundle processEntriesForMethod(Bundle requestBundle, Bundle responseBund
* @param responseIndexAndEntries
* the hashmap containing bundle entry indexes and their associated response entries
* @param entryIndex
* the bundle entry index of the bundle entry being processed
* the bundle entry index of the bundle entry being processed
* @param requestURL
* the request URL
* @param absoluteUri
Expand All @@ -1712,15 +1712,15 @@ private Bundle processEntriesForMethod(Bundle requestBundle, Bundle responseBund
*/
private void processEntryForGet(Bundle.Entry responseEntry, Map<Integer, Bundle.Entry> responseIndexAndEntries, Integer entryIndex,
FHIRUrlParser requestURL, String absoluteUri, String requestDescription, long initialTime) throws Exception {

String[] pathTokens = requestURL.getPathTokens();
MultivaluedMap<String, String> queryParams = requestURL.getQueryParameters();
Resource resource = null;

// Process a GET (read, vread, history, search, etc.).
// Determine the type of request from the path tokens.
if (pathTokens.length > 0 && pathTokens[pathTokens.length - 1].startsWith("$")) {
// This is a custom operation request.
// This is a custom operation request.

// Chop off the '$' and save the name
String operationName = pathTokens[pathTokens.length - 1].substring(1);
Expand Down Expand Up @@ -1789,7 +1789,7 @@ private void processEntryForGet(Bundle.Entry responseEntry, Map<Integer, Bundle.
* @param responseIndexAndEntries
* the hashmap containing bundle entry indexes and their associated response entries
* @param entryIndex
* the bundle entry index of the bundle entry being processed
* the bundle entry index of the bundle entry being processed
* @param localRefMap
* the map of local references to external references
* @param requestURL
Expand All @@ -1805,7 +1805,7 @@ private void processEntryForGet(Bundle.Entry responseEntry, Map<Integer, Bundle.
private void processEntryForPost(Bundle.Entry requestEntry, Bundle.Entry responseEntry, Map<Integer, Bundle.Entry> responseIndexAndEntries,
Integer entryIndex, Map<String, String> localRefMap, FHIRUrlParser requestURL, String absoluteUri, String requestDescription, long initialTime)
throws Exception {

Bundle.Entry.Builder responseEntryBuilder = responseEntry.toBuilder();
Bundle.Entry.Response response = responseEntry.getResponse();
Bundle.Entry.Response.Builder responseBuilder = response.toBuilder();
Expand Down Expand Up @@ -1862,7 +1862,7 @@ private void processEntryForPost(Bundle.Entry requestEntry, Bundle.Entry respons
responseBuilder.status(SC_OK_STRING);
responseIndexAndEntries.put(entryIndex, responseEntryBuilder.resource(searchResults).response(responseBuilder.build()).build());
logBundleRequestCompletedMsg(requestDescription, initialTime, SC_OK);

} else if (pathTokens.length == 1) {
// This is a 'create' request.

Expand All @@ -1883,27 +1883,27 @@ private void processEntryForPost(Bundle.Entry requestEntry, Bundle.Entry respons

// Determine if we have a pre-generated resource ID
String resourceId = retrieveGeneratedIdentifier(localRefMap, localIdentifier);

// Perform the 'create' or 'update' operation.
FHIRRestOperationResponse ior;
Bundle.Entry.Request request = requestEntry.getRequest();
String ifNoneExist = request.getIfNoneExist() != null && request.getIfNoneExist().getValue() != null && !request.getIfNoneExist().getValue().isEmpty()
String ifNoneExist = request.getIfNoneExist() != null && request.getIfNoneExist().getValue() != null && !request.getIfNoneExist().getValue().isEmpty()
? request.getIfNoneExist().getValue() : null;
if (ifNoneExist != null || resourceId == null) {
ior = doCreate(pathTokens[0], resource, ifNoneExist, null, !DO_VALIDATION);
} else {
resource = resource.toBuilder().id(resourceId).build();
ior = doUpdate(pathTokens[0], resourceId, resource, null, null, null, !DO_VALIDATION);
}

// Get the updated resource from FHIRRestOperationResponse which has the correct ID, meta, etc.
resource = ior.getResource();

// If a local identifier was present and not already mapped to its external identifier, add mapping.
if (localIdentifier != null && localRefMap.get(localIdentifier) == null) {
addLocalRefMapping(localRefMap, localIdentifier, null, resource);
}

// Process and replace bundle entry
Bundle.Entry resultEntry =
setBundleResponseFields(responseEntry, resource, ior.getOperationOutcome(), ior.getLocationURI(), ior.getStatus().getStatusCode(), requestDescription, initialTime);
Expand All @@ -1925,7 +1925,7 @@ private void processEntryForPost(Bundle.Entry requestEntry, Bundle.Entry respons
* @param responseIndexAndEntries
* the hashmap containing bundle entry indexes and their associated response entries
* @param entryIndex
* the bundle entry index of the bundle entry being processed
* the bundle entry index of the bundle entry being processed
* @param localRefMap
* the map of local references to external references
* @param requestURL
Expand All @@ -1941,7 +1941,7 @@ private void processEntryForPost(Bundle.Entry requestEntry, Bundle.Entry respons
private void processEntryForPut(Bundle.Entry requestEntry, Bundle.Entry responseEntry, Map<Integer, Bundle.Entry> responseIndexAndEntries,
Integer entryIndex, Map<String, String> localRefMap, FHIRUrlParser requestURL, String absoluteUri, String requestDescription, long initialTime)
throws Exception {

String[] pathTokens = requestURL.getPathTokens();
String type = null;
String id = null;
Expand Down Expand Up @@ -1986,7 +1986,7 @@ private void processEntryForPut(Bundle.Entry requestEntry, Bundle.Entry response
addLocalRefMapping(localRefMap, localIdentifier, null, ior.getResource());
}
}

// Process and replace bundle entry.
Bundle.Entry resultEntry =
setBundleResponseFields(responseEntry, ior.getResource(), ior.getOperationOutcome(), ior.getLocationURI(), ior.getStatus().getStatusCode(), requestDescription, initialTime);
Expand All @@ -2001,7 +2001,7 @@ private void processEntryForPut(Bundle.Entry requestEntry, Bundle.Entry response
* @param responseIndexAndEntries
* the hashmap containing bundle entry indexes and their associated response entries
* @param entryIndex
* the bundle entry index of the bundle entry being processed
* the bundle entry index of the bundle entry being processed
* @param requestURL
* the request URL
* @param requestDescription
Expand All @@ -2012,7 +2012,7 @@ private void processEntryForPut(Bundle.Entry requestEntry, Bundle.Entry response
*/
private void processEntryForDelete(Bundle.Entry responseEntry, Map<Integer, Bundle.Entry> responseIndexAndEntries, Integer entryIndex,
FHIRUrlParser requestURL, String requestDescription, long initialTime) throws Exception {

String[] pathTokens = requestURL.getPathTokens();
String type = null;
String id = null;
Expand Down Expand Up @@ -2135,7 +2135,7 @@ public int compare(Integer indexA, Integer indexB) {
return 0;
}
}

/**
* Returns the specified BundleEntry's path component of the 'url' field.
*
Expand Down Expand Up @@ -2177,7 +2177,7 @@ private MultivaluedMap<String, String> getQueryParameterMap(String queryString)
*
* @param requestBundle
* the bundle containing the requests
*
*
* @return local reference map
*/
private Map<String, String> buildLocalRefMap(Bundle requestBundle, Bundle responseBundle) throws Exception {
Expand All @@ -2188,15 +2188,15 @@ private Map<String, String> buildLocalRefMap(Bundle requestBundle, Bundle respon
Bundle.Entry.Request request = requestEntry.getRequest();
Bundle.Entry responseEntry = responseBundle.getEntry().get(entryIndex);
Bundle.Entry.Response response = responseEntry.getResponse();

// Only add mappings for POST and PUT requests where response is OK.
if (response.getStatus().equals(SC_OK_STRING) &&
(request.getMethod().equals(HTTPVerb.POST) || request.getMethod().equals(HTTPVerb.PUT))) {

// Retrieve the local identifier from the request entry (if present).
String localIdentifier = retrieveLocalIdentifier(requestEntry);
if (localIdentifier != null) {

// Retrieve the resource from the request entry (if present).
Resource resource = requestEntry.getResource();
if (resource != null) {
Expand All @@ -2220,7 +2220,7 @@ private Map<String, String> buildLocalRefMap(Bundle requestBundle, Bundle respon
}
}
}

return localRefMap;
}

Expand Down Expand Up @@ -2274,7 +2274,7 @@ private String retrieveLocalIdentifier(Bundle.Entry requestEntry) {
}

/**
* This method will retrieve the generated identifier associated with the specified local identifier from the
* This method will retrieve the generated identifier associated with the specified local identifier from the
* local ref map, or return null if there is no mapping for the local identifier.
*
* @param localRefMap
Expand Down Expand Up @@ -2330,7 +2330,19 @@ private Bundle.Entry setBundleResponseFields(Bundle.Entry responseEntry, Resourc
if (HTTPReturnPreference.REPRESENTATION.equals(FHIRRequestContext.get().getReturnPreference())) {
bundleEntryBuilder.resource(resource);
} else if (HTTPReturnPreference.OPERATION_OUTCOME.equals(FHIRRequestContext.get().getReturnPreference())) {
bundleEntryBuilder.resource(operationOutcome);
OperationOutcome responseEntryOutcome = (OperationOutcome)responseEntry.getResource();
if (responseEntryOutcome != null && !responseEntryOutcome.equals(FHIRUtil.ALL_OK)) {
if (operationOutcome.equals(FHIRUtil.ALL_OK)) {
bundleEntryBuilder.resource(responseEntryOutcome);
} else {
List<Issue> issues = new ArrayList<>();
issues.addAll(responseEntryOutcome.getIssue());
issues.addAll(operationOutcome.getIssue());
bundleEntryBuilder.resource(FHIRUtil.buildOperationOutcome(issues));
}
} else {
bundleEntryBuilder.resource(operationOutcome);
}
}
}
if (locationURI != null) {
Expand Down
Loading

0 comments on commit 3c1b11e

Please sign in to comment.