Merge pull request #56 from InseeFr/quality

Quality
InseeFr · May 13, 2024 · a119d66 · a119d66
2 parents 003b0e5 + d6d6828
commit a119d66
Show file tree

Hide file tree

Showing 10 changed files with 65 additions and 82 deletions.
diff --git a/pom.xml b/pom.xml
@@ -31,10 +31,6 @@
         <jackson.version>2.17.1</jackson.version>
     </properties>
     <dependencies>
-        <dependency>
-            <groupId>org.springframework.boot</groupId>
-            <artifactId>spring-boot-starter-oauth2-resource-server</artifactId>
-        </dependency>
         <dependency>
             <groupId>org.springframework.boot</groupId>
             <artifactId>spring-boot-starter-security</artifactId>
@@ -43,10 +39,6 @@
             <groupId>org.springframework.boot</groupId>
             <artifactId>spring-boot-starter-web</artifactId>
         </dependency>
-        <dependency>
-            <groupId>org.springframework.boot</groupId>
-            <artifactId>spring-boot-starter-actuator</artifactId>
-        </dependency>
         <dependency>
             <groupId>org.springframework.boot</groupId>
             <artifactId>spring-boot-starter-data-mongodb</artifactId>

diff --git a/src/main/java/fr/insee/genesis/controller/rest/ResponseController.java b/src/main/java/fr/insee/genesis/controller/rest/ResponseController.java
@@ -25,7 +25,6 @@
 import io.swagger.v3.oas.annotations.Operation;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.stereotype.Controller;
 import org.springframework.web.bind.annotation.DeleteMapping;
@@ -277,6 +276,7 @@ private void treatCampaignWithMode(String campaignName, Mode mode, List<GenesisE
         if (dataFiles.isEmpty()) {
             errors.add(new NoDataError("No data file found", Mode.getEnumFromModeName(mode.getModeName())));
             log.info("No data file found in folder {}", dataFolder);
+            return;
         }
         VariablesMap variablesMap;
         try {

diff --git a/src/main/java/fr/insee/genesis/controller/sources/ddi/DDIReader.java b/src/main/java/fr/insee/genesis/controller/sources/ddi/DDIReader.java
@@ -78,7 +78,7 @@ private static VariablesMap readVariables(Path variablesFilePath)
 
                     // Store the group
                     Group group;
-                    if (parentGroupName == null || StringUtils.isEmpty(parentGroupName)) {
+                    if (!groupElement.hasAttribute("parent") || StringUtils.isEmpty(parentGroupName)) {
                         rootGroupName = groupName;
                         group = variablesMap.getRootGroup();
 
@@ -186,8 +186,9 @@ private static String getFirstChildValue(Element variableElement, String childTa
 
     private static Node getFirstChildNode(Element variableElement, String childTagName) {
         NodeList children = variableElement.getElementsByTagName(childTagName);
-        if (children == null)
+        if (children.getLength() == 0){
             return null;
+        }
         return children.item(0);
     }
 

diff --git a/src/main/java/fr/insee/genesis/controller/sources/xml/LunaticXmlDataSequentialParser.java b/src/main/java/fr/insee/genesis/controller/sources/xml/LunaticXmlDataSequentialParser.java
@@ -6,6 +6,7 @@
 import javax.xml.stream.XMLEventReader;
 import javax.xml.stream.XMLInputFactory;
 import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.events.Attribute;
 import javax.xml.stream.events.StartElement;
 import javax.xml.stream.events.XMLEvent;
 import java.io.IOException;
@@ -125,7 +126,6 @@ private LunaticXmlSurveyUnit parseSurveyUnit(final XMLEventReader reader) throws
                 }
             }
         }
-
         return xmlSurveyUnit;
     }
 
@@ -138,13 +138,10 @@ private List<LunaticXmlCollectedData> readCollected(XMLEventReader reader) throw
                 return lunaticXmlCollectedDataList;
             }
 
-
             if(event.isStartElement()){
                 final StartElement element = event.asStartElement();
                 final String variableName = element.getName().getLocalPart();
-
                 LunaticXmlCollectedData variable = readNextCollectedVariable(reader, variableName);
-
                 lunaticXmlCollectedDataList.add(variable);
             }
         }
