Address issues when importing US Core (#1986)

* Work on US Core import

* Work on US Core

* Resolve issues with US Core import

* Test fix

hapi-fhir-base/src/main/java/ca/uhn/fhir/util/SearchParameterUtil.java

@@ -28,6 +28,7 @@ import org.hl7.fhir.instance.model.api.IBase;
 import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.instance.model.api.IPrimitiveType;
 
+import javax.annotation.Nullable;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -49,4 +50,27 @@ public class SearchParameterUtil {
 		return retVal;
 	}
 
+	@Nullable
+	public static String getCode(FhirContext theContext, IBaseResource theResource) {
+		return getStringChild(theContext, theResource, "code");
+	}
+
+	@Nullable
+	public static String getExpression(FhirContext theFhirContext, IBaseResource theResource) {
+		return getStringChild(theFhirContext, theResource, "expression");
+	}
+
+	private static String getStringChild(FhirContext theFhirContext, IBaseResource theResource, String theChildName) {
+		Validate.notNull(theFhirContext, "theContext must not be null");
+		Validate.notNull(theResource, "theResource must not be null");
+		RuntimeResourceDefinition def = theFhirContext.getResourceDefinition(theResource);
+
+		BaseRuntimeChildDefinition base = def.getChildByName(theChildName);
+		return base
+			.getAccessor()
+			.getFirstValueOrNull(theResource)
+			.map(t -> ((IPrimitiveType<?>) t))
+			.map(t -> t.getValueAsString())
+			.orElse(null);
+	}
 }

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

@@ -90,6 +90,7 @@ import ca.uhn.fhir.validation.IValidatorModule;
 import ca.uhn.fhir.validation.ValidationOptions;
 import ca.uhn.fhir.validation.ValidationResult;
 import org.apache.commons.lang3.Validate;
+import org.apache.commons.text.WordUtils;
 import org.hl7.fhir.instance.model.api.IBaseCoding;
 import org.hl7.fhir.instance.model.api.IBaseMetaType;
 import org.hl7.fhir.instance.model.api.IBaseOperationOutcome;
@@ -123,7 +124,10 @@ import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.UUID;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 
+import static org.apache.commons.lang3.StringUtils.defaultString;
 import static org.apache.commons.lang3.StringUtils.isNotBlank;
 
 public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends BaseHapiFhirDao<T> implements IFhirResourceDao<T> {
@@ -761,8 +765,16 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
 		}
 
 		if (myDaoConfig.isMarkResourcesForReindexingUponSearchParameterChange()) {
-			if (isNotBlank(theExpression) && theExpression.contains(".")) {
+
-				final String resourceType = theExpression.substring(0, theExpression.indexOf('.'));
+			String expression = defaultString(theExpression);
+
+			Set<String> typesToMark = myDaoRegistry
+				.getRegisteredDaoTypes()
+				.stream()
+				.filter(t -> WordUtils.containsAllWords(expression, t))
+				.collect(Collectors.toSet());
+
+			for (String resourceType : typesToMark) {
 				ourLog.debug("Marking all resources of type {} for reindexing due to updated search parameter with path: {}", resourceType, theExpression);
 
 				TransactionTemplate txTemplate = new TransactionTemplate(myPlatformTransactionManager);
@@ -774,6 +786,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
 
 				ourLog.debug("Marked resources of type {} for reindexing", resourceType);
 			}
+
 		}
 
 		mySearchParamRegistry.requestRefresh();

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

@@ -89,9 +89,30 @@ public class FhirResourceDaoSearchParameterR4 extends BaseHapiFhirResourceDao<Se
 	}
 
 	public static void validateSearchParam(ISearchParamRegistry theSearchParamRegistry, ISearchParamExtractor theSearchParamExtractor, String theCode, Enum<?> theType, Enum<?> theStatus, List<? extends IPrimitiveType> theBase, String theExpression, FhirContext theContext, DaoConfig theDaoConfig) {
+		/*
+		 * If overriding built-in SPs is disabled on this server, make sure we aren't
+		 * doing that
+		 */
+		if (theDaoConfig.getModelConfig().isDefaultSearchParamsCanBeOverridden() == false) {
+			for (IPrimitiveType<?> nextBaseType : theBase) {
+				String nextBase = nextBaseType.getValueAsString();
+				RuntimeSearchParam existingSearchParam = theSearchParamRegistry.getActiveSearchParam(nextBase, theCode);
+				if (existingSearchParam != null && existingSearchParam.getId() == null) {
+					throw new UnprocessableEntityException("Can not override built-in search parameter " + nextBase + ":" + theCode + " because overriding is disabled on this server");
+				}
+			}
+		}
+
+		/*
+		 * Everything below is validating that the SP is actually valid. We'll only do that if the
+		 * SPO is active, so that we don't block people from uploading works-in-progress
+		 */
 		if (theStatus == null) {
 			throw new UnprocessableEntityException("SearchParameter.status is missing or invalid");
 		}
+		if (!theStatus.name().equals("ACTIVE")) {
+			return;
+		}
 
 		if (ElementUtil.isEmpty(theBase) && (theType == null || !Enumerations.SearchParamType.COMPOSITE.name().equals(theType.name()))) {
 			throw new UnprocessableEntityException("SearchParameter.base is missing");
@@ -150,18 +171,6 @@ public class FhirResourceDaoSearchParameterR4 extends BaseHapiFhirResourceDao<Se
 			}
 		} // if have expression
 
