fix: lunatic shape from (#869)

* refactor(ddi): add loop references in variable groups mapping

* refactor(loops): make some methods re-usable

* fix: lunatic shape from property

The previous implementation returned the the first variable of the corresponding variable group. Yet the order of variables in a variable group is not strictly enforced in current DDI specification. So this information has to be retrieved another way.

* test(shape-from): update unit tests

* chore: version 3.15.8

.github/workflows/sonar.yml

@@ -12,6 +12,9 @@ on:
       - 'README**.md'
       - 'Dockerfile'
       - '.github/**'
+      - '/build.gradle'
+      # temporary exclude root build.gradle file from test workflow trigger
+      # todo: externailze the project version somewhere so to not trigger tests for nothing when only project version has changed
 
 jobs:
   build:

build.gradle

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

eno-core/src/main/java/fr/insee/eno/core/model/variable/VariableGroup.java

@@ -1,18 +1,32 @@
 package fr.insee.eno.core.model.variable;
 
+import fr.insee.eno.core.annotations.Contexts.Context;
 import fr.insee.eno.core.annotations.DDI;
 import fr.insee.eno.core.model.EnoObject;
+import fr.insee.eno.core.parameter.Format;
+import logicalproduct33.VariableGroupType;
 import lombok.Getter;
 import lombok.Setter;
+import reusable33.ReferenceType;
 
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 
+/**
+ * A variable group contains all variables that share a common "scope". For collected variables, this means variables
+ * that are collected in a loop or a dynamic table, and loops that are linked. Example: loop "A" contains variables
+ * "foo" and "bar", loop "B" is linked to loop "A" and contains the variable "gus", then these two loops corresponds
+ * to a single variable group that contains variables "foo", "bar" and "gus". Calculated and external are defined at
+ * a certain level (i.e. scope) in Pogues that corresponds to questionnaire-level, or a main loop, or a dynamic table.
+ */
 @Getter
 @Setter
+@Context(format = Format.DDI, type = VariableGroupType.class)
 public class VariableGroup extends EnoObject {
 
+    public static final String DDI_QUESTIONNAIRE_TYPE = "Questionnaire";
+
     @DDI("!getVariableGroupNameList().isEmpty ? getVariableGroupNameArray(0).getStringArray(0).getStringValue() : null")
     private String name;
 
@@ -22,9 +36,27 @@ public class VariableGroup extends EnoObject {
     @DDI("getVariableReferenceList().![#index.get(#this.getIDArray(0).getStringValue())]")
     private final List<Variable> variables = new ArrayList<>();
 
+    /** Ordered list of iterable (i.e. loop or dynamic table) identifiers that correspond to this variable group.
+     * The first reference is either a "main" loop or a dynamic table, the others are linked loops.
+     * If the variable group corresponds to questionnaire-level variables, this list is empty. */
+    @DDI("T(fr.insee.eno.core.model.variable.VariableGroup).getDDILoopReferences(#this)")
+    private final List<String> loopReferences = new ArrayList<>();
+
     public Optional<Variable> getVariableByName(String variableName) {
         return variables.stream()
                 .filter(variable -> variable.getName().equals(variableName))
                 .findFirst();
     }
+
+    public static List<String> getDDILoopReferences(VariableGroupType ddiVariableGroup) {
+        if (DDI_QUESTIONNAIRE_TYPE.equals(ddiVariableGroup.getTypeOfVariableGroup().getStringValue()))
+            return new ArrayList<>();
+        // iterable means either loop or dynamic table
+        List<String> iterableReferences = new ArrayList<>();
+        for (ReferenceType reference : ddiVariableGroup.getBasedOnObject().getBasedOnReferenceList()) {
+            iterableReferences.add(reference.getIDArray(0).getStringValue());
+        }
+        return iterableReferences;
+    }
+
 }

eno-core/src/main/java/fr/insee/eno/core/processing/out/steps/lunatic/LunaticLoopResolution.java

@@ -20,6 +20,7 @@
 import java.math.BigInteger;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Optional;
 
 /** Lunatic technical processing for loops.
  * Requires: sorted components, hierarchy. */
@@ -175,25 +176,51 @@ private void setLinkedLoopIterations(Loop lunaticLoop, LinkedLoop enoLinkedLoop)
                 enoLinkedLoop.getId(), reference));
     }
 
