Fix Translation on Expanded Criteria with Reference Attribute Filters

pom.xml

@@ -32,7 +32,7 @@
         <hapi-fhir.version>6.10.3</hapi-fhir.version>
         <testcontainers.version>1.19.4</testcontainers.version>
         <slf4j.version>2.0.11</slf4j.version>
-        <ontology.version>2.1.27</ontology.version>
+        <ontology.version>2.2.0-RC</ontology.version>
     </properties>
 
     <dependencies>

src/main/java/de/numcodex/sq2cql/model/Mapping.java

@@ -32,41 +32,42 @@ public final class Mapping {
     private final String termCodeFhirPath;
 
     public Mapping(ContextualTermCode key, String resourceType, String valueFhirPath, String valueType, List<Modifier> fixedCriteria,
-                   List<AttributeMapping> attributeMappings, String timeRestrictionFhirPath, TermCode primaryCode, String termCodeFhirPath) {
+                   Map<TermCode, AttributeMapping> attributeMappings, String timeRestrictionFhirPath, TermCode primaryCode, String termCodeFhirPath) {
         this.key = requireNonNull(key);
         this.resourceType = requireNonNull(resourceType);
         this.valueFhirPath = valueFhirPath;
         this.valueType = valueType;
-        this.fixedCriteria = List.copyOf(fixedCriteria);
-        this.attributeMappings = (attributeMappings == null ? Map.of() : attributeMappings.stream()
-                .collect(Collectors.toMap(AttributeMapping::key, Function.identity())));
+        this.fixedCriteria = fixedCriteria;
+        this.attributeMappings = attributeMappings;
         this.timeRestrictionFhirPath = timeRestrictionFhirPath;
         this.primaryCode = primaryCode;
         this.termCodeFhirPath = termCodeFhirPath;
     }
 
     public static Mapping of(ContextualTermCode key, String resourceType) {
-        return new Mapping(key, resourceType, "value", null, List.of(), List.of(), null, null, null);
+        return new Mapping(key, resourceType, "value", null, List.of(), Map.of(), null, null, null);
     }
 
     public static Mapping of(ContextualTermCode key, String resourceType, String valueFhirPath) {
-        return new Mapping(key, resourceType, valueFhirPath, null, List.of(), List.of(), null, null, null);
+        return new Mapping(key, resourceType, valueFhirPath, null, List.of(), Map.of(), null, null, null);
     }
 
     public static Mapping of(ContextualTermCode key, String resourceType, String valueFhirPath, String valueType) {
-        return new Mapping(key, resourceType, valueFhirPath, valueType, List.of(), List.of(), null, null, null);
+        return new Mapping(key, resourceType, valueFhirPath, valueType, List.of(), Map.of(), null, null, null);
     }
 
     public static Mapping of(ContextualTermCode key, String resourceType, String valueFhirPath, String valueType, List<Modifier> fixedCriteria, List<AttributeMapping> attributeMappings) {
         return new Mapping(key, resourceType, valueFhirPath == null ? "value" : valueFhirPath, valueType,
                 fixedCriteria == null ? List.of() : List.copyOf(fixedCriteria),
-                attributeMappings, null, null, null);
+                (attributeMappings == null ? Map.of() : attributeMappings.stream()
+                        .collect(Collectors.toMap(AttributeMapping::key, Function.identity()))), null, null, null);
     }
 
     public static Mapping of(ContextualTermCode key, String resourceType, String valueFhirPath, String valueType, List<Modifier> fixedCriteria, List<AttributeMapping> attributeMappings, String timeRestrictionFhirPath) {
         return new Mapping(key, resourceType, valueFhirPath == null ? "value" : valueFhirPath, valueType,
                 fixedCriteria == null ? List.of() : List.copyOf(fixedCriteria),
-                attributeMappings, timeRestrictionFhirPath, null, null);
+                (attributeMappings == null ? Map.of() : attributeMappings.stream()
+                        .collect(Collectors.toMap(AttributeMapping::key, Function.identity()))), timeRestrictionFhirPath, null, null);
     }
 
     @JsonCreator