@@ -168,16 +165,11 @@ private LunaticXmlCollectedData readNextCollectedVariable(XMLEventReader reader,
                 final String stateName = element.getName().getLocalPart();
 
                 List<ValueType> values = new ArrayList<>();
-                if(element.isStartElement()
-                        && event.asStartElement().getName().getLocalPart().equals(stateName)
-                        && event.asStartElement().getAttributeByName(new QName("type")) != null
-                ){
+                if(element.isStartElement() && getType(element) != null && !getType(element).getValue().equals("null")){
                     //If only 1 value (determined by presence of type)
-                    String type = element.getAttributeByName(new QName("type")).getValue();
-                    if(!type.equals("null")){
-                        String value = reader.getElementText();
-                        values.add(new ValueType(value, type));
-                    }
+                    String type = getType(element).getValue();
+                    String value = reader.getElementText();
+                    values.add(new ValueType(value, type));
                 }else{
                     //If multiple value
                     values = readValues(reader, stateName);
@@ -207,10 +199,13 @@ private LunaticXmlCollectedData readNextCollectedVariable(XMLEventReader reader,
         return variable;
     }
 
+	private static Attribute getType(StartElement element) {
+		return element.getAttributeByName(new QName("type"));
+	}
+
     private List<ValueType> readValues(XMLEventReader reader, String stateName) throws XMLStreamException {
         List<ValueType> values = new ArrayList<>();
 
-
         while(reader.hasNext()){
             final XMLEvent event = reader.nextEvent();
 
@@ -222,7 +217,7 @@ private List<ValueType> readValues(XMLEventReader reader, String stateName) thro
             if(event.isStartElement()) {
                 final StartElement element = event.asStartElement();
 
-                String type = element.getAttributeByName(new QName("type")).getValue();
+                String type = getType(element).getValue();
                 String value = reader.getElementText();
 
                 values.add(new ValueType(value, type));
@@ -251,7 +246,7 @@ private List<LunaticXmlOtherData> readCalculatedOrExternal(XMLEventReader reader
                 LunaticXmlOtherData variable = new LunaticXmlOtherData();
                 variable.setVariableName(variableName);
 
-                String type = element.getAttributeByName(new QName("type")).getValue();
+                String type = getType(element).getValue();
                 String value = reader.getElementText();
 
                 List<ValueType> values = new ArrayList<>();

diff --git a/src/main/java/fr/insee/genesis/controller/utils/SaxonTransformer.java b/src/main/java/fr/insee/genesis/controller/utils/SaxonTransformer.java
@@ -1,18 +1,17 @@
 package fr.insee.genesis.controller.utils;
 
-import java.io.IOException;
-import java.io.InputStream;
-import java.net.URL;
-import java.nio.file.Path;
+import lombok.extern.slf4j.Slf4j;
 
 import javax.xml.transform.Transformer;
 import javax.xml.transform.TransformerConfigurationException;
 import javax.xml.transform.TransformerException;
 import javax.xml.transform.TransformerFactory;
 import javax.xml.transform.stream.StreamResult;
 import javax.xml.transform.stream.StreamSource;
-
-import lombok.extern.slf4j.Slf4j;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URL;
+import java.nio.file.Path;
 
 @Slf4j
 public class SaxonTransformer {
@@ -34,10 +33,8 @@ public void xslTransform(URL inputXmlURL, String inputXslPath, Path outputXmlPat
         }
 
         // Get the XSL file
-        StreamSource xslSource;
-        InputStream xslInput;
-        xslInput = SaxonTransformer.class.getClassLoader().getResourceAsStream(inputXslPath);
-        xslSource = new StreamSource(xslInput);
+        InputStream xslInput = SaxonTransformer.class.getClassLoader().getResourceAsStream(inputXslPath);
+        StreamSource xslSource = new StreamSource(xslInput);
         xslSource.setSystemId(inputXslPath);
 
         // Instantiation of the XSL transformer factory
@@ -56,8 +53,10 @@ public void xslTransform(URL inputXmlURL, String inputXslPath, Path outputXmlPat
 
         try {
             xmlInput.close();
-            xslInput.close();
-        } catch (IOException e) {
+			if (xslInput != null) {
+				xslInput.close();
+			}
+		} catch (IOException e) {
             log.error("IOException occurred when trying to close the streams after XSL transformation.", e);
         }
 

diff --git a/src/main/java/fr/insee/genesis/domain/dtos/Mode.java b/src/main/java/fr/insee/genesis/domain/dtos/Mode.java
@@ -1,8 +1,10 @@
 package fr.insee.genesis.domain.dtos;
 
 import io.swagger.v3.oas.annotations.media.Schema;
+import lombok.Getter;
 import org.springframework.lang.Nullable;
 
+@Getter
 public enum Mode {
 
 	WEB("WEB", "WEB"),TEL("TEL", "ENQ"),F2F("F2F", "ENQ"),OTHER("OTHER", ""),PAPER("PAPER", "");
@@ -13,25 +15,21 @@ public enum Mode {
 
 	private final String folder;
 
-	Mode(String modeName, String folder) {
+	Mode(@Nullable String modeName, String folder) {
 		this.modeName = modeName;
 		this.folder = folder;
 	}
 
 	public static Mode getEnumFromModeName(String modeName) {
+		if (modeName == null){
+			return null;
+		}
 		for (Mode mode : Mode.values()) {
-			if (mode.getModeName().equals(modeName)) {
+			if (modeName.equals(mode.getModeName())) {
 				return mode;
 			}
 		}
 		return null;
 	}
 
-	public String getModeName() {
-		return modeName;
-	}
-
-	public String getFolder() {
-		return folder;
-	}
 }
diff --git a/src/main/java/fr/insee/genesis/domain/ports/api/ScheduleApiPort.java b/src/main/java/fr/insee/genesis/domain/ports/api/ScheduleApiPort.java
@@ -1,10 +1,9 @@
 package fr.insee.genesis.domain.ports.api;
 
-import fr.insee.genesis.exceptions.GenesisException;
 import fr.insee.genesis.exceptions.InvalidCronExpressionException;
 import fr.insee.genesis.exceptions.NotFoundException;
-import fr.insee.genesis.infrastructure.model.document.schedule.StoredSurveySchedule;
 import fr.insee.genesis.infrastructure.model.document.schedule.ServiceToCall;
+import fr.insee.genesis.infrastructure.model.document.schedule.StoredSurveySchedule;
 
 import java.time.LocalDateTime;
 import java.util.List;

diff --git a/src/main/java/fr/insee/genesis/domain/service/SurveyUnitUpdateImpl.java b/src/main/java/fr/insee/genesis/domain/service/SurveyUnitUpdateImpl.java
@@ -1,6 +1,5 @@
 package fr.insee.genesis.domain.service;
 
-import fr.insee.genesis.domain.dtos.SurveyUnitId;
 import fr.insee.genesis.domain.dtos.*;
 import fr.insee.genesis.domain.ports.api.SurveyUnitUpdateApiPort;
 import fr.insee.genesis.domain.ports.spi.SurveyUnitUpdatePersistencePort;
@@ -42,28 +41,30 @@ public Stream<SurveyUnitUpdateDto> findByIdQuestionnaire(String idQuestionnaire)
         return surveyUnitUpdatePersistencePort.findByIdQuestionnaire(idQuestionnaire);
     }
 
+    /**
+     * In this method we want to get the latest update for each variable of a survey unit
+     * But we need to separate the updates by mode
+     * So we will calculate the latest state for a given collection mode
+     * @param idUE
+     * @param idQuest
+     * @return
+     */
     @Override
     public List<SurveyUnitUpdateDto> findLatestByIdAndByMode(String idUE, String idQuest) {
-        //In this method we want to get the latest update for each variable of a survey unit
-        //But we need to separate the updates by mode
-        //So we will calculate the latest state for a given collection mode
         List<SurveyUnitUpdateDto> latestUpdatesbyVariables = new ArrayList<>();
         List<SurveyUnitUpdateDto> surveyUnitUpdateDtos = surveyUnitUpdatePersistencePort.findByIds(idUE, idQuest);
         List<Mode> modes = getDistinctsModes(surveyUnitUpdateDtos);
         modes.forEach(mode ->{
-            List<SurveyUnitUpdateDto> suByMode = new ArrayList<>();
-            surveyUnitUpdateDtos.forEach(surveyUnitUpdateDto -> {
-                if (surveyUnitUpdateDto.getMode().equals(mode)){
-                    suByMode.add(surveyUnitUpdateDto);
-                }
-            });
-            //Sorting update by date (lastest updates first by date of upload in database)
-            suByMode.sort((o1, o2) -> o2.getRecordDate().compareTo(o1.getRecordDate()));
+            List<SurveyUnitUpdateDto> suByMode = surveyUnitUpdateDtos.stream()
+                    .filter(surveyUnitUpdateDto -> surveyUnitUpdateDto.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
-            latestUpdatesbyVariables.add(suByMode.get(0));
+            latestUpdatesbyVariables.add(suByMode.getFirst());
             //We keep the name of already added variables to skip them in older updates
             List<String> addedVariables = new ArrayList<>();
-            SurveyUnitUpdateDto latestUpdate = suByMode.get(0);
+            SurveyUnitUpdateDto latestUpdate = suByMode.getFirst();
 
             latestUpdate.getCollectedVariables().forEach(variableStateDto -> addedVariables.add(variableStateDto.getIdVar()));
             latestUpdate.getExternalVariables().forEach(externalVariableDto -> addedVariables.add(externalVariableDto.getIdVar()));
@@ -72,19 +73,19 @@ public List<SurveyUnitUpdateDto> findLatestByIdAndByMode(String idUE, String idQ
                 List<CollectedVariableDto> variablesToKeep = new ArrayList<>();
                 List<VariableDto> externalToKeep = new ArrayList<>();
                 // We iterate over the variables of the update and add them to the list if they are not already added
-                surveyUnitUpdateDto.getCollectedVariables().forEach(variableStateDto -> {
-                    if (!addedVariables.contains(variableStateDto.getIdVar())){
-                        variablesToKeep.add(variableStateDto);
-                        addedVariables.add(variableStateDto.getIdVar());
-                    }
-                });
+                surveyUnitUpdateDto.getCollectedVariables().stream()
+                        .filter(variableStateDto -> !addedVariables.contains(variableStateDto.getIdVar()))
+                        .forEach(variableStateDto -> {
+                           variablesToKeep.add(variableStateDto);
+                           addedVariables.add(variableStateDto.getIdVar());
+                        });
                 if (surveyUnitUpdateDto.getExternalVariables() != null){
-                    surveyUnitUpdateDto.getExternalVariables().forEach(externalVariableDto -> {
-                        if (!addedVariables.contains(externalVariableDto.getIdVar())) {
+                    surveyUnitUpdateDto.getExternalVariables().stream()
+                         .filter(externalVariableDto -> !addedVariables.contains(externalVariableDto.getIdVar()))
+                         .forEach(externalVariableDto -> {
                             externalToKeep.add(externalVariableDto);
                             addedVariables.add(externalVariableDto.getIdVar());
-                        }
-                    });
+                         });
                 }
 
                 // If there are new variables, we add the update to the list of latest updates

diff --git a/src/main/java/fr/insee/genesis/exceptions/GenesisException.java b/src/main/java/fr/insee/genesis/exceptions/GenesisException.java
@@ -3,18 +3,20 @@
 import lombok.AllArgsConstructor;
 import lombok.Getter;
 
+import java.io.Serial;
+
+@Getter
 @AllArgsConstructor
 public class GenesisException extends Exception{
 
     /**
 	 * 
 	 */
+	@Serial
 	private static final long serialVersionUID = 3356078796351491095L;
 
-	@Getter
-    private final int status;
+	private final int status;
 
-    @Getter
     private final String message;
 
 }
diff --git a/src/test/java/fr/insee/genesis/controller/adapter/LunaticXmlAdapterTest.java b/src/test/java/fr/insee/genesis/controller/adapter/LunaticXmlAdapterTest.java
@@ -5,11 +5,7 @@
 import fr.insee.genesis.controller.sources.ddi.Variable;
 import fr.insee.genesis.controller.sources.ddi.VariableType;
 import fr.insee.genesis.controller.sources.ddi.VariablesMap;
-import fr.insee.genesis.controller.sources.xml.LunaticXmlCollectedData;
-import fr.insee.genesis.controller.sources.xml.LunaticXmlData;
-import fr.insee.genesis.controller.sources.xml.LunaticXmlOtherData;
-import fr.insee.genesis.controller.sources.xml.LunaticXmlSurveyUnit;
-import fr.insee.genesis.controller.sources.xml.ValueType;
+import fr.insee.genesis.controller.sources.xml.*;
 import fr.insee.genesis.domain.dtos.CollectedVariableDto;
 import fr.insee.genesis.domain.dtos.DataState;
 import fr.insee.genesis.domain.dtos.Mode;