Skip to content

Commit

Permalink
issue #1351 - Check SearchParameter search restrictions
Browse files Browse the repository at this point in the history
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
  • Loading branch information
tbieste committed Oct 13, 2020
1 parent 81cea9f commit c4d0802
Show file tree
Hide file tree
Showing 4 changed files with 313 additions and 3 deletions.
89 changes: 86 additions & 3 deletions fhir-search/src/main/java/com/ibm/fhir/search/util/SearchUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
import com.ibm.fhir.model.type.Canonical;
import com.ibm.fhir.model.type.Code;
import com.ibm.fhir.model.type.code.ResourceType;
import com.ibm.fhir.model.type.code.SearchComparator;
import com.ibm.fhir.model.type.code.SearchModifierCode;
import com.ibm.fhir.model.type.code.SearchParamType;
import com.ibm.fhir.model.util.JsonSupport;
import com.ibm.fhir.model.util.ModelSupport;
Expand All @@ -47,6 +49,7 @@
import com.ibm.fhir.path.exception.FHIRPathException;
import com.ibm.fhir.search.SearchConstants;
import com.ibm.fhir.search.SearchConstants.Modifier;
import com.ibm.fhir.search.SearchConstants.Prefix;
import com.ibm.fhir.search.SearchConstants.Type;
import com.ibm.fhir.search.SummaryValueSet;
import com.ibm.fhir.search.compartment.CompartmentUtil;
Expand Down Expand Up @@ -771,14 +774,21 @@ public static FHIRSearchContext parseQueryParameters(Class<?> resourceType,
throw SearchExceptionUtil.buildNewInvalidSearchException(msg);
}
}

