diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5659-npe-system-bulk-export-patientid-interceptor.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5659-npe-system-bulk-export-patientid-interceptor.yaml new file mode 100644 index 00000000000..ea41710dc9c --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5659-npe-system-bulk-export-patientid-interceptor.yaml @@ -0,0 +1,5 @@ +--- +type: fix +issue: 5659 +title: "Previously, after registering built-in interceptor `PatientIdPartitionInterceptor`, the system bulk export +(with no filters) operation would fail with a NullPointerException. This has been fixed." diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/interceptor/model/ReadPartitionIdRequestDetails.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/interceptor/model/ReadPartitionIdRequestDetails.java index 4d3e61d9fce..4023248362a 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/interceptor/model/ReadPartitionIdRequestDetails.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/interceptor/model/ReadPartitionIdRequestDetails.java @@ -116,7 +116,7 @@ public class ReadPartitionIdRequestDetails extends PartitionIdRequestDetails { } else if (theResourceType != null) { op = RestOperationTypeEnum.EXTENDED_OPERATION_TYPE; } else { - op = RestOperationTypeEnum.EXTENDED_OPERATION_INSTANCE; + op = RestOperationTypeEnum.EXTENDED_OPERATION_SERVER; } return new ReadPartitionIdRequestDetails(theResourceType, op, null, null, null, null, theExtendedOperationName); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/interceptor/PatientIdPartitionInterceptorTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/interceptor/PatientIdPartitionInterceptorTest.java index 0e432bbcee5..2a15939ab66 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/interceptor/PatientIdPartitionInterceptorTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/interceptor/PatientIdPartitionInterceptorTest.java @@ -1,24 +1,32 @@ package ca.uhn.fhir.jpa.interceptor; import ca.uhn.fhir.i18n.Msg; +import ca.uhn.fhir.interceptor.model.ReadPartitionIdRequestDetails; +import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; -import ca.uhn.fhir.jpa.dao.r4.BaseJpaR4SystemTest; import ca.uhn.fhir.jpa.model.config.PartitionSettings; import ca.uhn.fhir.jpa.model.entity.ResourceTable; -import ca.uhn.fhir.rest.api.server.SystemRequestDetails; +import ca.uhn.fhir.jpa.provider.BaseResourceProviderR4Test; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.searchparam.extractor.ISearchParamExtractor; import ca.uhn.fhir.jpa.util.SqlQuery; import ca.uhn.fhir.model.api.Include; +import ca.uhn.fhir.rest.api.Constants; +import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.IBundleProvider; +import ca.uhn.fhir.rest.api.server.SystemRequestDetails; +import ca.uhn.fhir.rest.api.server.bulk.BulkExportJobParameters; import ca.uhn.fhir.rest.param.ReferenceParam; import ca.uhn.fhir.rest.param.TokenOrListParam; import ca.uhn.fhir.rest.param.TokenParam; import ca.uhn.fhir.rest.server.exceptions.MethodNotAllowedException; +import ca.uhn.fhir.rest.server.provider.ProviderConstants; import ca.uhn.fhir.util.BundleBuilder; import ca.uhn.fhir.util.MultimapCollector; import com.google.common.collect.ListMultimap; import com.google.common.collect.Multimap; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpPost; import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.Encounter; import org.hl7.fhir.r4.model.Enumerations; @@ -32,6 +40,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.mock.mockito.SpyBean; import java.io.IOException; import java.util.List; @@ -47,21 +56,25 @@ import static org.hamcrest.Matchers.matchesPattern; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.verify; -public class PatientIdPartitionInterceptorTest extends BaseJpaR4SystemTest { - +public class PatientIdPartitionInterceptorTest extends BaseResourceProviderR4Test { public static final int ALTERNATE_DEFAULT_ID = -1; - private PatientIdPartitionInterceptor mySvc; + private ForceOffsetSearchModeInterceptor myForceOffsetSearchModeInterceptor; @Autowired private ISearchParamExtractor mySearchParamExtractor; + @SpyBean + @Autowired + private PatientIdPartitionInterceptor mySvc; + @Override @BeforeEach public void before() throws Exception { super.before(); - mySvc = new PatientIdPartitionInterceptor(myFhirContext, mySearchParamExtractor, myPartitionSettings); myForceOffsetSearchModeInterceptor = new ForceOffsetSearchModeInterceptor(); myInterceptorRegistry.registerInterceptor(mySvc); @@ -539,4 +552,29 @@ public class PatientIdPartitionInterceptorTest extends BaseJpaR4SystemTest { return (Patient)update.getResource(); } + @Test + public void testIdentifyForRead_serverOperation_returnsAllPartitions() { + ReadPartitionIdRequestDetails readRequestDetails = ReadPartitionIdRequestDetails.forOperation(null, null, ProviderConstants.OPERATION_EXPORT); + RequestPartitionId requestPartitionId = mySvc.identifyForRead(readRequestDetails, mySrd); + assertEquals(requestPartitionId, RequestPartitionId.allPartitions()); + assertEquals(RestOperationTypeEnum.EXTENDED_OPERATION_SERVER, readRequestDetails.getRestOperationType()); + } + + @Test + public void testSystemBulkExport_withPatientIdPartitioningWithNoResourceType_usesNonPatientSpecificPartition() throws IOException { + final BulkExportJobParameters options = new BulkExportJobParameters(); + options.setExportStyle(BulkExportJobParameters.ExportStyle.SYSTEM); + options.setOutputFormat(Constants.CT_FHIR_NDJSON); + + HttpPost post = new HttpPost(myServer.getBaseUrl() + "/" + ProviderConstants.OPERATION_EXPORT); + post.addHeader(Constants.HEADER_PREFER, Constants.HEADER_PREFER_RESPOND_ASYNC); + + try (CloseableHttpResponse postResponse = myServer.getHttpClient().execute(post)){ + ourLog.info("Response: {}",postResponse); + assertEquals(202, postResponse.getStatusLine().getStatusCode()); + assertEquals("Accepted", postResponse.getStatusLine().getReasonPhrase()); + } + + verify(mySvc).provideNonPatientSpecificQueryResponse(any()); + } } diff --git a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/export/BulkDataExportProvider.java b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/export/BulkDataExportProvider.java index 1df5e286a99..b08e4bcba17 100644 --- a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/export/BulkDataExportProvider.java +++ b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/export/BulkDataExportProvider.java @@ -29,6 +29,7 @@ import ca.uhn.fhir.i18n.Msg; 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.interceptor.model.ReadPartitionIdRequestDetails; import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.jpa.api.config.JpaStorageSettings; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; @@ -185,9 +186,12 @@ public class BulkDataExportProvider { theOptions.setResourceTypes(resourceTypes); } + ReadPartitionIdRequestDetails theDetails = + ReadPartitionIdRequestDetails.forOperation(null, null, ProviderConstants.OPERATION_EXPORT); + // Determine and validate partition permissions (if needed). RequestPartitionId partitionId = - myRequestPartitionHelperService.determineReadPartitionForRequest(theRequestDetails, null); + myRequestPartitionHelperService.determineReadPartitionForRequest(theRequestDetails, theDetails); myRequestPartitionHelperService.validateHasPartitionPermissions(theRequestDetails, "Binary", partitionId); theOptions.setPartitionId(partitionId); diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/interceptor/PatientIdPartitionInterceptor.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/interceptor/PatientIdPartitionInterceptor.java index f399b19a383..a1399ddb592 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/interceptor/PatientIdPartitionInterceptor.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/interceptor/PatientIdPartitionInterceptor.java @@ -33,21 +33,21 @@ import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.searchparam.extractor.ISearchParamExtractor; import ca.uhn.fhir.jpa.util.ResourceCompartmentUtil; import ca.uhn.fhir.model.api.IQueryParameterType; -import ca.uhn.fhir.rest.api.RestSearchParameterTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.param.ReferenceParam; import ca.uhn.fhir.rest.server.exceptions.MethodNotAllowedException; +import ca.uhn.fhir.rest.server.provider.ProviderConstants; import jakarta.annotation.Nonnull; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.r4.model.IdType; import org.springframework.beans.factory.annotation.Autowired; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Optional; -import java.util.stream.Collectors; -import static org.apache.commons.lang3.StringUtils.isBlank; +import static org.apache.commons.lang3.StringUtils.isEmpty; import static org.apache.commons.lang3.StringUtils.isNotBlank; /** @@ -117,14 +117,15 @@ public class PatientIdPartitionInterceptor { @Hook(Pointcut.STORAGE_PARTITION_IDENTIFY_READ) public RequestPartitionId identifyForRead( ReadPartitionIdRequestDetails theReadDetails, RequestDetails theRequestDetails) { - if (isBlank(theReadDetails.getResourceType())) { - return provideNonCompartmentMemberTypeResponse(null); - } - RuntimeResourceDefinition resourceDef = myFhirContext.getResourceDefinition(theReadDetails.getResourceType()); - List compartmentSps = - ResourceCompartmentUtil.getPatientCompartmentSearchParams(resourceDef); - if (compartmentSps.isEmpty()) { - return provideNonCompartmentMemberTypeResponse(null); + + List compartmentSps = Collections.emptyList(); + if (!isEmpty(theReadDetails.getResourceType())) { + RuntimeResourceDefinition resourceDef = + myFhirContext.getResourceDefinition(theReadDetails.getResourceType()); + compartmentSps = ResourceCompartmentUtil.getPatientCompartmentSearchParams(resourceDef); + if (compartmentSps.isEmpty()) { + return provideNonCompartmentMemberTypeResponse(null); + } } //noinspection EnumSwitchStatementWhichMissesCases @@ -158,11 +159,20 @@ public class PatientIdPartitionInterceptor { } break; - + case EXTENDED_OPERATION_SERVER: + String extendedOp = theReadDetails.getExtendedOperationName(); + if (ProviderConstants.OPERATION_EXPORT.equals(extendedOp)) { + return provideNonPatientSpecificQueryResponse(theReadDetails); + } + break; default: // nothing } + if (isEmpty(theReadDetails.getResourceType())) { + return provideNonCompartmentMemberTypeResponse(null); + } + // If we couldn't identify a patient ID by the URL, let's try using the // conditional target if we have one if (theReadDetails.getConditionalTargetOrNull() != null) { @@ -172,15 +182,6 @@ public class PatientIdPartitionInterceptor { return provideNonPatientSpecificQueryResponse(theReadDetails); } - @Nonnull - private List getCompartmentSearchParams(RuntimeResourceDefinition resourceDef) { - return resourceDef.getSearchParams().stream() - .filter(param -> param.getParamType() == RestSearchParameterTypeEnum.REFERENCE) - .filter(param -> param.getProvidesMembershipInCompartments() != null - && param.getProvidesMembershipInCompartments().contains("Patient")) - .collect(Collectors.toList()); - } - private List getResourceIdList( SearchParameterMap theParams, String theParamName, String theResourceType, boolean theExpectOnlyOneBool) { List idParts = new ArrayList<>();