diff --git a/fhirpath/src/main/java/au/csiro/pathling/fhirpath/collection/EmptyCollection.java b/fhirpath/src/main/java/au/csiro/pathling/fhirpath/collection/EmptyCollection.java index 587aab5390..49223a5c31 100644 --- a/fhirpath/src/main/java/au/csiro/pathling/fhirpath/collection/EmptyCollection.java +++ b/fhirpath/src/main/java/au/csiro/pathling/fhirpath/collection/EmptyCollection.java @@ -2,7 +2,7 @@ import au.csiro.pathling.fhirpath.FhirPathType; import au.csiro.pathling.fhirpath.column.ColumnRepresentation; -import au.csiro.pathling.fhirpath.column.EmptyRepresentation; +import au.csiro.pathling.fhirpath.column.DefaultRepresentation; import au.csiro.pathling.fhirpath.definition.NodeDefinition; import au.csiro.pathling.fhirpath.operator.Comparable; import jakarta.annotation.Nonnull; @@ -16,7 +16,7 @@ public class EmptyCollection extends Collection { private static final EmptyCollection INSTANCE = new EmptyCollection( - EmptyRepresentation.getInstance(), Optional.empty(), Optional.empty(), Optional.empty(), + DefaultRepresentation.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); protected EmptyCollection(@Nonnull final ColumnRepresentation column, diff --git a/fhirpath/src/main/java/au/csiro/pathling/fhirpath/collection/mixed/MixedCollection.java b/fhirpath/src/main/java/au/csiro/pathling/fhirpath/collection/mixed/MixedCollection.java index 107b8da3fd..93c7ab3124 100644 --- a/fhirpath/src/main/java/au/csiro/pathling/fhirpath/collection/mixed/MixedCollection.java +++ b/fhirpath/src/main/java/au/csiro/pathling/fhirpath/collection/mixed/MixedCollection.java @@ -1,7 +1,7 @@ package au.csiro.pathling.fhirpath.collection.mixed; import au.csiro.pathling.fhirpath.collection.Collection; -import au.csiro.pathling.fhirpath.column.EmptyRepresentation; +import au.csiro.pathling.fhirpath.column.DefaultRepresentation; import au.csiro.pathling.fhirpath.definition.ChoiceDefinition; import jakarta.annotation.Nonnull; import java.util.Optional; @@ -16,7 +16,7 @@ public abstract class MixedCollection extends Collection { protected MixedCollection() { - super(EmptyRepresentation.getInstance(), Optional.empty(), Optional.empty(), Optional.empty(), + super(DefaultRepresentation.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); } diff --git a/fhirpath/src/main/java/au/csiro/pathling/fhirpath/column/ColumnRepresentation.java b/fhirpath/src/main/java/au/csiro/pathling/fhirpath/column/ColumnRepresentation.java index 1729d4b376..ba26e4e402 100644 --- a/fhirpath/src/main/java/au/csiro/pathling/fhirpath/column/ColumnRepresentation.java +++ b/fhirpath/src/main/java/au/csiro/pathling/fhirpath/column/ColumnRepresentation.java @@ -54,7 +54,7 @@ * @author John Grimes */ public abstract class ColumnRepresentation { - + /** * Create a new {@link ColumnRepresentation} from the result of a function that takes two * {@link Column} operands and returns a single {@link Column} result. @@ -414,7 +414,7 @@ public ColumnRepresentation countDistinct() { * @return A new {@link ColumnRepresentation} that is the result of the check */ @Nonnull - public ColumnRepresentation empty() { + public ColumnRepresentation isEmpty() { return vectorize( c -> when(c.isNotNull(), size(c).equalTo(0)).otherwise(true), Column::isNull); diff --git a/fhirpath/src/main/java/au/csiro/pathling/fhirpath/column/DefaultRepresentation.java b/fhirpath/src/main/java/au/csiro/pathling/fhirpath/column/DefaultRepresentation.java index d4d5c51f1b..f81e48f68d 100644 --- a/fhirpath/src/main/java/au/csiro/pathling/fhirpath/column/DefaultRepresentation.java +++ b/fhirpath/src/main/java/au/csiro/pathling/fhirpath/column/DefaultRepresentation.java @@ -30,6 +30,7 @@ import lombok.Getter; import lombok.ToString; import org.apache.spark.sql.Column; +import org.apache.spark.sql.functions; import org.hl7.fhir.r4.model.Enumerations.FHIRDefinedType; /** @@ -44,7 +45,23 @@ @AllArgsConstructor public class DefaultRepresentation extends ColumnRepresentation { - Column value; + private static final DefaultRepresentation EMPTY_REPRESENTATION = new DefaultRepresentation( + functions.lit(null)); + + /** + * Gets the empty representation. + * + * @return A singleton instance of an empty representation + */ + @Nonnull + public static ColumnRepresentation empty() { + return EMPTY_REPRESENTATION; + } + + /** + * The column value represented by this object. + */ + private Column value; /** * Create a new {@link ColumnRepresentation} from a literal value. @@ -53,7 +70,7 @@ public class DefaultRepresentation extends ColumnRepresentation { * @return A new {@link ColumnRepresentation} representing the value */ @Nonnull - public static ColumnRepresentation literal(@Nonnull final Object value) { + public static DefaultRepresentation literal(@Nonnull final Object value) { if (value instanceof BigDecimal) { // If the literal is a BigDecimal, represent it as a DecimalRepresentation. return new DecimalRepresentation(lit(value)); @@ -120,5 +137,5 @@ protected Column traverseColumn(@Nonnull final String fieldName) { public ColumnRepresentation getField(@Nonnull final String fieldName) { return copyOf(value.getField(fieldName)); } - + } diff --git a/fhirpath/src/main/java/au/csiro/pathling/fhirpath/column/EmptyRepresentation.java b/fhirpath/src/main/java/au/csiro/pathling/fhirpath/column/EmptyRepresentation.java deleted file mode 100644 index 65885c0382..0000000000 --- a/fhirpath/src/main/java/au/csiro/pathling/fhirpath/column/EmptyRepresentation.java +++ /dev/null @@ -1,72 +0,0 @@ -package au.csiro.pathling.fhirpath.column; - -import static org.apache.spark.sql.functions.lit; - -import jakarta.annotation.Nonnull; -import java.util.Optional; -import java.util.function.Function; -import org.apache.spark.sql.Column; -import org.hl7.fhir.r4.model.Enumerations.FHIRDefinedType; - -/** - * Describes a representation of an empty collection. - * - * @author Piotr Szul - * @author John Grimes - */ -public class EmptyRepresentation extends ColumnRepresentation { - - static final ColumnRepresentation INSTANCE = new EmptyRepresentation(); - static final Column NULL_LITERAL = lit(null); - - /** - * @return A singleton instance of this class - */ - @Nonnull - public static ColumnRepresentation getInstance() { - return INSTANCE; - } - - @Override - public Column getValue() { - return NULL_LITERAL; - } - - @Override - protected ColumnRepresentation copyOf(@Nonnull final Column newValue) { - return this; - } - - @Nonnull - @Override - public ColumnRepresentation vectorize(@Nonnull final Function arrayExpression, - @Nonnull final Function singularExpression) { - return this; - } - - @Nonnull - @Override - public ColumnRepresentation flatten() { - return this; - } - - @Nonnull - @Override - public EmptyRepresentation traverse(@Nonnull final String fieldName) { - return this; - } - - @Nonnull - @Override - public EmptyRepresentation traverse(@Nonnull final String fieldName, - @Nonnull final Optional fhirType) { - return traverse(fieldName); - } - - @Nonnull - @Override - public EmptyRepresentation getField(@Nonnull final String fieldName) { - return this; - } - -} diff --git a/fhirpath/src/main/java/au/csiro/pathling/fhirpath/execution/NullEvaluator.java b/fhirpath/src/main/java/au/csiro/pathling/fhirpath/execution/NullEvaluator.java index b11f6623fb..430f605ea5 100644 --- a/fhirpath/src/main/java/au/csiro/pathling/fhirpath/execution/NullEvaluator.java +++ b/fhirpath/src/main/java/au/csiro/pathling/fhirpath/execution/NullEvaluator.java @@ -1,7 +1,7 @@ package au.csiro.pathling.fhirpath.execution; import au.csiro.pathling.fhirpath.collection.ResourceCollection; -import au.csiro.pathling.fhirpath.column.EmptyRepresentation; +import au.csiro.pathling.fhirpath.column.DefaultRepresentation; import au.csiro.pathling.fhirpath.function.registry.StaticFunctionRegistry; import ca.uhn.fhir.context.FhirContext; import jakarta.annotation.Nonnull; @@ -26,7 +26,7 @@ static class NullResourceResolver extends BaseResourceResolver { @Nonnull protected ResourceCollection createResource(@Nonnull final ResourceType resourceType) { return ResourceCollection.build( - new EmptyRepresentation(), + DefaultRepresentation.empty(), getFhirContext(), resourceType); } } diff --git a/fhirpath/src/main/java/au/csiro/pathling/fhirpath/function/provider/ExistenceFunctions.java b/fhirpath/src/main/java/au/csiro/pathling/fhirpath/function/provider/ExistenceFunctions.java index 990abe355a..9b40bd5238 100644 --- a/fhirpath/src/main/java/au/csiro/pathling/fhirpath/function/provider/ExistenceFunctions.java +++ b/fhirpath/src/main/java/au/csiro/pathling/fhirpath/function/provider/ExistenceFunctions.java @@ -60,7 +60,7 @@ public static BooleanCollection exists(@Nonnull final Collection input, @FhirPathFunction @Nonnull public static BooleanCollection empty(@Nonnull final Collection input) { - return BooleanCollection.build(input.getColumn().empty()); + return BooleanCollection.build(input.getColumn().isEmpty()); } /** diff --git a/fhirpath/src/main/java/au/csiro/pathling/fhirpath/function/provider/TerminologyFunctions.java b/fhirpath/src/main/java/au/csiro/pathling/fhirpath/function/provider/TerminologyFunctions.java index beefcc7dd3..b9847607bf 100644 --- a/fhirpath/src/main/java/au/csiro/pathling/fhirpath/function/provider/TerminologyFunctions.java +++ b/fhirpath/src/main/java/au/csiro/pathling/fhirpath/function/provider/TerminologyFunctions.java @@ -28,7 +28,6 @@ import au.csiro.pathling.fhirpath.collection.StringCollection; import au.csiro.pathling.fhirpath.column.ColumnRepresentation; import au.csiro.pathling.fhirpath.column.DefaultRepresentation; -import au.csiro.pathling.fhirpath.column.EmptyRepresentation; import au.csiro.pathling.fhirpath.definition.ElementDefinition; import au.csiro.pathling.fhirpath.function.FhirPathFunction; import au.csiro.pathling.sql.udf.PropertyUdf; @@ -71,7 +70,7 @@ public static StringCollection display(@Nonnull final CodingCollection input, .transformWithUdf("display", Optional.ofNullable(language) .map(StringCollection::getColumn) .map(ColumnRepresentation::singular) - .orElse(EmptyRepresentation.getInstance())) + .orElse(DefaultRepresentation.empty())) .removeNulls() ); } @@ -122,7 +121,7 @@ public static Collection property(@Nonnull final CodingCollection input, Optional.ofNullable(language) .map(StringCollection::getColumn) .map(ColumnRepresentation::singular) - .orElse(EmptyRepresentation.getInstance()) + .orElse(DefaultRepresentation.empty()) ).flatten().removeNulls(); return Collection.build(resultCtx, propertyType, @@ -160,11 +159,11 @@ public static StringCollection designation(@Nonnull final CodingCollection input Optional.ofNullable(use) .map(CodingCollection::getColumn) .map(ColumnRepresentation::singular) - .orElse(EmptyRepresentation.getInstance()), + .orElse(DefaultRepresentation.empty()), Optional.ofNullable(language) .map(StringCollection::getColumn) .map(ColumnRepresentation::singular) - .orElse(EmptyRepresentation.getInstance()) + .orElse(DefaultRepresentation.empty()) ) .flatten().removeNulls() ); @@ -279,7 +278,7 @@ public static CodingCollection translate(@Nonnull final Concepts input, .transform(c -> functions.split(c, ",")), Optional.ofNullable(target).map(StringCollection::getColumn) .map(ColumnRepresentation::singular) - .orElse(EmptyRepresentation.getInstance()) + .orElse(DefaultRepresentation.empty()) )); } diff --git a/fhirpath/src/main/java/au/csiro/pathling/view/UnnestingSelection.java b/fhirpath/src/main/java/au/csiro/pathling/view/UnnestingSelection.java index f8a0135650..4129420e49 100644 --- a/fhirpath/src/main/java/au/csiro/pathling/view/UnnestingSelection.java +++ b/fhirpath/src/main/java/au/csiro/pathling/view/UnnestingSelection.java @@ -25,7 +25,6 @@ import au.csiro.pathling.fhirpath.FhirPath; import au.csiro.pathling.fhirpath.collection.Collection; import au.csiro.pathling.fhirpath.column.DefaultRepresentation; -import au.csiro.pathling.fhirpath.column.EmptyRepresentation; import jakarta.annotation.Nonnull; import java.util.List; import java.util.stream.Collectors; @@ -71,7 +70,7 @@ public ProjectionResult evaluate(@Nonnull final ProjectionContext context) { // This is a way to evaluate the expression for the purpose of getting the types of the result. final ProjectionContext stubContext = context.withInputContext( - unnestingCollection.map(__ -> EmptyRepresentation.getInstance())); + unnestingCollection.map(__ -> DefaultRepresentation.empty())); final List stubResults = components.stream() .map(s -> s.evaluate(stubContext)) .toList(); diff --git a/fhirpath/src/test/java/au/csiro/pathling/fhirpath/column/DefaultRepresentationTest.java b/fhirpath/src/test/java/au/csiro/pathling/fhirpath/column/DefaultRepresentationTest.java index ebbf794dd8..62fc1acc30 100644 --- a/fhirpath/src/test/java/au/csiro/pathling/fhirpath/column/DefaultRepresentationTest.java +++ b/fhirpath/src/test/java/au/csiro/pathling/fhirpath/column/DefaultRepresentationTest.java @@ -176,11 +176,11 @@ void testNot() { @Test void testEmpty() { new ColumnAsserts() - .assertEquals(true, nullValue().empty()) - .assertEquals(false, valueOf(17).empty()) - .assertEquals(true, nullArray().empty()) - .assertEquals(true, emptyArray().empty()) - .assertEquals(false, arrayOf(1, 2, 3).empty()) + .assertEquals(true, nullValue().isEmpty()) + .assertEquals(false, valueOf(17).isEmpty()) + .assertEquals(true, nullArray().isEmpty()) + .assertEquals(true, emptyArray().isEmpty()) + .assertEquals(false, arrayOf(1, 2, 3).isEmpty()) .check(); } diff --git a/fhirpath/src/test/java/au/csiro/pathling/fhirpath/yaml/YamlFhirpathTest.java b/fhirpath/src/test/java/au/csiro/pathling/fhirpath/yaml/YamlFhirpathTest.java index 96aaf41161..6cc36a4997 100644 --- a/fhirpath/src/test/java/au/csiro/pathling/fhirpath/yaml/YamlFhirpathTest.java +++ b/fhirpath/src/test/java/au/csiro/pathling/fhirpath/yaml/YamlFhirpathTest.java @@ -1,5 +1,6 @@ package au.csiro.pathling.fhirpath.yaml; +import au.csiro.pathling.test.yaml.YamlConfig; import au.csiro.pathling.test.yaml.YamlSpec; import au.csiro.pathling.test.yaml.YamlSpecTestBase; import jakarta.annotation.Nonnull; @@ -9,6 +10,9 @@ @Slf4j @Tag("UnitTest") +@YamlConfig( + resourceBase = "fhirpath-ptl/resources" +) public class YamlFhirpathTest extends YamlSpecTestBase { @YamlSpec("fhirpath-ptl/cases/literals.yaml") diff --git a/fhirpath/src/test/java/au/csiro/pathling/fhirpath/yaml/YamlReferenceImplTest.java b/fhirpath/src/test/java/au/csiro/pathling/fhirpath/yaml/YamlReferenceImplTest.java index c8c37046b0..f29b1dceed 100644 --- a/fhirpath/src/test/java/au/csiro/pathling/fhirpath/yaml/YamlReferenceImplTest.java +++ b/fhirpath/src/test/java/au/csiro/pathling/fhirpath/yaml/YamlReferenceImplTest.java @@ -15,7 +15,10 @@ @Slf4j @Tag("YamlTest") -@YamlConfig("fhirpath-js/config.yaml") +@YamlConfig( + config = "fhirpath-js/config.yaml", + resourceBase = "fhirpath-js/resources" +) public class YamlReferenceImplTest extends YamlSpecCachedTestBase { diff --git a/fhirpath/src/test/java/au/csiro/pathling/test/yaml/YamlConfig.java b/fhirpath/src/test/java/au/csiro/pathling/test/yaml/YamlConfig.java index 973ed8ee55..cbfc84a666 100644 --- a/fhirpath/src/test/java/au/csiro/pathling/test/yaml/YamlConfig.java +++ b/fhirpath/src/test/java/au/csiro/pathling/test/yaml/YamlConfig.java @@ -6,5 +6,7 @@ @Retention(RetentionPolicy.RUNTIME) public @interface YamlConfig { - String value(); + String config() default ""; + + String resourceBase() default ""; } diff --git a/fhirpath/src/test/java/au/csiro/pathling/test/yaml/YamlSpecTestBase.java b/fhirpath/src/test/java/au/csiro/pathling/test/yaml/YamlSpecTestBase.java index e2dc2f6e41..3ff417ed1b 100644 --- a/fhirpath/src/test/java/au/csiro/pathling/test/yaml/YamlSpecTestBase.java +++ b/fhirpath/src/test/java/au/csiro/pathling/test/yaml/YamlSpecTestBase.java @@ -27,6 +27,7 @@ import ca.uhn.fhir.parser.IParser; import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; +import java.io.File; import java.math.BigDecimal; import java.math.RoundingMode; import java.util.List; @@ -342,8 +343,17 @@ public Stream provideArguments(ExtensionContext context) { final Optional testConfigPath = context.getTestClass() .flatMap(c -> Optional.ofNullable(c.getAnnotation(YamlConfig.class))) - .map(YamlConfig::value); + .map(YamlConfig::config) + .filter(s -> !s.isBlank()); + + final Optional resourceBase = context.getTestClass() + .flatMap(c -> Optional.ofNullable(c.getAnnotation(YamlConfig.class))) + .map(YamlConfig::resourceBase) + .filter(s -> !s.isBlank()); + testConfigPath.ifPresent(s -> log.info("Loading test config from: {}", s)); + resourceBase.ifPresent(s -> log.info("Resource base : {}", s)); + final TestConfig testConfig = testConfigPath .map(TestResources::getResourceAsString) .map(TestConfig::fromYaml) @@ -376,7 +386,7 @@ public Stream provideArguments(ExtensionContext context) { .map(ts -> StdRuntimeCase.of(ts, Optional.ofNullable(ts.getInputFile()) .map(f -> (Function) FhirResolverFactory.of( - getResourceAsString("fhirpath-js/resources/" + f))) + getResourceAsString(resourceBase.orElse("") + File.separator + f))) .orElse(defaultResolverFactory), excluder.apply(ts) )) diff --git a/fhirpath/src/test/resources/fhirpath-js/config.yaml b/fhirpath/src/test/resources/fhirpath-js/config.yaml index 680f80252c..74242ddefc 100644 --- a/fhirpath/src/test/resources/fhirpath-js/config.yaml +++ b/fhirpath/src/test/resources/fhirpath-js/config.yaml @@ -231,13 +231,6 @@ excludeSet: any: - "** null is not empty" - "** exists for null should return true" - - title: 'Incorrect exist() for empty collections' - id: '#2149' - type: bug - any: - - "** empty nothing" - - "** exists for undefined coll should return false" - - "** exists with criteria should work for not exists coll" - title: 'Incorrect not() for empty collections' id: '#2150' type: bug @@ -268,11 +261,6 @@ excludeSet: any: - "** anyFalse for empty coll is false" - "** anyFalse for non-exists coll is false" - - title: "Incorrect count() for empty collections" - id: '#2149' - type: bug - any: - - "** 0 if nothing" - title: Filtering and projection exclusions glob: "fhirpath-js/cases/5.2_filtering_and_projection.yaml" exclude: @@ -607,12 +595,6 @@ excludeSet: - "** expression with extension() for primitive type (using FHIR model data)" - "** value of extension of extension (using FHIR model data)" - "** id of extension of extension" - - title: "empty() should be true for empty collections" - id: '#2149' - type: bug - any: - - "** empty url" - - "** empty input collection" - title: Factory exclusions glob: 'fhirpath-js/cases/factory.yaml' exclude: @@ -655,12 +637,6 @@ excludeSet: type: feature any: - "Patient.name.exists(given)" - - title: "empty() and exist() should return true/false for empty collections" - id: '#2149' - type: bug - any: - - "Patient.ups.exists()" - - "Patient.ups.empty()" - title: "Unsupported function type()" type: feature any: diff --git a/fhirpath/src/test/resources/fhirpath-ptl/cases/existence_functions.yaml b/fhirpath/src/test/resources/fhirpath-ptl/cases/existence_functions.yaml index 1a925e0ab2..458aff8504 100644 --- a/fhirpath/src/test/resources/fhirpath-ptl/cases/existence_functions.yaml +++ b/fhirpath/src/test/resources/fhirpath-ptl/cases/existence_functions.yaml @@ -2,10 +2,21 @@ tests: - 'Testing the empty() function in various scenarios': - desc: '** Empty litera should return true **' expression: '{}.empty()' - result: [ ] + result: [ true ] - desc: '** Empty collection should return true **' expression: 'nothing.empty()' - result: [ ] + result: [ true ] + - desc: '** Computed empty collection is empty **' + expression: 'n1.where($this=0).empty()' + result: [ true ] + - desc: '** Resource singular empty collection is empty **' + expression: 'gender.empty()' + inputfile: 'Patient-empty.json' + result: [ true ] + - desc: '** Resource plural empty collection is empty **' + expression: 'name.given.empty()' + inputfile: 'Patient-empty.json' + result: [ true ] - desc: '** Singular integer is not empty **' expression: 'n1.empty()' result: [ false ] @@ -34,11 +45,20 @@ tests: - desc: '** Empty literal should return 0 **' expression: '{}.count()' result: [ 0 ] - disable: true # empty collection - desc: '** Empty collection has count 0 **' expression: 'nothing.count()' result: [ 0 ] - disable: true # empty collection + - desc: '** Computed empty collection has count 0 **' + expression: 'n1.where($this=0).count()' + result: [ 0 ] + - desc: '** Resource singular empty collection has count 0 **' + expression: 'gender.count()' + inputfile: 'Patient-empty.json' + result: [ 0 ] + - desc: '** Resource plural empty collection has count 0 **' + expression: 'name.given.count()' + inputfile: 'Patient-empty.json' + result: [ 0 ] - desc: '** Singular integer has count 1 **' expression: 'n1.count()' result: [ 1 ] @@ -55,14 +75,23 @@ tests: expression: 'e1.xy.y.count()' result: [ 1 ] - 'Testing the exists() function in various scenarios': - - desc: '** Empty literal should return false **' + - desc: '** Empty literal should does not exist **' expression: '{}.exists()' result: [ false ] - disable: true # empty collection - - desc: '** Empty collection should return false **' + - desc: '** Empty collection does not exist **' expression: 'nothing.exists()' result: [ false ] - disable: true # empty collection + - desc: '** Computed empty collection does not exist **' + expression: 'n1.where($this=0).exists()' + result: [ false ] + - desc: '** Resource singular empty collection does not exist **' + expression: 'gender.exists()' + inputfile: 'Patient-empty.json' + result: [ false ] + - desc: '** Resource plural empty collection does not exist **' + expression: 'name.given.exists()' + inputfile: 'Patient-empty.json' + result: [ false ] - desc: '** Singular integer exists **' expression: 'n1.exists()' result: [ true ] diff --git a/fhirpath/src/test/resources/fhirpath-ptl/resources/Patient-empty.json b/fhirpath/src/test/resources/fhirpath-ptl/resources/Patient-empty.json new file mode 100644 index 0000000000..bce8c2a439 --- /dev/null +++ b/fhirpath/src/test/resources/fhirpath-ptl/resources/Patient-empty.json @@ -0,0 +1,4 @@ +{ + "resourceType": "Patient", + "id": "empty" +}