Merge pull request #1369 from hapifhir/2023-07-gg-more_fhirpath_checking

2023 07 gg more fhirpath checking
This commit is contained in:
Grahame Grieve 2023-08-01 04:58:07 +10:00 committed by GitHub
commit ac6933bfc2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 80 additions and 14 deletions

View File

@ -42,6 +42,7 @@ import org.hl7.fhir.exceptions.DefinitionException;
import org.hl7.fhir.r5.context.IWorkerContext; import org.hl7.fhir.r5.context.IWorkerContext;
import org.hl7.fhir.r5.model.ElementDefinition.ElementDefinitionBindingComponent; import org.hl7.fhir.r5.model.ElementDefinition.ElementDefinitionBindingComponent;
import org.hl7.fhir.r5.model.ExpressionNode.CollectionStatus; import org.hl7.fhir.r5.model.ExpressionNode.CollectionStatus;
import org.hl7.fhir.utilities.CommaSeparatedStringBuilder;
import org.hl7.fhir.utilities.Utilities; import org.hl7.fhir.utilities.Utilities;
@ -116,6 +117,16 @@ public class TypeDetails {
return uri.startsWith(FP_NS); return uri.startsWith(FP_NS);
} }
public String describeMin() {
if (uri.startsWith(FP_NS)) {
return "System."+uri.substring(FP_NS.length());
}
if (uri.startsWith("http://hl7.org/fhir/StructureDefinition/")) {
return "FHIR."+uri.substring("http://hl7.org/fhir/StructureDefinition/".length());
}
return uri;
}
} }
private List<ProfiledType> types = new ArrayList<ProfiledType>(); private List<ProfiledType> types = new ArrayList<ProfiledType>();
@ -399,6 +410,12 @@ public class TypeDetails {
public String describe() { public String describe() {
return getTypes().toString(); return getTypes().toString();
} }
public String describeMin() {
CommaSeparatedStringBuilder b = new CommaSeparatedStringBuilder();
for (ProfiledType pt : types)
b.append(pt.describeMin());
return b.toString();
}
public String getType() { public String getType() {
for (ProfiledType pt : types) for (ProfiledType pt : types)
return pt.uri; return pt.uri;

View File

@ -67,6 +67,7 @@ import org.hl7.fhir.r5.model.Timing.EventTiming;
import org.hl7.fhir.r5.model.Timing.TimingRepeatComponent; import org.hl7.fhir.r5.model.Timing.TimingRepeatComponent;
import org.hl7.fhir.r5.model.Timing.UnitsOfTime; import org.hl7.fhir.r5.model.Timing.UnitsOfTime;
import org.hl7.fhir.r5.model.UriType; import org.hl7.fhir.r5.model.UriType;
import org.hl7.fhir.r5.model.UsageContext;
import org.hl7.fhir.r5.model.ValueSet; import org.hl7.fhir.r5.model.ValueSet;
import org.hl7.fhir.r5.model.ValueSet.ConceptReferenceComponent; import org.hl7.fhir.r5.model.ValueSet.ConceptReferenceComponent;
import org.hl7.fhir.r5.model.ValueSet.ConceptReferenceDesignationComponent; import org.hl7.fhir.r5.model.ValueSet.ConceptReferenceDesignationComponent;
@ -1517,6 +1518,12 @@ public class DataRenderer extends Renderer implements CodeResolver {
x.addText(!p.hasEnd() ? "(ongoing)" : displayDateTime(p.getEndElement())); x.addText(!p.hasEnd() ? "(ongoing)" : displayDateTime(p.getEndElement()));
} }
public void renderUsageContext(XhtmlNode x, UsageContext u) throws FHIRFormatError, DefinitionException, IOException {
renderCoding(x, u.getCode());
x.tx(": ");
render(x, u.getValue());
}
public void renderDataRequirement(XhtmlNode x, DataRequirement dr) throws FHIRFormatError, DefinitionException, IOException { public void renderDataRequirement(XhtmlNode x, DataRequirement dr) throws FHIRFormatError, DefinitionException, IOException {
XhtmlNode tbl = x.table("grid"); XhtmlNode tbl = x.table("grid");
XhtmlNode tr = tbl.tr(); XhtmlNode tr = tbl.tr();

View File

@ -487,6 +487,9 @@ public class ProfileDrivenRenderer extends ResourceRenderer {
} else if (e instanceof DataRequirement) { } else if (e instanceof DataRequirement) {
DataRequirement p = (DataRequirement) e; DataRequirement p = (DataRequirement) e;
renderDataRequirement(x, p); renderDataRequirement(x, p);
} else if (e instanceof UsageContext) {
UsageContext p = (UsageContext) e;
renderUsageContext(x, p);
} else if (e instanceof PrimitiveType) { } else if (e instanceof PrimitiveType) {
x.tx(((PrimitiveType) e).primitiveValue()); x.tx(((PrimitiveType) e).primitiveValue());
} else if (e instanceof ElementDefinition) { } else if (e instanceof ElementDefinition) {

View File

@ -3388,7 +3388,11 @@ public class FHIRPathEngine {
} }
case OfType : { case OfType : {
checkParamTypes(exp, exp.getFunction().toCode(), paramTypes, new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_String)); checkParamTypes(exp, exp.getFunction().toCode(), paramTypes, new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_String));
TypeDetails td = new TypeDetails(CollectionStatus.SINGLETON, exp.getParameters().get(0).getName()); String tn = exp.getParameters().get(0).getName();
if (typeCastIsImpossible(focus, tn)) {
typeWarnings.add(worker.formatMessage(I18nConstants.FHIRPATH_OFTYPE_IMPOSSIBLE, focus.describeMin(), tn, exp.toString()));
}
TypeDetails td = new TypeDetails(CollectionStatus.SINGLETON, tn);
if (td.typesHaveTargets()) { if (td.typesHaveTargets()) {
td.addTargets(focus.getTargets()); td.addTargets(focus.getTargets());
} }
@ -3707,6 +3711,10 @@ public class FHIRPathEngine {
throw new Error("not Implemented yet"); throw new Error("not Implemented yet");
} }
private boolean typeCastIsImpossible(TypeDetails focus, String tn) {
return !focus.hasType(tn);
}
private boolean isExpressionParameter(ExpressionNode exp, int i) { private boolean isExpressionParameter(ExpressionNode exp, int i) {
switch (i) { switch (i) {
case 0: case 0:

View File

@ -12,7 +12,7 @@ public interface IValidatorResourceFetcher {
Element fetch(IResourceValidator validator, Object appContext, String url) throws FHIRException, IOException; Element fetch(IResourceValidator validator, Object appContext, String url) throws FHIRException, IOException;
boolean resolveURL(IResourceValidator validator, Object appContext, String path, String url, String type) throws IOException, FHIRException; boolean resolveURL(IResourceValidator validator, Object appContext, String path, String url, String type, boolean canonical) throws IOException, FHIRException;
byte[] fetchRaw(IResourceValidator validator, String url) throws IOException; // for attachment checking byte[] fetchRaw(IResourceValidator validator, String url) throws IOException; // for attachment checking

View File

@ -947,6 +947,8 @@ public class I18nConstants {
public static final String FHIRPATH_COLLECTION_STATUS_OPERATION_LEFT = "FHIRPATH_COLLECTION_STATUS_OPERATION_LEFT"; public static final String FHIRPATH_COLLECTION_STATUS_OPERATION_LEFT = "FHIRPATH_COLLECTION_STATUS_OPERATION_LEFT";
public static final String FHIRPATH_COLLECTION_STATUS_OPERATION_RIGHT = "FHIRPATH_COLLECTION_STATUS_OPERATION_RIGHT"; public static final String FHIRPATH_COLLECTION_STATUS_OPERATION_RIGHT = "FHIRPATH_COLLECTION_STATUS_OPERATION_RIGHT";
public static final String ED_INVARIANT_KEY_ALREADY_USED = "ED_INVARIANT_KEY_ALREADY_USED"; public static final String ED_INVARIANT_KEY_ALREADY_USED = "ED_INVARIANT_KEY_ALREADY_USED";
public static final String FHIRPATH_OFTYPE_IMPOSSIBLE = "FHIRPATH_OFTYPE_IMPOSSIBLE";
public static final String ED_SEARCH_EXPRESSION_ERROR = "ED_SEARCH_EXPRESSION_ERROR";

View File

@ -1003,3 +1003,5 @@ LIQUID_VARIABLE_ILLEGAL = Liquid Exception: The variable name ''{0}'' cannot be
ED_INVARIANT_DIFF_NO_SOURCE = The invariant {0} defined in the differential must have no source, or the source must be the same as the profile ED_INVARIANT_DIFF_NO_SOURCE = The invariant {0} defined in the differential must have no source, or the source must be the same as the profile
FHIRPATH_COLLECTION_STATUS_OPERATION_LEFT = The left side is inherently a collection, and so the expression ''{0}'' may fail or return false if there is more than one item in the content being evaluated FHIRPATH_COLLECTION_STATUS_OPERATION_LEFT = The left side is inherently a collection, and so the expression ''{0}'' may fail or return false if there is more than one item in the content being evaluated
FHIRPATH_COLLECTION_STATUS_OPERATION_RIGHT = The right side is inherently a collection, and so this expression ''{0}'' may fail or return false if there is more than one item in the content being evaluated FHIRPATH_COLLECTION_STATUS_OPERATION_RIGHT = The right side is inherently a collection, and so this expression ''{0}'' may fail or return false if there is more than one item in the content being evaluated
FHIRPATH_OFTYPE_IMPOSSIBLE = The type specified in ofType is {1} which is not a possible candidate for the existing types ({0}) in the expression {2}. Check the paths and types to be sure this is what is intended
ED_SEARCH_EXPRESSION_ERROR = Error in search expression ''{0}'': {1}

View File

@ -1112,7 +1112,7 @@ public class ValidationEngine implements IValidatorResourceFetcher, IValidationP
} }
@Override @Override
public boolean resolveURL(IResourceValidator validator, Object appContext, String path, String url, String type) throws FHIRException { public boolean resolveURL(IResourceValidator validator, Object appContext, String path, String url, String type, boolean canonical) throws FHIRException {
// some of this logic might take a while, and it's not going to change once loaded // some of this logic might take a while, and it's not going to change once loaded
if (resolvedUrls .containsKey(type+"|"+url)) { if (resolvedUrls .containsKey(type+"|"+url)) {
return resolvedUrls.get(type+"|"+url); return resolvedUrls.get(type+"|"+url);
@ -1154,7 +1154,7 @@ public class ValidationEngine implements IValidatorResourceFetcher, IValidationP
} }
if (fetcher != null) { if (fetcher != null) {
try { try {
boolean ok = fetcher.resolveURL(validator, appContext, path, url, type); boolean ok = fetcher.resolveURL(validator, appContext, path, url, type, canonical);
resolvedUrls.put(type+"|"+url, ok); resolvedUrls.put(type+"|"+url, ok);
return ok; return ok;
} catch (Exception e) { } catch (Exception e) {

View File

@ -78,7 +78,7 @@ public class StandAloneValidatorFetcher implements IValidatorResourceFetcher, IV
} }
@Override @Override
public boolean resolveURL(IResourceValidator validator, Object appContext, String path, String url, String type) throws IOException, FHIRException { public boolean resolveURL(IResourceValidator validator, Object appContext, String path, String url, String type, boolean canonical) throws IOException, FHIRException {
if (!Utilities.isAbsoluteUrl(url)) { if (!Utilities.isAbsoluteUrl(url)) {
return false; return false;
} }
@ -283,7 +283,7 @@ public class StandAloneValidatorFetcher implements IValidatorResourceFetcher, IV
@Override @Override
public void findResource(Object validator, String url) { public void findResource(Object validator, String url) {
try { try {
resolveURL((IResourceValidator) validator, null, null, url, null); resolveURL((IResourceValidator) validator, null, null, url, null, false);
} catch (Exception e) { } catch (Exception e) {
} }
} }

View File

@ -2766,7 +2766,7 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat
found = isDefinitionURL(url) || (allowExamples && (url.contains("example.org") || url.contains("acme.com")) || url.contains("acme.org")) /* || (url.startsWith("http://hl7.org/fhir/tools")) */ || found = isDefinitionURL(url) || (allowExamples && (url.contains("example.org") || url.contains("acme.com")) || url.contains("acme.org")) /* || (url.startsWith("http://hl7.org/fhir/tools")) */ ||
SpecialExtensions.isKnownExtension(url) || isXverUrl(url); SpecialExtensions.isKnownExtension(url) || isXverUrl(url);
if (!found) { if (!found) {
found = fetcher.resolveURL(this, hostContext, path, url, type); found = fetcher.resolveURL(this, hostContext, path, url, type, type.equals("canonical"));
} }
} catch (IOException e1) { } catch (IOException e1) {
found = false; found = false;

View File

@ -46,9 +46,16 @@ public class SearchParameterValidator extends BaseValidator {
public boolean validateSearchParameter(List<ValidationMessage> errors, Element cs, NodeStack stack) { public boolean validateSearchParameter(List<ValidationMessage> errors, Element cs, NodeStack stack) {
boolean ok = true; boolean ok = true;
String url = cs.getNamedChildValue("url"); // String url = cs.getNamedChildValue("url");
String master = cs.getNamedChildValue("derivedFrom");
if (cs.hasChild("expression")) {
List<String> bases = new ArrayList<>();
for (Element b : cs.getChildrenByName("base")) {
bases.add(b.primitiveValue());
}
ok = checkExpression(errors, stack.push(cs.getNamedChild("expression"), -1, null, null), cs.getNamedChildValue("expression"), bases) && ok;
}
String master = cs.getNamedChildValue("derivedFrom");
if (!Utilities.noString(master)) { if (!Utilities.noString(master)) {
SearchParameter sp = context.fetchResource(SearchParameter.class, master); SearchParameter sp = context.fetchResource(SearchParameter.class, master);
if (warning(errors, NO_RULE_DATE, IssueType.BUSINESSRULE,stack.getLiteralPath(), sp != null, I18nConstants.SEARCHPARAMETER_NOTFOUND, master)) { if (warning(errors, NO_RULE_DATE, IssueType.BUSINESSRULE,stack.getLiteralPath(), sp != null, I18nConstants.SEARCHPARAMETER_NOTFOUND, master)) {
@ -67,13 +74,33 @@ public class SearchParameterValidator extends BaseValidator {
String expOther = canonicalise(sp.getExpression(), bases); String expOther = canonicalise(sp.getExpression(), bases);
warning(errors, NO_RULE_DATE, IssueType.BUSINESSRULE,stack.getLiteralPath(), expThis.equals(expOther), I18nConstants.SEARCHPARAMETER_EXP_WRONG, master, sp.getExpression(), cs.getNamedChildValue("expression")); warning(errors, NO_RULE_DATE, IssueType.BUSINESSRULE,stack.getLiteralPath(), expThis.equals(expOther), I18nConstants.SEARCHPARAMETER_EXP_WRONG, master, sp.getExpression(), cs.getNamedChildValue("expression"));
} }
// todo: check compositions // todo: check compositions
} }
} }
return ok; return ok;
} }
private boolean checkExpression(List<ValidationMessage> errors, NodeStack stack, String expression, List<String> bases) {
boolean ok = true;
try {
List<String> warnings = new ArrayList<>();
fpe.checkOnTypes(null, null, bases, fpe.parse(expression), warnings);
for (String s : warnings) {
warning(errors, "2023-07-27", IssueType.BUSINESSRULE, stack, false, s);
}
} catch (Exception e) {
if (debug) {
e.printStackTrace();
}
ok = rule(errors, "2023-06-19", IssueType.INVALID, stack, false, I18nConstants.ED_SEARCH_EXPRESSION_ERROR, expression, e.getMessage()) && ok;
}
return ok;
}
private String canonicalise(String path, List<String> bases) { private String canonicalise(String path, List<String> bases) {
ExpressionNode exp = fpe.parse(path); ExpressionNode exp = fpe.parse(path);
List<ExpressionNode> pass = new ArrayList<>(); List<ExpressionNode> pass = new ArrayList<>();
while (exp != null) { while (exp != null) {

View File

@ -433,7 +433,7 @@ public class R4R5MapTester implements IValidatorResourceFetcher {
} }
@Override @Override
public boolean resolveURL(IResourceValidator validator, Object appContext, String path, String url, String type) public boolean resolveURL(IResourceValidator validator, Object appContext, String path, String url, String type, boolean canonical)
throws IOException, FHIRException { throws IOException, FHIRException {
return true; return true;
} }

View File

@ -732,7 +732,7 @@ public class ValidationTests implements IEvaluationContext, IValidatorResourceFe
return CodedContentValidationPolicy.VALUESET; return CodedContentValidationPolicy.VALUESET;
} }
@Override @Override
public boolean resolveURL(IResourceValidator validator, Object appContext, String path, String url, String type) throws IOException, FHIRException { public boolean resolveURL(IResourceValidator validator, Object appContext, String path, String url, String type, boolean canonical) throws IOException, FHIRException {
return !url.contains("example.org") && !url.startsWith("http://hl7.org/fhir/invalid"); return !url.contains("example.org") && !url.startsWith("http://hl7.org/fhir/invalid");
} }