From 8b8db82666d68f2cd677cfd38253a11094dd6c70 Mon Sep 17 00:00:00 2001 From: IanMMarshall <49525404+IanMMarshall@users.noreply.github.com> Date: Thu, 22 Jul 2021 05:17:17 -0400 Subject: [PATCH] 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 --- ...ckage-loading-with-unnamed-partitions.yaml | 4 ++ .../fhir/jpa/packages/JpaPackageCache.java | 7 +- .../jpa/packages/PackageInstallerSvcImpl.java | 3 - .../jpa/packages/JpaPackageCacheTest.java | 33 ++++++++++ .../ca/uhn/fhir/jpa/packages/NpmR4Test.java | 65 +++++++++++++++++++ 5 files changed, 108 insertions(+), 4 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2808-allow-package-loading-with-unnamed-partitions.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2808-allow-package-loading-with-unnamed-partitions.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2808-allow-package-loading-with-unnamed-partitions.yaml new file mode 100644 index 00000000000..24789d6335a --- /dev/null +++ b/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." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/JpaPackageCache.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/JpaPackageCache.java index 6974dcf7b21..aa57973a90d 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/JpaPackageCache.java +++ b/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(); - requestDetails.setTenantId(JpaConstants.DEFAULT_PARTITION_NAME); + 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(); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImpl.java index d3d3bbda7f3..5a463fca932 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImpl.java +++ b/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); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/JpaPackageCacheTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/JpaPackageCacheTest.java index 04846a9ef18..29be4e2aebc 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/JpaPackageCacheTest.java +++ b/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 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")) { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/NpmR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/NpmR4Test.java index b6c1594d4c3..84ef640e24c 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/NpmR4Test.java +++ b/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 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 myResponses = new HashMap<>();