Throw 404 when requesting $export of non-existent Group or Patient (#…

…4890)

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_6_0/4886-bulk-export-target-404.yaml

@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 4886
+title: "Requests to start an $export of Patient or Group will now fail with 404 ResourceNotFound when the target
+ resources do not exist. Before, the system would start a bulk export background job which would then fail."

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

@@ -5,6 +5,7 @@
 import ca.uhn.fhir.interceptor.model.RequestPartitionId;
 import ca.uhn.fhir.jpa.api.config.JpaStorageSettings;
 import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
+import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao;
 import ca.uhn.fhir.jpa.api.model.Batch2JobInfo;
 import ca.uhn.fhir.jpa.api.model.Batch2JobOperationResult;
 import ca.uhn.fhir.jpa.api.model.BulkExportJobResults;
@@ -98,6 +99,8 @@ public class BulkDataExportProviderTest {
  private final HttpClientExtension myClient = new HttpClientExtension();
  @Mock
  private IBatch2JobRunner myJobRunner;
+ @Mock
+ IFhirResourceDao myFhirResourceDao;
  @InjectMocks
  private BulkDataExportProvider myProvider;
  @RegisterExtension
@@ -140,6 +143,8 @@ public void injectStorageSettings() {
  myProvider.setStorageSettings(myStorageSettings);
  DaoRegistry daoRegistry = mock(DaoRegistry.class);
  lenient().when(daoRegistry.getRegisteredDaoTypes()).thenReturn(Set.of("Patient", "Observation", "Encounter"));
+
+ lenient().when(daoRegistry.getResourceDao(anyString())).thenReturn(myFhirResourceDao);
  myProvider.setDaoRegistry(daoRegistry);
 
  }

hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/BulkExportProviderR4Test.java

@@ -0,0 +1,66 @@
+package ca.uhn.fhir.jpa.provider.r4;
+
+import ca.uhn.fhir.jpa.provider.BaseResourceProviderR4Test;
+import ca.uhn.fhir.model.primitive.IdDt;
+import ca.uhn.fhir.model.primitive.StringDt;
+import ca.uhn.fhir.rest.api.MethodOutcome;
+import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
+import org.hl7.fhir.r4.model.Parameters;
+import org.hl7.fhir.r4.model.StringType;
+import org.junit.jupiter.api.Test;
+
+import static ca.uhn.fhir.jpa.model.util.JpaConstants.OPERATION_EXPORT;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+public class BulkExportProviderR4Test extends BaseResourceProviderR4Test {
+ @Test
+ void testBulkExport_groupNotExists_throws404() {
+ // given no data
+
+ ResourceNotFoundException e = assertThrows(ResourceNotFoundException.class,
+ () -> myClient
+ .operation().onInstance("Group/ABC_not_exist").named(OPERATION_EXPORT)
+ .withNoParameters(Parameters.class)
+ .withAdditionalHeader("Prefer", "respond-async")
+ .returnMethodOutcome()
+ .execute(),
+ "$export of missing Group throws 404");
+
+ assertThat(e.getStatusCode(), equalTo(404));
+ }
+
+ @Test
+ void testBulkExport_patientNotExists_throws404() {
+ // given no data
+
+ ResourceNotFoundException e = assertThrows(ResourceNotFoundException.class,
+ () -> myClient
+ .operation().onInstance("Patient/ABC_not_exist").named(OPERATION_EXPORT)
+ .withNoParameters(Parameters.class)
+ .withAdditionalHeader("Prefer", "respond-async")
+ .returnMethodOutcome()
+ .execute(),
+ "$export of missing Patient throws 404");
+
+ assertThat(e.getStatusCode(), equalTo(404));
+ }
+
+
+ @Test
+ void testBulkExport_typePatientIdNotExists_throws404() {
+ // given no data
+
+ ResourceNotFoundException e = assertThrows(ResourceNotFoundException.class,
+ () -> myClient
+ .operation().onType("Patient").named(OPERATION_EXPORT)
+ .withParameter(Parameters.class, "patient", new StringType("Patient/abc-no-way"))
+ .withAdditionalHeader("Prefer", "respond-async")
+ .returnMethodOutcome()
+ .execute(),
+ "Patient/$export with missing patient throws 404");
+
+ assertThat(e.getStatusCode(), equalTo(404));
+ }
+}

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

@@ -38,6 +38,7 @@
 import ca.uhn.fhir.jpa.model.util.JpaConstants;
 import ca.uhn.fhir.jpa.partition.IRequestPartitionHelperSvc;
 import ca.uhn.fhir.jpa.util.BulkExportUtils;
+import ca.uhn.fhir.model.primitive.IdDt;
 import ca.uhn.fhir.model.primitive.StringDt;
 import ca.uhn.fhir.rest.annotation.IdParam;
 import ca.uhn.fhir.rest.annotation.Operation;
@@ -47,6 +48,7 @@
 import ca.uhn.fhir.rest.api.PreferHeader;
 import ca.uhn.fhir.rest.api.RequestTypeEnum;
 import ca.uhn.fhir.rest.api.server.RequestDetails;
+import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
 import ca.uhn.fhir.rest.api.server.bulk.BulkDataExportOptions;
 import ca.uhn.fhir.rest.server.RestfulServerUtils;
 import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
@@ -60,6 +62,8 @@
 import ca.uhn.fhir.util.SearchParameterUtil;
 import ca.uhn.fhir.util.UrlUtil;
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.Lists;
+import org.apache.commons.collections4.ListUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.hl7.fhir.instance.model.api.IBaseOperationOutcome;
 import org.hl7.fhir.instance.model.api.IIdType;
@@ -211,6 +215,9 @@ public void groupExport(
 
  validatePreferAsyncHeader(theRequestDetails, JpaConstants.OPERATION_EXPORT);
 
+ // verify the Group exists before starting the job
+ validateTargetsExists(theRequestDetails, "Group", List.of(theIdParam));
+
  BulkDataExportOptions bulkDataExportOptions = buildGroupBulkExportOptions(theOutputFormat, theType, theSince, theTypeFilter, theIdParam, theMdm, theExportIdentifier, theTypePostFetchFilterUrl);
 
  if (isNotEmpty(bulkDataExportOptions.getResourceTypes())) {
@@ -223,6 +230,24 @@ public void groupExport(
  startJob(theRequestDetails, bulkDataExportOptions);
  }
 
+ /**
+ * Throw ResourceNotFound if the target resources don't exist.
+ * Otherwise, we start a bulk-export job which then fails, reporting a 500.
+ *
+ * @param theRequestDetails the caller details
+ * @param theTargetResourceName the type of the target
+ * @param theIdParams the id(s) to verify exist
+ */
+ private void validateTargetsExists(RequestDetails theRequestDetails, String theTargetResourceName, Iterable<IIdType> theIdParams) {
+ RequestPartitionId partitionId = myRequestPartitionHelperService.determineReadPartitionForRequestForRead(theRequestDetails, theTargetResourceName, theIdParams.iterator().next());
+ SystemRequestDetails requestDetails = new SystemRequestDetails().setRequestPartitionId(partitionId);
+ for (IIdType nextId: theIdParams) {
+ myDaoRegistry.getResourceDao(theTargetResourceName)
+ .read(nextId, requestDetails);
+ }
+
+ }
+
  private void validateResourceTypesAllContainPatientSearchParams(Set<String> theResourceTypes) {
  if (theResourceTypes != null) {
  List<String> badResourceTypes = theResourceTypes.stream()
@@ -259,6 +284,11 @@ public void patientExport(
  ServletRequestDetails theRequestDetails
  ) {
  validatePreferAsyncHeader(theRequestDetails, JpaConstants.OPERATION_EXPORT);
+
+ if (thePatient != null) {
+ validateTargetsExists(theRequestDetails, "Patient", Lists.transform(thePatient, s -> new IdDt(s.getValue())));
+ }
+
  BulkDataExportOptions bulkDataExportOptions = buildPatientBulkExportOptions(theOutputFormat, theType, theSince, theTypeFilter, theExportIdentifier, thePatient, theTypePostFetchFilterUrl);
  validateResourceTypesAllContainPatientSearchParams(bulkDataExportOptions.getResourceTypes());
 
@@ -280,6 +310,9 @@ public void patientInstanceExport(
  ServletRequestDetails theRequestDetails
  ) {
  validatePreferAsyncHeader(theRequestDetails, JpaConstants.OPERATION_EXPORT);
+
+ validateTargetsExists(theRequestDetails, "Patient", List.of(theIdParam));
+
  BulkDataExportOptions bulkDataExportOptions = buildPatientBulkExportOptions(theOutputFormat, theType, theSince, theTypeFilter, theExportIdentifier, theIdParam, theTypePostFetchFilterUrl);
  validateResourceTypesAllContainPatientSearchParams(bulkDataExportOptions.getResourceTypes());
 
@@ -529,7 +562,7 @@ private Set<String> splitTypeFilters(List<IPrimitiveType<String>> theTypeFilter)
  public static void validatePreferAsyncHeader(ServletRequestDetails theRequestDetails, String theOperationName) {
  String preferHeader = theRequestDetails.getHeader(Constants.HEADER_PREFER);
  PreferHeader prefer = RestfulServerUtils.parsePreferHeader(null, preferHeader);
- if (prefer.getRespondAsync() == false) {
+ if (!prefer.getRespondAsync()) {
  throw new InvalidRequestException(Msg.code(513) + "Must request async processing for " + theOperationName);
  }
  }