Upgrade core to 5.6.97, make adjustments in hapi-fhir, and ensure tha…

…t all tests pass (#4579)

* First commit:  Create new branch from the release branch with changes from James' branch.  This probably won't compile as the work is incomplete.

* Second round of changes from integrating James' branch.

* Mark most test failures with TODOs.

* Add whitespace

* Add changes to FhirPathR4 to set FHIRPathEngine to non-strict FP evaluation.

* Fix CreatePackageCommandTest to assert null instead of empty string.  Comments on tests that fail due to the double-quote encoding problem.

* Downgrade to core 5.6.97.

* Fix another test and remove TODOs.

* Fix changelog.

* Clean up some erroneous changes and TODOs.

---------

Co-authored-by: Tadgh <[email protected]>

hapi-fhir-base/src/main/java/ca/uhn/fhir/context/support/DefaultProfileValidationSupport.java

@@ -133,6 +133,15 @@ private void initializeResourceLists() {
  structureDefinitionResources.add("/org/hl7/fhir/r4/model/profile/profiles-others.xml");
  structureDefinitionResources.add("/org/hl7/fhir/r4/model/extension/extension-definitions.xml");
  break;
+ case R4B:
+ terminologyResources.add("/org/hl7/fhir/r4b/model/valueset/valuesets.xml");
+ terminologyResources.add("/org/hl7/fhir/r4b/model/valueset/v2-tables.xml");
+ terminologyResources.add("/org/hl7/fhir/r4b/model/valueset/v3-codesystems.xml");
+ structureDefinitionResources.add("/org/hl7/fhir/r4b/model/profile/profiles-resources.xml");
+ structureDefinitionResources.add("/org/hl7/fhir/r4b/model/profile/profiles-types.xml");
+ structureDefinitionResources.add("/org/hl7/fhir/r4b/model/profile/profiles-others.xml");
+ structureDefinitionResources.add("/org/hl7/fhir/r4b/model/extension/extension-definitions.xml");
+ break;
  case R5:
  structureDefinitionResources.add("/org/hl7/fhir/r5/model/profile/profiles-resources.xml");
  structureDefinitionResources.add("/org/hl7/fhir/r5/model/profile/profiles-types.xml");
@@ -221,15 +230,41 @@ private IBaseResource fetchCodeSystemOrValueSet(String theSystem, boolean codeSy
  @Override
  public IBaseResource fetchStructureDefinition(String theUrl) {
  String url = theUrl;
- if (url.startsWith(URL_PREFIX_STRUCTURE_DEFINITION)) {
- // no change
- } else if (url.indexOf('/') == -1) {
- url = URL_PREFIX_STRUCTURE_DEFINITION + url;
- } else if (StringUtils.countMatches(url, '/') == 1) {
- url = URL_PREFIX_STRUCTURE_DEFINITION_BASE + url;
+ if (!url.startsWith(URL_PREFIX_STRUCTURE_DEFINITION)) {
+ if (url.indexOf('/') == -1) {
+  url = URL_PREFIX_STRUCTURE_DEFINITION + url;
+ } else if (StringUtils.countMatches(url, '/') == 1) {
+  url = URL_PREFIX_STRUCTURE_DEFINITION_BASE + url;
+ }
  }
  Map<String, IBaseResource> structureDefinitionMap = provideStructureDefinitionMap();
- return structureDefinitionMap.get(url);
+ IBaseResource retVal = structureDefinitionMap.get(url);
+ if (retVal == null) {
+
+ if (url.startsWith(URL_PREFIX_STRUCTURE_DEFINITION)) {
+
+ /*
+ * A few built-in R4 SearchParameters have the wrong casing for primitive
+ * search parameters eg "value.as(String)" when it should be
+ * "value.as(string)". This lets us be a bit lenient about this.
+ */
+ if (myCtx.getVersion().getVersion() == FhirVersionEnum.R4 || myCtx.getVersion().getVersion() == FhirVersionEnum.R4B || myCtx.getVersion().getVersion() == FhirVersionEnum.R5) {
+ String end = url.substring(URL_PREFIX_STRUCTURE_DEFINITION.length());
+ if (Character.isUpperCase(end.charAt(0))) {
+ String newEnd = Character.toLowerCase(end.charAt(0)) + end.substring(1);
+ String alternateUrl = URL_PREFIX_STRUCTURE_DEFINITION + newEnd;
+ retVal = structureDefinitionMap.get(alternateUrl);
+ if (retVal != null) {
+ retVal = myCtx.newTerser().clone(retVal);
+ myCtx.newTerser().setElement(retVal, "type", end);
+ }
+ }
+ }
+
+ }
+
+ }
+ return retVal;
  }
 
  @Override

hapi-fhir-bom/pom.xml

@@ -237,6 +237,11 @@
  <artifactId>hapi-fhir-validation-resources-r4</artifactId>
  <version>${project.version}</version>
  </dependency>
+ <dependency>
+ <groupId>${project.groupId}</groupId>
+ <artifactId>hapi-fhir-validation-resources-r4b</artifactId>
+ <version>${project.version}</version>
+ </dependency>
  <dependency>
  <groupId>${project.groupId}</groupId>
  <artifactId>hapi-fhir-validation-resources-r5</artifactId>

hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/CreatePackageCommandTest.java

@@ -103,7 +103,7 @@ public void testCreatePackage() throws IOException {
  {
  "name" : "com.example.ig",
  "version" : "1.0.1",
- "description" : "",
+ "description" : null,
  "fhirVersions" : ["4.0.1"],
  "dependencies" : {
  "hl7.fhir.core" : "4.0.1",
@@ -158,7 +158,7 @@ public void testCreatePackage_NoDependencies() throws IOException {
  {
  "name" : "com.example.ig",
  "version" : "1.0.1",
- "description" : "",
+ "description" : null,
  "fhirVersions" : ["4.0.1"]
  }
  """;

hapi-fhir-client/src/main/java/ca/uhn/fhir/rest/client/interceptor/LoggingInterceptor.java

@@ -197,43 +197,49 @@ public void setLogger(Logger theLogger) {
  /**
  * Should a summary (one line) for each request be logged, containing the URL and other information
  */
- public void setLogRequestBody(boolean theValue) {
+ public LoggingInterceptor setLogRequestBody(boolean theValue) {
  myLogRequestBody = theValue;
+ return this;
  }
 
  /**
  * Should headers for each request be logged, containing the URL and other information
  */
- public void setLogRequestHeaders(boolean theValue) {
+ public LoggingInterceptor setLogRequestHeaders(boolean theValue) {
  myLogRequestHeaders = theValue;
+ return this;
  }
 
  /**
  * Should a summary (one line) for each request be logged, containing the URL and other information
  */
- public void setLogRequestSummary(boolean theValue) {
+ public LoggingInterceptor setLogRequestSummary(boolean theValue) {
  myLogRequestSummary = theValue;
+ return this;
  }
 
  /**
  * Should a summary (one line) for each request be logged, containing the URL and other information
  */
- public void setLogResponseBody(boolean theValue) {
+ public LoggingInterceptor setLogResponseBody(boolean theValue) {
  myLogResponseBody = theValue;
+ return this;
  }
 
  /**
  * Should headers for each request be logged, containing the URL and other information
  */
- public void setLogResponseHeaders(boolean theValue) {
+ public LoggingInterceptor setLogResponseHeaders(boolean theValue) {
  myLogResponseHeaders = theValue;
+ return this;
  }
 
  /**
  * Should a summary (one line) for each request be logged, containing the URL and other information
  */
- public void setLogResponseSummary(boolean theValue) {
+ public LoggingInterceptor setLogResponseSummary(boolean theValue) {
  myLogResponseSummary = theValue;
+ return this;
  }
 
 }

hapi-fhir-dist/pom.xml

@@ -212,6 +212,11 @@
  <artifactId>hapi-fhir-validation-resources-r4</artifactId>
  <version>${project.version}</version>
  </dependency>
+ <dependency>
+ <groupId>ca.uhn.hapi.fhir</groupId>
+ <artifactId>hapi-fhir-validation-resources-r4b</artifactId>
+ <version>${project.version}</version>
+ </dependency>
 
  <dependency>
  <groupId>ca.uhn.hapi.fhir</groupId>

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_4_0/4582-upgrade-core-to-5-6-97.yaml

@@ -0,0 +1,4 @@
+---
+type: fix
+issue: 4582
+title: "Upgrade dependency on core to 5.6.97 including hapi-fhir code enhancements and unit test fixes."

hapi-fhir-jpaserver-base/pom.xml

@@ -132,6 +132,11 @@
  <artifactId>hapi-fhir-validation-resources-r4</artifactId>
  <version>${project.version}</version>
  </dependency>
+ <dependency>
+ <groupId>ca.uhn.hapi.fhir</groupId>
+ <artifactId>hapi-fhir-validation-resources-r4b</artifactId>
+ <version>${project.version}</version>
+ </dependency>
  <dependency>
  <groupId>ca.uhn.hapi.fhir</groupId>
  <artifactId>hapi-fhir-validation-resources-r5</artifactId>

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/PackageLoaderConfig.java

@@ -24,6 +24,7 @@
 import ca.uhn.fhir.jpa.packages.loader.PackageLoaderSvc;
 import ca.uhn.fhir.jpa.packages.loader.PackageResourceParsingSvc;
 import org.hl7.fhir.utilities.npm.PackageClient;
+import org.hl7.fhir.utilities.npm.PackageServer;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 
@@ -34,8 +35,8 @@ public class PackageLoaderConfig {
  public PackageLoaderSvc packageLoaderSvc() {
  PackageLoaderSvc svc = new PackageLoaderSvc();
  svc.getPackageServers().clear();
- svc.getPackageServers().add(PackageClient.PRIMARY_SERVER);
- svc.getPackageServers().add(PackageClient.SECONDARY_SERVER);
+ svc.getPackageServers().add(PackageServer.primaryServer());
+ svc.getPackageServers().add(PackageServer.secondaryServer());
  return svc;
  }
 

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

@@ -159,7 +159,7 @@ public CodeValidationResult validateCode(IIdType theCodeSystemId, IPrimitiveType
  String codeSystemUrl;
  if (theCodeSystemId != null) {
  IBaseResource codeSystem = read(theCodeSystemId, theRequestDetails);
- codeSystemUrl = CommonCodeSystemsTerminologyService.getCodeSystemUrl(codeSystem);
+ codeSystemUrl = CommonCodeSystemsTerminologyService.getCodeSystemUrl(myFhirContext, codeSystem);
  } else if (isNotBlank(toStringValue(theCodeSystemUrl))) {
  codeSystemUrl = toStringValue(theCodeSystemUrl);
  } else {

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

@@ -176,8 +176,8 @@ public IValidationSupport.CodeValidationResult validateCode(IPrimitiveType<Strin
  String valueSetIdentifier;
  if (theValueSetId != null) {
  IBaseResource valueSet = read(theValueSetId, theRequestDetails);
- StringBuilder valueSetIdentifierBuilder = new StringBuilder(CommonCodeSystemsTerminologyService.getValueSetUrl(valueSet));
- String valueSetVersion = CommonCodeSystemsTerminologyService.getValueSetVersion(valueSet);
+ StringBuilder valueSetIdentifierBuilder = new StringBuilder(CommonCodeSystemsTerminologyService.getValueSetUrl(myFhirContext, valueSet));
+ String valueSetVersion = CommonCodeSystemsTerminologyService.getValueSetVersion(myFhirContext, valueSet);
  if (valueSetVersion != null) {
  valueSetIdentifierBuilder.append("|").append(valueSetVersion);
  }

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

@@ -60,6 +60,7 @@
 import org.hl7.fhir.instance.model.api.IPrimitiveType;
 import org.hl7.fhir.utilities.npm.BasePackageCacheManager;
 import org.hl7.fhir.utilities.npm.NpmPackage;
+import org.hl7.fhir.utilities.npm.PackageServer;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -129,9 +130,9 @@ public class JpaPackageCache extends BasePackageCacheManager implements IHapiPac
  private IBinaryStorageSvc myBinaryStorageSvc;
 
  @Override
- public void addPackageServer(@Nonnull String theUrl) {
+ public void addPackageServer(@Nonnull PackageServer thePackageServer) {
  assert myPackageLoaderSvc != null;
- myPackageLoaderSvc.addPackageServer(theUrl);
+ myPackageLoaderSvc.addPackageServer(thePackageServer);
  }
 
  @Override
@@ -150,7 +151,7 @@ public String getPackageUrl(String theS) throws IOException {
  }
 
  @Override
- public List<String> getPackageServers() {
+ public List<PackageServer> getPackageServers() {
  return myPackageLoaderSvc.getPackageServers();
  }
 

hapi-fhir-jpaserver-searchparam/pom.xml

@@ -92,6 +92,11 @@
  <artifactId>hapi-fhir-validation-resources-r4</artifactId>
  <version>${project.version}</version>
  </dependency>
+ <dependency>
+ <groupId>ca.uhn.hapi.fhir</groupId>
+ <artifactId>hapi-fhir-validation-resources-r4b</artifactId>
+ <version>${project.version}</version>
+ </dependency>
  <dependency>
  <groupId>ca.uhn.hapi.fhir</groupId>
  <artifactId>hapi-fhir-validation-resources-r5</artifactId>

hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java

@@ -99,8 +99,10 @@
 import static ca.uhn.fhir.rest.api.RestSearchParameterTypeEnum.REFERENCE;
 import static org.apache.commons.lang3.StringUtils.isBlank;
 import static org.apache.commons.lang3.StringUtils.isNotBlank;
+import static org.apache.commons.lang3.StringUtils.startsWith;
 import static org.apache.commons.lang3.StringUtils.trim;
 
+// TODO: come back to this and scrutinize the changes James did to this class
 public abstract class BaseSearchParamExtractor implements ISearchParamExtractor {
 
  public static final Set<String> COORDS_INDEX_PATHS;
@@ -1300,7 +1302,30 @@ private void addString_Address(String theResourceType, Set<ResourceIndexedSearch
 
  }
 
- private <T> SearchParamSet<T> extractSearchParams(IBaseResource theResource, IExtractor<T> theExtractor, RestSearchParameterTypeEnum theSearchParamType, boolean theWantLocalReferences) {
+ /**
+ * Ignore any of the Resource-level search params. This is kind of awkward, but here is why
+ * we do it:
+ * <p>
+ * The ReadOnlySearchParamCache supplies these params, and they have paths associated with
+ * them. E.g. HAPI's SearchParamRegistryImpl will know about the _id search parameter and
+ * assigns it the path "Resource.id". All of these parameters have indexing code paths in the
+ * server that don't rely on the existence of the SearchParameter. For example, we have a
+ * dedicated column on ResourceTable that handles the _id parameter.
+ * <p>
+ * Until 6.2.0 the FhirPath evaluator didn't actually resolve any values for these paths
+ * that started with Resource instead of the actual resource name, so it never actually
+ * made a difference that these parameters existed because they'd never actually result
+ * in any index rows. In 6.4.0 that bug was fixed in the core FhirPath engine. We don't
+ * want that fix to result in pointless index rows for things like _id and _tag, so we
+ * ignore them here.
+ * <p>
+ * Note that you can still create a search parameter that includes a path like
+ * "meta.tag" if you really need to create an SP that actually does index _tag. This
+ * is needed if you want to search for tags in <code>INLINE</code> tag storage mode.
+ * This is the only way you could actually specify a FhirPath expression for those
+ * prior to 6.2.0 so this isn't a breaking change.
+ */
+ <T> SearchParamSet<T> extractSearchParams(IBaseResource theResource, IExtractor<T> theExtractor, RestSearchParameterTypeEnum theSearchParamType, boolean theWantLocalReferences) {
  SearchParamSet<T> retVal = new SearchParamSet<>();
 
  Collection<RuntimeSearchParam> searchParams = getSearchParams(theResource);
@@ -1312,6 +1337,11 @@ private <T> SearchParamSet<T> extractSearchParams(IBaseResource theResource, IEx
  continue;
  }
 
+ // See the method javadoc for an explanation of this
+ if (startsWith(nextSpDef.getPath(), "Resource.")) {
+ continue;
+ }
+
  extractSearchParam(nextSpDef, theResource, theExtractor, retVal, theWantLocalReferences);
  }
  return retVal;

hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/SearchParamExtractorR4.java

@@ -32,6 +32,7 @@
 import org.hl7.fhir.instance.model.api.IBase;
 import org.hl7.fhir.r4.context.IWorkerContext;
 import org.hl7.fhir.r4.hapi.ctx.HapiWorkerContext;
+import org.hl7.fhir.r4.utils.FHIRPathEngine;
 import org.hl7.fhir.r4.model.Base;
 import org.hl7.fhir.r4.model.ExpressionNode;
 import org.hl7.fhir.r4.model.IdType;
@@ -133,7 +134,7 @@ public List<Base> executeFunction(Object appContext, List<Base> focus, String fu
 
 
  @Override
- public Base resolveReference(Object theAppContext, String theUrl) throws FHIRException {
+ public Base resolveReference(Object theAppContext, String theUrl, Base theRefContext) throws FHIRException {
 
  /*
  * When we're doing resolution within the SearchParamExtractor, if we want

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

@@ -95,6 +95,8 @@ public static ReadOnlySearchParamCache fromFhirContext(@Nonnull FhirContext theF
  IBaseBundle allSearchParameterBundle = null;
  if (theFhirContext.getVersion().getVersion() == FhirVersionEnum.R4) {
  allSearchParameterBundle = (IBaseBundle) theFhirContext.newJsonParser().parseResource(ClasspathUtil.loadResourceAsStream("org/hl7/fhir/r4/model/sp/search-parameters.json"));
+ } else if (theFhirContext.getVersion().getVersion() == FhirVersionEnum.R4B) {
+ allSearchParameterBundle = (IBaseBundle) theFhirContext.newXmlParser().parseResource(ClasspathUtil.loadResourceAsStream("org/hl7/fhir/r4b/model/sp/search-parameters.xml"));
  } else if (theFhirContext.getVersion().getVersion() == FhirVersionEnum.R5) {
  allSearchParameterBundle = (IBaseBundle) theFhirContext.newXmlParser().parseResource(ClasspathUtil.loadResourceAsStream("org/hl7/fhir/r5/model/sp/search-parameters.xml"));
  }

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

@@ -79,6 +79,7 @@ public RuntimeSearchParam canonicalizeSearchParameter(IBaseResource theSearchPar
  retVal = canonicalizeSearchParameterDstu3((org.hl7.fhir.dstu3.model.SearchParameter) theSearchParameter);
  break;
  case R4:
+ case R4B:
  case R5:
  retVal = canonicalizeSearchParameterR4Plus(theSearchParameter);
  break;

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

@@ -20,6 +20,7 @@
 import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.r4.model.Patient;
 import org.hl7.fhir.utilities.npm.IPackageCacheManager;
+import org.hl7.fhir.utilities.npm.PackageServer;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -75,7 +76,7 @@ public void before() throws Exception {
 
  myPort = JettyUtil.getPortForStartedServer(myServer);
  jpaPackageCache.getPackageServers().clear();
- jpaPackageCache.addPackageServer("http:https://localhost:" + myPort);
+ jpaPackageCache.addPackageServer(new PackageServer("http:https://localhost:" + myPort));
 
  myFakeNpmServlet.getResponses().clear();
  }

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

@@ -15,6 +15,7 @@
 import org.hl7.fhir.dstu3.model.OperationOutcome;
 import org.hl7.fhir.dstu3.model.StructureDefinition;
 import org.hl7.fhir.dstu3.model.ValueSet;
+import org.hl7.fhir.utilities.npm.PackageServer;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -61,7 +62,7 @@ public void before() throws Exception {
 
  int port = JettyUtil.getPortForStartedServer(myServer);
  jpaPackageCache.getPackageServers().clear();
- jpaPackageCache.addPackageServer("http:https://localhost:" + port);
+ jpaPackageCache.addPackageServer(new PackageServer("http:https://localhost:" + port));
 
  myResponses.clear();
  }