Don't install draft package contents (#2099)

* Don't install draft package contents

* Add changelog

* Add changelog

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_2_0/2099_dont_install_draft_package_contents.yaml

@@ -0,0 +1,6 @@
+---
+type: fix
+issue: 2099
+title: "When using an NPM package spec in STORE_AND_INSTALL mode, conformance resources will only be stored if they
+  have a status of `active`. This fixes a bug wgere installing the US Core IG caused searches to stop working due to
+  draft Search Parameters."

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_2_0/2099_dont_override_sps_with_draft.yaml

@@ -0,0 +1,6 @@
+---
+type: add
+issue: 2099
+title: "Stored SearchParameter resources with a status of DRAFT will no longer override and disable existing built-in
+   search parameters. This is done in order to avoid issues caused by uploading NPM packages such as US Core that
+   contain a large number of draft parameters."

hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/model/DaoMethodOutcome.java

@@ -28,6 +28,7 @@ public class DaoMethodOutcome extends MethodOutcome {
 
 	private IBasePersistedResource myEntity;
 	private IBaseResource myPreviousResource;
+	private boolean myNop;
 
 	/**
 	 * Constructor
@@ -36,6 +37,20 @@ public class DaoMethodOutcome extends MethodOutcome {
 		super();
 	}
 
+	/**
+	 * Was this a NO-OP - Typically because of an update to a resource that already matched the contents provided
+	 */
+	public boolean isNop() {
+		return myNop;
+	}
+
+	/**
+	 * Was this a NO-OP - Typically because of an update to a resource that already matched the contents provided
+	 */
+	public void setNop(boolean theNop) {
+		myNop = theNop;
+	}
+
 	public IBasePersistedResource getEntity() {
 		return myEntity;
 	}

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java

@@ -29,6 +29,7 @@ import ca.uhn.fhir.interceptor.api.Pointcut;
 import ca.uhn.fhir.jpa.api.config.DaoConfig;
 import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome;
 import ca.uhn.fhir.jpa.model.cross.IBasePersistedResource;
+import ca.uhn.fhir.jpa.model.entity.ResourceTable;
 import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
 import ca.uhn.fhir.jpa.searchparam.registry.ISearchParamRegistry;
 import ca.uhn.fhir.jpa.util.JpaInterceptorBroadcaster;
@@ -128,6 +129,12 @@ public abstract class BaseStorageDao {
 	protected DaoMethodOutcome toMethodOutcome(RequestDetails theRequest, @Nonnull final IBasePersistedResource theEntity, @Nonnull IBaseResource theResource) {
 		DaoMethodOutcome outcome = new DaoMethodOutcome();
 
+		if (theEntity instanceof ResourceTable) {
+			if (((ResourceTable) theEntity).isUnchangedInCurrentOperation()) {
+				outcome.setNop(true);
+			}
+		}
+
 		IIdType id = null;
 		if (theResource.getIdElement().getValue() != null) {
 			id = theResource.getIdElement();

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

@@ -26,6 +26,7 @@ import ca.uhn.fhir.context.support.IValidationSupport;
 import ca.uhn.fhir.context.support.ValidationSupportContext;
 import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
 import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao;
+import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome;
 import ca.uhn.fhir.jpa.dao.data.INpmPackageVersionDao;
 import ca.uhn.fhir.jpa.model.entity.NpmPackageVersionEntity;
 import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
@@ -35,7 +36,6 @@ import ca.uhn.fhir.rest.param.StringParam;
 import ca.uhn.fhir.rest.param.TokenParam;
 import ca.uhn.fhir.rest.param.UriParam;
 import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
-import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException;
 import ca.uhn.fhir.util.FhirTerser;
 import ca.uhn.fhir.util.SearchParameterUtil;
 import com.google.common.annotations.VisibleForTesting;
@@ -208,6 +208,7 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 			count[i] = resources.size();
 
 			for (IBaseResource next : resources) {
+
 				try {
 					next = isStructureDefinitionWithoutSnapshot(next) ? generateSnapshot(next) : next;
 					create(next, theOutcome);
@@ -215,6 +216,7 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 					ourLog.warn("Failed to upload resource of type {} with ID {} - Error: {}", myFhirContext.getResourceType(next), next.getIdElement().getValue(), e.toString());
 					throw new ImplementationGuideInstallationException(String.format("Error installing IG %s#%s: %s", name, version, e.toString()), e);
 				}
+
 			}
 
 		}
@@ -321,10 +323,11 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 			} else {
 
 				ourLog.info("Updating existing resource matching {}", map.toNormalizedQueryString(myFhirContext));
-				theOutcome.incrementResourcesInstalled(myFhirContext.getResourceType(theResource));
 				theResource.setId(searchResult.getResources(0, 1).get(0).getIdElement().toUnqualifiedVersionless());
-				dao.update(theResource);
+				DaoMethodOutcome outcome = dao.update(theResource);
-
+				if (!outcome.isNop()) {
+					theOutcome.incrementResourcesInstalled(myFhirContext.getResourceType(theResource));
+				}
 			}
 
 		}
@@ -349,6 +352,13 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 			}
 		}
 
