Use FHIRPath expression parser for custom SP validation

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

@@ -404,8 +404,8 @@ class ModelScanner {
 				if (paramType == null) {
 					throw new ConfigurationException("Search param " + searchParam.name() + " has an invalid type: " + searchParam.type());
 				}
-				Set<String> providesMembershipInCompartments = null;
+				Set<String> providesMembershipInCompartments;
-				providesMembershipInCompartments = new HashSet<String>();
+				providesMembershipInCompartments = new HashSet<>();
 				for (Compartment next : searchParam.providesMembershipIn()) {
 					if (paramType != RestSearchParameterTypeEnum.REFERENCE) {
 						StringBuilder b = new StringBuilder();
@@ -427,7 +427,8 @@ class ModelScanner {
 				}
 
 
-				RuntimeSearchParam param = new RuntimeSearchParam(searchParam.name(), searchParam.description(), searchParam.path(), paramType, providesMembershipInCompartments, toTargetList(searchParam.target()), RuntimeSearchParamStatusEnum.ACTIVE);
+				Collection<String> base = Collections.singletonList(theResourceDef.getName());
+				RuntimeSearchParam param = new RuntimeSearchParam(null, null, searchParam.name(), searchParam.description(), searchParam.path(), paramType, null, providesMembershipInCompartments, toTargetList(searchParam.target()), RuntimeSearchParamStatusEnum.ACTIVE, base);
 				theResourceDef.addSearchParam(param);
 				nameToParam.put(param.getName(), param);
 			}

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

@@ -2,6 +2,7 @@ package ca.uhn.fhir.jpa.dao.r4;
 
 import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.context.FhirVersionEnum;
+import ca.uhn.fhir.fluentpath.IFluentPath;
 import ca.uhn.fhir.jpa.dao.BaseSearchParamExtractor;
 import ca.uhn.fhir.jpa.dao.DaoConfig;
 import ca.uhn.fhir.jpa.dao.IFhirResourceDaoSearchParameter;
@@ -13,7 +14,11 @@ import ca.uhn.fhir.util.ElementUtil;
 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 org.hl7.fhir.r4.hapi.ctx.DefaultProfileValidationSupport;
+import org.hl7.fhir.r4.hapi.ctx.HapiWorkerContext;
 import org.hl7.fhir.r4.model.*;
+import org.hl7.fhir.r4.utils.FHIRLexer;
+import org.hl7.fhir.r4.utils.FHIRPathEngine;
 import org.springframework.beans.factory.annotation.Autowired;
 
 import java.util.List;
@@ -29,9 +34,9 @@ import static org.apache.commons.lang3.StringUtils.isBlank;
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  * http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -42,6 +47,7 @@ import static org.apache.commons.lang3.StringUtils.isBlank;
 
 public class FhirResourceDaoSearchParameterR4 extends FhirResourceDaoR4<SearchParameter> implements IFhirResourceDaoSearchParameter<SearchParameter> {
 
+	public static final DefaultProfileValidationSupport VALIDATION_SUPPORT = new DefaultProfileValidationSupport();
 	@Autowired
 	private IFhirSystemDao<Bundle, Meta> mySystemDao;
 
@@ -88,7 +94,7 @@ public class FhirResourceDaoSearchParameterR4 extends FhirResourceDaoR4<SearchPa
 			throw new UnprocessableEntityException("SearchParameter.status is missing or invalid");
 		}
 
-		if (ElementUtil.isEmpty(theBase)) {
+		if (ElementUtil.isEmpty(theBase) && (theType == null || !Enumerations.SearchParamType.COMPOSITE.name().equals(theType.name()))) {
 			throw new UnprocessableEntityException("SearchParameter.base is missing");
 		}
 
@@ -102,35 +108,11 @@ public class FhirResourceDaoSearchParameterR4 extends FhirResourceDaoR4<SearchPa
 
 		} else {
 
-			theExpression = theExpression.trim();
+			FHIRPathEngine fhirPathEngine = new FHIRPathEngine(new HapiWorkerContext(theContext, VALIDATION_SUPPORT));
-
+			try {
-			String[] expressionSplit = BaseSearchParamExtractor.SPLIT.split(theExpression);
+				fhirPathEngine.parse(theExpression);
-			for (String nextPath : expressionSplit) {
+			} catch (FHIRLexer.FHIRLexerException e) {
-				nextPath = nextPath.trim();
+				throw new UnprocessableEntityException("Invalid SearchParameter.expression value \"" + theExpression + "\": " + e.getMessage());
-
-				int dotIdx = nextPath.indexOf('.');
-				if (dotIdx == -1) {
-					throw new UnprocessableEntityException("Invalid SearchParameter.expression value \"" + nextPath + "\". Must start with a resource name");
-				}
-
-				String resourceName = nextPath.substring(0, dotIdx);
-				try {
-					theContext.getResourceDefinition(resourceName);
-				} catch (DataFormatException e) {
-					throw new UnprocessableEntityException("Invalid SearchParameter.expression value \"" + nextPath + "\": " + e.getMessage());
-				}
-
-				if (theContext.getVersion().getVersion().isEqualOrNewerThan(FhirVersionEnum.DSTU3)) {
-					if (theDaoConfig.isValidateSearchParameterExpressionsOnSave()) {
-						IBaseResource temporaryInstance = theContext.getResourceDefinition(resourceName).newInstance();
-						try {
-							theContext.newFluentPath().evaluate(temporaryInstance, nextPath, IBase.class);
-						} catch (Exception e) {
-							String msg = theContext.getLocalizer().getMessage(FhirResourceDaoSearchParameterR4.class, "invalidSearchParamExpression", nextPath, e.getMessage());
-							throw new UnprocessableEntityException(msg, e);
-						}
-					}
-				}
 			}
 
 		} // if have expression

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

@@ -0,0 +1,80 @@
+package ca.uhn.fhir.jpa.dao.r4;
+
+import ca.uhn.fhir.context.FhirContext;
+import ca.uhn.fhir.context.RuntimeResourceDefinition;
+import ca.uhn.fhir.context.RuntimeSearchParam;
+import ca.uhn.fhir.jpa.dao.DaoConfig;
+import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
+import org.hl7.fhir.r4.model.Enumerations;
+import org.hl7.fhir.r4.model.SearchParameter;
+import org.junit.Before;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.commons.lang3.StringUtils.isBlank;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+public class FhirResourceDaoSearchParameterR4Test {
+
+	private static final Logger ourLog = LoggerFactory.getLogger(FhirResourceDaoSearchParameterR4Test.class);
+	private FhirContext myCtx;
+	private FhirResourceDaoSearchParameterR4 myDao;
+
+	@Before
+	public void before() {
+		myCtx = FhirContext.forR4();
+		myDao = new FhirResourceDaoSearchParameterR4();
+		myDao.setContext(myCtx);
+		myDao.setConfig(new DaoConfig());
+	}
+
+	@Test
+	public void testValidateAllBuiltInSearchParams() {
+
+		for (String nextResource : myCtx.getResourceNames()) {
+			RuntimeResourceDefinition nextResDef = myCtx.getResourceDefinition(nextResource);
+			for (RuntimeSearchParam nextp : nextResDef.getSearchParams()) {
+				if (nextp.getName().equals("_id")) {
+					continue;
+				}
+				if (nextp.getName().equals("_language")) {
+					continue;
+				}
+				if (isBlank(nextp.getPath())) {
+					continue;
+				}
+
+				SearchParameter nextSearchParameter = new SearchParameter();
+				nextSearchParameter.setExpression(nextp.getPath());
+				nextSearchParameter.setStatus(Enumerations.PublicationStatus.ACTIVE);
+				nextSearchParameter.setType(Enumerations.SearchParamType.fromCode(nextp.getParamType().getCode()));
+				nextp.getBase().forEach(t -> nextSearchParameter.addBase(t));
+
+				ourLog.info("Validating {}.{}", nextResource, nextp.getName());
+				myDao.validateResourceForStorage(nextSearchParameter, null);
+			}
+		}
+
+
+	}
+
+
+	@Test
+	public void testValidateInvalidExpression() {
+		SearchParameter nextSearchParameter = new SearchParameter();
+		nextSearchParameter.setExpression("Patient////");
+		nextSearchParameter.setStatus(Enumerations.PublicationStatus.ACTIVE);
+		nextSearchParameter.setType(Enumerations.SearchParamType.STRING);
+		nextSearchParameter.addBase("Patient");
+
+		try {
+			myDao.validateResourceForStorage(nextSearchParameter, null);
+			fail();
+		} catch (UnprocessableEntityException e) {
+			assertEquals("Invalid SearchParameter.expression value \"Patient////\": Error at 1, 1: Premature ExpressionNode termination at unexpected token \"////\"", e.getMessage());
+		}
+	}
+
+}

src/changes/changes.xml

@@ -165,7 +165,7 @@
 				and only option previously) or allow any IDs including alphanumeric.
 			</action>
 			<action type="add" issue="1103" dev="ruthakm">
-				It is now possible to use your own IMessageResolver instance in the narrative 
+				It is now possible to use your own IMessageResolver instance in the narrative
 				generator. Thanks to Ruth Alkema for the pull request!
 			</action>
 			<action type="fix" issue="1071" dev="volsch">
@@ -177,6 +177,10 @@
 				<![CDATA[<code>_format</code>]]>
 				parameter should be included in requests.
 			</action>
+			<action type="add">
+				JPA server R4 SearchParameter custom expression validation is now done using the
+				actual FHIRPath evaluator, meaning it is more rigorous in what it can find.
+			</action>
 		</release>
 
 		<release version="3.5.0" date="2018-09-17">