From 5a978646c5928517eceec2a88eaee313b3d6a28b Mon Sep 17 00:00:00 2001 From: Tadgh Date: Wed, 21 Sep 2022 21:18:12 -0700 Subject: [PATCH] Add changelog, test, and implementation (#4055) --- .../6_2_0/4051-poll-status-incomplete.yaml | 5 +++ .../jpa/bulk/BulkDataExportProviderTest.java | 4 +- .../fhir/jpa/bulk/BulkExportUseCaseTest.java | 40 +++++++++++++++++++ .../r4/BaseResourceProviderR4Test.java | 1 + .../ca/uhn/fhir/jpa/test/BaseJpaR4Test.java | 3 ++ .../export/BulkExportCreateReportStep.java | 17 ++++++++ .../models/BulkExportJobParameters.java | 13 +++++- .../jpa/api/model/BulkExportJobResults.java | 11 +++++ .../jpa/api/model/BulkExportParameters.java | 14 +++++++ .../provider/BulkDataExportProvider.java | 11 ++++- 10 files changed, 113 insertions(+), 6 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/4051-poll-status-incomplete.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/4051-poll-status-incomplete.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/4051-poll-status-incomplete.yaml new file mode 100644 index 00000000000..76a7ef109c4 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/4051-poll-status-incomplete.yaml @@ -0,0 +1,5 @@ +--- +type: fix +issue: 4051 +jira: SMILE-5169 +title: "Fixed the `$poll-export-status` endpoint so that when a job is complete, this endpoint now correctly includes the `request` and `requiresAccessToken` attributes." 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 b20cb4db946..6f4dc9b91f3 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 @@ -205,8 +205,6 @@ public class BulkDataExportProviderTest { when(myJobRunner.startNewJob(any())) .thenReturn(createJobStartResponse()); - InstantType now = InstantType.now(); - Parameters input = new Parameters(); HttpPost post = new HttpPost("http://localhost:" + myPort + "/" + JpaConstants.OPERATION_EXPORT); post.addHeader(Constants.HEADER_PREFER, Constants.HEADER_PREFER_RESPOND_ASYNC); @@ -356,6 +354,7 @@ public class BulkDataExportProviderTest { ids.add(new IdType("Binary/222").getValueAsString()); ids.add(new IdType("Binary/333").getValueAsString()); BulkExportJobResults results = new BulkExportJobResults(); + HashMap> map = new HashMap<>(); map.put("Patient", ids); results.setResourceTypeToBinaryIds(map); @@ -665,7 +664,6 @@ public class BulkDataExportProviderTest { ourLog.info("Request: {}", post); try (CloseableHttpResponse response = myClient.execute(post)) { ourLog.info("Response: {}", response.toString()); - assertEquals(202, response.getStatusLine().getStatusCode()); assertEquals("Accepted", response.getStatusLine().getReasonPhrase()); assertEquals("http://localhost:" + myPort + "/$export-poll-status?_jobId=" + A_JOB_ID, response.getFirstHeader(Constants.HEADER_CONTENT_LOCATION).getValue()); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/BulkExportUseCaseTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/BulkExportUseCaseTest.java index 3e6794df379..8cf27d3a2f4 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/BulkExportUseCaseTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/BulkExportUseCaseTest.java @@ -4,6 +4,7 @@ import ca.uhn.fhir.jpa.api.config.DaoConfig; import ca.uhn.fhir.jpa.api.model.BulkExportJobResults; import ca.uhn.fhir.jpa.api.svc.IBatch2JobRunner; import ca.uhn.fhir.jpa.batch.models.Batch2JobStartResponse; +import ca.uhn.fhir.jpa.bulk.export.model.BulkExportResponseJson; import ca.uhn.fhir.jpa.provider.r4.BaseResourceProviderR4Test; import ca.uhn.fhir.jpa.util.BulkExportUtils; import ca.uhn.fhir.parser.IParser; @@ -37,6 +38,7 @@ import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -45,8 +47,10 @@ import java.util.Map; import static org.awaitility.Awaitility.await; import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.notNullValue; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.not; @@ -59,6 +63,42 @@ public class BulkExportUseCaseTest extends BaseResourceProviderR4Test { @Autowired private IBatch2JobRunner myJobRunner; + @Nested + public class SpecConformanceTests { + @Test + public void testPollingLocationContainsAllRequiredAttributesUponCompletion() throws IOException { + + //Given a patient exists + Patient p = new Patient(); + p.setId("Pat-1"); + myClient.update().resource(p).execute(); + + //And Given we start a bulk export job + HttpGet httpGet = new HttpGet(myClient.getServerBase() + "/$export?_type=Patient"); + httpGet.addHeader(Constants.HEADER_PREFER, Constants.HEADER_PREFER_RESPOND_ASYNC); + String pollingLocation; + try (CloseableHttpResponse status = ourHttpClient.execute(httpGet)) { + Header[] headers = status.getHeaders("Content-Location"); + pollingLocation = headers[0].getValue(); + } + String jobId = pollingLocation.substring(pollingLocation.indexOf("_jobId=") + 7); + myBatch2JobHelper.awaitJobCompletion(jobId); + + //Then: When the poll shows as complete, all attributes should be filled. + HttpGet statusGet = new HttpGet(pollingLocation); + try (CloseableHttpResponse status = ourHttpClient.execute(statusGet)) { + String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8); + ourLog.info(responseContent); + + BulkExportResponseJson result = JsonUtil.deserialize(responseContent, BulkExportResponseJson.class); + assertThat(result.getRequest(), is(equalTo(myClient.getServerBase() + "/$export?_type=Patient"))); + assertThat(result.getRequiresAccessToken(), is(equalTo(true))); + assertThat(result.getTransactionTime(), is(notNullValue())); + assertThat(result.getError(), is(empty())); + assertThat(result.getOutput(), is(not(empty()))); + } + } + } @Nested public class SystemBulkExportTests { @Test diff --git a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/provider/r4/BaseResourceProviderR4Test.java b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/provider/r4/BaseResourceProviderR4Test.java index 06e926fd4c4..98e047c4ac5 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/provider/r4/BaseResourceProviderR4Test.java +++ b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/provider/r4/BaseResourceProviderR4Test.java @@ -126,6 +126,7 @@ public abstract class BaseResourceProviderR4Test extends BaseJpaR4Test { ourRestServer = new RestfulServer(myFhirContext); ourRestServer.registerProviders(myResourceProviders.createProviders()); ourRestServer.registerProvider(myBinaryAccessProvider); + ourRestServer.registerProvider(myBulkDataExportProvider); ourRestServer.getInterceptorService().registerInterceptor(myBinaryStorageInterceptor); ourRestServer.getFhirContext().setNarrativeGenerator(new DefaultThymeleafNarrativeGenerator()); ourRestServer.setDefaultResponseEncoding(EncodingEnum.XML); diff --git a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/BaseJpaR4Test.java b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/BaseJpaR4Test.java index e68f8b0466a..482427ddbd8 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/BaseJpaR4Test.java +++ b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/BaseJpaR4Test.java @@ -39,6 +39,7 @@ import ca.uhn.fhir.jpa.batch.api.IBatchJobSubmitter; import ca.uhn.fhir.jpa.binary.provider.BinaryAccessProvider; import ca.uhn.fhir.jpa.binary.interceptor.BinaryStorageInterceptor; import ca.uhn.fhir.jpa.bulk.export.api.IBulkDataExportJobSchedulingHelper; +import ca.uhn.fhir.jpa.bulk.export.provider.BulkDataExportProvider; import ca.uhn.fhir.jpa.dao.IFulltextSearchSvc; import ca.uhn.fhir.jpa.dao.data.IForcedIdDao; import ca.uhn.fhir.jpa.dao.data.IMdmLinkJpaRepository; @@ -263,6 +264,8 @@ public abstract class BaseJpaR4Test extends BaseJpaTest implements ITestDataBuil @Autowired protected BinaryAccessProvider myBinaryAccessProvider; @Autowired + protected BulkDataExportProvider myBulkDataExportProvider; + @Autowired protected BinaryStorageInterceptor myBinaryStorageInterceptor; @Autowired protected ApplicationContext myAppCtx; diff --git a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/export/BulkExportCreateReportStep.java b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/export/BulkExportCreateReportStep.java index 9364623bc49..2b155ff17f3 100644 --- a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/export/BulkExportCreateReportStep.java +++ b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/export/BulkExportCreateReportStep.java @@ -22,6 +22,7 @@ package ca.uhn.fhir.batch2.jobs.export; import ca.uhn.fhir.batch2.api.ChunkExecutionDetails; import ca.uhn.fhir.batch2.api.IJobDataSink; +import ca.uhn.fhir.batch2.api.IJobInstance; import ca.uhn.fhir.batch2.api.IReductionStepWorker; import ca.uhn.fhir.batch2.api.JobExecutionFailedException; import ca.uhn.fhir.batch2.api.RunOutcome; @@ -29,6 +30,7 @@ import ca.uhn.fhir.batch2.api.StepExecutionDetails; import ca.uhn.fhir.batch2.jobs.export.models.BulkExportBinaryFileId; import ca.uhn.fhir.batch2.jobs.export.models.BulkExportJobParameters; import ca.uhn.fhir.batch2.model.ChunkOutcome; +import ca.uhn.fhir.batch2.model.JobInstance; import ca.uhn.fhir.jpa.api.model.BulkExportJobResults; import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; @@ -51,6 +53,9 @@ public class BulkExportCreateReportStep implements IReductionStepWorker theDataSink) throws JobExecutionFailedException { BulkExportJobResults results = new BulkExportJobResults(); + String requestUrl = getOriginatingRequestUrl(theStepExecutionDetails, results); + results.setOriginalRequestUrl(requestUrl); + if (myResourceToBinaryIds != null) { ourLog.info("Bulk Export Report creation step"); @@ -69,6 +74,18 @@ public class BulkExportCreateReportStep implements IReductionStepWorker theStepExecutionDetails, BulkExportJobResults results) { + IJobInstance instance = theStepExecutionDetails.getInstance(); + String url = ""; + if (instance instanceof JobInstance) { + JobInstance jobInstance = (JobInstance) instance; + BulkExportJobParameters parameters = jobInstance.getParameters(BulkExportJobParameters.class); + String originalRequestUrl = parameters.getOriginalRequestUrl(); + url = originalRequestUrl; + } + return url; + } + @NotNull @Override public ChunkOutcome consume(ChunkExecutionDetails myPatientIds; - // Stuff for group export only + @JsonProperty("originalRequestUrl") + private String myOriginalRequestUrl; /** * The group id @@ -134,6 +135,14 @@ public class BulkExportJobParameters extends BulkExportJobBase { myExpandMdm = theExpandMdm; } + private void setOriginalRequestUrl(String theOriginalRequestUrl) { + this.myOriginalRequestUrl = theOriginalRequestUrl; + } + + public String getOriginalRequestUrl() { + return myOriginalRequestUrl; + } + public static BulkExportJobParameters createFromExportJobParameters(BulkExportParameters theParameters) { BulkExportJobParameters params = new BulkExportJobParameters(); params.setResourceTypes(theParameters.getResourceTypes()); @@ -144,6 +153,8 @@ public class BulkExportJobParameters extends BulkExportJobBase { params.setStartDate(theParameters.getStartDate()); params.setExpandMdm(theParameters.isExpandMdm()); params.setPatientIds(theParameters.getPatientIds()); + params.setOriginalRequestUrl(theParameters.getOriginalRequestUrl()); return params; } + } diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/model/BulkExportJobResults.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/model/BulkExportJobResults.java index 29ae4652ea8..3fc9b9132cd 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/model/BulkExportJobResults.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/model/BulkExportJobResults.java @@ -35,6 +35,9 @@ public class BulkExportJobResults implements IModelJson { @JsonProperty("reportMessage") private String myReportMsg; + @JsonProperty("originalRequestUrl") + private String myOriginalRequestUrl; + public BulkExportJobResults() { } @@ -45,6 +48,14 @@ public class BulkExportJobResults implements IModelJson { return myResourceTypeToBinaryIds; } + public String getOriginalRequestUrl() { + return myOriginalRequestUrl; + } + + public void setOriginalRequestUrl(String theOriginalRequestUrl) { + myOriginalRequestUrl = theOriginalRequestUrl; + } + public void setResourceTypeToBinaryIds(Map> theResourceTypeToBinaryIds) { myResourceTypeToBinaryIds = theResourceTypeToBinaryIds; } diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/model/BulkExportParameters.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/model/BulkExportParameters.java index d4af19cb314..75a60f9bb72 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/model/BulkExportParameters.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/model/BulkExportParameters.java @@ -70,11 +70,17 @@ public class BulkExportParameters extends Batch2BaseJobParameters { */ private boolean myExpandMdm; + /** * Patient id(s) */ private List myPatientIds; + /** + * The request which originated the request. + */ + private String myOriginalRequestUrl; + public boolean isExpandMdm() { return myExpandMdm; } @@ -145,4 +151,12 @@ public class BulkExportParameters extends Batch2BaseJobParameters { public void setPatientIds(List thePatientIds) { myPatientIds = thePatientIds; } + + public String getOriginalRequestUrl() { + return myOriginalRequestUrl; + } + + public void setOriginalRequestUrl(String theOriginalRequestUrl) { + myOriginalRequestUrl = theOriginalRequestUrl; + } } 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 e33e3d82ca4..af9d2e95278 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 @@ -123,9 +123,14 @@ public class BulkDataExportProvider { // get cache boolean boolean useCache = shouldUseCache(theRequestDetails); + + BulkExportParameters parameters = BulkExportUtils.createBulkExportJobParametersFromExportOptions(theOptions); parameters.setUseExistingJobsFirst(useCache); + // Set the original request URL as part of the job information, as this is used in the poll-status-endpoint, and is needed for the report. + parameters.setOriginalRequestUrl(theRequestDetails.getCompleteUrl()); + // start job Batch2JobStartResponse response = myJobRunner.startNewJob(parameters); @@ -282,6 +287,9 @@ public class BulkDataExportProvider { BulkExportResponseJson bulkResponseDocument = new BulkExportResponseJson(); bulkResponseDocument.setTransactionTime(info.getEndTime()); // completed + bulkResponseDocument.setRequiresAccessToken(true); + + String report = info.getReport(); if (isEmpty(report)) { // this should never happen, but just in case... @@ -289,9 +297,8 @@ public class BulkDataExportProvider { response.getWriter().close(); } else { BulkExportJobResults results = JsonUtil.deserialize(report, BulkExportJobResults.class); - - // if there is a message.... bulkResponseDocument.setMsg(results.getReportMsg()); + bulkResponseDocument.setRequest(results.getOriginalRequestUrl()); String serverBase = getDefaultPartitionServerBase(theRequestDetails);