From 7a6eb5d1df7c01c786f522a9469759b696ad4b7d Mon Sep 17 00:00:00 2001 From: Lee Surprenant Date: Wed, 13 Jul 2022 11:34:56 -0500 Subject: [PATCH] issue #3712 - check all CodeSystem concept designations (#3771) * 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 * Apply suggestions from code review Signed-off-by: Lee Surprenant --- .../fhir/term/service/FHIRTermService.java | 89 ++++++++++++++----- .../service/test/FHIRTermServiceTest.java | 77 ++++++++++++++++ .../service/test/package/CodeSystem-cs1.json | 5 +- .../service/test/package/CodeSystem-cs2.json | 5 +- 4 files changed, 154 insertions(+), 22 deletions(-) diff --git a/term/fhir-term/src/main/java/com/ibm/fhir/term/service/FHIRTermService.java b/term/fhir-term/src/main/java/com/ibm/fhir/term/service/FHIRTermService.java index ad9013dfe0f..22d834304bb 100644 --- a/term/fhir-term/src/main/java/com/ibm/fhir/term/service/FHIRTermService.java +++ b/term/fhir-term/src/main/java/com/ibm/fhir/term/service/FHIRTermService.java @@ -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)) { @@ -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) { @@ -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 @@ -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 @@ -1167,29 +1166,79 @@ private List 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); } diff --git a/term/fhir-term/src/test/java/com/ibm/fhir/term/service/test/FHIRTermServiceTest.java b/term/fhir-term/src/test/java/com/ibm/fhir/term/service/test/FHIRTermServiceTest.java index b3844aa4145..071f23e09e8 100644 --- a/term/fhir-term/src/test/java/com/ibm/fhir/term/service/test/FHIRTermServiceTest.java +++ b/term/fhir-term/src/test/java/com/ibm/fhir/term/service/test/FHIRTermServiceTest.java @@ -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; @@ -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()); + } } diff --git a/term/fhir-term/src/test/resources/fhir/term/service/test/package/CodeSystem-cs1.json b/term/fhir-term/src/test/resources/fhir/term/service/test/package/CodeSystem-cs1.json index 213798e323d..ec33ba8ff83 100644 --- a/term/fhir-term/src/test/resources/fhir/term/service/test/package/CodeSystem-cs1.json +++ b/term/fhir-term/src/test/resources/fhir/term/service/test/package/CodeSystem-cs1.json @@ -10,7 +10,10 @@ "concept": [ { "code": "a", - "display": "Concept a" + "display": "Concept a", + "designation": [{ + "value": "a" + }] }, { "code": "b", diff --git a/term/fhir-term/src/test/resources/fhir/term/service/test/package/CodeSystem-cs2.json b/term/fhir-term/src/test/resources/fhir/term/service/test/package/CodeSystem-cs2.json index aa763327569..1c9298cb54b 100644 --- a/term/fhir-term/src/test/resources/fhir/term/service/test/package/CodeSystem-cs2.json +++ b/term/fhir-term/src/test/resources/fhir/term/service/test/package/CodeSystem-cs2.json @@ -10,7 +10,10 @@ "concept": [ { "code": "d", - "display": "Concept d" + "display": "Concept d", + "designation": [{ + "value": "d" + }] }, { "code": "e",