Avoid creating ResourcePersistentId for placeholder resources with nu…

…ll ID (#3158) * Add check before mapping storage ID to resource ID in TransactionDetails. * Add change log. * Changed to instead prevent creation of ResourcePersistentId with null ID value. * Changed to instead prevent ResourcePersistentId being created with null resource ID. Co-authored-by: ianmarshall <[email protected]>
hapifhir · Nov 30, 2021 · 323f3d7 · 323f3d7
1 parent ba8b809
commit 323f3d7
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 6 deletions.
diff --git a/...n/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3158-creation-of-versioned-placeholders.yaml b/...n/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3158-creation-of-versioned-placeholders.yaml
@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 3158
+title: "Resource links were previously not being consistently created in cases where references were versioned and
+  pointing to recently auto-created placeholder resources."
diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java
@@ -316,12 +316,15 @@ public List<ResourcePersistentId> resolveResourcePersistentIdsWithCache(RequestP
 			TypedQuery<ForcedId> query = myEntityManager.createQuery(criteriaQuery);
 			List<ForcedId> results = query.getResultList();
 			for (ForcedId nextId : results) {
-				ResourcePersistentId persistentId = new ResourcePersistentId(nextId.getResourceId());
-				populateAssociatedResourceId(nextId.getResourceType(), nextId.getForcedId(), persistentId);
-				retVal.add(persistentId);
-
-				String key = toForcedIdToPidKey(theRequestPartitionId, nextId.getResourceType(), nextId.getForcedId());
-				myMemoryCacheService.putAfterCommit(MemoryCacheService.CacheEnum.FORCED_ID_TO_PID, key, persistentId);
+				// Check if the nextId has a resource ID. It may have a null resource ID if a commit is still pending.
+				if (nextId.getResourceId() != null) {
+					ResourcePersistentId persistentId = new ResourcePersistentId(nextId.getResourceId());
+					populateAssociatedResourceId(nextId.getResourceType(), nextId.getForcedId(), persistentId);
+					retVal.add(persistentId);
+
+					String key = toForcedIdToPidKey(theRequestPartitionId, nextId.getResourceType(), nextId.getForcedId());
+					myMemoryCacheService.putAfterCommit(MemoryCacheService.CacheEnum.FORCED_ID_TO_PID, key, persistentId);
+				}
 			}
 
 		}

diff --git a/...er-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java b/...er-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java
@@ -13,6 +13,7 @@
 import ca.uhn.fhir.util.BundleBuilder;
 import ca.uhn.fhir.util.HapiExtensions;
 import com.google.common.collect.Sets;
+import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.instance.model.api.IIdType;
 import org.hl7.fhir.r4.model.AuditEvent;
 import org.hl7.fhir.r4.model.BooleanType;
@@ -29,6 +30,7 @@
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
+import java.util.HashSet;
 import java.util.List;
 
 import static org.hamcrest.CoreMatchers.equalTo;
@@ -650,4 +652,52 @@ public void testAutocreatePlaceholderWithExistingTargetWithServerAssignedIdTest(
 		assertNotNull(retObservation);
 	}
 
+	@Test
+	public void testMultipleVersionedReferencesToAutocreatedPlaceholder() {
+		myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true);
+		HashSet<String> refPaths = new HashSet<>();
+		refPaths.add("Observation.subject");
+		myModelConfig.setAutoVersionReferenceAtPaths(refPaths);
+
+
+		Observation obs1 = new Observation();
+		obs1.setId("Observation/DEF1");
+		Reference patientRef = new Reference("Patient/RED");
+		obs1.setSubject(patientRef);
+		BundleBuilder builder = new BundleBuilder(myFhirCtx);
+		Observation obs2 = new Observation();
+		obs2.setId("Observation/DEF2");
+		obs2.setSubject(patientRef);
+		builder.addTransactionUpdateEntry(obs1);
+		builder.addTransactionUpdateEntry(obs2);
+
+		mySystemDao.transaction(new SystemRequestDetails(), (Bundle) builder.getBundle());
+
+		// verify links created to Patient placeholder from both Observations
+		IBundleProvider outcome = myPatientDao.search(SearchParameterMap.newSynchronous().addRevInclude(IBaseResource.INCLUDE_ALL));
+		assertEquals(3, outcome.getAllResources().size());
+	}
+
+	@Test
+	public void testMultipleReferencesToAutocreatedPlaceholder() {
+		myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true);
+
+		Observation obs1 = new Observation();
+		obs1.setId("Observation/DEF1");
+		Reference patientRef = new Reference("Patient/RED");
+		obs1.setSubject(patientRef);
+		BundleBuilder builder = new BundleBuilder(myFhirCtx);
+		Observation obs2 = new Observation();
+		obs2.setId("Observation/DEF2");
+		obs2.setSubject(patientRef);
+		builder.addTransactionUpdateEntry(obs1);
+		builder.addTransactionUpdateEntry(obs2);
+
+		mySystemDao.transaction(new SystemRequestDetails(), (Bundle) builder.getBundle());
+
+		// verify links created to Patient placeholder from both Observations
+		IBundleProvider outcome = myPatientDao.search(SearchParameterMap.newSynchronous().addRevInclude(IBaseResource.INCLUDE_ALL));
+		assertEquals(3, outcome.getAllResources().size());
+	}
+
 }