+		List<IPrimitiveType> statusTypes = myFhirContext.newFhirPath().evaluate(theResource, "status", IPrimitiveType.class);
+		if (statusTypes.size() > 0) {
+			if (!statusTypes.get(0).getValueAsString().equals("active")) {
+				return false;
+			}
+		}
+
 		return true;
 	}
 

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchCustomSearchParamTest.java

@@ -1,6 +1,7 @@
 package ca.uhn.fhir.jpa.dao.r4;
 
 import ca.uhn.fhir.context.FhirContext;
+import ca.uhn.fhir.context.RuntimeSearchParam;
 import ca.uhn.fhir.interceptor.api.HookParams;
 import ca.uhn.fhir.interceptor.api.IAnonymousInterceptor;
 import ca.uhn.fhir.interceptor.api.Pointcut;
@@ -462,6 +463,24 @@ public class FhirResourceDaoR4SearchCustomSearchParamTest extends BaseJpaR4Test
 
 	}
 
+	@Test
+	public void testBuiltInSearchParameterNotReplacedByDraftSearchParameter() {
+		myModelConfig.setDefaultSearchParamsCanBeOverridden(true);
+
+		SearchParameter memberSp = new SearchParameter();
+		memberSp.setCode("family");
+		memberSp.addBase("Patient");
+		memberSp.setType(Enumerations.SearchParamType.STRING);
+		memberSp.setExpression("Patient.name.family");
+		memberSp.setStatus(Enumerations.PublicationStatus.DRAFT);
+		mySearchParameterDao.create(memberSp, mySrd);
+
+		mySearchParamRegistry.forceRefresh();
+
+		RuntimeSearchParam sp = mySearchParamRegistry.getActiveSearchParam("Patient", "family");
+		assertEquals(RuntimeSearchParam.RuntimeSearchParamStatusEnum.ACTIVE, sp.getStatus());
+	}
+
 
 
 	@Test

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

@@ -49,6 +49,7 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
 
@@ -114,6 +115,20 @@ public class NpmTestR4 extends BaseJpaR4Test {
 
 		PackageInstallationSpec spec = new PackageInstallationSpec().setName("hl7.fhir.us.core").setVersion("3.1.0").setInstallMode(PackageInstallationSpec.InstallModeEnum.STORE_AND_INSTALL).setFetchDependencies(true);
 		igInstaller.install(spec);
+
+		runInTransaction(()->{
+			SearchParameterMap map = SearchParameterMap.newSynchronous(SearchParameter.SP_BASE, new TokenParam("NamingSystem"));
+			IBundleProvider outcome = mySearchParameterDao.search(map);
+			List<IBaseResource> resources = outcome.getResources(0, outcome.sizeOrThrowNpe());
+			for (int i = 0; i < resources.size(); i++) {
+				ourLog.info("**************************************************************************");
+				ourLog.info("**************************************************************************");
+				ourLog.info("Res " + i);
+				ourLog.info(myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(resources.get(i)));
+			}
+		});
+
+		igInstaller.install(spec);
 	}
 
 
