Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

FHIRPathSpecTest update and related changes #2749

Merged
merged 2 commits into from
Sep 15, 2021
Merged

FHIRPathSpecTest update and related changes #2749

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
66eda23
Update the FHIRPath tests
lmsurpre Sep 8, 2021
a303e9d
Updated method name per PR comments
JohnTimm Sep 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathAbstractTemporalValue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,11 @@ public boolean isComparableTo(FHIRPathNode other) {
FHIRPathTemporalValue temporalValue = (other instanceof FHIRPathTemporalValue) ?
(FHIRPathTemporalValue) other : (FHIRPathTemporalValue) other.getValue();

if ((temporalAccessor instanceof LocalTime && !(temporalValue.temporalAccessor() instanceof LocalTime))
|| (!(temporalAccessor instanceof LocalTime) && temporalValue.temporalAccessor() instanceof LocalTime)) {
return false;
}

int startIndex = 0;

if (temporalAccessor instanceof LocalTime && temporalValue.temporalAccessor() instanceof LocalTime) {
Expand Down
26 changes: 16 additions & 10 deletions fhir-path/src/main/java/com/ibm/fhir/path/evaluator/FHIRPathEvaluator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,17 @@ private Collection<FHIRPathNode> exists(List<ExpressionContext> arguments) {
if (arguments.isEmpty()) {
return !getCurrentContext().isEmpty() ? SINGLETON_TRUE : SINGLETON_FALSE;
}
return evaluatesToTrue(visit(arguments.get(0))) ? SINGLETON_TRUE : SINGLETON_FALSE;

for (FHIRPathNode node : getCurrentContext()) {
pushContext(singleton(node));
if (evaluatesToTrue(visit(arguments.get(0)))) {
popContext();
return SINGLETON_TRUE;
}
popContext();
}

return SINGLETON_FALSE;
}

private Collection<FHIRPathNode> getCurrentContext() {
Expand Down Expand Up @@ -873,7 +883,9 @@ public Collection<FHIRPathNode> visitEqualityExpression(FHIRPathParser.EqualityE
return afterEvaluation(ctx, SINGLETON_FALSE);
}

if (!validateEqualityOperands(left, right)) {
// for quantity operands: Attempting to operate on quantities with invalid units will result in empty ({ }).
// for temporal operands: If one input has a value for the precision and the other does not, the comparison stops and the result is empty ({ })
if (!isEqualityOperationValid(left, right)) {
Copy link
Collaborator

@JohnTimm JohnTimm Sep 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we are really checking the operands here. I think perhaps:

if (!equalityOperandsValid(left, right))

or

if (!equalityOperandsAreValid(left, right))

reads a little better.

return afterEvaluation(ctx, empty());
}

Expand All @@ -898,7 +910,7 @@ public Collection<FHIRPathNode> visitEqualityExpression(FHIRPathParser.EqualityE
return afterEvaluation(ctx, result);
}

private boolean validateEqualityOperands(Collection<FHIRPathNode> left, Collection<FHIRPathNode> right) {
private boolean isEqualityOperationValid(Collection<FHIRPathNode> left, Collection<FHIRPathNode> right) {
if (left.size() != right.size()) {
throw new IllegalArgumentException();
}
Expand All @@ -914,12 +926,6 @@ private boolean validateEqualityOperands(Collection<FHIRPathNode> left, Collecti
!getTemporalValue(leftNode).precision().equals(getTemporalValue(rightNode).precision())) {
return false;
}
// TODO: change to this when we update to a newer version of the test file
/*
if (hasTemporalValue(leftNode) && hasTemporalValue(rightNode) && !leftNode.isComparableTo(rightNode)) {
return false;
}
*/
}

return true;
Expand Down Expand Up @@ -1213,7 +1219,7 @@ public Collection<FHIRPathNode> visitQuantity(FHIRPathParser.QuantityContext ctx
beforeEvaluation(ctx);
String number = ctx.NUMBER().getText();
String text = ctx.unit().getText();
String unit = text.substring(1, text.length() - 1);
String unit = text.startsWith("'") ? text.substring(1, text.length() - 1) : text;
return afterEvaluation(ctx, singleton(FHIRPathQuantityValue.quantityValue(new BigDecimal(number), unit)));
}

Expand Down
16 changes: 11 additions & 5 deletions fhir-path/src/main/java/com/ibm/fhir/path/function/AllTrueFunction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,18 @@ public int getMinArity() {
public int getMaxArity() {
return 0;
}

@Override
public Collection<FHIRPathNode> apply(EvaluationContext evaluationContext, Collection<FHIRPathNode> context, List<Collection<FHIRPathNode>> arguments) {
return context.stream().allMatch(node -> node.isSystemValue() &&
node.asSystemValue().isBooleanValue() &&
node.asSystemValue().asBooleanValue().isTrue()) ?
SINGLETON_TRUE : SINGLETON_FALSE;
for (FHIRPathNode node : context) {
if (node.isSystemValue() && node.asSystemValue().isBooleanValue()) {
if (node.asSystemValue().asBooleanValue().isFalse()) {
return SINGLETON_FALSE;
}
} else {
throw new IllegalArgumentException("Invalid argument; expected boolean but found " + node.type());
}
}
return SINGLETON_TRUE;
}
}
19 changes: 4 additions & 15 deletions fhir-path/src/main/java/com/ibm/fhir/path/util/FHIRPathUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.antlr.v4.runtime.CommonTokenStream;
import org.antlr.v4.runtime.RecognitionException;
import org.antlr.v4.runtime.Recognizer;
import org.antlr.v4.runtime.Token;
import org.antlr.v4.runtime.misc.ParseCancellationException;

import com.ibm.fhir.model.patch.exception.FHIRPatchException;
Expand Down Expand Up @@ -127,14 +128,6 @@ public void syntaxError(Recognizer<?, ?> recognizer, Object offendingSymbol, int
private FHIRPathUtil() { }

public static ExpressionContext compile(String expr) {
int startIndex = -1;
for (int i = 0; i < expr.length(); i++) {
if (!Character.isWhitespace(expr.charAt(i))) {
startIndex = i;
break;
}
}

int stopIndex = -1;
for (int i = expr.length() - 1; i >= 0; i--) {
if (!Character.isWhitespace(expr.charAt(i))) {
Expand All @@ -143,7 +136,7 @@ public static ExpressionContext compile(String expr) {
}
}

if (startIndex == -1 || stopIndex == -1) {
if (stopIndex == -1) {
throw new IllegalArgumentException("Invalid FHIRPath expression: '" + expr + "'");
}

Expand All @@ -158,12 +151,8 @@ public static ExpressionContext compile(String expr) {
parser.addErrorListener(SYNTAX_ERROR_LISTENER);

ExpressionContext expressionContext = parser.expression();

if (expressionContext.getStart() == null || expressionContext.getStop() == null) {
throw new IllegalArgumentException("FHIRPath expression was parsed but start and/or stop token was null");
}

if (expressionContext.getStart().getStartIndex() != startIndex || expressionContext.getStop().getStopIndex() != stopIndex) {
List<Token> hiddenTokensToRight = tokens.getHiddenTokensToRight(expressionContext.getStop().getTokenIndex());
if (hiddenTokensToRight == null && expressionContext.getStop().getStopIndex() != stopIndex) {
throw new IllegalArgumentException("FHIRPath expression parsing error at: '" + expr.charAt(expressionContext.getStop().getStopIndex() + 1) + "'");
}

Expand Down
8 changes: 0 additions & 8 deletions fhir-path/src/test/java/com/ibm/fhir/path/test/BooleanEvaluationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

package com.ibm.fhir.path.test;

import static com.ibm.fhir.path.evaluator.FHIRPathEvaluator.SINGLETON_FALSE;
import static com.ibm.fhir.path.evaluator.FHIRPathEvaluator.SINGLETON_TRUE;
import static org.testng.Assert.assertEquals;

Expand All @@ -24,11 +23,4 @@ public void testBooleanEvaluation1() throws Exception {
Collection<FHIRPathNode> result = evaluator.evaluate("true and 'foo'");
assertEquals(result, SINGLETON_TRUE);
}

@Test
public void testBooleanEvaluation2() throws Exception {
FHIRPathEvaluator evaluator = FHIRPathEvaluator.evaluator();
Collection<FHIRPathNode> result = evaluator.evaluate("(true | 'foo').allTrue()");
assertEquals(result, SINGLETON_FALSE);
}
}
37 changes: 25 additions & 12 deletions fhir-path/src/test/java/com/ibm/fhir/path/test/FHIRPathSpecTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package com.ibm.fhir.path.test;

import static com.ibm.fhir.path.util.FHIRPathUtil.getBooleanValue;
import static com.ibm.fhir.path.util.FHIRPathUtil.getDate;
import static com.ibm.fhir.path.util.FHIRPathUtil.getDateTime;
import static com.ibm.fhir.path.util.FHIRPathUtil.getNumberValue;
import static com.ibm.fhir.path.util.FHIRPathUtil.getStringValue;
Expand Down Expand Up @@ -34,16 +35,18 @@

import com.ibm.fhir.model.format.Format;
import com.ibm.fhir.model.parser.FHIRParser;
import com.ibm.fhir.model.resource.Observation;
import com.ibm.fhir.model.resource.Patient;
import com.ibm.fhir.model.resource.Resource;
import com.ibm.fhir.model.test.TestUtil;
import com.ibm.fhir.model.util.XMLSupport;
import com.ibm.fhir.path.FHIRPathDateTimeValue;
import com.ibm.fhir.path.FHIRPathDateValue;
import com.ibm.fhir.path.FHIRPathNode;
import com.ibm.fhir.path.FHIRPathQuantityValue;
import com.ibm.fhir.path.FHIRPathTimeValue;
import com.ibm.fhir.path.evaluator.FHIRPathEvaluator;
import com.ibm.fhir.path.evaluator.FHIRPathEvaluator.EvaluationContext;
import com.ibm.fhir.path.exception.FHIRPathException;
import com.ibm.fhir.path.util.FHIRPathUtil;

/**
* Executes all the FHIRPath tests shipped with the FHIRPath specification
Expand Down Expand Up @@ -141,8 +144,15 @@ private void executeTest() throws Exception {
assertEquals(getStringValue(singleton(result)).toString(), expectedOutput.text);
break;
case "date":
if (FHIRPathUtil.hasDateTimeValue(singleton(result))) {
// FHIR says that FHIR dates are actually FHIRPath dateTimes: https://www.hl7.org/fhir/fhirpath.html
assertEquals(getDateTime(singleton(result)), FHIRPathDateValue.dateValue(expectedOutput.text.substring(1)).date());
} else {
assertEquals(getDate(singleton(result)), FHIRPathDateValue.dateValue(expectedOutput.text.substring(1)).date());
}
break;
case "dateTime":
assertEquals(getDateTime(singleton(result)).toString(), expectedOutput.text);
assertEquals(getDateTime(singleton(result)), FHIRPathDateTimeValue.dateTimeValue(expectedOutput.text.substring(1)).dateTime());
break;
case "decimal":
case "integer":
Expand All @@ -155,7 +165,7 @@ private void executeTest() throws Exception {
assertEquals(getStringValue(singleton(result)).toString(), expectedOutput.text);
break;
case "time":
assertEquals(getTime(singleton(result)).toString(), expectedOutput.text);
assertEquals(getTime(singleton(result)), FHIRPathTimeValue.timeValue(expectedOutput.text).time());
break;
}
}
Expand Down Expand Up @@ -298,17 +308,20 @@ private static Object[] createSingleTest() throws Exception {
}

public static void main(String[] args) throws Exception {
try (InputStream in = FHIRPathSpecTest.class.getClassLoader().getResourceAsStream("FHIRPath/input/patient-example.xml")) {
Patient patient = FHIRParser.parser(Format.XML).parse(in);
FHIRPathEvaluator evaluator = FHIRPathEvaluator.evaluator();
Collection<FHIRPathNode> result = evaluator.evaluate(patient, "Patient.active.type().name = 'boolean'");
System.out.println("result: " + result);
}
try (InputStream in = FHIRPathSpecTest.class.getClassLoader().getResourceAsStream("FHIRPath/input/observation-example.xml")) {
Observation observation = FHIRParser.parser(Format.XML).parse(in);
Resource resource = FHIRParser.parser(Format.XML).parse(in);
FHIRPathEvaluator evaluator = FHIRPathEvaluator.evaluator();
Collection<FHIRPathNode> result = evaluator.evaluate(observation, "(Observation.value as Period).unit");
Collection<FHIRPathNode> result = evaluator.evaluate(resource, "Observation.issued is instant");
System.out.println("result: " + result);
}
// try (InputStream in = FHIRPathSpecTest.class.getClassLoader().getResourceAsStream("FHIRPath/input/observation-example.xml")) {
// Observation observation = FHIRParser.parser(Format.XML).parse(in);
// FHIRPathEvaluator evaluator = FHIRPathEvaluator.evaluator();
// Collection<FHIRPathNode> result = evaluator.evaluate(observation, "(Observation.value as Period).unit");
// System.out.println("result: " + result);
// }



}
}
34 changes: 32 additions & 2 deletions fhir-path/src/test/java/com/ibm/fhir/path/test/FHIRPathUtilTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,35 @@
import com.ibm.fhir.path.util.FHIRPathUtil;

public class FHIRPathUtilTest {

@Test (expectedExceptions = IllegalArgumentException.class)
void testCompileWithFailure1() throws Exception {
FHIRPathUtil.compile("@T14:34:28Z.is(Time)");
}

@Test (expectedExceptions = IllegalArgumentException.class)
void testCompileWithFailure2() throws Exception {
FHIRPathUtil.compile("@T14:34:28Z.is(Time) //Still invalid");
}

@Test (expectedExceptions = IllegalArgumentException.class)
void testCompileWithFailure3() throws Exception {
FHIRPathUtil.compile("@T14:34:28 //Still invalid\n + @T14:34:28Z");
}

@Test
void testCompileWithSuccess() throws Exception {
FHIRPathUtil.compile("//Comment \n 2 + 2");
FHIRPathUtil.compile("2 + 2 \n //Comment \n + 2");
FHIRPathUtil.compile("2 + 2 //Comment + 4");

FHIRPathUtil.compile("/*Comment*/ \n 2 + 2");
FHIRPathUtil.compile("/*Comment \n */ 2 + 2");
FHIRPathUtil.compile("2 + 2 \n /*Comment*/ \n + 2");
FHIRPathUtil.compile("2 + 2 /* \nComment\n/ */ + 2");
FHIRPathUtil.compile("2 + 2 /*Comment + 4*/");
}

@Test
void testAdd() throws Exception {
HumanName name1 = HumanName.builder()
Expand All @@ -40,7 +69,7 @@ void testAdd() throws Exception {
fhirpathPatient = FHIRPathUtil.add(fhirpathPatient, "Patient", "deceased", Boolean.TRUE);
fhirpathPatient = FHIRPathUtil.add(fhirpathPatient, "Patient", "name", name1);
fhirpathPatient = FHIRPathUtil.add(fhirpathPatient, "Patient", "name", name2);

assertEquals(fhirpathPatient, builderPatient);
}

Expand Down Expand Up @@ -140,7 +169,7 @@ void testReplace() throws Exception {
.deceased(Boolean.TRUE)
.name(name1)
.build();

Patient builderPatient = patient.toBuilder()
.deceased(Boolean.FALSE)
.name(Collections.singleton(patient.getName().get(0).toBuilder()
Expand Down Expand Up @@ -175,4 +204,5 @@ void testMove() throws Exception {

assertEquals(fhirpathPatient, builderPatient);
}

}
Loading