Validate include statements for JPA (#3331)

* Validate include statements for JPA

* Add changelog

* Optmize test fix

hapi-fhir-base/src/main/java/ca/uhn/fhir/model/api/Include.java

@@ -1,5 +1,6 @@
 package ca.uhn.fhir.model.api;
 
+import static org.apache.commons.lang3.StringUtils.defaultString;
 import static org.apache.commons.lang3.StringUtils.isBlank;
 import static org.apache.commons.lang3.StringUtils.isNotBlank;
 
@@ -34,6 +35,9 @@
  * equality. Prior to HAPI 1.2 (and FHIR DSTU2) the recurse property did not exist, so this may merit consideration when
  * upgrading servers.
  * </p>
+ * <p>
+ * Note on thrwead safety: This class is not thread safe.
+ * </p>
  */
 public class Include implements Serializable {
 
@@ -42,6 +46,9 @@ public class Include implements Serializable {
 	private final boolean myImmutable;
 	private boolean myIterate;
 	private String myValue;
+	private String myParamType;
+	private String myParamName;
+	private String myParamTargetType;
 
 	/**
 	 * Constructor for <b>non-recursive</b> include
@@ -50,8 +57,7 @@ public class Include implements Serializable {
 	 *           The <code>_include</code> value, e.g. "Patient:name"
 	 */
 	public Include(String theValue) {
-		myValue = theValue;
-		myImmutable = false;
+		this(theValue, false);
 	}
 
 	/**
@@ -63,9 +69,7 @@ public Include(String theValue) {
 	 *           Should the include recurse
 	 */
 	public Include(String theValue, boolean theIterate) {
-		myValue = theValue;
-		myIterate = theIterate;
-		myImmutable = false;
+		this(theValue, theIterate, false);
 	}
 
 	/**
@@ -77,7 +81,7 @@ public Include(String theValue, boolean theIterate) {
 	 *           Should the include recurse
 	 */
 	public Include(String theValue, boolean theIterate, boolean theImmutable) {
-		myValue = theValue;
+		setValue(theValue);
 		myIterate = theIterate;
 		myImmutable = theImmutable;
 	}
@@ -128,41 +132,21 @@ public boolean equals(Object obj) {
 	 * Returns the portion of the value before the first colon
 	 */
 	public String getParamType() {
-		int firstColon = myValue.indexOf(':');
-		if (firstColon == -1 || firstColon == myValue.length() - 1) {
-			return null;
-		}
-		return myValue.substring(0, firstColon);
+		return myParamType;
 	}
 
 	/**
 	 * Returns the portion of the value after the first colon but before the second colon
 	 */
 	public String getParamName() {
-		int firstColon = myValue.indexOf(':');
-		if (firstColon == -1 || firstColon == myValue.length() - 1) {
-			return null;
-		}
-		int secondColon = myValue.indexOf(':', firstColon + 1);
-		if (secondColon != -1) {
-			return myValue.substring(firstColon + 1, secondColon);
-		}
-		return myValue.substring(firstColon + 1);
+		return myParamName;
 	}
 
 	/**
 	 * Returns the portion of the string after the second colon, or null if there are not two colons in the value.
 	 */
 	public String getParamTargetType() {
-		int firstColon = myValue.indexOf(':');
-		if (firstColon == -1 || firstColon == myValue.length() - 1) {
-			return null;
-		}
-		int secondColon = myValue.indexOf(':', firstColon + 1);
-		if (secondColon != -1) {
-			return myValue.substring(secondColon + 1);
-		}
-		return null;
+		return myParamTargetType;
 
 	}
 
@@ -207,7 +191,34 @@ public void setValue(String theValue) {
 		if (myImmutable) {
 			throw new IllegalStateException("Can not change the value of this include");
 		}
+
+		String value = defaultString(theValue);
+
+		int firstColon = value.indexOf(':');
+		String paramType;
+		String paramName;
+		String paramTargetType;
+		if (firstColon == -1 || firstColon == value.length() - 1) {
+			paramType = null;
+			paramName = null;
+			paramTargetType = null;
+		} else {
+			paramType = value.substring(0, firstColon);
+			int secondColon = value.indexOf(':', firstColon + 1);
+			if (secondColon == -1) {
+				paramName = value.substring(firstColon + 1);
+				paramTargetType = null;
+			} else {
+				paramName =  value.substring(firstColon + 1, secondColon);
+				paramTargetType = value.substring(secondColon + 1);
+			}
+		}
+
+		myParamType = paramType;
+		myParamName = paramName;
+		myParamTargetType = paramTargetType;
 		myValue = theValue;
+
 	}
 
 	/**

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/api/Constants.java

@@ -290,6 +290,7 @@ public class Constants {
 	public static final String SUBSCRIPTION_MULTITYPE_SUFFIX = "]";
 	public static final String SUBSCRIPTION_MULTITYPE_STAR = "*";
 	public static final String SUBSCRIPTION_STAR_CRITERIA = SUBSCRIPTION_MULTITYPE_PREFIX + SUBSCRIPTION_MULTITYPE_STAR + SUBSCRIPTION_MULTITYPE_SUFFIX;
+	public static final String INCLUDE_STAR = "*";
 
 	static {
 		CHARSET_UTF8 = StandardCharsets.UTF_8;

hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties

@@ -121,6 +121,7 @@ ca.uhn.fhir.jpa.patch.FhirPatch.invalidMoveSourceIndex=Invalid move source index
 ca.uhn.fhir.jpa.patch.FhirPatch.invalidMoveDestinationIndex=Invalid move destination index {0} for path {1} - Only have {2} existing entries
 ca.uhn.fhir.jpa.searchparam.extractor.BaseSearchParamExtractor.externalReferenceNotAllowed=Resource contains external reference to URL "{0}" but this server is not configured to allow external references
 ca.uhn.fhir.jpa.searchparam.extractor.BaseSearchParamExtractor.failedToExtractPaths=Failed to extract values from resource using FHIRPath "{0}": {1}
+ca.uhn.fhir.jpa.search.SearchCoordinatorSvcImpl.invalidInclude=Invalid {0} parameter value: "{1}". {2}
 ca.uhn.fhir.jpa.dao.LegacySearchBuilder.invalidQuantityPrefix=Unable to handle quantity prefix "{0}" for value: {1}
 ca.uhn.fhir.jpa.dao.LegacySearchBuilder.invalidNumberPrefix=Unable to handle number prefix "{0}" for value: {1}
 ca.uhn.fhir.jpa.dao.LegacySearchBuilder.sourceParamDisabled=The _source parameter is disabled on this server

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3331-validate-includes-for-jpa.yaml

@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 3331
+title: "The JPA server will now validate `_include` and `_revinclude` statements before beginning
+  search processing. This prevents an ugly JPA error when some invalid params are used."