add tests and fix mass ingestion and patient compartment interceptor …

…compatibility in mass ingestion mode
hapifhir · Jan 7, 2025 · 4046a20 · 4046a20
1 parent 37ab80e
commit 4046a20
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 10 deletions.
diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java
@@ -29,6 +29,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.interceptor.executor.InterceptorService;
 import ca.uhn.fhir.interceptor.model.RequestPartitionId;
 import ca.uhn.fhir.jpa.api.config.JpaStorageSettings;
 import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
@@ -47,6 +48,7 @@
 import ca.uhn.fhir.jpa.dao.data.IResourceHistoryProvenanceDao;
 import ca.uhn.fhir.jpa.dao.tx.HapiTransactionService;
 import ca.uhn.fhir.jpa.delete.DeleteConflictUtil;
+import ca.uhn.fhir.jpa.interceptor.PatientCompartmentEnforcingInterceptor;
 import ca.uhn.fhir.jpa.model.cross.IBasePersistedResource;
 import ca.uhn.fhir.jpa.model.dao.JpaPid;
 import ca.uhn.fhir.jpa.model.entity.BaseHasResource;
@@ -2572,6 +2574,9 @@ protected DaoMethodOutcome doUpdateForUpdateOrPatch(
 					null);
 		}
 
+		boolean theShouldForcePopulateOldResourceForProcessing = myInterceptorBroadcaster instanceof InterceptorService
+				&& ((InterceptorService) myInterceptorBroadcaster)
+						.hasRegisteredInterceptor(PatientCompartmentEnforcingInterceptor.class);
 		return super.doUpdateForUpdateOrPatch(
 				theRequest,
 				theResourceId,
@@ -2581,7 +2586,8 @@ protected DaoMethodOutcome doUpdateForUpdateOrPatch(
 				theResource,
 				theEntity,
 				theOperationType,
-				theTransactionDetails);
+				theTransactionDetails,
+				theShouldForcePopulateOldResourceForProcessing);
 	}
 
 	/**

diff --git a/...src/test/java/ca/uhn/fhir/jpa/interceptor/PatientCompartmentEnforcingInterceptorTest.java b/...src/test/java/ca/uhn/fhir/jpa/interceptor/PatientCompartmentEnforcingInterceptorTest.java
@@ -1,5 +1,9 @@
 package ca.uhn.fhir.jpa.interceptor;
 
+import ca.uhn.fhir.jpa.api.config.JpaStorageSettings;
+
+import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome;
+
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import ca.uhn.fhir.jpa.model.config.PartitionSettings;
 import ca.uhn.fhir.jpa.provider.BaseResourceProviderR4Test;
@@ -11,10 +15,10 @@
 import org.hl7.fhir.r4.model.Patient;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.springframework.beans.factory.annotation.Autowired;
 
-import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class PatientCompartmentEnforcingInterceptorTest extends BaseResourceProviderR4Test {
@@ -56,11 +60,14 @@ public void after() throws Exception {
 		myPartitionSettings.setUnnamedPartitionMode(defaultPartitionSettings.isUnnamedPartitionMode());
 		myPartitionSettings.setDefaultPartitionId(defaultPartitionSettings.getDefaultPartitionId());
 		myPartitionSettings.setAllowReferencesAcrossPartitions(defaultPartitionSettings.getAllowReferencesAcrossPartitions());
-	}
 
+		myStorageSettings.setMassIngestionMode(false);
+	}
 
-	@Test
-	public void testUpdateResource_whenCrossingPatientCompartment_throws() {
+	@ParameterizedTest
+	@ValueSource(booleans = {true, false})
+	public void testUpdateResource_whenCrossingPatientCompartment_throws(boolean theMassIngestionEnabled) {
+		myStorageSettings.setMassIngestionMode(theMassIngestionEnabled);
 		myPartitionSettings.setAllowReferencesAcrossPartitions(PartitionSettings.CrossPartitionReferenceMode.ALLOWED_UNQUALIFIED);
 		createPatientA();
 		createPatientB();
@@ -76,8 +83,10 @@ public void testUpdateResource_whenCrossingPatientCompartment_throws() {
 		assertEquals("HAPI-2476: Resource compartment changed. Was a referenced Patient changed?", thrown.getMessage());
 	}
 
-	@Test
-	public void testUpdateResource_whenNotCrossingPatientCompartment_allows() {
+	@ParameterizedTest
+	@ValueSource(booleans = {true, false})
+	public void testUpdateResource_whenNotCrossingPatientCompartment_allows(boolean theMassIngestionEnabled) {
+		myStorageSettings.setMassIngestionMode(theMassIngestionEnabled);
 		createPatientA();
 
 		Observation obs = new Observation();
@@ -87,7 +96,8 @@ public void testUpdateResource_whenNotCrossingPatientCompartment_allows() {
 		obs.getNote().add(new Annotation().setText("some text"));
 		obs.setStatus(Observation.ObservationStatus.CORRECTED);
 
-		myObservationDao.update(obs, new SystemRequestDetails());
+		DaoMethodOutcome outcome = myObservationDao.update(obs, new SystemRequestDetails());
+		assertEquals("Patient/A", ((Observation) outcome.getResource()).getSubject().getReference());
 	}
 
 	private void createPatientA() {

diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageResourceDao.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageResourceDao.java
@@ -226,6 +226,30 @@ protected DaoMethodOutcome doUpdateForUpdateOrPatch(
 			IBasePersistedResource theEntity,
 			RestOperationTypeEnum theOperationType,
 			TransactionDetails theTransactionDetails) {
+		return doUpdateForUpdateOrPatch(
+				theRequest,
+				theResourceId,
+				theMatchUrl,
+				thePerformIndexing,
+				theForceUpdateVersion,
+				theResource,
+				theEntity,
+				theOperationType,
+				theTransactionDetails,
+				false);
+	}
+
+	protected DaoMethodOutcome doUpdateForUpdateOrPatch(
+			RequestDetails theRequest,
+			IIdType theResourceId,
+			String theMatchUrl,
+			boolean thePerformIndexing,
+			boolean theForceUpdateVersion,
+			T theResource,
+			IBasePersistedResource theEntity,
+			RestOperationTypeEnum theOperationType,
+			TransactionDetails theTransactionDetails,
+			boolean theShouldForcePopulateOldResourceForProcessing) {
 		if (theResourceId.hasVersionIdPart()
 				&& Long.parseLong(theResourceId.getVersionIdPart()) != theEntity.getVersion()) {
 			throw new ResourceVersionConflictException(
@@ -239,7 +263,7 @@ protected DaoMethodOutcome doUpdateForUpdateOrPatch(
 		}
 
 		IBaseResource oldResource;
-		if (getStorageSettings().isMassIngestionMode()) {
+		if (getStorageSettings().isMassIngestionMode() && !theShouldForcePopulateOldResourceForProcessing) {
 			oldResource = null;
 		} else {
 			oldResource = getStorageResourceParser().toResource(theEntity, false);

diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/util/ResourceCompartmentUtil.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/util/ResourceCompartmentUtil.java
@@ -53,6 +53,11 @@ public class ResourceCompartmentUtil {
 	 */
 	public static Optional<String> getPatientCompartmentIdentity(
 			IBaseResource theResource, FhirContext theFhirContext, ISearchParamExtractor theSearchParamExtractor) {
+		if (theResource == null) {
+			// The resource may be null in mass ingestion mode
+			return Optional.empty();
+		}
+
 		RuntimeResourceDefinition resourceDef = theFhirContext.getResourceDefinition(theResource);
 		List<RuntimeSearchParam> patientCompartmentSps =
 				ResourceCompartmentUtil.getPatientCompartmentSearchParams(resourceDef);

diff --git a/hapi-fhir-storage/src/test/java/ca/uhn/fhir/jpa/util/ResourceCompartmentUtilTest.java b/hapi-fhir-storage/src/test/java/ca/uhn/fhir/jpa/util/ResourceCompartmentUtilTest.java
@@ -128,6 +128,13 @@ void whenNoPatientResource_returnsPatientCompartment() {
 			assertThat(result).contains("P01");
 //			}
 		}
+
+		@Test
+		void nullResource_shouldNotThrowNPE() {
+			// The input resource may be null when mass ingestion is enabled.
+			Optional<String> result = ResourceCompartmentUtil.getPatientCompartmentIdentity(null, myFhirContext, mySearchParamExtractor);
+			assertThat(result).isEmpty();
+		}
 	}
 
 	private List<RuntimeSearchParam> getMockSearchParams(boolean providePatientCompartment) {