fix: add null iterations

alexisszmundy · alexisszmundy · commit 84f33f475030 · 2026-06-15T14:32:10.000+02:00
diff --git a/src/main/java/fr/insee/genesis/controller/rest/responses/ResponseController.java b/src/main/java/fr/insee/genesis/controller/rest/responses/ResponseController.java
@@ -34,6 +34,7 @@
 import fr.insee.genesis.exceptions.ReviewDisabledException;
 import fr.insee.genesis.infrastructure.utils.FileUtils;
 import fr.insee.modelefiliere.RawResponseDto;
+import io.swagger.v3.oas.annotations.Hidden;
 import io.swagger.v3.oas.annotations.Operation;
 import io.swagger.v3.oas.annotations.Parameter;
 import io.swagger.v3.oas.annotations.media.Schema;
@@ -333,7 +334,7 @@ public ResponseEntity<SurveyUnitSimplifiedDto> getLatestByInterrogationOneObject
     @Operation(summary = "Returns the response with the latest variables for a collectionInstrument, mode and " +
             "interrogation")
     @GetMapping(path = "/{collectionInstrumentId}/{mode}/{interrogationId}")
-    @PreAuthorize("hasRole('USER_KRAFTWERK')")
+    @PreAuthorize("hasRole('ADMIN')")
     public ResponseEntity<SurveyUnitSimplifiedDto> getResponseByCollectionInstrumentAndInterrogation(
             @PathVariable("collectionInstrumentId") String collectionInstrumentId,
             @PathVariable("interrogationId") String interrogationId,
@@ -407,6 +408,7 @@ public ResponseEntity<List<SurveyUnitSimplifiedDto>> getLatestForInterrogationLi
         return ResponseEntity.ok(results);
     }
 
