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 #2292 - error handling and memberOf optimization #2323

Merged
merged 5 commits into from
May 7, 2021
Merged

Issue #2292 - error handling and memberOf optimization #2323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c351ce5
Issue #2292 - error handling and memberOf optimization
JohnTimm May 7, 2021
f2f8a1f
Issue #2292 - minor tweak
JohnTimm May 7, 2021
a295190
Issue #2292 - enable caching on RegistryTermServiceProvider.getConcept
JohnTimm May 7, 2021
5795211
Issue #2292 - added performance test
JohnTimm May 7, 2021
d7471c1
Issue #2292 - added try-catch block in memberOf function
JohnTimm May 7, 2021
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
174 changes: 137 additions & 37 deletions fhir-path/src/main/java/com/ibm/fhir/path/function/MemberOfFunction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@
import java.util.Collection;
import java.util.List;

import com.ibm.fhir.model.resource.CodeSystem;
import com.ibm.fhir.model.resource.ValueSet;
import com.ibm.fhir.model.resource.ValueSet.Compose;
import com.ibm.fhir.model.resource.ValueSet.Compose.Include;
import com.ibm.fhir.model.type.Boolean;
import com.ibm.fhir.model.type.Code;
import com.ibm.fhir.model.type.CodeableConcept;
Expand All @@ -41,6 +44,8 @@
import com.ibm.fhir.path.evaluator.FHIRPathEvaluator.EvaluationContext;
import com.ibm.fhir.term.service.FHIRTermService;
import com.ibm.fhir.term.service.ValidationOutcome;
import com.ibm.fhir.term.service.exception.FHIRTermServiceException;
import com.ibm.fhir.term.util.CodeSystemSupport;