@@ -86,7 +87,8 @@ public static Mapping of(@JsonProperty("context") TermCode context,
                 valueFhirPath == null ? "value" : valueFhirPath,
                 valueType,
                 fixedCriteria == null ? List.of() : List.copyOf(fixedCriteria),
-                attributeMappings,
+                (attributeMappings == null ? Map.of() : attributeMappings.stream()
+                        .collect(Collectors.toMap(AttributeMapping::key, Function.identity()))),
                 timeRestrictionFhirPath,
                 primaryCode,
                 termCodeFhirPath);
@@ -123,7 +125,8 @@ public Optional<String> timeRestrictionFhirPath() {
     /**
      * Returns the primary code of this mapping. The primary code is used in the retrieve CQL
      * expression to identify the resources of interest. The path for the primary code path is
-     * implicitly given in CQL but can be looked up at https://github.com/cqframework/clinical_quality_language/blob/master/Src/java/quick/src/main/resources/org/hl7/fhir/fhir-modelinfo-4.0.1.xml
+     * implicitly given in CQL but can be looked up
+     * <a href="https://github.com/cqframework/clinical_quality_language/blob/master/Src/java/quick/src/main/resources/org/hl7/fhir/fhir-modelinfo-4.0.1.xml">here</a>
      *
      * @return the primary code of this mapping or the key if no primary code is defined
      */

src/main/java/de/numcodex/sq2cql/model/cql/AliasedQuerySource.java

@@ -2,6 +2,8 @@
 
 import de.numcodex.sq2cql.PrintContext;
 
+import java.util.Map;
+
 import static java.util.Objects.requireNonNull;
 
 public record AliasedQuerySource(Expression<?> querySource, IdentifierExpression alias) {
@@ -19,4 +21,9 @@ public String print(PrintContext printContext) {
         assert printContext.precedence() == 0;
         return "%s %s".formatted(querySource.print(printContext.increase()), alias.print(printContext));
     }
+
+    public AliasedQuerySource withIncrementedSuffixes(Map<String, Integer> increments) {
+        return new AliasedQuerySource(querySource.withIncrementedSuffixes(increments),
+                alias.withIncrementedSuffixes(increments));
+    }
 }

src/main/java/de/numcodex/sq2cql/model/cql/BetweenExpression.java

@@ -2,6 +2,8 @@
 
 import de.numcodex.sq2cql.PrintContext;
 
+import java.util.Map;
+
 import static java.util.Objects.requireNonNull;
 
 public record BetweenExpression(Expression<?> value,
@@ -26,4 +28,10 @@ public String print(PrintContext printContext) {
         return printContext.parenthesize(PRECEDENCE, "%s between %s and %s".formatted(value.print(childPrintContext),
                 lowerBound.print(childPrintContext), upperBound.print(childPrintContext)));
     }
+
+    @Override
+    public DefaultExpression withIncrementedSuffixes(Map<String, Integer> increments) {
+        return new BetweenExpression(value.withIncrementedSuffixes(increments),
+                lowerBound.withIncrementedSuffixes(increments), upperBound.withIncrementedSuffixes(increments));
+    }
 }

src/main/java/de/numcodex/sq2cql/model/cql/Clause.java

@@ -2,7 +2,11 @@
 
 import de.numcodex.sq2cql.PrintContext;
 
+import java.util.Map;
+
 public interface Clause {
 
     String print(PrintContext printContext);
+
+    Clause withIncrementedSuffixes(Map<String, Integer> increments);
 }

src/main/java/de/numcodex/sq2cql/model/cql/ComparatorExpression.java

@@ -3,6 +3,8 @@
 import de.numcodex.sq2cql.PrintContext;
 import de.numcodex.sq2cql.model.common.Comparator;
 
+import java.util.Map;
+
 import static java.util.Objects.requireNonNull;
 
 public record ComparatorExpression(Expression<?> a, Comparator comparator,
@@ -29,4 +31,10 @@ public String print(PrintContext printContext) {
         return printContext.parenthesize(precedence, "%s %s %s".formatted(a.print(childPrintContext), comparator,
                 b.print(childPrintContext)));
     }
+
+    @Override
+    public DefaultExpression withIncrementedSuffixes(Map<String, Integer> increments) {
+        return new ComparatorExpression(a.withIncrementedSuffixes(increments), comparator,
+                b.withIncrementedSuffixes(increments));
+    }
 }

src/main/java/de/numcodex/sq2cql/model/cql/Container.java

@@ -1,6 +1,5 @@
 package de.numcodex.sq2cql.model.cql;
 
-import de.numcodex.sq2cql.Lists;
 import de.numcodex.sq2cql.Maps;
 import de.numcodex.sq2cql.Sets;
 
@@ -116,10 +115,16 @@ public static <T extends Expression<T>> BinaryOperator<Container<T>> combiner(Bi
                     b.expression.withIncrementedSuffixes(bIncrements)),
                     Sets.union(a.codeSystemDefinitions, b.codeSystemDefinitions),
                     Sets.union(a.unfilteredDefinitions, b.unfilteredDefinitions),
-                    Lists.concat(a.withIncrementedSuffixes(aIncrements), b.withIncrementedSuffixes(bIncrements)));
+                    ExpressionDefinitions.unionByName(a.withIncrementedSuffixes(aIncrements),
+                            b.withIncrementedSuffixes(bIncrements)));
         };
     }
 
