Fixed the issue of bulk export returning the wrong path if "Fixed Value for Endpoint Base URL" is set (#3958)

* Fixed the issue of bulk export returning the wrong path if "Fixed Value for Endpoint Base URL" is set

* Minor modifications according to the suggestions

* Modify the msg code number

* Modify the tests to be parameterized test

* Modifications according to review suggestions

* Update the msg code
This commit is contained in:
Qingyixia 2022-09-06 11:40:03 -04:00 committed by GitHub
parent dd5c49a9ef
commit f3e6837fce
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 61 additions and 11 deletions

View File

@ -25,7 +25,7 @@ public final class Msg {
/** /**
* IMPORTANT: Please update the following comment after you add a new code * IMPORTANT: Please update the following comment after you add a new code
* Last used code value: 2135 * Last used code value: 2136
*/ */
private Msg() {} private Msg() {}

View File

@ -0,0 +1,6 @@
---
type: fix
issue: 3956
title: "Previously, if the Endpoint Base URL is set to something different from the default value, the URL that export-poll-status returned is incorrect.
After correcting the export-poll-status URL, the binary file URL returned is also incorrect. This error has also been fixed and the URLs that are returned
from $export and $export-poll-status will not contain the extra path from 'Fixed Value for Endpoint Base URL'."

View File

@ -14,6 +14,7 @@ import ca.uhn.fhir.jpa.bulk.export.provider.BulkDataExportProvider;
import ca.uhn.fhir.jpa.model.util.JpaConstants; import ca.uhn.fhir.jpa.model.util.JpaConstants;
import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.client.apache.ResourceEntity; import ca.uhn.fhir.rest.client.apache.ResourceEntity;
import ca.uhn.fhir.rest.server.HardcodedServerAddressStrategy;
import ca.uhn.fhir.rest.server.RestfulServer; import ca.uhn.fhir.rest.server.RestfulServer;
import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
import ca.uhn.fhir.test.utilities.JettyUtil; import ca.uhn.fhir.test.utilities.JettyUtil;
@ -39,6 +40,9 @@ import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.mockito.ArgumentCaptor; import org.mockito.ArgumentCaptor;
import org.mockito.InjectMocks; import org.mockito.InjectMocks;
import org.mockito.Mock; import org.mockito.Mock;
@ -53,6 +57,7 @@ import java.util.Date;
import java.util.HashMap; import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import java.util.stream.Stream;
import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsInAnyOrder;
@ -90,6 +95,14 @@ public class BulkDataExportProviderTest {
@InjectMocks @InjectMocks
private BulkDataExportProvider myProvider; private BulkDataExportProvider myProvider;
private RestfulServer servlet;
static Stream<Arguments> paramsProvider(){
return Stream.of(
Arguments.arguments(true),
Arguments.arguments(false)
);
}
@AfterEach @AfterEach
public void after() throws Exception { public void after() throws Exception {
@ -102,7 +115,7 @@ public class BulkDataExportProviderTest {
myServer = new Server(0); myServer = new Server(0);
ServletHandler proxyHandler = new ServletHandler(); ServletHandler proxyHandler = new ServletHandler();
RestfulServer servlet = new RestfulServer(myCtx); servlet = new RestfulServer(myCtx);
servlet.registerProvider(myProvider); servlet.registerProvider(myProvider);
ServletHolder servletHolder = new ServletHolder(servlet); ServletHolder servletHolder = new ServletHolder(servlet);
proxyHandler.addServletWithMapping(servletHolder, "/*"); proxyHandler.addServletWithMapping(servletHolder, "/*");
@ -116,6 +129,12 @@ public class BulkDataExportProviderTest {
myClient = builder.build(); myClient = builder.build();
} }
public void startWithFixedBaseUrl() {
String baseUrl = "http://localhost:" + myPort + "/fixedvalue";
HardcodedServerAddressStrategy hardcodedServerAddressStrategy = new HardcodedServerAddressStrategy(baseUrl);
servlet.setServerAddressStrategy(hardcodedServerAddressStrategy);
}
private BulkExportParameters verifyJobStart() { private BulkExportParameters verifyJobStart() {
ArgumentCaptor<Batch2BaseJobParameters> startJobCaptor = ArgumentCaptor.forClass(Batch2BaseJobParameters.class); ArgumentCaptor<Batch2BaseJobParameters> startJobCaptor = ArgumentCaptor.forClass(Batch2BaseJobParameters.class);
verify(myJobRunner).startNewJob(startJobCaptor.capture()); verify(myJobRunner).startNewJob(startJobCaptor.capture());
@ -135,8 +154,14 @@ public class BulkDataExportProviderTest {
return createJobStartResponse(A_JOB_ID); return createJobStartResponse(A_JOB_ID);
} }
@Test @ParameterizedTest
public void testSuccessfulInitiateBulkRequest_Post() throws IOException { @MethodSource("paramsProvider")
public void testSuccessfulInitiateBulkRequest_Post_WithFixedBaseURL(Boolean baseUrlFixed) throws IOException {
// setup
if (baseUrlFixed) {
startWithFixedBaseUrl();
}
String patientResource = "Patient"; String patientResource = "Patient";
String practitionerResource = "Practitioner"; String practitionerResource = "Practitioner";
String filter = "Patient?identifier=foo"; String filter = "Patient?identifier=foo";
@ -314,9 +339,14 @@ public class BulkDataExportProviderTest {
} }
} }
@Test @ParameterizedTest
public void testPollForStatus_COMPLETED() throws IOException { @MethodSource("paramsProvider")
public void testPollForStatus_COMPLETED_WithFixedBaseURL(boolean baseUrlFixed) throws IOException {
// setup // setup
if (baseUrlFixed) {
startWithFixedBaseUrl();
}
Batch2JobInfo info = new Batch2JobInfo(); Batch2JobInfo info = new Batch2JobInfo();
info.setJobId(A_JOB_ID); info.setJobId(A_JOB_ID);
info.setStatus(BulkExportJobStatusEnum.COMPLETE); info.setStatus(BulkExportJobStatusEnum.COMPLETE);

View File

@ -47,6 +47,7 @@ import ca.uhn.fhir.rest.api.RequestTypeEnum;
import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.RequestDetails;
import ca.uhn.fhir.rest.api.server.bulk.BulkDataExportOptions; import ca.uhn.fhir.rest.api.server.bulk.BulkDataExportOptions;
import ca.uhn.fhir.rest.server.RestfulServerUtils; import ca.uhn.fhir.rest.server.RestfulServerUtils;
import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
import ca.uhn.fhir.rest.server.util.CompositeInterceptorBroadcaster; import ca.uhn.fhir.rest.server.util.CompositeInterceptorBroadcaster;
@ -102,7 +103,7 @@ public class BulkDataExportProvider {
@OperationParam(name = JpaConstants.PARAM_EXPORT_SINCE, min = 0, max = 1, typeName = "instant") IPrimitiveType<Date> theSince, @OperationParam(name = JpaConstants.PARAM_EXPORT_SINCE, min = 0, max = 1, typeName = "instant") IPrimitiveType<Date> theSince,
@OperationParam(name = JpaConstants.PARAM_EXPORT_TYPE_FILTER, min = 0, max = OperationParam.MAX_UNLIMITED, typeName = "string") List<IPrimitiveType<String>> theTypeFilter, @OperationParam(name = JpaConstants.PARAM_EXPORT_TYPE_FILTER, min = 0, max = OperationParam.MAX_UNLIMITED, typeName = "string") List<IPrimitiveType<String>> theTypeFilter,
ServletRequestDetails theRequestDetails ServletRequestDetails theRequestDetails
) { ) throws Exception {
// JPA export provider // JPA export provider
validatePreferAsyncHeader(theRequestDetails, JpaConstants.OPERATION_EXPORT); validatePreferAsyncHeader(theRequestDetails, JpaConstants.OPERATION_EXPORT);
@ -112,7 +113,7 @@ public class BulkDataExportProvider {
} }
private void startJob(ServletRequestDetails theRequestDetails, private void startJob(ServletRequestDetails theRequestDetails,
BulkDataExportOptions theOptions) { BulkDataExportOptions theOptions){
// permission check // permission check
HookParams params = (new HookParams()).add(BulkDataExportOptions.class, theOptions) HookParams params = (new HookParams()).add(BulkDataExportOptions.class, theOptions)
.add(RequestDetails.class, theRequestDetails) .add(RequestDetails.class, theRequestDetails)
@ -145,7 +146,17 @@ public class BulkDataExportProvider {
} }
private String getServerBase(ServletRequestDetails theRequestDetails) { private String getServerBase(ServletRequestDetails theRequestDetails) {
return StringUtils.removeEnd(theRequestDetails.getServerBaseForRequest(), "/"); if (theRequestDetails.getCompleteUrl().contains(theRequestDetails.getServerBaseForRequest())) {
// Base URL not Fixed
return StringUtils.removeEnd(theRequestDetails.getServerBaseForRequest(), "/");
} else {
// Base URL Fixed
int index = StringUtils.indexOf(theRequestDetails.getCompleteUrl(), theRequestDetails.getOperation());
if (index == -1) {
return null;
}
return theRequestDetails.getCompleteUrl().substring(0, index - 1);
}
} }
private String getDefaultPartitionServerBase(ServletRequestDetails theRequestDetails) { private String getDefaultPartitionServerBase(ServletRequestDetails theRequestDetails) {
@ -168,7 +179,7 @@ public class BulkDataExportProvider {
@OperationParam(name = JpaConstants.PARAM_EXPORT_TYPE_FILTER, min = 0, max = OperationParam.MAX_UNLIMITED, typeName = "string") List<IPrimitiveType<String>> theTypeFilter, @OperationParam(name = JpaConstants.PARAM_EXPORT_TYPE_FILTER, min = 0, max = OperationParam.MAX_UNLIMITED, typeName = "string") List<IPrimitiveType<String>> theTypeFilter,
@OperationParam(name = JpaConstants.PARAM_EXPORT_MDM, min = 0, max = 1, typeName = "boolean") IPrimitiveType<Boolean> theMdm, @OperationParam(name = JpaConstants.PARAM_EXPORT_MDM, min = 0, max = 1, typeName = "boolean") IPrimitiveType<Boolean> theMdm,
ServletRequestDetails theRequestDetails ServletRequestDetails theRequestDetails
) { ) throws Exception {
ourLog.debug("Received Group Bulk Export Request for Group {}", theIdParam); ourLog.debug("Received Group Bulk Export Request for Group {}", theIdParam);
ourLog.debug("_type={}", theIdParam); ourLog.debug("_type={}", theIdParam);
ourLog.debug("_since={}", theSince); ourLog.debug("_since={}", theSince);
@ -227,7 +238,7 @@ public class BulkDataExportProvider {
@OperationParam(name = JpaConstants.PARAM_EXPORT_SINCE, min = 0, max = 1, typeName = "instant") IPrimitiveType<Date> theSince, @OperationParam(name = JpaConstants.PARAM_EXPORT_SINCE, min = 0, max = 1, typeName = "instant") IPrimitiveType<Date> theSince,
@OperationParam(name = JpaConstants.PARAM_EXPORT_TYPE_FILTER, min = 0, max = OperationParam.MAX_UNLIMITED, typeName = "string") List<IPrimitiveType<String>> theTypeFilter, @OperationParam(name = JpaConstants.PARAM_EXPORT_TYPE_FILTER, min = 0, max = OperationParam.MAX_UNLIMITED, typeName = "string") List<IPrimitiveType<String>> theTypeFilter,
ServletRequestDetails theRequestDetails ServletRequestDetails theRequestDetails
) { ) throws Exception {
validatePreferAsyncHeader(theRequestDetails, JpaConstants.OPERATION_EXPORT); validatePreferAsyncHeader(theRequestDetails, JpaConstants.OPERATION_EXPORT);
BulkDataExportOptions bulkDataExportOptions = buildPatientBulkExportOptions(theOutputFormat, theType, theSince, theTypeFilter); BulkDataExportOptions bulkDataExportOptions = buildPatientBulkExportOptions(theOutputFormat, theType, theSince, theTypeFilter);
validateResourceTypesAllContainPatientSearchParams(bulkDataExportOptions.getResourceTypes()); validateResourceTypesAllContainPatientSearchParams(bulkDataExportOptions.getResourceTypes());
@ -390,6 +401,9 @@ public class BulkDataExportProvider {
public void writePollingLocationToResponseHeaders(ServletRequestDetails theRequestDetails, JobInfo theOutcome) { public void writePollingLocationToResponseHeaders(ServletRequestDetails theRequestDetails, JobInfo theOutcome) {
String serverBase = getServerBase(theRequestDetails); String serverBase = getServerBase(theRequestDetails);
if (serverBase == null) {
throw new InternalErrorException(Msg.code(2136) + "Unable to get the server base.");
}
String pollLocation = serverBase + "/" + JpaConstants.OPERATION_EXPORT_POLL_STATUS + "?" + JpaConstants.PARAM_EXPORT_POLL_STATUS_JOB_ID + "=" + theOutcome.getJobMetadataId(); String pollLocation = serverBase + "/" + JpaConstants.OPERATION_EXPORT_POLL_STATUS + "?" + JpaConstants.PARAM_EXPORT_POLL_STATUS_JOB_ID + "=" + theOutcome.getJobMetadataId();
HttpServletResponse response = theRequestDetails.getServletResponse(); HttpServletResponse response = theRequestDetails.getServletResponse();