Skip to content

Commit

Permalink
issue #1351 - Check search parameter combinations
Browse files Browse the repository at this point in the history 
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
  • Loading branch information
@tbieste
tbieste committed Oct 26, 2020
1 parent 493e9b8 commit 1feaa61
Show file tree
Hide file tree
Showing 6 changed files with 306 additions and 42 deletions.
18 changes: 18 additions & 0 deletions docs/src/pages/guides/FHIRServerUsersGuide.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1520,9 +1520,15 @@ This section contains reference information about each of the configuration prop
|`fhirServer/resources/Resource/interactions`|string list|A list of strings that represent the RESTful interactions (create, read, vread, update, patch, delete, history, and/or search) supported for resource types. Omitting this property is equivalent to supporting all FHIR interactions for the supported resources. An empty list, `[]`, can be used to indicate that no REST methods are supported. This property can be overridden for specific resource types via the `fhirServer/resources/<resourceType>/interactions` property.|
|`fhirServer/resources/Resource/searchParameters`|object|The set of search parameters to support for all supported resource types. Omitting this property is equivalent to supporting all search parameters in the server's registry that apply to resource type "Resource" (all resources). An empty object, `{}`, can be used to indicate that no global search parameters are supported.|
|`fhirServer/resources/Resource/searchParameters/<code>`|string|The URL of the search parameter definition to use for the search parameter `<code>`. Individual resource types may override this value via `fhirServer/resources/<resourceType>/searchParameters/<code>`|
|`fhirServer/resources/Resource/searchIncludes`|string list|A comma-separated list of \_include values supported for all resource types. Individual resource types may override this value via `fhirServer/resources/<resourceType>/searchIncludes`. Omitting this property is equivalent to supporting all \_include values for the supported resources. An empty list, `[]`, can be used to indicate that no \_include values are supported.|
|`fhirServer/resources/Resource/searchRevIncludes`|string list|A comma-separated list of \_revinclude values supported for all resource types. Individual resource types may override this value via `fhirServer/resources/<resourceType>/searchRevIncludes`. Omitting this property is equivalent to supporting all \_revinclude values for the supported resources. An empty list, `[]`, can be used to indicate that no \_revinclude values are supported.|
|`fhirServer/resources/Resource/searchParameterCombinations`|string list|A comma-separated list of search parameter combinations supported for all resource types. Each search parameter combination is a string, where a plus sign, `+`, separates the search parameters that can be used in combination. To indicate that searching without any search parameters is allowed, an empty string must be included in the list. Including an asterisk, `*`, in the list indicates support of any search parameter combination. Individual resource types may override this value via `fhirServer/resources/<resourceType>/searchParameterCombinations`. Omitting this property is equivalent to supporting any search parameter combination.|
|`fhirServer/resources/<resourceType>/interactions`|string list|A list of strings that represent the RESTful interactions (create, read, vread, update, patch, delete, history, and/or search) to support for this resource type. For resources without the property, the value of `fhirServer/resources/Resource/interactions` is used.|
|`fhirServer/resources/<resourceType>/searchParameters`|object|The set of search parameters to support for this resource type. Global search parameters defined on the `Resource` resource can be overridden on a per-resourceType basis.|
|`fhirServer/resources/<resourceType>/searchParameters/<code>`|string|The URL of the search parameter definition to use for the search parameter `<code>` on resources of type `<resourceType>`.|
|`fhirServer/resources/<resourceType>/searchIncludes`|string list|A comma-separated list of \_include values supported for this resource type. An empty list, `[]`, can be used to indicate that no \_include values are supported. For resources without the property, the value of `fhirServer/resources/Resource/searchIncludes` is used.|
|`fhirServer/resources/<resourceType>/searchRevIncludes`|string list|A comma-separated list of \_revinclude values supported for this resource type. An empty list, `[]`, can be used to indicate that no \_revinclude values are supported. For resources without the property, the value of `fhirServer/resources/Resource/searchRevIncludes` is used.|
|`fhirServer/resources/<resourceType>/searchParameterCombinations`|string list|A comma-separated list of search parameter combinations supported for this resource type. Each search parameter combination is a string, where a plus sign, `+`, separates the search parameters that can be used in combination. To indicate that searching without any search parameters is allowed, an empty string must be included in the list. Including an asterisk, `*`, in the list indicates support of any search parameter combination. For resources without the property, the value of `fhirServer/resources/Resource/searchParameterCombinations` is used.|
|`fhirServer/notifications/common/includeResourceTypes`|string list|A comma-separated list of resource types for which notification event messages should be published.|
|`fhirServer/notifications/websocket/enabled`|boolean|A boolean flag which indicates whether or not websocket notifications are enabled.|
|`fhirServer/notifications/kafka/enabled`|boolean|A boolean flag which indicates whether or not kafka notifications are enabled.|
Expand Down Expand Up @@ -1604,9 +1610,15 @@ This section contains reference information about each of the configuration prop
|`fhirServer/resources/open`|true|
|`fhirServer/resources/Resource/interactions`|null (all interactions supported)|
|`fhirServer/resources/Resource/searchParameters`|null (all global search parameters supported)|
|`fhirServer/resources/Resource/searchIncludes`|null (all \_include values supported)|
|`fhirServer/resources/Resource/searchRevIncludes`|null (all \_revinclude values supported)|
|`fhirServer/resources/Resource/searchParameterCombinations`|null (all search parameter combinations supported)|
|`fhirServer/resources/<resourceType>/interactions`|null (inherets from `fhirServer/resources/Resource/interactions`)|
|`fhirServer/resources/<resourceType>/searchParameters`|null (all type-specific search parameters supported)|
|`fhirServer/resources/<resourceType>/searchParameters/<code>`|null|
|`fhirServer/resources/<resourceType>/searchIncludes`|null (inherets from `fhirServer/resources/Resource/searchIncludes`)|
|`fhirServer/resources/<resourceType>/searchRevIncludes`|null (inherets from `fhirServer/resources/Resource/searchRevIncludes`)|
|`fhirServer/resources/<resourceType>/searchParameterCombinations`|null (inherets from `fhirServer/resources/Resource/searchParameterCombinations`)|
|`fhirServer/notifications/common/includeResourceTypes`|`["*"]`|
|`fhirServer/notifications/websocket/enabled`|false|
|`fhirServer/notifications/kafka/enabled`|false|
Expand Down Expand Up @@ -1680,9 +1692,15 @@ must restart the server for that change to take effect.
|`fhirServer/resources/Resource/interactions`|Y|Y|
|`fhirServer/resources/Resource/searchParameters`|Y|Y|
|`fhirServer/resources/Resource/searchParameters/<code>`|Y|Y|
|`fhirServer/resources/Resource/searchIncludes`|Y|Y|
|`fhirServer/resources/Resource/searchRevIncludes`|Y|Y|
|`fhirServer/resources/Resource/searchParameterCombinations`|Y|Y|
|`fhirServer/resources/<resourceType>/interactions`|Y|Y|
|`fhirServer/resources/<resourceType>/searchParameters`|Y|Y|
|`fhirServer/resources/<resourceType>/searchParameters/<code>`|Y|Y|
|`fhirServer/resources/<resourceType>/searchIncludes`|Y|Y|
|`fhirServer/resources/<resourceType>/searchRevIncludes`|Y|Y|
|`fhirServer/resources/<resourceType>/searchParameterCombinations`|Y|Y|
|`fhirServer/notifications/common/includeResourceTypes`|N|N|
|`fhirServer/notifications/websocket/enabled`|N|N|
|`fhirServer/notifications/kafka/enabled`|N|N|
Expand Down
1 change: 1 addition & 0 deletions fhir-config/src/main/java/com/ibm/fhir/config/FHIRConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public class FHIRConfiguration {
public static final String PROPERTY_FIELD_RESOURCES_SEARCH_PARAMETERS = "searchParameters";
public static final String PROPERTY_FIELD_RESOURCES_SEARCH_INCLUDES = "searchIncludes";
public static final String PROPERTY_FIELD_RESOURCES_SEARCH_REV_INCLUDES = "searchRevIncludes";
public static final String PROPERTY_FIELD_RESOURCES_SEARCH_PARAMETER_COMBINATIONS = "searchParameterCombinations";

// Auth and security properties
public static final String PROPERTY_SECURITY_CORS = "fhirServer/security/cors";
Expand Down
30 changes: 22 additions & 8 deletions fhir-search/src/main/java/com/ibm/fhir/search/exception/SearchExceptionUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,17 @@ public class SearchExceptionUtil {
private static final String ILLEGAL_EXCEPTION = "SearchParameter filter property values must be an array of String.";
private static final String ILLEGAL_ARGUMENT_EXCEPTION = "No constant with value '%s' found.";
private static final String PARSE_PARAMETER_EXCEPTION = "An error occurred while parsing parameter '%s'.";
private static final String PARSE_PARAMETERS_EXCEPTION = "An error occurred while parsing parameters.";
private static final String CHAINED_PARAMETER_EXCEPTION = "Unable to parse chained parameter: '%s'";
private static final String BADFORMAT_EXCEPTION = "Invalid Date Time Format found please use 'yyyy-mm-ddThh:mm:ss[Z|(+|-)hh:mm].'";

private SearchExceptionUtil() {
// No Op
}

/**
* creates an invalid search exception.
*
*
* @param msg
* @return
*/
Expand All @@ -38,7 +39,7 @@ public static FHIRSearchException buildNewInvalidSearchException(final String ms

/**
* creates a new parse parameter exception
*
*
* @param name
* @param e
* @return
Expand All @@ -49,9 +50,22 @@ public static FHIRSearchException buildNewParseParameterException(final String n
return new FHIRSearchException(msg, e).withIssue(ooi);
}

/**
* creates a new parse parameters exception
*
* @param name
* @param e
* @return
*/
public static FHIRSearchException buildNewParseParametersException(Exception e) {
String msg = String.format(PARSE_PARAMETERS_EXCEPTION);
OperationOutcome.Issue ooi = FHIRUtil.buildOperationOutcomeIssue(msg, IssueType.INVALID);
return new FHIRSearchException(msg, e).withIssue(ooi);
}

/**
* creates a new chained parameter exception
*
*
* @param name
* @param e
* @return
Expand All @@ -64,7 +78,7 @@ public static FHIRSearchException buildNewChainedParameterException(final String

/**
* builds an illegal state exception for a search filter execution
*
*
* @return
*/
public static IllegalStateException buildNewIllegalStateException() {
Expand All @@ -73,17 +87,17 @@ public static IllegalStateException buildNewIllegalStateException() {

/**
* builds an illegal Argument exception.
*
*
* @param val
* @return
*/
public static IllegalArgumentException buildNewIllegalArgumentException(final String val) {
return new IllegalArgumentException(String.format(ILLEGAL_ARGUMENT_EXCEPTION, val));
}

/**
* build data time format exception
*
*
* @param exception e
* @return
*/
Expand Down
122 changes: 119 additions & 3 deletions fhir-search/src/main/java/com/ibm/fhir/search/util/SearchUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ public class SearchUtil {
private static final String DIFFERENT_MODIFIYERRESOURCETYPES_FOUND_FOR_RESOURCETYPES =
"Different Modifier resource types are found for search parameter [%s] of the to-be-searched resource types.";

// Other Constants
private static final String SEARCH_PARAM_COMBINATION_ANY = "*";
private static final String SEARCH_PARAM_COMBINATION_DELIMITER = "\\+";

// The functionality is split into a new class.
private static final Sort sort = new Sort();

Expand Down Expand Up @@ -825,6 +829,16 @@ public static FHIRSearchContext parseQueryParameters(Class<?> resourceType,
}
} // end for

try {
// Check for valid search parameter combinations
checkSearchParameterCombinations(resourceType, parameters);

} catch (FHIRSearchException se) {
throw se;
} catch (Exception e) {
throw SearchExceptionUtil.buildNewParseParametersException(e);
}

context.setSearchParameters(parameters);
return context;
}
Expand Down Expand Up @@ -911,6 +925,109 @@ private static void checkSearchParameterRestrictions(String parameterCode, Searc
}
}

/**
* Checks that the combination of search parameters is valid.
*
* @param resourceType
* the resource type
* @param parameters
* the query parameters to check
* @throws Exception
* an exception
*/
private static void checkSearchParameterCombinations(Class<?> resourceType, List<QueryParameter> parameters)
throws Exception {

List<Set<String>> validCombinations = getSearchParameterCombinations(resourceType.getSimpleName());
if (validCombinations != null) {
Set<String> searchParameterCodes = parameters.stream().map(qp -> qp.getCode()).collect(Collectors.toSet());

// Check that search parameter codes are a valid combinations
if (!validCombinations.contains(searchParameterCodes)) {
String msg;
if (searchParameterCodes.isEmpty()) {
msg = "A valid search parameter combination is required";
} else {
msg = "Search parameter combination is not valid";
}
throw SearchExceptionUtil.buildNewInvalidSearchException(msg);
}
}
}

/**
* Retrieves the search parameter combinations.
*
* @param resourceType
* the resource type
* @return list of allowed search parameter combinations, or null if any search parameter combination is allowed
* @throws Exception
* an exception
*/
private static List<Set<String>> getSearchParameterCombinations(String resourceType) throws Exception {

List<Set<String>> spCombinations = null;

// Retrieve the "resources" config property group.
PropertyGroup rsrcsGroup = FHIRConfigHelper.getPropertyGroup(FHIRConfiguration.PROPERTY_RESOURCES);
if (rsrcsGroup != null) {
List<PropertyEntry> rsrcsEntries = rsrcsGroup.getProperties();
if (rsrcsEntries != null && !rsrcsEntries.isEmpty()) {
List<String> combinations = null;

// Try to find search parameter combinations property for matching resource type
for (PropertyEntry rsrcsEntry : rsrcsEntries) {
if (resourceType.equals(rsrcsEntry.getName())) {
PropertyGroup resourceTypeGroup = (PropertyGroup) rsrcsEntry.getValue();
if (resourceTypeGroup != null) {
combinations = resourceTypeGroup.getStringListProperty(FHIRConfiguration.PROPERTY_FIELD_RESOURCES_SEARCH_PARAMETER_COMBINATIONS);
break;
}
}
}

// Otherwise, try to find search parameter combinations property for "Resource" resource type
if (combinations == null) {
for (PropertyEntry rsrcsEntry : rsrcsEntries) {

// Check if matching resource type
if (SearchConstants.RESOURCE_RESOURCE.equals(rsrcsEntry.getName())) {
PropertyGroup resourceTypeGroup = (PropertyGroup) rsrcsEntry.getValue();
if (resourceTypeGroup != null) {
combinations =
resourceTypeGroup.getStringListProperty(FHIRConfiguration.PROPERTY_FIELD_RESOURCES_SEARCH_PARAMETER_COMBINATIONS);
break;
}
}
}
}

// Convert the delimited combinations to a list of sets
if (combinations != null) {
spCombinations = new ArrayList<>();
for (String combination : combinations) {
Set<String> combinationSet = new HashSet<>();
if (!combination.isEmpty()) {
// If any search parameter combination is allowed, return null
if (SEARCH_PARAM_COMBINATION_ANY.equals(combination)) {
return null;
}
for (String spString : combination.split(SEARCH_PARAM_COMBINATION_DELIMITER)) {
if (spString.trim().isEmpty()) {
throw SearchExceptionUtil.buildNewIllegalStateException();
}
combinationSet.add(spString);
}
}
spCombinations.add(combinationSet);
}
}
}
}

return spCombinations;
}

/**
* Common logic from handling a single queryParameterValueString based on its type
*/
Expand Down Expand Up @@ -1659,7 +1776,7 @@ private static List<String> getSearchIncludeRestrictions(String resourceType) th
List<PropertyEntry> rsrcsEntries = rsrcsGroup.getProperties();
if (rsrcsEntries != null && !rsrcsEntries.isEmpty()) {

// Try find search includes property for matching resource type
// Try to find search includes property for matching resource type
for (PropertyEntry rsrcsEntry : rsrcsEntries) {
if (resourceType.equals(rsrcsEntry.getName())) {
PropertyGroup resourceTypeGroup = (PropertyGroup) rsrcsEntry.getValue();
Expand All @@ -1669,7 +1786,7 @@ private static List<String> getSearchIncludeRestrictions(String resourceType) th
}
}

// Otherwise, try find search includes property for "Resource" resource type
// Otherwise, try to find search includes property for "Resource" resource type
for (PropertyEntry rsrcsEntry : rsrcsEntries) {

// Check if matching resource type
Expand All @@ -1680,7 +1797,6 @@ private static List<String> getSearchIncludeRestrictions(String resourceType) th
}
}
}

}
}

Expand Down
Loading

0 comments on commit 1feaa61

Please sign in to comment.