+    /**
+     * Returns a map of patient definitions name identifier prefixes to their maximum numerical suffixes.
+     *
+     * @return a map of identifier prefixes to numerical suffixes
+     */
     private Map<String, Integer> suffixes() {
         return patientDefinitions.stream()
                 .map(ExpressionDefinition::suffixes)
@@ -198,7 +203,29 @@ public Container<DefaultExpression> moveToPatientContext(String name) {
         }
         var identifier = SuffixedIdentifierExpression.of(name, suffixes().getOrDefault(name, 0));
         return new Container<>(new WrapperExpression(identifier), codeSystemDefinitions, unfilteredDefinitions,
-                Lists.append(patientDefinitions, ExpressionDefinition.of(identifier, expression)));
+                ExpressionDefinitions.appendByUniqueName(patientDefinitions, ExpressionDefinition.of(identifier, expression)));
+    }
+
+    /**
+     * Moves the expression of this container into the patient context and returns a Container with an
+     * {@link IdentifierExpression} holding {@code name}.
+     * <p>
+     * If this Container just holds an {@link IdentifierExpression} it's not moved.
+     * <p>
+     * The difference to {@link #moveToPatientContext(String)} is that the name given has to be already unique as it
+     * will not get a suffix that gets incremented on collision. So the caller has to be sure that there will be no
+     * expression with different content but the same name.
+     *
+     * @param name the name of the expression definition in the Patient context that has to be already unique
+     * @return a Container with an {@link IdentifierExpression} holding {@code name}
+     */
+    public Container<DefaultExpression> moveToPatientContextWithUniqueName(String name) {
+        if (expression == null) {
+            return map(WrapperExpression::new);
+        }
+        var identifier = StandardIdentifierExpression.of(name);
+        return new Container<>(new WrapperExpression(identifier), codeSystemDefinitions, unfilteredDefinitions,
+                ExpressionDefinitions.appendByUniqueName(patientDefinitions, ExpressionDefinition.of(identifier, expression)));
     }
 
     /**
@@ -227,7 +254,7 @@ public <U extends Expression<U>> Container<U> flatMap(Function<? super T, Contai
             return new Container<>(container.expression.withIncrementedSuffixes(increments),
                     Sets.union(codeSystemDefinitions, container.codeSystemDefinitions),
                     Sets.union(unfilteredDefinitions, container.unfilteredDefinitions),
-                    Lists.concat(patientDefinitions, container.patientDefinitions.stream()
+                    ExpressionDefinitions.unionByName(patientDefinitions, container.patientDefinitions.stream()
                             .map(d -> d.withIncrementedSuffixes(increments))
                             .toList()));
         }

src/main/java/de/numcodex/sq2cql/model/cql/ExistsExpression.java

@@ -2,6 +2,8 @@
 
 import de.numcodex.sq2cql.PrintContext;
 
+import java.util.Map;
+
 import static java.util.Objects.requireNonNull;
 
 public record ExistsExpression(Expression<?> expression) implements DefaultExpression {
@@ -21,4 +23,9 @@ public String print(PrintContext printContext) {
         return printContext.parenthesize(PRECEDENCE, "exists " + expression.print(printContext
                 .withPrecedence(PRECEDENCE)));
     }
+
+    @Override
+    public DefaultExpression withIncrementedSuffixes(Map<String, Integer> increments) {
+        return new ExistsExpression(expression.withIncrementedSuffixes(increments));
+    }
 }

src/main/java/de/numcodex/sq2cql/model/cql/Expression.java

@@ -18,6 +18,11 @@ public interface Expression<T extends Expression<T>> {
 
     String print(PrintContext printContext);
 
+    /**
+     * Returns a map of identifier prefixes to numerical suffixes of this expression and all children.
+     *
+     * @return a map of identifier prefixes to numerical suffixes
+     */
     default Map<String, Integer> suffixes() {
         return Map.of();
     }

