Skip to content

Commit

Permalink
issue #3712 - check all CodeSystem concept designations (#3771)
Browse files Browse the repository at this point in the history 
* issue #3712 - check all CodeSystem concept designations

1. Split `validateDisplay` into two flavors; now
validateCode(CodeSystem, Coding, ValidationParameters) will use the
passed CodeSystem to determine case sensitivity (instead of looking it
up via the registry)

2. When the Coding.display doesn't match the lookup display, we now look
for a match within the CodesSystem.concept.designation values before
calling it invalid.

3. Added a designation to a couple sample code systems and added
testValidateDisplay tests for both case-sensitive and case-insensitive
behavior.

Signed-off-by: Lee Surprenant <lmsurpre@merative.com>

* Apply suggestions from code review

Signed-off-by: Lee Surprenant <lmsurpre@merative.com>
  • Loading branch information
@lmsurpre
lmsurpre authored Jul 13, 2022
1 parent 00e0735 commit 7a6eb5d
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 22 deletions.
89 changes: 69 additions & 20 deletions term/fhir-term/src/main/java/com/ibm/fhir/term/service/FHIRTermService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -435,9 +435,8 @@ public LookupOutcome lookup(Coding coding, LookupParameters parameters) {
* the coding to lookup
* @param parameters
* the lookup parameters
*
* @return
* the outcome of the lookup
* the outcome of the lookup; null if the passed CodeSystem is not applicable to the system and version of the coding
*/
public LookupOutcome lookup(CodeSystem codeSystem, Coding coding, LookupParameters parameters) {
if (!LookupParameters.EMPTY.equals(parameters)) {
Expand Down Expand Up @@ -828,7 +827,7 @@ public ValidationOutcome validateCode(CodeSystem codeSystem, Coding coding, Vali
}
LookupOutcome outcome = lookup(codeSystem, coding, LookupParameters.EMPTY);
if (outcome != null) {
return validateDisplay(null, coding, outcome);
return validateDisplay(coding, outcome, CodeSystemSupport.isCaseSensitive(codeSystem));
} else {
StringBuilder message = new StringBuilder("Code '");
if (coding != null && coding.getCode() != null) {
Expand Down Expand Up @@ -976,7 +975,7 @@ public ValidationOutcome validateCode(ValueSet valueSet, CodeableConcept codeabl
if (result) {
LookupOutcome outcome = lookup(coding);
if (outcome != null || coding.getDisplay() == null) {
return validateDisplay(null, coding, outcome);
return validateDisplay(coding, outcome);
}

// lookup outcome was null and we have a non-null display, so include a message in the response
Expand Down Expand Up @@ -1054,7 +1053,7 @@ public ValidationOutcome validateCode(ValueSet valueSet, Coding coding, Validati
if (result) {
LookupOutcome outcome = lookup(coding);
if (outcome != null || coding.getDisplay() == null) {
return validateDisplay(null, coding, outcome);
return validateDisplay(coding, outcome);
}

// lookup outcome was null and we have a non-null display, so include a message in the response
Expand Down Expand Up @@ -1167,29 +1166,79 @@ private List<FHIRTermServiceProvider> loadProviders() {
return providers;
}

private ValidationOutcome validateDisplay(CodeSystem codeSystem, Coding coding, LookupOutcome lookupOutcome) {
/**
* Validate a coding display against the passed lookupOutcome, inferring case sensitivity from the registry
*
* @param coding
* the coding
* @param lookupOutcome
* the outcome of a CodeSystem lookup
* @return
* the outcome of validation; true if either the lookupOutcome or coding parameters are null or have a null display
* @implNote this method looks up the implied CodeSystem from the FHIR registry to determine case sensitivity,
* then delegates to {@link #validateDisplay(Coding, LookupOutcome, boolean)}
*/
private ValidationOutcome validateDisplay(Coding coding, LookupOutcome lookupOutcome) {
if (coding == null) {
return buildValidationOutcome(true, null, lookupOutcome);
}

java.lang.String system = (coding.getSystem() != null) ? coding.getSystem().getValue() : null;
java.lang.String version = (coding.getVersion() != null) ? coding.getVersion().getValue() : null;

boolean caseSensitive = false;
if (system != null) {
java.lang.String url = (version != null) ? system + "|" + version : system;
caseSensitive = CodeSystemSupport.isCaseSensitive(url);
}

return validateDisplay(coding, lookupOutcome, caseSensitive);
}

/**
* Validate a coding display against the passed lookupOutcome
*
* @param coding
* the coding
* @param lookupOutcome
* the outcome of a CodeSystem lookup
* @param isCaseSensitive
* whether the underlying CodeSystem is case-sensitive or not
* @return
* the outcome of validation; true if either the lookupOutcome or coding parameters are null or have a null display
*/
private ValidationOutcome validateDisplay(Coding coding, LookupOutcome lookupOutcome, boolean isCaseSensitive) {
if (lookupOutcome == null || coding == null ||
lookupOutcome.getDisplay() == null || coding.getDisplay() == null ||
lookupOutcome.getDisplay().getValue() == null && coding.getDisplay().getValue() == null) {
return buildValidationOutcome(true, null, lookupOutcome);
}

java.lang.String system = null;
if (coding.getSystem() != null) {
system = coding.getSystem().getValue();
} else if (codeSystem != null && codeSystem.getUrl() != null) {
system = codeSystem.getUrl().getValue();
java.lang.String displayToValidate = isCaseSensitive ? coding.getDisplay().getValue() : normalize(coding.getDisplay().getValue());

boolean result = false;
if (isCaseSensitive) {
result = lookupOutcome.getDisplay().getValue().equals(displayToValidate);
if (!result) {
// call it valid if any of the designations match the display value
result = lookupOutcome.getDesignation().stream()
.anyMatch(d -> d.getValue() != null && displayToValidate.equals(d.getValue().getValue()));
}
} else {
result = normalize(lookupOutcome.getDisplay().getValue()).equals(displayToValidate);
if (!result) {
// call it valid if any of the normalized designations match the normalized display value
result = lookupOutcome.getDesignation().stream()
.anyMatch(d -> d.getValue() != null && displayToValidate.equals(normalize(d.getValue().getValue())));
}
}
boolean caseSensitive = (codeSystem != null) ? CodeSystemSupport.isCaseSensitive(codeSystem) : false;
if (codeSystem == null && system != null) {
java.lang.String version = (coding.getVersion() != null) ? coding.getVersion().getValue() : null;
java.lang.String url = (version != null) ? system + "|" + version : system;
caseSensitive = CodeSystemSupport.isCaseSensitive(url);

java.lang.String message = null;
if (!result) {
java.lang.String system = (coding.getSystem() != null) ? coding.getSystem().getValue() : null;
message = java.lang.String.format("The display '%s' is incorrect for code '%s' from code system '%s'",
coding.getDisplay().getValue(), coding.getCode().getValue(), system);
}
boolean result = caseSensitive ? lookupOutcome.getDisplay().equals(coding.getDisplay()) :
normalize(lookupOutcome.getDisplay().getValue()).equals(normalize(coding.getDisplay().getValue()));
java.lang.String message = !result ? java.lang.String.format("The display '%s' is incorrect for code '%s' from code system '%s'",
coding.getDisplay().getValue(), coding.getCode().getValue(), system) : null;

return buildValidationOutcome(result, message, lookupOutcome);
}
Expand Down
77 changes: 77 additions & 0 deletions term/fhir-term/src/test/java/com/ibm/fhir/term/service/test/FHIRTermServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import static com.ibm.fhir.term.util.ValueSetSupport.getContains;
import static com.ibm.fhir.term.util.ValueSetSupport.getValueSet;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertTrue;

Expand Down Expand Up @@ -585,4 +586,80 @@ public void testTranslate2() throws Exception {

assertEquals(outcome, expected);
}

@Test
public void testValidateDisplay_caseSensitive() throws Exception {
CodeSystem codeSystem = getCodeSystem("http://ibm.com/fhir/CodeSystem/cs1");
Coding coding = Coding.builder()
.system(Uri.of("http://ibm.com/fhir/CodeSystem/cs1"))
.code(Code.of("a"))
.build();;
ValidationOutcome outcome;

coding = coding.toBuilder()
.display("Concept a")
.build();
outcome = FHIRTermService.getInstance().validateCode(codeSystem, coding);
assertTrue(outcome.getResult().getValue(),
(outcome.getMessage() != null) ? outcome.getMessage().getValue() : outcome.getDisplay().getValue());

coding = coding.toBuilder()
.display("a")
.build();
outcome = FHIRTermService.getInstance().validateCode(codeSystem, coding);
assertTrue(outcome.getResult().getValue(),
(outcome.getMessage() != null) ? outcome.getMessage().getValue() : outcome.getDisplay().getValue());

coding = Coding.builder()
.display("Concept A")
.build();
outcome = FHIRTermService.getInstance().validateCode(codeSystem, coding);
assertFalse(outcome.getResult().getValue(),
(outcome.getMessage() != null) ? outcome.getMessage().getValue() : outcome.getDisplay().getValue());

coding = Coding.builder()
.display("A")
.build();
outcome = FHIRTermService.getInstance().validateCode(codeSystem, coding);
assertFalse(outcome.getResult().getValue(),
(outcome.getMessage() != null) ? outcome.getMessage().getValue() : outcome.getDisplay().getValue());
}

@Test
public void testValidateDisplay_caseInsensitive() throws Exception {
CodeSystem codeSystem = getCodeSystem("http://ibm.com/fhir/CodeSystem/cs2");
Coding coding = Coding.builder()
.system(Uri.of("http://ibm.com/fhir/CodeSystem/cs2"))
.code(Code.of("d"))
.build();
ValidationOutcome outcome;

coding = coding.toBuilder()
.display("Concept d")
.build();
outcome = FHIRTermService.getInstance().validateCode(codeSystem, coding);
assertTrue(outcome.getResult().getValue(),
(outcome.getMessage() != null) ? outcome.getMessage().getValue() : outcome.getDisplay().getValue());

coding = coding.toBuilder()
.display("d")
.build();
outcome = FHIRTermService.getInstance().validateCode(codeSystem, coding);
assertTrue(outcome.getResult().getValue(),
(outcome.getMessage() != null) ? outcome.getMessage().getValue() : outcome.getDisplay().getValue());

coding = coding.toBuilder()
.display("Concept D")
.build();
outcome = FHIRTermService.getInstance().validateCode(codeSystem, coding);
assertTrue(outcome.getResult().getValue(),
(outcome.getMessage() != null) ? outcome.getMessage().getValue() : outcome.getDisplay().getValue());

coding = coding.toBuilder()
.display("D")
.build();
outcome = FHIRTermService.getInstance().validateCode(codeSystem, coding);
assertTrue(outcome.getResult().getValue(),
(outcome.getMessage() != null) ? outcome.getMessage().getValue() : outcome.getDisplay().getValue());
}
}
5 changes: 4 additions & 1 deletion term/fhir-term/src/test/resources/fhir/term/service/test/package/CodeSystem-cs1.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@
"concept": [
{
"code": "a",
"display": "Concept a"
"display": "Concept a",
"designation": [{
"value": "a"
}]
},
{
"code": "b",
Expand Down
5 changes: 4 additions & 1 deletion term/fhir-term/src/test/resources/fhir/term/service/test/package/CodeSystem-cs2.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@
"concept": [
{
"code": "d",
"display": "Concept d"
"display": "Concept d",
"designation": [{
"value": "d"
}]
},
{
"code": "e",
Expand Down

0 comments on commit 7a6eb5d

Please sign in to comment.