honeymoose
/
hapi-fhir
mirror of https://github.com/hapifhir/hapi-fhir.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

3685 When bulk exporting, if no resource type param is provided, defa… (#4233) 

* 3685 When bulk exporting, if no resource type param is provided, default to all registered types.

* Update test case.

* Cleaned up changelog.

* Added test case for multiple resource types.

* Added failing test case for not returning Binary resource.

* Refactor solution.

Co-authored-by: kylejule <kyle.jule@smilecdr.com>
This commit is contained in:
KGJ-software 2022-11-04 09:43:30 -06:00 committed by GitHub
parent b965347b8b
commit 045ceb37a8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 157 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/4232-default-export-bulk-to-all-registered-resource-types.yaml Normal file
View File

@ -0,0 +1,4 @@
---
type: fix
issue: 4232
title: "Previously during Bulk Export, if no `_type` parameter was provided, an error would be thrown. This has been changed, and if the `_type` parameter is omitted, Bulk Export will default to all registered types."

9
hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/BulkDataExportProviderTest.java
View File

@ -2,6 +2,7 @@ package ca.uhn.fhir.jpa.bulk;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.jpa.api.config.DaoConfig;
import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
import ca.uhn.fhir.jpa.api.model.Batch2JobInfo;
import ca.uhn.fhir.jpa.api.model.Batch2JobOperationResult;
import ca.uhn.fhir.jpa.api.model.BulkExportJobResults;
@ -57,6 +58,7 @@ import java.util.ArrayList;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.stream.Stream;
@ -74,6 +76,8 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@ -94,7 +98,7 @@ public class BulkDataExportProviderTest {
private IBatch2JobRunner myJobRunner;
private DaoConfig myDaoConfig;
private DaoRegistry myDaoRegistry;
private CloseableHttpClient myClient;
@InjectMocks
@ -137,6 +141,9 @@ public class BulkDataExportProviderTest {
public void injectDaoConfig() {
myDaoConfig = new DaoConfig();
myProvider.setDaoConfig(myDaoConfig);
myDaoRegistry = mock(DaoRegistry.class);
lenient().when(myDaoRegistry.getRegisteredDaoTypes()).thenReturn(Set.of("Patient", "Observation", "Encounter"));
myProvider.setDaoRegistry(myDaoRegistry);
}
public void startWithFixedBaseUrl() {

125
hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/BulkExportUseCaseTest.java
View File

@ -25,6 +25,7 @@ import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.r4.model.Binary;
import org.hl7.fhir.r4.model.Bundle;
import org.hl7.fhir.r4.model.Coverage;
import org.hl7.fhir.r4.model.Encounter;
import org.hl7.fhir.r4.model.Enumerations;
import org.hl7.fhir.r4.model.Group;
import org.hl7.fhir.r4.model.IdType;
@ -48,6 +49,7 @@ import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import static org.awaitility.Awaitility.await;
import static org.hamcrest.CoreMatchers.is;
@ -56,6 +58,7 @@ 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.hasItem;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.not;
import static org.junit.jupiter.api.Assertions.assertEquals;
@ -63,7 +66,6 @@ import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
public class BulkExportUseCaseTest extends BaseResourceProviderR4Test {
private static final Logger ourLog = LoggerFactory.getLogger(BulkExportUseCaseTest.class);
@ -113,6 +115,126 @@ public class BulkExportUseCaseTest extends BaseResourceProviderR4Test {
assertThat(responseContent, containsString("\"error\" : [ ]"));
}
}
@Test
public void export_shouldExportPatientResource_whenTypeParameterOmitted() 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");
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);
String expectedOriginalUrl = myClient.getServerBase() + "/$export";
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(expectedOriginalUrl)));
assertThat(result.getRequiresAccessToken(), is(equalTo(true)));
assertThat(result.getTransactionTime(), is(notNullValue()));
assertThat(result.getOutput(), is(not(empty())));
//We assert specifically on content as the deserialized version will "helpfully" fill in missing fields.
assertThat(responseContent, containsString("\"error\" : [ ]"));
}
}
@Test
public void export_shouldExportPatientAndObservationAndEncounterResources_whenTypeParameterOmitted() throws IOException {
Patient patient = new Patient();
patient.setId("Pat-1");
myClient.update().resource(patient).execute();
Observation observation = new Observation();
observation.setId("Obs-1");
myClient.update().resource(observation).execute();
Encounter encounter = new Encounter();
encounter.setId("Enc-1");
myClient.update().resource(encounter).execute();
HttpGet httpGet = new HttpGet(myClient.getServerBase() + "/$export");
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);
HttpGet statusGet = new HttpGet(pollingLocation);
String expectedOriginalUrl = myClient.getServerBase() + "/$export";
try (CloseableHttpResponse status = ourHttpClient.execute(statusGet)) {
String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
BulkExportResponseJson result = JsonUtil.deserialize(responseContent, BulkExportResponseJson.class);
assertThat(result.getRequest(), is(equalTo(expectedOriginalUrl)));
assertThat(result.getRequiresAccessToken(), is(equalTo(true)));
assertThat(result.getTransactionTime(), is(notNullValue()));
assertEquals(result.getOutput().size(), 3);
assertEquals(1, result.getOutput().stream().filter(o -> o.getType().equals("Patient")).collect(Collectors.toList()).size());
assertEquals(1, result.getOutput().stream().filter(o -> o.getType().equals("Observation")).collect(Collectors.toList()).size());
assertEquals(1, result.getOutput().stream().filter(o -> o.getType().equals("Encounter")).collect(Collectors.toList()).size());
//We assert specifically on content as the deserialized version will "helpfully" fill in missing fields.
assertThat(responseContent, containsString("\"error\" : [ ]"));
}
}
@Test
public void export_shouldNotExportBinaryResource_whenTypeParameterOmitted() throws IOException {
Patient patient = new Patient();
patient.setId("Pat-1");
myClient.update().resource(patient).execute();
Binary binary = new Binary();
binary.setId("Bin-1");
myClient.update().resource(binary).execute();
HttpGet httpGet = new HttpGet(myClient.getServerBase() + "/$export");
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);
HttpGet statusGet = new HttpGet(pollingLocation);
String expectedOriginalUrl = myClient.getServerBase() + "/$export";
try (CloseableHttpResponse status = ourHttpClient.execute(statusGet)) {
String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
BulkExportResponseJson result = JsonUtil.deserialize(responseContent, BulkExportResponseJson.class);
assertThat(result.getRequest(), is(equalTo(expectedOriginalUrl)));
assertThat(result.getRequiresAccessToken(), is(equalTo(true)));
assertThat(result.getTransactionTime(), is(notNullValue()));
assertEquals(result.getOutput().size(), 1);
assertEquals(1, result.getOutput().stream().filter(o -> o.getType().equals("Patient")).collect(Collectors.toList()).size());
assertEquals(0, result.getOutput().stream().filter(o -> o.getType().equals("Binary")).collect(Collectors.toList()).size());
//We assert specifically on content as the deserialized version will "helpfully" fill in missing fields.
assertThat(responseContent, containsString("\"error\" : [ ]"));
}
}
}
@Nested
@ -233,7 +355,6 @@ public class BulkExportUseCaseTest extends BaseResourceProviderR4Test {
}
@Nested
public class PatientBulkExportTests {

7
hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/export/BulkExportJobParametersValidator.java
View File

@ -43,11 +43,8 @@ public class BulkExportJobParametersValidator implements IJobParametersValidator
List<String> errorMsgs = new ArrayList<>();
// initial validation
if (theParameters.getResourceTypes() == null || theParameters.getResourceTypes().isEmpty()) {
errorMsgs.add("Resource Types are required for an export job.");
}
else {
List<String> resourceTypes = theParameters.getResourceTypes();
if (resourceTypes != null && !resourceTypes.isEmpty()) {
for (String resourceType : theParameters.getResourceTypes()) {
if (resourceType.equalsIgnoreCase("Binary")) {
errorMsgs.add("Bulk export of Binary resources is forbidden");

1
hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/export/FetchResourceIdsStep.java
View File

@ -31,6 +31,7 @@ import ca.uhn.fhir.batch2.jobs.export.models.BulkExportJobParameters;
import ca.uhn.fhir.batch2.jobs.models.Id;
import ca.uhn.fhir.i18n.Msg;
import ca.uhn.fhir.jpa.api.config.DaoConfig;
import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
import ca.uhn.fhir.jpa.bulk.export.api.IBulkExportProcessor;
import ca.uhn.fhir.jpa.bulk.export.model.ExportPIDIteratorParameters;
import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId;

5
hapi-fhir-storage-batch2-jobs/src/test/java/ca/uhn/fhir/batch2/jobs/export/BulkExportJobParametersValidatorTest.java
View File

@ -130,7 +130,7 @@ public class BulkExportJobParametersValidatorTest {
}
@Test
public void validate_omittedResourceTypes_returnsErrorMessages() {
public void validate_omittedResourceTypes_returnsNoErrorMessages() {
// setup
BulkExportJobParameters parameters = createSystemExportParameters();
parameters.setResourceTypes(null);
@ -140,8 +140,7 @@ public class BulkExportJobParametersValidatorTest {
// verify
assertNotNull(results);
assertEquals(1, results.size());
assertTrue(results.contains("Resource Types are required for an export job."));
assertEquals(0, results.size());
}
@Test

18
hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/bulk/export/provider/BulkDataExportProvider.java
View File

@ -26,6 +26,7 @@ import ca.uhn.fhir.interceptor.api.HookParams;
import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster;
import ca.uhn.fhir.interceptor.api.Pointcut;
import ca.uhn.fhir.jpa.api.config.DaoConfig;
import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
import ca.uhn.fhir.jpa.api.model.Batch2JobInfo;
import ca.uhn.fhir.jpa.api.model.Batch2JobOperationResult;
import ca.uhn.fhir.jpa.api.model.BulkExportJobResults;
@ -57,7 +58,6 @@ import ca.uhn.fhir.util.OperationOutcomeUtil;
import ca.uhn.fhir.util.SearchParameterUtil;
import ca.uhn.fhir.util.UrlUtil;
import com.google.common.annotations.VisibleForTesting;
import ca.uhn.fhir.util.SearchParameterUtil;
import org.apache.commons.lang3.StringUtils;
import org.hl7.fhir.instance.model.api.IBaseOperationOutcome;
import org.hl7.fhir.instance.model.api.IIdType;
@ -69,6 +69,7 @@ import org.springframework.beans.factory.annotation.Autowired;
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
@ -101,6 +102,9 @@ public class BulkDataExportProvider {
@Autowired
private DaoConfig myDaoConfig;
@Autowired
private DaoRegistry myDaoRegistry;
/**
* $export
*/
@ -137,6 +141,13 @@ public class BulkDataExportProvider {
// 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());
// If no _type parameter is provided, default to all resource types except Binary
if (theOptions.getResourceTypes() == null || theOptions.getResourceTypes().isEmpty()) {
List<String> resourceTypes = new ArrayList<>(myDaoRegistry.getRegisteredDaoTypes());
resourceTypes.remove("Binary");
parameters.setResourceTypes(resourceTypes);
}
// start job
Batch2JobStartResponse response = myJobRunner.startNewJob(parameters);
@ -487,4 +498,9 @@ public class BulkDataExportProvider {
public void setDaoConfig(DaoConfig theDaoConfig) {
myDaoConfig = theDaoConfig;
}
@VisibleForTesting
public void setDaoRegistry(DaoRegistry theDaoRegistry) {
myDaoRegistry = theDaoRegistry;
}
}