+    /**
+     * Returns the first response name that belong to the reference loop.
+     * @param enoLinkedLoop Eno linked loop passed for logging purposes.
+     * @param enoReferenceLoop Eno reference ("main") loop.
+     * @param enoIndex Eno index.
+     * @return The first response name (string) that belong to the reference loop.
+     */
     public static String findFirstVariableOfReference(LinkedLoop enoLinkedLoop, StandaloneLoop enoReferenceLoop,
-                                                       EnoIndex enoIndex) {
-        AbstractSequence startSequence = (AbstractSequence) enoIndex.get(enoReferenceLoop.getLoopScope().get(0).getId());
-        if (startSequence.getSequenceStructure().isEmpty()) {
+                                                      EnoIndex enoIndex) {
+        String contextErrorMessage = String.format(
+                "Unable to find its first question to compute Lunatic \"iterations\" expression for linked loop '%s'.",
+                enoLinkedLoop.getId());
+        return findFirstResponseNameOfLoop(enoReferenceLoop, enoIndex, contextErrorMessage);
+    }
+
+    /**
+     * Returns the first response name that belong to the loop.
+     * @param enoLoop A eno loop object.
+     * @param enoIndex Eno index.
+     * @param contextErrorMessage Context that will be added in the exception message if retrieving the response name fails.
+     * @return The first response name (string) that belong to the loop.
+     */
+    public static String findFirstResponseNameOfLoop(fr.insee.eno.core.model.navigation.Loop enoLoop,
+                                                     EnoIndex enoIndex,
+                                                     String contextErrorMessage) {
+        AbstractSequence firstSequenceOfLoop = (AbstractSequence) enoIndex.get(enoLoop.getLoopScope().get(0).getId());
+        if (firstSequenceOfLoop.getSequenceStructure().isEmpty())
             throw new LunaticLoopException(String.format(
-                    "Linked loop '%s' is based on loop '%s'. " +
-                            "This loop is defined to start at sequence '%s', which is empty. " +
-                            "Unable to find its first question to compute Lunatic \"iterations\" expression.",
-                    enoLinkedLoop.getId(), enoReferenceLoop.getId(), startSequence.getId()));
-        }
-        String firstQuestionId = findFirstQuestionId(startSequence, enoIndex);
-        EnoObject firstQuestion = enoIndex.get(firstQuestionId);
-        if (! (firstQuestion instanceof SingleResponseQuestion)) {
+                    "Loop '%s' is defined to start at sequence '%s', which is empty. %s",
+                    enoLoop.getId(), firstSequenceOfLoop.getId(), contextErrorMessage));
+        String firstQuestionId = findFirstQuestionId(firstSequenceOfLoop, enoIndex);
+        Optional<String> variableName = getVariableNameFromQuestionId(firstQuestionId, enoIndex);
+        if (variableName.isEmpty())
             throw new LunaticLoopException(String.format(
-                    "Linked loop '%s' is based on loop '%s' that starts at sequence '%s'. " +
-                            "This first question of the sequence is not a \"simple\" question.",
-                    enoLinkedLoop.getId(), enoReferenceLoop.getId(), startSequence.getId()));
-        }
-        return ((SingleResponseQuestion) firstQuestion).getResponse().getVariableName();
+                    "Loop '%s' that starts at sequence '%s'. " +
+                            "This first question of the sequence is not a \"simple\" question. %s",
+                    enoLoop.getId(), firstSequenceOfLoop.getId(), contextErrorMessage));
+        return variableName.get();
+    }
+
+    private static Optional<String> getVariableNameFromQuestionId(String questionId, EnoIndex enoIndex) {
+        EnoObject firstQuestion = enoIndex.get(questionId);
+        if (! (firstQuestion instanceof SingleResponseQuestion))
+            return Optional.empty();
+        return Optional.of(((SingleResponseQuestion) firstQuestion).getResponse().getVariableName());
     }
 
     /**

eno-core/src/main/java/fr/insee/eno/core/processing/out/steps/lunatic/calculatedvariable/ShapefromAttributeRetrievalFromVariableGroups.java

@@ -1,8 +1,13 @@
 package fr.insee.eno.core.processing.out.steps.lunatic.calculatedvariable;
 
+import fr.insee.eno.core.exceptions.technical.MappingException;
 import fr.insee.eno.core.model.EnoQuestionnaire;
+import fr.insee.eno.core.model.navigation.Loop;
+import fr.insee.eno.core.model.question.DynamicTableQuestion;
 import fr.insee.eno.core.model.variable.Variable;
 import fr.insee.eno.core.model.variable.VariableGroup;
+import fr.insee.eno.core.processing.out.steps.lunatic.LunaticLoopResolution;
+import fr.insee.eno.core.reference.EnoIndex;
 
 import java.util.Optional;
 
@@ -14,23 +19,87 @@
 public class ShapefromAttributeRetrievalFromVariableGroups implements ShapefromAttributeRetrieval {
 
     /**
-     *
+     * Retrieves the Lunatic "shapeFrom" attribute. It is empty if the calculated variable with given name is not in a
+     * loop. Otherwise, it is the name of the first collected variable that is in the scope of the corresponding loop.
+     * This method uses the Eno questionnaire to retrieve this information.
      * @param lunaticCalculatedVariableName name of the calculated variable
      * @param enoQuestionnaire eno questionnaire
      * @return the first collected variable if the calculated variable is in a loop, nothing otherwise
      */
     public Optional<Variable> getShapeFrom(String lunaticCalculatedVariableName, EnoQuestionnaire enoQuestionnaire) {
+        EnoIndex enoIndex = enoQuestionnaire.getIndex();
+        return getShapeFrom(lunaticCalculatedVariableName, enoQuestionnaire, enoIndex);
+    }
+
+    // Note: the pattern with eno questionnaire and eno index is not satisfying as it currently is.
+    // It would probably better to separate the two.
+    // The first method is there so to not change the signature of the "shape from retrieval" interface for now.
+
+    // Note 2: using the eno index as little as possible to improve code maintainability in exchange
+    // for a slight loss of performance.
+
+    private Optional<Variable> getShapeFrom(String lunaticCalculatedVariableName, EnoQuestionnaire enoQuestionnaire,
+                                           EnoIndex enoIndex) {
         // retrieve the variable group in which the variable is present, excluding the "Questionnaire"
-        // variable group which does not corresponds to a "loop" variable group
+        // variable group which does not correspond to a "loop" variable group
         Optional<VariableGroup> variableGroup = enoQuestionnaire.getVariableGroups().stream()
-                .filter(vGroup -> !vGroup.getType().equals("Questionnaire"))
+                .filter(vGroup -> !vGroup.getType().equals(VariableGroup.DDI_QUESTIONNAIRE_TYPE))
                 .filter(vGroup -> vGroup.getVariableByName(lunaticCalculatedVariableName).isPresent())
                 .findFirst();
 
-        return variableGroup.flatMap(
-                        vGroup -> vGroup.getVariables().stream()
-                                .filter(variable -> variable.getCollectionType().equals(Variable.CollectionType.COLLECTED))
-                                .findFirst()
-                );
+        if (variableGroup.isEmpty())
+            return Optional.empty();
+
+        // then retrieve the "main" loop or dynamic table that corresponds to this variable group
+        String mainIterableReference = variableGroup.get().getLoopReferences().get(0);
+
+        // loop case
+        Optional<Loop> mainLoop = enoQuestionnaire.getLoops().stream()
+                .filter(loop -> mainIterableReference.equals(loop.getId()))
+                .findAny();
+        if (mainLoop.isPresent())
+            return Optional.of(findFirstCollectedVariableOfLoop(mainLoop.get(), enoQuestionnaire, enoIndex));
+
+        // dynamic table case
+        Optional<DynamicTableQuestion> dynamicTable = enoQuestionnaire.getMultipleResponseQuestions().stream()
+                .filter(DynamicTableQuestion.class::isInstance)
+                .map(DynamicTableQuestion.class::cast)
+                .filter(loop -> mainIterableReference.equals(loop.getId()))
+                .findAny();
+        if (dynamicTable.isPresent())
+            return Optional.of(findFirstCollectedVariableOfDynamicTable(dynamicTable.get(), enoQuestionnaire));
+
+        throw new MappingException(String.format(
+                "Unable to retrieve 'shapeFrom' property for the calculated variable '%s'. " +
+                        "This probably means that some information has not been mapped as expected during mapping.",
+                lunaticCalculatedVariableName));
     }
