fix(hotfix no information about minio objects reported in error) WIP

- Modify exception message to log more information - Adapt exception handler to process these exceptions - Start writing test
InseeFr · Dec 12, 2024 · bee1204 · bee1204
1 parent 3aa58f6
commit bee1204
Show file tree

Hide file tree

Showing 7 changed files with 135 additions and 22 deletions.
diff --git a/src/main/java/fr/insee/rmes/bauhaus_services/FileSystemOperation.java b/src/main/java/fr/insee/rmes/bauhaus_services/FileSystemOperation.java
@@ -22,7 +22,7 @@ public void delete(String path) {
         try {
             Files.delete(Paths.get(path));
         } catch (IOException e) {
-            throw new RmesFileException("Failed to delete file: " + path, e);
+            throw new RmesFileException(path, "Failed to delete file: " + path, e);
         }
     }
 
@@ -31,7 +31,7 @@ public InputStream read(String fileName) {
         try {
             return Files.newInputStream(Paths.get(config.getDocumentsStorageGestion()).resolve(fileName));
         } catch (IOException e) {
-            throw new RmesFileException("Failed to read file: " + fileName, e);
+            throw new RmesFileException(fileName, "Failed to read file: " + fileName, e);
         }
     }
 
@@ -40,7 +40,7 @@ public void write(InputStream content, Path destPath) {
         try {
             Files.copy(content, destPath, StandardCopyOption.REPLACE_EXISTING);
         } catch (IOException e) {
-            throw new RmesFileException("Failed to write file: " + destPath, e);
+            throw new RmesFileException(destPath.toString(),"Failed to write file: " + destPath, e);
         }
     }
 
@@ -51,7 +51,7 @@ public void copy(String srcPath, String destPath)  {
         try {
             Files.copy(file, targetPath.resolve(file.getFileName()), StandardCopyOption.REPLACE_EXISTING);
         } catch (IOException e) {
-            throw new RmesFileException("Failed to copy file : " + srcPath + " to " + destPath, e);
+            throw new RmesFileException(srcPath, "Failed to copy file : " + srcPath + " to " + destPath, e);
         }
     }
 