+    //Kraftwerk uses this
     @Operation(summary = "Retrieve responses for a collection instrument and a list of interrogations",
             description = "Return the latest state for each variable for the given interrogationIds and a given collection instrument (formerly questionnaire).<br>" +
                     "For a given id, the endpoint returns a document by collection mode (if there is more than one)<br>" +
@@ -696,6 +698,7 @@ private static String getSuccessMessage(boolean isAnyDataSaved) {
      */
     //TODO Unused for now, reuse code for optimizations, also move it to service
     @Deprecated
+    @Hidden
     @Operation(summary = "Retrieve all responses for a questionnaire and a list of UE",
             description = "Return the latest state for each variable for the given ids and a given questionnaire.<br>" +
                     "For a given id, the endpoint returns a document by collection mode (if there is more than one).")
diff --git a/src/main/java/fr/insee/genesis/domain/service/surveyunit/SurveyUnitService.java b/src/main/java/fr/insee/genesis/domain/service/surveyunit/SurveyUnitService.java
@@ -115,13 +115,13 @@ private List<SurveyUnitModel> getLatestSurveyUnitModels(List<SurveyUnitModel> su
         List<SurveyUnitModel> latestUpdatesByVariables = new ArrayList<>();
 
         List<Mode> modes = getDistinctsModes(surveyUnitModels);
-        modes.forEach(mode ->{
+        modes.forEach(mode -> {
             List<SurveyUnitModel> suByMode = surveyUnitModels.stream()
                     .filter(surveyUnitModel -> surveyUnitModel.getMode().equals(mode))
                     .sorted((o1, o2) -> o2.getRecordDate().compareTo(o1.getRecordDate())) //Sorting update by date (latest updates first by date of upload in database)
                     .toList();
 
-            //We had all the variables of the oldest update
+            //We add all the variables of the latest update
             latestUpdatesByVariables.add(suByMode.getFirst());
             //We keep the name of already added variables to skip them in older updates
             Set<VarIdScopeTuple> addedVariables = new HashSet<>();
@@ -142,7 +142,7 @@ private List<SurveyUnitModel> getLatestSurveyUnitModels(List<SurveyUnitModel> su
             ;
 
             suByMode.forEach(surveyUnitModel -> {
-                //Get non null usualSurveyUnitId
+                //Get non-null usualSurveyUnitId
                 if (surveyUnitModel.getUsualSurveyUnitId() != null){
                     latestUpdate.setUsualSurveyUnitId(surveyUnitModel.getUsualSurveyUnitId());
                 }
@@ -173,9 +173,8 @@ private List<SurveyUnitModel> getLatestSurveyUnitModels(List<SurveyUnitModel> su
 
                 // If there are new variables, we add the update to the list of latest updates
                 if (!collectedVariablesToKeep.isEmpty() || !externalVariablesToKeep.isEmpty()){
-                    surveyUnitModel.setCollectedVariables(collectedVariablesToKeep);
-                    surveyUnitModel.setExternalVariables(externalVariablesToKeep);
-                    latestUpdatesByVariables.add(surveyUnitModel);
+                    latestUpdate.getCollectedVariables().addAll(collectedVariablesToKeep);
+                    latestUpdate.getExternalVariables().addAll(externalVariablesToKeep);
                 }
             });
         });
diff --git a/src/test/java/fr/insee/genesis/controller/rest/responses/RawResponseControllerIT.java b/src/test/java/fr/insee/genesis/controller/rest/responses/RawResponseControllerIT.java
@@ -44,6 +44,7 @@
 import java.util.Set;
 
 import static fr.insee.genesis.domain.utils.JsonUtils.jsonToMap;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyList;
 import static org.mockito.ArgumentMatchers.argThat;
@@ -518,6 +519,90 @@ void process_raw_response_keep_null_test(boolean isNullSurveyUnitValues) {
             Assertions.assertThat(variableDocument.getValue()).isNull();
         }
 
+        @ParameterizedTest
+        @ValueSource(booleans = {false, true})
+        @WithMockUser(roles = "SCHEDULER")
+        @DisplayName("Filiere model raw data null values should be added if iterations already exists")
+        @SneakyThrows
+        void process_raw_response_add_null_iterations_test(boolean isNullSurveyUnitValues) {
+            //GIVEN
+            String collectionInstrumentId = "TESTQUEST";
+            Mode mode = Mode.WEB;
+            List<String> interrogationIds = List.of("INTERRO1");
+
+            //Raw response with absent variables
+            String variableName = "VAR1";
+            String collectedValue = "value1";
+            Map<String, String> collectedVariablesAndValues = new HashMap<>();
+
+            String externalVariableName = "EXTVAR1";
+            String externalValue = "externalvalue1";
+            Map<String, String> externalVariablesAndValues = new HashMap<>();
+
+            setFiliereModelTestMockBehaviour(
+                    collectionInstrumentId,
+                    mode,
+                    interrogationIds,
+                    collectedVariablesAndValues,
+                    externalVariablesAndValues,
+                    true
+            );
+
+            //Survey unit that already exists for first interrogation with null values or not
+            SurveyUnitDocument alreadyPresentSurveyUnitDocument = getSurveyUnitDocumentWithIterations(
+                    collectionInstrumentId,
+                    interrogationIds.getFirst(),
+                    variableName,
+                    isNullSurveyUnitValues ? null : collectedValue,
+                    externalVariableName,
+                    isNullSurveyUnitValues ? null : externalValue
+            );
+
+            when(surveyUnitMongoDBRepository.findByCollectionInstrumentIdAndInterrogationIds(collectionInstrumentId, interrogationIds))
+                    .thenReturn(List.of(alreadyPresentSurveyUnitDocument));
+
+            // WHEN
+            mockMvc.perform(post("/raw-responses/%s/process".formatted(collectionInstrumentId))
+                            .with(csrf())
+                            .contentType(MediaType.APPLICATION_JSON))
+                    .andExpect(status().isOk());
+
+            //THEN
+            @SuppressWarnings("unchecked")
+            ArgumentCaptor<List<SurveyUnitDocument>> listArgumentCaptor =
+                    ArgumentCaptor.forClass(List.class);
+            verify(surveyUnitMongoDBRepository, times(1))
+                    .insert(listArgumentCaptor.capture());
+
+            //Document must have null variables values
+            List<SurveyUnitDocument> savedDocuments = listArgumentCaptor.getValue();
+            Assertions.assertThat(savedDocuments).isNotNull().hasSize(interrogationIds.size());
+            SurveyUnitDocument savedDocument = savedDocuments.stream().filter(
+                            surveyUnitDocument ->
+                                    surveyUnitDocument.getInterrogationId().equals(interrogationIds.getFirst()))
+                    .toList().getFirst();
+
+            Assertions.assertThat(savedDocument.getCollectedVariables())
+                    .isNotNull()
+                    .hasSize(3)
+                    .allSatisfy(v -> {
+                        assertThat(v.getVarId()).isEqualTo(variableName);
+                        assertThat(v.getValue()).isNull();
+                    })
+                    .extracting(VariableDocument::getIteration)
+                    .containsExactlyInAnyOrder(1, 2, 3);
+
+            Assertions.assertThat(savedDocument.getExternalVariables())
+                    .isNotNull()
+                    .hasSize(3)
+                    .allSatisfy(v -> {
+                        assertThat(v.getVarId()).isEqualTo(externalVariableName);
+                        assertThat(v.getValue()).isNull();
+                    })
+                    .extracting(VariableDocument::getIteration)
+                    .containsExactlyInAnyOrder(1, 2, 3);
+        }
+
         @Test
         @WithMockUser(roles = "SCHEDULER")
         @DisplayName("Filiere model raw data shouldn't overwrite with null if not existent before")
@@ -1289,5 +1374,41 @@ private SurveyUnitDocument getSurveyUnitDocument(
         return alreadyPresentSurveyUnitDocument;
     }
 
+    private SurveyUnitDocument getSurveyUnitDocumentWithIterations(
+            String collectionInstrumentId,
+            String interrogationId,
+            String variableName,
+            String collectedValue,
+            String externalVariableName,
+            String externalValue
+    ) {
+        SurveyUnitDocument alreadyPresentSurveyUnitDocument = new SurveyUnitDocument();
+        alreadyPresentSurveyUnitDocument.setCollectionInstrumentId(collectionInstrumentId);
+        alreadyPresentSurveyUnitDocument.setInterrogationId(interrogationId);
+        alreadyPresentSurveyUnitDocument.setMode(Mode.WEB.getModeName());
+        alreadyPresentSurveyUnitDocument.setState("COLLECTED");
+
+        alreadyPresentSurveyUnitDocument.setCollectedVariables(new ArrayList<>());
+        for(int i = 1; i <= 3; i++) {
+            VariableDocument oldVariable = new VariableDocument();
+            oldVariable.setVarId(variableName);
+            oldVariable.setIteration(i);
+            oldVariable.setValue(collectedValue);
+            oldVariable.setScope(Constants.ROOT_GROUP_NAME);
+            alreadyPresentSurveyUnitDocument.getCollectedVariables().add(oldVariable);
+        }
+
+        alreadyPresentSurveyUnitDocument.setExternalVariables(new ArrayList<>());
+        for(int i = 1; i <= 3; i++) {
+            VariableDocument oldVariable = new VariableDocument();
+            oldVariable.setVarId(externalVariableName);
+            oldVariable.setIteration(i);
+            oldVariable.setValue(externalValue);
+            oldVariable.setScope(Constants.ROOT_GROUP_NAME);
+            alreadyPresentSurveyUnitDocument.getExternalVariables().add(oldVariable);
+        }
+        return alreadyPresentSurveyUnitDocument;
+    }
+
     //TODO GET tests
 }
diff --git a/src/test/java/fr/insee/genesis/domain/converter/rawdata/RawDataConverterTest.java b/src/test/java/fr/insee/genesis/domain/converter/rawdata/RawDataConverterTest.java
@@ -11,6 +11,8 @@
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.mockito.ArgumentCaptor;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
@@ -154,6 +156,7 @@ void bigDecimalIntegerDouble() {
     @Nested
     class convertCollectedVariablesTests{
         @Test
+        @DisplayName("Should no nothing if no COLLECTED in payload and no last SU")
         void convertCollectedVariables_shouldDoNothing_whenNoCollectedKeyAndNoLastSurveyUnit() {
             Map<String, Object> payload = new HashMap<>();
 
@@ -176,12 +179,13 @@ void convertCollectedVariables_shouldDoNothing_whenNoCollectedKeyAndNoLastSurvey
         }
 
         @Test
+        @DisplayName("Should no nothing if no COLLECTED in payload and last SU does not match datastate")
         void convertCollectedVariables_shouldDoNothing_whenNoCollectedKeyAndLastSurveyUnitStateDoesNotMatch() {
-            Map<String, Object> payload = new HashMap<>(); // pas de clé "COLLECTED"
+            Map<String, Object> payload = new HashMap<>();
 
             SurveyUnitModel lastSurveyUnit = SurveyUnitModel.builder()
                     .interrogationId("INT1")
-                    .state(DataState.FORCED) // différent de DataState.COLLECTED
+                    .state(DataState.FORCED)
                     .collectedVariables(List.of(
                             VariableModel.builder().varId("VAR1").value("val").iteration(1).build()
                     ))
@@ -206,12 +210,13 @@ void convertCollectedVariables_shouldDoNothing_whenNoCollectedKeyAndLastSurveyUn
         }
 
         @Test
+        @DisplayName("Should add null variable if no COLLECTED in payload and last SU has the variable")
         void convertCollectedVariables_shouldAddNullVariable_whenNoCollectedKeyAndLastSurveyUnitHasSingleVariable() {
-            Map<String, Object> payload = new HashMap<>(); // pas de clé "COLLECTED"
+            Map<String, Object> payload = new HashMap<>();
 
             SurveyUnitModel lastSurveyUnit = SurveyUnitModel.builder()
                     .interrogationId("INT1")
-                    .state(DataState.COLLECTED) // même état que dataState attendu
+                    .state(DataState.COLLECTED)
                     .collectedVariables(List.of(
                             VariableModel.builder().varId("VAR1").value("oldVal").iteration(1).build()
                     ))
@@ -242,9 +247,17 @@ void convertCollectedVariables_shouldAddNullVariable_whenNoCollectedKeyAndLastSu
                     });
         }
 
-        @Test
-        void convertCollectedVariables_shouldAddNullVariablesForEachIteration_whenNoCollectedKeyAndLastSurveyUnitHasMultipleIterations() {
-            Map<String, Object> payload = new HashMap<>(); // pas de clé "COLLECTED"
+        @ParameterizedTest
+        @ValueSource(booleans = {false, true})
+        @DisplayName("Should add null iterations if variable or COLLECED key absent in payload and last SU has the variable with iterations")
+        void convertCollectedVariables_shouldAddNullVariablesForEachIteration_whenLastSurveyUnitHasMultipleIterations(
+                boolean isCollectedKeyPresentInPayload
+        ) {
+            Map<String, Object> payload = new HashMap<>();
+            if(isCollectedKeyPresentInPayload) {
+                Map<String, Object> dataMap = new HashMap<>();
+                payload.put("COLLECTED", dataMap);
+            }
 
             SurveyUnitModel lastSurveyUnit = SurveyUnitModel.builder()
                     .interrogationId("INT1")
@@ -280,14 +293,78 @@ void convertCollectedVariables_shouldAddNullVariablesForEachIteration_whenNoColl
                     .extracting(VariableModel::iteration)
                     .containsExactlyInAnyOrder(1, 2, 3);
         }
+
+        @Test
+        @DisplayName("Should add null iterations that are absent in payload")
+        void convertCollectedVariables_shouldAddNullIterations_whenLastSurveyUnitHasMultipleIterations() {
+            //GIVEN
+            String variableName = "VAR1";
+
+            Map<String, Object> dataMap = new HashMap<>();
+            List<String> newValues = new ArrayList<>();
+            newValues.add(null); //Null raw value on first iteration
+            newValues.addAll(List.of("new2", "new3"));
+            dataMap.put(variableName, Map.of("COLLECTED", newValues));
+            Map<String, Object> payload = new HashMap<>();
+            payload.put("COLLECTED", dataMap);
+
+            SurveyUnitModel lastSurveyUnit = SurveyUnitModel.builder()
+                    .interrogationId("INT1")
+                    .state(DataState.COLLECTED)
+                    .collectedVariables(List.of(
+                            VariableModel.builder().varId(variableName).value("old1").iteration(1).build(),
+                            VariableModel.builder().varId(variableName).value("old2").iteration(2).build(),
+                            VariableModel.builder().varId(variableName).value("old3").iteration(3).build()
+                    ))
+                    .build();
+
+            SurveyUnitModel dst = SurveyUnitModel.builder()
+                    .interrogationId("INT1")
+                    .collectedVariables(new ArrayList<>())
+                    .build();
+
+            //WHEN
+            rawDataConverterTestImpl.convertCollectedVariables(
+                    payload,
+                    "INT1",
+                    lastSurveyUnit,
+                    dst,
+                    DataState.COLLECTED,
+                    RawDataModelType.FILIERE,
+                    variablesMap
+            );
+
+            assertThat(dst.getCollectedVariables())
+                    .hasSize(3)
+                    .extracting(VariableModel::iteration)
+                    .containsExactlyInAnyOrder(1, 2, 3);
+
+            //First iteration present and null
+            assertThat(dst.getCollectedVariables())
+                    .filteredOn(variableModel -> variableModel.varId().equals(variableName)
+                            && variableModel.iteration().equals(1))
+                    .extracting(VariableModel::value)
+                    .containsOnlyNulls();
+
+            //Other iterations
+            assertThat(dst.getCollectedVariables())
+                    .filteredOn(variableModel ->
+                            variableModel.iteration().equals(2) || variableModel.iteration().equals(3))
+                    .hasSize(2)
+                    .allSatisfy(v -> {
+                        assertThat(v.varId()).isEqualTo("VAR1");
+                    })
+                    .extracting(VariableModel::value)
+                    .containsExactlyInAnyOrder("new2","new3");
+        }
     }
 
     @DisplayName("convertExternalVariables tests")
     @Nested
     class convertExternalVariablesTests {
         @Test
         void convertExternalVariables_shouldDoNothing_whenNoExternalKeyAndNoLastSurveyUnit() {
-            Map<String, Object> payload = new HashMap<>(); // pas de clé "EXTERNAL"
+            Map<String, Object> payload = new HashMap<>();
 
             SurveyUnitModel dst = SurveyUnitModel.builder()
                     .interrogationId("INT1")
diff --git a/src/test/java/fr/insee/genesis/domain/service/surveyunit/SurveyUnitServiceTest.java b/src/test/java/fr/insee/genesis/domain/service/surveyunit/SurveyUnitServiceTest.java
