5339 bulk export errors when patient compartment searchparameter of the resource is not present (#5340)
* added failing test * implemented solution, and fixed test * added changelog * fixed formatting * fixed test not passing * fixed test and formatting * added warning for failing the active search param check * code review change --------- Co-authored-by: Steven Li <steven@smilecdr.com>
This commit is contained in:
parent
5411622dee
commit
4e9bd153cf
|
@ -0,0 +1,6 @@
|
|||
---
|
||||
type: fix
|
||||
issue: 5339
|
||||
jira: SMILE-7236
|
||||
title: "Previously, Bulk Export will error when processing a resource if the patient compartment SearchParameter of that resource is not present.
|
||||
This has been fixed, the new behaviour is to ignore such resources."
|
|
@ -53,6 +53,7 @@ import ca.uhn.fhir.rest.param.HasOrListParam;
|
|||
import ca.uhn.fhir.rest.param.HasParam;
|
||||
import ca.uhn.fhir.rest.param.ReferenceOrListParam;
|
||||
import ca.uhn.fhir.rest.param.ReferenceParam;
|
||||
import ca.uhn.fhir.rest.server.util.ISearchParamRegistry;
|
||||
import ca.uhn.fhir.util.ExtensionUtil;
|
||||
import ca.uhn.fhir.util.HapiExtensions;
|
||||
import ca.uhn.fhir.util.Logs;
|
||||
|
@ -121,6 +122,9 @@ public class JpaBulkExportProcessor implements IBulkExportProcessor<JpaPid> {
|
|||
@Autowired
|
||||
private IHapiTransactionService myHapiTransactionService;
|
||||
|
||||
@Autowired
|
||||
private ISearchParamRegistry mySearchParamRegistry;
|
||||
|
||||
private IFhirPath myFhirPath;
|
||||
|
||||
@Override
|
||||
|
@ -298,28 +302,36 @@ public class JpaBulkExportProcessor implements IBulkExportProcessor<JpaPid> {
|
|||
private LinkedHashSet<JpaPid> getRelatedResourceTypePids(
|
||||
ExportPIDIteratorParameters theParams, RuntimeResourceDefinition theDef) throws IOException {
|
||||
LinkedHashSet<JpaPid> pids = new LinkedHashSet<>();
|
||||
// expand the group pid -> list of patients in that group (list of patient pids)
|
||||
Set<JpaPid> expandedMemberResourceIds = expandAllPatientPidsFromGroup(theParams);
|
||||
assert !expandedMemberResourceIds.isEmpty();
|
||||
Logs.getBatchTroubleshootingLog()
|
||||
.debug("{} has been expanded to members:[{}]", theParams.getGroupId(), expandedMemberResourceIds);
|
||||
// Check if the patient compartment search parameter is active to enable export of this resource
|
||||
RuntimeSearchParam activeSearchParam =
|
||||
getActivePatientSearchParamForCurrentResourceType(theParams.getResourceType());
|
||||
if (activeSearchParam != null) {
|
||||
// expand the group pid -> list of patients in that group (list of patient pids)
|
||||
Set<JpaPid> expandedMemberResourceIds = expandAllPatientPidsFromGroup(theParams);
|
||||
assert !expandedMemberResourceIds.isEmpty();
|
||||
Logs.getBatchTroubleshootingLog()
|
||||
.debug("{} has been expanded to members:[{}]", theParams.getGroupId(), expandedMemberResourceIds);
|
||||
|
||||
// for each patient pid ->
|
||||
// search for the target resources, with their correct patient references, chunked.
|
||||
// The results will be jammed into myReadPids
|
||||
QueryChunker<JpaPid> queryChunker = new QueryChunker<>();
|
||||
queryChunker.chunk(expandedMemberResourceIds, QUERY_CHUNK_SIZE, (idChunk) -> {
|
||||
try {
|
||||
queryResourceTypeWithReferencesToPatients(pids, idChunk, theParams, theDef);
|
||||
} catch (IOException ex) {
|
||||
// we will never see this;
|
||||
// SearchBuilder#QueryIterator does not (nor can ever) throw
|
||||
// an IOException... but Java requires the check,
|
||||
// so we'll put a log here (just in the off chance)
|
||||
ourLog.error("Couldn't close query iterator ", ex);
|
||||
throw new RuntimeException(Msg.code(2346) + "Couldn't close query iterator", ex);
|
||||
}
|
||||
});
|
||||
// for each patient pid ->
|
||||
// search for the target resources, with their correct patient references, chunked.
|
||||
// The results will be jammed into myReadPids
|
||||
QueryChunker<JpaPid> queryChunker = new QueryChunker<>();
|
||||
queryChunker.chunk(expandedMemberResourceIds, QUERY_CHUNK_SIZE, (idChunk) -> {
|
||||
try {
|
||||
queryResourceTypeWithReferencesToPatients(pids, idChunk, theParams, theDef);
|
||||
} catch (IOException ex) {
|
||||
// we will never see this;
|
||||
// SearchBuilder#QueryIterator does not (nor can ever) throw
|
||||
// an IOException... but Java requires the check,
|
||||
// so we'll put a log here (just in the off chance)
|
||||
ourLog.error("Couldn't close query iterator ", ex);
|
||||
throw new RuntimeException(Msg.code(2346) + "Couldn't close query iterator", ex);
|
||||
}
|
||||
});
|
||||
} else {
|
||||
ourLog.warn("No active patient compartment search parameter(s) for resource type "
|
||||
+ theParams.getResourceType());
|
||||
}
|
||||
return pids;
|
||||
}
|
||||
|
||||
|
@ -600,6 +612,22 @@ public class JpaBulkExportProcessor implements IBulkExportProcessor<JpaPid> {
|
|||
}
|
||||
}
|
||||
|
||||
private RuntimeSearchParam getActivePatientSearchParamForCurrentResourceType(String theResourceType) {
|
||||
String activeSearchParamName = "";
|
||||
String resourceToCheck = theResourceType;
|
||||
if (!PATIENT_BULK_EXPORT_FORWARD_REFERENCE_RESOURCE_TYPES.contains(theResourceType)) {
|
||||
activeSearchParamName =
|
||||
getPatientSearchParamForCurrentResourceType(theResourceType).getName();
|
||||
} else if ("Practitioner".equalsIgnoreCase(theResourceType)) {
|
||||
resourceToCheck = "Patient";
|
||||
activeSearchParamName = "general-practitioner";
|
||||
} else if ("Organization".equalsIgnoreCase(theResourceType)) {
|
||||
resourceToCheck = "Patient";
|
||||
activeSearchParamName = "organization";
|
||||
}
|
||||
return mySearchParamRegistry.getActiveSearchParam(resourceToCheck, activeSearchParamName);
|
||||
}
|
||||
|
||||
/**
|
||||
* Must not be called for resources types listed in PATIENT_BULK_EXPORT_FORWARD_REFERENCE_RESOURCE_TYPES
|
||||
*
|
||||
|
|
|
@ -1400,7 +1400,7 @@ public class SearchBuilder implements ISearchBuilder<JpaPid> {
|
|||
q.setMaxResults(maxCount);
|
||||
}
|
||||
if (hasDesiredResourceTypes) {
|
||||
q.setParameter("desired_target_resource_types", String.join(", ", desiredResourceTypes));
|
||||
q.setParameter("desired_target_resource_types", desiredResourceTypes);
|
||||
}
|
||||
List<?> results = q.getResultList();
|
||||
for (Object nextRow : results) {
|
||||
|
|
|
@ -2,6 +2,7 @@ package ca.uhn.fhir.jpa.bulk.export.svc;
|
|||
|
||||
import ca.uhn.fhir.context.FhirContext;
|
||||
import ca.uhn.fhir.context.RuntimeResourceDefinition;
|
||||
import ca.uhn.fhir.context.RuntimeSearchParam;
|
||||
import ca.uhn.fhir.interceptor.model.RequestPartitionId;
|
||||
import ca.uhn.fhir.jpa.api.config.JpaStorageSettings;
|
||||
import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
|
||||
|
@ -23,12 +24,15 @@ import ca.uhn.fhir.mdm.api.MdmMatchResultEnum;
|
|||
import ca.uhn.fhir.mdm.dao.IMdmLinkDao;
|
||||
import ca.uhn.fhir.mdm.model.MdmPidTuple;
|
||||
import ca.uhn.fhir.model.primitive.IdDt;
|
||||
import ca.uhn.fhir.rest.api.RestSearchParameterTypeEnum;
|
||||
import ca.uhn.fhir.rest.api.server.RequestDetails;
|
||||
import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
|
||||
import ca.uhn.fhir.rest.api.server.bulk.BulkExportJobParameters;
|
||||
import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
|
||||
import ca.uhn.fhir.rest.server.util.ISearchParamRegistry;
|
||||
import org.hl7.fhir.instance.model.api.IIdType;
|
||||
import org.hl7.fhir.r4.model.Group;
|
||||
import org.hl7.fhir.r4.model.IdType;
|
||||
import org.hl7.fhir.r4.model.Observation;
|
||||
import org.hl7.fhir.r4.model.Patient;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
@ -139,6 +143,9 @@ public class JpaBulkExportProcessorTest {
|
|||
@Mock
|
||||
private MdmExpansionCacheSvc myMdmExpansionCacheSvc;
|
||||
|
||||
@Mock
|
||||
private ISearchParamRegistry mySearchParamRegistry;
|
||||
|
||||
@Spy
|
||||
private IHapiTransactionService myTransactionService = new NonTransactionalHapiTransactionService();
|
||||
|
||||
|
@ -409,6 +416,8 @@ public class JpaBulkExportProcessorTest {
|
|||
ISearchBuilder<JpaPid> observationSearchBuilder = mock(ISearchBuilder.class);
|
||||
|
||||
// when
|
||||
RuntimeSearchParam searchParam = new RuntimeSearchParam(new IdType("1"), "", "", "", "", RestSearchParameterTypeEnum.STRING, Collections.singleton(""), Collections.singleton(""), RuntimeSearchParam.RuntimeSearchParamStatusEnum.ACTIVE, Collections.singleton(""));
|
||||
when(mySearchParamRegistry.getActiveSearchParam(any(), any())).thenReturn(searchParam);
|
||||
// expandAllPatientPidsFromGroup
|
||||
when(myDaoRegistry.getResourceDao(eq("Group")))
|
||||
.thenReturn(groupDao);
|
||||
|
|
|
@ -12,12 +12,16 @@ import ca.uhn.fhir.jpa.api.model.BulkExportJobResults;
|
|||
import ca.uhn.fhir.jpa.batch.models.Batch2JobStartResponse;
|
||||
import ca.uhn.fhir.jpa.model.util.JpaConstants;
|
||||
import ca.uhn.fhir.jpa.provider.BaseResourceProviderR4Test;
|
||||
import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
|
||||
import ca.uhn.fhir.rest.api.Constants;
|
||||
import ca.uhn.fhir.rest.api.RequestTypeEnum;
|
||||
import ca.uhn.fhir.rest.api.server.IBundleProvider;
|
||||
import ca.uhn.fhir.rest.api.server.RequestDetails;
|
||||
import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
|
||||
import ca.uhn.fhir.rest.api.server.bulk.BulkExportJobParameters;
|
||||
import ca.uhn.fhir.rest.client.apache.ResourceEntity;
|
||||
import ca.uhn.fhir.rest.param.StringParam;
|
||||
import ca.uhn.fhir.rest.param.TokenParam;
|
||||
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
|
||||
import ca.uhn.fhir.rest.server.provider.ProviderConstants;
|
||||
import ca.uhn.fhir.test.utilities.HttpClientExtension;
|
||||
|
@ -50,6 +54,7 @@ import org.hl7.fhir.r4.model.Practitioner;
|
|||
import org.hl7.fhir.r4.model.Provenance;
|
||||
import org.hl7.fhir.r4.model.QuestionnaireResponse;
|
||||
import org.hl7.fhir.r4.model.Reference;
|
||||
import org.hl7.fhir.r4.model.SearchParameter;
|
||||
import org.hl7.fhir.r4.model.ServiceRequest;
|
||||
import org.hl7.fhir.r4.model.StringType;
|
||||
import org.junit.jupiter.api.AfterEach;
|
||||
|
@ -149,6 +154,99 @@ public class BulkDataExportTest extends BaseResourceProviderR4Test {
|
|||
verifyBulkExportResults(options, Collections.singletonList("Patient/PF"), Collections.singletonList("Patient/PM"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testGroupBulkExportWithMissingObservationSearchParams() {
|
||||
mySearchParameterDao.update(createDisabledObservationPatientSearchParameter(), mySrd);
|
||||
mySearchParamRegistry.forceRefresh();
|
||||
|
||||
// Create some resources
|
||||
Patient patient = new Patient();
|
||||
patient.setId("PF");
|
||||
patient.setGender(Enumerations.AdministrativeGender.FEMALE);
|
||||
patient.setActive(true);
|
||||
myClient.update().resource(patient).execute();
|
||||
|
||||
patient = new Patient();
|
||||
patient.setId("PM");
|
||||
patient.setGender(Enumerations.AdministrativeGender.MALE);
|
||||
patient.setActive(true);
|
||||
myClient.update().resource(patient).execute();
|
||||
|
||||
Group group = new Group();
|
||||
group.setId("Group/G");
|
||||
group.setActive(true);
|
||||
group.addMember().getEntity().setReference("Patient/PF");
|
||||
group.addMember().getEntity().setReference("Patient/PM");
|
||||
myClient.update().resource(group).execute();
|
||||
|
||||
Observation observation = new Observation();
|
||||
observation.setStatus(Observation.ObservationStatus.AMENDED);
|
||||
observation.setSubject(new Reference("Patient/PF"));
|
||||
String obsId = myClient.create().resource(observation).execute().getId().toUnqualifiedVersionless().getValue();
|
||||
|
||||
// set the export options
|
||||
BulkExportJobParameters options = new BulkExportJobParameters();
|
||||
options.setResourceTypes(Sets.newHashSet("Patient", "Observation"));
|
||||
options.setGroupId("Group/G");
|
||||
options.setExportStyle(BulkExportJobParameters.ExportStyle.GROUP);
|
||||
options.setOutputFormat(Constants.CT_FHIR_NDJSON);
|
||||
verifyBulkExportResults(options, List.of("Patient/PF", "Patient/PM"), Collections.singletonList(obsId));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testGroupBulkExportWithMissingPatientSearchParams() {
|
||||
mySearchParameterDao.update(createDisabledPatientPractitionerSearchParameter(), mySrd);
|
||||
mySearchParamRegistry.forceRefresh();
|
||||
|
||||
Practitioner practitioner = new Practitioner();
|
||||
practitioner.setActive(true);
|
||||
String practId = myClient.create().resource(practitioner).execute().getId().toUnqualifiedVersionless().getValue();
|
||||
|
||||
// Create some resources
|
||||
Patient patient = new Patient();
|
||||
patient.setId("P1");
|
||||
patient.setActive(true);
|
||||
patient.addGeneralPractitioner().setReference(practId);
|
||||
myClient.update().resource(patient).execute();
|
||||
|
||||
Group group = new Group();
|
||||
group.setId("Group/G");
|
||||
group.setActive(true);
|
||||
group.addMember().getEntity().setReference("Patient/P1");
|
||||
myClient.update().resource(group).execute();
|
||||
|
||||
// set the export options
|
||||
BulkExportJobParameters options = new BulkExportJobParameters();
|
||||
options.setResourceTypes(Sets.newHashSet("Patient", "Practitioner"));
|
||||
options.setGroupId("Group/G");
|
||||
options.setExportStyle(BulkExportJobParameters.ExportStyle.GROUP);
|
||||
options.setOutputFormat(Constants.CT_FHIR_NDJSON);
|
||||
verifyBulkExportResults(options, Collections.singletonList("Patient/P1"), Collections.singletonList(practId));
|
||||
}
|
||||
|
||||
private SearchParameter createDisabledObservationPatientSearchParameter() {
|
||||
SearchParameter observation_patient = new SearchParameter();
|
||||
observation_patient.setId("clinical-patient");
|
||||
observation_patient.addBase("Observation");
|
||||
observation_patient.setStatus(Enumerations.PublicationStatus.RETIRED);
|
||||
observation_patient.setCode("patient");
|
||||
observation_patient.setType(Enumerations.SearchParamType.REFERENCE);
|
||||
observation_patient.addTarget("Patient");
|
||||
observation_patient.setExpression("Observation.subject.where(resolve() is Patient)");
|
||||
return observation_patient;
|
||||
}
|
||||
|
||||
private SearchParameter createDisabledPatientPractitionerSearchParameter() {
|
||||
SearchParameter patient_practitioner = new SearchParameter();
|
||||
patient_practitioner.setId("Patient-general-practitioner");
|
||||
patient_practitioner.addBase("Patient");
|
||||
patient_practitioner.setStatus(Enumerations.PublicationStatus.RETIRED);
|
||||
patient_practitioner.setCode("general-practitioner");
|
||||
patient_practitioner.setType(Enumerations.SearchParamType.REFERENCE);
|
||||
patient_practitioner.addTarget("Practitioner");
|
||||
patient_practitioner.setExpression("Patient.generalPractitioner");
|
||||
return patient_practitioner;
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testGroupBulkExportWithTypeFilter_OnTags_InlineTagMode() {
|
||||
|
@ -446,12 +544,12 @@ public class BulkDataExportTest extends BaseResourceProviderR4Test {
|
|||
|
||||
// set the export options
|
||||
BulkExportJobParameters options = new BulkExportJobParameters();
|
||||
options.setResourceTypes(Sets.newHashSet("Patient", "Encounter"));
|
||||
options.setResourceTypes(Sets.newHashSet("Patient", "Encounter", "Practitioner", "Organization"));
|
||||
options.setGroupId("Group/G1");
|
||||
options.setFilters(new HashSet<>());
|
||||
options.setExportStyle(BulkExportJobParameters.ExportStyle.GROUP);
|
||||
options.setOutputFormat(Constants.CT_FHIR_NDJSON);
|
||||
verifyBulkExportResults(options, List.of("Patient/P1", practId, orgId, encId, encId2, locId), List.of("Patient/P2", orgId2, encId3, locId2));
|
||||
verifyBulkExportResults(options, List.of("Patient/P1", practId, orgId, encId, encId2), List.of("Patient/P2", orgId2, encId3));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -487,7 +585,7 @@ public class BulkDataExportTest extends BaseResourceProviderR4Test {
|
|||
|
||||
// set the export options
|
||||
BulkExportJobParameters options = new BulkExportJobParameters();
|
||||
options.setResourceTypes(Sets.newHashSet("Patient", "Encounter", "Observation"));
|
||||
options.setResourceTypes(Sets.newHashSet("Patient", "Encounter", "Observation", "Practitioner"));
|
||||
options.setGroupId("Group/G1");
|
||||
options.setFilters(new HashSet<>());
|
||||
options.setExportStyle(BulkExportJobParameters.ExportStyle.GROUP);
|
||||
|
@ -544,7 +642,7 @@ public class BulkDataExportTest extends BaseResourceProviderR4Test {
|
|||
|
||||
// set the export options
|
||||
BulkExportJobParameters options = new BulkExportJobParameters();
|
||||
options.setResourceTypes(Sets.newHashSet("Patient", "Observation", "Provenance"));
|
||||
options.setResourceTypes(Sets.newHashSet("Patient", "Observation", "Provenance", "Device"));
|
||||
options.setGroupId("Group/G1");
|
||||
options.setFilters(new HashSet<>());
|
||||
options.setExportStyle(BulkExportJobParameters.ExportStyle.GROUP);
|
||||
|
@ -944,10 +1042,7 @@ public class BulkDataExportTest extends BaseResourceProviderR4Test {
|
|||
} catch (IOException e) {
|
||||
fail(e.toString());
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
return jobInstance;
|
||||
}
|
||||
|
||||
for (String containedString : theContainedList) {
|
||||
|
|
Loading…
Reference in New Issue