Fix NullPointerException when performing a system bulk export in the presence of PatientIdPartitionInterceptor. (#5660)

This commit is contained in:
Martha Mitran 2024-02-02 10:19:11 -08:00 committed by GitHub
parent 0537ab58db
commit 0f3c744f11
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 77 additions and 29 deletions

View File

@ -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."

View File

@ -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);

View File

@ -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());
}
}

View File

@ -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);

View File

@ -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<RuntimeSearchParam> compartmentSps =
ResourceCompartmentUtil.getPatientCompartmentSearchParams(resourceDef);
if (compartmentSps.isEmpty()) {
return provideNonCompartmentMemberTypeResponse(null);
List<RuntimeSearchParam> 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<RuntimeSearchParam> 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<String> getResourceIdList(
SearchParameterMap theParams, String theParamName, String theResourceType, boolean theExpectOnlyOneBool) {
List<String> idParts = new ArrayList<>();