fix: ddi expression references resolution (#873)

* test: unit test for ddi expression references resolution

* test: add overlapping failing case

* fix: ddi expression references resolution

Fix cases when binding references overlap.

* chore: bump version

build.gradle

@@ -16,7 +16,7 @@ tasks.withType(JavaCompile).configureEach {
 
 allprojects {
     group = 'fr.insee.eno'
-    version = '3.15.8'
+    version = '3.15.9'
 }
 
 subprojects {

eno-core/src/main/java/fr/insee/eno/core/processing/in/steps/ddi/DDIResolveVariableReferencesInExpressions.java

@@ -9,6 +9,11 @@
 import fr.insee.eno.core.model.variable.Variable;
 import fr.insee.eno.core.processing.ProcessingStep;
 
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.List;
+import java.util.Set;
+
 public class DDIResolveVariableReferencesInExpressions implements ProcessingStep<EnoQuestionnaire> {
 
     /**
@@ -47,10 +52,28 @@ private void resolveExpression(StandaloneLoop standaloneLoop) {
 
     private static void resolveExpression(CalculatedExpression expression) {
         String value = expression.getValue();
-        for (BindingReference bindingReference : expression.getBindingReferences()) {
+        List<BindingReference> orderedBindingReferences = orderById(expression.getBindingReferences());
+        // Iterate on the reverse order, so that if some references overlap, the longer one is replaced first
+        for (int i = orderedBindingReferences.size() - 1; i >= 0; i --) {
+            BindingReference bindingReference = orderedBindingReferences.get(i);
             value = value.replace(bindingReference.getId(), bindingReference.getVariableName());
         }
         expression.setValue(value);
     }
 
+    /**
+     * Binding reference identifiers can overlap, so the replacement of references by the variable name must be done
+     * carefully in a rather precise order. This method returns a list containing given binding references, ordered by
+     * id.
+     * @param bindingReferences A set of binding references.
+     * @return A list of the binding references, ordered by id.
+     */
+    private static List<BindingReference> orderById(Set<BindingReference> bindingReferences) {
+        List<BindingReference> res = new ArrayList<>(bindingReferences);
+        res.sort(Comparator.comparing(BindingReference::getId));
+        return res;
+    }
+
+    // Note: orderByIdLength would have worked also
+
 }

eno-core/src/test/java/fr/insee/eno/core/processing/in/steps/ddi/DDIResolveVariableReferencesInExpressionsTest.java

@@ -0,0 +1,91 @@
+package fr.insee.eno.core.processing.in.steps.ddi;
+
+import fr.insee.eno.core.model.EnoQuestionnaire;
+import fr.insee.eno.core.model.calculated.BindingReference;
+import fr.insee.eno.core.model.calculated.CalculatedExpression;
+import fr.insee.eno.core.model.navigation.Filter;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+
+import java.util.LinkedHashSet;
+import java.util.Set;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class DDIResolveVariableReferencesInExpressionsTest {
+
+    @Test
+    void filterExpression_oneReference() {
+        //
+        EnoQuestionnaire enoQuestionnaire = new EnoQuestionnaire();
+        Filter filter = new Filter();
+        CalculatedExpression calculatedExpression= new CalculatedExpression();
+        calculatedExpression.setValue("foo-reference = 1");
+        calculatedExpression.getBindingReferences()
+                .add(new BindingReference("foo-reference", "FOO"));
+        filter.setExpression(calculatedExpression);
+        enoQuestionnaire.getFilters().add(filter);
+        //
+        new DDIResolveVariableReferencesInExpressions().apply(enoQuestionnaire);
+        //
+        assertEquals("FOO = 1", enoQuestionnaire.getFilters().get(0).getExpression().getValue());
+    }
+
+    /**
+     * In some cases variable references can overlap. This could lead to incorrect replacement of references by the
+     * corresponding variable name. This nested class contains a group of tests for these cases.
+     */
+    @Nested
+    class OverlappingCases {
+
+        /**
+         * This test uses an ordered implementation of Set for binding references, to simulate what could happen with
+         * real data.
+         */
+        @Test
+        void filterExpression_overlappingReferences_ascendingCase() {
+            //
+            EnoQuestionnaire enoQuestionnaire = new EnoQuestionnaire();
+            Filter filter = new Filter();
+            CalculatedExpression calculatedExpression= new CalculatedExpression();
+            calculatedExpression.setValue("foo-ref-1 = 1 and foo-ref-10 = 1");
+            Set<BindingReference> bindingReferences = new LinkedHashSet<>();
+            bindingReferences.add(new BindingReference("foo-ref-1", "FOO_A"));
+            bindingReferences.add(new BindingReference("foo-ref-10", "FOO_K"));
+            calculatedExpression.setBindingReferences(bindingReferences);
+            filter.setExpression(calculatedExpression);
+            enoQuestionnaire.getFilters().add(filter);
+            //
+            new DDIResolveVariableReferencesInExpressions().apply(enoQuestionnaire);
+            //
+            assertEquals("FOO_A = 1 and FOO_K = 1",
+                    enoQuestionnaire.getFilters().get(0).getExpression().getValue());
+        }
+
+        /**
+         * Same test with binding references in the reverse order. (These two tests could have been a parametrized
+         * test, but it would have been harder to read.)
+         */
+        @Test
+        void filterExpression_overlappingReferences_descendingCase() {
+            //
+            EnoQuestionnaire enoQuestionnaire = new EnoQuestionnaire();
+            Filter filter = new Filter();
+            CalculatedExpression calculatedExpression= new CalculatedExpression();
+            calculatedExpression.setValue("foo-ref-1 = 1 and foo-ref-10 = 1");
+            Set<BindingReference> bindingReferences = new LinkedHashSet<>();
+            bindingReferences.add(new BindingReference("foo-ref-10", "FOO_K"));
+            bindingReferences.add(new BindingReference("foo-ref-1", "FOO_A"));
+            calculatedExpression.setBindingReferences(bindingReferences);
+            filter.setExpression(calculatedExpression);
+            enoQuestionnaire.getFilters().add(filter);
+            //
+            new DDIResolveVariableReferencesInExpressions().apply(enoQuestionnaire);
+            //
+            assertEquals("FOO_A = 1 and FOO_K = 1",
+                    enoQuestionnaire.getFilters().get(0).getExpression().getValue());
+        }
+
+    }
+
+}