Skip to content

Commit

Permalink
Issue #1409 - implement token search by codeSystem only
Browse files Browse the repository at this point in the history 
Signed-off-by: Mike Schroeder <mschroed@us.ibm.com>
  • Loading branch information
@michaelwschroeder
michaelwschroeder committed Apr 28, 2021
1 parent 5e93139 commit 0afc6a6
Show file tree
Hide file tree
Showing 7 changed files with 126 additions and 36 deletions.
3 changes: 1 addition & 2 deletions docs/src/pages/Conformance.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,11 +230,10 @@ For search parameters of type token, resource values are not indexed unless the
* `[parameter]=[code]`
* `[parameter]=[system]|[code]`
* `[parameter]=|[code]`
* `[parameter]=[system]|`

However, the `|[code]` variant currently behaves like the `[code]` option, matching code values irrespective of the system instead of matching only on elements with missing/null system values as defined in the spec.

The IBM FHIR Server does not yet support searching a token value by codesystem, irrespective of the value (`|[system]|`).

For search parameters of type token that are defined on data fields of type `ContactPoint`, the FHIR server currently uses the `ContactPoint.system` and the `ContactPoint.value` instead of the `ContactPoint.use` field as described in the specification.

For search parameters of type token that are defined on data fields of type `string`, `code`, `Coding`, `CodeableConcept`, and `Identifier`, the case-sensitivity of the specified code system is used to determine how resource values are indexed and how the FHIR server performs the search. The following table describes how a resource value is indexed, based on the case-sensitivity of the code system specified in the resource value (or in the case of `code` data fields, the code system bound to the data field if the field has a required binding):
Expand Down
56 changes: 28 additions & 28 deletions fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/JDBCQueryBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1255,44 +1255,44 @@ private SqlQueryData processTokenParm(Class<?> resourceType, QueryParameter quer
Modifier.ABOVE.equals(queryParm.getModifier()) || Modifier.BELOW.equals(queryParm.getModifier())) {
populateCodesSubSegment(whereClauseSegment, queryParm.getModifier(), value, tableAlias);
} else {
// Include code
if (LIKE.equals(operator)) {
whereClauseSegment.append(tableAlias + DOT).append(TOKEN_VALUE).append(operator).append(BIND_VAR);

// Must escape special wildcard characters _ and % in the parameter value string.
String textSearchString = SqlParameterEncoder.encode(value.getValueCode())
.replace(PERCENT_WILDCARD, ESCAPE_PERCENT)
.replace(UNDERSCORE_WILDCARD, ESCAPE_UNDERSCORE) + PERCENT_WILDCARD;
bindVariables.add(SearchUtil.normalizeForSearch(textSearchString));
appendEscape = true;
} else {
// Determine code system case-sensitivity
boolean codeSystemIsCaseSensitive = false;
if (value.getValueSystem() != null && !value.getValueSystem().isEmpty()) {
// Include code if present.
if (value.getValueCode() != null) {
if (LIKE.equals(operator)) {
whereClauseSegment.append(tableAlias + DOT).append(TOKEN_VALUE).append(operator).append(BIND_VAR);

// Normalize code if code system is not case-sensitive. Otherwise leave code as is.
codeSystemIsCaseSensitive = CodeSystemSupport.isCaseSensitive(value.getValueSystem());
bindVariables.add(SqlParameterEncoder.encode(codeSystemIsCaseSensitive ?
value.getValueCode() : SearchUtil.normalizeForSearch(value.getValueCode())));
// Must escape special wildcard characters _ and % in the parameter value string.
String textSearchString = SqlParameterEncoder.encode(value.getValueCode())
.replace(PERCENT_WILDCARD, ESCAPE_PERCENT)
.replace(UNDERSCORE_WILDCARD, ESCAPE_UNDERSCORE) + PERCENT_WILDCARD;
bindVariables.add(SearchUtil.normalizeForSearch(textSearchString));
appendEscape = true;
} else {
// If no code system specified, search against both normalized code and unmodified code.
whereClauseSegment.append(tableAlias + DOT).append(TOKEN_VALUE);
if (NE.equals(operator)) {
whereClauseSegment.append(NOT);
// Determine code system case-sensitivity
boolean codeSystemIsCaseSensitive = false;
if (value.getValueSystem() != null && !value.getValueSystem().isEmpty()) {
whereClauseSegment.append(tableAlias + DOT).append(TOKEN_VALUE).append(operator).append(BIND_VAR);

// Normalize code if code system is not case-sensitive. Otherwise leave code as is.
codeSystemIsCaseSensitive = CodeSystemSupport.isCaseSensitive(value.getValueSystem());
bindVariables.add(SqlParameterEncoder.encode(codeSystemIsCaseSensitive ?
value.getValueCode() : SearchUtil.normalizeForSearch(value.getValueCode())));
} else {
// If no code system specified, search against both normalized code and unmodified code.
whereClauseSegment.append(tableAlias + DOT).append(TOKEN_VALUE);
if (NE.equals(operator)) {
whereClauseSegment.append(NOT);
}
whereClauseSegment.append(IN).append(LEFT_PAREN).append(BIND_VAR).append(COMMA).append(BIND_VAR).append(RIGHT_PAREN);
bindVariables.add(SqlParameterEncoder.encode(value.getValueCode()));
bindVariables.add(SqlParameterEncoder.encode(SearchUtil.normalizeForSearch(value.getValueCode())));
}
whereClauseSegment.append(IN).append(LEFT_PAREN).append(BIND_VAR).append(COMMA).append(BIND_VAR).append(RIGHT_PAREN);
bindVariables.add(SqlParameterEncoder.encode(value.getValueCode()));
bindVariables.add(SqlParameterEncoder.encode(SearchUtil.normalizeForSearch(value.getValueCode())));
}
}

// Include system if present.
if (value.getValueSystem() != null && !value.getValueSystem().isEmpty()) {
Long commonTokenValueId = null;
if (NE.equals(operator)) {
whereClauseSegment.append(OR);
} else {
if (value.getValueCode() != null) {
whereClauseSegment.append(AND);

// use #1929 optimization if we can
Expand Down
22 changes: 18 additions & 4 deletions ...rsistence/src/test/java/com/ibm/fhir/persistence/search/test/AbstractSearchTokenTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ public void testSearchToken_code() throws Exception {
// system is case-sensitive
assertSearchReturnsSavedResource("text-status", "http://hl7.org/fhir/narrative-status|extensions");
assertSearchDoesntReturnSavedResource("text-status", "http://hl7.org/fhir/narrative-status|EXTENSIONS");

assertSearchReturnsSavedResource("text-status", "http://hl7.org/fhir/narrative-status|");
assertSearchDoesntReturnSavedResource("text-status", "http://hl7.org/fhir/narrative-status||");
assertSearchDoesntReturnSavedResource("text-status", "system|");
}

@Test
Expand Down Expand Up @@ -235,7 +239,11 @@ public void testSearchToken_CodeableConcept() throws Exception {
// system is case-sensitive
assertSearchReturnsSavedResource("CodeableConcept-validCodeAndSystem", "http://terminology.hl7.org/CodeSystem/v3-ObservationInterpretation|AA");
assertSearchDoesntReturnSavedResource("CodeableConcept-validCodeAndSystem", "http://terminology.hl7.org/CodeSystem/v3-ObservationInterpretation|aa");
// assertSearchReturnsSavedResource("CodeableConcept", "http://example.org/codesystem|");

assertSearchReturnsSavedResource("CodeableConcept", "http://example.org/codesystem|");
assertSearchDoesntReturnSavedResource("CodeableConcept", "http://example.org/codesystem||");
assertSearchDoesntReturnSavedResource("CodeableConcept", "example.org/codesystem|");
assertSearchReturnsSavedResource("CodeableConcept-validCodeAndSystem", "http://terminology.hl7.org/CodeSystem/v3-ObservationInterpretation|");

// This shouldn't return any results because the CodeableConcept has a system
// assertSearchDoesntReturnSavedResource("CodeableConcept", "|code");
Expand Down Expand Up @@ -358,7 +366,11 @@ public void testSearchToken_Coding() throws Exception {
// system is not case-sensitive
assertSearchReturnsSavedResource("Coding-validCaseInsensitiveCodeAndSystem", "http://terminology.hl7.org/CodeSystem/v2-0001|F");
assertSearchReturnsSavedResource("Coding-validCaseInsensitiveCodeAndSystem", "http://terminology.hl7.org/CodeSystem/v2-0001|f");
// assertSearchReturnsSavedResource("Coding", "http://example.org/codesystem|");

assertSearchReturnsSavedResource("Coding", "http://example.org/codesystem|");
assertSearchDoesntReturnSavedResource("Coding", "http://example.org/codesystem||");
assertSearchDoesntReturnSavedResource("Coding", "Http://example.org/codesystem|");
assertSearchReturnsSavedResource("Coding-validCodeAndSystem", "http://terminology.hl7.org/CodeSystem/v3-ObservationInterpretation|");

// This shouldn't return any results because the Coding has a system
// assertSearchDoesntReturnSavedResource("Coding", "|code");
Expand Down Expand Up @@ -495,8 +507,10 @@ public void testSearchToken_Identifier() throws Exception {
assertSearchReturnsSavedResource("Identifier-validValueAndSystem", "http://hl7.org/fhir/identifier-use|official");
assertSearchDoesntReturnSavedResource("Identifier-validValueAndSystem", "http://hl7.org/fhir/identifier-use|OFFICIAL");

// Search specifying only a system value is not currently supported (see issue #1409)
// assertSearchReturnsSavedResource("Identifier", "http://example.org/identifiersystem|");
assertSearchReturnsSavedResource("Identifier", "http://example.org/identifiersystem|");
assertSearchDoesntReturnSavedResource("Identifier", "http://example.org/identifiersystem||");
assertSearchDoesntReturnSavedResource("Identifier", "http://example.org/IdentifierSystem|");
assertSearchReturnsSavedResource("Identifier-validValueAndSystem", "http://hl7.org/fhir/identifier-use|");

// This shouldn't return any results because the Identifier has a system
// assertSearchDoesntReturnSavedResource("Identifier", "|code");
Expand Down
9 changes: 8 additions & 1 deletion fhir-search/src/main/java/com/ibm/fhir/search/parameters/QueryParameterValue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,14 @@ public String toString() {
delim = outputBuilder(returnString, delim, valueCode);
delim = outputBuilder(returnString, delim, valueString);
delim = outputBuilder(returnString, delim, valueDate);


// Special handling required for token search of form "system|". In that case, valueSystem
// will not be null, but all following values will be null, so the above processing will
// not append the delimiter. Check if we have that case, and if so, append the delimiter.
if (valueSystem != null && valueCode == null && valueString == null && valueDate == null) {
returnString.append(SearchConstants.PARAMETER_DELIMITER);
}

// token search with :of-type modifier is handled internally as a composite search
if (component != null && !component.isEmpty()) {
String componentDelim = "";
Expand Down
4 changes: 4 additions & 0 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 @@ -1379,7 +1379,11 @@ private static List<QueryParameterValue> parseQueryParameterValuesString(SearchP
} else if (parts.length == 2) {
parameterValue.setValueSystem(unescapeSearchParm(parts[0]));
parameterValue.setValueCode(unescapeSearchParm(parts[1]));
} else if (parts.length == 1 && v.endsWith("|") && v.indexOf("|") == v.length()-1) {
// Only a system was specified (uri followed by a single '|')
parameterValue.setValueSystem(unescapeSearchParm(parts[0]));
} else {
// Treat as a single code.
// Optimization for search parameters that always reference the same system, added under #1929
if (!Modifier.MISSING.equals(modifier)) {
try {
Expand Down
17 changes: 17 additions & 0 deletions fhir-search/src/test/java/com/ibm/fhir/search/uri/test/UriTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,4 +140,21 @@ public void testUriWithTotalParameter() throws URISyntaxException {

assertEquals(SearchUtil.buildSearchSelfUri(requestUriString, ctx), expectedUri);
}

@Test
public void testUriWithSystemOnly() throws URISyntaxException {
String expectedUri = "https://test?_count=10&param=system%7C&_page=1";
String requestUriString = "https://test?param=system|";

FHIRSearchContext ctx = FHIRSearchContextFactory.createSearchContext();
ctx.setPageNumber(1);
ctx.setPageSize(10);
QueryParameterValue paramVal = new QueryParameterValue();
paramVal.setValueSystem("system");
QueryParameter queryParameter = new QueryParameter(Type.TOKEN, "param", null, null, Collections.singletonList(paramVal));
ctx.setSearchParameters(Collections.singletonList(queryParameter));

assertEquals(SearchUtil.buildSearchSelfUri(requestUriString, ctx), expectedUri);
}

}
51 changes: 50 additions & 1 deletion fhir-server-test/src/test/java/com/ibm/fhir/server/test/SearchTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2252,7 +2252,7 @@ public void testSearchConditionVerificationStatusBelowNotFound() {
}

@Test(groups = { "server-search" }, dependsOnMethods = {"testCreateCondition" })
public void testSearchConditionTagCaseSensitivity() {
public void testSearchConditionEvidenceCaseSensitivity() {
WebTarget target = getWebTarget();
Response response =
target.path("Condition")
Expand Down Expand Up @@ -2360,4 +2360,53 @@ public void testSearchConditionTagCaseSensitivity() {
assertTrue(bundle.getEntry().size() >= 1);
}

@Test(groups = { "server-search" }, dependsOnMethods = {"testCreateCondition" })
public void testSearchConditionEvidenceCodeAndSystem() {
WebTarget target = getWebTarget();
Response response =
target.path("Condition")
.queryParam("evidence", "http://terminology.hl7.org/CodeSystem/v3-ObservationValue|A4")
.request(FHIRMediaType.APPLICATION_FHIR_JSON)
.header("X-FHIR-TENANT-ID", tenantName)
.header("X-FHIR-DSID", dataStoreId)
.get();
assertResponse(response, Response.Status.OK.getStatusCode());
Bundle bundle = response.readEntity(Bundle.class);
assertNotNull(bundle);
if (DEBUG_SEARCH) {
SearchAllTest.generateOutput(bundle);
}
assertTrue(bundle.getEntry().size() >= 1);

response =
target.path("Condition")
.queryParam("evidence", "A4")
.request(FHIRMediaType.APPLICATION_FHIR_JSON)
.header("X-FHIR-TENANT-ID", tenantName)
.header("X-FHIR-DSID", dataStoreId)
.get();
assertResponse(response, Response.Status.OK.getStatusCode());
bundle = response.readEntity(Bundle.class);
assertNotNull(bundle);
if (DEBUG_SEARCH) {
SearchAllTest.generateOutput(bundle);
}
assertTrue(bundle.getEntry().size() >= 1);

response =
target.path("Condition")
.queryParam("evidence", "http://terminology.hl7.org/CodeSystem/v3-ObservationValue|")
.request(FHIRMediaType.APPLICATION_FHIR_JSON)
.header("X-FHIR-TENANT-ID", tenantName)
.header("X-FHIR-DSID", dataStoreId)
.get();
assertResponse(response, Response.Status.OK.getStatusCode());
bundle = response.readEntity(Bundle.class);
assertNotNull(bundle);
if (DEBUG_SEARCH) {
SearchAllTest.generateOutput(bundle);
}
assertTrue(bundle.getEntry().size() >= 1);
}

}

0 comments on commit 0afc6a6

Please sign in to comment.