Merge pull request #307 from hapifhir/gg-v512

value set validation improvements

RELEASE_NOTES.md

@@ -0,0 +1,14 @@
+Validator:
+* add support for -bundle parameter to allow validating just one resource (/type) in a bundle
+* improved reporting of errors and warnings for unknown code systems on required bindings
+* pass dependencies to the server for imported value sets etc
+* use server side caching for more efficient use of bandwidth 
+* Fix NPE loading packages from simplifier or old packages (and don't lazy load packages passed to command line)
+
+Other code changes:
+* further work on comparing CapabilityStatements (nearly, but not quite, finished)
+* More work on timeouts in terminology client
+* Fix for parsing error in R3/R4 sparse arrays for primitives types
+* Improve terminology client logging
+* don't reload a package if already loaded
+* rendering: fix NPEs rendering patient summary, and render expressions for quantities

org.hl7.fhir.convertors/src/main/java/org/hl7/fhir/convertors/VersionConvertor_10_50.java

@@ -3439,6 +3439,8 @@ public class VersionConvertor_10_50 {
         for (ParametersParameterComponent p : src.getParameter()) {
             if (p.getName().equals("system"))
                 res.addCodeSystem().setUri(p.getValue().primitiveValue());
+            if (p.getName().equals("expansion.parameter"))
+              res.getExpansion().addParameter().setName(p.getValue().primitiveValue());
         }
         return res;
     }

org.hl7.fhir.convertors/src/main/java/org/hl7/fhir/convertors/VersionConvertor_30_50.java

@@ -5539,6 +5539,8 @@ public class VersionConvertor_30_50 {
         for (ParametersParameterComponent p : src.getParameter()) {
             if (p.getName().equals("system"))
                 res.addCodeSystem().setUri(p.getValue().primitiveValue());
+            if (p.getName().equals("expansion.parameter"))
+              res.getExpansion().addParameter().setName(p.getValue().primitiveValue());
         }
         return res;
     }

org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/BaseWorkerContext.java

@@ -90,6 +90,7 @@ import org.hl7.fhir.r5.model.StructureDefinition.TypeDerivationRule;
 import org.hl7.fhir.r5.model.StructureMap;
 import org.hl7.fhir.r5.model.TerminologyCapabilities;
 import org.hl7.fhir.r5.model.TerminologyCapabilities.TerminologyCapabilitiesCodeSystemComponent;
+import org.hl7.fhir.r5.model.TerminologyCapabilities.TerminologyCapabilitiesExpansionParameterComponent;
 import org.hl7.fhir.r5.model.UriType;
 import org.hl7.fhir.r5.model.ValueSet;
 import org.hl7.fhir.r5.model.Bundle.BundleEntryComponent;
@@ -156,6 +157,7 @@ public abstract class BaseWorkerContext extends I18nBase implements IWorkerConte
   private Object lock = new Object(); // used as a lock for the data that follows
   protected String version;
   private String cacheId;
+  private boolean isTxCaching;
   private Set<String> cached = new HashSet<>();
   
   private Map<String, Map<String, Resource>> allResourcesById = new HashMap<String, Map<String, Resource>>();
@@ -504,7 +506,7 @@ public abstract class BaseWorkerContext extends I18nBase implements IWorkerConte
         if (txcaps == null) {
           try {
             log("Terminology server: Check for supported code systems for "+system);
-            txcaps = txClient.getTerminologyCapabilities();
+            setTxCaps(txClient.getTerminologyCapabilities());
           } catch (Exception e) {
             if (canRunWithoutTerminology) {
               noTerminologyServer = true;
@@ -520,11 +522,6 @@ public abstract class BaseWorkerContext extends I18nBase implements IWorkerConte
               throw new TerminologyServiceException(e);
             }
           }
-          if (txcaps != null) {
-            for (TerminologyCapabilitiesCodeSystemComponent tccs : txcaps.getCodeSystem()) {
-              supportedCodeSystems.add(tccs.getUri());
-            }
-          }
           if (supportedCodeSystems.contains(system)) {
             return true;
           }
@@ -588,7 +585,7 @@ public abstract class BaseWorkerContext extends I18nBase implements IWorkerConte
     Parameters p = expParameters.copy(); 
     p.setParameter("includeDefinition", false);
     p.setParameter("excludeNested", !hierarchical);
-    if (cacheId != null) {
+    if (isTxCaching && cacheId != null) {
       p.addParameter().setName("cache-id").setValue(new StringType(cacheId));              
     }
     addDependentResources(p, vs);
@@ -663,7 +660,7 @@ public abstract class BaseWorkerContext extends I18nBase implements IWorkerConte
     }
     
     // if that failed, we try to expand on the server
-    if (cacheId != null) {
+    if (isTxCaching && cacheId != null) {
       p.addParameter().setName("cache-id").setValue(new StringType(cacheId));              
     }
     addDependentResources(p, vs);
@@ -927,11 +924,11 @@ public abstract class BaseWorkerContext extends I18nBase implements IWorkerConte
   }
 
   private ValidationResult validateOnServer(ValueSet vs, Parameters pin) throws FHIRException {
-    if (cacheId != null) {
+    if (isTxCaching && cacheId != null) {
       pin.addParameter().setName("cache-id").setValue(new StringType(cacheId));              
     }
     if (vs != null) {
-      if (cacheId != null && cached.contains(vs.getUrl()+"|"+vs.getVersion())) {
+      if (isTxCaching && cacheId != null && cached.contains(vs.getUrl()+"|"+vs.getVersion())) {
         pin.addParameter().setName("url").setValue(new UriType(vs.getUrl()+"|"+vs.getVersion()));        
       } else {
         pin.addParameter().setName("valueSet").setResource(vs);
@@ -976,7 +973,7 @@ public abstract class BaseWorkerContext extends I18nBase implements IWorkerConte
     for (CanonicalType c : inc.getValueSet()) {
       ValueSet vs = fetchResource(ValueSet.class, c.getValue());
       if (vs != null) {
-        if (cacheId == null || !cached.contains(vs.getVUrl())) {
+        if (isTxCaching && cacheId == null || !cached.contains(vs.getVUrl())) {
           pin.addParameter().setName("tx-resource").setResource(vs);
           cached.add(vs.getVUrl());
         }
@@ -985,7 +982,7 @@ public abstract class BaseWorkerContext extends I18nBase implements IWorkerConte
     }
     CodeSystem cs = fetchResource(CodeSystem.class, inc.getSystem());
     if (cs != null) {
-      if (cacheId == null || !cached.contains(cs.getVUrl())) {
+      if (isTxCaching && cacheId == null || !cached.contains(cs.getVUrl())) {
         pin.addParameter().setName("tx-resource").setResource(cs);
         cached.add(cs.getVUrl());
       }
@@ -1859,4 +1856,23 @@ public abstract class BaseWorkerContext extends I18nBase implements IWorkerConte
     this.cacheId = cacheId;
   }
 
+  public TerminologyCapabilities getTxCaps() {
+    return txcaps;
+  }
+
+  public void setTxCaps(TerminologyCapabilities txCaps) {
+    this.txcaps = txCaps;
+    if (txCaps != null) {
+      for (TerminologyCapabilitiesExpansionParameterComponent t : txcaps.getExpansion().getParameter()) {
+        if ("cache-id".equals(t.getName())) {
+          isTxCaching = true;
+        }
+      }
+      for (TerminologyCapabilitiesCodeSystemComponent tccs : txcaps.getCodeSystem()) {
+        supportedCodeSystems.add(tccs.getUri());
+      }
+    }
+  }
+
+  
 }

org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/SimpleWorkerContext.java

@@ -63,6 +63,7 @@ import org.hl7.fhir.r5.formats.XmlParser;
 import org.hl7.fhir.r5.model.Bundle;
 import org.hl7.fhir.r5.model.Bundle.BundleEntryComponent;
 import org.hl7.fhir.r5.model.CanonicalResource;
+import org.hl7.fhir.r5.model.CapabilityStatement;
 import org.hl7.fhir.r5.model.ElementDefinition.ElementDefinitionBindingComponent;
 import org.hl7.fhir.r5.model.Questionnaire;
 import org.hl7.fhir.r5.model.Resource;
@@ -293,7 +294,9 @@ public class SimpleWorkerContext extends BaseWorkerContext implements IWorkerCon
         txLog = new HTMLClientLogger(log);
       }
       txClient.setLogger(txLog);
-      return txClient.getCapabilitiesStatementQuick().getSoftware().getVersion();
+      CapabilityStatement cps = txClient.getCapabilitiesStatementQuick();
+      setTxCaps(txClient.getTerminologyCapabilities());
+      return cps.getSoftware().getVersion();
     } catch (Exception e) {
       throw new FHIRException(formatMessage(I18nConstants.UNABLE_TO_CONNECT_TO_TERMINOLOGY_SERVER_USE_PARAMETER_TX_NA_TUN_RUN_WITHOUT_USING_TERMINOLOGY_SERVICES_TO_VALIDATE_LOINC_SNOMED_ICDX_ETC_ERROR__, e.getMessage()), e);
     }

org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/renderers/ValueSetRenderer.java

@@ -6,17 +6,23 @@ import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Date;
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
+import java.util.Set;
 
 import org.hl7.fhir.exceptions.DefinitionException;
 import org.hl7.fhir.exceptions.FHIRException;
 import org.hl7.fhir.exceptions.FHIRFormatError;
 import org.hl7.fhir.exceptions.TerminologyServiceException;
+import org.hl7.fhir.r5.context.IWorkerContext.CodingValidationRequest;
 import org.hl7.fhir.r5.context.IWorkerContext.ValidationResult;
 import org.hl7.fhir.r5.model.BooleanType;
 import org.hl7.fhir.r5.model.CanonicalResource;
 import org.hl7.fhir.r5.model.CodeSystem;
 import org.hl7.fhir.r5.model.CodeSystem.ConceptDefinitionComponent;
+import org.hl7.fhir.r5.model.Coding;
 import org.hl7.fhir.r5.model.ConceptMap;
 import org.hl7.fhir.r5.model.DataType;
 import org.hl7.fhir.r5.model.DomainResource;
@@ -182,7 +188,7 @@ public class ValueSetRenderer extends TerminologyRenderer {
         p.addText(" and may be missing codes, or include codes that are not valid");
       }
     }
-
+    
     generateVersionNotice(x, vs.getExpansion());
 
     CodeSystem allCS = null;
@@ -737,6 +743,9 @@ public class ValueSetRenderer extends TerminologyRenderer {
             li.code(inc.getVersion()); 
           }
 
+          // for performance reasons, we do all the fetching in one batch
+          Map<String, ConceptDefinitionComponent> definitions = getConceptsForCodes(e, inc);
+          
           XhtmlNode t = li.table("none");
           boolean hasComments = false;
           boolean hasDefinition = false;
@@ -750,7 +759,7 @@ public class ValueSetRenderer extends TerminologyRenderer {
           for (ConceptReferenceComponent c : inc.getConcept()) {
             XhtmlNode tr = t.tr();
             XhtmlNode td = tr.td();
-            ConceptDefinitionComponent cc = getConceptForCode(e, c.getCode(), inc);
+            ConceptDefinitionComponent cc = definitions.get(c.getCode()); 
             addCodeToTable(false, inc.getSystem(), c.getCode(), c.hasDisplay()? c.getDisplay() : cc != null ? cc.getDisplay() : "", td);
 
             td = tr.td();
@@ -844,24 +853,13 @@ public class ValueSetRenderer extends TerminologyRenderer {
   }
 
 
-  private ConceptDefinitionComponent getConceptForCode(CodeSystem e, String code, ConceptSetComponent inc) {
+  private Map<String, ConceptDefinitionComponent> getConceptsForCodes(CodeSystem e, ConceptSetComponent inc) {
-    if (code == null) {
+    if (e == null) {
-      return null;
+      e = getContext().getWorker().fetchCodeSystem(inc.getSystem());
     }
-    // first, look in the code systems
-    if (e == null)
-    e = getContext().getWorker().fetchCodeSystem(inc.getSystem());
-    if (e != null) {
-      ConceptDefinitionComponent v = getConceptForCode(e.getConcept(), code);
-      if (v != null)
-        return v;
-    }
-
-    if (context.isNoSlowLookup())
-      return null;
     
-    if (!getContext().getWorker().hasCache()) {
+    ValueSetExpansionComponent vse = null;
-      ValueSetExpansionComponent vse;
+    if (!context.isNoSlowLookup() && !getContext().getWorker().hasCache()) {
       try {
         ValueSetExpansionOutcome vso = getContext().getWorker().expandVS(inc, false);   
         ValueSet valueset = vso.getValueset();
@@ -872,16 +870,39 @@ public class ValueSetRenderer extends TerminologyRenderer {
       } catch (TerminologyServiceException e1) {
         return null;
       }
-      if (vse != null) {
-        ConceptDefinitionComponent v = getConceptForCodeFromExpansion(vse.getContains(), code);
-      if (v != null)
-        return v;
     }
+    
+    Map<String, ConceptDefinitionComponent> results = new HashMap<>();
+    List<CodingValidationRequest> serverList = new ArrayList<>();
+    
+    // 1st pass, anything we can resolve internally
+    for (ConceptReferenceComponent cc : inc.getConcept()) {
+      String code = cc.getCode();
+      ConceptDefinitionComponent v = null;
+      if (e != null) {
+        v = getConceptForCode(e.getConcept(), code);
+      }
+      if (v == null && vse != null) {
+        v = getConceptForCodeFromExpansion(vse.getContains(), code);
+      }
+      if (v != null) {
+        results.put(code, v);
+      } else {
+        serverList.add(new CodingValidationRequest(new Coding(inc.getSystem(), code, null)));
+      }
     }
-
+    if (!context.isNoSlowLookup() && !serverList.isEmpty()) {
-    return getContext().getWorker().validateCode(getContext().getTerminologyServiceOptions(), inc.getSystem(), code, null).asConceptDefinition();
+      getContext().getWorker().validateCodeBatch(getContext().getTerminologyServiceOptions(), serverList, null);
+      for (CodingValidationRequest vr : serverList) {
+        ConceptDefinitionComponent v = vr.getResult().asConceptDefinition();
+        if (v != null) {
+          results.put(vr.getCoding().getCode(), v);
+        }
+      }
+    }
+    return results;
   }
-
+  
   private ConceptDefinitionComponent getConceptForCode(List<ConceptDefinitionComponent> list, String code) {
     for (ConceptDefinitionComponent c : list) {
     if (code.equals(c.getCode()))

org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/ValueSetCheckerSimple.java

@@ -369,12 +369,33 @@ public class ValueSetCheckerSimple implements ValueSetChecker {
     }
     return true;
   }
+
+  private ConceptDefinitionComponent findCodeInConcept(ConceptDefinitionComponent concept, String code) {
+    if (code.equals(concept.getCode())) {
+      return concept;
+    }
+    ConceptDefinitionComponent cc = findCodeInConcept(concept.getConcept(), code);
+    if (cc != null) {
+      return cc;
+    }
+    if (concept.hasUserData(CodeSystemUtilities.USER_DATA_CROSS_LINK)) {
+      List<ConceptDefinitionComponent> children = (List<ConceptDefinitionComponent>) concept.getUserData(CodeSystemUtilities.USER_DATA_CROSS_LINK);
+      for (ConceptDefinitionComponent c : children) {
+        cc = findCodeInConcept(c, code);
+        if (cc != null) {
+          return cc;
+        }
+      }
+    }
+    return null;
+  }
+  
   private ConceptDefinitionComponent findCodeInConcept(List<ConceptDefinitionComponent> concept, String code) {
     for (ConceptDefinitionComponent cc : concept) {
       if (code.equals(cc.getCode())) {
         return cc;
       }
-      ConceptDefinitionComponent c = findCodeInConcept(cc.getConcept(), code);
+      ConceptDefinitionComponent c = findCodeInConcept(cc, code);
       if (c != null) {
         return c;
       }
@@ -449,7 +470,7 @@ public class ValueSetCheckerSimple implements ValueSetChecker {
         Boolean nok = inComponent(vsi, system, code, valueset.getCompose().getInclude().size() == 1);
         if (nok == null && result == false) {
           result = null;
-        } else if (!nok) {
+        } else if (nok) {
           result = false;
         }
       }
@@ -547,7 +568,7 @@ public class ValueSetCheckerSimple implements ValueSetChecker {
     if (cc == null) {
       return false;
     }
-    cc = findCodeInConcept(cc.getConcept(), code);
+    cc = findCodeInConcept(cc, code);
     return cc != null;
   }
 

org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/InstanceValidator.java

@@ -3645,7 +3645,7 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat
     } else if (element.getType().equals("StructureDefinition")) {
       new StructureDefinitionValidator(context, timeTracker, fpe).validateStructureDefinition(errors, element, stack);
     } else if (element.getType().equals("ValueSet")) {
-      new ValueSetValidator(context, timeTracker).validateValueSet(errors, element, stack);
+      new ValueSetValidator(context, timeTracker, this).validateValueSet(errors, element, stack);
     }
   }
 

org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/type/ValueSetValidator.java

@@ -21,6 +21,7 @@ import org.hl7.fhir.utilities.validation.ValidationMessage.Source;
 import org.hl7.fhir.utilities.validation.ValidationOptions;
 import org.hl7.fhir.validation.BaseValidator;
 import org.hl7.fhir.validation.TimeTracker;
+import org.hl7.fhir.validation.instance.InstanceValidator;
 import org.hl7.fhir.validation.instance.type.ValueSetValidator.VSCodingValidationRequest;
 import org.hl7.fhir.validation.instance.utils.NodeStack;
 
@@ -41,10 +42,13 @@ public class ValueSetValidator extends BaseValidator {
     
   }
 
-  public ValueSetValidator(IWorkerContext context, TimeTracker timeTracker) {
+  private InstanceValidator parent;
+
+  public ValueSetValidator(IWorkerContext context, TimeTracker timeTracker, InstanceValidator parent) {
     super(context);
     source = Source.InstanceValidator;
     this.timeTracker = timeTracker;
+    this.parent = parent;
   }
   
   public void validateValueSet(List<ValidationMessage> errors, Element vs, NodeStack stack) {
@@ -52,28 +56,28 @@ public class ValueSetValidator extends BaseValidator {
       List<Element> composes = vs.getChildrenByName("compose");
       int cc = 0;
       for (Element compose : composes) {
-        validateValueSetCompose(errors, compose, stack.push(compose, cc, null, null));
+        validateValueSetCompose(errors, compose, stack.push(compose, cc, null, null), vs.getNamedChildValue("url"));
         cc++;
       }
     }
   }
 
-  private void validateValueSetCompose(List<ValidationMessage> errors, Element compose, NodeStack stack) {
+  private void validateValueSetCompose(List<ValidationMessage> errors, Element compose, NodeStack stack, String vsid) {
     List<Element> includes = compose.getChildrenByName("include");
     int ci = 0;
     for (Element include : includes) {
-      validateValueSetInclude(errors, include, stack.push(include, ci, null, null));
+      validateValueSetInclude(errors, include, stack.push(include, ci, null, null), vsid);
       ci++;
     }    
     List<Element> excludes = compose.getChildrenByName("exclude");
     int ce = 0;
     for (Element exclude : excludes) {
-      validateValueSetInclude(errors, exclude, stack.push(exclude, ce, null, null));
+      validateValueSetInclude(errors, exclude, stack.push(exclude, ce, null, null), vsid);
       ce++;
     }    
   }
   
-  private void validateValueSetInclude(List<ValidationMessage> errors, Element include, NodeStack stack) {
+  private void validateValueSetInclude(List<ValidationMessage> errors, Element include, NodeStack stack, String vsid) {
     String system = include.getChildValue("system");
     String version = include.getChildValue("version");
     List<Element> valuesets = include.getChildrenByName("valueSet");
@@ -111,7 +115,14 @@ public class ValueSetValidator extends BaseValidator {
         cc++;
       }    
       if (batch.size() > 0) {
+        long t = System.currentTimeMillis();
+        if (parent.isDebug()) {
+          System.out.println("  : Validate "+batch.size()+" codes from "+system+" for "+vsid);
+        }
         context.validateCodeBatch(ValidationOptions.defaults(), batch, null);
+        if (parent.isDebug()) {
+          System.out.println("  :   .. "+(System.currentTimeMillis()-t)+"ms");
+        }
         for (VSCodingValidationRequest cv : batch) {
           if (version == null) {
             warning(errors, IssueType.BUSINESSRULE, cv.getStack().getLiteralPath(), cv.getResult().isOk(), I18nConstants.VALUESET_INCLUDE_INVALID_CONCEPT_CODE, system, cv.getCoding().getCode());