From f583649f8740d66c802bf93d6add9b27c2f9a332 Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Fri, 5 Jul 2019 20:23:58 +1000 Subject: [PATCH] fix generation of error locations using FHIRPath or XPath properly --- .../utils/OperationOutcomeUtilities.java | 3 ++- .../utils/OperationOutcomeUtilities.java | 6 ++++- .../r4/utils/OperationOutcomeUtilities.java | 6 ++++- .../r5/utils/OperationOutcomeUtilities.java | 8 +++++- .../src/main/resources/gen/gen.xml | 2 +- .../org/hl7/fhir/utilities/Utilities.java | 26 +++++++++++++++++++ .../validation/ValidationMessage.java | 2 +- .../hl7/fhir/r5/validation/BaseValidator.java | 4 +-- .../fhir/r5/validation/InstanceValidator.java | 19 +++++++++++--- .../org/hl7/fhir/r5/validation/Validator.java | 15 ++++++++++- 10 files changed, 78 insertions(+), 13 deletions(-) diff --git a/org.hl7.fhir.dstu2/src/main/java/org/hl7/fhir/dstu2/utils/OperationOutcomeUtilities.java b/org.hl7.fhir.dstu2/src/main/java/org/hl7/fhir/dstu2/utils/OperationOutcomeUtilities.java index 7ee30935c..170a6d6ab 100644 --- a/org.hl7.fhir.dstu2/src/main/java/org/hl7/fhir/dstu2/utils/OperationOutcomeUtilities.java +++ b/org.hl7.fhir.dstu2/src/main/java/org/hl7/fhir/dstu2/utils/OperationOutcomeUtilities.java @@ -27,6 +27,7 @@ import org.hl7.fhir.dstu2.model.OperationOutcome.IssueSeverity; import org.hl7.fhir.dstu2.model.OperationOutcome.IssueType; import org.hl7.fhir.dstu2.model.OperationOutcome.OperationOutcomeIssueComponent; import org.hl7.fhir.dstu2.model.StringType; +import org.hl7.fhir.utilities.Utilities; import org.hl7.fhir.utilities.validation.ValidationMessage; public class OperationOutcomeUtilities { @@ -37,7 +38,7 @@ public class OperationOutcomeUtilities { issue.setCode(convert(message.getType())); if (message.getLocation() != null) { StringType s = new StringType(); - s.setValue(message.getLocation()+(message.getLine()>= 0 && message.getCol() >= 0 ? " (line "+Integer.toString(message.getLine())+", col"+Integer.toString(message.getCol())+")" : "") ); + s.setValue(Utilities.fhirPathToXPath(message.getLocation())+(message.getLine()>= 0 && message.getCol() >= 0 ? " (line "+Integer.toString(message.getLine())+", col"+Integer.toString(message.getCol())+")" : "") ); issue.getLocation().add(s); } issue.setSeverity(convert(message.getLevel())); diff --git a/org.hl7.fhir.dstu3/src/main/java/org/hl7/fhir/dstu3/utils/OperationOutcomeUtilities.java b/org.hl7.fhir.dstu3/src/main/java/org/hl7/fhir/dstu3/utils/OperationOutcomeUtilities.java index 545326a44..b786c176f 100644 --- a/org.hl7.fhir.dstu3/src/main/java/org/hl7/fhir/dstu3/utils/OperationOutcomeUtilities.java +++ b/org.hl7.fhir.dstu3/src/main/java/org/hl7/fhir/dstu3/utils/OperationOutcomeUtilities.java @@ -27,6 +27,7 @@ import org.hl7.fhir.dstu3.model.OperationOutcome.IssueSeverity; import org.hl7.fhir.dstu3.model.OperationOutcome.IssueType; import org.hl7.fhir.dstu3.model.OperationOutcome.OperationOutcomeIssueComponent; import org.hl7.fhir.dstu3.model.StringType; +import org.hl7.fhir.utilities.Utilities; import org.hl7.fhir.utilities.validation.ValidationMessage; public class OperationOutcomeUtilities { @@ -36,8 +37,11 @@ public class OperationOutcomeUtilities { OperationOutcomeIssueComponent issue = new OperationOutcome.OperationOutcomeIssueComponent(); 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()); + // also, populate the XPath variant StringType s = new StringType(); - s.setValue(message.getLocation()+(message.getLine()>= 0 && message.getCol() >= 0 ? " (line "+Integer.toString(message.getLine())+", col"+Integer.toString(message.getCol())+")" : "") ); + s.setValue(Utilities.fhirPathToXPath(message.getLocation())+(message.getLine()>= 0 && message.getCol() >= 0 ? " (line "+Integer.toString(message.getLine())+", col"+Integer.toString(message.getCol())+")" : "") ); issue.getLocation().add(s); } issue.setSeverity(convert(message.getLevel())); diff --git a/org.hl7.fhir.r4/src/main/java/org/hl7/fhir/r4/utils/OperationOutcomeUtilities.java b/org.hl7.fhir.r4/src/main/java/org/hl7/fhir/r4/utils/OperationOutcomeUtilities.java index a43bb2423..12e15ca49 100644 --- a/org.hl7.fhir.r4/src/main/java/org/hl7/fhir/r4/utils/OperationOutcomeUtilities.java +++ b/org.hl7.fhir.r4/src/main/java/org/hl7/fhir/r4/utils/OperationOutcomeUtilities.java @@ -27,6 +27,7 @@ import org.hl7.fhir.r4.model.OperationOutcome.IssueSeverity; import org.hl7.fhir.r4.model.OperationOutcome.IssueType; import org.hl7.fhir.r4.model.OperationOutcome.OperationOutcomeIssueComponent; import org.hl7.fhir.r4.model.StringType; +import org.hl7.fhir.utilities.Utilities; import org.hl7.fhir.utilities.validation.ValidationMessage; public class OperationOutcomeUtilities { @@ -36,8 +37,11 @@ public class OperationOutcomeUtilities { OperationOutcomeIssueComponent issue = new OperationOutcome.OperationOutcomeIssueComponent(); 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()); + // also, populate the XPath variant StringType s = new StringType(); - s.setValue(message.getLocation()+(message.getLine()>= 0 && message.getCol() >= 0 ? " (line "+Integer.toString(message.getLine())+", col"+Integer.toString(message.getCol())+")" : "") ); + s.setValue(Utilities.fhirPathToXPath(message.getLocation())+(message.getLine()>= 0 && message.getCol() >= 0 ? " (line "+Integer.toString(message.getLine())+", col"+Integer.toString(message.getCol())+")" : "") ); issue.getLocation().add(s); } issue.setSeverity(convert(message.getLevel())); 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 ac4b491b0..8aa031214 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 @@ -28,6 +28,7 @@ import org.hl7.fhir.r5.model.OperationOutcome.IssueSeverity; import org.hl7.fhir.r5.model.OperationOutcome.IssueType; import org.hl7.fhir.r5.model.OperationOutcome.OperationOutcomeIssueComponent; import org.hl7.fhir.r5.model.StringType; +import org.hl7.fhir.utilities.Utilities; import org.hl7.fhir.utilities.validation.ValidationMessage; public class OperationOutcomeUtilities { @@ -36,11 +37,16 @@ public class OperationOutcomeUtilities { public static OperationOutcomeIssueComponent convertToIssue(ValidationMessage message, OperationOutcome op) { OperationOutcomeIssueComponent issue = new OperationOutcome.OperationOutcomeIssueComponent(); 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()); + // also, populate the XPath variant StringType s = new StringType(); - s.setValue(message.getLocation()+(message.getLine()>= 0 && message.getCol() >= 0 ? " (line "+Integer.toString(message.getLine())+", col"+Integer.toString(message.getCol())+")" : "") ); + s.setValue(Utilities.fhirPathToXPath(message.getLocation())+(message.getLine()>= 0 && message.getCol() >= 0 ? " (line "+Integer.toString(message.getLine())+", col"+Integer.toString(message.getCol())+")" : "") ); issue.getLocation().add(s); } + // pass through line/col if they're present if (message.getLine() != 0) issue.addExtension().setUrl(ToolingExtensions.EXT_ISSUE_LINE).setValue(new IntegerType(message.getLine())); if (message.getCol() != 0) diff --git a/org.hl7.fhir.r5/src/main/resources/gen/gen.xml b/org.hl7.fhir.r5/src/main/resources/gen/gen.xml index c2d185782..d52962a14 100644 --- a/org.hl7.fhir.r5/src/main/resources/gen/gen.xml +++ b/org.hl7.fhir.r5/src/main/resources/gen/gen.xml @@ -111,7 +111,7 @@ Roel

- authored: Jun 17, 2013 11:00:00 PM + authored: 18/06/2013 9:00:00 AM

author: diff --git a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/Utilities.java b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/Utilities.java index 46785ee5d..13d659a8d 100644 --- a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/Utilities.java +++ b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/Utilities.java @@ -1195,6 +1195,32 @@ public class Utilities { } + /** + * Only handles simple FHIRPath expressions of the type produced by the validator + * + * @param path + * @return + */ + public static String fhirPathToXPath(String path) { + String[] p = path.split("\\."); + CommaSeparatedStringBuilder b = new CommaSeparatedStringBuilder("."); + int i = 0; + while (i < p.length) { + String s = p[i]; + if (s.contains("[")) { + String si = s.substring(s.indexOf("[")+1, s.length()-1); + s = s.substring(0, s.indexOf("["))+"["+Integer.toString(Integer.parseInt(si)+1)+"]"; + } + if (i < p.length - 1 && p[i+1].startsWith(".ofType(")) { + i++; + s = s + capitalize(p[i].substring(8, p.length-1)); + } + b.append(s); + i++; + } + return b.toString(); + } + } diff --git a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/validation/ValidationMessage.java b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/validation/ValidationMessage.java index 280ab599a..7e1de8053 100644 --- a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/validation/ValidationMessage.java +++ b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/validation/ValidationMessage.java @@ -479,7 +479,7 @@ public class ValidationMessage implements Comparator, Compara private Source source; private int line; private int col; - private String location; + private String location; // fhirPath private String message; private IssueType type; private IssueSeverity level; diff --git a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/r5/validation/BaseValidator.java b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/r5/validation/BaseValidator.java index f353043c9..9dc56c767 100644 --- a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/r5/validation/BaseValidator.java +++ b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/r5/validation/BaseValidator.java @@ -331,8 +331,8 @@ public class BaseValidator { protected boolean warning(List errors, IssueType type, int line, int col, String path, boolean thePass, String msg, Object... theMessageArguments) { if (!thePass) { msg = formatMessage(msg, theMessageArguments); - IssueSeverity severity = IssueSeverity.WARNING; - addValidationMessage(errors, type, line, col, path, msg, severity); + IssueSeverity severity = IssueSeverity.WARNING; + addValidationMessage(errors, type, line, col, path, msg, severity); } return thePass; diff --git a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/r5/validation/InstanceValidator.java b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/r5/validation/InstanceValidator.java index c9bdf12d8..5d7f1761f 100644 --- a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/r5/validation/InstanceValidator.java +++ b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/r5/validation/InstanceValidator.java @@ -4416,7 +4416,7 @@ private boolean isAnswerRequirementFulfilled(QuestionnaireItemComponent qItem, L String nb = cursor == 0 ? "--" : parent.getChildren().get(cursor-1).getName(); String na = cursor >= parent.getChildren().size() - 1 ? "--" : parent.getChildren().get(cursor+1).getName(); if (name().equals(nb) || name().equals(na) ) { - return lastCount + 1; + return lastCount; } else return -1; } @@ -4447,10 +4447,21 @@ private boolean isAnswerRequirementFulfilled(QuestionnaireItemComponent qItem, L public String path() { int i = count(); String sfx = ""; - if (i > -1) { - sfx = "[" + Integer.toString(lastCount + 1) + "]"; + String n = name(); + String fn = ""; + if (element().getProperty().isChoice()) { + String en = element().getProperty().getName(); + en = en.substring(0, en.length()-3); + String t = n.substring(en.length()); + if (isPrimitiveType(Utilities.uncapitalize(t))) + t = Utilities.uncapitalize(t); + n = en; + fn = ".ofType("+t+")"; } - return basePath + "." + name() + sfx; + if (i > -1 || element().isList()) { + sfx = "[" + Integer.toString(lastCount) + "]"; + } + return basePath + "." + n + sfx+fn; } } diff --git a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/r5/validation/Validator.java b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/r5/validation/Validator.java index 3db881ea2..49330d9ce 100644 --- a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/r5/validation/Validator.java +++ b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/r5/validation/Validator.java @@ -72,6 +72,7 @@ import org.hl7.fhir.r5.model.Constants; import org.hl7.fhir.r5.model.DomainResource; import org.hl7.fhir.r5.model.FhirPublication; import org.hl7.fhir.r5.model.ImplementationGuide; +import org.hl7.fhir.r5.model.IntegerType; import org.hl7.fhir.r5.model.OperationOutcome; import org.hl7.fhir.r5.model.OperationOutcome.OperationOutcomeIssueComponent; import org.hl7.fhir.r5.model.Resource; @@ -527,7 +528,19 @@ public class Validator { } private static String getIssueSummary(OperationOutcomeIssueComponent issue) { - return " " + issue.getSeverity().getDisplay() + " @ " + issue.getLocation().get(0).asStringValue() + " : " + issue.getDetails().getText(); + String loc = null; + if (issue.hasExpression()) { + int line = ToolingExtensions.readIntegerExtension(issue, ToolingExtensions.EXT_ISSUE_LINE, -1); + int col = ToolingExtensions.readIntegerExtension(issue, ToolingExtensions.EXT_ISSUE_COL, -1); + loc = issue.getExpression().get(0).asStringValue() + (line >= 0 && col >= 0 ? " (line "+Integer.toString(line)+", col"+Integer.toString(col)+")" : ""); + } else if (issue.hasLocation()) { + loc = issue.getLocation().get(0).asStringValue(); + } else { + int line = ToolingExtensions.readIntegerExtension(issue, ToolingExtensions.EXT_ISSUE_LINE, -1); + int col = ToolingExtensions.readIntegerExtension(issue, ToolingExtensions.EXT_ISSUE_COL, -1); + loc = (line >= 0 && col >= 0 ? "line "+Integer.toString(line)+", col"+Integer.toString(col) : "??"); + } + return " " + issue.getSeverity().getDisplay() + " @ " + loc + " : " + issue.getDetails().getText(); }