src/main/java/de/numcodex/sq2cql/model/cql/ExpressionDefinition.java

@@ -29,6 +29,11 @@ public String print(PrintContext printContext) {
                 expression.print(newPrintContext));
     }
 
+    /**
+     * Returns a map of the {@code name} identifier prefix to its numerical suffix if the name is suffixed.
+     *
+     * @return a map of identifier prefix to its numerical suffix
+     */
     public Map<String, Integer> suffixes() {
         return name.suffixes();
     }

src/main/java/de/numcodex/sq2cql/model/cql/ExpressionDefinitions.java

@@ -0,0 +1,26 @@
+package de.numcodex.sq2cql.model.cql;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public interface ExpressionDefinitions {
+
+    static List<ExpressionDefinition> appendByUniqueName(List<ExpressionDefinition> a, ExpressionDefinition b) {
+        if (a.stream().noneMatch(d -> d.name().equals(b.name()))) {
+            var newList = new ArrayList<>(a);
+            newList.add(b);
+            return List.copyOf(newList);
+        }
+        return a;
+    }
+
+    static List<ExpressionDefinition> unionByName(List<ExpressionDefinition> a, List<ExpressionDefinition> b) {
+        var newList = new ArrayList<>(a);
+        for (var def : b) {
+            if (newList.stream().noneMatch(d -> d.name().equals(def.name()))) {
+                newList.add(def);
+            }
+        }
+        return List.copyOf(newList);
+    }
+}

src/main/java/de/numcodex/sq2cql/model/cql/IntervalSelector.java

@@ -2,6 +2,8 @@
 
 import de.numcodex.sq2cql.PrintContext;
 
