From f3e6837fce04eda6e9e76a3acfbbe4040dc8683d Mon Sep 17 00:00:00 2001 From: Qingyixia <106992634+Qingyixia@users.noreply.github.com> Date: Tue, 6 Sep 2022 11:40:03 -0400 Subject: [PATCH] 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 --- .../src/main/java/ca/uhn/fhir/i18n/Msg.java | 2 +- ...ed-value-for-endpoint-base-url-is-set.yaml | 6 +++ .../jpa/bulk/BulkDataExportProviderTest.java | 40 ++++++++++++++++--- .../provider/BulkDataExportProvider.java | 24 ++++++++--- 4 files changed, 61 insertions(+), 11 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/3956-bulk-export-returning-the-wrong-path-if-fixed-value-for-endpoint-base-url-is-set.yaml diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/i18n/Msg.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/i18n/Msg.java index 35ad3d41781..f96fed683c9 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/i18n/Msg.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/i18n/Msg.java @@ -25,7 +25,7 @@ public final class Msg { /** * IMPORTANT: Please update the following comment after you add a new code - * Last used code value: 2135 + * Last used code value: 2136 */ private Msg() {} diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/3956-bulk-export-returning-the-wrong-path-if-fixed-value-for-endpoint-base-url-is-set.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/3956-bulk-export-returning-the-wrong-path-if-fixed-value-for-endpoint-base-url-is-set.yaml new file mode 100644 index 00000000000..5e46871c3b3 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/3956-bulk-export-returning-the-wrong-path-if-fixed-value-for-endpoint-base-url-is-set.yaml @@ -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'." diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/BulkDataExportProviderTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/BulkDataExportProviderTest.java index 0cb75c6401f..b20cb4db946 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/BulkDataExportProviderTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/BulkDataExportProviderTest.java @@ -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.rest.api.Constants; 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.exceptions.ResourceNotFoundException; 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.Test; 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.InjectMocks; import org.mockito.Mock; @@ -53,6 +57,7 @@ import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.concurrent.TimeUnit; +import java.util.stream.Stream; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; @@ -90,6 +95,14 @@ public class BulkDataExportProviderTest { @InjectMocks private BulkDataExportProvider myProvider; + private RestfulServer servlet; + + static Stream paramsProvider(){ + return Stream.of( + Arguments.arguments(true), + Arguments.arguments(false) + ); + } @AfterEach public void after() throws Exception { @@ -102,7 +115,7 @@ public class BulkDataExportProviderTest { myServer = new Server(0); ServletHandler proxyHandler = new ServletHandler(); - RestfulServer servlet = new RestfulServer(myCtx); + servlet = new RestfulServer(myCtx); servlet.registerProvider(myProvider); ServletHolder servletHolder = new ServletHolder(servlet); proxyHandler.addServletWithMapping(servletHolder, "/*"); @@ -116,6 +129,12 @@ public class BulkDataExportProviderTest { myClient = builder.build(); } + public void startWithFixedBaseUrl() { + String baseUrl = "http://localhost:" + myPort + "/fixedvalue"; + HardcodedServerAddressStrategy hardcodedServerAddressStrategy = new HardcodedServerAddressStrategy(baseUrl); + servlet.setServerAddressStrategy(hardcodedServerAddressStrategy); + } + private BulkExportParameters verifyJobStart() { ArgumentCaptor startJobCaptor = ArgumentCaptor.forClass(Batch2BaseJobParameters.class); verify(myJobRunner).startNewJob(startJobCaptor.capture()); @@ -135,8 +154,14 @@ public class BulkDataExportProviderTest { return createJobStartResponse(A_JOB_ID); } - @Test - public void testSuccessfulInitiateBulkRequest_Post() throws IOException { + @ParameterizedTest + @MethodSource("paramsProvider") + public void testSuccessfulInitiateBulkRequest_Post_WithFixedBaseURL(Boolean baseUrlFixed) throws IOException { + // setup + if (baseUrlFixed) { + startWithFixedBaseUrl(); + } + String patientResource = "Patient"; String practitionerResource = "Practitioner"; String filter = "Patient?identifier=foo"; @@ -314,9 +339,14 @@ public class BulkDataExportProviderTest { } } - @Test - public void testPollForStatus_COMPLETED() throws IOException { + @ParameterizedTest + @MethodSource("paramsProvider") + public void testPollForStatus_COMPLETED_WithFixedBaseURL(boolean baseUrlFixed) throws IOException { // setup + if (baseUrlFixed) { + startWithFixedBaseUrl(); + } + Batch2JobInfo info = new Batch2JobInfo(); info.setJobId(A_JOB_ID); info.setStatus(BulkExportJobStatusEnum.COMPLETE); diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/bulk/export/provider/BulkDataExportProvider.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/bulk/export/provider/BulkDataExportProvider.java index 8a627a7ed21..cc4a866195f 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/bulk/export/provider/BulkDataExportProvider.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/bulk/export/provider/BulkDataExportProvider.java @@ -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.bulk.BulkDataExportOptions; 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.servlet.ServletRequestDetails; 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 theSince, @OperationParam(name = JpaConstants.PARAM_EXPORT_TYPE_FILTER, min = 0, max = OperationParam.MAX_UNLIMITED, typeName = "string") List> theTypeFilter, ServletRequestDetails theRequestDetails - ) { + ) throws Exception { // JPA export provider validatePreferAsyncHeader(theRequestDetails, JpaConstants.OPERATION_EXPORT); @@ -112,7 +113,7 @@ public class BulkDataExportProvider { } private void startJob(ServletRequestDetails theRequestDetails, - BulkDataExportOptions theOptions) { + BulkDataExportOptions theOptions){ // permission check HookParams params = (new HookParams()).add(BulkDataExportOptions.class, theOptions) .add(RequestDetails.class, theRequestDetails) @@ -145,7 +146,17 @@ public class BulkDataExportProvider { } 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) { @@ -168,7 +179,7 @@ public class BulkDataExportProvider { @OperationParam(name = JpaConstants.PARAM_EXPORT_TYPE_FILTER, min = 0, max = OperationParam.MAX_UNLIMITED, typeName = "string") List> theTypeFilter, @OperationParam(name = JpaConstants.PARAM_EXPORT_MDM, min = 0, max = 1, typeName = "boolean") IPrimitiveType theMdm, ServletRequestDetails theRequestDetails - ) { + ) throws Exception { ourLog.debug("Received Group Bulk Export Request for Group {}", theIdParam); ourLog.debug("_type={}", theIdParam); ourLog.debug("_since={}", theSince); @@ -227,7 +238,7 @@ public class BulkDataExportProvider { @OperationParam(name = JpaConstants.PARAM_EXPORT_SINCE, min = 0, max = 1, typeName = "instant") IPrimitiveType theSince, @OperationParam(name = JpaConstants.PARAM_EXPORT_TYPE_FILTER, min = 0, max = OperationParam.MAX_UNLIMITED, typeName = "string") List> theTypeFilter, ServletRequestDetails theRequestDetails - ) { + ) throws Exception { validatePreferAsyncHeader(theRequestDetails, JpaConstants.OPERATION_EXPORT); BulkDataExportOptions bulkDataExportOptions = buildPatientBulkExportOptions(theOutputFormat, theType, theSince, theTypeFilter); validateResourceTypesAllContainPatientSearchParams(bulkDataExportOptions.getResourceTypes()); @@ -390,6 +401,9 @@ public class BulkDataExportProvider { public void writePollingLocationToResponseHeaders(ServletRequestDetails theRequestDetails, JobInfo theOutcome) { 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(); HttpServletResponse response = theRequestDetails.getServletResponse();