3423 include with client assigned id partition (#3448)

* changes to doResolvePersistentIds

* make changes to tests

* Add code changes after the review
This commit is contained in:
samuelwlee2 2022-03-08 09:48:33 -07:00 committed by GitHub
parent ac80c7ffaa
commit cde39dcc6c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 77 additions and 19 deletions

View File

@ -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."

View File

@ -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<Integer> 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<Predicate> getOptionalPartitionPredicate(RequestPartitionId theRequestPartitionId, CriteriaBuilder cb, Root<ForcedId> 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<Integer> 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();

View File

@ -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);
}
}

View File

@ -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);
}
}