-		// If overriding built-in SPs is disabled on this server, make sure we aren't
-		// doing that
-		if (theDaoConfig.getModelConfig().isDefaultSearchParamsCanBeOverridden() == false) {
-			for (IPrimitiveType<?> nextBaseType : theBase) {
-				String nextBase = nextBaseType.getValueAsString();
-				RuntimeSearchParam existingSearchParam = theSearchParamRegistry.getActiveSearchParam(nextBase, theCode);
-				if (existingSearchParam != null && existingSearchParam.getId() == null) {
-					throw new UnprocessableEntityException("Can not override built-in search parameter " + nextBase + ":" + theCode + " because overriding is disabled on this server");
-				}
-			}
-		}
-
 	}
 
 }

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

@@ -33,6 +33,8 @@ 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.util.FhirTerser;
+import ca.uhn.fhir.util.SearchParameterUtil;
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.Lists;
 import com.google.gson.Gson;
 import com.google.gson.JsonElement;
@@ -55,6 +57,9 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import static org.apache.commons.lang3.StringUtils.defaultString;
+import static org.apache.commons.lang3.StringUtils.isBlank;
+
 /**
  * @since 5.1.0
  */
@@ -73,9 +78,9 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 
 	boolean enabled = true;
 	@Autowired
-	private FhirContext fhirContext;
+	private FhirContext myFhirContext;
 	@Autowired
-	private DaoRegistry daoRegistry;
+	private DaoRegistry myDaoRegistry;
 	@Autowired
 	private IValidationSupport validationSupport;
 	@Autowired
