Skip resource updating completely if found in package load. (#4036)

* add ability to skip loading package resources that already exists, to avoid reindex when loading IGs

* Add a test for validating no updates or creates are called on anything but binaries

* Add changelog

Co-authored-by: Craig McClendon <craig.mcclendon@accenture.com>

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/3989-skip-package-install-on-match.yaml

@@ -0,0 +1,5 @@
+---
+type: add
+issue: 3989
+title: "Provided the ability to have the NPM package installer skip installing a package if it is already installed and matches the version requested. This can be controlled by
+the `reloadExisting` attribute in PackageInstallationSpec. It defaults to `true`, which is the existing behaviour. Thanks to  Craig McClendon (@XcrigX) for the contribution!"

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

@@ -43,6 +43,8 @@ public class PackageInstallOutcomeJson {
 	@JsonProperty("resourcesInstalled")
 	private Map<String, Integer> myResourcesInstalled;
 
+
+
 	public List<String> getMessage() {
 		if (myMessage == null) {
 			myMessage = new ArrayList<>();

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

@@ -34,12 +34,10 @@ import java.util.List;
 import java.util.function.Supplier;
 
 @Schema(
-	name = "PackageInstallationSpec",
+	name = "PackageInstallationSpec", description = "Defines a set of instructions for package installation"
-	description =
+	)
-		"Defines a set of instructions for package installation"
-)
 @JsonPropertyOrder({
-	"name", "version", "packageUrl", "installMode", "installResourceTypes", "validationMode"
+	"name", "version", "packageUrl", "installMode", "installResourceTypes", "validationMode", "reloadExisting"
 })
 @ExampleSupplier({PackageInstallationSpec.ExampleSupplier.class, PackageInstallationSpec.ExampleSupplier2.class})
 @JsonInclude(JsonInclude.Include.NON_NULL)
@@ -64,6 +62,9 @@ public class PackageInstallationSpec {
 	@Schema(description = "Should dependencies be automatically resolved, fetched and installed with the same settings")
 	@JsonProperty("fetchDependencies")
 	private boolean myFetchDependencies;
+	@Schema(description = "Should existing resources be reloaded. Defaults to true, but can be set to false to avoid re-index operations for existing search parameters")
+	@JsonProperty("reloadExisting")
+	private boolean myReloadExisting = true;
 	@Schema(description = "Any values provided here will be interpreted as a regex. Dependencies with an ID matching any regex will be skipped.")
 	private List<String> myDependencyExcludes;
 	@JsonIgnore
@@ -145,6 +146,14 @@ public class PackageInstallationSpec {
 		return this;
 	}
 
+	public boolean isReloadExisting() {
+		return myReloadExisting;
+	}
+
+	public void setReloadExisting(boolean reloadExisting) {
+		this.myReloadExisting = reloadExisting;
+	}
+
 	public PackageInstallationSpec addDependencyExclude(String theExclude) {
 		getDependencyExcludes().add(theExclude);
 		return this;

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

@@ -92,7 +92,7 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 		"ConceptMap",
 		"SearchParameter",
 		"Subscription"
-	));
+		));
 
 	boolean enabled = true;
 	@Autowired
@@ -163,7 +163,7 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 					return existing.isPresent();
 				});
 				if (exists) {
-					ourLog.info("Package {}#{} is already installed", theInstallationSpec.getName(), theInstallationSpec.getVersion());
+						ourLog.info("Package {}#{} is already installed", theInstallationSpec.getName(), theInstallationSpec.getVersion());
 				}
 
 				NpmPackage npmPackage = myPackageCacheManager.installPackage(theInstallationSpec);
@@ -225,7 +225,7 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 
 				try {
 					next = isStructureDefinitionWithoutSnapshot(next) ? generateSnapshot(next) : next;
-					create(next, theOutcome);
+					create(next, theInstallationSpec, theOutcome);
 				} catch (Exception e) {
 					ourLog.warn("Failed to upload resource of type {} with ID {} - Error: {}", myFhirContext.getResourceType(next), next.getIdElement().getValue(), e.toString());
 					throw new ImplementationGuideInstallationException(Msg.code(1286) + String.format("Error installing IG %s#%s: %s", name, version, e), e);
@@ -323,7 +323,7 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 		return resources;
 	}
 
-	private void create(IBaseResource theResource, PackageInstallOutcomeJson theOutcome) {
+	private void create(IBaseResource theResource, PackageInstallationSpec theInstallationSpec, PackageInstallOutcomeJson theOutcome) {
 		IFhirResourceDao dao = myDaoRegistry.getResourceDao(theResource.getClass());
 		SearchParameterMap map = createSearchParameterMapFor(theResource);
 		IBundleProvider searchResult = searchResource(dao, map);
@@ -347,11 +347,15 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 					ourLog.info("Created resource with existing id");
 				}
 			} else {
-			ourLog.info("Updating existing resource matching {}", map.toNormalizedQueryString(myFhirContext));
+				if (theInstallationSpec.isReloadExisting()) {
-				theResource.setId(searchResult.getResources(0, 1).get(0).getIdElement().toUnqualifiedVersionless());
+					ourLog.info("Updating existing resource matching {}", map.toNormalizedQueryString(myFhirContext));
-				DaoMethodOutcome outcome = updateResource(dao, theResource);
+					theResource.setId(searchResult.getResources(0, 1).get(0).getIdElement().toUnqualifiedVersionless());
-				if (!outcome.isNop()) {
+					DaoMethodOutcome outcome = updateResource(dao, theResource);
-					theOutcome.incrementResourcesInstalled(myFhirContext.getResourceType(theResource));
+					if (!outcome.isNop()) {
+						theOutcome.incrementResourcesInstalled(myFhirContext.getResourceType(theResource));
+					}
+				} else {
+					ourLog.info("Skipping update of existing resource matching {}", map.toNormalizedQueryString(myFhirContext));
 				}
 			}
 		}

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