+
+    // Note: not sure why this method returns a Variable object and not a CollectedVariable, maybe it could be improved
+
+    private Variable findFirstCollectedVariableOfLoop(Loop enoLoop, EnoQuestionnaire enoQuestionnaire, EnoIndex enoIndex) {
+        String contextErrorMessage = "Unable to find its first question to retrieve Lunatic \"shapeFrom\" property.";
+        String firstQuestionVariableName = LunaticLoopResolution.findFirstResponseNameOfLoop(
+                enoLoop, enoIndex, contextErrorMessage);
+        return getVariableFromName(enoQuestionnaire, firstQuestionVariableName);
+    }
+
+    private Variable findFirstCollectedVariableOfDynamicTable(DynamicTableQuestion enoDynamicTable,
+                                                              EnoQuestionnaire enoQuestionnaire) {
+        String variableName =  enoDynamicTable.getVariableNames().get(0);
+        return getVariableFromName(enoQuestionnaire, variableName);
+    }
+
+    private static Variable getVariableFromName(EnoQuestionnaire enoQuestionnaire, String variableName) {
+        Optional<Variable> searchedVariable = enoQuestionnaire.getVariables()
+                .stream()
+                .filter(variable -> variableName.equals(variable.getName())).findAny();
+        if (searchedVariable.isEmpty())
+            throw new MappingException(String.format(
+                    "Unable to find variable with name '%s' in the Eno questionnaire variables " +
+                            "(while trying to retrieve Lunatic 'shapeFrom' property).",
+                    variableName));
+        return searchedVariable.get();
+    }
+
 }

