diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3423-include_with_client_assigned_ID_partition.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3423-include_with_client_assigned_ID_partition.yaml new file mode 100644 index 00000000000..45e3e692a58 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3423-include_with_client_assigned_ID_partition.yaml @@ -0,0 +1,5 @@ +--- +type: fix +issue: 3423 +jira: SMILE-3664 +title: "Fixed a bug when searching for the resource persistent id in the partitioned environment. This happens when the Cross-Partition Reference Mode is set to ALLOWED_UNQUALIFIED and it is searching for the wrong partition id and resulted in resource persistent id not found." 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 41289a746f8..8e45afec3d5 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 @@ -31,6 +31,7 @@ import ca.uhn.fhir.jpa.model.config.PartitionSettings; import ca.uhn.fhir.jpa.model.cross.IResourceLookup; import ca.uhn.fhir.jpa.model.cross.ResourceLookup; import ca.uhn.fhir.jpa.model.entity.ForcedId; +import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.search.builder.SearchBuilder; import ca.uhn.fhir.jpa.util.MemoryCacheService; import ca.uhn.fhir.jpa.util.QueryChunker; @@ -311,23 +312,7 @@ public class IdHelperService implements IIdHelperService { Predicate idCriteria = cb.equal(from.get("myForcedId").as(String.class), next.getIdPart()); andPredicates.add(idCriteria); - - if (theRequestPartitionId.isDefaultPartition() && myPartitionSettings.getDefaultPartitionId() == null) { - Predicate partitionIdCriteria = cb.isNull(from.get("myPartitionIdValue").as(Integer.class)); - andPredicates.add(partitionIdCriteria); - } else if (!theRequestPartitionId.isAllPartitions()) { - List partitionIds = theRequestPartitionId.getPartitionIds(); - partitionIds = replaceDefaultPartitionIdIfNonNull(myPartitionSettings, partitionIds); - - if (partitionIds.size() > 1) { - Predicate partitionIdCriteria = from.get("myPartitionIdValue").as(Integer.class).in(partitionIds); - andPredicates.add(partitionIdCriteria); - } else { - Predicate partitionIdCriteria = cb.equal(from.get("myPartitionIdValue").as(Integer.class), partitionIds.get(0)); - andPredicates.add(partitionIdCriteria); - } - } - + getOptionalPartitionPredicate(theRequestPartitionId, cb, from).ifPresent(andPredicates::add); predicates.add(cb.and(andPredicates.toArray(EMPTY_PREDICATE_ARRAY))); } @@ -346,9 +331,33 @@ public class IdHelperService implements IIdHelperService { myMemoryCacheService.putAfterCommit(MemoryCacheService.CacheEnum.FORCED_ID_TO_PID, key, persistentId); } } - } + /** + * Return optional predicate for searching on forcedId + * 1. If the partition mode is ALLOWED_UNQUALIFIED, the return optional predicate will be empty, so search is across all partitions. + * 2. If it is default partition and default partition id is null, then return predicate for null partition. + * 3. If the requested partition search is not all partition, return the request partition as predicate. + */ + private Optional getOptionalPartitionPredicate(RequestPartitionId theRequestPartitionId, CriteriaBuilder cb, Root from) { + if (myPartitionSettings.isAllowUnqualifiedCrossPartitionReference()) { + return Optional.empty(); + } else if (theRequestPartitionId.isDefaultPartition() && myPartitionSettings.getDefaultPartitionId() == null) { + Predicate partitionIdCriteria = cb.isNull(from.get("myPartitionIdValue").as(Integer.class)); + return Optional.of(partitionIdCriteria); + } else if (!theRequestPartitionId.isAllPartitions()) { + List partitionIds = theRequestPartitionId.getPartitionIds(); + partitionIds = replaceDefaultPartitionIdIfNonNull(myPartitionSettings, partitionIds); + if (partitionIds.size() > 1) { + Predicate partitionIdCriteria = from.get("myPartitionIdValue").as(Integer.class).in(partitionIds); + return Optional.of(partitionIdCriteria); + } else if (partitionIds.size() == 1){ + Predicate partitionIdCriteria = cb.equal(from.get("myPartitionIdValue").as(Integer.class), partitionIds.get(0)); + return Optional.of(partitionIdCriteria); + } + } + return Optional.empty(); + } private void populateAssociatedResourceId(String nextResourceType, String forcedId, ResourcePersistentId persistentId) { IIdType resourceId = myFhirCtx.getVersion().newIdType(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java index dd1d0a85962..51f6d09a45f 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java @@ -2,6 +2,7 @@ package ca.uhn.fhir.jpa.provider.r4; import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.jpa.entity.PartitionEntity; +import ca.uhn.fhir.jpa.model.config.PartitionSettings; import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.model.util.JpaConstants; import ca.uhn.fhir.jpa.partition.SystemRequestDetails; @@ -13,15 +14,19 @@ import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.CapabilityStatement; +import org.hl7.fhir.r4.model.Condition; import org.hl7.fhir.r4.model.IdType; import org.hl7.fhir.r4.model.Organization; import org.hl7.fhir.r4.model.Patient; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import java.util.Date; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.hasSize; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; @@ -31,6 +36,14 @@ import static org.junit.jupiter.api.Assertions.fail; @SuppressWarnings("Duplicates") public class MultitenantServerR4Test extends BaseMultitenantResourceProviderR4Test implements ITestDataBuilder { + @Override + @AfterEach + public void after() throws Exception { + super.after(); + myPartitionSettings.setAllowReferencesAcrossPartitions(PartitionSettings.CrossPartitionReferenceMode.NOT_ALLOWED); + assertFalse(myPartitionSettings.isAllowUnqualifiedCrossPartitionReference()); + } + @Test public void testFetchCapabilityStatement() { myTenantClientInterceptor.setTenantId(TENANT_A); @@ -118,7 +131,6 @@ public class MultitenantServerR4Test extends BaseMultitenantResourceProviderR4Te } } - @Test public void testCreateAndRead_NonPartitionableResource_DefaultTenant() { @@ -341,4 +353,28 @@ public class MultitenantServerR4Test extends BaseMultitenantResourceProviderR4Te } } + + @Test + public void testIncludeInTenantWithAssignedID() throws Exception { + IIdType idA = createResource("Patient", withTenant(JpaConstants.DEFAULT_PARTITION_NAME), withId("test"), withFamily("Smith"), withActiveTrue()); + createConditionWithAllowedUnqualified(idA); + Bundle response = myClient.search().byUrl(myClient.getServerBase() + "/" + TENANT_A + "/Condition?subject=Patient/" + idA.getIdPart() + "&_include=Condition:subject").returnBundle(Bundle.class).execute(); + assertThat(response.getEntry(), hasSize(2)); + } + + @Test + public void testIncludeInTenantWithAutoGeneratedID() throws Exception { + IIdType idA = createResource("Patient", withTenant(JpaConstants.DEFAULT_PARTITION_NAME), withFamily("Smith"), withActiveTrue()); + createConditionWithAllowedUnqualified(idA); + Bundle response = myClient.search().byUrl(myClient.getServerBase() + "/" + TENANT_A + "/Condition?subject=Patient/" + idA.getIdPart() + "&_include=Condition:subject").returnBundle(Bundle.class).execute(); + assertThat(response.getEntry(), hasSize(2)); + } + + private void createConditionWithAllowedUnqualified(IIdType idA) { + myPartitionSettings.setAllowReferencesAcrossPartitions(PartitionSettings.CrossPartitionReferenceMode.ALLOWED_UNQUALIFIED); + IIdType idB = createResource("Condition", withTenant(TENANT_A), withObservationCode("http://cs", "A")); + Condition theCondition = myClient.read().resource(Condition.class).withId(idB).execute(); + theCondition.getSubject().setReference("Patient/" + idA.getIdPart()); + doUpdateResource(theCondition); + } } diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/config/PartitionSettings.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/config/PartitionSettings.java index b6275b480b7..31811f0aee6 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/config/PartitionSettings.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/config/PartitionSettings.java @@ -151,4 +151,12 @@ public class PartitionSettings { } + /** + * If enabled the JPA server will allow unqualified cross partition reference + * + */ + public boolean isAllowUnqualifiedCrossPartitionReference() { + return myAllowReferencesAcrossPartitions.equals(PartitionSettings.CrossPartitionReferenceMode.ALLOWED_UNQUALIFIED); + } + }