Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

issue-3455 - adding resource info from location when return preferenc… #4130

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import java.io.ByteArrayInputStream;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Properties;
import java.util.UUID;
Expand Down Expand Up @@ -46,7 +45,6 @@
import org.linuxforhealth.fhir.client.FHIRRequestHeader;
import org.linuxforhealth.fhir.client.FHIRResponse;
import org.linuxforhealth.fhir.config.ConfigurationService;
import org.linuxforhealth.fhir.config.FHIRConfigHelper;
import org.linuxforhealth.fhir.config.FHIRConfiguration;
import org.linuxforhealth.fhir.config.PropertyGroup;
import org.linuxforhealth.fhir.core.FHIRMediaType;
Expand Down Expand Up @@ -130,6 +128,7 @@ public class BundleTest extends FHIRServerTestBase {
private static final String PATIENT_EXTENSION_URL = "http://my.url.domain.com/acme-healthcare/related-patient";

private static final String PREFER_HEADER_RETURN_REPRESENTATION = "return=representation";
private static final String PREFER_HEADER_RETURN_OPERATION_OUTCOME = "return=OperationOutcome";
private static final String PREFER_HEADER_NAME = "Prefer";

private static Boolean kafkaAuditEnabled = false;
Expand Down Expand Up @@ -987,64 +986,69 @@ public void testBatchCompartmentSearch() throws Exception {
@Test(groups = { "batch" }, dependsOnMethods = { "testBatchUpdates" })
public void testBatchMixture() throws Exception {
String method = "testBatchMixture";
WebTarget target = getWebTarget();

// change at least one field so that the update below isn't skipped
patientB1 = patientB1.toBuilder()
.deceased(true)
.build();

// Perform a mixture of request types.
Bundle bundle = buildBundle(BundleType.BATCH);
// create
bundle = addRequestToBundle(null, bundle, HTTPVerb.POST, "Patient", null,
TestUtil.readLocalResource("Patient_DavidOrtiz.json"));
// update
bundle = addRequestToBundle(null, bundle, HTTPVerb.PUT, "Patient/" + patientB1.getId(), null,
patientB1);
// read
bundle = addRequestToBundle(null, bundle, HTTPVerb.GET, "Patient/" + patientB2.getId(), null, null);
// vread
bundle = addRequestToBundle(null, bundle, HTTPVerb.GET, locationB1, null, null);
// history
bundle = addRequestToBundle(null, bundle, HTTPVerb.GET, "Patient/" + patientB1.getId() + "/_history",
null, null);
// search
bundle = addRequestToBundle(null, bundle, HTTPVerb.GET, "Patient?family=Ortiz&_count=100", null, null);

printBundle(method, "request", bundle);

Entity<Bundle> entity = Entity.entity(bundle, FHIRMediaType.APPLICATION_FHIR_JSON);
Response response = target.request()
.header(PREFER_HEADER_NAME, PREFER_HEADER_RETURN_REPRESENTATION)
.post(entity, Response.class);
assertResponse(response, Response.Status.OK.getStatusCode());

Bundle responseBundle = getEntityWithExtraWork(response,method);

assertResponseBundle(responseBundle, BundleType.BATCH_RESPONSE, 6);
assertGoodPostPutResponse(responseBundle.getEntry().get(0), Status.CREATED.getStatusCode());
assertGoodPostPutResponse(responseBundle.getEntry().get(1), Status.OK.getStatusCode());
assertGoodGetResponse(responseBundle.getEntry().get(2), Status.OK.getStatusCode());
assertGoodGetResponse(responseBundle.getEntry().get(3), Status.OK.getStatusCode());
assertGoodGetResponse(responseBundle.getEntry().get(4), Status.OK.getStatusCode());
assertGoodGetResponse(responseBundle.getEntry().get(5), Status.OK.getStatusCode());

Bundle resultSet;

// Verify the history results.
resultSet = (Bundle) responseBundle.getEntry().get(4).getResource();
assertNotNull(resultSet);
assertTrue(resultSet.getEntry().size() > 2);

// Verify the search results.
resultSet = (Bundle) responseBundle.getEntry().get(5).getResource();
assertNotNull(resultSet);
assertTrue(resultSet.getEntry().size() >= 1);

List<String> preferredHeaders = List.of(PREFER_HEADER_RETURN_REPRESENTATION, PREFER_HEADER_RETURN_OPERATION_OUTCOME);
for (String preferredHeader : preferredHeaders) {
WebTarget target = getWebTarget();

patientB1 = (Patient) responseBundle.getEntry().get(1).getResource();
// change at least one field so that the update below isn't skipped
patientB1 = patientB1.toBuilder()
.deceased(true)
.build();

// Perform a mixture of request types.
Bundle bundle = buildBundle(BundleType.BATCH);
// create
bundle = addRequestToBundle(null, bundle, HTTPVerb.POST, "Patient", null,
TestUtil.readLocalResource("Patient_DavidOrtiz.json"));
// update
bundle = addRequestToBundle(null, bundle, HTTPVerb.PUT, "Patient/" + patientB1.getId(), null,
patientB1);
// read
bundle = addRequestToBundle(null, bundle, HTTPVerb.GET, "Patient/" + patientB2.getId(), null, null);
// vread
bundle = addRequestToBundle(null, bundle, HTTPVerb.GET, locationB1, null, null);
// history
bundle = addRequestToBundle(null, bundle, HTTPVerb.GET, "Patient/" + patientB1.getId() + "/_history",
null, null);
// search
bundle = addRequestToBundle(null, bundle, HTTPVerb.GET, "Patient?family=Ortiz&_count=100", null, null);

printBundle(method, "request", bundle);

Entity<Bundle> entity = Entity.entity(bundle, FHIRMediaType.APPLICATION_FHIR_JSON);
Response response = target.request()
.header(PREFER_HEADER_NAME, preferredHeader)
.post(entity, Response.class);
assertResponse(response, Response.Status.OK.getStatusCode());

Bundle responseBundle = getEntityWithExtraWork(response,method);

assertResponseBundle(responseBundle, BundleType.BATCH_RESPONSE, 6);
assertGoodPostPutResponse(responseBundle.getEntry().get(0), Status.CREATED.getStatusCode());
assertGoodPostPutResponse(responseBundle.getEntry().get(1), Status.OK.getStatusCode());
assertGoodGetResponse(responseBundle.getEntry().get(2), Status.OK.getStatusCode());
assertGoodGetResponse(responseBundle.getEntry().get(3), Status.OK.getStatusCode());
assertGoodGetResponse(responseBundle.getEntry().get(4), Status.OK.getStatusCode());
assertGoodGetResponse(responseBundle.getEntry().get(5), Status.OK.getStatusCode());

Bundle resultSet;

// Verify the history results.
resultSet = (Bundle) responseBundle.getEntry().get(4).getResource();
assertNotNull(resultSet);
assertTrue(resultSet.getEntry().size() > 2);

// Verify the search results.
resultSet = (Bundle) responseBundle.getEntry().get(5).getResource();
assertNotNull(resultSet);
assertTrue(resultSet.getEntry().size() >= 1);
if (preferredHeader.equals(PREFER_HEADER_RETURN_REPRESENTATION)) {
patientB1 = (Patient) responseBundle.getEntry().get(1).getResource();
}
}
}


@Test(groups = { "transaction" })
public void testTransactionCreates() throws Exception {
Expand Down Expand Up @@ -1542,51 +1546,58 @@ public void testTransactionMixture() throws Exception {
if (!transactionSupported.booleanValue()) {
return;
}

WebTarget target = getWebTarget();

// Perform a mixture of request types.
Bundle bundle = buildBundle(BundleType.TRANSACTION);
// create
bundle = addRequestToBundle(null, bundle, HTTPVerb.POST, "Patient", null,
TestUtil.readLocalResource("Patient_DavidOrtiz.json"));
// update
bundle = addRequestToBundle(null, bundle, HTTPVerb.PUT, "Patient/" + patientT1.getId(), null,
patientT1);
// read
bundle = addRequestToBundle(null, bundle, HTTPVerb.GET, "Patient/" + patientT2.getId(), null, null);
// vread
bundle = addRequestToBundle(null, bundle, HTTPVerb.GET, locationT1, null, null);
// history
// bundle = addRequestToBundle(null, bundle, HTTPVerb.GET, "Patient/" + patientT1.getId() + "/_history", null, null);
// search
// bundle = addRequestToBundle(null, bundle, HTTPVerb.GET, "Patient?family=Ortiz&_count=100", null, null);

printBundle(method, "request", bundle);

Entity<Bundle> entity = Entity.entity(bundle, FHIRMediaType.APPLICATION_FHIR_JSON);
Response response = target.request().post(entity, Response.class);
assertResponse(response, Response.Status.OK.getStatusCode());
Bundle responseBundle = getEntityWithExtraWork(response,method);
assertResponseBundle(responseBundle, BundleType.TRANSACTION_RESPONSE, 4);
assertGoodPostPutResponse(responseBundle.getEntry().get(0), Status.CREATED.getStatusCode());
assertGoodPostPutResponse(responseBundle.getEntry().get(1), Status.OK.getStatusCode());
assertGoodGetResponse(responseBundle.getEntry().get(2), Status.OK.getStatusCode());
assertGoodGetResponse(responseBundle.getEntry().get(3), Status.OK.getStatusCode());
// assertGoodGetResponse(responseBundle.getEntry().get(4), Status.OK.getStatusCode());
// assertGoodGetResponse(responseBundle.getEntry().get(5), Status.OK.getStatusCode());

// Bundle resultSet;

// Verify the history results.
// resultSet = (Bundle) FHIRUtil.getResourceContainerResource(responseBundle.getEntry().get(4).getResource());
// assertNotNull(resultSet);
// assertTrue(resultSet.getEntry().size() > 2);

// Verify the search results.
// resultSet = (Bundle) FHIRUtil.getResourceContainerResource(responseBundle.getEntry().get(5).getResource());
// assertNotNull(resultSet);
// assertTrue(resultSet.getEntry().size() > 3);

List<String> preferredHeaders = List.of(PREFER_HEADER_RETURN_REPRESENTATION, PREFER_HEADER_RETURN_OPERATION_OUTCOME);
for (String preferredHeader : preferredHeaders) {
WebTarget target = getWebTarget();

// Perform a mixture of request types.
Bundle bundle = buildBundle(BundleType.TRANSACTION);
// create
bundle = addRequestToBundle(null, bundle, HTTPVerb.POST, "Patient", null,
TestUtil.readLocalResource("Patient_DavidOrtiz.json"));
// update
bundle = addRequestToBundle(null, bundle, HTTPVerb.PUT, "Patient/" + patientT1.getId(), null,
patientT1);
// read
bundle = addRequestToBundle(null, bundle, HTTPVerb.GET, "Patient/" + patientT2.getId(), null, null);
// vread
bundle = addRequestToBundle(null, bundle, HTTPVerb.GET, locationT1, null, null);
// history
// bundle = addRequestToBundle(null, bundle, HTTPVerb.GET, "Patient/" + patientT1.getId() + "/_history", null, null);
// search
// bundle = addRequestToBundle(null, bundle, HTTPVerb.GET, "Patient?family=Ortiz&_count=100", null, null);
Comment on lines +1566 to +1569
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the original test had these commented out too, but it makes me wonder why...


printBundle(method, "request", bundle);

Entity<Bundle> entity = Entity.entity(bundle, FHIRMediaType.APPLICATION_FHIR_JSON);
Response response = target.request()
.header(PREFER_HEADER_NAME, preferredHeader)
.post(entity, Response.class);
assertResponse(response, Response.Status.OK.getStatusCode());
Bundle responseBundle = getEntityWithExtraWork(response,method);
assertResponseBundle(responseBundle, BundleType.TRANSACTION_RESPONSE, 4);
assertGoodPostPutResponse(responseBundle.getEntry().get(0), Status.CREATED.getStatusCode());
assertGoodPostPutResponse(responseBundle.getEntry().get(1), Status.OK.getStatusCode());
assertGoodGetResponse(responseBundle.getEntry().get(2), Status.OK.getStatusCode());
assertGoodGetResponse(responseBundle.getEntry().get(3), Status.OK.getStatusCode());
// assertGoodGetResponse(responseBundle.getEntry().get(4), Status.OK.getStatusCode());
// assertGoodGetResponse(responseBundle.getEntry().get(5), Status.OK.getStatusCode());

// Bundle resultSet;

// Verify the history results.
// resultSet = (Bundle) FHIRUtil.getResourceContainerResource(responseBundle.getEntry().get(4).getResource());
// assertNotNull(resultSet);
// assertTrue(resultSet.getEntry().size() > 2);

// Verify the search results.
// resultSet = (Bundle) FHIRUtil.getResourceContainerResource(responseBundle.getEntry().get(5).getResource());
// assertNotNull(resultSet);
// assertTrue(resultSet.getEntry().size() > 3);
}


}

@Test(groups = { "batch" })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.linuxforhealth.fhir.config.FHIRConfiguration;
import org.linuxforhealth.fhir.config.FHIRRequestContext;
import org.linuxforhealth.fhir.core.FHIRUtilities;
import org.linuxforhealth.fhir.core.HTTPReturnPreference;
import org.linuxforhealth.fhir.core.util.handler.IPHandler;
import org.linuxforhealth.fhir.model.resource.Basic;
import org.linuxforhealth.fhir.model.resource.Bundle;
Expand Down Expand Up @@ -395,7 +396,7 @@ private static void logBundleBatch(AuditLogService auditLogSvc, HttpServletReque

AuditLogEntry entry = initLogEntry(AuditLogEventType.FHIR_BUNDLE);

populateAuditLogEntry(entry, request, responseEntry.getResource(), startTime, endTime, responseStatus);
populateBundleAuditLogEntry(entry, responseEntry, request, responseEntry.getResource(), startTime, endTime, responseStatus);
if (requestEntry.getRequest() != null && requestEntry.getRequest().getMethod() != null) {
boolean operation = requestEntry.getRequest().getUrl().getValue().contains("$")
|| requestEntry.getRequest().getUrl().getValue().contains("/%24");
Expand Down Expand Up @@ -431,7 +432,6 @@ private static void logBundleBatch(AuditLogService auditLogSvc, HttpServletReque
// Only for BATCH we want to override the REQUEST URI and Status Code
StringBuilder builder = new StringBuilder();
builder.append(request.getRequestURI())
.append("/")
.append(loc);
entry.getContext()
.setApiParameters(
Expand Down Expand Up @@ -479,7 +479,7 @@ private static void logBundleTransaction(AuditLogService auditLogSvc, HttpServle
for (Entry bundleEntry : responseBundle.getEntry()) {
Bundle.Entry requestEntry = iter.next();
entry = initLogEntry(AuditLogEventType.FHIR_BUNDLE);
populateAuditLogEntry(entry, request, bundleEntry.getResource(), startTime, endTime, responseStatus);
populateBundleAuditLogEntry(entry, bundleEntry, request, bundleEntry.getResource(), startTime, endTime, responseStatus);
entry.setDescription("FHIR Bundle Transaction request");
entry.getContext().setAction(selectActionForBundleEntry(requestEntry));
auditLogSvc.logEntry(entry);
Expand Down Expand Up @@ -746,6 +746,64 @@ protected static AuditLogEntry populateAuditLogEntry(AuditLogEntry entry, HttpSe
return entry;
}

/**
* Populates the passed audit log entry for Bundle entry.
* This method will populate the resource id, resource type and version id from the
* Bundle response entry location when the return preference is "OperationOutcome".
* When the return preference is "representation" the populateAuditLogEntry method will
* populate the resource id, resource type and version id.
* @param entry
* The AuditLogEntry to be populated.
* @param responseEntry
* The Bundle response entry.
* @param request
* The HttpServletRequest representation of the REST request.
* @param resource
* The Resource object.
* @param startTime
* The start time of the request execution.
* @param endTime
* The end time of the request execution.
* @param responseStatus
* The response status.
* @return AuditLogEntry - an initialized audit log entry.
*/
protected static AuditLogEntry populateBundleAuditLogEntry(AuditLogEntry entry, Bundle.Entry responseEntry, HttpServletRequest request, Resource resource,
Date startTime, Date endTime, Response.Status responseStatus) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor formatting: for method signatures that span multiple lines we double-indent to offset from the first line of the implementation

Suggested change
Date startTime, Date endTime, Response.Status responseStatus) {
Date startTime, Date endTime, Response.Status responseStatus) {

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been fixed.

final String METHODNAME = "populateBundleAuditLogEntry";
log.entering(CLASSNAME, METHODNAME);

// call populateAuditLogEntry to populate common attributes to all rest
populateAuditLogEntry(entry, request, resource, startTime, endTime, responseStatus);

// Populate the resource id, resource type and version id from the
// Bundle response entry location when the return preference is "OperationOutcome".
if (HTTPReturnPreference.OPERATION_OUTCOME.equals(FHIRRequestContext.get().getReturnPreference())) {
if (responseEntry.getResponse() != null && responseEntry.getResponse().getLocation() != null) {
String location = responseEntry.getResponse().getLocation().getValue();
String[] parts = location.split("/");
String resourceType = null;
String id = null;
String versionId = null;
if (parts.length == 10) {
resourceType = parts[6];
id = parts[7];
versionId = parts[9];
} else if (parts.length == 4) {
resourceType = parts[0];
id = parts[1];
versionId = parts[3];
}
lmsurpre marked this conversation as resolved.
Show resolved Hide resolved
entry.getContext().setData(Data.builder().resourceType(resourceType).build());
entry.getContext().getData().setId(id);
entry.getContext().getData().setVersionId(versionId);
}
}

log.exiting(CLASSNAME, METHODNAME);
return entry;
}

/**
* Builds and returns an AuditLogEntry with the minimum required fields populated.
*
Expand Down