diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5469-package-installer-property-filter-resource-on-status.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5469-package-installer-property-filter-resource-on-status.yaml new file mode 100644 index 00000000000..1e998f245db --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5469-package-installer-property-filter-resource-on-status.yaml @@ -0,0 +1,6 @@ +--- +type: add +issue: 5469 +jira: SMILE-7594 +title: "There is a programmatic filter enabled which skips installation for certain resources based on their status +in PackageInstallerSvcImpl. The filter can now be controlled via StorageSettings." 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 bc7fe8f5f34..6b09789cca6 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 @@ -28,6 +28,7 @@ import ca.uhn.fhir.context.support.IValidationSupport; import ca.uhn.fhir.context.support.ValidationSupportContext; import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.interceptor.model.RequestPartitionId; +import ca.uhn.fhir.jpa.api.config.JpaStorageSettings; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; @@ -72,7 +73,6 @@ import javax.annotation.PostConstruct; import static ca.uhn.fhir.jpa.packages.util.PackageUtils.DEFAULT_INSTALL_TYPES; import static ca.uhn.fhir.util.SearchParameterUtil.getBaseAsStrings; -import static org.apache.commons.lang3.StringUtils.defaultString; import static org.apache.commons.lang3.StringUtils.isBlank; /** @@ -114,6 +114,9 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc { @Autowired private PackageResourceParsingSvc myPackageResourceParsingSvc; + @Autowired + private JpaStorageSettings myStorageSettings; + /** * Constructor */ @@ -506,7 +509,7 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc { if ("SearchParameter".equals(resourceType)) { String code = SearchParameterUtil.getCode(myFhirContext, theResource); - if (defaultString(code).startsWith("_")) { + if (!isBlank(code) && code.startsWith("_")) { ourLog.warn( "Failed to validate resource of type {} with url {} - Error: Resource code starts with \"_\"", theResource.fhirType(), @@ -548,7 +551,7 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc { * and {@link org.hl7.fhir.r4.model.Communication}, the status field doesn't necessarily need to be set to 'active' * for that resource to be eligible for upload via packages. For example, all {@link org.hl7.fhir.r4.model.Subscription} * have a status of {@link org.hl7.fhir.r4.model.Subscription.SubscriptionStatus#REQUESTED} when they are originally - * inserted into the database, so we accept that value for {@link org.hl7.fhir.r4.model.Subscription} isntead. + * inserted into the database, so we accept that value for {@link org.hl7.fhir.r4.model.Subscription} instead. * Furthermore, {@link org.hl7.fhir.r4.model.DocumentReference} and {@link org.hl7.fhir.r4.model.Communication} can * exist with a wide variety of values for status that include ones such as * {@link org.hl7.fhir.r4.model.Communication.CommunicationStatus#ENTEREDINERROR}, @@ -560,6 +563,9 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc { * @return {@link Boolean#TRUE} if the status value of this resource is acceptable for package upload. */ private boolean isValidResourceStatusForPackageUpload(IBaseResource theResource) { + if (!myStorageSettings.isValidateResourceStatusForPackageUpload()) { + return true; + } List statusTypes = myFhirContext.newFhirPath().evaluate(theResource, "status", IPrimitiveType.class); // Resource does not have a status field diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImplTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImplTest.java index df28a3f2689..75e8cee5e15 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImplTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/PackageInstallerSvcImplTest.java @@ -3,6 +3,7 @@ package ca.uhn.fhir.jpa.packages; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.FhirVersionEnum; import ca.uhn.fhir.context.support.IValidationSupport; +import ca.uhn.fhir.jpa.api.config.JpaStorageSettings; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; import ca.uhn.fhir.jpa.dao.data.INpmPackageVersionDao; @@ -26,9 +27,11 @@ import org.hl7.fhir.r4.model.SearchParameter; import org.hl7.fhir.r4.model.Subscription; import org.hl7.fhir.utilities.npm.NpmPackage; import org.hl7.fhir.utilities.npm.PackageGenerator; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import org.mockito.ArgumentCaptor; import org.mockito.Captor; @@ -46,10 +49,10 @@ import java.util.Collection; import java.util.Iterator; import java.util.List; import java.util.Optional; +import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.params.provider.Arguments.arguments; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -78,6 +81,8 @@ public class PackageInstallerSvcImplTest { private SearchParameterHelper mySearchParameterHelper; @Mock private SearchParameterMap mySearchParameterMap; + @Mock + private JpaStorageSettings myStorageSettings; @Spy private FhirContext myCtx = FhirContext.forR4Cached(); @Spy @@ -103,111 +108,66 @@ public class PackageInstallerSvcImplTest { mySvc.assertFhirVersionsAreCompatible("R4", "R4B"); } - @Test - public void testValidForUpload_SearchParameterWithMetaParam() { - SearchParameter sp = new SearchParameter(); - sp.setCode("_id"); - assertFalse(mySvc.validForUpload(sp)); - } + @Nested + class ValidForUploadTest { + public static Stream parametersIsValidForUpload() { + SearchParameter sp1 = new SearchParameter(); + sp1.setCode("_id"); - @Test - public void testValidForUpload_SearchParameterWithNoBase() { - SearchParameter sp = new SearchParameter(); - sp.setCode("name"); - sp.setExpression("Patient.name"); - sp.setStatus(Enumerations.PublicationStatus.ACTIVE); - assertFalse(mySvc.validForUpload(sp)); - } + SearchParameter sp2 = new SearchParameter(); + sp2.setCode("name"); + sp2.setExpression("Patient.name"); + sp2.setStatus(Enumerations.PublicationStatus.ACTIVE); - @Test - public void testValidForUpload_SearchParameterWithNoExpression() { - SearchParameter sp = new SearchParameter(); - sp.setCode("name"); - sp.addBase("Patient"); - sp.setStatus(Enumerations.PublicationStatus.ACTIVE); - assertFalse(mySvc.validForUpload(sp)); - } + SearchParameter sp3 = new SearchParameter(); + sp3.setCode("name"); + sp3.addBase("Patient"); + sp3.setStatus(Enumerations.PublicationStatus.ACTIVE); + SearchParameter sp4 = new SearchParameter(); + sp4.setCode("name"); + sp4.addBase("Patient"); + sp4.setExpression("Patient.name"); + sp4.setStatus(Enumerations.PublicationStatus.ACTIVE); - @Test - public void testValidForUpload_GoodSearchParameter() { - SearchParameter sp = new SearchParameter(); - sp.setCode("name"); - sp.addBase("Patient"); - sp.setExpression("Patient.name"); - sp.setStatus(Enumerations.PublicationStatus.ACTIVE); - assertTrue(mySvc.validForUpload(sp)); - } + SearchParameter sp5 = new SearchParameter(); + sp5.setCode("name"); + sp5.addBase("Patient"); + sp5.setExpression("Patient.name"); + sp5.setStatus(Enumerations.PublicationStatus.DRAFT); - @Test - public void testValidForUpload_RequestedSubscription() { - Subscription.SubscriptionChannelComponent subscriptionChannelComponent = - new Subscription.SubscriptionChannelComponent() - .setType(Subscription.SubscriptionChannelType.RESTHOOK) - .setEndpoint("https://tinyurl.com/2p95e27r"); - Subscription subscription = new Subscription(); - subscription.setCriteria("Patient?name=smith"); - subscription.setChannel(subscriptionChannelComponent); - subscription.setStatus(Subscription.SubscriptionStatus.REQUESTED); - assertTrue(mySvc.validForUpload(subscription)); - } + return Stream.of( + arguments(sp1, false, false), + arguments(sp2, false, true), + arguments(sp3, false, true), + arguments(sp4, true, true), + arguments(sp5, true, false), + arguments(createSubscription(Subscription.SubscriptionStatus.REQUESTED), true, true), + arguments(createSubscription(Subscription.SubscriptionStatus.ERROR), true, false), + arguments(createSubscription(Subscription.SubscriptionStatus.ACTIVE), true, false), + arguments(createDocumentReference(Enumerations.DocumentReferenceStatus.ENTEREDINERROR), true, true), + arguments(createDocumentReference(Enumerations.DocumentReferenceStatus.NULL), true, false), + arguments(createDocumentReference(null), true, false), + arguments(createCommunication(Communication.CommunicationStatus.NOTDONE), true, true), + arguments(createCommunication(Communication.CommunicationStatus.NULL), true, false), + arguments(createCommunication(null), true, false)); + } - @Test - public void testValidForUpload_ErrorSubscription() { - Subscription.SubscriptionChannelComponent subscriptionChannelComponent = - new Subscription.SubscriptionChannelComponent() - .setType(Subscription.SubscriptionChannelType.RESTHOOK) - .setEndpoint("https://tinyurl.com/2p95e27r"); - Subscription subscription = new Subscription(); - subscription.setCriteria("Patient?name=smith"); - subscription.setChannel(subscriptionChannelComponent); - subscription.setStatus(Subscription.SubscriptionStatus.ERROR); - assertFalse(mySvc.validForUpload(subscription)); - } + @ParameterizedTest + @MethodSource(value = "parametersIsValidForUpload") + public void testValidForUpload_withResource(IBaseResource theResource, + boolean theTheMeetsOtherFilterCriteria, + boolean theMeetsStatusFilterCriteria) { + if (theTheMeetsOtherFilterCriteria) { + when(myStorageSettings.isValidateResourceStatusForPackageUpload()).thenReturn(true); + } + assertEquals(theTheMeetsOtherFilterCriteria && theMeetsStatusFilterCriteria, mySvc.validForUpload(theResource)); - @Test - public void testValidForUpload_ActiveSubscription() { - Subscription.SubscriptionChannelComponent subscriptionChannelComponent = - new Subscription.SubscriptionChannelComponent() - .setType(Subscription.SubscriptionChannelType.RESTHOOK) - .setEndpoint("https://tinyurl.com/2p95e27r"); - Subscription subscription = new Subscription(); - subscription.setCriteria("Patient?name=smith"); - subscription.setChannel(subscriptionChannelComponent); - subscription.setStatus(Subscription.SubscriptionStatus.ACTIVE); - assertFalse(mySvc.validForUpload(subscription)); - } - - @Test - public void testValidForUpload_DocumentRefStatusValuePresent() { - DocumentReference documentReference = new DocumentReference(); - documentReference.setStatus(Enumerations.DocumentReferenceStatus.ENTEREDINERROR); - assertTrue(mySvc.validForUpload(documentReference)); - } - - @Test - public void testValidForUpload_DocumentRefStatusValueNull() { - DocumentReference documentReference = new DocumentReference(); - documentReference.setStatus(Enumerations.DocumentReferenceStatus.NULL); - assertFalse(mySvc.validForUpload(documentReference)); - documentReference.setStatus(null); - assertFalse(mySvc.validForUpload(documentReference)); - } - - @Test - public void testValidForUpload_CommunicationStatusValuePresent() { - Communication communication = new Communication(); - communication.setStatus(Communication.CommunicationStatus.NOTDONE); - assertTrue(mySvc.validForUpload(communication)); - } - - @Test - public void testValidForUpload_CommunicationStatusValueNull() { - Communication communication = new Communication(); - communication.setStatus(Communication.CommunicationStatus.NULL); - assertFalse(mySvc.validForUpload(communication)); - communication.setStatus(null); - assertFalse(mySvc.validForUpload(communication)); + if (theTheMeetsOtherFilterCriteria) { + when(myStorageSettings.isValidateResourceStatusForPackageUpload()).thenReturn(false); + } + assertEquals(theTheMeetsOtherFilterCriteria, mySvc.validForUpload(theResource)); + } } @Test @@ -346,4 +306,28 @@ public class PackageInstallerSvcImplTest { searchParameter.setExpression("someExpression"); return searchParameter; } + + private static Subscription createSubscription(Subscription.SubscriptionStatus theSubscriptionStatus) { + Subscription.SubscriptionChannelComponent subscriptionChannelComponent = + new Subscription.SubscriptionChannelComponent() + .setType(Subscription.SubscriptionChannelType.RESTHOOK) + .setEndpoint("https://tinyurl.com/2p95e27r"); + Subscription subscription = new Subscription(); + subscription.setCriteria("Patient?name=smith"); + subscription.setChannel(subscriptionChannelComponent); + subscription.setStatus(theSubscriptionStatus); + return subscription; + } + + private static DocumentReference createDocumentReference(Enumerations.DocumentReferenceStatus theDocumentStatus) { + DocumentReference documentReference = new DocumentReference(); + documentReference.setStatus(theDocumentStatus); + return documentReference; + } + + private static Communication createCommunication(Communication.CommunicationStatus theCommunicationStatus) { + Communication communication = new Communication(); + communication.setStatus(theCommunicationStatus); + return communication; + } } diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/StorageSettings.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/StorageSettings.java index b14872263ce..cf543b41520 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/StorageSettings.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/StorageSettings.java @@ -146,6 +146,14 @@ public class StorageSettings { */ private boolean myLanguageSearchParameterEnabled = false; + /** + * If set to false, all resource types will be installed via package installer, regardless of their status. + * Otherwise, resources will be filtered based on status according to some criteria which can be found in + * PackageInstallerSvcImpl#isValidResourceStatusForPackageUpload + * @since 7.0.0 + */ + private boolean myValidateResourceStatusForPackageUpload = true; + /** * If set to true, the server will prevent the creation of Subscriptions which cannot be evaluated IN-MEMORY. This can improve * overall server performance. @@ -1319,6 +1327,22 @@ public class StorageSettings { myLanguageSearchParameterEnabled = theLanguageSearchParameterEnabled; } + /** + * @return true if the filter is enabled for resources installed via package installer, false otherwise + * @since 7.0.0 + */ + public boolean isValidateResourceStatusForPackageUpload() { + return myValidateResourceStatusForPackageUpload; + } + + /** + * Should resources being installed via package installer be filtered. + * @since 7.0.0 + */ + public void setValidateResourceStatusForPackageUpload(boolean theValidateResourceStatusForPackageUpload) { + myValidateResourceStatusForPackageUpload = theValidateResourceStatusForPackageUpload; + } + private static void validateTreatBaseUrlsAsLocal(String theUrl) { Validate.notBlank(theUrl, "Base URL must not be null or empty"); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/term/ValueSetExpansionWithHierarchyR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/term/ValueSetExpansionWithHierarchyR4Test.java index a03e78028b9..607491ec002 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/term/ValueSetExpansionWithHierarchyR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/term/ValueSetExpansionWithHierarchyR4Test.java @@ -97,13 +97,13 @@ public class ValueSetExpansionWithHierarchyR4Test extends BaseTermR4Test { @ParameterizedTest @MethodSource(value = "parametersValueSets") - public void testExpandValueSet_whenUsingHierarchicalCodeSystem_willExpandSuccessfully(ValueSet theValueSet, int theExpectedConceptExpensionCount) { + public void testExpandValueSet_whenUsingHierarchicalCodeSystem_willExpandSuccessfully(ValueSet theValueSet, int theExpectedConceptExpansionCount) { myValueSetDao.create(theValueSet, mySrd); myTermSvc.preExpandDeferredValueSetsToTerminologyTables(); Optional optionalTermValueSet = runInTransaction(() -> myTermValueSetDao.findTermValueSetByUrlAndNullVersion(theValueSet.getUrl())); assertTrue(optionalTermValueSet.isPresent()); TermValueSet expandedTermValueSet = optionalTermValueSet.get(); assertEquals(TermValueSetPreExpansionStatusEnum.EXPANDED, expandedTermValueSet.getExpansionStatus()); - assertEquals(theExpectedConceptExpensionCount, expandedTermValueSet.getTotalConcepts()); + assertEquals(theExpectedConceptExpansionCount, expandedTermValueSet.getTotalConcepts()); } }