/**
* Implementation of the 'memberOf' FHIRPath function per: <a href="http://hl7.org/fhir/fhirpath.html#functions">http://hl7.org/fhir/fhirpath.html#functions</a>
Expand Down Expand Up @@ -96,48 +101,48 @@ public Collection<FHIRPathNode> apply(EvaluationContext evaluationContext, Colle

ValueSet valueSet = getValueSet(url);
if (valueSet != null) {

// Validate against data-absent-reason extension (only if using extended version of operation)
if (strength != null && ValidationSupport.hasOnlyDataAbsentReasonExtension(element)) {
return SINGLETON_TRUE;
}

FHIRTermService service = FHIRTermService.getInstance();
if (isExpanded(valueSet) || service.isExpandable(valueSet)) {

// Validate against expanded value set
if (element.is(Code.class)) {
Uri system = getSystem(evaluationContext.getTree(), elementNode);
Code code = element.as(Code.class);
if ((system != null && validateCode(service, valueSet, system, null, code, null, evaluationContext, elementNode, strength)) ||
(system == null && validateCode(service, valueSet, code, evaluationContext, elementNode, strength))) {
return SINGLETON_TRUE;
}
} else if (element.is(Coding.class)) {
Coding coding = element.as(Coding.class);
if (validateCode(service, valueSet, coding, evaluationContext, elementNode, strength)) {
return SINGLETON_TRUE;
}
} else if (element.is(CodeableConcept.class)) {
CodeableConcept codeableConcept = element.as(CodeableConcept.class);
if (codeableConcept.getCoding() != null && validateCode(service, valueSet, codeableConcept, evaluationContext, elementNode, strength)) {
return SINGLETON_TRUE;
}
} else if (element.is(Quantity.class)) {
Quantity quantity = element.as(Quantity.class);
if (validateCode(service, valueSet, quantity.getSystem(), null, quantity.getCode(), null, evaluationContext, elementNode, strength)) {
return SINGLETON_TRUE;
}
} else {
// element.is(FHIR_STRING) || element.is(Uri.class)
Code code = element.is(FHIR_STRING) ? Code.of(element.as(FHIR_STRING).getValue()) : Code.of(element.as(Uri.class).getValue());
if (validateCode(service, valueSet, code, evaluationContext, elementNode, strength)) {
return SINGLETON_TRUE;
try {
// Validate against expanded value set
if (element.is(Code.class)) {
Uri system = getSystem(evaluationContext.getTree(), elementNode);
Code code = element.as(Code.class);
if ((system != null && validateCode(service, valueSet, system, null, code, null, evaluationContext, elementNode, strength)) ||
(system == null && validateCode(service, valueSet, code, evaluationContext, elementNode, strength))) {
return SINGLETON_TRUE;
}
} else if (element.is(Coding.class)) {
Coding coding = element.as(Coding.class);
if (validateCode(service, valueSet, coding, evaluationContext, elementNode, strength)) {
return SINGLETON_TRUE;
}
} else if (element.is(CodeableConcept.class)) {
CodeableConcept codeableConcept = element.as(CodeableConcept.class);
if (codeableConcept.getCoding() != null && validateCode(service, valueSet, codeableConcept, evaluationContext, elementNode, strength)) {
return SINGLETON_TRUE;
}
} else if (element.is(Quantity.class)) {
Quantity quantity = element.as(Quantity.class);
if (validateCode(service, valueSet, quantity.getSystem(), null, quantity.getCode(), null, evaluationContext, elementNode, strength)) {
return SINGLETON_TRUE;
}
} else {
// element.is(FHIR_STRING) || element.is(Uri.class)
Code code = element.is(FHIR_STRING) ? Code.of(element.as(FHIR_STRING).getValue()) : Code.of(element.as(Uri.class).getValue());
if (validateCode(service, valueSet, code, evaluationContext, elementNode, strength)) {
return SINGLETON_TRUE;
}
}
return membershipCheckFailed(evaluationContext, elementNode, url, strength);
} catch (FHIRTermServiceException e) {
generateIssue(evaluationContext, IssueSeverity.WARNING, IssueType.INCOMPLETE, "Membership check was not performed: value set '" + url + "' could not be expanded due to the following error: " + e.getMessage(), elementNode.path());
}
return membershipCheckFailed(evaluationContext, elementNode, url, strength);
} else if (isSyntaxBased(valueSet)) {

// Validate against syntax-based value set
try {
ValidationSupport.checkValueSetBinding(elementNode.element(), elementNode.path(), valueSet.getUrl().getValue(), null);
Expand Down Expand Up @@ -179,7 +184,14 @@ private boolean validateCode(FHIRTermService service, ValueSet valueSet, Uri sys
if (system == null && version == null && code == null && display == null) {
return false;
}
ValidationOutcome outcome = service.validateCode(valueSet, system, version, code, display);
ValidationOutcome outcome = null;
if (convertsToCodeSystemValidateCode(valueSet, system, version, code)) {
// optimization
CodeSystem codeSystem = getCodeSystem(valueSet, system, version);
outcome = service.validateCode(codeSystem, code, display);
} else {
outcome = service.validateCode(valueSet, system, version, code, display);
}
if (Boolean.FALSE.equals(outcome.getResult())) {
generateIssue(outcome, evaluationContext, elementNode, strength);
return false;
Expand All @@ -188,7 +200,14 @@ private boolean validateCode(FHIRTermService service, ValueSet valueSet, Uri sys
}

private boolean validateCode(FHIRTermService service, ValueSet valueSet, Coding coding, EvaluationContext evaluationContext, FHIRPathElementNode elementNode, String strength) {
ValidationOutcome outcome = service.validateCode(valueSet, coding);
ValidationOutcome outcome = null;
if (convertsToCodeSystemValidateCode(valueSet, coding)) {
// optimization
CodeSystem codeSystem = getCodeSystem(valueSet, coding);
outcome = service.validateCode(codeSystem, coding);
} else {
outcome = service.validateCode(valueSet, coding);
}
if (Boolean.FALSE.equals(outcome.getResult())) {
generateIssue(outcome, evaluationContext, elementNode, strength);
return false;
Expand All @@ -197,7 +216,25 @@ private boolean validateCode(FHIRTermService service, ValueSet valueSet, Coding
}

private boolean validateCode(FHIRTermService service, ValueSet valueSet, CodeableConcept codeableConcept, EvaluationContext evaluationContext, FHIRPathElementNode elementNode, String strength) {
ValidationOutcome outcome = service.validateCode(valueSet, codeableConcept);
ValidationOutcome outcome = null;
if (convertsToCodeSystemValidateCode(valueSet, codeableConcept)) {
// optimization
for (Coding coding : codeableConcept.getCoding()) {
CodeSystem codeSystem = getCodeSystem(valueSet, coding);
outcome = service.validateCode(codeSystem, coding);
if (Boolean.TRUE.equals(outcome.getResult())) {
break;
}
}
if (Boolean.FALSE.equals(outcome.getResult())) {
outcome = outcome.toBuilder()
.message(null)
.display(null)
.build();
}
} else {
outcome = service.validateCode(valueSet, codeableConcept);
}
if (Boolean.FALSE.equals(outcome.getResult())) {
generateIssue(outcome, evaluationContext, elementNode, strength);
return false;
Expand Down Expand Up @@ -250,7 +287,7 @@ private Collection<FHIRPathNode> membershipCheckFailed(EvaluationContext evaluat
* @return
* the URI-typed sibling of the given element node with name "system", or null if no such sibling exists
*/
protected Uri getSystem(FHIRPathTree tree, FHIRPathElementNode elementNode) {
private Uri getSystem(FHIRPathTree tree, FHIRPathElementNode elementNode) {
if (tree != null) {
FHIRPathNode systemNode = tree.getSibling(elementNode, "system");
if (systemNode != null && FHIRPathType.FHIR_URI.equals(systemNode.type())) {
Expand All @@ -259,4 +296,67 @@ protected Uri getSystem(FHIRPathTree tree, FHIRPathElementNode elementNode) {
}
return null;
}

private boolean convertsToCodeSystemValidateCode(ValueSet valueSet, CodeableConcept codeableConcept) {
if (codeableConcept.getCoding().isEmpty()) {
return false;
}
for (Coding coding : codeableConcept.getCoding()) {
if (!convertsToCodeSystemValidateCode(valueSet, coding)) {
return false;
}
}
return true;
}

private boolean convertsToCodeSystemValidateCode(ValueSet valueSet, Coding coding) {
return convertsToCodeSystemValidateCode(valueSet, coding.getSystem(), coding.getVersion(), coding.getCode());
}

private boolean convertsToCodeSystemValidateCode(ValueSet valueSet, Uri system, com.ibm.fhir.model.type.String version, Code code) {
if (system == null || system.getValue() == null || code == null || code.getValue() == null) {
return false;
}

if (isExpanded(valueSet)) {
return false;
}

Compose compose = valueSet.getCompose();

if (!compose.getExclude().isEmpty()) {
return false;
}

for (Include include : compose.getInclude()) {
if (!include.getConcept().isEmpty() ||
!include.getFilter().isEmpty() ||
!include.getValueSet().isEmpty() ||
include.getSystem() == null) {
return false;
}
}

return hasCodeSystem(valueSet, system, version);
}

private boolean hasCodeSystem(ValueSet valueSet, Uri system, com.ibm.fhir.model.type.String version) {
return getCodeSystem(valueSet, system, version) != null;
}

private CodeSystem getCodeSystem(ValueSet valueSet, Coding coding) {
return getCodeSystem(valueSet, coding.getSystem(), coding.getVersion());
}

private CodeSystem getCodeSystem(ValueSet valueSet, Uri system, com.ibm.fhir.model.type.String version) {
Compose compose = valueSet.getCompose();
for (Include include : compose.getInclude()) {
if (include.getSystem().equals(system) &&
(include.getVersion() == null || version == null || include.getVersion().equals(version))) {
String url = (version != null && version.getValue() != null) ? system.getValue() + "|" + version.getValue() : system.getValue();
return CodeSystemSupport.getCodeSystem(url);
}
}
return null;
}
}
30 changes: 25 additions & 5 deletions ...erm-remote/src/main/java/com/ibm/fhir/term/remote/provider/RemoteTermServiceProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,12 @@
import com.ibm.fhir.model.resource.ValueSet.Compose.Include;
import com.ibm.fhir.model.resource.ValueSet.Compose.Include.Filter;
import com.ibm.fhir.model.type.Code;
import com.ibm.fhir.model.type.CodeableConcept;
import com.ibm.fhir.model.type.Coding;
import com.ibm.fhir.model.type.code.ConceptSubsumptionOutcome;
import com.ibm.fhir.model.type.code.FilterOperator;
import com.ibm.fhir.model.type.code.IssueSeverity;
import com.ibm.fhir.model.type.code.IssueType;
import com.ibm.fhir.model.type.code.PublicationStatus;
import com.ibm.fhir.provider.FHIRJsonProvider;
import com.ibm.fhir.provider.FHIRProvider;
Expand Down Expand Up @@ -241,7 +244,7 @@ public <R> Set<R> getConcepts(CodeSystem codeSystem, List<Filter> filters, Funct
return result;
}

throw errorOccurred(response, "ValueSet $expand");
throw errorOccurred(response, "ValueSet", "expand");
} finally {
if (response != null) {
response.close();
Expand Down Expand Up @@ -356,7 +359,7 @@ public boolean subsumes(CodeSystem codeSystem, Code codeA, Code codeB) {
}
}

throw errorOccurred(response, "CodeSystem $subsumes");
throw errorOccurred(response, "CodeSystem", "subsumes");
} finally {
if (response != null) {
response.close();
Expand All @@ -382,15 +385,32 @@ private Parameters buildValueSetExpandParameters(CodeSystem codeSystem, List<Fil
.build();
}

private FHIRTermServiceException errorOccurred(Response response, String op) {
private FHIRTermServiceException errorOccurred(Response response, String type, String op) {
OperationOutcome outcome = null;
try {
outcome = response.readEntity(OperationOutcome.class);
} catch (IllegalArgumentException | ProcessingException e) {
log.log(Level.SEVERE, "An error occurred while reading the entity", e);
}
List<Issue> issues = (outcome != null) ? outcome.getIssue() : Collections.emptyList();
return new FHIRTermServiceException("An error occurred during the " + op + " operation", issues);

String message = String.format("RemoteTermServiceProvider: a communication or processing error occurred during the remote %s %s operation", type, op);

List<Issue> issues = new ArrayList<>();

issues.add(Issue.builder()
.severity(IssueSeverity.ERROR)
.code(IssueType.EXCEPTION)
.details(CodeableConcept.builder()
.text(string(message))
.build())
.diagnostics(string(String.format("Remote endpoint: %s/%s/$%s", base, type, op)))
.build());

if (outcome != null) {
issues.addAll(outcome.getIssue());
}

return new FHIRTermServiceException(message, issues);
}

private KeyStore loadKeyStoreFile(Configuration.TrustStore trustStore) {
Expand Down
4 changes: 3 additions & 1 deletion 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 @@ -24,6 +24,7 @@
import java.util.function.Function;
import java.util.stream.Collectors;

import com.ibm.fhir.cache.CachingProxy;
import com.ibm.fhir.model.resource.CodeSystem;
import com.ibm.fhir.model.resource.CodeSystem.Concept;
import com.ibm.fhir.model.resource.ConceptMap;
Expand All @@ -40,6 +41,7 @@
import com.ibm.fhir.model.type.Uri;
import com.ibm.fhir.model.type.code.CodeSystemHierarchyMeaning;
import com.ibm.fhir.model.type.code.ConceptSubsumptionOutcome;
import com.ibm.fhir.term.config.FHIRTermConfig;
import com.ibm.fhir.term.service.LookupOutcome.Designation;
import com.ibm.fhir.term.service.LookupOutcome.Property;
import com.ibm.fhir.term.service.TranslationOutcome.Match;
Expand Down Expand Up @@ -1011,7 +1013,7 @@ private Uri getSource(ConceptMap conceptMap) {

private List<FHIRTermServiceProvider> loadProviders() {
List<FHIRTermServiceProvider> providers = new ArrayList<>();
providers.add(new RegistryTermServiceProvider());
providers.add(FHIRTermConfig.isCachingDisabled() ? new RegistryTermServiceProvider() : CachingProxy.newInstance(FHIRTermServiceProvider.class, new RegistryTermServiceProvider()));
Iterator<FHIRTermServiceProvider> iterator = ServiceLoader.load(FHIRTermServiceProvider.class).iterator();
while (iterator.hasNext()) {
providers.add(iterator.next());
Expand Down
2 changes: 2 additions & 0 deletions fhir-term/src/main/java/com/ibm/fhir/term/service/provider/RegistryTermServiceProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import java.util.Set;
import java.util.function.Function;

import com.ibm.fhir.cache.annotation.Cacheable;
import com.ibm.fhir.model.resource.CodeSystem;
import com.ibm.fhir.model.resource.CodeSystem.Concept;
import com.ibm.fhir.model.resource.ValueSet.Compose.Include.Filter;
Expand All @@ -31,6 +32,7 @@ public Set<Concept> closure(CodeSystem codeSystem, Code code) {
return CodeSystemSupport.getConcepts(concept);
}

@Cacheable
@Override
public Concept getConcept(CodeSystem codeSystem, Code code) {
checkArguments(codeSystem, code);
Expand Down
3 changes: 3 additions & 0 deletions fhir-term/src/main/java/com/ibm/fhir/term/util/ValueSetSupport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import com.ibm.fhir.registry.resource.FHIRRegistryResource;
import com.ibm.fhir.term.config.FHIRTermConfig;
import com.ibm.fhir.term.service.FHIRTermService;
import com.ibm.fhir.term.service.exception.FHIRTermServiceException;

/**
* A utility class for expanding FHIR value sets
Expand Down Expand Up @@ -211,6 +212,8 @@ private static Map<java.lang.String, Set<java.lang.String>> computeCodeSetMap(Va
}
}
return codeSetMap;
} catch (FHIRTermServiceException e) {
throw e;
} catch (Exception e) {
java.lang.String url = (valueSet.getUrl() != null) ? valueSet.getUrl().getValue() : "<no url>";
java.lang.String version = (valueSet.getVersion() != null) ? valueSet.getVersion().getValue() : "<no version>";
Expand Down
Loading