Swallow exceptions in getCases() when some cases do not have case_metadata (#67)

etiennehomer · web-flow · commit b216754a3d9d · 2025-02-18T11:20:16.000+01:00
Signed-off-by: Etienne Homer &lt;etiennehomer@gmail.com&gt;
diff --git a/src/main/java/com/powsybl/caseserver/service/CaseService.java b/src/main/java/com/powsybl/caseserver/service/CaseService.java
@@ -49,7 +49,7 @@ default void validateCaseName(String caseName) {
     }
 
     default CaseMetadataEntity getCaseMetaDataEntity(UUID caseUuid) {
-        return getCaseMetadataRepository().findById(caseUuid).orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "case " + caseUuid + NOT_FOUND));
+        return getCaseMetadataRepository().findById(caseUuid).orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Metadata of case " + caseUuid + NOT_FOUND));
     }
 
     default Boolean isUploadedAsPlainFile(UUID caseUuid) {
diff --git a/src/main/java/com/powsybl/caseserver/service/FsCaseService.java b/src/main/java/com/powsybl/caseserver/service/FsCaseService.java
@@ -100,20 +100,25 @@ String getFormat(Path caseFile) {
     public List<CaseInfos> getCases() {
         try (Stream<Path> walk = Files.walk(getStorageRootDir())) {
             return walk.filter(Files::isRegularFile)
-                    .map(this::getCaseInfos)
+                    .map(this::getCaseInfoSafely)
                     .filter(Objects::nonNull)
-                    .map(this::removeGzipExtensionFromPlainFile)
                     .toList();
         } catch (IOException e) {
             throw new UncheckedIOException(e);
         }
     }
 
     private CaseInfos getCaseInfos(Path file) {
+        CaseInfos caseInfos = createInfos(file, UUID.fromString(file.getParent().getFileName().toString()));
+        return removeGzipExtensionFromPlainFile(caseInfos);
+    }
+
+    private CaseInfos getCaseInfoSafely(Path file) {
         Objects.requireNonNull(file);
         try {
-            return createInfos(file, UUID.fromString(file.getParent().getFileName().toString()));
+            return getCaseInfos(file);
         } catch (Exception e) {
+            // This method is called by getCases() that is a method for supervision and administration. We do not want the request to stop and fail on error cases.
             LOGGER.error("Error processing file {}: {}", file.getFileName(), e.getMessage(), e);
             return null;
         }
@@ -129,9 +134,6 @@ public String getCaseName(UUID caseUuid) {
         if (caseInfos == null) {
             throw CaseException.createFileNameNotFound(caseUuid);
         }
-        if (Boolean.TRUE.equals(isUploadedAsPlainFile(caseUuid))) {
-            return removeExtension(caseInfos.getName(), GZIP_EXTENSION);
-        }
         return caseInfos.getName();
     }
 
@@ -142,8 +144,7 @@ public CaseInfos getCaseInfos(UUID caseUuid) {
             LOGGER.error("The directory with the following uuid doesn't exist: {}", caseUuid);
             return null;
         }
-        CaseInfos caseInfos = getCaseInfos(file);
-        return this.removeGzipExtensionFromPlainFile(caseInfos);
+        return getCaseInfos(file);
     }
 
     public Path getCaseFile(UUID caseUuid) {
diff --git a/src/main/java/com/powsybl/caseserver/service/MetadataService.java b/src/main/java/com/powsybl/caseserver/service/MetadataService.java
@@ -31,7 +31,7 @@ public MetadataService(CaseMetadataRepository caseMetadataRepository) {
 
     @Transactional
     public void disableCaseExpiration(UUID caseUuid) {
-        CaseMetadataEntity caseMetadataEntity = caseMetadataRepository.findById(caseUuid).orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "case " + caseUuid + NOT_FOUND));
+        CaseMetadataEntity caseMetadataEntity = caseMetadataRepository.findById(caseUuid).orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Metadata of case " + caseUuid + NOT_FOUND));
         caseMetadataEntity.setExpirationDate(null);
     }
 }
diff --git a/src/main/java/com/powsybl/caseserver/service/S3CaseService.java b/src/main/java/com/powsybl/caseserver/service/S3CaseService.java
@@ -270,14 +270,25 @@ public List<CaseInfos> getCases() {
         List<CaseInfos> caseInfosList = new ArrayList<>();
         CaseInfos caseInfos;
         for (S3Object o : getCaseS3Objects(rootDirectory + DELIMITER)) {
-            caseInfos = getCaseInfos(parseUuidFromKey(o.key()));
+            caseInfos = getCaseInfoSafely(parseUuidFromKey(o.key()));
             if (Objects.nonNull(caseInfos)) {
                 caseInfosList.add(caseInfos);
             }
         }
         return caseInfosList;
     }
 
+    private CaseInfos getCaseInfoSafely(UUID uuid) {
+        Objects.requireNonNull(uuid);
+        try {
+            return getCaseInfos(uuid);
+        } catch (Exception e) {
+            // This method is called by getCases() that is a method for supervision and administration. We do not want the request to stop and fail on error cases.
+            LOGGER.error("Error processing case with uuid {}: {}", uuid, e.getMessage(), e);
+            return null;
+        }
+    }
+
     @Override
     public boolean caseExists(UUID uuid) {
         return !getCaseS3Objects(uuid).isEmpty();
diff --git a/src/test/java/com/powsybl/caseserver/service/AbstractCaseControllerTest.java b/src/test/java/com/powsybl/caseserver/service/AbstractCaseControllerTest.java
@@ -57,7 +57,7 @@
 @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.MOCK)
 @ContextConfigurationWithTestChannel
 abstract class AbstractCaseControllerTest {
-    private static final String TEST_CASE = "testCase.xiidm";
+    static final String TEST_CASE = "testCase.xiidm";
     private static final String TEST_TAR_CASE = "tarCase.tar";
     private static final String TEST_CASE_FORMAT = "XIIDM";
     private static final String NOT_A_NETWORK = "notANetwork.txt";
@@ -80,11 +80,11 @@ abstract class AbstractCaseControllerTest {
     OutputDestination outputDestination;
 
     @Autowired
-    private ObjectMapper mapper;
+    ObjectMapper mapper;
 
     FileSystem fileSystem;
 
-    private final String caseImportDestination = "case.import.destination";
+    final String caseImportDestination = "case.import.destination";
 
     @AfterEach
     public void tearDown() throws Exception {
@@ -93,7 +93,7 @@ public void tearDown() throws Exception {
         TestUtils.assertQueuesEmptyThenClear(destinations, outputDestination);
     }
 
-    private void createStorageDir() throws IOException {
+    void createStorageDir() throws IOException {
         Path path = fileSystem.getPath(caseService.getRootDirectory());
         if (!Files.exists(path)) {
             Files.createDirectories(path);
@@ -467,7 +467,7 @@ void testDuplicateNonIndexedCase() throws Exception {
         assertFalse(caseMetadataRepository.findById(duplicateCaseUuid).get().isIndexed());
     }
 
-    private UUID importCase(String testCase, Boolean withExpiration) throws Exception {
+    UUID importCase(String testCase, Boolean withExpiration) throws Exception {
         String importedCase;
         if (withExpiration) {
             importedCase = mvc.perform(multipart("/v1/cases")
@@ -722,11 +722,17 @@ private static String getDateSearchTerm(String entsoeFormatDate) {
     }
 
     @Test
-    void invalidFileInCaseDirectoryShouldBeIgnored() throws Exception {
+    void casesWithoutMetadataShouldBeIgnored() throws Exception {
         createStorageDir();
-        Path filePath = fileSystem.getPath(caseService.getRootDirectory()).resolve("randomFile.txt");
-        Files.createFile(filePath);
+
+        // add a case file in a UUID named directory but no metadata in the database
+        UUID caseUuid = importCase(TEST_CASE, false);
+        assertNotNull(outputDestination.receive(1000, caseImportDestination));
+        caseMetadataRepository.deleteById(caseUuid);
+
+        // import a case properly
         importCase(TEST_CASE, false);
+        assertNotNull(outputDestination.receive(1000, caseImportDestination));
 
         MvcResult mvcResult = mvc.perform(get("/v1/cases"))
                 .andExpect(status().isOk())
@@ -736,10 +742,9 @@ void invalidFileInCaseDirectoryShouldBeIgnored() throws Exception {
         assertEquals(1, caseInfos.size());
         assertEquals(TEST_CASE, caseInfos.get(0).getName());
 
-        Files.delete(filePath);
+        caseService.deleteCase(caseUuid);
         mvc.perform(delete("/v1/cases"))
                 .andExpect(status().isOk());
-        assertNotNull(outputDestination.receive(1000, caseImportDestination));
     }
 
     @Test
diff --git a/src/test/java/com/powsybl/caseserver/service/FsCaseControllerTest.java b/src/test/java/com/powsybl/caseserver/service/FsCaseControllerTest.java
@@ -6,18 +6,28 @@
  */
 package com.powsybl.caseserver.service;
 
+import com.fasterxml.jackson.core.type.TypeReference;
 import com.google.common.jimfs.Configuration;
 import com.google.common.jimfs.Jimfs;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
+import com.powsybl.caseserver.dto.CaseInfos;
 import com.powsybl.computation.ComputationManager;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.test.context.TestPropertySource;
+import org.springframework.test.web.servlet.MvcResult;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.List;
 
 /**
  * @author Ghazwa Rehili <ghazwa.rehili at rte-france.com>
@@ -44,4 +54,28 @@ void testStorageNotCreated() throws Exception {
         mvc.perform(delete("/v1/cases")).andExpect(status().isUnprocessableEntity());
     }
 
+    @Test
+    void invalidFileInCaseDirectoryShouldBeIgnored() throws Exception {
+        createStorageDir();
+
+        // add a random file in the storage, not stored in a UUID named directory
+        Path filePath = fileSystem.getPath(caseService.getRootDirectory()).resolve("randomFile.txt");
+        Files.createFile(filePath);
+
+        // import a case properly
+        importCase(TEST_CASE, false);
+
+        MvcResult mvcResult = mvc.perform(get("/v1/cases"))
+                .andExpect(status().isOk())
+                .andReturn();
+        String resultAsString = mvcResult.getResponse().getContentAsString();
+        List<CaseInfos> caseInfos = mapper.readValue(resultAsString, new TypeReference<>() { });
+        assertEquals(1, caseInfos.size());
+        assertEquals(TEST_CASE, caseInfos.get(0).getName());
+
+        Files.delete(filePath);
+        mvc.perform(delete("/v1/cases"))
+                .andExpect(status().isOk());
+        assertNotNull(outputDestination.receive(1000, caseImportDestination));
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -49,7 +49,7 @@ default void validateCaseName(String caseName) {`
`49`	`49`	`}`
`50`	`50`
`51`	`51`	`default CaseMetadataEntity getCaseMetaDataEntity(UUID caseUuid) {`
`52`		`- return getCaseMetadataRepository().findById(caseUuid).orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "case " + caseUuid + NOT_FOUND));`
	`52`	`+ return getCaseMetadataRepository().findById(caseUuid).orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Metadata of case " + caseUuid + NOT_FOUND));`
`53`	`53`	`}`
`54`	`54`
`55`	`55`	`default Boolean isUploadedAsPlainFile(UUID caseUuid) {`
Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@ public MetadataService(CaseMetadataRepository caseMetadataRepository) {`
`31`	`31`
`32`	`32`	`@Transactional`
`33`	`33`	`public void disableCaseExpiration(UUID caseUuid) {`
`34`		`- CaseMetadataEntity caseMetadataEntity = caseMetadataRepository.findById(caseUuid).orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "case " + caseUuid + NOT_FOUND));`
	`34`	`+ CaseMetadataEntity caseMetadataEntity = caseMetadataRepository.findById(caseUuid).orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Metadata of case " + caseUuid + NOT_FOUND));`
`35`	`35`	`caseMetadataEntity.setExpirationDate(null);`
`36`	`36`	`}`
`37`	`37`	`}`