Issue 3486 (#3487)

* added failing tests

* tests pass

* Added changelog and javadocs

* fixed package tests for resources with no active field

* adding jira ticket number

* feedback from code review'

* switching to old style switch statement'

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3486-package-upload-non-active-resources.yaml

@@ -0,0 +1,8 @@
+---
+type: fix
+issue: 3486
+jira: SMILE-3626
+title: "When doing package upload, all resources were filtered by `status=active`. That is inappropriate for some types. 
+For Subscription, we need to check if the value is requested, for DocumentReference and Communication other values 
+outside the expected active and not active also exist. This change adds a check for those types of resources other than 
+ the one normally done for active matching value."

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

@@ -417,15 +417,49 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 
 		}
 
-		List<IPrimitiveType> statusTypes = myFhirContext.newFhirPath().evaluate(theResource, "status", IPrimitiveType.class);
+		if (!isValidResourceStatusForPackageUpload(theResource)) {
-		if (statusTypes.size() > 0 && !statusTypes.get(0).getValueAsString().equals("active")) {
+			ourLog.warn("Failed to validate resource of type {} with ID {} - Error: Resource status not accepted value.",
-			ourLog.warn("Failed to validate resource of type {} with ID {} - Error: Resource status not equal to \"active\"", theResource.fhirType(), theResource.getIdElement().getValue());
+				theResource.fhirType(), theResource.getIdElement().getValue());
 			return false;
 		}
 
 		return true;
 	}
 
+	/**
+	 * For resources like {@link org.hl7.fhir.r4.model.Subscription}, {@link org.hl7.fhir.r4.model.DocumentReference},
+	 * 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.
+	 * 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},
+	 * {@link org.hl7.fhir.r4.model.Communication.CommunicationStatus#UNKNOWN},
+	 * {@link org.hl7.fhir.r4.model.DocumentReference.ReferredDocumentStatus#ENTEREDINERROR},
+	 * {@link org.hl7.fhir.r4.model.DocumentReference.ReferredDocumentStatus#PRELIMINARY}, and others, which while not considered
+	 * 'final' values, should still be uploaded for reference.
+	 *
+	 * @return {@link Boolean#TRUE} if the status value of this resource is acceptable for package upload.
+	 */
+	private boolean isValidResourceStatusForPackageUpload(IBaseResource theResource) {
+		List<IPrimitiveType> statusTypes = myFhirContext.newFhirPath().evaluate(theResource, "status", IPrimitiveType.class);
+		// Resource does not have a status field
+		if (statusTypes.isEmpty()) return true;
+		// Resource has a null status field
+		if (statusTypes.get(0).getValue() == null) return false;
+		// Resource has a status, and we need to check based on type
+		switch (theResource.fhirType()) {
+			case "Subscription":
+				return (statusTypes.get(0).getValueAsString().equals("requested"));
+			case "DocumentReference":
+			case "Communication":
+				return (!statusTypes.get(0).getValueAsString().equals("?"));
+			default:
+				return (statusTypes.get(0).getValueAsString().equals("active"));
+		}
+	}
+
 	private boolean isStructureDefinitionWithoutSnapshot(IBaseResource r) {
 		boolean retVal = false;
 		FhirTerser terser = myFhirContext.newTerser();

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

@@ -1,8 +1,11 @@
 package ca.uhn.fhir.jpa.packages;
 
 import ca.uhn.fhir.context.FhirContext;
+import org.hl7.fhir.r4.model.Communication;
+import org.hl7.fhir.r4.model.DocumentReference;
 import org.hl7.fhir.r4.model.Enumerations;
 import org.hl7.fhir.r4.model.SearchParameter;
+import org.hl7.fhir.r4.model.Subscription;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -56,4 +59,74 @@ class PackageInstallerSvcImplTest {
 		assertTrue(mySvc.validForUpload(sp));
 	}
 
+	@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));
+	}
+
+	@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));
+	}
+
+	@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));
+	}
 }