From 8f6f75f4bf392ed6c187a45a6a9035a9261e537f Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Thu, 9 Jan 2025 17:08:28 +1100 Subject: [PATCH] beef up validation of CodeSystem properties that are codes --- .../src/main/resources/Messages.properties | 8 ++ .../instance/InstanceValidator.java | 15 ++- .../instance/type/CodeSystemValidator.java | 124 ++++++++++++++++-- .../4.0.1/vs-externals.json | 19 ++- .../5.0.0/vs-externals.json | 4 + 5 files changed, 147 insertions(+), 23 deletions(-) diff --git a/org.hl7.fhir.utilities/src/main/resources/Messages.properties b/org.hl7.fhir.utilities/src/main/resources/Messages.properties index 179613c3a..ee6a0e482 100644 --- a/org.hl7.fhir.utilities/src/main/resources/Messages.properties +++ b/org.hl7.fhir.utilities/src/main/resources/Messages.properties @@ -1207,3 +1207,11 @@ EXTENSION_FHIR_VERSION_LATEST = The definition of the extension ''{0}'' specifie VALUESET_INCLUDE_WRONG_VS = The system ''{0}'' is actually a value set VALUESET_INCLUDE_WRONG_VS_HINT = The system ''{0}'' is actually a value set, which itself refers to the system ''{1}'' so that may be what is intended here VALUESET_INCLUDE_WRONG_VS_MANY = The system ''{0}'' is actually a value set, which itself refers to the systems {1} +CODESYSTEM_PROPERTY_URI_UNKNOWN = The property uri ''{0}'' is not known, so the property values can not be validated fully +CODESYSTEM_PROPERTY_URI_UNKNOWN_TYPE = The property uri ''{0}'' is not known, so the property values can not be validated fully; unless specified elsewhere, codes will be treated as internal references +CODESYSTEM_PROPERTY_URI_UNKNOWN_BASE = The base property uri ''{0}'' is not known, so the property values can not be validated fully +CODESYSTEM_PROPERTY_URI_INVALID = The code ''{0}'' in the CodeSystem {2} for uri ''{1}'' is not valid +CODESYSTEM_PROPERTY_CODE_DEFAULT_WARNING = The type is ''code'', but no ValueSet information was found, so the codes will be validated as internal codes +CODESYSTEM_PROPERTY_VALUESET_NOT_FOUND = The ValueSet {0} is unknown, so the property codes cannot be validated +CODESYSTEM_PROPERTY_BAD_INTERNAL_REFERENCE = The code ''{0}'' is not a valid code in this code system +CODESYSTEM_PROPERTY_BAD_PROPERTY_CODE = The code ''{0}'' is not a valid code in the value set ''{1}'' 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 b333d9397..f87dcfa03 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 @@ -7503,9 +7503,12 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat if (!Utilities.noString(msg)) { msg = msg + " (log: " + msg + ")"; } + String msgId = null; if (inv.hasSource()) { - msg = context.formatMessage(I18nConstants.INV_FAILED_SOURCE, inv.getKey() + ": '" + inv.getHuman()+"'", inv.getSource())+msg; + msg = context.formatMessage(I18nConstants.INV_FAILED_SOURCE, inv.getKey() + ": '" + inv.getHuman()+"'", inv.getSource())+msg; + msgId = inv.getSource()+"#"+inv.getKey(); } else { + msgId = profile.getUrl()+"#"+inv.getKey(); msg = context.formatMessage(I18nConstants.INV_FAILED, inv.getKey() + ": '" + inv.getHuman()+"'")+msg; } String invId = (inv.hasSource() ? inv.getSource() : profile.getUrl()) + "#"+inv.getKey(); @@ -7514,15 +7517,15 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat ToolingExtensions.readBooleanExtension(inv, ToolingExtensions.EXT_BEST_PRACTICE)) { msg = msg +" (Best Practice Recommendation)"; if (bpWarnings == BestPracticeWarningLevel.Hint) - hintInv(errors, NO_RULE_DATE, IssueType.INVARIANT, element.line(), element.col(), path, invOK, msg, invId); + hintInv(errors, NO_RULE_DATE, IssueType.INVARIANT, element.line(), element.col(), path, invOK, msg, invId, msgId); else if (/*bpWarnings == null || */ bpWarnings == BestPracticeWarningLevel.Warning) - warningInv(errors, NO_RULE_DATE, IssueType.INVARIANT, element.line(), element.col(), path, invOK, msg, invId); + warningInv(errors, NO_RULE_DATE, IssueType.INVARIANT, element.line(), element.col(), path, invOK, msg, invId, msgId); else if (bpWarnings == BestPracticeWarningLevel.Error) - ok = ruleInv(errors, NO_RULE_DATE, IssueType.INVARIANT, element.line(), element.col(), path, invOK, msg, invId) && ok; + ok = ruleInv(errors, NO_RULE_DATE, IssueType.INVARIANT, element.line(), element.col(), path, invOK, msg, invId, msgId) && ok; } else if (inv.getSeverity() == ConstraintSeverity.ERROR) { - ok = ruleInv(errors, NO_RULE_DATE, IssueType.INVARIANT, element.line(), element.col(), path, invOK, msg, invId) && ok; + ok = ruleInv(errors, NO_RULE_DATE, IssueType.INVARIANT, element.line(), element.col(), path, invOK, msg, invId, msgId) && ok; } else if (inv.getSeverity() == ConstraintSeverity.WARNING) { - warningInv(errors, NO_RULE_DATE, IssueType.INVARIANT, element.line(), element.col(), path, invOK, msg, invId); + warningInv(errors, NO_RULE_DATE, IssueType.INVARIANT, element.line(), element.col(), path, invOK, msg, invId, msgId); } } return ok; diff --git a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/type/CodeSystemValidator.java b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/type/CodeSystemValidator.java index c318dd9f0..b7ad116cc 100644 --- a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/type/CodeSystemValidator.java +++ b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/type/CodeSystemValidator.java @@ -8,11 +8,14 @@ import java.util.Map; import java.util.Set; import org.hl7.fhir.exceptions.FHIRException; +import org.hl7.fhir.r4.model.codesystems.CodesystemAltcodeKind; import org.hl7.fhir.r5.elementmodel.Element; import org.hl7.fhir.r5.model.CodeSystem; import org.hl7.fhir.r5.model.CodeSystem.ConceptDefinitionComponent; +import org.hl7.fhir.r5.model.CodeSystem.ConceptPropertyComponent; import org.hl7.fhir.r5.model.ValueSet; import org.hl7.fhir.r5.terminologies.CodeSystemUtilities; +import org.hl7.fhir.r5.terminologies.utilities.ValidationResult; import org.hl7.fhir.utilities.CanonicalPair; import org.hl7.fhir.utilities.Utilities; import org.hl7.fhir.utilities.VersionUtilities; @@ -21,6 +24,7 @@ import org.hl7.fhir.utilities.validation.ValidationMessage; import org.hl7.fhir.utilities.validation.ValidationMessage.IssueType; import org.hl7.fhir.utilities.validation.ValidationOptions; import org.hl7.fhir.validation.BaseValidator; +import org.hl7.fhir.validation.cli.model.ValidationOutcome; import org.hl7.fhir.validation.instance.type.CodeSystemValidator.KnownProperty; import org.hl7.fhir.validation.instance.type.CodeSystemValidator.PropertyDef; import org.hl7.fhir.validation.instance.utils.NodeStack; @@ -60,7 +64,7 @@ public class CodeSystemValidator extends BaseValidator { } public enum CodeValidationRule { - NO_VALIDATION, INTERNAL_CODE, VS_ERROR, VS_WARNING + NO_VALIDATION, INTERNAL_CODE, INTERNAL_CODE_WARNING, VS_ERROR, VS_WARNING } public class PropertyDef { @@ -69,7 +73,7 @@ public class CodeSystemValidator extends BaseValidator { private String type; private CodeValidationRule rule; - private String valueset; + private ValueSet valueset; protected PropertyDef(String uri, String code, String type) { super(); @@ -78,7 +82,7 @@ public class CodeSystemValidator extends BaseValidator { this.type = type; } - public void setCodeValidationRules(CodeValidationRule rule, String valueset) { + public void setCodeValidationRules(CodeValidationRule rule, ValueSet valueset) { this.rule = rule; this.valueset = valueset; } @@ -92,13 +96,18 @@ public class CodeSystemValidator extends BaseValidator { public String getType() { return type; } - public String getValueset() { + public ValueSet getValueset() { return valueset; } + public CodeValidationRule getRule() { + return rule; + } + } private static final String VS_PROP_STATUS = null; + private Set propertyCodes = new HashSet(); public CodeSystemValidator(BaseValidator parent) { super(parent); @@ -237,11 +246,48 @@ public class CodeSystemValidator extends BaseValidator { PropertyDef pd = new PropertyDef(uri, code, type); KnownProperty ukp = null; KnownProperty ckp = null; + boolean foundPropDefn = false; + CodeValidationRule ruleFromUri = CodeValidationRule.INTERNAL_CODE_WARNING; + String valuesetFromUri = null; + if (uri != null) { if (rule(errors, "2024-03-06", IssueType.BUSINESSRULE, cs.line(), cs.col(), stack.getLiteralPath(), Utilities.isAbsoluteUrl(uri), I18nConstants.CODESYSTEM_PROPERTY_ABSOLUTE_URI, uri)) { if (rule(errors, "2024-03-06", IssueType.BUSINESSRULE, cs.line(), cs.col(), stack.getLiteralPath(), !properties.containsKey(uri), I18nConstants.CODESYSTEM_PROPERTY_DUPLICATE_URI, uri)) { properties.put(uri, pd); + if (uri.contains("#")) { + String base = uri.substring(0, uri.indexOf("#")); + String pcode = uri.substring(uri.indexOf("#")+1); + CodeSystem pcs = context.findTxResource(CodeSystem.class, base); + if (pcs == null) { + warning(errors, "2025-01-09", IssueType.NOTFOUND, cs.line(), cs.col(), stack.getLiteralPath(), false, I18nConstants.CODESYSTEM_PROPERTY_URI_UNKNOWN_BASE, base); + } else { + ConceptDefinitionComponent cc = CodeSystemUtilities.findCode(pcs.getConcept(), pcode); + if (rule(errors, "2025-01-09", IssueType.INVALID, cs.line(), cs.col(), stack.getLiteralPath(), cc != null, I18nConstants.CODESYSTEM_PROPERTY_URI_INVALID, pcode, base, pcs.present())) { + foundPropDefn = true; + if ("code".equals(type)) { + ConceptPropertyComponent ccp = CodeSystemUtilities.getProperty(cc, "binding"); + if (ccp != null && ccp.hasValue() && ccp.getValue().hasPrimitiveValue()) { + ruleFromUri = CodeValidationRule.VS_ERROR; + valuesetFromUri = ccp.getValue().primitiveValue(); + } else { + ruleFromUri = CodeValidationRule.INTERNAL_CODE_WARNING; + } + } + } else { + ok = false; + if ("code".equals(type)) { + ruleFromUri = CodeValidationRule.INTERNAL_CODE_WARNING; + } + } + } + } else { + if ("code".equals(type)) { + warning(errors, "2025-01-09", IssueType.NOTFOUND, cs.line(), cs.col(), stack.getLiteralPath(), false, I18nConstants.CODESYSTEM_PROPERTY_URI_UNKNOWN_TYPE, uri); + } else { + hint(errors, "2025-01-09", IssueType.NOTFOUND, cs.line(), cs.col(), stack.getLiteralPath(), false, I18nConstants.CODESYSTEM_PROPERTY_URI_UNKNOWN, uri); + } + } } else { ok = false; } @@ -352,21 +398,26 @@ public class CodeSystemValidator extends BaseValidator { pd.setCodeValidationRules(CodeValidationRule.INTERNAL_CODE, null); break; case Status: - pd.setCodeValidationRules(CodeValidationRule.VS_WARNING, VS_PROP_STATUS); + pd.setCodeValidationRules(CodeValidationRule.VS_WARNING, findVS(errors, cs, stack, VS_PROP_STATUS, I18nConstants.CODESYSTEM_PROPERTY_VALUESET_NOT_FOUND)); break; default: break; } } else if ("code".equals(pd.getType())) { - if (property.hasExtension("http://hl7.org/fhir/6.0/StructureDefinition/extension-CodeSystem.property.valueSet")) { - pd.setCodeValidationRules(CodeValidationRule.VS_ERROR, property.getExtensionValue("http://hl7.org/fhir/6.0/StructureDefinition/extension-CodeSystem.property.valueSet").primitiveValue()); + if (property.hasExtension("http://hl7.org/fhir/StructureDefinition/codesystem-property-valueset", "http://hl7.org/fhir/6.0/StructureDefinition/extension-CodeSystem.property.valueSet")) { + pd.setCodeValidationRules(CodeValidationRule.VS_ERROR, findVS(errors, cs, stack, + property.getExtensionValue("http://hl7.org/fhir/StructureDefinition/codesystem-property-valueset", "http://hl7.org/fhir/6.0/StructureDefinition/extension-CodeSystem.property.valueSet").primitiveValue(), + I18nConstants.CODESYSTEM_PROPERTY_VALUESET_NOT_FOUND)); + } else if (foundPropDefn && valuesetFromUri != null) { + pd.setCodeValidationRules(ruleFromUri, findVS(errors, cs, stack, valuesetFromUri, I18nConstants.CODESYSTEM_PROPERTY_VALUESET_NOT_FOUND)); } else if (VersionUtilities.isR6Plus(context.getVersion())) { hint(errors, "2024-03-18", IssueType.BUSINESSRULE, cs.line(), cs.col(), stack.getLiteralPath(), ukp != null && type.equals(ukp.getType()), I18nConstants.CODESYSTEM_PROPERTY_CODE_WARNING); } else { - + pd.setCodeValidationRules(ruleFromUri, null); + hint(errors, "2025-01-09", IssueType.BUSINESSRULE, cs.line(), cs.col(), stack.getLiteralPath(), false, I18nConstants.CODESYSTEM_PROPERTY_CODE_DEFAULT_WARNING); } - } else if ("Coding".equals(pd.getType()) && property.hasExtension("http://hl7.org/fhir/6.0/StructureDefinition/extension-CodeSystem.property.valueSet")) { - pd.setCodeValidationRules(CodeValidationRule.VS_ERROR, property.getExtensionValue("http://hl7.org/fhir/6.0/StructureDefinition/extension-CodeSystem.property.valueSet").primitiveValue()); + } else if ("Coding".equals(pd.getType()) && property.hasExtension("http://hl7.org/fhir/StructureDefinition/codesystem-property-valueset", "http://hl7.org/fhir/6.0/StructureDefinition/extension-CodeSystem.property.valueSet")) { + pd.setCodeValidationRules(CodeValidationRule.VS_ERROR, findVS(errors, cs, stack, property.getExtensionValue("http://hl7.org/fhir/StructureDefinition/codesystem-property-valueset", "http://hl7.org/fhir/6.0/StructureDefinition/extension-CodeSystem.property.valueSet").primitiveValue(), I18nConstants.CODESYSTEM_PROPERTY_VALUESET_NOT_FOUND)); } if (uri == null) { @@ -382,6 +433,18 @@ public class CodeSystemValidator extends BaseValidator { return ok; } + private ValueSet findVS(List errors, Element cs, NodeStack stack, String url, String message) { + if (url == null) { + return null; + } else { + ValueSet vs = context.findTxResource(ValueSet.class, url); + if (vs != null) { + warning(errors, "2025-01-09", IssueType.NOTFOUND, cs.line(), cs.col(), stack.getLiteralPath(), false, message, url); + } + return vs; + } + } + private boolean isBaseSpec(String url) { return url.startsWith("http://hl7.org/fhir/") && !url.substring(20).contains("/"); } @@ -480,7 +543,9 @@ public class CodeSystemValidator extends BaseValidator { if (rule(errors, "2024-03-06", IssueType.BUSINESSRULE, cs.line(), cs.col(), stack.getLiteralPath(), defn != null, I18nConstants.CODESYSTEM_PROPERTY_UNDEFINED, code) && rule(errors, "2024-03-06", IssueType.BUSINESSRULE, cs.line(), cs.col(), stack.getLiteralPath(), value != null, I18nConstants.CODESYSTEM_PROPERTY_NO_VALUE, code) && rule(errors, "2024-03-06", IssueType.BUSINESSRULE, cs.line(), cs.col(), stack.getLiteralPath(), value.fhirType().equals(defn.type), I18nConstants.CODESYSTEM_PROPERTY_WRONG_TYPE, code, value.fhirType(), defn.type)) { - // nothing? + if ("code".equals(value.fhirType())) { + checkCodeProperty(errors, cs, stack, defn, value.primitiveValue(), codes); + } } else { ok = false; } @@ -492,6 +557,43 @@ public class CodeSystemValidator extends BaseValidator { return ok; } + private void checkCodeProperty(List errors, Element cs, NodeStack stack, PropertyDef defn, String code, Set codes) { + switch (defn.getRule()) { + case INTERNAL_CODE: + if (!isSeenPropertyCode(defn, code)) { + rule(errors, "2025-01-09", IssueType.INVALID, cs.line(), cs.col(), stack.getLiteralPath(), codes.contains(code), I18nConstants.CODESYSTEM_PROPERTY_BAD_INTERNAL_REFERENCE, code); + } + break; + case INTERNAL_CODE_WARNING: + if (!isSeenPropertyCode(defn, code)) { + warning(errors, "2025-01-09", IssueType.INVALID, cs.line(), cs.col(), stack.getLiteralPath(), codes.contains(code), I18nConstants.CODESYSTEM_PROPERTY_BAD_INTERNAL_REFERENCE, code); + } + break; + case VS_ERROR: + if (defn.getValueset() != null && !isSeenPropertyCode(defn, code)) { + ValidationResult vo = context.validateCode(baseOptions, code, defn.getValueset()); + rule(errors, "2025-01-09", IssueType.INVALID, cs.line(), cs.col(), stack.getLiteralPath(), vo.isOk(), I18nConstants.CODESYSTEM_PROPERTY_BAD_PROPERTY_CODE, code); + } + break; + case VS_WARNING: + if (defn.getValueset() != null && !isSeenPropertyCode(defn, code)) { + ValidationResult vo = context.validateCode(baseOptions, code, defn.getValueset()); + warning(errors, "2025-01-09", IssueType.INVALID, cs.line(), cs.col(), stack.getLiteralPath(), vo.isOk(), I18nConstants.CODESYSTEM_PROPERTY_BAD_PROPERTY_CODE, code, defn.getValueset().getVersionedUrl()); + } + break; + default: + case NO_VALIDATION: + break; + } + } + + private boolean isSeenPropertyCode(PropertyDef defn, String code) { + String key = defn.getValueset() != null ? defn.getValueset().getVersionedUrl()+"#"+code : "null#"+code; + boolean isnew = propertyCodes.contains(key); + propertyCodes.add(key); + return isnew; + } + private boolean checkShareableCodeSystem(List errors, Element cs, NodeStack stack) { if (parent.isForPublication()) { if (isHL7(cs)) { diff --git a/org.hl7.fhir.validation/src/test/resources/txCache/org.hl7.fhir.validation/4.0.1/vs-externals.json b/org.hl7.fhir.validation/src/test/resources/txCache/org.hl7.fhir.validation/4.0.1/vs-externals.json index 62253e09a..bfc24d8e9 100644 --- a/org.hl7.fhir.validation/src/test/resources/txCache/org.hl7.fhir.validation/4.0.1/vs-externals.json +++ b/org.hl7.fhir.validation/src/test/resources/txCache/org.hl7.fhir.validation/4.0.1/vs-externals.json @@ -1,17 +1,24 @@ { - "http://fhir.abda.de/eRezeptAbgabedaten/ValueSet/DAV-VS-ERP-DEUEV-Anlage-8" : null, - "http://loinc.org/vs/LL378-1" : null, + "http://loinc.org|2.76" : null, "http://www.rfc-editor.org/bcp/bcp13.txt" : null, - "http://loinc.org/vs/LL1971-2" : null, + "#f3b2bd36-199b-4591-b4db-f49db0912b6|null" : null, + "#c1|null" : null, "http://hl7.org/fhir/us/davinci-hrex/ValueSet/hrex-endpoint-name" : null, "http://fhir.ch/ig/ch-ig/ValueSet/ch-ig-example" : null, "http://hl7.org/fhir/us/qicore/ValueSet/qicore-negation-reason" : null, + "http://fhir.ch/ig/ch-ig/ValueSet/OrganizationType" : null, + "https://fhir.kbv.de/ValueSet/KBV_VS_SFHIR_ICD_SEITENLOKALISATION" : null, + "http://fhir.abda.de/eRezeptAbgabedaten/ValueSet/DAV-VS-ERP-DEUEV-Anlage-8" : null, + "http://loinc.org/vs/LL378-1" : null, + "http://something/something|null" : null, + "http://hl7.org/fhir/CodeSystem/c1|null" : null, + "http://hl7.org/fhir/uv/sdc/CodeSystem/CSPHQ9|null" : null, + "http://loinc.org/vs/LL1971-2" : null, + "#f3b2bd36-199b-4591-b4db-f49db0912b62|null" : null, "http://hl7.org/fhir/smart-app-launch/ValueSet/user-access-category" : null, "https://healthterminologies.gov.au/fhir/ValueSet/australian-immunisation-register-vaccine-1" : { "server" : "https://tx.ontoserver.csiro.au/fhir", "filename" : "vs-0ce1569f-f985-44cb-a5a0-9d205efd76b4.json" }, - "http://fhir.ch/ig/ch-ig/ValueSet/OrganizationType" : null, - "http://loinc.org/vs/LL4048-6" : null, - "https://fhir.kbv.de/ValueSet/KBV_VS_SFHIR_ICD_SEITENLOKALISATION" : null + "http://loinc.org/vs/LL4048-6" : null } diff --git a/org.hl7.fhir.validation/src/test/resources/txCache/org.hl7.fhir.validation/5.0.0/vs-externals.json b/org.hl7.fhir.validation/src/test/resources/txCache/org.hl7.fhir.validation/5.0.0/vs-externals.json index c09095f3e..7a3a08b98 100644 --- a/org.hl7.fhir.validation/src/test/resources/txCache/org.hl7.fhir.validation/5.0.0/vs-externals.json +++ b/org.hl7.fhir.validation/src/test/resources/txCache/org.hl7.fhir.validation/5.0.0/vs-externals.json @@ -1,8 +1,12 @@ { "http://hl7.org/fhir/ValueSet/ucum-vitals-common|4.0.0" : null, "http://loinc.org/vs" : null, + "#cs1|null" : null, + "#cs|null" : null, "http://hl7.org.au/fhir/ValueSet/au-hl7v2-0203" : null, "http://hl7.org/fhir/ValueSet/name-use|4.0.1" : null, + "http://something|null" : null, + "http://snomed.info/sct|null" : null, "http://hl7.org/fhir/ValueSet/observation-status|4.0.0" : null, "http://hl7.org/fhir/ValueSet/identifier-use|4.0.1" : null, "http://snomed.info/sct?fhir_vs" : null