Merge pull request #2208 from aehrc/issue/2149

Correcting the count, exits and empty for empty collections.

Author: piotrszul

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,

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());
   }
 

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);

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));
   }
-  
+
 }

fhirpath/src/main/java/au/csiro/pathling/fhirpath/column/EmptyRepresentation.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);
     }
   }

fhirpath/src/main/java/au/csiro/pathling/fhirpath/execution/NullEvaluator.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());
   }
 
   /**

fhirpath/src/main/java/au/csiro/pathling/fhirpath/function/provider/ExistenceFunctions.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())
         ));
   }
 

fhirpath/src/main/java/au/csiro/pathling/fhirpath/function/provider/TerminologyFunctions.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<ProjectionResult> stubResults = components.stream()
         .map(s -> s.evaluate(stubContext))
         .toList();

fhirpath/src/main/java/au/csiro/pathling/view/UnnestingSelection.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();
   }
 

fhirpath/src/test/java/au/csiro/pathling/fhirpath/column/DefaultRepresentationTest.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")

fhirpath/src/test/java/au/csiro/pathling/fhirpath/yaml/YamlFhirpathTest.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 {
 
 

fhirpath/src/test/java/au/csiro/pathling/fhirpath/yaml/YamlReferenceImplTest.java

@@ -6,5 +6,7 @@
 @Retention(RetentionPolicy.RUNTIME)
 public @interface YamlConfig {
 
-  String value();
+  String config() default "";
+
+  String resourceBase() default "";
 }

fhirpath/src/test/java/au/csiro/pathling/test/yaml/YamlConfig.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<? extends Arguments> provideArguments(ExtensionContext context) {
 
       final Optional<String> testConfigPath = context.getTestClass()
           .flatMap(c -> Optional.ofNullable(c.getAnnotation(YamlConfig.class)))
-          .map(YamlConfig::value);
+          .map(YamlConfig::config)
+          .filter(s -> !s.isBlank());
+
+      final Optional<String> 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<? extends Arguments> provideArguments(ExtensionContext context) {
           .map(ts -> StdRuntimeCase.of(ts,
               Optional.ofNullable(ts.getInputFile())
                   .map(f -> (Function<RuntimeContext, ResourceResolver>) FhirResolverFactory.of(
-                      getResourceAsString("fhirpath-js/resources/" + f)))
+                      getResourceAsString(resourceBase.orElse("") + File.separator + f)))
                   .orElse(defaultResolverFactory),
               excluder.apply(ts)
           ))