Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

issue #3712 - check all CodeSystem concept designations #3771

Merged
merged 2 commits into from
Jul 13, 2022
Merged

issue #3712 - check all CodeSystem concept designations #3771

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
423d04e
issue #3712 - check all CodeSystem concept designations
lmsurpre Jul 11, 2022
20bcb33
Apply suggestions from code review
lmsurpre Jul 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 69 additions & 20 deletions term/fhir-term/src/main/java/com/ibm/fhir/term/service/FHIRTermService.java
View file
Open in desktop
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;
punktilious marked this conversation as resolved.
Show resolved Hide resolved
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()
lmsurpre marked this conversation as resolved.
Show resolved Hide resolved
.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()
lmsurpre marked this conversation as resolved.
Show resolved Hide resolved
.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
Open in desktop
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
Open in desktop
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
Open in desktop
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