From 098b2895bcae1ffa51b6ee8f79b1b81fbcc4da4b Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Tue, 4 Oct 2022 14:06:30 +1100 Subject: [PATCH] Markdown changes for FHIR-38714 + fix up test framework for validator to use OperationOutcome --- .../r5/utils/OperationOutcomeUtilities.java | 29 +++ .../hl7/fhir/utilities/MarkDownProcessor.java | 32 +++- .../fhir/utilities/i18n/I18nConstants.java | 1 + .../utilities/json/JsonTrackingParser.java | 2 +- .../src/main/resources/Messages.properties | 4 +- .../tests/MarkdownPreprocessorTesting.java | 30 +++ .../hl7/fhir/validation/ValidationEngine.java | 3 + .../fhir/validation/cli/model/CliContext.java | 12 ++ .../cli/model/HtmlInMarkdownCheck.java | 30 +++ .../cli/services/ValidationService.java | 1 + .../hl7/fhir/validation/cli/utils/Params.java | 15 +- .../instance/InstanceValidator.java | 59 +++++- .../validation/tests/ValidationTests.java | 171 +++++++++++++----- 13 files changed, 338 insertions(+), 51 deletions(-) create mode 100644 org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/tests/MarkdownPreprocessorTesting.java create mode 100644 org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/cli/model/HtmlInMarkdownCheck.java diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/utils/OperationOutcomeUtilities.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/utils/OperationOutcomeUtilities.java index bc1e7e092..803d54369 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/utils/OperationOutcomeUtilities.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/utils/OperationOutcomeUtilities.java @@ -129,4 +129,33 @@ public class OperationOutcomeUtilities { } return res; } + + + public static OperationOutcomeIssueComponent convertToIssueSimple(ValidationMessage message, OperationOutcome op) { + OperationOutcomeIssueComponent issue = new OperationOutcome.OperationOutcomeIssueComponent(); + issue.setUserData("source.vm", message); + issue.setCode(convert(message.getType())); + + if (message.getLocation() != null) { + // message location has a fhirPath in it. We need to populate the expression + issue.addExpression(message.getLocation()); + } + if (message.getLine() >= 0 && message.getCol() >= 0) { + issue.setDiagnostics("["+message.getLine()+","+message.getCol()+"]"); + } + issue.setSeverity(convert(message.getLevel())); + CodeableConcept c = new CodeableConcept(); + c.setText(message.getMessage()); + issue.setDetails(c); + return issue; + } + + public static OperationOutcome createOutcomeSimple(List messages) { + OperationOutcome res = new OperationOutcome(); + for (ValidationMessage vm : messages) { + res.addIssue(convertToIssueSimple(vm, res)); + } + return res; + } + } \ No newline at end of file diff --git a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/MarkDownProcessor.java b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/MarkDownProcessor.java index 279ab2be9..1ef695297 100644 --- a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/MarkDownProcessor.java +++ b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/MarkDownProcessor.java @@ -64,11 +64,41 @@ public class MarkDownProcessor { } switch (dialect) { case DARING_FIREBALL : return Processor.process(source); - case COMMON_MARK : return processCommonMark(source); + case COMMON_MARK : return processCommonMark(preProcess(source)); default: throw new Error("Unknown Markdown Dialect: "+dialect.toString()+" at "+context); } } + /** + * This deals with a painful problem created by the intersection of previous publishing processes + * and the way commonmark specifies that < is handled in content. For control reasons, the FHIR specification does + * not allow raw html tags in the markdown + * + * This check finds any raw <[x] where [x] is any alpha character, and prepends \ to it so that it + * renders as a < (e.g. gets escaped in the output HTML) + * + * This is public to enable testing (not for direct use otherwise) + * + * @param source + * @return + */ + public static String preProcess(String source) { + StringBuilder b = new StringBuilder(); + for (int i = 0; i < source.length(); i++) { + char last = i > 0 ? source.charAt(i-1) : 0; + char current = source.charAt(i); + char next = i < source.length() -1 ? source.charAt(i+1) : 0; + if (current == '<' && Character.isAlphabetic(next) && last != '\\') { + b.append('\\'); + b.append(current); + } else { + b.append(current); + } + } + return b.toString(); + } + + private String processCommonMark(String source) { Set extensions = Collections.singleton(TablesExtension.create()); Parser parser = Parser.builder().extensions(extensions).build(); 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 bd0e289fe..a226608f3 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 @@ -488,6 +488,7 @@ public class I18nConstants { public static final String TYPE_SPECIFIC_CHECKS_DT_BASE64_VALID = "Type_Specific_Checks_DT_Base64_Valid"; public static final String TYPE_SPECIFIC_CHECKS_DT_BOOLEAN_VALUE = "Type_Specific_Checks_DT_Boolean_Value"; public static final String TYPE_SPECIFIC_CHECKS_DT_CODE_WS = "Type_Specific_Checks_DT_Code_WS"; + public static final String TYPE_SPECIFIC_CHECKS_DT_MARKDOWN_HTML = "TYPE_SPECIFIC_CHECKS_DT_MARKDOWN_HTML"; public static final String TYPE_SPECIFIC_CHECKS_DT_DATETIME_REASONABLE = "Type_Specific_Checks_DT_DateTime_Reasonable"; public static final String TYPE_SPECIFIC_CHECKS_DT_DATETIME_REGEX = "Type_Specific_Checks_DT_DateTime_Regex"; public static final String TYPE_SPECIFIC_CHECKS_DT_DATETIME_TZ = "Type_Specific_Checks_DT_DateTime_TZ"; diff --git a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/json/JsonTrackingParser.java b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/json/JsonTrackingParser.java index 47e5e27a8..84fc35ba4 100644 --- a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/json/JsonTrackingParser.java +++ b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/json/JsonTrackingParser.java @@ -686,7 +686,7 @@ public class JsonTrackingParser { } public static void write(JsonObject json, File file) throws IOException { - Gson gson = new GsonBuilder().setPrettyPrinting().create(); + Gson gson = new GsonBuilder().setPrettyPrinting().disableHtmlEscaping().create(); String jcnt = gson.toJson(json); TextFile.stringToFile(jcnt, file); } diff --git a/org.hl7.fhir.utilities/src/main/resources/Messages.properties b/org.hl7.fhir.utilities/src/main/resources/Messages.properties index b57cffc81..74112502d 100644 --- a/org.hl7.fhir.utilities/src/main/resources/Messages.properties +++ b/org.hl7.fhir.utilities/src/main/resources/Messages.properties @@ -384,7 +384,7 @@ Error_parsing_JSON_ = Error parsing JSON: {0} Node_type__is_not_allowed = Node type {0} is not allowed CDATA_is_not_allowed = CDATA is not allowed Undefined_element_ = Undefined element ''{0}'' -Undefined_attribute__on__for_type__properties__ = Undefined attribute ''@{0}'' on {1} for type {2} (properties = {3}) +Undefined_attribute__on__for_type__properties__ = Undefined attribute ''@{0}'' on {1} for type {2} Text_should_not_be_present = Text should not be present (''{0}'') Wrong_namespace__expected_ = Wrong namespace - expected ''{0}'' Element_must_have_some_content = Element must have some content @@ -734,3 +734,5 @@ MEASURE_SHAREABLE_MISSING = The ShareableMeasure profile says that the {0} eleme MEASURE_SHAREABLE_EXTRA_MISSING = The ShareableMeasure profile recommends that the {0} element is populated, but it is not present. Published measures SHOULD conform to the ShareableMeasure profile MEASURE_SHAREABLE_MISSING_HL7 = The ShareableMeasure profile says that the {0} element is mandatory, but it is not found. HL7 Published measures SHALL conform to the ShareableMeasure profile MEASURE_SHAREABLE_EXTRA_MISSING_HL7 = The ShareableMeasure profile recommends that the {0} element is populated, but it is not found. HL7 Published measures SHALL conform to the ShareableMeasure profile +TYPE_SPECIFIC_CHECKS_DT_MARKDOWN_HTML = The markdown contains content that appears to be an embedded HTML tag starting at ''{0}''. This will (or SHOULD) be escaped by the presentation layer. The content should be checked to confirm that this is the desired behaviour + diff --git a/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/tests/MarkdownPreprocessorTesting.java b/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/tests/MarkdownPreprocessorTesting.java new file mode 100644 index 000000000..cb3249a60 --- /dev/null +++ b/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/tests/MarkdownPreprocessorTesting.java @@ -0,0 +1,30 @@ +package org.hl7.fhir.utilities.tests; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.io.IOException; + +import org.hl7.fhir.utilities.MarkDownProcessor; +import org.junit.jupiter.api.Test; + +public class MarkdownPreprocessorTesting { + + @Test + public void testSimple() throws IOException { + assertEquals(MarkDownProcessor.preProcess("1 < 2"), "1 < 2"); + } + + @Test + public void testHTML() throws IOException { + assertEquals(MarkDownProcessor.preProcess(""), "\\"); + assertEquals(MarkDownProcessor.preProcess("\\"), "\\"); + } + + + @Test + public void testBorder() throws IOException { + assertEquals(MarkDownProcessor.preProcess("<>"), "<>"); + assertEquals(MarkDownProcessor.preProcess("><"), "><"); + } + +} diff --git a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/ValidationEngine.java b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/ValidationEngine.java index 33c051671..76fbf57ea 100644 --- a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/ValidationEngine.java +++ b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/ValidationEngine.java @@ -53,6 +53,7 @@ import org.hl7.fhir.utilities.npm.ToolsVersion; import org.hl7.fhir.utilities.validation.ValidationMessage; import org.hl7.fhir.utilities.xhtml.XhtmlComposer; import org.hl7.fhir.validation.BaseValidator.ValidationControl; +import org.hl7.fhir.validation.cli.model.HtmlInMarkdownCheck; import org.hl7.fhir.validation.cli.services.IPackageInstaller; import org.hl7.fhir.validation.cli.utils.*; import org.hl7.fhir.validation.instance.InstanceValidator; @@ -160,6 +161,7 @@ public class ValidationEngine implements IValidatorResourceFetcher, IValidationP @Getter @Setter private boolean allowExampleUrls; @Getter @Setter private boolean showMessagesFromReferences; @Getter @Setter private boolean doImplicitFHIRPathStringConversion; + @Getter @Setter private HtmlInMarkdownCheck htmlInMarkdownCheck; @Getter @Setter private Locale locale; @Getter @Setter private List igs = new ArrayList<>(); @Getter @Setter private List extensionDomains = new ArrayList<>(); @@ -625,6 +627,7 @@ public class ValidationEngine implements IValidatorResourceFetcher, IValidationP validator.getValidationControl().putAll(validationControl); validator.setQuestionnaireMode(questionnaireMode); validator.setLevel(level); + validator.setHtmlInMarkdownCheck(htmlInMarkdownCheck); validator.setNoUnicodeBiDiControlChars(noUnicodeBiDiControlChars); validator.setDoImplicitFHIRPathStringConversion(doImplicitFHIRPathStringConversion); if (format == FhirFormat.SHC) { diff --git a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/cli/model/CliContext.java b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/cli/model/CliContext.java index ec5df3e11..af6d73837 100644 --- a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/cli/model/CliContext.java +++ b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/cli/model/CliContext.java @@ -48,6 +48,8 @@ public class CliContext { private boolean wantInvariantsInMessages = false; @JsonProperty("doImplicitFHIRPathStringConversion") private boolean doImplicitFHIRPathStringConversion = false; + @JsonProperty("htmlInMarkdownCheck") + private HtmlInMarkdownCheck htmlInMarkdownCheck = HtmlInMarkdownCheck.WARNING; @JsonProperty("map") private String map = null; @@ -248,6 +250,16 @@ public class CliContext { this.doImplicitFHIRPathStringConversion = doImplicitFHIRPathStringConversion; } + @JsonProperty("htmlInMarkdownCheck") + public HtmlInMarkdownCheck getHtmlInMarkdownCheck() { + return htmlInMarkdownCheck; + } + + @JsonProperty("htmlInMarkdownCheck") + public void setHtmlInMarkdownCheck(HtmlInMarkdownCheck htmlInMarkdownCheck) { + this.htmlInMarkdownCheck = htmlInMarkdownCheck; + } + @JsonProperty("locale") public String getLanguageCode() { return locale; diff --git a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/cli/model/HtmlInMarkdownCheck.java b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/cli/model/HtmlInMarkdownCheck.java new file mode 100644 index 000000000..08585daa6 --- /dev/null +++ b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/cli/model/HtmlInMarkdownCheck.java @@ -0,0 +1,30 @@ +package org.hl7.fhir.validation.cli.model; + +import org.hl7.fhir.utilities.Utilities; + +public enum HtmlInMarkdownCheck { + NONE, + WARNING, + ERROR; + + public static boolean isValidCode(String q) { + return fromCode(q) != null; + } + + public static HtmlInMarkdownCheck fromCode(String q) { + if (Utilities.noString(q)) { + return null; + } + q = q.toLowerCase(); + if (Utilities.existsInList(q, "n", "none", "ignore", "i")) { + return NONE; + } + if (Utilities.existsInList(q, "w", "warning", "warnings", "warn")) { + return WARNING; + } + if (Utilities.existsInList(q, "e", "error", "errors", "err")) { + return ERROR; + } + return null; + } +} diff --git a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/cli/services/ValidationService.java b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/cli/services/ValidationService.java index 457bd4b35..0faa41b41 100644 --- a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/cli/services/ValidationService.java +++ b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/cli/services/ValidationService.java @@ -372,6 +372,7 @@ public class ValidationService { validator.setAssumeValidRestReferences(cliContext.isAssumeValidRestReferences()); validator.setShowMessagesFromReferences(cliContext.isShowMessagesFromReferences()); validator.setDoImplicitFHIRPathStringConversion(cliContext.isDoImplicitFHIRPathStringConversion()); + validator.setHtmlInMarkdownCheck(cliContext.getHtmlInMarkdownCheck()); validator.setNoExtensibleBindingMessages(cliContext.isNoExtensibleBindingMessages()); validator.setNoUnicodeBiDiControlChars(cliContext.isNoUnicodeBiDiControlChars()); validator.setNoInvariantChecks(cliContext.isNoInvariants()); diff --git a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/cli/utils/Params.java b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/cli/utils/Params.java index e80378cc0..17e7d0800 100644 --- a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/cli/utils/Params.java +++ b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/cli/utils/Params.java @@ -5,6 +5,7 @@ import org.hl7.fhir.r5.terminologies.JurisdictionUtilities; import org.hl7.fhir.r5.utils.validation.BundleValidationRule; import org.hl7.fhir.utilities.VersionUtilities; import org.hl7.fhir.validation.cli.model.CliContext; +import org.hl7.fhir.validation.cli.model.HtmlInMarkdownCheck; import java.io.File; import java.util.Arrays; @@ -69,7 +70,8 @@ public class Params { public static final String ALLOW_EXAMPLE_URLS = "-allow-example-urls"; public static final String OUTPUT_STYLE = "-output-style"; public static final String DO_IMPLICIT_FHIRPATH_STRING_CONVERSION = "-implicit-fhirpath-string-conversions"; - private static final Object JURISDICTION = "-jurisdiction"; + public static final String JURISDICTION = "-jurisdiction"; + public static final String HTML_IN_MARKDOWN = "-html-in-markdown"; public static final String RUN_TESTS = "-run-tests"; @@ -176,6 +178,17 @@ public class Params { cliContext.setShowMessagesFromReferences(true); } else if (args[i].equals(DO_IMPLICIT_FHIRPATH_STRING_CONVERSION)) { cliContext.setDoImplicitFHIRPathStringConversion(true); + } else if (args[i].equals(HTML_IN_MARKDOWN)) { + if (i + 1 == args.length) + throw new Error("Specified "+HTML_IN_MARKDOWN+" without indicating mode"); + else { + String q = args[++i]; + if (!HtmlInMarkdownCheck.isValidCode(q)) { + throw new Error("Specified "+HTML_IN_MARKDOWN+" with na invalid code - must be ignore, warning, or error"); + } else { + cliContext.setHtmlInMarkdownCheck(HtmlInMarkdownCheck.fromCode(q)); + } + } } else if (args[i].equals(LOCALE)) { if (i + 1 == args.length) { throw new Error("Specified -locale without indicating locale"); 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 597a64cbc..0dc80ac8d 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 @@ -42,6 +42,8 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Calendar; import java.util.Collection; +import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -142,6 +144,7 @@ import org.hl7.fhir.r5.utils.ToolingExtensions; import org.hl7.fhir.r5.utils.XVerExtensionManager; import org.hl7.fhir.r5.utils.validation.constants.*; import org.hl7.fhir.utilities.CommaSeparatedStringBuilder; +import org.hl7.fhir.utilities.MarkDownProcessor; import org.hl7.fhir.utilities.SIDUtilities; import org.hl7.fhir.utilities.SimpleTimeTracker; import org.hl7.fhir.utilities.TimeTracker; @@ -159,9 +162,12 @@ import org.hl7.fhir.utilities.validation.ValidationOptions; import org.hl7.fhir.utilities.xhtml.NodeType; import org.hl7.fhir.utilities.xhtml.XhtmlNode; import org.hl7.fhir.validation.BaseValidator; +import org.hl7.fhir.validation.cli.model.HtmlInMarkdownCheck; import org.hl7.fhir.validation.cli.utils.QuestionnaireMode; import org.hl7.fhir.validation.cli.utils.ValidationLevel; import org.hl7.fhir.validation.instance.InstanceValidator.CanonicalResourceLookupResult; +import org.hl7.fhir.validation.instance.InstanceValidator.CanonicalTypeSorter; +import org.hl7.fhir.validation.instance.InstanceValidator.StructureDefinitionSorterByUrl; import org.hl7.fhir.validation.instance.type.BundleValidator; import org.hl7.fhir.validation.instance.type.CodeSystemValidator; import org.hl7.fhir.validation.instance.type.MeasureValidator; @@ -199,6 +205,25 @@ import com.google.gson.JsonObject; */ public class InstanceValidator extends BaseValidator implements IResourceValidator { + + public class StructureDefinitionSorterByUrl implements Comparator { + + @Override + public int compare(StructureDefinition o1, StructureDefinition o2) { + return o1.getUrl().compareTo(o2.getUrl()); + } + + } + + public class CanonicalTypeSorter implements Comparator { + + @Override + public int compare(CanonicalType o1, CanonicalType o2) { + return o1.getValue().compareTo(o2.getValue()); + } + + } + public class CanonicalResourceLookupResult { private CanonicalResource resource; @@ -387,6 +412,7 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat private boolean noCheckAggregation; private boolean wantCheckSnapshotUnchanged; private boolean noUnicodeBiDiControlChars; + private HtmlInMarkdownCheck htmlInMarkdownCheck; private List igs = new ArrayList<>(); private List extensionDomains = new ArrayList(); @@ -2134,7 +2160,7 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat if (!"xhtml".equals(type)) { if (securityChecks) { rule(errors, IssueType.INVALID, e.line(), e.col(), path, !containsHtmlTags(e.primitiveValue()), I18nConstants.SECURITY_STRING_CONTENT_ERROR); - } else { + } else if (!"markdown".equals(type)){ hint(errors, IssueType.INVALID, e.line(), e.col(), path, !containsHtmlTags(e.primitiveValue()), I18nConstants.SECURITY_STRING_CONTENT_WARNING); } } @@ -2318,6 +2344,17 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat checkPrimitiveBinding(hostContext, errors, path, type, context, e, profile, node); } + if (type.equals("markdown") && htmlInMarkdownCheck != HtmlInMarkdownCheck.NONE) { + String raw = e.primitiveValue(); + String processed = MarkDownProcessor.preProcess(raw); + if (!raw.equals(processed)) { + int i = 0; + while (i < raw.length() && raw.charAt(1) == processed.charAt(i)) { + i++; + } + warningOrError(htmlInMarkdownCheck == HtmlInMarkdownCheck.ERROR, errors, IssueType.INVALID, e.line(), e.col(), path, false, I18nConstants.TYPE_SPECIFIC_CHECKS_DT_MARKDOWN_HTML, raw.subSequence(i, 2)); + } + } if (type.equals("xhtml")) { XhtmlNode xhtml = e.getXhtml(); if (xhtml != null) { // if it is null, this is an error already noted in the parsers @@ -3226,7 +3263,10 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat return true; } - private String asListByUrl(Collection list) { + private String asListByUrl(Collection coll) { + List list = new ArrayList<>(); + list.addAll(coll); + Collections.sort(list, new StructureDefinitionSorterByUrl()); CommaSeparatedStringBuilder b = new CommaSeparatedStringBuilder(); for (StructureDefinition sd : list) { b.append(sd.getUrl()); @@ -3234,7 +3274,10 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat return b.toString(); } - private String asList(Collection list) { + private String asList(Collection coll) { + List list = new ArrayList<>(); + list.addAll(coll); + Collections.sort(list, new CanonicalTypeSorter()); CommaSeparatedStringBuilder b = new CommaSeparatedStringBuilder(); for (CanonicalType c : list) { b.append(c.getValue()); @@ -6025,6 +6068,16 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat this.noUnicodeBiDiControlChars = noUnicodeBiDiControlChars; } + + + public HtmlInMarkdownCheck getHtmlInMarkdownCheck() { + return htmlInMarkdownCheck; + } + + public void setHtmlInMarkdownCheck(HtmlInMarkdownCheck htmlInMarkdownCheck) { + this.htmlInMarkdownCheck = htmlInMarkdownCheck; + } + public Coding getJurisdiction() { return jurisdiction; } diff --git a/org.hl7.fhir.validation/src/test/java/org/hl7/fhir/validation/tests/ValidationTests.java b/org.hl7.fhir.validation/src/test/java/org/hl7/fhir/validation/tests/ValidationTests.java index 5ef3f6758..e626dc7bb 100644 --- a/org.hl7.fhir.validation/src/test/java/org/hl7/fhir/validation/tests/ValidationTests.java +++ b/org.hl7.fhir.validation/src/test/java/org/hl7/fhir/validation/tests/ValidationTests.java @@ -1,6 +1,10 @@ package org.hl7.fhir.validation.tests; +import static org.junit.Assert.assertFalse; +import static org.junit.jupiter.api.Assertions.assertEquals; + import java.io.ByteArrayInputStream; +import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; @@ -22,6 +26,8 @@ import org.hl7.fhir.convertors.factory.VersionConvertorFactory_10_50; import org.hl7.fhir.convertors.factory.VersionConvertorFactory_14_50; import org.hl7.fhir.convertors.factory.VersionConvertorFactory_30_50; import org.hl7.fhir.convertors.factory.VersionConvertorFactory_40_50; +import org.hl7.fhir.r5.model.OperationOutcome; +import org.hl7.fhir.r5.model.OperationOutcome.OperationOutcomeIssueComponent; import org.hl7.fhir.exceptions.DefinitionException; import org.hl7.fhir.exceptions.FHIRException; import org.hl7.fhir.exceptions.FHIRFormatError; @@ -31,6 +37,7 @@ import org.hl7.fhir.r5.elementmodel.Element; import org.hl7.fhir.r5.elementmodel.Manager; import org.hl7.fhir.r5.elementmodel.Manager.FhirFormat; import org.hl7.fhir.r5.elementmodel.ObjectConverter; +import org.hl7.fhir.r5.formats.IParser.OutputStyle; import org.hl7.fhir.r5.formats.JsonParser; import org.hl7.fhir.r5.formats.XmlParser; import org.hl7.fhir.r5.model.Base; @@ -46,6 +53,7 @@ import org.hl7.fhir.r5.model.ValueSet; import org.hl7.fhir.r5.test.utils.TestingUtilities; import org.hl7.fhir.r5.utils.FHIRPathEngine; import org.hl7.fhir.r5.utils.FHIRPathEngine.IEvaluationContext; +import org.hl7.fhir.r5.utils.OperationOutcomeUtilities; import org.hl7.fhir.r5.utils.validation.IResourceValidator; import org.hl7.fhir.r5.utils.validation.IValidationPolicyAdvisor; import org.hl7.fhir.r5.utils.validation.constants.BestPracticeWarningLevel; @@ -56,17 +64,20 @@ import org.hl7.fhir.r5.utils.validation.IValidatorResourceFetcher; import org.hl7.fhir.r5.utils.validation.constants.ContainedReferenceValidationPolicy; import org.hl7.fhir.r5.utils.validation.constants.ReferenceValidationPolicy; import org.hl7.fhir.utilities.SimpleHTTPClient.HTTPResult; +import org.hl7.fhir.utilities.json.JsonTrackingParser; import org.hl7.fhir.utilities.json.JsonUtilities; import org.hl7.fhir.utilities.npm.NpmPackage; import org.hl7.fhir.utilities.validation.ValidationMessage; import org.hl7.fhir.utilities.validation.ValidationMessage.IssueSeverity; import org.hl7.fhir.validation.IgLoader; import org.hl7.fhir.validation.ValidationEngine; +import org.hl7.fhir.validation.cli.model.HtmlInMarkdownCheck; import org.hl7.fhir.validation.cli.services.StandAloneValidatorFetcher; import org.hl7.fhir.validation.instance.InstanceValidator; import org.junit.AfterClass; import org.junit.Assert; import org.junit.Test; +import org.junit.jupiter.api.Assertions; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameters; @@ -82,6 +93,7 @@ import com.google.gson.JsonObject; public class ValidationTests implements IEvaluationContext, IValidatorResourceFetcher, IValidationPolicyAdvisor { public final static boolean PRINT_OUTPUT_TO_CONSOLE = true; + private static final boolean BUILD_NEW = true; @Parameters(name = "{index}: id {0}") public static Iterable data() throws IOException { @@ -254,7 +266,9 @@ public class ValidationTests implements IEvaluationContext, IValidatorResourceFe if (content.has("security-checks")) { val.setSecurityChecks(content.get("security-checks").getAsBoolean()); } - + if (content.has("noHtmlInMarkdown")) { + val.setHtmlInMarkdownCheck(HtmlInMarkdownCheck.ERROR); + } if (content.has("logical")==false) { val.setAssumeValidRestReferences(content.has("assumeValidRestReferences") ? content.get("assumeValidRestReferences").getAsBoolean() : false); System.out.println(String.format("Start Validating (%d to set up)", (System.nanoTime() - setup) / 1000000)); @@ -333,6 +347,9 @@ public class ValidationTests implements IEvaluationContext, IValidatorResourceFe checkOutcomes(errorsLogical, logical, "logical", name); } logger.verifyHasNoRequests(); + if (BUILD_NEW) { + JsonTrackingParser.write(manifest, new File(Utilities.path("[tmp]", "validator", "manifest.new.json"))); + } } @@ -400,57 +417,123 @@ public class ValidationTests implements IEvaluationContext, IValidatorResourceFe } } - private void checkOutcomes(List errors, JsonObject focus, String profile, String name) { + private void checkOutcomes(List errors, JsonObject focus, String profile, String name) throws IOException { JsonObject java = focus.getAsJsonObject("java"); - int ec = 0; - int wc = 0; - int hc = 0; - List errLocs = new ArrayList<>(); - for (ValidationMessage vm : errors) { - if (vm.getLevel() == IssueSeverity.FATAL || vm.getLevel() == IssueSeverity.ERROR) { - ec++; - if (PRINT_OUTPUT_TO_CONSOLE) { - System.out.println(vm.getDisplay()); - } - errLocs.add(vm.getLocation()); - } - if (vm.getLevel() == IssueSeverity.WARNING) { - wc++; - if (PRINT_OUTPUT_TO_CONSOLE) { - System.out.println(vm.getDisplay()); - } - } - if (vm.getLevel() == IssueSeverity.INFORMATION) { - hc++; - if (PRINT_OUTPUT_TO_CONSOLE) { - System.out.println(vm.getDisplay()); - } + OperationOutcome goal = (OperationOutcome) new JsonParser().parse(java.getAsJsonObject("outcome")); + OperationOutcome actual = OperationOutcomeUtilities.createOutcomeSimple(errors); + actual.setText(null); + String json = new JsonParser().setOutputStyle(OutputStyle.PRETTY).composeString(actual); + + List fails = new ArrayList<>(); + + Map map = new HashMap<>(); + for (OperationOutcomeIssueComponent issGoal : goal.getIssue()) { + OperationOutcomeIssueComponent issActual = findMatchingIssue(actual, issGoal); + if (issActual == null) { + fails.add("Expected Issue not found: "+issGoal.toString()); + } else { + map.put(issActual, issGoal); } } - if (!TestingUtilities.getSharedWorkerContext(version).isNoTerminologyServer() || !focus.has("tx-dependent")) { - Assert.assertEquals("Test " + name + (profile == null ? "" : " profile: "+ profile) + ": Expected " + Integer.toString(java.get("errorCount").getAsInt()) + " errors, but found " + Integer.toString(ec) + ".", java.get("errorCount").getAsInt(), ec); + for (OperationOutcomeIssueComponent issActual : actual.getIssue()) { + if (PRINT_OUTPUT_TO_CONSOLE) { + System.out.println(issActual.toString()); + } + OperationOutcomeIssueComponent issGoal = map.get(issActual); + if (issGoal == null) { + fails.add("Unexpected Issue found: "+issActual.toString()); + } + } + if (goal.getIssue().size() != actual.getIssue().size() && fails.isEmpty()) { + fails.add("Issue count mismatch (check for duplicate error messages)"); + } + + if (fails.size() > 0) { + for (String s : fails) { + System.out.println(s); + } + System.out.println(""); + System.out.println("========================================================"); + System.out.println(""); + System.out.println(json); + System.out.println(""); + System.out.println("========================================================"); + System.out.println(""); + Assertions.fail(fails.toString()); + } +// int ec = 0; +// int wc = 0; +// int hc = 0; +// List errLocs = new ArrayList<>(); +// for (ValidationMessage vm : errors) { +// if (vm.getLevel() == IssueSeverity.FATAL || vm.getLevel() == IssueSeverity.ERROR) { +// ec++; +// if (PRINT_OUTPUT_TO_CONSOLE) { +// System.out.println(vm.getDisplay()); +// } +// errLocs.add(vm.getLocation()); +// } +// if (vm.getLevel() == IssueSeverity.WARNING) { +// wc++; +// if (PRINT_OUTPUT_TO_CONSOLE) { +// System.out.println(vm.getDisplay()); +// } +// } +// if (vm.getLevel() == IssueSeverity.INFORMATION) { +// hc++; +// if (PRINT_OUTPUT_TO_CONSOLE) { +// System.out.println(vm.getDisplay()); +// } +// } +// } +// if (!TestingUtilities.getSharedWorkerContext(version).isNoTerminologyServer() || !focus.has("tx-dependent")) { +// Assert.assertEquals("Test " + name + (profile == null ? "" : " profile: "+ profile) + ": Expected " + Integer.toString(java.get("errorCount").getAsInt()) + " errors, but found " + Integer.toString(ec) + ".", java.get("errorCount").getAsInt(), ec); +// if (java.has("warningCount")) { +// Assert.assertEquals( "Test " + name + (profile == null ? "" : " profile: "+ profile) + ": Expected " + Integer.toString(java.get("warningCount").getAsInt()) + " warnings, but found " + Integer.toString(wc) + ".", java.get("warningCount").getAsInt(), wc); +// } +// if (java.has("infoCount")) { +// Assert.assertEquals( "Test " + name + (profile == null ? "" : " profile: "+ profile) + ": Expected " + Integer.toString(java.get("infoCount").getAsInt()) + " hints, but found " + Integer.toString(hc) + ".", java.get("infoCount").getAsInt(), hc); +// } +// } +// if (java.has("error-locations")) { +// JsonArray el = java.getAsJsonArray("error-locations"); +// Assert.assertEquals( "locations count is not correct", errLocs.size(), el.size()); +// for (int i = 0; i < errLocs.size(); i++) { +// Assert.assertEquals("Location should be " + el.get(i).getAsString() + ", but was " + errLocs.get(i), errLocs.get(i), el.get(i).getAsString()); +// } +// } + if (BUILD_NEW) { + if (java.has("output")) { + java.remove("output"); + } + if (java.has("error-locations")) { + java.remove("error-locations"); + } if (java.has("warningCount")) { - Assert.assertEquals( "Test " + name + (profile == null ? "" : " profile: "+ profile) + ": Expected " + Integer.toString(java.get("warningCount").getAsInt()) + " warnings, but found " + Integer.toString(wc) + ".", java.get("warningCount").getAsInt(), wc); + java.remove("warningCount"); } if (java.has("infoCount")) { - Assert.assertEquals( "Test " + name + (profile == null ? "" : " profile: "+ profile) + ": Expected " + Integer.toString(java.get("infoCount").getAsInt()) + " hints, but found " + Integer.toString(hc) + ".", java.get("infoCount").getAsInt(), hc); + java.remove("infoCount"); + } + if (java.has("errorCount")) { + java.remove("errorCount"); + } + if (java.has("outcome")) { + java.remove("outcome"); + } + JsonObject oj = JsonTrackingParser.parse(json, null); + java.add("outcome", oj); + } + } + + private OperationOutcomeIssueComponent findMatchingIssue(OperationOutcome oo, OperationOutcomeIssueComponent iss) { + for (OperationOutcomeIssueComponent t : oo.getIssue()) { + if (t.getExpression().get(0).getValue().equals(iss.getExpression().get(0).getValue()) && t.getCode() == iss.getCode() && t.getSeverity() == iss.getSeverity() + && (t.hasDiagnostics() ? t.getDiagnostics().equals(iss.getDiagnostics()) : !iss.hasDiagnostics()) && t.getDetails().getText().trim().equals(iss.getDetails().getText().trim())) { + return t; } } - if (java.has("error-locations")) { - JsonArray el = java.getAsJsonArray("error-locations"); - Assert.assertEquals( "locations count is not correct", errLocs.size(), el.size()); - for (int i = 0; i < errLocs.size(); i++) { - Assert.assertEquals("Location should be " + el.get(i).getAsString() + ", but was " + errLocs.get(i), errLocs.get(i), el.get(i).getAsString()); - } - } - if (focus.has("output")) { - focus.remove("output"); - } - JsonArray vr = new JsonArray(); - java.add("output", vr); - for (ValidationMessage vm : errors) { - vr.add(vm.getDisplay()); - } + return null; } private org.hl7.fhir.r4.model.Parameters makeExpProfile() {