@@ -90,7 +95,7 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 
 	@PostConstruct
 	public void initialize() {
-		switch (fhirContext.getVersion().getVersion()) {
+		switch (myFhirContext.getVersion().getVersion()) {
 			case R5:
 			case R4:
 			case DSTU3:
@@ -100,7 +105,7 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 			case DSTU2_HL7ORG:
 			case DSTU2_1:
 			default: {
-				ourLog.info("IG installation not supported for version: {}", fhirContext.getVersion().getVersion());
+				ourLog.info("IG installation not supported for version: {}", myFhirContext.getVersion().getVersion());
 				enabled = false;
 			}
 		}
@@ -159,7 +164,7 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 		String version = npmPackage.getNpm().get("version").getAsString();
 
 		String fhirVersion = npmPackage.fhirVersion();
-		String currentFhirVersion = fhirContext.getVersion().getVersion().getFhirVersionString();
+		String currentFhirVersion = myFhirContext.getVersion().getVersion().getFhirVersionString();
 		assertFhirVersionsAreCompatible(fhirVersion, currentFhirVersion);
 
 		List<String> installTypes;
@@ -264,9 +269,9 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 			for (String file : filesForType) {
 				try {
 					byte[] content = pkg.getFolders().get("package").fetchFile(file);
-					resources.add(fhirContext.newJsonParser().parseResource(new String(content)));
+					resources.add(myFhirContext.newJsonParser().parseResource(new String(content)));
 				} catch (IOException e) {
-					throw new InternalErrorException("Cannot install resource of type "+type+": Could not fetch file "+ file, e);
+					throw new InternalErrorException("Cannot install resource of type " + type + ": Could not fetch file " + file, e);
 				}
 			}
 		}
@@ -277,10 +282,14 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 	 * Create a resource or update it, if its already existing.
 	 */
 	private void createOrUpdate(IBaseResource resource) {
-		IFhirResourceDao dao = daoRegistry.getResourceDao(resource.getClass());
+		IFhirResourceDao dao = myDaoRegistry.getResourceDao(resource.getClass());
 		IBundleProvider searchResult = dao.search(createSearchParameterMapFor(resource));
 		if (searchResult.isEmpty()) {
-			dao.create(resource);
+
+			if (validForUpload(resource)) {
+				dao.create(resource);
+			}
+
 		} else {
 			IBaseResource existingResource = verifySearchResultFor(resource, searchResult);
 			if (existingResource != null) {
@@ -290,8 +299,30 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 		}
 	}
 
+	boolean validForUpload(IBaseResource theResource) {
+		String resourceType = myFhirContext.getResourceType(theResource);
+		if ("SearchParameter".equals(resourceType)) {
+
+			String code = SearchParameterUtil.getCode(myFhirContext, theResource);
+			if (defaultString(code).startsWith("_")) {
+				return false;
+			}
+
+			String expression = SearchParameterUtil.getExpression(myFhirContext, theResource);
+			if (isBlank(expression)) {
+				return false;
+			}
+
+			if (SearchParameterUtil.getBaseAsStrings(myFhirContext, theResource).isEmpty()) {
+				return false;
+			}
+		}
+
+		return true;
+	}
+
 	private boolean isStructureDefinitionWithoutSnapshot(IBaseResource r) {
-		FhirTerser terser = fhirContext.newTerser();
+		FhirTerser terser = myFhirContext.newTerser();
 		return r.getClass().getSimpleName().equals("StructureDefinition") &&
 			terser.getSingleValueOrNull(r, "snapshot") == null;
 	}
@@ -319,7 +350,7 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 	}
 
 	private IBaseResource verifySearchResultFor(IBaseResource resource, IBundleProvider searchResult) {
-		FhirTerser terser = fhirContext.newTerser();
+		FhirTerser terser = myFhirContext.newTerser();
 		if (resource.getClass().getSimpleName().equals("NamingSystem")) {
 			if (searchResult.size() > 1) {
 				ourLog.warn("Expected 1 NamingSystem with unique ID {}, found {}. Will not attempt to update resource.",
@@ -346,7 +377,7 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 	}
 
 	private String extractUniqeIdFromNamingSystem(IBaseResource resource) {
-		FhirTerser terser = fhirContext.newTerser();
+		FhirTerser terser = myFhirContext.newTerser();
 		IBase uniqueIdComponent = (IBase) terser.getSingleValueOrNull(resource, "uniqueId");
 		if (uniqueIdComponent == null) {
 			throw new ImplementationGuideInstallationException("NamingSystem does not have uniqueId component.");
@@ -356,17 +387,22 @@ public class PackageInstallerSvcImpl implements IPackageInstallerSvc {
 	}
 
 	private String extractIdFromSubscription(IBaseResource resource) {
-		FhirTerser terser = fhirContext.newTerser();
+		FhirTerser terser = myFhirContext.newTerser();
 		IPrimitiveType asPrimitiveType = (IPrimitiveType) terser.getSingleValueOrNull(resource, "id");
 		return (String) asPrimitiveType.getValue();
 	}
 
 	private String extractUniqueUrlFromMetadataResouce(IBaseResource resource) {
-		FhirTerser terser = fhirContext.newTerser();
+		FhirTerser terser = myFhirContext.newTerser();
 		IPrimitiveType asPrimitiveType = (IPrimitiveType) terser.getSingleValueOrNull(resource, "url");
 		return (String) asPrimitiveType.getValue();
 	}
 
+	@VisibleForTesting
+	void setFhirContextForUnitTest(FhirContext theCtx) {
+		myFhirContext = theCtx;
+	}
+
 	private static IBaseResource getFirstResourceFrom(IBundleProvider searchResult) {
 		try {
 			return searchResult.getResources(0, 0).get(0);

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseTermReadSvcImpl.java

@@ -1605,6 +1605,9 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc {
 							if (element.hasTarget()) {
 								TermConceptMapGroupElementTarget termConceptMapGroupElementTarget;
 								for (ConceptMap.TargetElementComponent elementTarget : element.getTarget()) {
+									if (isBlank(elementTarget.getCode())) {
+										continue;
+									}
 									termConceptMapGroupElementTarget = new TermConceptMapGroupElementTarget();
 									termConceptMapGroupElementTarget.setConceptMapGroupElement(termConceptMapGroupElement);
 									termConceptMapGroupElementTarget.setCode(elementTarget.getCode());

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

@@ -1086,6 +1086,51 @@ public class FhirResourceDaoR4ConceptMapTest extends BaseJpaR4Test {
 	}
 
 
+	/**
+	 * Handle ConceptMaps where targets are missing, such as this one:
+	 *
+	 * https://www.hl7.org/fhir/conceptmap-example-specimen-type.html
+	 */
+	@Test
+	public void testUploadConceptMapWithMappingTargetsMissing() {
+
+		ConceptMap cm = new ConceptMap();
+		cm.setUrl("http://foo");
+		cm.setSource(new CanonicalType("http://source"));
+		cm.setTarget(new CanonicalType("http://target"));
+		cm.addGroup().addElement().setCode("source1").addTarget().setCode("target1").setEquivalence(ConceptMapEquivalence.EQUAL);
+		cm.addGroup().addElement().setCode("source2"); // no target
+		cm.addGroup().addElement().setCode("source3").addTarget().setComment("No target code"); // no target code
+		myConceptMapDao.create(cm);
+
+		runInTransaction(()->{
+			TranslationRequest translationRequest = new TranslationRequest();
+			translationRequest.getCodeableConcept().addCoding()
+				.setSystem("http://source")
+				.setCode("source1");
+			translationRequest.setTarget(new UriType("http://target"));
+
+			ourLog.info("*** About to translate");
+			TranslationResult translationResult = myConceptMapDao.translate(translationRequest, null);
+			ourLog.info("*** Done translating");
+
+			assertTrue(translationResult.getResult().booleanValue());
+			assertEquals("Matches found!", translationResult.getMessage().getValueAsString());
+
+			assertEquals(1, translationResult.getMatches().size());
+
+			TranslationMatch translationMatch = translationResult.getMatches().get(0);
+			assertEquals("equal", translationMatch.getEquivalence().getCode());
+			Coding concept = translationMatch.getConcept();
+			assertEquals("target1", concept.getCode());
+			assertEquals(null, concept.getDisplay());
+			assertEquals("http://target", concept.getSystem());
+		});
+
+
+	}
+
+
 
 	@Test
 	public void testUploadAndApplyR4DemoConceptMap() throws IOException {

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

@@ -91,6 +91,46 @@ public class FhirResourceDaoR4SearchCustomSearchParamTest extends BaseJpaR4Test
 	public void beforeDisableResultReuse() {
 		myDaoConfig.setReuseCachedSearchResultsForMillis(null);
 		myModelConfig.setDefaultSearchParamsCanBeOverridden(new ModelConfig().isDefaultSearchParamsCanBeOverridden());
+		myDaoConfig.setMarkResourcesForReindexingUponSearchParameterChange(new DaoConfig().isMarkResourcesForReindexingUponSearchParameterChange());
+	}
+
+	@Test
+	public void testStoreSearchParamWithBracketsInExpression() {
+		myDaoConfig.setMarkResourcesForReindexingUponSearchParameterChange(true);
+
+		SearchParameter fooSp = new SearchParameter();
+		fooSp.setCode("foo");
+		fooSp.addBase("ActivityDefinition");
+		fooSp.setType(Enumerations.SearchParamType.REFERENCE);
+		fooSp.setTitle("FOO SP");
+		fooSp.setExpression("(ActivityDefinition.useContext.value as Quantity) | (ActivityDefinition.useContext.value as Range)");
+		fooSp.setXpathUsage(org.hl7.fhir.r4.model.SearchParameter.XPathUsageType.NORMAL);
+		fooSp.setStatus(org.hl7.fhir.r4.model.Enumerations.PublicationStatus.ACTIVE);
+
+		// Ensure that no exceptions are thrown
+		mySearchParameterDao.create(fooSp, mySrd);
+		mySearchParamRegistry.forceRefresh();
+	}
+
+	/**
+	 * Draft search parameters should be ok even if they aren't completely valid
+	 */
+	@Test
+	public void testStoreDraftSearchParam_DontValidate() {
+		myDaoConfig.setMarkResourcesForReindexingUponSearchParameterChange(true);
+
+		SearchParameter fooSp = new SearchParameter();
+		fooSp.setCode("foo");
+		fooSp.addBase("ActivityDefinition");
+		fooSp.setType(Enumerations.SearchParamType.REFERENCE);
+		fooSp.setTitle("FOO SP");
+		fooSp.setExpression("FOO FOO FOO");
+		fooSp.setXpathUsage(org.hl7.fhir.r4.model.SearchParameter.XPathUsageType.NORMAL);
+		fooSp.setStatus(Enumerations.PublicationStatus.DRAFT);
+
+		// Ensure that no exceptions are thrown
+		mySearchParameterDao.create(fooSp, mySrd);
+		mySearchParamRegistry.forceRefresh();
 	}
 
 

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

@@ -27,6 +27,7 @@ import org.hl7.fhir.r4.model.StructureDefinition;
 import org.hl7.fhir.utilities.cache.NpmPackage;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -95,6 +96,18 @@ public class NpmTestR4 extends BaseJpaR4Test {
 	}
 
 
+	@Disabled("This test is super slow so don't run by default")
+	@Test
+	public void testInstallUsCore() {
+		JpaPackageCache jpaPackageCache = ProxyUtil.getSingletonTarget(myPackageCacheManager, JpaPackageCache.class);
+		jpaPackageCache.getPackageServers().clear();
+		jpaPackageCache.addPackageServer("https://packages.fhir.org");
+
+		PackageInstallationSpec spec = new PackageInstallationSpec().setName("hl7.fhir.us.core").setVersion("3.1.0").setInstallMode(PackageInstallationSpec.InstallModeEnum.STORE_AND_INSTALL).setFetchDependencies(true);
+		igInstaller.install(spec);
+	}
+
+
 	@Test
 	public void testCacheDstu3Package() throws Exception {
 		byte[] bytes = loadClasspathBytes("/packages/nictiz.fhir.nl.stu3.questionnaires-1.0.2.tgz");

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

@@ -0,0 +1,60 @@
+package ca.uhn.fhir.jpa.packages;
+
+import ca.uhn.fhir.context.FhirContext;
+import ca.uhn.fhir.context.FhirVersionEnum;
+import org.hl7.fhir.r4.model.Enumerations;
+import org.hl7.fhir.r4.model.SearchParameter;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class PackageInstallerSvcImplTest {
+	private final FhirContext myCtx = FhirContext.forCached(FhirVersionEnum.R4);
+	private PackageInstallerSvcImpl mySvc;
+
+
+	@BeforeEach
+	public void before() {
+		mySvc = new PackageInstallerSvcImpl();
+		mySvc.setFhirContextForUnitTest(myCtx);
+	}
+
+	@Test
+	public void testValidForUpload_SearchParameterWithMetaParam() {
+		SearchParameter sp = new SearchParameter();
+		sp.setCode("_id");
+		assertFalse(mySvc.validForUpload(sp));
+	}
+
+	@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));
+	}
+
+	@Test
+	public void testValidForUpload_SearchParameterWithNoExpression() {
+		SearchParameter sp = new SearchParameter();
+		sp.setCode("name");
+		sp.addBase("Patient");
+		sp.setStatus(Enumerations.PublicationStatus.ACTIVE);
+		assertFalse(mySvc.validForUpload(sp));
+	}
+
+
+	@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));
+	}
+
+}