From fa8977880945386e7c9079e79c952ac9df290988 Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Thu, 13 Aug 2020 12:36:36 +1000 Subject: [PATCH 01/13] Handle unknown code systems better when checking codes from unknown systems --- .../fhir/r5/context/BaseWorkerContext.java | 1 - .../r5/terminologies/ValueSetChecker.java | 2 +- .../terminologies/ValueSetCheckerSimple.java | 43 ++++++++++++++----- .../fhir/utilities/i18n/I18nConstants.java | 1 + .../src/main/resources/Messages.properties | 1 + 5 files changed, 35 insertions(+), 13 deletions(-) diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/BaseWorkerContext.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/BaseWorkerContext.java index 149760bd6..045ca5eb7 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/BaseWorkerContext.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/BaseWorkerContext.java @@ -970,7 +970,6 @@ public abstract class BaseWorkerContext extends I18nBase implements IWorkerConte for (ConceptSetComponent inc : vs.getCompose().getExclude()) { addDependentResources(pin, inc); } - } private void addDependentResources(Parameters pin, ConceptSetComponent inc) { diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/ValueSetChecker.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/ValueSetChecker.java index 3d74a1956..7d0d01329 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/ValueSetChecker.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/ValueSetChecker.java @@ -36,6 +36,6 @@ import org.hl7.fhir.r5.utils.EOperationOutcome; public interface ValueSetChecker { - boolean codeInValueSet(String system, String code) throws ETooCostly, EOperationOutcome, Exception; + Boolean codeInValueSet(String system, String code) throws ETooCostly, EOperationOutcome, Exception; } \ No newline at end of file diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/ValueSetCheckerSimple.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/ValueSetCheckerSimple.java index 273c60e64..d885ca2fb 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/ValueSetCheckerSimple.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/ValueSetCheckerSimple.java @@ -54,6 +54,7 @@ import org.hl7.fhir.r5.model.ValueSet.ConceptReferenceDesignationComponent; import org.hl7.fhir.r5.model.ValueSet.ConceptSetComponent; import org.hl7.fhir.r5.model.ValueSet.ConceptSetFilterComponent; import org.hl7.fhir.r5.model.ValueSet.ValueSetExpansionContainsComponent; +import org.hl7.fhir.r5.terminologies.ValueSetExpander.TerminologyServiceErrorClass; import org.hl7.fhir.utilities.CommaSeparatedStringBuilder; import org.hl7.fhir.utilities.Utilities; import org.hl7.fhir.utilities.i18n.I18nConstants; @@ -98,11 +99,18 @@ public class ValueSetCheckerSimple implements ValueSetChecker { } } if (valueset != null && options.getValueSetMode() != ValueSetMode.NO_MEMBERSHIP_CHECK) { - boolean ok = false; + Boolean result = false; for (Coding c : code.getCoding()) { - ok = ok || codeInValueSet(c.getSystem(), c.getCode()); + Boolean ok = codeInValueSet(c.getSystem(), c.getCode()); + if (ok == null && result == false) { + result = null; + } else if (ok) { + result = true; + } } - if (!ok) { + if (result == null) { + warnings.add(0, context.formatMessage(I18nConstants.UNABLE_TO_CHECK_IF_THE_PROVIDED_CODES_ARE_IN_THE_VALUE_SET_, valueset.getUrl())); + } else if (!result) { errors.add(0, context.formatMessage(I18nConstants.NONE_OF_THE_PROVIDED_CODES_ARE_IN_THE_VALUE_SET_, valueset.getUrl())); } } @@ -119,7 +127,7 @@ public class ValueSetCheckerSimple implements ValueSetChecker { String warningMessage = null; // first, we validate the concept itself - ValidationResult res =null; + ValidationResult res = null; boolean inExpansion = false; String system = code.hasSystem() ? code.getSystem() : getValueSetSystem(); if (options.getValueSetMode() != ValueSetMode.CHECK_MEMERSHIP_ONLY) { @@ -416,24 +424,34 @@ public class ValueSetCheckerSimple implements ValueSetChecker { } @Override - public boolean codeInValueSet(String system, String code) throws FHIRException { + public Boolean codeInValueSet(String system, String code) throws FHIRException { + Boolean result = false; if (valueset.hasExpansion()) { return checkExpansion(new Coding(system, code, null)); } else if (valueset.hasCompose()) { - boolean ok = false; for (ConceptSetComponent vsi : valueset.getCompose().getInclude()) { - ok = ok || inComponent(vsi, system, code, valueset.getCompose().getInclude().size() == 1); + Boolean ok = inComponent(vsi, system, code, valueset.getCompose().getInclude().size() == 1); + if (ok == null && result == false) { + result = null; + } else if (ok) { + result = true; + break; + } } for (ConceptSetComponent vsi : valueset.getCompose().getExclude()) { - ok = ok && !inComponent(vsi, system, code, valueset.getCompose().getInclude().size() == 1); + Boolean nok = inComponent(vsi, system, code, valueset.getCompose().getInclude().size() == 1); + if (nok == null && result == false) { + result = null; + } else if (!nok) { + result = false; + } } - return ok; } - return false; + return result; } - private boolean inComponent(ConceptSetComponent vsi, String system, String code, boolean only) throws FHIRException { + private Boolean inComponent(ConceptSetComponent vsi, String system, String code, boolean only) throws FHIRException { for (UriType uri : vsi.getValueSet()) { if (inImport(uri.getValue(), system, code)) { return true; @@ -463,6 +481,9 @@ public class ValueSetCheckerSimple implements ValueSetChecker { vs.setUrl(Utilities.makeUuidUrn()); vs.getCompose().addInclude(vsi); ValidationResult res = context.validateCode(options.noClient(), new Coding(system, code, null), vs); + if (res.getErrorClass() == TerminologyServiceErrorClass.UNKNOWN || res.getErrorClass() == TerminologyServiceErrorClass.CODESYSTEM_UNSUPPORTED || res.getErrorClass() == TerminologyServiceErrorClass.VALUESET_UNSUPPORTED) { + return null; + } return res.isOk(); } else { if (vsi.hasFilter()) { diff --git a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/i18n/I18nConstants.java b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/i18n/I18nConstants.java index 8f0fe4a0d..83cfc6cbb 100644 --- a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/i18n/I18nConstants.java +++ b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/i18n/I18nConstants.java @@ -230,6 +230,7 @@ public class I18nConstants { public static final String NEEDS_A_SNAPSHOT = "needs_a_snapshot"; public static final String NODE_TYPE__IS_NOT_ALLOWED = "Node_type__is_not_allowed"; public static final String NONE_OF_THE_PROVIDED_CODES_ARE_IN_THE_VALUE_SET_ = "None_of_the_provided_codes_are_in_the_value_set_"; + public static final String UNABLE_TO_CHECK_IF_THE_PROVIDED_CODES_ARE_IN_THE_VALUE_SET_ = "UNABLE_TO_CHECK_IF_THE_PROVIDED_CODES_ARE_IN_THE_VALUE_SET_"; public static final String NOT_DONE_YET = "Not_done_yet"; public static final String NOT_DONE_YET_CANT_FETCH_ = "not_done_yet_cant_fetch_"; public static final String NOT_DONE_YET_VALIDATORHOSTSERVICESCHECKFUNCTION = "Not_done_yet_ValidatorHostServicescheckFunction"; diff --git a/org.hl7.fhir.utilities/src/main/resources/Messages.properties b/org.hl7.fhir.utilities/src/main/resources/Messages.properties index c8837a180..398340857 100644 --- a/org.hl7.fhir.utilities/src/main/resources/Messages.properties +++ b/org.hl7.fhir.utilities/src/main/resources/Messages.properties @@ -596,3 +596,4 @@ BUNDLE_RULE_NONE = No Rule BUNDLE_RULE_UNKNOWN = Bundle Rule refers to invalid resource {0} BUNDLE_RULE_INVALID_INDEX = Bundle Rules index is invalid ({0}) BUNDLE_RULE_PROFILE_UNKNOWN = Bundle Rules profile {1} is unknown for {0} +UNABLE_TO_CHECK_IF_THE_PROVIDED_CODES_ARE_IN_THE_VALUE_SET_ = Unable to determine whether the provided codes are in the value set {0} because the value set or code system is not known to the validator From c5f4d2396f4baceb8824b8ee9169fcc056a154a6 Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Thu, 13 Aug 2020 14:28:28 +1000 Subject: [PATCH 02/13] more work on code validation --- .../r5/terminologies/ValueSetCheckerSimple.java | 13 ++++++++++--- .../instance/type/QuestionnaireValidator.java | 1 - 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/ValueSetCheckerSimple.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/ValueSetCheckerSimple.java index d885ca2fb..ae9f901f7 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/ValueSetCheckerSimple.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/ValueSetCheckerSimple.java @@ -159,13 +159,20 @@ public class ValueSetCheckerSimple implements ValueSetChecker { } if (cs != null /*&& (cs.getContent() == CodeSystemContentMode.COMPLETE || cs.getContent() == CodeSystemContentMode.FRAGMENT)*/) { + if (!(cs.getContent() == CodeSystemContentMode.COMPLETE || cs.getContent() == CodeSystemContentMode.FRAGMENT)) { + // we can't validate that here. + throw new FHIRException("Unable to evaluate based on empty code system"); + } res = validateCode(code, cs); } else { - // it's in the expansion, but we could find it in a code system - res = findCodeInExpansion(code); + // well, we didn't find a code system - try the expansion? + // disabled waiting for discussion + throw new Error("No try the server"); } } else { - inExpansion = checkExpansion(code); + // disabled waiting for discussion + throw new Error("No try the server"); +// inExpansion = checkExpansion(code); } // then, if we have a value set, we check it's in the value set diff --git a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/type/QuestionnaireValidator.java b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/type/QuestionnaireValidator.java index 227d68f94..492a689af 100644 --- a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/type/QuestionnaireValidator.java +++ b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/type/QuestionnaireValidator.java @@ -180,7 +180,6 @@ public class QuestionnaireValidator extends BaseValidator { } } if (hint(errors, IssueType.REQUIRED, element.line(), element.col(), stack.getLiteralPath(), questionnaire != null, I18nConstants.QUESTIONNAIRE_QR_Q_NONE)) { - long t = System.nanoTime(); Questionnaire qsrc = questionnaire.startsWith("#") ? loadQuestionnaire(element, questionnaire.substring(1)) : context.fetchResource(Questionnaire.class, questionnaire); if (warning(errors, IssueType.REQUIRED, q.line(), q.col(), stack.getLiteralPath(), qsrc != null, I18nConstants.QUESTIONNAIRE_QR_Q_NOTFOUND, questionnaire)) { boolean inProgress = "in-progress".equals(element.getNamedChildValue("status")); From 0b57d266cddccacf6cdade290bb9d870108f6e54 Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Thu, 13 Aug 2020 17:57:14 +1000 Subject: [PATCH 03/13] more work on comparison --- .../CapabilityStatementComparer.java | 68 ++++++++++++++++++- .../CapabilityStatementUtilities.java | 3 +- .../comparison/tests/ComparisonTests.java | 5 ++ 3 files changed, 71 insertions(+), 5 deletions(-) diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/comparison/CapabilityStatementComparer.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/comparison/CapabilityStatementComparer.java index 9be261a15..0f82da2ce 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/comparison/CapabilityStatementComparer.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/comparison/CapabilityStatementComparer.java @@ -10,8 +10,11 @@ import java.util.Map; import org.hl7.fhir.exceptions.DefinitionException; import org.hl7.fhir.exceptions.FHIRException; import org.hl7.fhir.r5.comparison.ResourceComparer.MessageCounts; +import org.hl7.fhir.r5.context.IWorkerContext; import org.hl7.fhir.r5.model.BackboneElement; import org.hl7.fhir.r5.model.BooleanType; +import org.hl7.fhir.r5.model.CanonicalResource; +import org.hl7.fhir.r5.model.CanonicalType; import org.hl7.fhir.r5.model.CapabilityStatement; import org.hl7.fhir.r5.model.CapabilityStatement.CapabilityStatementRestComponent; import org.hl7.fhir.r5.model.CapabilityStatement.CapabilityStatementRestResourceComponent; @@ -28,6 +31,8 @@ import org.hl7.fhir.r5.model.Element; import org.hl7.fhir.r5.model.Enumeration; import org.hl7.fhir.r5.model.Extension; import org.hl7.fhir.r5.model.PrimitiveType; +import org.hl7.fhir.r5.model.Resource; +import org.hl7.fhir.r5.model.StructureDefinition; import org.hl7.fhir.utilities.Utilities; import org.hl7.fhir.utilities.validation.ValidationMessage; import org.hl7.fhir.utilities.validation.ValidationMessage.IssueSeverity; @@ -406,7 +411,7 @@ public class CapabilityStatementComparer extends CanonicalResourceComparer { } private void compareRestResource(StructuralMatch sm, CapabilityStatementRestResourceComponent l, CapabilityStatementRestResourceComponent r, String path, CapabilityStatementComparison res, CapabilityStatementRestResourceComponent union, CapabilityStatementRestResourceComponent intersection) { - compareStrings(path, sm.getMessages(), l.getProfile(), r.getProfile(), "profile", IssueSeverity.WARNING, res); + compareProfiles(path, sm, l.getProfileElement(), r.getProfileElement(), res, union, intersection); // todo: supported profiles compareStrings(path, sm.getMessages(), l.getDocumentation(), r.getDocumentation(), "documentation", IssueSeverity.INFORMATION, res); compareExpectations(sm, l, r, path, res, union, intersection); @@ -425,6 +430,57 @@ public class CapabilityStatementComparer extends CanonicalResourceComparer { compareOperations(sm, l.getOperation(), r.getOperation(), path, res, union.getOperation(), intersection.getOperation()); } + private void compareProfiles(String path, StructuralMatch combined, CanonicalType left, CanonicalType right, CapabilityStatementComparison res, CapabilityStatementRestResourceComponent union, CapabilityStatementRestResourceComponent intersection) { + if (!left.hasValue() && !right.hasValue()) { + // nothing in this case + } else if (!left.hasValue()) { + // the intersection is anything in right. The union is everything (or nothing, in this case) + intersection.setProfileElement(right.copy()); + combined.getChildren().add(new StructuralMatch(vmI(IssueSeverity.WARNING, "Added this profile", path), right).setName("profile")); + } else if (!right.hasValue()) { + // the intersection is anything in right. The union is everything (or nothing, in this case) + intersection.setProfileElement(left.copy()); + combined.getChildren().add(new StructuralMatch(left, vmI(IssueSeverity.WARNING, "Removed this profile", path)).setName("profile")); + } else { + // profiles on both sides... + StructureDefinition sdLeft = session.getContextLeft().fetchResource(StructureDefinition.class, left.getValue()); + StructureDefinition sdRight = session.getContextRight().fetchResource(StructureDefinition.class, right.getValue()); + if (sdLeft == null && sdRight == null) { + combined.getChildren().add(new StructuralMatch(left, right, vmI(IssueSeverity.ERROR, "Cannot compare profiles because neither is known", path)).setName("profile")); + } else if (sdLeft == null) { + combined.getChildren().add(new StructuralMatch(left, right, vmI(IssueSeverity.ERROR, "Cannot compare profiles because '"+left.getValue()+"' is not known", path)).setName("profile")); + } else if (sdRight == null) { + combined.getChildren().add(new StructuralMatch(left, right, vmI(IssueSeverity.ERROR, "Cannot compare profiles because '"+right.getValue()+"' is not known", path)).setName("profile")); + } else if (sdLeft.getUrl().equals(sdRight.getUrl())) { + intersection.setProfileElement(left.copy()); + union.setProfileElement(left.copy()); + combined.getChildren().add(new StructuralMatch(left, right).setName("profile")); + } else if (profileInherits(sdLeft, sdRight, session.getContextLeft())) { + // if left inherits from right: + intersection.setProfileElement(left.copy()); + union.setProfileElement(right.copy()); + combined.getChildren().add(new StructuralMatch(left, right, vmI(IssueSeverity.WARNING, "Changed this profile to a broader profile", path)).setName("profile")); + } else if (profileInherits(sdRight, sdLeft, session.getContextRight())) { + intersection.setProfileElement(right.copy()); + union.setProfileElement(left.copy()); + combined.getChildren().add(new StructuralMatch(left, right, vmI(IssueSeverity.WARNING, "Changed this profile to a narrower one", path)).setName("profile")); + } else { + combined.getChildren().add(new StructuralMatch(left, right, vmI(IssueSeverity.WARNING, "Different", path)).setName("profile")); + throw new Error("Not done yet"); + } + } + } + + private boolean profileInherits(StructureDefinition sdFocus, StructureDefinition sdOther, IWorkerContext ctxt) { + while (sdFocus != null) { + if (sdFocus.getUrl().equals(sdOther.getUrl()) && sdFocus.getVersion().equals(sdOther.getVersion())) { + return true; + } + sdFocus = ctxt.fetchResource(StructureDefinition.class, sdFocus.getBaseDefinition()); + } + return false; + } + private void compareItemProperty(StructuralMatch combined, String name, PrimitiveType left, PrimitiveType right, String path, CapabilityStatementComparison res, PrimitiveType union, PrimitiveType intersection, IssueSeverity issueSeverity) { if (!left.isEmpty() || !right.isEmpty()) { if (left.isEmpty()) { @@ -846,9 +902,15 @@ public class CapabilityStatementComparer extends CanonicalResourceComparer { r.getCells().add(gen.new Cell(null, null, t.getName(), null, null)); PrimitiveType left = t.hasLeft() ? (PrimitiveType) t.getLeft() : null; PrimitiveType right = t.hasRight() ? (PrimitiveType) t.getRight() : null; - r.getCells().add(style(gen.new Cell(null, null, left != null ? left.primitiveValue() : "", null, null), left != null ? left.primitiveValue() : null, right != null ? right.primitiveValue() : null, true)); + CanonicalResource crL = left == null ? null : (CanonicalResource) session.getContextLeft().fetchResource(Resource.class, left.primitiveValue()); + CanonicalResource crR = right == null ? null : (CanonicalResource) session.getContextRight().fetchResource(Resource.class, right.primitiveValue()); + String refL = crL != null && crL.hasUserData("path") ? crL.getUserString("path") : null; + String dispL = crL != null && refL != null ? crL.present() : left == null ? "" : left.primitiveValue(); + String refR = crR != null && crR.hasUserData("path") ? crR.getUserString("path") : null; + String dispR = crR != null && refR != null ? crR.present() : right == null ? "" : right.primitiveValue(); + r.getCells().add(style(gen.new Cell(null, refL, dispL, null, null), left != null ? left.primitiveValue() : null, right != null ? right.primitiveValue() : null, true)); r.getCells().add(gen.new Cell(null, null, "", null, null)); - r.getCells().add(style(gen.new Cell(null, null, right != null ? right.primitiveValue() : "", null, null), left != null ? left.primitiveValue() : null, right != null ? right.primitiveValue() : null, false)); + r.getCells().add(style(gen.new Cell(null, refR, dispR, null, null), left != null ? left.primitiveValue() : null, right != null ? right.primitiveValue() : null, false)); r.getCells().add(gen.new Cell(null, null, "", null, null)); r.getCells().add(cellForMessages(gen, t.getMessages())); return r; diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/conformance/CapabilityStatementUtilities.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/conformance/CapabilityStatementUtilities.java index 2eb3e7b43..57d1edee7 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/conformance/CapabilityStatementUtilities.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/conformance/CapabilityStatementUtilities.java @@ -240,8 +240,7 @@ public class CapabilityStatementUtilities { XhtmlNode tr = tbl.tr(); tr.style("background-color: #dddddd"); tr.td().b().addText(r.getType()); - tr.td().tx("Present"); - + tr.td().tx("Present"); if (o == null) { union.addResource(r); diff --git a/org.hl7.fhir.validation/src/test/java/org/hl7/fhir/comparison/tests/ComparisonTests.java b/org.hl7.fhir.validation/src/test/java/org/hl7/fhir/comparison/tests/ComparisonTests.java index 332e58051..367977e38 100644 --- a/org.hl7.fhir.validation/src/test/java/org/hl7/fhir/comparison/tests/ComparisonTests.java +++ b/org.hl7.fhir.validation/src/test/java/org/hl7/fhir/comparison/tests/ComparisonTests.java @@ -8,6 +8,8 @@ import org.hl7.fhir.convertors.VersionConvertor_10_50; import org.hl7.fhir.convertors.VersionConvertor_14_50; import org.hl7.fhir.convertors.VersionConvertor_30_50; import org.hl7.fhir.convertors.VersionConvertor_40_50; +import org.hl7.fhir.convertors.loaders.R4ToR5Loader; +import org.hl7.fhir.convertors.loaders.BaseLoaderR5.NullLoaderKnowledgeProvider; import org.hl7.fhir.exceptions.DefinitionException; import org.hl7.fhir.exceptions.FHIRException; import org.hl7.fhir.exceptions.FHIRFormatError; @@ -104,6 +106,9 @@ public class ComparisonTests { if (context == null) { System.out.println("---- Load R5 ----------------------------------------------------------------"); context = TestingUtilities.context(); + FilesystemPackageCacheManager pcm = new FilesystemPackageCacheManager(true, ToolsVersion.TOOLS_VERSION); + NpmPackage npm = pcm.loadPackage("hl7.fhir.us.core#3.1.0"); + context.loadFromPackage(npm, new R4ToR5Loader(new String[] { "CapabilityStatement", "StructureDefinition", "ValueSet", "CodeSystem", "SearchParameter", "OperationDefinition", "Questionnaire","ConceptMap","StructureMap", "NamingSystem"}, new NullLoaderKnowledgeProvider())); } if (!new File(Utilities.path("[tmp]", "comparison")).exists()) { From 63b93480d91c40e6d7b8d1a7d44a565bc248bd30 Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Thu, 13 Aug 2020 17:57:38 +1000 Subject: [PATCH 04/13] hack workaround for UTG NUCC problem. --- .../org/hl7/fhir/r5/context/CanonicalResourceManager.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/CanonicalResourceManager.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/CanonicalResourceManager.java index bc93b942e..c1db27961 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/CanonicalResourceManager.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/CanonicalResourceManager.java @@ -212,6 +212,11 @@ public class CanonicalResourceManager { } public void see(CachedCanonicalResource cr) { + // ignore UTG NUCC erroneous code system + if (cr.getPackageInfo().getId().startsWith("hl7.terminology") && "http://nucc.org/provider-taxonomy".equals(cr.getUrl())) { + return; + } + if (enforceUniqueId && map.containsKey(cr.getId())) { drop(cr.getId()); } From dc2126c7863ed5fd5ff867d007a231d44e216012 Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Thu, 13 Aug 2020 17:58:03 +1000 Subject: [PATCH 05/13] adjust exception type --- .../org/hl7/fhir/r5/terminologies/ValueSetCheckerSimple.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/ValueSetCheckerSimple.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/ValueSetCheckerSimple.java index ae9f901f7..8e28bf481 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/ValueSetCheckerSimple.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/ValueSetCheckerSimple.java @@ -167,11 +167,11 @@ public class ValueSetCheckerSimple implements ValueSetChecker { } else { // well, we didn't find a code system - try the expansion? // disabled waiting for discussion - throw new Error("No try the server"); + throw new FHIRException("No try the server"); } } else { // disabled waiting for discussion - throw new Error("No try the server"); + throw new FHIRException("No try the server"); // inExpansion = checkExpansion(code); } From 67f7176d3bc5086578241d0e0a65ace193818765 Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Thu, 13 Aug 2020 17:58:36 +1000 Subject: [PATCH 06/13] more work on timeouts --- .../src/main/java/org/hl7/fhir/r5/utils/client/ClientUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/utils/client/ClientUtils.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/utils/client/ClientUtils.java index d85404114..1688c010f 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/utils/client/ClientUtils.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/utils/client/ClientUtils.java @@ -310,7 +310,7 @@ public class ClientUtils { } catch (InterruptedException e) { } } else { - if (tryCount > 1) { + if (tryCount > 4) { System.out.println("Giving up: "+ioe.getMessage()+" (R5 / "+(System.currentTimeMillis()-t)+"ms / "+Utilities.describeSize(payload.length)+" for "+message+")"); } throw new EFhirClientException("Error sending HTTP Post/Put Payload: "+ioe.getMessage(), ioe); From 4133a36c69f8349c4fff33182eb83fe915042da8 Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Thu, 13 Aug 2020 17:59:01 +1000 Subject: [PATCH 07/13] improve error message --- .../main/java/org/hl7/fhir/utilities/i18n/I18nConstants.java | 3 ++- org.hl7.fhir.utilities/src/main/resources/Messages.properties | 3 ++- .../org/hl7/fhir/validation/instance/InstanceValidator.java | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/i18n/I18nConstants.java b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/i18n/I18nConstants.java index 83cfc6cbb..a7da3071b 100644 --- a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/i18n/I18nConstants.java +++ b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/i18n/I18nConstants.java @@ -364,7 +364,8 @@ public class I18nConstants { public static final String TERMINOLOGY_TX_CONFIRM_1 = "Terminology_TX_Confirm_1"; public static final String TERMINOLOGY_TX_CONFIRM_2 = "Terminology_TX_Confirm_2"; public static final String TERMINOLOGY_TX_CONFIRM_3 = "Terminology_TX_Confirm_3"; - public static final String TERMINOLOGY_TX_CONFIRM_4 = "Terminology_TX_Confirm_4"; + public static final String TERMINOLOGY_TX_CONFIRM_4a = "Terminology_TX_Confirm_4a"; + public static final String TERMINOLOGY_TX_CONFIRM_4b = "Terminology_TX_Confirm_4b"; public static final String TERMINOLOGY_TX_CONFIRM_5 = "Terminology_TX_Confirm_5"; public static final String TERMINOLOGY_TX_CONFIRM_6 = "Terminology_TX_Confirm_6"; public static final String TERMINOLOGY_TX_DISPLAY_WRONG = "Terminology_TX_Display_Wrong"; diff --git a/org.hl7.fhir.utilities/src/main/resources/Messages.properties b/org.hl7.fhir.utilities/src/main/resources/Messages.properties index 398340857..2d4659d93 100644 --- a/org.hl7.fhir.utilities/src/main/resources/Messages.properties +++ b/org.hl7.fhir.utilities/src/main/resources/Messages.properties @@ -135,7 +135,8 @@ Terminology_TX_Coding_Count = Expected {0} but found {1} coding elements Terminology_TX_Confirm_1 = Could not confirm that the codes provided are in the value set {0} and a code from this value set is required (class = {1}) Terminology_TX_Confirm_2 = Could not confirm that the codes provided are in the value set {0} and a code should come from this value set unless it has no suitable code (class = {1}) Terminology_TX_Confirm_3 = Could not confirm that the codes provided are in the value set {0} and a code is recommended to come from this value set (class = {1}) -Terminology_TX_Confirm_4 = Could not confirm that the codes provided are in the value set {0}, and a code from this value set is required +Terminology_TX_Confirm_4a = The code provided ({2}) is not in the value set {0}, and a code from this value set is required: {1} +Terminology_TX_Confirm_4b = The codes provided ({2}) are not in the value set {0}, and a code from this value set is required: {1} Terminology_TX_Confirm_5 = Could not confirm that the codes provided are in the value set {0}, and a code should come from this value set unless it has no suitable code Terminology_TX_Confirm_6 = Could not confirm that the codes provided are in the value set {0}, and a code is recommended to come from this value set Terminology_TX_Display_Wrong = Display should be ''{0}'' diff --git a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/InstanceValidator.java b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/InstanceValidator.java index 77f941dc0..2dd75abd5 100644 --- a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/InstanceValidator.java +++ b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/InstanceValidator.java @@ -1138,7 +1138,7 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat txHint(errors, vr.getTxLink(), IssueType.CODEINVALID, element.line(), element.col(), path, false, I18nConstants.TERMINOLOGY_TX_BINDING_NOSERVER); else if (vr.getErrorClass() != null && vr.getErrorClass().isInfrastructure()) { if (binding.getStrength() == BindingStrength.REQUIRED) - txWarning(errors, vr.getTxLink(), IssueType.CODEINVALID, element.line(), element.col(), path, false, I18nConstants.TERMINOLOGY_TX_CONFIRM_4, describeReference(binding.getValueSet(), valueset)); + txWarning(errors, vr.getTxLink(), IssueType.CODEINVALID, element.line(), element.col(), path, false, I18nConstants.TERMINOLOGY_TX_CONFIRM_4a, describeReference(binding.getValueSet(), valueset), vr.getMessage(), system+"#"+code); else if (binding.getStrength() == BindingStrength.EXTENSIBLE) { if (binding.hasExtension("http://hl7.org/fhir/StructureDefinition/elementdefinition-maxValueSet")) checkMaxValueSet(errors, path, element, profile, ToolingExtensions.readStringExtension(binding, "http://hl7.org/fhir/StructureDefinition/elementdefinition-maxValueSet"), c, stack); @@ -1357,7 +1357,7 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat txHint(errors, vr.getTxLink(), IssueType.CODEINVALID, element.line(), element.col(), path, false, I18nConstants.TERMINOLOGY_TX_BINDING_NOSERVER); else if (vr.getErrorClass() != null && !vr.getErrorClass().isInfrastructure()) { if (binding.getStrength() == BindingStrength.REQUIRED) - txRule(errors, vr.getTxLink(), IssueType.CODEINVALID, element.line(), element.col(), path, false, I18nConstants.TERMINOLOGY_TX_CONFIRM_4, describeReference(binding.getValueSet(), valueset)); + txRule(errors, vr.getTxLink(), IssueType.CODEINVALID, element.line(), element.col(), path, false, I18nConstants.TERMINOLOGY_TX_CONFIRM_4a, describeReference(binding.getValueSet(), valueset), vr.getMessage(), theSystem+"#"+theCode); else if (binding.getStrength() == BindingStrength.EXTENSIBLE) { if (binding.hasExtension("http://hl7.org/fhir/StructureDefinition/elementdefinition-maxValueSet")) checkMaxValueSet(errors, path, element, profile, ToolingExtensions.readStringExtension(binding, "http://hl7.org/fhir/StructureDefinition/elementdefinition-maxValueSet"), c, stack); From db56dfb888bdc8167d09a6966eeaaf54a7132ba3 Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Thu, 13 Aug 2020 18:53:36 +1000 Subject: [PATCH 08/13] fix NPE? --- .../java/org/hl7/fhir/r5/context/CanonicalResourceManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/CanonicalResourceManager.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/CanonicalResourceManager.java index c1db27961..289f7b52d 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/CanonicalResourceManager.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/CanonicalResourceManager.java @@ -213,7 +213,7 @@ public class CanonicalResourceManager { public void see(CachedCanonicalResource cr) { // ignore UTG NUCC erroneous code system - if (cr.getPackageInfo().getId().startsWith("hl7.terminology") && "http://nucc.org/provider-taxonomy".equals(cr.getUrl())) { + if (cr.getPackageInfo() != null && cr.getPackageInfo().getId() != null && cr.getPackageInfo().getId().startsWith("hl7.terminology") && "http://nucc.org/provider-taxonomy".equals(cr.getUrl())) { return; } From 61871e7c63bcb013f42bac93c0347016ac3c3b24 Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Thu, 13 Aug 2020 20:33:38 +1000 Subject: [PATCH 09/13] release notes --- RELEASE_NOTES.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index e69de29bb..437986021 100644 --- a/RELEASE_NOTES.md +++ b/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 From 5a7e130daafa0a77291fa7a7b06f03134823dedb Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Thu, 13 Aug 2020 23:16:48 +1000 Subject: [PATCH 10/13] Improve error message --- .../src/main/resources/Messages.properties | 10 +++++----- .../fhir/validation/instance/InstanceValidator.java | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/org.hl7.fhir.utilities/src/main/resources/Messages.properties b/org.hl7.fhir.utilities/src/main/resources/Messages.properties index 2d4659d93..a040a3ea2 100644 --- a/org.hl7.fhir.utilities/src/main/resources/Messages.properties +++ b/org.hl7.fhir.utilities/src/main/resources/Messages.properties @@ -147,18 +147,18 @@ Terminology_TX_Error_Coding2 = Error {0} validating Coding: {1} Terminology_TX_NoValid_1 = None of the codes provided are in the value set {0} ({1}), and a code from this value set is required) (codes = {2}) Terminology_TX_NoValid_10 = The code provided is not in the maximum value set {0} ({1}), and a code from this value set is required) (code = {2}#{3}) Terminology_TX_NoValid_11 = The code provided is not in the maximum value set {0} ({1}{2}) -Terminology_TX_NoValid_12 = The Coding provided is not in the value set {0}, and a code is required from this value set. {1} +Terminology_TX_NoValid_12 = The Coding provided ({2}) is not in the value set {0}, and a code is required from this value set. {1} Terminology_TX_NoValid_13 = The Coding provided ({2}) is not in the value set {0}, and a code should come from this value set unless it has no suitable code. {1} -Terminology_TX_NoValid_14 = The Coding provided is not in the value set {0}, and a code is recommended to come from this value set. {1} +Terminology_TX_NoValid_14 = The Coding provided ({2}) is not in the value set {0}, and a code is recommended to come from this value set. {1} Terminology_TX_NoValid_15 = The value provided (''{0}'') could not be validated in the absence of a terminology server Terminology_TX_NoValid_16 = The value provided (''{0}'') is not in the value set {1} ({2}), and a code is required from this value set){3} Terminology_TX_NoValid_17 = The value provided (''{0}'') is not in the value set {1} ({2}), and a code should come from this value set unless it has no suitable code){3} Terminology_TX_NoValid_18 = The value provided (''{0}'') is not in the value set {1} ({2}), and a code is recommended to come from this value set){3} Terminology_TX_NoValid_2 = None of the codes provided are in the value set {0} ({1}), and a code should come from this value set unless it has no suitable code) (codes = {2}) Terminology_TX_NoValid_3 = None of the codes provided are in the value set {0} ({1}), and a code is recommended to come from this value set) (codes = {2}) -Terminology_TX_NoValid_4 = The Coding provided is not in the value set {0}, and a code is required from this value set{1} -Terminology_TX_NoValid_5 = The Coding provided is not in the value set {0}, and a code should come from this value set unless it has no suitable code{1} -Terminology_TX_NoValid_6 = The Coding provided is not in the value set {0}, and a code is recommended to come from this value set{1} +Terminology_TX_NoValid_4 = The Coding provided ({2}) is not in the value set {0}, and a code is required from this value set{1} +Terminology_TX_NoValid_5 = The Coding provided ({2}) is not in the value set {0}, and a code should come from this value set unless it has no suitable code{1} +Terminology_TX_NoValid_6 = The Coding provided ({2}) is not in the value set {0}, and a code is recommended to come from this value set{1} Terminology_TX_NoValid_7 = None of the codes provided could be validated against the maximum value set {0} ({1}), (error = {2}) Terminology_TX_NoValid_8 = None of the codes provided are in the maximum value set {0} ({1}), and a code from this value set is required) (codes = {2}) Terminology_TX_NoValid_9 = The code provided could not be validated against the maximum value set {0} ({1}), (error = {2}) diff --git a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/InstanceValidator.java b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/InstanceValidator.java index 2dd75abd5..2469011ad 100644 --- a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/InstanceValidator.java +++ b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/InstanceValidator.java @@ -1150,15 +1150,15 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat } } } else if (binding.getStrength() == BindingStrength.REQUIRED) - txRule(errors, vr.getTxLink(), IssueType.CODEINVALID, element.line(), element.col(), path, false, I18nConstants.TERMINOLOGY_TX_NOVALID_4, describeReference(binding.getValueSet(), valueset), (vr.getMessage() != null ? " (error message = " + vr.getMessage() + ")" : "")); + txRule(errors, vr.getTxLink(), IssueType.CODEINVALID, element.line(), element.col(), path, false, I18nConstants.TERMINOLOGY_TX_NOVALID_4, describeReference(binding.getValueSet(), valueset), (vr.getMessage() != null ? " (error message = " + vr.getMessage() + ")" : ""), system+"#"+code); else if (binding.getStrength() == BindingStrength.EXTENSIBLE) { if (binding.hasExtension("http://hl7.org/fhir/StructureDefinition/elementdefinition-maxValueSet")) checkMaxValueSet(errors, path, element, profile, ToolingExtensions.readStringExtension(binding, "http://hl7.org/fhir/StructureDefinition/elementdefinition-maxValueSet"), c, stack); else - txWarning(errors, vr.getTxLink(), IssueType.CODEINVALID, element.line(), element.col(), path, false, I18nConstants.TERMINOLOGY_TX_NOVALID_5, describeReference(binding.getValueSet(), valueset), (vr.getMessage() != null ? " (error message = " + vr.getMessage() + ")" : "")); + txWarning(errors, vr.getTxLink(), IssueType.CODEINVALID, element.line(), element.col(), path, false, I18nConstants.TERMINOLOGY_TX_NOVALID_5, describeReference(binding.getValueSet(), valueset), (vr.getMessage() != null ? " (error message = " + vr.getMessage() + ")" : ""), system+"#"+code); } else if (binding.getStrength() == BindingStrength.PREFERRED) { if (baseOnly) { - txHint(errors, vr.getTxLink(), IssueType.CODEINVALID, element.line(), element.col(), path, false, I18nConstants.TERMINOLOGY_TX_NOVALID_6, describeReference(binding.getValueSet(), valueset), (vr.getMessage() != null ? " (error message = " + vr.getMessage() + ")" : "")); + txHint(errors, vr.getTxLink(), IssueType.CODEINVALID, element.line(), element.col(), path, false, I18nConstants.TERMINOLOGY_TX_NOVALID_6, describeReference(binding.getValueSet(), valueset), (vr.getMessage() != null ? " (error message = " + vr.getMessage() + ")" : ""), system+"#"+code); } } } @@ -1369,7 +1369,7 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat } } } else if (binding.getStrength() == BindingStrength.REQUIRED) - txRule(errors, vr.getTxLink(), IssueType.CODEINVALID, element.line(), element.col(), path, false, I18nConstants.TERMINOLOGY_TX_NOVALID_12, describeReference(binding.getValueSet(), valueset), getErrorMessage(vr.getMessage())); + txRule(errors, vr.getTxLink(), IssueType.CODEINVALID, element.line(), element.col(), path, false, I18nConstants.TERMINOLOGY_TX_NOVALID_12, describeReference(binding.getValueSet(), valueset), getErrorMessage(vr.getMessage()), theSystem+"#"+theCode); else if (binding.getStrength() == BindingStrength.EXTENSIBLE) { if (binding.hasExtension("http://hl7.org/fhir/StructureDefinition/elementdefinition-maxValueSet")) checkMaxValueSet(errors, path, element, profile, ToolingExtensions.readStringExtension(binding, "http://hl7.org/fhir/StructureDefinition/elementdefinition-maxValueSet"), c, stack); @@ -1377,7 +1377,7 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat txWarning(errors, vr.getTxLink(), IssueType.CODEINVALID, element.line(), element.col(), path, false, I18nConstants.TERMINOLOGY_TX_NOVALID_13, describeReference(binding.getValueSet(), valueset), getErrorMessage(vr.getMessage()), c.getSystem()+"#"+c.getCode()); } else if (binding.getStrength() == BindingStrength.PREFERRED) { if (baseOnly) { - txHint(errors, vr.getTxLink(), IssueType.CODEINVALID, element.line(), element.col(), path, false, I18nConstants.TERMINOLOGY_TX_NOVALID_14, describeReference(binding.getValueSet(), valueset), getErrorMessage(vr.getMessage())); + txHint(errors, vr.getTxLink(), IssueType.CODEINVALID, element.line(), element.col(), path, false, I18nConstants.TERMINOLOGY_TX_NOVALID_14, describeReference(binding.getValueSet(), valueset), getErrorMessage(vr.getMessage()), theSystem+"#"+theCode); } } } From 7771c7f231a8c94754cdad9f6019007902518be2 Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Fri, 14 Aug 2020 08:09:02 +1000 Subject: [PATCH 11/13] Significant improvement to performance rendering value sets, and only use server side caching if the server declares it supports it --- .../convertors/VersionConvertor_10_50.java | 2 + .../convertors/VersionConvertor_30_50.java | 2 + .../fhir/r5/context/BaseWorkerContext.java | 40 +++++++---- .../fhir/r5/context/SimpleWorkerContext.java | 5 +- .../fhir/r5/renderers/ValueSetRenderer.java | 71 ++++++++++++------- 5 files changed, 82 insertions(+), 38 deletions(-) diff --git a/org.hl7.fhir.convertors/src/main/java/org/hl7/fhir/convertors/VersionConvertor_10_50.java b/org.hl7.fhir.convertors/src/main/java/org/hl7/fhir/convertors/VersionConvertor_10_50.java index 4198ade73..1bd647afd 100644 --- a/org.hl7.fhir.convertors/src/main/java/org/hl7/fhir/convertors/VersionConvertor_10_50.java +++ b/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; } diff --git a/org.hl7.fhir.convertors/src/main/java/org/hl7/fhir/convertors/VersionConvertor_30_50.java b/org.hl7.fhir.convertors/src/main/java/org/hl7/fhir/convertors/VersionConvertor_30_50.java index bf375aa0a..fedf56bb1 100644 --- a/org.hl7.fhir.convertors/src/main/java/org/hl7/fhir/convertors/VersionConvertor_30_50.java +++ b/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; } diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/BaseWorkerContext.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/BaseWorkerContext.java index 045ca5eb7..70ffd0734 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/BaseWorkerContext.java +++ b/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 cached = new HashSet<>(); private Map> allResourcesById = new HashMap>(); @@ -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()); + } + } + } + + } \ No newline at end of file diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/SimpleWorkerContext.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/SimpleWorkerContext.java index eb624a08b..efef85aa6 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/context/SimpleWorkerContext.java +++ b/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); } diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/renderers/ValueSetRenderer.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/renderers/ValueSetRenderer.java index 93f81735a..e435b4707 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/renderers/ValueSetRenderer.java +++ b/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 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) { - if (code == null) { - return null; + private Map getConceptsForCodes(CodeSystem e, ConceptSetComponent inc) { + if (e == 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; + ValueSetExpansionComponent vse = null; + 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 results = new HashMap<>(); + List 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))); + } } - - return getContext().getWorker().validateCode(getContext().getTerminologyServiceOptions(), inc.getSystem(), code, null).asConceptDefinition(); + if (!context.isNoSlowLookup() && !serverList.isEmpty()) { + 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 list, String code) { for (ConceptDefinitionComponent c : list) { if (code.equals(c.getCode())) From cbf70365f181489930d485b32d3dfdca30d763c8 Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Sun, 16 Aug 2020 06:48:06 +1000 Subject: [PATCH 12/13] fix value set validation to deal with multi-heirarchy --- .../terminologies/ValueSetCheckerSimple.java | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/ValueSetCheckerSimple.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/ValueSetCheckerSimple.java index 8e28bf481..e8f89e701 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/terminologies/ValueSetCheckerSimple.java +++ b/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 children = (List) 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 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; } From 4e30cd7849b9bdef12c8b3369c11a6396f083643 Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Sun, 16 Aug 2020 06:48:33 +1000 Subject: [PATCH 13/13] Add logging for value set validation performance --- .../instance/InstanceValidator.java | 2 +- .../instance/type/ValueSetValidator.java | 23 ++++++++++++++----- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/InstanceValidator.java b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/InstanceValidator.java index 2469011ad..93de432bc 100644 --- a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/InstanceValidator.java +++ b/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); } } diff --git a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/type/ValueSetValidator.java b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/type/ValueSetValidator.java index 8e284a902..354de77e1 100644 --- a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/type/ValueSetValidator.java +++ b/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 errors, Element vs, NodeStack stack) { @@ -52,28 +56,28 @@ public class ValueSetValidator extends BaseValidator { List 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 errors, Element compose, NodeStack stack) { + private void validateValueSetCompose(List errors, Element compose, NodeStack stack, String vsid) { List 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 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 errors, Element include, NodeStack stack) { + private void validateValueSetInclude(List errors, Element include, NodeStack stack, String vsid) { String system = include.getChildValue("system"); String version = include.getChildValue("version"); List 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());