From 843517f7ba30656f2a30dae7ee50375768780995 Mon Sep 17 00:00:00 2001 From: IanMMarshall <49525404+IanMMarshall@users.noreply.github.com> Date: Mon, 15 Nov 2021 16:36:30 -0500 Subject: [PATCH] Avoid creating ResourcePersistentId for placeholder resources with null 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 --- ...58-creation-of-versioned-placeholders.yaml | 5 ++ .../fhir/jpa/dao/index/IdHelperService.java | 13 +++-- ...irResourceDaoCreatePlaceholdersR4Test.java | 50 +++++++++++++++++++ 3 files changed, 63 insertions(+), 5 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3158-creation-of-versioned-placeholders.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3158-creation-of-versioned-placeholders.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3158-creation-of-versioned-placeholders.yaml new file mode 100644 index 00000000000..20d2fd864af --- /dev/null +++ b/hapi-fhir-docs/src/main/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 index 2efcbc28aca..aedce78f880 100644 --- 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 class IdHelperService { TypedQuery query = myEntityManager.createQuery(criteriaQuery); List results = query.getResultList(); for (ForcedId nextId : results) { - ResourcePersistentId persistentId = new ResourcePersistentId(nextId.getResourceId()); - populateAssociatedResourceId(nextId.getResourceType(), nextId.getForcedId(), persistentId); - retVal.add(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); + String key = toForcedIdToPidKey(theRequestPartitionId, nextId.getResourceType(), nextId.getForcedId()); + myMemoryCacheService.putAfterCommit(MemoryCacheService.CacheEnum.FORCED_ID_TO_PID, key, persistentId); + } } } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java index 1588c81ba80..8a9ec107422 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java @@ -13,6 +13,7 @@ import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; 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.AfterEach; 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 class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { assertNotNull(retObservation); } + @Test + public void testMultipleVersionedReferencesToAutocreatedPlaceholder() { + myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); + HashSet 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()); + } + }