for (String queryParameterValueString : queryParameters.get(name)) {

// Build list of processed query parameters
List<QueryParameter> curParameterList = new ArrayList<>();
for (String paramValueString : params) {
QueryParameter parameter = new QueryParameter(type, parameterCode, modifier, modifierResourceTypeName);
List<QueryParameterValue> queryParameterValues =
processQueryParameterValueString(resourceType, searchParameter, modifier, parameter.getModifierResourceTypeName(), queryParameterValueString);
processQueryParameterValueString(resourceType, searchParameter, modifier, parameter.getModifierResourceTypeName(), paramValueString);
parameter.getValues().addAll(queryParameterValues);
curParameterList.add(parameter);
parameters.add(parameter);
}

// Check search restrictions based on the SearchParameter
checkSearchParameterRestrictions(parameterCode, searchParameter, curParameterList);

} // end else
} catch (FHIRSearchException se) {
// There's a number of places that throw within this try block. In all cases we want the same behavior:
Expand All @@ -800,7 +810,80 @@ public static FHIRSearchContext parseQueryParameters(Class<?> resourceType,
context.setSearchParameters(parameters);
return context;
}

/**
* Checks the query parameters (with the same parameter code) against any search restrictions specified
* in the SearchParameter resource for that parameter code.
*
* @param parameterCode
* the parameter code
* @param searchParameter
* the SearchParameter resource
* @param queryParameters
* the query parameters to check
* @throws FHIRSearchException
* if a search restriction is found that is not followed
*/
private static void checkSearchParameterRestrictions(String parameterCode, SearchParameter searchParameter, List<QueryParameter> queryParameters)
throws FHIRSearchException {

boolean allowMultipleAnd =
searchParameter.getMultipleAnd() == null || !searchParameter.getMultipleAnd().hasValue() || searchParameter.getMultipleAnd().getValue();
boolean allowMultipleOr =
searchParameter.getMultipleOr() == null || !searchParameter.getMultipleOr().hasValue() || searchParameter.getMultipleOr().getValue();
List<SearchComparator> comparators = searchParameter.getComparator();
List<SearchModifierCode> modifiers = searchParameter.getModifier();

// Check multipleAnd
if (!allowMultipleAnd && queryParameters.size() > 1) {
String msg =
"Search parameter '" + parameterCode + "' does not allow multiple parameters";
throw SearchExceptionUtil.buildNewInvalidSearchException(msg);
}

for (QueryParameter queryParameter : queryParameters) {

// Check multipleOr
if (!allowMultipleOr && queryParameter.getValues().size() > 1) {
String msg =
"Search parameter '" + parameterCode + "' does not allow multiple values";
throw SearchExceptionUtil.buildNewInvalidSearchException(msg);
}

// Check modifier
Modifier modifier = queryParameter.getModifier();
if (modifier != null && modifiers != null && !modifiers.isEmpty()) {
// Special handling of "type" modifier
if (modifier == Modifier.TYPE) {
if (!modifiers.contains(SearchModifierCode.TYPE)) {
String msg =
"Search parameter '" + parameterCode + "' does not allow modifier '" + queryParameter.getModifierResourceTypeName() + "'";
throw SearchExceptionUtil.buildNewInvalidSearchException(msg);
}
} else {
if (!modifiers.contains(SearchModifierCode.of(modifier.value()))) {
String msg =
"Search parameter '" + parameterCode + "' does not allow modifier '" + modifier.value() + "'";
throw SearchExceptionUtil.buildNewInvalidSearchException(msg);
}
}
}

// Check comparator
for (QueryParameterValue queryParameterValue : queryParameter.getValues()) {
Prefix prefix = queryParameterValue.getPrefix();
if (prefix != null && comparators != null && !comparators.isEmpty()) {
// Check if prefix is found in list of valid comparators
if (!comparators.contains(SearchComparator.of(prefix.value()))) {
String msg =
"Search parameter '" + parameterCode + "' does not allow comparator '" + prefix.value() + "'";
throw SearchExceptionUtil.buildNewInvalidSearchException(msg);
}
}
}
}
}

/**
* Common logic from handling a single queryParameterValueString based on its type
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
/*
* (C) Copyright IBM Corp. 2019, 2020
*
* SPDX-License-Identifier: Apache-2.0
*/

package com.ibm.fhir.search.parameters;

import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;

import com.ibm.fhir.config.FHIRConfiguration;
import com.ibm.fhir.config.FHIRRequestContext;
import com.ibm.fhir.model.resource.Patient;
import com.ibm.fhir.search.exception.FHIRSearchException;
import com.ibm.fhir.search.test.BaseSearchTest;
import com.ibm.fhir.search.util.SearchUtil;

/**
* Tests the detection of search restrictions defined by a SearchParameter.
*/
public class SearchParameterRestrictionTest extends BaseSearchTest {

private static final String TENANT_ID = "tenant7";

@Override
@BeforeClass
public void setup() {
FHIRConfiguration.setConfigHome("src/test/resources");
}

@Test
public void testMultipleOrAllowed() throws Exception {
FHIRRequestContext.set(new FHIRRequestContext(TENANT_ID));

Map<String, List<String>> queryParameters = new HashMap<>();
queryParameters.put("multiple-birth-count", Collections.singletonList("eq2,eq3,eq4"));

SearchUtil.parseQueryParameters(Patient.class, queryParameters);
}

@Test(expectedExceptions = { FHIRSearchException.class })
public void testMultipleOrDisllowed() throws Exception {
FHIRRequestContext.set(new FHIRRequestContext(TENANT_ID));

Map<String, List<String>> queryParameters = new HashMap<>();
queryParameters.put("multiple-birth-count-basic", Collections.singletonList("eq2,eq3"));

SearchUtil.parseQueryParameters(Patient.class, queryParameters);
}

@Test
public void testMultipleAndAllowed() throws Exception {
FHIRRequestContext.set(new FHIRRequestContext(TENANT_ID));

Map<String, List<String>> queryParameters = new HashMap<>();
queryParameters.put("multiple-birth-count", Arrays.asList("eq2","eq3"));

SearchUtil.parseQueryParameters(Patient.class, queryParameters);
}

@Test(expectedExceptions = { FHIRSearchException.class })
public void testMultipleAndDisallowed() throws Exception {
FHIRRequestContext.set(new FHIRRequestContext(TENANT_ID));

Map<String, List<String>> queryParameters = new HashMap<>();
queryParameters.put("multiple-birth-count-basic", Arrays.asList("eq2","eq3"));

SearchUtil.parseQueryParameters(Patient.class, queryParameters);
}

@Test
public void testComparatorAllowed() throws Exception {
FHIRRequestContext.set(new FHIRRequestContext(TENANT_ID));

Map<String, List<String>> queryParameters = new HashMap<>();
queryParameters.put("multiple-birth-count", Collections.singletonList("eq2"));

SearchUtil.parseQueryParameters(Patient.class, queryParameters);
}

@Test(expectedExceptions = { FHIRSearchException.class })
public void testComparatorDisallowed() throws Exception {
FHIRRequestContext.set(new FHIRRequestContext(TENANT_ID));

Map<String, List<String>> queryParameters = new HashMap<>();
queryParameters.put("multiple-birth-count-basic", Collections.singletonList("eq2"));

SearchUtil.parseQueryParameters(Patient.class, queryParameters);
}

@Test
public void testOtherComparatorDisallowed() throws Exception {
FHIRRequestContext.set(new FHIRRequestContext(TENANT_ID));

Map<String, List<String>> queryParameters = new HashMap<>();
queryParameters.put("multiple-birth-count-basic", Collections.singletonList("gt2"));

SearchUtil.parseQueryParameters(Patient.class, queryParameters);
}

@Test
public void testModifierAllowed() throws Exception {
FHIRRequestContext.set(new FHIRRequestContext(TENANT_ID));

Map<String, List<String>> queryParameters = new HashMap<>();
queryParameters.put("multiple-birth-count:missing", Collections.singletonList("true"));

SearchUtil.parseQueryParameters(Patient.class, queryParameters);
}

@Test(expectedExceptions = { FHIRSearchException.class })
public void testModifierDisallowed() throws Exception {
FHIRRequestContext.set(new FHIRRequestContext(TENANT_ID));

Map<String, List<String>> queryParameters = new HashMap<>();
queryParameters.put("multiple-birth-count-basic:missing", Collections.singletonList("true"));

SearchUtil.parseQueryParameters(Patient.class, queryParameters);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
{
"resourceType": "Bundle",
"id": "searchParams",
"meta": {
"lastUpdated": "2020-07-30T22:37:54.724+11:00"
},
"type": "collection",
"entry": [{
"fullUrl": "http://ibm.com/fhir/SearchParameter/Patient-multiple-birth-count",
"resource": {
"resourceType": "SearchParameter",
"id": "Patient-multiple-birth-count",
"url": "http://hl7.org/fhir/SearchParameter/Patient-multiple-birth-count",
"version": "4.0.0",
"name": "multiple-birth-count",
"status": "draft",
"experimental": false,
"date": "2018-12-27T22:37:54+11:00",
"publisher": "IBM FHIR Server Test",
"contact": [{
"telecom": [{
"system": "url",
"value": "http://ibm.com/fhir"
}]
},
{
"telecom": [{
"system": "url",
"value": "http://ibm.com/fhir"
}]
}],
"description": "The multiple birth count has been provided and satisfies this search value",
"code": "multiple-birth-count",
"base": ["Patient"],
"type": "number",
"expression": "(Patient.multipleBirth as integer)",
"xpath": "f:Patient/f:multipleBirthInteger",
"xpathUsage": "normal",
"multipleOr": true,
"multipleAnd": true,
"modifier": ["missing"],
"comparator": ["eq",
"ne",
"gt",
"ge",
"lt",
"le",
"sa",
"eb",
"ap"]
}
},
{
"fullUrl": "http://ibm.com/fhir/SearchParameter/Patient-multiple-birth-count-basic",
"resource": {
"resourceType": "SearchParameter",
"id": "Patient-multiple-birth-count-basic",
"url": "http://hl7.org/fhir/SearchParameter/Patient-multiple-birth-count-basic",
"version": "4.0.0",
"name": "multiple-birth-count-basic",
"status": "draft",
"experimental": false,
"date": "2018-12-27T22:37:54+11:00",
"publisher": "IBM FHIR Server Test",
"contact": [{
"telecom": [{
"system": "url",
"value": "http://ibm.com/fhir"
}]
},
{
"telecom": [{
"system": "url",
"value": "http://ibm.com/fhir"
}]
}],
"description": "The multiple birth count has been provided and satisfies this search value",
"code": "multiple-birth-count-basic",
"base": ["Patient"],
"type": "number",
"expression": "(Patient.multipleBirth as integer)",
"xpath": "f:Patient/f:multipleBirthInteger",
"xpathUsage": "normal",
"multipleOr": false,
"multipleAnd": false,
"modifier": ["type"],
"comparator": ["gt"]
}
}]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"__comment": "FHIR Server configuration",
"fhirServer": {
"_comment": "Just hiding this property because 'include-all' should be the default",
"_searchParameterFilter": {
"*": "*"
}
}
}

0 comments on commit c4d0802

Please sign in to comment.