@@ -360,5 +360,4 @@ public class SearchParamRegistryImpl implements ISearchParamRegistry, IResourceC
 	public void setSearchParameterCanonicalizerForUnitTest(SearchParameterCanonicalizer theSearchParameterCanonicalizerForUnitTest) {
 		mySearchParameterCanonicalizer = theSearchParameterCanonicalizerForUnitTest;
 	}
-
 }

hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/packages/IgInstallerDstu3Test.java

@@ -1,8 +1,12 @@
 package ca.uhn.fhir.jpa.packages;
 
 import ca.uhn.fhir.i18n.Msg;
+import ca.uhn.fhir.interceptor.api.IAnonymousInterceptor;
+import ca.uhn.fhir.interceptor.api.Pointcut;
 import ca.uhn.fhir.jpa.api.config.DaoConfig;
 import ca.uhn.fhir.jpa.dao.data.INpmPackageVersionDao;
+import ca.uhn.fhir.jpa.model.entity.IBaseResourceEntity;
+import ca.uhn.fhir.jpa.model.entity.NpmPackageVersionEntity;
 import ca.uhn.fhir.jpa.test.BaseJpaDstu3Test;
 import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
 import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
@@ -11,6 +15,9 @@ import ca.uhn.fhir.test.utilities.ProxyUtil;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.servlet.ServletHandler;
 import org.eclipse.jetty.servlet.ServletHolder;
+import org.hl7.fhir.instance.model.api.IBaseBinary;
+import org.hl7.fhir.instance.model.api.IBaseResource;
+import org.hl7.fhir.r4.model.Patient;
 import org.hl7.fhir.utilities.npm.IPackageCacheManager;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
@@ -19,11 +26,17 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 
+import java.util.Date;
+import java.util.Optional;
+import java.util.concurrent.Callable;
 import java.util.stream.Collectors;
 
 import static ca.uhn.fhir.util.ClasspathUtil.loadResourceAsByteArray;
+import static com.healthmarketscience.sqlbuilder.Conditions.not;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.hasItem;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -168,6 +181,54 @@ public class IgInstallerDstu3Test extends BaseJpaDstu3Test {
 		runInTransaction(() -> {
 			assertTrue(myPackageVersionDao.findByPackageIdAndVersion("nictiz.fhir.nl.stu3.questionnaires", "1.0.2").isPresent());
 		});
+	}
+
+	private void ensureNoCreatesOrUpdates(Callable theCallable) throws Exception {
+		myInterceptorRegistry.registerAnonymousInterceptor(Pointcut.STORAGE_PRESTORAGE_RESOURCE_CREATED, (thePointcut, t) -> {
+			IBaseResource iBaseResource = t.get(IBaseResource.class);
+			if (iBaseResource instanceof IBaseBinary) {
+				return;
+			}
+			throw new RuntimeException("Not allowed!");
+		});
+		myInterceptorRegistry.registerAnonymousInterceptor(Pointcut.STORAGE_PRESTORAGE_RESOURCE_UPDATED, (thePointcut, t) -> {
+			IBaseResource iBaseResource = t.get(IBaseResource.class);
+			if (iBaseResource instanceof IBaseBinary) {
+				return;
+			}
+			throw new RuntimeException("Not allowed!");
+		});
+
+		try {
+			theCallable.call();
+		} finally {
+			myInterceptorRegistry.unregisterAllAnonymousInterceptors();
+		}
+
+
+	}
+	@Test
+	public void testMultipleUploads() throws Exception {
+		myDaoConfig.setAllowExternalReferences(true);
+		PackageInstallationSpec installationSpec = new PackageInstallationSpec()
+			.setName("nictiz.fhir.nl.stu3.questionnaires")
+			.setVersion("1.0.2")
+			.setInstallMode(PackageInstallationSpec.InstallModeEnum.STORE_AND_INSTALL)
+			.setPackageUrl("classpath:/packages/nictiz.fhir.nl.stu3.questionnaires-1.0.2.tgz");
+
+
+		igInstaller.install(installationSpec);
+
+		installationSpec.setReloadExisting(true);
+		try {
+			ensureNoCreatesOrUpdates(() -> igInstaller.install(installationSpec));
+			fail();
+		} catch (RuntimeException e) {
+			assertThat(e.getMessage(), is(containsString("Not allowed!")));
+		}
+
+		installationSpec.setReloadExisting(false);
+		ensureNoCreatesOrUpdates(() -> igInstaller.install(installationSpec));
 
 	}
 

hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/packages/NpmR4Test.java

@@ -951,4 +951,6 @@ public class NpmR4Test extends BaseJpaR4Test {
 		});
 	}
 
+
+
 }

pom.xml

@@ -757,6 +757,10 @@
 			<name>Gjergj Sheldija</name>
 		</developer>
 		<developer>
+			<id>XcrigX</id>
+			<name>Craig McClendon</name>
+		</developer>
+    <developer>
 			<id>dyoung-work</id>
 		</developer>
 	</developers>