+import java.util.Map;
+
 import static java.util.Objects.requireNonNull;
 
 public record IntervalSelector(Expression<?> intervalStart, Expression<?> intervalEnd) implements DefaultExpression {
@@ -19,4 +21,10 @@ public static IntervalSelector of(Expression<?> intervalStart, Expression<?> int
     public String print(PrintContext printContext) {
         return "Interval[%s, %s]".formatted(intervalStart.print(printContext), intervalEnd.print(printContext));
     }
+
+    @Override
+    public DefaultExpression withIncrementedSuffixes(Map<String, Integer> increments) {
+        return new IntervalSelector(intervalStart.withIncrementedSuffixes(increments),
+                intervalEnd.withIncrementedSuffixes(increments));
+    }
 }

src/main/java/de/numcodex/sq2cql/model/cql/InvocationExpression.java

@@ -2,6 +2,8 @@
 
 import de.numcodex.sq2cql.PrintContext;
 
+import java.util.Map;
+
 import static java.util.Objects.requireNonNull;
 
 /**
@@ -22,4 +24,9 @@ public static InvocationExpression of(Expression<?> expression, String invocatio
     public String print(PrintContext printContext) {
         return "%s.%s".formatted(expression.print(printContext), invocation);
     }
+
+    @Override
+    public DefaultExpression withIncrementedSuffixes(Map<String, Integer> increments) {
+        return new InvocationExpression(expression.withIncrementedSuffixes(suffixes()), invocation);
+    }
 }

src/main/java/de/numcodex/sq2cql/model/cql/MembershipExpression.java

@@ -2,6 +2,8 @@
 
 import de.numcodex.sq2cql.PrintContext;
 
+import java.util.Map;
+
 import static java.util.Objects.requireNonNull;
 
 public record MembershipExpression(Expression<?> a, String op, Expression<?> b) implements DefaultExpression {
@@ -27,4 +29,10 @@ public String print(PrintContext printContext) {
         return printContext.parenthesize(PRECEDENCE, "%s %s %s".formatted(a.print(childPrintContext), op,
                 b.print(childPrintContext)));
     }
+
+    @Override
+    public DefaultExpression withIncrementedSuffixes(Map<String, Integer> increments) {
+        return new MembershipExpression(a.withIncrementedSuffixes(increments), op,
+                b.withIncrementedSuffixes(increments));
+    }
 }

src/main/java/de/numcodex/sq2cql/model/cql/OrExpression.java

@@ -28,15 +28,15 @@ public static <T extends Expression<T>, U extends Expression<U>> DefaultExpressi
         }
     }
 
-    @Override
-    public DefaultExpression withIncrementedSuffixes(Map<String, Integer> increments) {
-        return new OrExpression(expressions.stream().map(e -> e.withIncrementedSuffixes(increments)).toList());
-    }
-
     @Override
     public String print(PrintContext printContext) {
         return printContext.parenthesize(PRECEDENCE, expressions.stream()
                 .map(printContext.withPrecedence(PRECEDENCE)::print)
                 .collect(joining(" or\n" + printContext.getIndent())));
     }
+
+    @Override
+    public DefaultExpression withIncrementedSuffixes(Map<String, Integer> increments) {
+        return new OrExpression(expressions.stream().map(e -> e.withIncrementedSuffixes(increments)).toList());
+    }
 }

src/main/java/de/numcodex/sq2cql/model/cql/QueryExpression.java

@@ -58,8 +58,10 @@ public String print(PrintContext printContext) {
 
     @Override
     public QueryExpression withIncrementedSuffixes(Map<String, Integer> increments) {
-        //TODO: decent into clauses
-        return this;
+        return new QueryExpression(sourceClause.withIncrementedSuffixes(increments),
+                queryInclusionClauses.stream().map(e -> e.withIncrementedSuffixes(increments)).toList(),
+                whereClause.withIncrementedSuffixes(increments),
+                returnClause == null ? null : returnClause.withIncrementedSuffixes(increments));
     }
 
     private List<Clause> clauses() {

src/main/java/de/numcodex/sq2cql/model/cql/QueryInclusionClause.java

@@ -1,4 +1,9 @@
 package de.numcodex.sq2cql.model.cql;
 
+import java.util.Map;
+
 public interface QueryInclusionClause extends Clause {
+
+    @Override
+    QueryInclusionClause withIncrementedSuffixes(Map<String, Integer> increments);
 }

src/main/java/de/numcodex/sq2cql/model/cql/RetrieveExpression.java

@@ -29,7 +29,8 @@ public String print(PrintContext printContext) {
 
     @Override
     public RetrieveExpression withIncrementedSuffixes(Map<String, Integer> increments) {
-        return new RetrieveExpression(resourceType, terminology.withIncrementedSuffixes(increments));
+        return new RetrieveExpression(resourceType,
+                terminology == null ? null : terminology.withIncrementedSuffixes(increments));
     }
 
     public IdentifierExpression alias() {