fix up validation of FHIRPaths in differentials, and add warnings when collection status isn't right

This commit is contained in:
Grahame Grieve 2023-07-27 17:49:57 +10:00
parent 6afb151dff
commit 4eff9a3b68
6 changed files with 83 additions and 18 deletions

View File

@ -359,6 +359,10 @@ public class ExpressionNode {
public enum CollectionStatus {
SINGLETON, ORDERED, UNORDERED;
boolean isList() {
return this == ORDERED || this == UNORDERED;
}
}
//the expression will have one of either name or constant

View File

@ -285,7 +285,7 @@ public class TypeDetails {
public void update(TypeDetails source) {
for (ProfiledType pt : source.types)
addType(pt);
if (collectionStatus == null)
if (collectionStatus == null || collectionStatus == CollectionStatus.SINGLETON)
collectionStatus = source.collectionStatus;
else if (source.collectionStatus == CollectionStatus.UNORDERED)
collectionStatus = source.collectionStatus;
@ -516,5 +516,8 @@ public class TypeDetails {
public static TypeDetails empty() {
return new TypeDetails(CollectionStatus.SINGLETON);
}
public boolean isList() {
return collectionStatus != null && collectionStatus.isList();
}
}

View File

@ -281,6 +281,7 @@ public class FHIRPathEngine {
private boolean doNotEnforceAsSingletonRule;
private boolean doNotEnforceAsCaseSensitive;
private boolean allowDoubleQuotes;
private List<String> typeWarnings = new ArrayList<>();
// if the fhir path expressions are allowed to use constants beyond those defined in the specification
// the application can implement them by providing a constant resolver
@ -656,7 +657,8 @@ public class FHIRPathEngine {
* @throws PathEngineException
* @if the path is not valid
*/
public TypeDetails checkOnTypes(Object appContext, String resourceType, List<String> typeList, ExpressionNode expr) throws FHIRLexerException, PathEngineException, DefinitionException {
public TypeDetails checkOnTypes(Object appContext, String resourceType, List<String> typeList, ExpressionNode expr, List<String> warnings) throws FHIRLexerException, PathEngineException, DefinitionException {
typeWarnings.clear();
// if context is a path that refers to a type, do that conversion now
TypeDetails types = new TypeDetails(CollectionStatus.SINGLETON);
@ -689,7 +691,9 @@ public class FHIRPathEngine {
}
}
}
return executeType(new ExecutionTypeContext(appContext, resourceType, types, types), types, expr, null, true, false);
TypeDetails res = executeType(new ExecutionTypeContext(appContext, resourceType, types, types), types, expr, null, true, false);
warnings.addAll(typeWarnings);
return res;
}
/**
@ -2017,18 +2021,54 @@ public class FHIRPathEngine {
}
private void checkCardinalityForComparabilitySame(TypeDetails left, Operation operation, TypeDetails right, ExpressionNode expr) {
if (left.isList() && !right.isList()) {
typeWarnings.add(worker.formatMessage(I18nConstants.FHIRPATH_COLLECTION_STATUS_OPERATION_LEFT, expr.toString()));
} else if (!left.isList() && right.isList()) {
typeWarnings.add(worker.formatMessage(I18nConstants.FHIRPATH_COLLECTION_STATUS_OPERATION_RIGHT, expr.toString()));
}
}
private void checkCardinalityForSingle(TypeDetails left, Operation operation, TypeDetails right, ExpressionNode expr) {
if (left.isList()) {
typeWarnings.add(worker.formatMessage(I18nConstants.FHIRPATH_COLLECTION_STATUS_OPERATION_LEFT, expr.toString()));
}
if (right.isList()) {
typeWarnings.add(worker.formatMessage(I18nConstants.FHIRPATH_COLLECTION_STATUS_OPERATION_RIGHT, expr.toString()));
}
}
private TypeDetails operateTypes(TypeDetails left, Operation operation, TypeDetails right, ExpressionNode expr) {
switch (operation) {
case Equals: return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Boolean);
case Equivalent: return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Boolean);
case NotEquals: return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Boolean);
case NotEquivalent: return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Boolean);
case LessThan: return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Boolean);
case Greater: return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Boolean);
case LessOrEqual: return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Boolean);
case GreaterOrEqual: return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Boolean);
case Is: return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Boolean);
case Equals:
checkCardinalityForComparabilitySame(left, operation, right, expr);
return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Boolean);
case Equivalent:
checkCardinalityForComparabilitySame(left, operation, right, expr);
return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Boolean);
case NotEquals:
checkCardinalityForComparabilitySame(left, operation, right, expr);
return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Boolean);
case NotEquivalent:
checkCardinalityForComparabilitySame(left, operation, right, expr);
return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Boolean);
case LessThan:
checkCardinalityForSingle(left, operation, right, expr);
return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Boolean);
case Greater:
checkCardinalityForSingle(left, operation, right, expr);
return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Boolean);
case LessOrEqual:
checkCardinalityForSingle(left, operation, right, expr);
return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Boolean);
case GreaterOrEqual:
checkCardinalityForSingle(left, operation, right, expr);
return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Boolean);
case Is:
checkCardinalityForSingle(left, operation, right, expr);
return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Boolean);
case As:
checkCardinalityForSingle(left, operation, right, expr);
TypeDetails td = new TypeDetails(CollectionStatus.SINGLETON, right.getTypes());
if (td.typesHaveTargets()) {
td.addTargets(left.getTargets());
@ -2040,6 +2080,7 @@ public class FHIRPathEngine {
case Xor: return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Boolean);
case Implies : return new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_Boolean);
case Times:
checkCardinalityForSingle(left, operation, right, expr);
TypeDetails result = new TypeDetails(CollectionStatus.SINGLETON);
if (left.hasType(worker, "integer") && right.hasType(worker, "integer")) {
result.addType(TypeDetails.FP_Integer);
@ -2048,6 +2089,7 @@ public class FHIRPathEngine {
}
return result;
case DivideBy:
checkCardinalityForSingle(left, operation, right, expr);
result = new TypeDetails(CollectionStatus.SINGLETON);
if (left.hasType(worker, "integer") && right.hasType(worker, "integer")) {
result.addType(TypeDetails.FP_Decimal);
@ -2056,9 +2098,11 @@ public class FHIRPathEngine {
}
return result;
case Concatenate:
checkCardinalityForSingle(left, operation, right, expr);
result = new TypeDetails(CollectionStatus.SINGLETON, TypeDetails.FP_String);
return result;
case Plus:
checkCardinalityForSingle(left, operation, right, expr);
result = new TypeDetails(CollectionStatus.SINGLETON);
if (left.hasType(worker, "integer") && right.hasType(worker, "integer")) {
result.addType(TypeDetails.FP_Integer);
@ -2075,6 +2119,7 @@ public class FHIRPathEngine {
}
return result;
case Minus:
checkCardinalityForSingle(left, operation, right, expr);
result = new TypeDetails(CollectionStatus.SINGLETON);
if (left.hasType(worker, "integer") && right.hasType(worker, "integer")) {
result.addType(TypeDetails.FP_Integer);
@ -2092,6 +2137,7 @@ public class FHIRPathEngine {
return result;
case Div:
case Mod:
checkCardinalityForSingle(left, operation, right, expr);
result = new TypeDetails(CollectionStatus.SINGLETON);
if (left.hasType(worker, "integer") && right.hasType(worker, "integer")) {
result.addType(TypeDetails.FP_Integer);
@ -3222,7 +3268,7 @@ public class FHIRPathEngine {
if (atEntry && Character.isUpperCase(exp.getName().charAt(0)) && hashTail(type).equals(exp.getName())) { // special case for start up
return new TypeDetails(CollectionStatus.SINGLETON, type);
}
TypeDetails result = new TypeDetails(null);
TypeDetails result = new TypeDetails(focus.getCollectionStatus());
getChildTypesByName(type, exp.getName(), result, exp, focus, elementDependencies);
return result;
}
@ -3265,6 +3311,10 @@ public class FHIRPathEngine {
} while (changed);
paramTypes.clear();
paramTypes.add(base);
} else if (exp.getFunction() == Function.Where || exp.getFunction() == Function.Select || exp.getFunction() == Function.Exists ||
exp.getFunction() == Function.All || exp.getFunction() == Function.AllTrue || exp.getFunction() == Function.AnyTrue
|| exp.getFunction() == Function.AllFalse || exp.getFunction() == Function.AnyFalse) {
evaluateParameters(context, focus.toSingleton(), exp, elementDependencies, paramTypes, false);
} else {
evaluateParameters(context, focus, exp, elementDependencies, paramTypes, false);
}

View File

@ -943,6 +943,9 @@ public class I18nConstants {
public static final String LIQUID_VARIABLE_ALREADY_ASSIGNED = "LIQUID_VARIABLE_ALREADY_ASSIGNED";
public static final String LIQUID_VARIABLE_ILLEGAL = "LIQUID_VARIABLE_ILLEGAL";
public static final String TERMINOLOGY_TX_SYSTEM_NOT_USABLE = "TERMINOLOGY_TX_SYSTEM_NOT_USABLE";
public static final String ED_INVARIANT_DIFF_NO_SOURCE = "ED_INVARIANT_DIFF_NO_SOURCE";
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";

View File

@ -184,7 +184,7 @@ Terminology_TX_NoValid_8 = None of the codes provided are in the maximum value s
Terminology_TX_NoValid_9 = The code provided ({2}) could not be validated against the maximum value set {0}, (error = {1})
Terminology_TX_System_Invalid = Invalid System URI: {0}
Terminology_TX_System_NotKnown = Code System URI ''{0}'' is unknown so the code cannot be validated
TERMINOLOGY_TX_SYSTEM_NOT_USABLE = Code System with URI ''{0}'' has no content so the code cannot be validated
TERMINOLOGY_TX_SYSTEM_NOT_USABLE = The definition for the Code System with URI ''{0}'' doesn't provide any codes so the code cannot be validated
Terminology_TX_System_Relative = Coding.system must be an absolute reference, not a local reference
Terminology_TX_System_Unknown = Unknown Code System ''{0}''
Terminology_TX_System_ValueSet = Invalid System URI: {0} - cannot use a value set URI as a system
@ -990,13 +990,15 @@ LIQUID_SYNTAX_EXPECTING = Script {0}: Found ''{1}'' expecting ''{2}'' parsing cy
LIQUID_SYNTAX_UNTERMINATED = Script {0}: Found unterminated string parsing cycle
LIQUID_UNKNOWN_FLOW_STMT = Script {0}: Unknown flow control statement ''{1}''
LIQUID_UNKNOWN_NOEND = Script {0}: Found end of script looking for {1}
LIQUID_SYNTAX_INCLUDE, = Script {0}: Error reading include: {1}
LIQUID_SYNTAX_INCLUDE = Script {0}: Error reading include: {1}
LIQUID_SYNTAX_LOOP = Script {0}: Error reading loop: {1}
LIQUID_SYNTAX_NOTERM = Script {0}: Unterminated Liquid statement {1}
LIQUID_UNKNOWN_NOTERM = Script {0}: Unterminated Liquid statement {1}
LIQUID_SYNTAX_COLON = Exception evaluating {0}: limit is not followed by '':''
LIQUID_SYNTAX_NUMBER = Exception evaluating {0}: limit is not followed by a number
LIQUID_SYNTAX_UNEXPECTED = Exception evaluating {0}: unexpected content at {1}
LIQUID_VARIABLE_ALREADY_ASSIGNED = "Liquid Exception: The variable ''{0}'' already has an assigned value
LIQUID_VARIABLE_ILLEGAL = "Liquid Exception: The variable name ''{0}'' cannot be used
LIQUID_VARIABLE_ALREADY_ASSIGNED = Liquid Exception: The variable ''{0}'' already has an assigned value
LIQUID_VARIABLE_ILLEGAL = Liquid Exception: The variable name ''{0}'' cannot be used
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_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

View File

@ -829,3 +829,6 @@ NO_VALID_DISPLAY_FOUND_other =
PRIMITIVE_VALUE_ALTERNATIVES_MESSAGE_one =
PRIMITIVE_VALUE_ALTERNATIVES_MESSAGE_many =
PRIMITIVE_VALUE_ALTERNATIVES_MESSAGE_other =
UNICODE_XML_BAD_CHARS_one =
UNICODE_XML_BAD_CHARS_many =
UNICODE_XML_BAD_CHARS_other =