diff --git a/src/main/java/fr/insee/rmes/bauhaus_services/MinioFilesOperation.java b/src/main/java/fr/insee/rmes/bauhaus_services/MinioFilesOperation.java
@@ -3,8 +3,6 @@
 import fr.insee.rmes.exceptions.RmesFileException;
 import io.minio.*;
 import io.minio.errors.*;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -14,19 +12,18 @@
 
 public record MinioFilesOperation(MinioClient minioClient, String bucketName, String directoryGestion, String directoryPublication) implements FilesOperations {
 
-    private static final Logger logger = LoggerFactory.getLogger(MinioFilesOperation.class);
-
     @Override
     public InputStream read(String pathFile){
-        String objectName= extractFileName(pathFile);
+        String fileName= extractFileName(pathFile);
+        String objectName = directoryGestion + "/" + fileName;
 
         try {
             return minioClient.getObject(GetObjectArgs.builder()
                     .bucket(bucketName)
-                    .object(directoryGestion +"/"+ objectName)
+                    .object(objectName)
                     .build());
         } catch (MinioException | InvalidKeyException | IOException | NoSuchAlgorithmException e) {
-            throw new RmesFileException("Error reading file: " + objectName, e);
+            throw new RmesFileException(fileName, "Error reading file: " + fileName+" as object `"+objectName+"` in bucket "+bucketName, e);
         }
     }
     private static String extractFileName(String filePath) {
@@ -38,36 +35,41 @@ private static String extractFileName(String filePath) {
 
 
     @Override
-    public void write(InputStream content, Path objectName) {
+    public void write(InputStream content, Path filePath) {
+        String filename = filePath.getFileName().toString();
+        String objectName = directoryGestion + "/" + filename;
         try {
             minioClient.putObject(PutObjectArgs.builder()
                     .bucket(bucketName)
-                    .object(directoryGestion +"/"+ objectName.getFileName().toString())
+                    .object(objectName)
                     .stream(content, content.available(), -1)
                     .build());
         } catch (IOException | ErrorResponseException | InsufficientDataException | InternalException |
                  InvalidKeyException | InvalidResponseException | NoSuchAlgorithmException | ServerException |
                  XmlParserException e) {
-            throw new RmesFileException("Error writing file: " + objectName, e);
+            throw new RmesFileException(filePath.toString(), "Error writing file: " + filename+ "as object `"+objectName+"` in bucket "+bucketName, e);
         }
     }
 
     @Override
     public void copy(String srcObjectName, String destObjectName)  {
 
+        String srcObject = directoryGestion + "/" + extractFileName(srcObjectName);
+        String destObject = directoryPublication + "/" + extractFileName(srcObjectName);
+
         try {
             CopySource source = CopySource.builder()
                     .bucket(bucketName)
-                    .object(directoryGestion +"/"+ extractFileName(srcObjectName))
+                    .object(srcObject)
                     .build();
 
             minioClient.copyObject(CopyObjectArgs.builder()
                     .bucket(bucketName)
-                    .object(directoryPublication +"/"+ extractFileName(srcObjectName))
+                    .object(destObject)
                     .source(source)
                     .build());
         } catch (MinioException | InvalidKeyException | IOException | NoSuchAlgorithmException e) {
-            throw new RmesFileException("Error copying file from " + srcObjectName + " to " + destObjectName, e);
+            throw new RmesFileException(srcObjectName,"Error copying file from `" + srcObject + "` to `" + destObject+"` in bucket "+bucketName, e);
         }
     }
 
@@ -85,7 +87,7 @@ public void delete(String objectName) {
                     .object(objectName)
                     .build());
         } catch (MinioException | InvalidKeyException | IOException | NoSuchAlgorithmException e) {
-            throw new RmesFileException("Error deleting file: " + objectName, e);
+            throw new RmesFileException(objectName,"Error deleting file: " + objectName+" in bucket "+bucketName, e);
         }
     }
 

diff --git a/...va/fr/insee/rmes/bauhaus_services/operations/documentations/documents/DocumentsUtils.java b/...va/fr/insee/rmes/bauhaus_services/operations/documentations/documents/DocumentsUtils.java
@@ -655,7 +655,7 @@ public Document buildDocumentFromJson(JSONObject jsonDoc) {
         return doc;
     }
 
-    private String getDocumentFilename(String id) throws RmesException {
+    protected String getDocumentFilename(String id) throws RmesException {
         JSONObject jsonDoc = getDocument(id, false);
         String url = getDocumentUrlFromDocument(jsonDoc);
         return getDocumentNameFromUrl(url);

diff --git a/src/main/java/fr/insee/rmes/exceptions/RmesExceptionHandler.java b/src/main/java/fr/insee/rmes/exceptions/RmesExceptionHandler.java
@@ -32,6 +32,12 @@ public final ResponseEntity<String> handleSubclassesOfRmesException(RmesExceptio
         return ResponseEntity.status(exception.getStatus()).body(exception.getDetails());
     }
 
+    @ExceptionHandler(RmesFileException.class)
+    public final ResponseEntity<String> handleRmesFileException(RmesFileException exception){
+        logger.error(exception.getMessage(), exception);
+        return ResponseEntity.internalServerError().body(exception.toString());
+    }
+
     @ExceptionHandler(RmesException.class)
     public final ResponseEntity<String> handleRmesException(RmesException exception){
         logger.error(exception.getMessageAndDetails(), exception);

diff --git a/src/main/java/fr/insee/rmes/exceptions/RmesFileException.java b/src/main/java/fr/insee/rmes/exceptions/RmesFileException.java
@@ -1,7 +1,22 @@
 package fr.insee.rmes.exceptions;
 
 public class RmesFileException extends RuntimeException {
-    public RmesFileException(String message, Throwable cause) {
+
+    private final String fileName;
+
+    public RmesFileException(String filename, String message, Throwable cause) {
         super(message, cause);
+        this.fileName = filename;
+    }
+
+    public String getFileName() {
+        return fileName;
+    }
+
+    @Override
+    public String toString() {
+        return "RmesFileException{" +
+                "fileName='" + fileName + '\'' +
+                '}';
     }
 }
diff --git a/src/test/java/fr/insee/rmes/integration/TestDocumentsResourcesWithMinio.java b/src/test/java/fr/insee/rmes/integration/TestDocumentsResourcesWithMinio.java
@@ -0,0 +1,91 @@
+package fr.insee.rmes.integration;
+
+import fr.insee.rmes.bauhaus_services.FilesOperations;
+import fr.insee.rmes.bauhaus_services.operations.ParentUtils;
+import fr.insee.rmes.bauhaus_services.operations.documentations.documents.DocumentsImpl;
+import fr.insee.rmes.bauhaus_services.operations.documentations.documents.DocumentsUtils;
+import fr.insee.rmes.bauhaus_services.rdf_utils.PublicationUtils;
+import fr.insee.rmes.bauhaus_services.rdf_utils.RepositoryGestion;
+import fr.insee.rmes.bauhaus_services.rdf_utils.RepositoryPublication;
+import fr.insee.rmes.bauhaus_services.stamps.StampsRestrictionServiceImpl;
+import fr.insee.rmes.config.BaseConfigForMvcTests;
+import fr.insee.rmes.config.Config;
+import fr.insee.rmes.exceptions.RmesFileException;
+import fr.insee.rmes.utils.IdGenerator;
+import fr.insee.rmes.webservice.operations.DocumentsResources;
+import io.minio.errors.MinioException;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
+import org.springframework.boot.test.context.TestConfiguration;
+import org.springframework.boot.test.mock.mockito.MockBean;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Import;
+import org.springframework.test.web.servlet.MockMvc;
+import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
+
+import static org.hamcrest.Matchers.containsString;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.when;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
+
+@WebMvcTest(DocumentsResources.class)
+@Import({BaseConfigForMvcTests.class, DocumentsImpl.class})
+class TestDocumentsResourcesWithMinio {
+
+    @Autowired
+    private MockMvc mockMvc;
+
+//    @MockBean
+//    private ParentUtils parentUtils;
+//    @MockBean
+//    private RepositoryPublication repositoryPublication;
+//    @MockBean
+//    private StampsRestrictionServiceImpl stampsRestrictionService;
+//    @MockBean
+//    private IdGenerator idGenerator;
+//    @MockBean
+//    private PublicationUtils publicationUtils;
+//    @MockBean
+//    private Config config;
+//    @MockBean
+//    private RepositoryGestion repositoryGestion;
+
+    @MockBean
+    FilesOperations filesOperations;
+
+    private final String fichierId="ID";
+
+    private static final String nomFichier = "nomFichier";
+
+    @Test
+    void shouldLogAndReturnInternalException_WhenErrorOccursInMinio() throws Exception {
+
+        String objectName = "directoryGestion/"+nomFichier;
+        String bucketName = "metadata_bucket";
+        when(filesOperations.read(anyString())).thenThrow(new RmesFileException(nomFichier, "Error reading file: " + nomFichier+
+                " as object `"+objectName+"` in bucket "+bucketName, new MinioException()));
+
+
+        mockMvc.perform(MockMvcRequestBuilders.get("/documents/document/" + fichierId + "/file"))
+                .andExpect(status().isInternalServerError())
+                .andExpect(content().string(containsString("fileName='" + nomFichier + "'")));
+    }
+
+    @TestConfiguration
+    static class ConfigurationForTest{
+
+        @Bean
+        public DocumentsUtils documentsUtils(FilesOperations filesOperations) {
+            return new DocumentsUtils(null, filesOperations){
+                @Override
+                protected String getDocumentFilename(String id){
+                    return nomFichier;
+                }
+            };
+        }
+
+    }
+
+}
diff --git a/src/test/java/fr/insee/rmes/webservice/DocumentsResourcesTest.java b/src/test/java/fr/insee/rmes/webservice/DocumentsResourcesTest.java
@@ -15,7 +15,6 @@
 import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
 
 import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.not;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.when;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
@@ -47,6 +46,6 @@ void shouldReturnInternalException() throws Exception {
 
         mockMvc.perform(MockMvcRequestBuilders.get("/documents/document/nomFichier/file"))
                 .andExpect(status().isInternalServerError())
-                .andExpect(content().string(not(containsString("nomFichier"))));
+                .andExpect(content().string(containsString("fileName='nomFichier'")));
     }
 }