Enable Package Loader when partitioning is enabled with unnamed partitions (#2808)

* Enable Package Loader when partitioning is enabled with unnamed partitions.

* Enable Package Loader when partitioning is enabled with unnamed partitions.

* Additional package loading problems found with partitioning.

* Additional package loading problems found with partitioning.

Co-authored-by: ianmarshall <ian@simpatico.ai>

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2808-allow-package-loading-with-unnamed-partitions.yaml

@@ -0,0 +1,4 @@
+---
+type: fix
+issue: 2808
+title: "Loading packages would fail when partitioning was enabled with unnamed partitions. This has been fixed."

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/JpaPackageCache.java

@@ -23,6 +23,7 @@ package ca.uhn.fhir.jpa.packages;
 import ca.uhn.fhir.context.BaseRuntimeChildDefinition;
 import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.context.FhirVersionEnum;
+import ca.uhn.fhir.interceptor.model.RequestPartitionId;
 import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
 import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao;
 import ca.uhn.fhir.jpa.api.model.ExpungeOptions;
@@ -331,7 +332,11 @@ public class JpaPackageCache extends BasePackageCacheManager implements IHapiPac
 
 		if (myPartitionSettings.isPartitioningEnabled()) {
 			SystemRequestDetails requestDetails = new SystemRequestDetails();
+			if (myPartitionSettings.isUnnamedPartitionMode() && myPartitionSettings.getDefaultPartitionId() != null) {
+				requestDetails.setRequestPartitionId(RequestPartitionId.fromPartitionId(myPartitionSettings.getDefaultPartitionId()));
+			} else {
 				requestDetails.setTenantId(JpaConstants.DEFAULT_PARTITION_NAME);
+			}
 			return (ResourceTable) getBinaryDao().create(theResourceBinary, requestDetails).getEntity();
  		} else {
 			return (ResourceTable) getBinaryDao().create(theResourceBinary).getEntity();

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImpl.java

@@ -359,7 +359,6 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 	private IBundleProvider searchResource(IFhirResourceDao theDao, SearchParameterMap theMap) {
 		if (myPartitionSettings.isPartitioningEnabled()) {
 			SystemRequestDetails requestDetails = new SystemRequestDetails();
-//			requestDetails.setTenantId(JpaConstants.DEFAULT_PARTITION_NAME);
 			return theDao.search(theMap, requestDetails);
 		} else {
 			return theDao.search(theMap);
@@ -369,7 +368,6 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 	private void createResource(IFhirResourceDao theDao, IBaseResource theResource) {
 		if (myPartitionSettings.isPartitioningEnabled()) {
 			SystemRequestDetails requestDetails = new SystemRequestDetails();
-			requestDetails.setTenantId(JpaConstants.DEFAULT_PARTITION_NAME);
 			theDao.create(theResource, requestDetails);
 		} else {
 			theDao.create(theResource);
@@ -379,7 +377,6 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 	private DaoMethodOutcome updateResource(IFhirResourceDao theDao, IBaseResource theResource) {
 		if (myPartitionSettings.isPartitioningEnabled()) {
 			SystemRequestDetails requestDetails = new SystemRequestDetails();
-			requestDetails.setTenantId(JpaConstants.DEFAULT_PARTITION_NAME);
 			return theDao.update(theResource, requestDetails);
 		} else {
 			return theDao.update(theResource);

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/JpaPackageCacheTest.java

@@ -45,6 +45,7 @@ public class JpaPackageCacheTest extends BaseJpaR4Test {
 	public void disablePartitioning() {
 		myPartitionSettings.setPartitioningEnabled(false);
 		myPartitionSettings.setDefaultPartitionId(new PartitionSettings().getDefaultPartitionId());
+		myPartitionSettings.setUnnamedPartitionMode(false);
 		myInterceptorService.unregisterInterceptor(myRequestTenantPartitionInterceptor);
 	}
 
@@ -103,6 +104,38 @@ public class JpaPackageCacheTest extends BaseJpaR4Test {
 		assertEquals("Deleting package basisprofil.de#0.2.40", deleteOutcomeMsgs.get(0));
 	}
 
+	@Test
+	public void testSaveAndDeletePackageUnnamedPartitionsEnabled() throws IOException {
+		myPartitionSettings.setPartitioningEnabled(true);
+		myPartitionSettings.setDefaultPartitionId(0);
+		myPartitionSettings.setUnnamedPartitionMode(true);
+		myInterceptorService.registerInterceptor(new PatientIdPartitionInterceptor());
+		myInterceptorService.registerInterceptor(myRequestTenantPartitionInterceptor);
+
+		try (InputStream stream = ClasspathUtil.loadResourceAsStream("/packages/hl7.fhir.uv.shorthand-0.12.0.tgz")) {
+			myPackageCacheManager.addPackageToCache("hl7.fhir.uv.shorthand", "0.12.0", stream, "hl7.fhir.uv.shorthand");
+		}
+
+		NpmPackage pkg;
+
+		pkg = myPackageCacheManager.loadPackage("hl7.fhir.uv.shorthand", null);
+		assertEquals("0.12.0", pkg.version());
+
+		pkg = myPackageCacheManager.loadPackage("hl7.fhir.uv.shorthand", "0.12.0");
+		assertEquals("0.12.0", pkg.version());
+
+		try {
+			myPackageCacheManager.loadPackage("hl7.fhir.uv.shorthand", "99");
+			fail();
+		} catch (ResourceNotFoundException e) {
+			assertEquals("Unable to locate package hl7.fhir.uv.shorthand#99", e.getMessage());
+		}
+
+		PackageDeleteOutcomeJson deleteOutcomeJson = myPackageCacheManager.uninstallPackage("hl7.fhir.uv.shorthand", "0.12.0");
+		List<String> deleteOutcomeMsgs = deleteOutcomeJson.getMessage();
+		assertEquals("Deleting package hl7.fhir.uv.shorthand#0.12.0", deleteOutcomeMsgs.get(0));
+	}
+
 	@Test
 	public void testSavePackageWithLongDescription() throws IOException {
 		try (InputStream stream = ClasspathUtil.loadResourceAsStream("/packages/package-davinci-cdex-0.2.0.tgz")) {

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/NpmR4Test.java

@@ -10,6 +10,7 @@ import ca.uhn.fhir.jpa.dao.data.INpmPackageDao;
 import ca.uhn.fhir.jpa.dao.data.INpmPackageVersionDao;
 import ca.uhn.fhir.jpa.dao.data.INpmPackageVersionResourceDao;
 import ca.uhn.fhir.jpa.dao.r4.BaseJpaR4Test;
+import ca.uhn.fhir.jpa.model.config.PartitionSettings;
 import ca.uhn.fhir.jpa.model.entity.NpmPackageEntity;
 import ca.uhn.fhir.jpa.model.entity.NpmPackageVersionEntity;
 import ca.uhn.fhir.jpa.model.entity.NpmPackageVersionResourceEntity;
@@ -127,6 +128,8 @@ public class NpmR4Test extends BaseJpaR4Test {
 		myDaoConfig.setAllowExternalReferences(new DaoConfig().isAllowExternalReferences());
 		myDaoConfig.setAutoCreatePlaceholderReferenceTargets(new DaoConfig().isAutoCreatePlaceholderReferenceTargets());
 		myPartitionSettings.setPartitioningEnabled(false);
+		myPartitionSettings.setUnnamedPartitionMode(false);
+		myPartitionSettings.setDefaultPartitionId(new PartitionSettings().getDefaultPartitionId());
 		myInterceptorService.unregisterInterceptor(myRequestTenantPartitionInterceptor);
 	}
 
@@ -451,6 +454,31 @@ public class NpmR4Test extends BaseJpaR4Test {
 
 	}
 
+	@Test
+	public void testInstallR4Package_Twice_partitioningEnabled() throws Exception {
+		myDaoConfig.setAllowExternalReferences(true);
+		myPartitionSettings.setPartitioningEnabled(true);
+		myInterceptorService.registerInterceptor(myRequestTenantPartitionInterceptor);
+
+		byte[] bytes = loadClasspathBytes("/packages/hl7.fhir.uv.shorthand-0.12.0.tgz");
+		myFakeNpmServlet.myResponses.put("/hl7.fhir.uv.shorthand/0.12.0", bytes);
+
+		PackageInstallOutcomeJson outcome;
+
+		PackageInstallationSpec spec = new PackageInstallationSpec().setName("hl7.fhir.uv.shorthand").setVersion("0.12.0").setInstallMode(PackageInstallationSpec.InstallModeEnum.STORE_AND_INSTALL);
+		outcome = myPackageInstallerSvc.install(spec);
+		assertEquals(1, outcome.getResourcesInstalled().get("CodeSystem"));
+
+		myPackageInstallerSvc.install(spec);
+		outcome = myPackageInstallerSvc.install(spec);
+		assertEquals(null, outcome.getResourcesInstalled().get("CodeSystem"));
+
+		// Ensure that we loaded the contents
+		IBundleProvider searchResult = myCodeSystemDao.search(SearchParameterMap.newSynchronous("url", new UriParam("http://hl7.org/fhir/uv/shorthand/CodeSystem/shorthand-code-system")));
+		assertEquals(1, searchResult.sizeOrThrowNpe());
+
+	}
+
 
 	@Test
 	public void testInstallR4PackageWithNoDescription() throws Exception {
@@ -801,6 +829,43 @@ public class NpmR4Test extends BaseJpaR4Test {
 		});
 	}
 
+	@Test
+	public void testInstallPkgContainingNonPartitionedResourcesPartitionsEnabled() throws Exception {
+		myDaoConfig.setAllowExternalReferences(true);
+		myPartitionSettings.setPartitioningEnabled(true);
+		myInterceptorService.registerInterceptor(myRequestTenantPartitionInterceptor);
+
+		byte[] bytes = loadClasspathBytes("/packages/test-logical-structuredefinition.tgz");
+		myFakeNpmServlet.myResponses.put("/test-logical-structuredefinition/1.0.0", bytes);
+
+		PackageInstallationSpec spec = new PackageInstallationSpec().setName("test-logical-structuredefinition").setVersion("1.0.0").setInstallMode(PackageInstallationSpec.InstallModeEnum.STORE_AND_INSTALL);
+		PackageInstallOutcomeJson outcome = myPackageInstallerSvc.install(spec);
+		assertEquals(2, outcome.getResourcesInstalled().get("StructureDefinition"));
+
+		// Be sure no further communication with the server
+		JettyUtil.closeServer(myServer);
+
+		// Search for the installed resource
+		runInTransaction(() -> {
+			// Confirm that Laborbefund (a logical StructureDefinition) was created without a snapshot.
+			SearchParameterMap map = SearchParameterMap.newSynchronous();
+			map.add(StructureDefinition.SP_URL, new UriParam("https://www.medizininformatik-initiative.de/fhir/core/modul-labor/StructureDefinition/LogicalModel/Laborbefund"));
+			IBundleProvider result = myStructureDefinitionDao.search(map);
+			assertEquals(1, result.sizeOrThrowNpe());
+			List<IBaseResource> resources = result.getResources(0,1);
+			assertFalse(((StructureDefinition)resources.get(0)).hasSnapshot());
+
+			// Confirm that DiagnosticLab (a resource StructureDefinition with differential but no snapshot) was created with a generated snapshot.
+			map = SearchParameterMap.newSynchronous();
+			map.add(StructureDefinition.SP_URL, new UriParam("https://www.medizininformatik-initiative.de/fhir/core/modul-labor/StructureDefinition/DiagnosticReportLab"));
+			result = myStructureDefinitionDao.search(map);
+			assertEquals(1, result.sizeOrThrowNpe());
+			resources = result.getResources(0,1);
+			assertTrue(((StructureDefinition)resources.get(0)).hasSnapshot());
+
+		});
+	}
+
 	static class FakeNpmServlet extends HttpServlet {
 
 		private final Map<String, byte[]> myResponses = new HashMap<>();