Final 8.0.0 Mergeback (hapifhir#6756)

tadgh · epeartree · peartree · web-flow · commit 250dae3c736e · 2025-02-24T08:18:39.000-08:00
* 6711 everything operation with partitioning enabled returns duplicates since the dbpm merge (hapifhir#6712) * Add test stub * Changelog * Wording * Trying to get test working * Initial commit of tests and potential hash fix * Remove json file * Remove dead code * Remove dead test, spotless, fix changelog * Limitation * Version bump to 8.0 * With partitioning selection mode set to request tenant, creating a cross partition subscription will fail if the default partition id is not null (hapifhir#6748) * initial failing test * fix with changelog and modification to previous changelog --------- Co-authored-by: peartree <etienne.poirier@smilecdr.com> * Updating version to: 8.0.1 post release. * Spotless --------- Co-authored-by: Etienne Poirier <33007955+epeartree@users.noreply.github.com> Co-authored-by: peartree <etienne.poirier@smilecdr.com> Co-authored-by: markiantorno <markiantorno@gmail.com> Co-authored-by: tadgh <garygraham@smiledigitalhealth.com>
diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/VersionEnum.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/VersionEnum.java
@@ -174,6 +174,7 @@ public enum VersionEnum {
 	V7_7_0,
 	V7_8_0,
 	V8_0_0,
+	V8_0_1,
 	V8_1_0,
 	V8_2_0;
 
diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/8_0_0/6686-failure-creating-cross-partition-subscription.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/8_0_0/6686-failure-creating-cross-partition-subscription.yaml
@@ -1,6 +1,7 @@
 ---
 type: fix
 issue: 6686
-title: "Previously, attempting to create a cross-partition subscription would fail if the default partition ID was
-assigned a value different than the default value(null). This issue is fixed."
+title: "With partitioning selection mode set to 'PATIENT_ID', attempting to create a cross-partition subscription 
+would fail if the default partition ID was assigned a value different than the default value(null). This issue is fixed."
+
 
diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/8_0_0/6711-everything-duplicate-regression.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/8_0_0/6711-everything-duplicate-regression.yaml
@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 6711
+jira: SMILE-9733
+title: "Introduction of Database Partitioning Mode temporarily introduced a regression in the Patient `$everything` operation on partitioned servers, in which duplicates would be returned in the bundle. This has been corrected."
diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/8_0_0/6747-request-tenant-failure-creating-cross-partition-subscription.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/8_0_0/6747-request-tenant-failure-creating-cross-partition-subscription.yaml
@@ -0,0 +1,6 @@
+---
+type: fix
+issue: 6747
+title: "With partitioning selection mode set to 'REQUEST_TENANT', attempting to create a cross-partition subscription 
+would fail if the default partition ID was assigned a value different than the default value(null). This issue is fixed."
+
diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa_partitioning/db_partition_mode.md b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa_partitioning/db_partition_mode.md
@@ -37,3 +37,9 @@ It is not yet possible to migrate an existing database schema between these two
 
 The CLI [Migrator](../server_jpa/upgrading.md) can be used to upgrade the database schema when upgrading HAPI FHIR. It must be configured to use the Database Partition Mode schema by using the `--flags` argument as [described here](../server_jpa/upgrading.md#database-partition-mode).
 
+# Limitations 
+
+As Database Partitioning Mode is being built from the ground up, there are currently some known limitations. 
+
+* MDM does not currently work with Database Partitioning Mode. This will be corrected in a future release
+
diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/partition/RequestPartitionHelperSvc.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/partition/RequestPartitionHelperSvc.java
@@ -144,7 +144,7 @@ public RequestPartitionId validateAndNormalizePartitionNames(RequestPartitionId
 				if (partition != null) {
 					ids.add(partition.getId());
 				} else {
-					ids.add(null);
+					ids.add(myPartitionSettings.getDefaultPartitionId());
 				}
 			}
 		}
diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java
@@ -1611,6 +1611,8 @@ public Set<JpaPid> loadIncludes(SearchBuilderLoadIncludesParameters<JpaPid> theP
 			for (JpaPid next : pidsToInclude) {
 				if (!original.contains(next) && !allAdded.contains(next)) {
 					nextRoundMatches.add(next);
+				} else {
+					ourLog.trace("Skipping include since it has already been seen. [jpaPid={}]", next);
 				}
 			}
 
diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/dao/JpaPid.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/dao/JpaPid.java
@@ -142,7 +142,7 @@ public boolean equals(Object theO) {
 			return false;
 		}
 		JpaPid jpaPid = (JpaPid) theO;
-		return Objects.equals(myId, jpaPid.myId) && Objects.equals(myPartitionIdValue, jpaPid.myPartitionIdValue);
+		return Objects.equals(myId, jpaPid.myId);
 	}
 
 	/**
@@ -151,7 +151,7 @@ public boolean equals(Object theO) {
 	 */
 	@Override
 	public int hashCode() {
-		return Objects.hash(myId, myPartitionIdValue);
+		return Objects.hash(myId);
 	}
 
 	@Override
diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSearchCacheR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSearchCacheR4Test.java
@@ -1,16 +1,26 @@
 package ca.uhn.fhir.jpa.dao.r4;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import ca.uhn.fhir.jpa.api.dao.PatientEverythingParameters;
+import ca.uhn.fhir.jpa.binary.provider.BinaryAccessProvider;
+import ca.uhn.fhir.jpa.rp.r4.PatientResourceProvider;
 import ca.uhn.fhir.jpa.search.PersistedJpaBundleProvider;
 import ca.uhn.fhir.jpa.search.cache.SearchCacheStatusEnum;
 import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
 import ca.uhn.fhir.jpa.util.SqlQuery;
+import ca.uhn.fhir.rest.api.server.IBundleProvider;
+import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
 import org.apache.commons.lang3.StringUtils;
+import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.instance.model.api.IIdType;
+import org.hl7.fhir.r4.model.IntegerType;
 import org.junit.jupiter.api.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
 
+import java.util.ArrayList;
 import java.util.List;
 import java.util.stream.Collectors;
 
diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/partition/RequestPartitionHelperSvcTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/partition/RequestPartitionHelperSvcTest.java
@@ -21,6 +21,7 @@
 import java.util.Set;
 import java.util.stream.Stream;
 
+import static ca.uhn.fhir.jpa.model.util.JpaConstants.DEFAULT_PARTITION_NAME;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -190,7 +191,7 @@ public void testValidateAndNormalizePartitionNames_withNameAndInvalidId_throwsEx
 	}
 
 	@ParameterizedTest
-	@MethodSource
+	@MethodSource("withPartitionIds")
 	public void testDefaultPartition_whenDefaultPartitionIsNotNull(Integer theRequestPartitionId) {
 		final Integer defaultPartitionId = 0;
 		myPartitionSettings.setDefaultPartitionId(defaultPartitionId);
@@ -206,7 +207,20 @@ public void testDefaultPartition_whenDefaultPartitionIsNotNull(Integer theReques
 		}
 	}
 
-	private static Stream<Integer> testDefaultPartition_whenDefaultPartitionIsNotNull(){
+	@ParameterizedTest
+	@MethodSource("withPartitionIds")
+	public void testValidateAndNormalizePartitionNames_willResolveDefaultPartitionNameToCorrectPartitionId(Integer theDefaultPartitionId) {
+		myPartitionSettings.setDefaultPartitionId(theDefaultPartitionId);
+
+		RequestPartitionId requestPartitionId = RequestPartitionId.fromPartitionName(DEFAULT_PARTITION_NAME);
+
+		RequestPartitionId normalizedRequestPartitionId = mySvc.validateAndNormalizePartitionNames(requestPartitionId);
+
+		assertThat(normalizedRequestPartitionId.isDefaultPartition(theDefaultPartitionId)).isTrue();
+
+	}
+
+	private static Stream<Integer> withPartitionIds(){
 		return Stream.of(null, 1,2,3);
 	}
 
diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java
@@ -12,6 +12,7 @@
 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.model.api.Include;
 import ca.uhn.fhir.rest.api.Constants;
 import ca.uhn.fhir.rest.api.server.RequestDetails;
 import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
@@ -36,9 +37,12 @@
 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.IntegerType;
 import org.hl7.fhir.r4.model.Organization;
+import org.hl7.fhir.r4.model.Parameters;
 import org.hl7.fhir.r4.model.Patient;
 import org.hl7.fhir.r4.model.Resource;
+import org.hl7.fhir.r4.model.StringType;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Disabled;
@@ -52,6 +56,7 @@
 
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Date;
@@ -83,6 +88,62 @@ public void after() throws Exception {
 		assertFalse(myPartitionSettings.isAllowUnqualifiedCrossPartitionReference());
 	}
 
+	@ParameterizedTest
+	@ValueSource(ints = {1, 50})
+	public void testPartitioningDoesNotReturnDuplicatesOnDoubleInclude(int theCount) {
+		myTenantClientInterceptor.setTenantId(TENANT_A);
+		IIdType patient = createPatient(withTenant(TENANT_A), withActiveTrue());
+		IIdType encounter = createEncounter(withTenant(TENANT_A), withSubject(patient.toUnqualifiedVersionless().toString()), withId("enc-"),  withIdentifier("http://example.com/", "code"));
+		createObservation(withTenant(TENANT_A), withSubject(patient.toUnqualifiedVersionless().toString()), withId("obs-"), withObservationCode("http://example.com/", "code"), withEncounter(encounter.toUnqualifiedVersionless().toString()));
+		Parameters parameters = new Parameters();
+		parameters.addParameter().setName("_count").setValue(new IntegerType(theCount));
+		parameters.addParameter().setName("_format").setValue(new StringType("json"));
+		Bundle execute = myClient.search().forResource("Patient").include(new Include("*")).revInclude(new Include("*")).count(theCount).returnBundle(Bundle.class).execute();
+
+
+		List<String> foundIds = new ArrayList<>();
+		foundIds.addAll(execute.getEntry().stream().map(entry -> entry.getResource().getIdElement().toUnqualifiedVersionless().toString()).toList());
+		//Ensure no duplicates in first page
+		assertThat(foundIds).doesNotHaveDuplicates();
+
+		while (execute.getLink("next") != null) {
+			execute = myClient.loadPage().next(execute).execute();
+			foundIds.addAll(execute.getEntry().stream().map(entry -> entry.getResource().getIdElement().toUnqualifiedVersionless().toString()).toList());
+		}
+		assertThat(foundIds).doesNotHaveDuplicates();
+		assertThat(foundIds).hasSize(3);
+	}
+
+	@ParameterizedTest
+	@ValueSource(ints = {1, 50})
+	public void testPartitioningDoesNotReturnDuplicatesOnPatientEverything(int theCount) throws IOException {
+		myTenantClientInterceptor.setTenantId(TENANT_A);
+		IIdType patient = createPatient(withTenant(TENANT_A), withActiveTrue());
+		IIdType encounter = createEncounter(withTenant(TENANT_A), withSubject(patient.toUnqualifiedVersionless().toString()), withId("enc-"),  withIdentifier("http://example.com/", "code"));
+		createObservation(withTenant(TENANT_A), withSubject(patient.toUnqualifiedVersionless().toString()), withId("obs-"), withObservationCode("http://example.com/", "code"), withEncounter(encounter.toUnqualifiedVersionless().toString()));
+
+		SystemRequestDetails systemRequestDetails = new SystemRequestDetails();
+		systemRequestDetails.setTenantId(TENANT_A);
+
+		Parameters parameters = new Parameters();
+		parameters.addParameter().setName("_count").setValue(new IntegerType(theCount));
+		parameters.addParameter().setName("_format").setValue(new StringType("json"));
+
+		Bundle everything = myClient.operation().onInstance(patient.toUnqualifiedVersionless().toString()).named("$everything").withParameters(parameters).returnResourceType(Bundle.class).execute();
+
+		List<String> foundIds = new ArrayList<>();
+		foundIds.addAll(everything.getEntry().stream().map(entry -> entry.getResource().getIdElement().toUnqualifiedVersionless().toString()).toList());
+		//Ensure no duplicates in first page
+		assertThat(foundIds).doesNotHaveDuplicates();
+
+		while (everything.getLink("next") != null) {
+			everything = myClient.loadPage().next(everything).execute();
+			foundIds.addAll(everything.getEntry().stream().map(entry -> entry.getResource().getIdElement().toUnqualifiedVersionless().toString()).toList());
+		}
+		assertThat(foundIds).doesNotHaveDuplicates();
+		assertThat(foundIds).hasSize(3);
+	}
+
 	@Test
 	public void testFetchCapabilityStatement() {
 		myTenantClientInterceptor.setTenantId(TENANT_A);
diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/partition/BaseRequestPartitionHelperSvc.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/partition/BaseRequestPartitionHelperSvc.java
@@ -378,7 +378,7 @@ private RequestPartitionId validateAndNormalizePartition(
 		// Replace null partition ID with non-null default partition ID if one is being used
 		if (myPartitionSettings.getDefaultPartitionId() != null
 				&& retVal.hasPartitionIds()
-				&& hasDefaultPartitionId(retVal)) {
+				&& retVal.hasDefaultPartitionId(null)) {
 			List<Integer> partitionIds = new ArrayList<>(retVal.getPartitionIds());
 			for (int i = 0; i < partitionIds.size(); i++) {
 				if (partitionIds.get(i) == null) {
diff --git a/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/ITestDataBuilder.java b/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/ITestDataBuilder.java
@@ -323,7 +323,11 @@ default ICreationArgument withGroupMember(@Nullable String theMember) {
 	}
 
 	default ICreationArgument withEncounter(@Nullable String theEncounter) {
-		return withReference("encounter", new IdType(theEncounter));
+		return withEncounter(new IdType(theEncounter));
+	}
+
+	default ICreationArgument withEncounter(@Nullable IIdType theEncounter) {
+		return withReference("encounter", theEncounter);
 	}
 
 	@Nonnull

-Original file line number
+Diff line change
 	V7_7_0,
 	V7_8_0,
 	V8_0_0,
 +	V8_0_1,
 	V8_1_0,
 	V8_2_0;
Original file line number	Diff line number	Diff line change
`@@ -144,7 +144,7 @@ public RequestPartitionId validateAndNormalizePartitionNames(RequestPartitionId`
`144`	`144`	`if (partition != null) {`
`145`	`145`	`ids.add(partition.getId());`
`146`	`146`	`} else {`
`147`		`- ids.add(null);`
	`147`	`+ ids.add(myPartitionSettings.getDefaultPartitionId());`
`148`	`148`	`}`
`149`	`149`	`}`
`150`	`150`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1611,6 +1611,8 @@ public Set<JpaPid> loadIncludes(SearchBuilderLoadIncludesParameters<JpaPid> theP`
`1611`	`1611`	`for (JpaPid next : pidsToInclude) {`
`1612`	`1612`	`if (!original.contains(next) && !allAdded.contains(next)) {`
`1613`	`1613`	`nextRoundMatches.add(next);`
	`1614`	`+ } else {`
	`1615`	`+ ourLog.trace("Skipping include since it has already been seen. [jpaPid={}]", next);`
`1614`	`1616`	`}`
`1615`	`1617`	`}`
`1616`	`1618`
Original file line number	Diff line number	Diff line change
`@@ -142,7 +142,7 @@ public boolean equals(Object theO) {`
`142`	`142`	`return false;`
`143`	`143`	`}`
`144`	`144`	`JpaPid jpaPid = (JpaPid) theO;`
`145`		`- return Objects.equals(myId, jpaPid.myId) && Objects.equals(myPartitionIdValue, jpaPid.myPartitionIdValue);`
	`145`	`+ return Objects.equals(myId, jpaPid.myId);`
`146`	`146`	`}`
`147`	`147`
`148`	`148`	`/**`
`@@ -151,7 +151,7 @@ public boolean equals(Object theO) {`
`151`	`151`	`*/`
`152`	`152`	`@Override`
`153`	`153`	`public int hashCode() {`
`154`		`- return Objects.hash(myId, myPartitionIdValue);`
	`154`	`+ return Objects.hash(myId);`
`155`	`155`	`}`
`156`	`156`
`157`	`157`	`@Override`