From 446dd5c7820d5f966b3704a56e38527313d31ab9 Mon Sep 17 00:00:00 2001 From: ianmarshall Date: Tue, 5 Jan 2021 17:51:04 -0500 Subject: [PATCH] Fixed package loading to enable loading of binary package files into the default partition when partitioning is enabled. --- .../fhir/jpa/packages/JpaPackageCache.java | 37 ++++++++-- .../packages/PackageBinaryRequestDetails.java | 35 +++++++++ .../partition/RequestPartitionHelperSvc.java | 13 +++- .../jpa/partition/SystemRequestDetails.java | 73 +++++++++++++++++++ .../jpa/packages/JpaPackageCacheTest.java | 49 +++++++++++-- 5 files changed, 193 insertions(+), 14 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/PackageBinaryRequestDetails.java create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/partition/SystemRequestDetails.java 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 caa425deaec..20e4046a53b 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 @@ -29,6 +29,7 @@ import ca.uhn.fhir.jpa.api.model.ExpungeOptions; 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.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; @@ -54,6 +55,7 @@ import org.apache.http.impl.conn.BasicHttpClientConnectionManager; import org.hl7.fhir.exceptions.FHIRException; import org.hl7.fhir.instance.model.api.IBaseBinary; import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.instance.model.api.IPrimitiveType; import org.hl7.fhir.utilities.npm.BasePackageCacheManager; import org.hl7.fhir.utilities.npm.NpmPackage; @@ -115,6 +117,8 @@ public class JpaPackageCache extends BasePackageCacheManager implements IHapiPac private FhirContext myCtx; @Autowired private PlatformTransactionManager myTxManager; + @Autowired + private PartitionSettings myPartitionSettings; @Override public NpmPackage loadPackageFromCacheOnly(String theId, @Nullable String theVersion) { @@ -205,7 +209,7 @@ public class JpaPackageCache extends BasePackageCacheManager implements IHapiPac return newTxTemplate().execute(tx -> { - ResourceTable persistedPackage = (ResourceTable) getBinaryDao().create(binary).getEntity(); + ResourceTable persistedPackage = createResourceBinary(binary); NpmPackageEntity pkg = myPackageDao.findByPackageId(thePackageId).orElseGet(() -> createPackage(npmPackage)); NpmPackageVersionEntity packageVersion = myPackageVersionDao.findByPackageIdAndVersion(thePackageId, packageVersionId).orElse(null); if (packageVersion != null) { @@ -282,7 +286,7 @@ public class JpaPackageCache extends BasePackageCacheManager implements IHapiPac byte[] minimizedContents = packageContext.newJsonParser().encodeResourceToString(resource).getBytes(StandardCharsets.UTF_8); IBaseBinary resourceBinary = createPackageResourceBinary(nextFile, minimizedContents, contentType); - ResourceTable persistedResource = (ResourceTable) getBinaryDao().create(resourceBinary).getEntity(); + ResourceTable persistedResource = createResourceBinary(resourceBinary); NpmPackageVersionResourceEntity resourceEntity = new NpmPackageVersionResourceEntity(); resourceEntity.setPackageVersion(packageVersion); @@ -319,6 +323,16 @@ public class JpaPackageCache extends BasePackageCacheManager implements IHapiPac } + private ResourceTable createResourceBinary(IBaseBinary theResourceBinary) { + + if (myPartitionSettings.isPartitioningEnabled()) { + PackageBinaryRequestDetails myRequestDetails = new PackageBinaryRequestDetails(); + return (ResourceTable) getBinaryDao().create(theResourceBinary, myRequestDetails).getEntity(); + } else { + return (ResourceTable) getBinaryDao().create(theResourceBinary).getEntity(); + } + } + private boolean updateCurrentVersionFlagForAllPackagesBasedOnNewIncomingVersion(String thePackageId, String thePackageVersion) { Collection existingVersions = myPackageVersionDao.findByPackageId(thePackageId); boolean retVal = true; @@ -578,16 +592,14 @@ public class JpaPackageCache extends BasePackageCacheManager implements IHapiPac ExpungeOptions options = new ExpungeOptions(); options.setExpungeDeletedResources(true).setExpungeOldVersions(true); - getBinaryDao().delete(next.getResourceBinary().getIdDt().toVersionless()); - getBinaryDao().forceExpungeInExistingTransaction(next.getResourceBinary().getIdDt().toVersionless(), options, null); + deleteAndExpungeResourceBinary(next.getResourceBinary().getIdDt().toVersionless(), options); } myPackageVersionDao.delete(packageVersion.get()); ExpungeOptions options = new ExpungeOptions(); options.setExpungeDeletedResources(true).setExpungeOldVersions(true); - getBinaryDao().delete(packageVersion.get().getPackageBinary().getIdDt().toVersionless()); - getBinaryDao().forceExpungeInExistingTransaction(packageVersion.get().getPackageBinary().getIdDt().toVersionless(), options, null); + deleteAndExpungeResourceBinary(packageVersion.get().getPackageBinary().getIdDt().toVersionless(), options); Collection remainingVersions = myPackageVersionDao.findByPackageId(thePackageId); if (remainingVersions.size() == 0) { @@ -622,6 +634,19 @@ public class JpaPackageCache extends BasePackageCacheManager implements IHapiPac return retVal; } + private void deleteAndExpungeResourceBinary(IIdType theResourceBinaryId, ExpungeOptions theOptions) { + + if (myPartitionSettings.isPartitioningEnabled()) { + PackageBinaryRequestDetails myRequestDetails = new PackageBinaryRequestDetails(); + getBinaryDao().delete(theResourceBinaryId, myRequestDetails).getEntity(); + getBinaryDao().forceExpungeInExistingTransaction(theResourceBinaryId, theOptions, myRequestDetails); + } else { + getBinaryDao().delete(theResourceBinaryId).getEntity(); + getBinaryDao().forceExpungeInExistingTransaction(theResourceBinaryId, theOptions, null); + } + } + + @Nonnull public List createSearchPredicates(PackageSearchSpec thePackageSearchSpec, CriteriaBuilder theCb, Root theRoot) { List predicates = new ArrayList<>(); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/PackageBinaryRequestDetails.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/PackageBinaryRequestDetails.java new file mode 100644 index 00000000000..963c80e0019 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/packages/PackageBinaryRequestDetails.java @@ -0,0 +1,35 @@ +package ca.uhn.fhir.jpa.packages; + +import ca.uhn.fhir.interceptor.api.HookParams; +import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster; +import ca.uhn.fhir.interceptor.api.Pointcut; +import ca.uhn.fhir.jpa.partition.SystemRequestDetails; + +public class PackageBinaryRequestDetails extends SystemRequestDetails { + /** + * Constructor + * + */ + public PackageBinaryRequestDetails() { + super(new MyInterceptorBroadcaster()); + } + + private static class MyInterceptorBroadcaster implements IInterceptorBroadcaster { + + @Override + public boolean callHooks(Pointcut thePointcut, HookParams theParams) { + return true; + } + + @Override + public Object callHooksAndReturnObject(Pointcut thePointcut, HookParams theParams) { + return null; + } + + @Override + public boolean hasHooks(Pointcut thePointcut) { + return false; + } + } + +} 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 index bb4899fb394..e9128b6c22b 100644 --- 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 @@ -28,6 +28,7 @@ import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.jpa.entity.PartitionEntity; import ca.uhn.fhir.jpa.model.config.PartitionSettings; import ca.uhn.fhir.jpa.model.util.JpaConstants; +import ca.uhn.fhir.jpa.packages.PackageBinaryRequestDetails; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; @@ -68,14 +69,20 @@ public class RequestPartitionHelperSvc implements IRequestPartitionHelperSvc { myPartitioningBlacklist.add("Subscription"); myPartitioningBlacklist.add("SearchParameter"); - // Validation + // Validation and Conformance myPartitioningBlacklist.add("StructureDefinition"); myPartitioningBlacklist.add("Questionnaire"); + myPartitioningBlacklist.add("CapabilityStatement"); + myPartitioningBlacklist.add("CompartmentDefinition"); + myPartitioningBlacklist.add("OperationDefinition"); // Terminology myPartitioningBlacklist.add("ConceptMap"); myPartitioningBlacklist.add("CodeSystem"); myPartitioningBlacklist.add("ValueSet"); + myPartitioningBlacklist.add("NamingSystem"); + myPartitioningBlacklist.add("StructureMap"); + } /** @@ -91,7 +98,7 @@ public class RequestPartitionHelperSvc implements IRequestPartitionHelperSvc { if (myPartitionSettings.isPartitioningEnabled()) { // Handle system requests - if (theRequest == null && myPartitioningBlacklist.contains(theResourceType)) { + if ((theRequest == null && myPartitioningBlacklist.contains(theResourceType)) || theRequest instanceof SystemRequestDetails) { return RequestPartitionId.defaultPartition(); } @@ -123,7 +130,7 @@ public class RequestPartitionHelperSvc implements IRequestPartitionHelperSvc { if (myPartitionSettings.isPartitioningEnabled()) { // Handle system requests - if (theRequest == null && myPartitioningBlacklist.contains(theResourceType)) { + if ((theRequest == null && myPartitioningBlacklist.contains(theResourceType)) || theRequest instanceof SystemRequestDetails) { return RequestPartitionId.defaultPartition(); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/partition/SystemRequestDetails.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/partition/SystemRequestDetails.java new file mode 100644 index 00000000000..baaf5bcb067 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/partition/SystemRequestDetails.java @@ -0,0 +1,73 @@ +package ca.uhn.fhir.jpa.partition; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster; +import ca.uhn.fhir.rest.api.server.RequestDetails; +import ca.uhn.fhir.rest.server.IRestfulServerDefaults; + +import java.io.IOException; +import java.io.InputStream; +import java.io.Reader; +import java.nio.charset.Charset; +import java.util.List; + +public class SystemRequestDetails extends RequestDetails { + public SystemRequestDetails(IInterceptorBroadcaster theInterceptorBroadcaster) { + super(theInterceptorBroadcaster); + } + + @Override + protected byte[] getByteStreamRequestContents() { + return new byte[0]; + } + + @Override + public Charset getCharset() { + return null; + } + + @Override + public FhirContext getFhirContext() { + return null; + } + + @Override + public String getHeader(String name) { + return null; + } + + @Override + public List getHeaders(String name) { + return null; + } + + @Override + public Object getAttribute(String theAttributeName) { + return null; + } + + @Override + public void setAttribute(String theAttributeName, Object theAttributeValue) { + + } + + @Override + public InputStream getInputStream() throws IOException { + return null; + } + + @Override + public Reader getReader() throws IOException { + return null; + } + + @Override + public IRestfulServerDefaults getServer() { + return null; + } + + @Override + public String getServerBaseForRequest() { + return null; + } +} 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 adef1a64ce8..0ba24b0bcc0 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 @@ -1,21 +1,20 @@ package ca.uhn.fhir.jpa.packages; +import ca.uhn.fhir.interceptor.api.IInterceptorService; import ca.uhn.fhir.jpa.dao.data.INpmPackageDao; import ca.uhn.fhir.jpa.dao.data.INpmPackageVersionDao; import ca.uhn.fhir.jpa.dao.r4.BaseJpaR4Test; -import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; -import ca.uhn.fhir.util.JsonUtil; -import org.hl7.fhir.utilities.npm.IPackageCacheManager; +import ca.uhn.fhir.rest.server.interceptor.partition.RequestTenantPartitionInterceptor; import org.hl7.fhir.utilities.npm.NpmPackage; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import java.io.IOException; import java.io.InputStream; +import java.util.List; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.contains; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.fail; @@ -28,6 +27,16 @@ public class JpaPackageCacheTest extends BaseJpaR4Test { private INpmPackageDao myPackageDao; @Autowired private INpmPackageVersionDao myPackageVersionDao; + @Autowired + private IInterceptorService myInterceptorService; + @Autowired + private RequestTenantPartitionInterceptor myRequestTenantPartitionInterceptor; + + @AfterEach + public void disablePartitioning() { + myPartitionSettings.setPartitioningEnabled(false); + myInterceptorService.unregisterInterceptor(myRequestTenantPartitionInterceptor); + } @Test @@ -53,6 +62,36 @@ public class JpaPackageCacheTest extends BaseJpaR4Test { } + @Test + public void testSaveAndDeletePackagePartitionsEnabled() throws IOException { + myPartitionSettings.setPartitioningEnabled(true); + myInterceptorService.registerInterceptor(myRequestTenantPartitionInterceptor); + + try (InputStream stream = IgInstallerDstu3Test.class.getResourceAsStream("/packages/basisprofil.de.tar.gz")) { + myPackageCacheManager.addPackageToCache("basisprofil.de", "0.2.40", stream, "basisprofil.de"); + } + + NpmPackage pkg; + + pkg = myPackageCacheManager.loadPackage("basisprofil.de", null); + assertEquals("0.2.40", pkg.version()); + + pkg = myPackageCacheManager.loadPackage("basisprofil.de", "0.2.40"); + assertEquals("0.2.40", pkg.version()); + + try { + myPackageCacheManager.loadPackage("basisprofil.de", "99"); + fail(); + } catch (ResourceNotFoundException e) { + assertEquals("Unable to locate package basisprofil.de#99", e.getMessage()); + } + + PackageDeleteOutcomeJson deleteOutcomeJson = myPackageCacheManager.uninstallPackage("basisprofil.de", "0.2.40"); + List deleteOutcomeMsgs = deleteOutcomeJson.getMessage(); + assertEquals("Deleting package basisprofil.de#0.2.40", deleteOutcomeMsgs.get(0)); + } + + @Test public void testSavePackageWithLongDescription() throws IOException { try (InputStream stream = IgInstallerDstu3Test.class.getResourceAsStream("/packages/package-davinci-cdex-0.2.0.tgz")) {