eno-core/src/test/java/fr/insee/eno/core/processing/out/steps/lunatic/calculatedvariable/ShapefromAttributeRetrievalFromVariableGroupsTest.java

@@ -1,10 +1,17 @@
 package fr.insee.eno.core.processing.out.steps.lunatic.calculatedvariable;
 
 import fr.insee.eno.core.model.EnoQuestionnaire;
+import fr.insee.eno.core.model.navigation.StandaloneLoop;
+import fr.insee.eno.core.model.question.DynamicTableQuestion;
+import fr.insee.eno.core.model.question.TextQuestion;
+import fr.insee.eno.core.model.response.Response;
+import fr.insee.eno.core.model.sequence.Sequence;
+import fr.insee.eno.core.model.sequence.StructureItemReference;
 import fr.insee.eno.core.model.variable.CalculatedVariable;
 import fr.insee.eno.core.model.variable.CollectedVariable;
 import fr.insee.eno.core.model.variable.Variable;
 import fr.insee.eno.core.model.variable.VariableGroup;
+import fr.insee.eno.core.reference.EnoIndex;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -28,9 +35,34 @@ class ShapefromAttributeRetrievalFromVariableGroupsTest {
     void init() {
         shapefromAttributeRetrieval = new ShapefromAttributeRetrievalFromVariableGroups();
         enoQuestionnaire = new EnoQuestionnaire();
+        EnoIndex enoIndex = new EnoIndex();
+
+        StandaloneLoop loop = new StandaloneLoop();
+        loop.setId("loop-id");
+        loop.getLoopScope().add(StructureItemReference.builder()
+                .id("sequence-id")
+                .type(StructureItemReference.StructureItemType.SEQUENCE)
+                .build());
+        Sequence sequence = new Sequence();
+        sequence.setId("sequence-id");
+        sequence.getSequenceStructure().add(StructureItemReference.builder()
+                .id("question-id")
+                .type(StructureItemReference.StructureItemType.QUESTION)
+                .build());
+        TextQuestion textQuestion = new TextQuestion();
+        textQuestion.setId("question-id");
+        textQuestion.setResponse(new Response());
+        textQuestion.getResponse().setVariableName("variable2");
+
+        enoIndex.put("sequence-id", sequence);
+        enoIndex.put("question-id", textQuestion);
+        enoQuestionnaire.setIndex(enoIndex);
+        enoQuestionnaire.getLoops().add(loop);
 
         variableGroup1 = new VariableGroup();
         variableGroup1.setType("Loop");
+        variableGroup1.getLoopReferences().addAll(List.of("loop-id", "linked-loop-id"));
+        // (the linked loop reference should be ignored as only the "main" loop counts for retrieving the 'shapeFrom')
         variable1 = new CalculatedVariable();
         variable1.setName("variable1");
         variable2 = new CollectedVariable();
@@ -43,14 +75,21 @@ void init() {
         variable3.setName("variable3");
         variableGroup2.getVariables().add(variable3);
 
+        DynamicTableQuestion dynamicTableQuestion = new DynamicTableQuestion();
+        dynamicTableQuestion.setId("dynamic-table-id");
+        dynamicTableQuestion.getVariableNames().add("variable4");
+        enoQuestionnaire.getMultipleResponseQuestions().add(dynamicTableQuestion);
+
         variableGroup3 = new VariableGroup();
         variableGroup3.setType("Loop");
+        variableGroup3.getLoopReferences().add("dynamic-table-id");
         variable4 = new CollectedVariable();
         variable4.setName("variable4");
         variable5 = new CalculatedVariable();
         variable5.setName("variable5");
         variableGroup3.getVariables().addAll(List.of(variable4, variable5));
 
+        enoQuestionnaire.getVariables().addAll(List.of(variable1, variable2, variable3, variable4, variable5));
         enoQuestionnaire.getVariableGroups().addAll(List.of(variableGroup1, variableGroup2, variableGroup3));
     }
 
@@ -67,13 +106,17 @@ void whenVariableNotExistingReturnEmpty() {
     }
 
     @Test
-    void whenVariableInLoopVariableGroupReturnFirstCollectedVariable() {
-        Optional<Variable> variable = shapefromAttributeRetrieval.getShapeFrom("variable5", enoQuestionnaire);
+    void whenVariableInLoopVariableGroup_returnFirstCollectedVariable() {
+        Optional<Variable> variable = shapefromAttributeRetrieval.getShapeFrom("variable1", enoQuestionnaire);
         assertTrue(variable.isPresent());
-        assertEquals("variable4", variable.get().getName());
+        assertEquals("variable2", variable.get().getName());
+    }
 
-        variable = shapefromAttributeRetrieval.getShapeFrom("variable1", enoQuestionnaire);
+    @Test
+    void whenVariableInDynamicTableVariableGroup_returnFirstCollectedVariable() {
+        Optional<Variable> variable = shapefromAttributeRetrieval.getShapeFrom("variable5", enoQuestionnaire);
         assertTrue(variable.isPresent());
-        assertEquals("variable2", variable.get().getName());
+        assertEquals("variable4", variable.get().getName());
     }
+
 }