@@ -162,7 +177,8 @@ public class NpmTestR4 extends BaseJpaR4Test {
 		myFakeNpmServlet.myResponses.put("/hl7.fhir.uv.shorthand/0.12.0", bytes);
 
 		PackageInstallationSpec spec = new PackageInstallationSpec().setName("hl7.fhir.uv.shorthand").setVersion("0.12.0").setInstallMode(PackageInstallationSpec.InstallModeEnum.STORE_AND_INSTALL);
-		igInstaller.install(spec);
+		PackageInstallOutcomeJson outcome = igInstaller.install(spec);
+		assertEquals(1, outcome.getResourcesInstalled().get("CodeSystem"));
 
 		// Be sure no further communication with the server
 		JettyUtil.closeServer(myServer);
@@ -215,11 +231,25 @@ public class NpmTestR4 extends BaseJpaR4Test {
 		runInTransaction(() -> {
 			SearchParameterMap map = SearchParameterMap.newSynchronous();
 			map.add(StructureDefinition.SP_URL, new UriParam("http://hl7.org/fhir/uv/shorthand/CodeSystem/shorthand-code-system"));
-			IBundleProvider outcome = myCodeSystemDao.search(map);
+			IBundleProvider result = myCodeSystemDao.search(map);
-			assertEquals(1, outcome.sizeOrThrowNpe());
+			assertEquals(1, result.sizeOrThrowNpe());
 		});
 	}
 
+
+	@Test
+	public void testInstallR4Package_DraftResourcesNotInstalled() throws Exception {
+		myDaoConfig.setAllowExternalReferences(true);
+
+		byte[] bytes = loadClasspathBytes("/packages/test-draft-sample.tgz");
+		myFakeNpmServlet.myResponses.put("/hl7.fhir.uv.shorthand/0.11.1", bytes);
+
+		PackageInstallationSpec spec = new PackageInstallationSpec().setName("hl7.fhir.uv.shorthand").setVersion("0.11.1").setInstallMode(PackageInstallationSpec.InstallModeEnum.STORE_AND_INSTALL);
+		PackageInstallOutcomeJson outcome = igInstaller.install(spec);
+		assertEquals(0, outcome.getResourcesInstalled().size(), outcome.getResourcesInstalled().toString());
+
+	}
+
 	@Test
 	public void testInstallR4Package_Twice() throws Exception {
 		myDaoConfig.setAllowExternalReferences(true);
@@ -236,6 +266,11 @@ public class NpmTestR4 extends BaseJpaR4Test {
 		igInstaller.install(spec);
 		outcome = igInstaller.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());
+
 	}
 
 
@@ -257,9 +292,6 @@ public class NpmTestR4 extends BaseJpaR4Test {
 		assertEquals(null, pkg.description());
 		assertEquals("UK.Core.r4", pkg.name());
 
-		// Ensure that we loaded the contents
-		IBundleProvider searchResult = myStructureDefinitionDao.search(SearchParameterMap.newSynchronous("url", new UriParam("https://fhir.nhs.uk/R4/StructureDefinition/UKCore-Patient")));
-		assertEquals(1, searchResult.sizeOrThrowNpe());
 	}
 
 	@Test

hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/registry/SearchParamRegistryImpl.java

@@ -291,6 +291,9 @@ public class SearchParamRegistryImpl implements ISearchParamRegistry {
 				if (runtimeSp == null) {
 					continue;
 				}
+				if (runtimeSp.getStatus() == RuntimeSearchParam.RuntimeSearchParamStatusEnum.DRAFT) {
+					continue;
+				}
 
 				for (String nextBaseName : SearchParameterUtil.getBaseAsStrings(myFhirContext, nextSp)) {
 					if (isBlank(nextBaseName)) {