Sonar code smells

.sonarlint/connectedMode.json

@@ -0,0 +1,4 @@
+{
+    "sonarQubeUri": "https://sonar.puzzle.ch",
+    "projectKey": "unilu-exam-feedback-tool"
+}

pom.xml

@@ -102,11 +102,8 @@
 					<goals>
 						<goal>compile</goal>
 						<goal>process-classes</goal>
-<!--						would be nice to skip those, but not possible with this plugin-->
-<!--						<goal>-Dcyclonedx.skip=true</goal>-->
-<!--						<goal>-Djacoco.skip=true</goal>-->
-<!--						<goal>-Dformatter.skip=true</goal>-->
-<!--						<goal>-Dproperties.skip=true</goal>-->
+<!--						would be nice to skip the following plugin executions because it increases the build time
+							those, but not possible with this plugin: cyclonedx, jacoco, formatter, properties-->
 					</goals>
 				</configuration>
 			</plugin>

src/main/java/ch/puzzle/eft/controller/ExamController.java

@@ -1,14 +1,14 @@
 package ch.puzzle.eft.controller;
 
 import ch.puzzle.eft.service.ExamService;
-import jakarta.servlet.http.HttpServletResponse;
 import org.springframework.core.io.FileSystemResource;
+import org.springframework.http.HttpHeaders;
 import org.springframework.http.MediaType;
 import org.springframework.http.ResponseEntity;
 import org.springframework.web.bind.annotation.*;
 
+import java.io.ByteArrayOutputStream;
 import java.io.File;
-import java.io.IOException;
 
 @RestController
 @RequestMapping("/exams")
@@ -20,18 +20,24 @@ public ExamController(ExamService examFileService) {
     }
 
     @GetMapping("/download-zip/{examNumber}")
-    public ResponseEntity<?> downloadSubject(@PathVariable("examNumber") String examNumber, HttpServletResponse response) throws IOException {
-        response.setHeader("Content-Disposition", "attachment; filename=" + examNumber + ".zip");
-        examFileService.convertSelectedFilesToZip(examNumber, response.getOutputStream());
+    public ResponseEntity<byte[]> downloadSubject(@PathVariable("examNumber") String examNumber) {
+        HttpHeaders responseHeaders = new HttpHeaders();
+        responseHeaders.set("Content-Disposition", "attachment; filename=" + examNumber + ".zip");
+        ByteArrayOutputStream byteArrayOutputStream = examFileService.convertSelectedFilesToZip(examNumber);
         return ResponseEntity.ok()
-                             .build();
+                             .headers(responseHeaders)
+                             .body(byteArrayOutputStream.toByteArray());
     }
 
     @GetMapping(value = "/download/{subject}/{fileName}", produces = MediaType.APPLICATION_PDF_VALUE)
     @ResponseBody
-    public ResponseEntity<FileSystemResource> downloadFile(@PathVariable("subject") String subject, @PathVariable("fileName") String fileName, HttpServletResponse response) {
+    public ResponseEntity<FileSystemResource> downloadFile(@PathVariable("subject") String subject, @PathVariable("fileName") String fileName) {
         File examFile = examFileService.getFileToDownload(subject, fileName);
-        response.setHeader("Content-Disposition", "attachment; filename=" + subject + ".pdf");
-        return ResponseEntity.ok(new FileSystemResource(examFile));
+        HttpHeaders responseHeaders = new HttpHeaders();
+
+        responseHeaders.set("Content-Disposition", "attachment; filename=" + subject + ".pdf");
+        return ResponseEntity.ok()
+                             .headers(responseHeaders)
+                             .body(new FileSystemResource(examFile));
     }
 }

src/main/java/ch/puzzle/eft/controller/ExceptionController.java

@@ -12,40 +12,39 @@
 import org.springframework.web.bind.annotation.ControllerAdvice;
 import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.bind.annotation.GetMapping;
-import org.springframework.web.bind.annotation.SessionAttributes;
 import org.springframework.web.servlet.resource.NoResourceFoundException;
 
 @ControllerAdvice
 @Controller
-@SessionAttributes("errorModel")
 public class ExceptionController implements ErrorController {
 
     private static final Logger logger = LoggerFactory.getLogger(ExceptionController.class);
+    private static final String ERROR_MODEL = "errorModel";
 
     @ExceptionHandler(NoResourceFoundException.class)
     public String handleNotFound(Model model, HttpSession session) {
-        session.setAttribute("errorModel", new ErrorModel("404", "Resource not found"));
+        session.setAttribute(ERROR_MODEL, new ErrorModel("404", "Resource not found"));
         return "redirect:/error";
     }
 
     @ExceptionHandler(Exception.class)
-    public String handleInternalServerError(Model model, HttpServletRequest req, Exception ex, HttpSession session) {
-        logger.warn("Request URL: {} raised an {}",
-                    req.getRequestURL(),
-                    ex.getClass()
-                      .getSimpleName());
-        session.setAttribute("errorModel", new ErrorModel("unknown", ex.getMessage()));
+    public String handleInternalServerError(HttpServletRequest req, Exception ex, HttpSession session) {
+        logger.error("Request URL: {} raised an {}",
+                     req.getRequestURL(),
+                     ex.getClass()
+                       .getSimpleName());
+        session.setAttribute(ERROR_MODEL, new ErrorModel("unknown", ex.getMessage()));
         return "redirect:/error";
     }
 
     @GetMapping("/error")
-    public String viewErrorPage(Model model) {
+    public String viewErrorPage() {
         return "error";
     }
 
     @GetMapping("/return")
     public String viewCompleteErrorPage(Model model, HttpSession session) {
-        session.removeAttribute("errorModel");
+        session.removeAttribute(ERROR_MODEL);
         return "redirect:/";
     }
 }

src/main/java/ch/puzzle/eft/controller/SiteController.java

@@ -13,7 +13,8 @@
 @Controller
 public class SiteController {
 
-    ExamService examFileService;
+    private final ExamService examFileService;
+    private static final String SEARCH_TEMPLATE = "search";
 
     public SiteController(ExamService examFileService) {
         this.examFileService = examFileService;
@@ -27,22 +28,21 @@ public String viewIndexPage(Model model) {
     @GetMapping("/search")
     public String viewSearchPage(Model model) {
         model.addAttribute("examNumberForm", new ExamNumberForm(null));
-        return "search";
+        return SEARCH_TEMPLATE;
     }
 
     @PostMapping("/search")
     public String viewValidatePage(@Valid ExamNumberForm examNumberForm, BindingResult bindingResult, Model model) {
         model.addAttribute("examNumberForm", examNumberForm);
-        if (bindingResult.hasErrors()) {
-            return "search";
+        if (!bindingResult.hasErrors()) {
+            try {
+                // TODO: Replace hardcoded marticulationNumber 11112222 with dynamic number after login is implemented
+                model.addAttribute("examFiles",
+                                   examFileService.getMatchingExams(examNumberForm.getExamNumber(), "11112222"));
+            } catch (ResponseStatusException e) {
+                bindingResult.rejectValue("examNumber", "error.examNumberForm", e.getMessage());
+            }
         }
-        try {
-            // TODO: Replace hardcoded marticulationNumber 11112222 with dynamic number after login is implemented
-            model.addAttribute("examFiles",
-                               examFileService.getMatchingExams(examNumberForm.getExamNumber(), "11112222"));
-        } catch (ResponseStatusException e) {
-            bindingResult.rejectValue("examNumber", "error.examNumberForm", e.getReason());
-        }
-        return "search";
+        return SEARCH_TEMPLATE;
     }
 }

src/main/java/ch/puzzle/eft/model/ErrorModel.java

@@ -1,8 +1,5 @@
 package ch.puzzle.eft.model;
 
-import org.springframework.context.annotation.Scope;
-import org.springframework.context.annotation.ScopedProxyMode;
-import org.springframework.web.context.WebApplicationContext;
 
 public class ErrorModel {
 

src/main/java/ch/puzzle/eft/service/ExamService.java

@@ -1,15 +1,14 @@
 package ch.puzzle.eft.service;
 
 import ch.puzzle.eft.model.ExamModel;
-import jakarta.servlet.ServletOutputStream;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.core.env.Environment;
 import org.springframework.http.HttpStatus;
-import org.springframework.http.ResponseEntity;
 import org.springframework.stereotype.Service;
 import org.springframework.web.server.ResponseStatusException;
 
+import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
@@ -18,8 +17,6 @@
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipOutputStream;
 
-import static java.util.stream.Collectors.toList;
-
 @Service
 public class ExamService {
     private static final Logger logger = LoggerFactory.getLogger(ExamService.class);
@@ -82,7 +79,8 @@ protected File[] getSubjectDirectories() {
     }
 
     public File getFileToDownload(String subjectName, String filename) {
-        File examToDownload = new File(getBasePath() + "/" + subjectName + "/" + filename);
+        List<String> pathParts = List.of(getBasePath(), subjectName, filename);
+        File examToDownload = new File(String.join(File.separator, pathParts));
         if (!examToDownload.exists()) {
             logger.info("No file found for subject {} and filename {}", subjectName, filename);
             throw new ResponseStatusException(HttpStatus.NOT_FOUND,
@@ -93,7 +91,9 @@ public File getFileToDownload(String subjectName, String filename) {
         return examToDownload;
     }
 
-    public ResponseEntity<Object> convertFilesToZip(List<ExamModel> examFileList, ServletOutputStream outputStream) {
+    public ByteArrayOutputStream convertFilesToZip(List<ExamModel> examFileList) {
+        ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+
         try (ZipOutputStream zos = new ZipOutputStream(outputStream)) {
             for (ExamModel examFile : examFileList) {
                 String name = examFile.getSubjectName() + ".pdf";
@@ -111,16 +111,15 @@ public ResponseEntity<Object> convertFilesToZip(List<ExamModel> examFileList, Se
                 zos.closeEntry();
             }
         } catch (IOException e) {
-            logger.warn(e.getMessage());
-            return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR);
-
+            logger.error(e.getMessage());
+            throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR);
         }
-        return null;
+        return outputStream;
     }
 
-    public void convertSelectedFilesToZip(String examNumber, ServletOutputStream outputStream) {
+    public ByteArrayOutputStream convertSelectedFilesToZip(String examNumber) {
         // TODO: Replace hardcoded marticulationNumber 11112222 with dynamic number after login is implemented
         List<ExamModel> matchingExams = getMatchingExams(examNumber, "11112222");
-        convertFilesToZip(matchingExams, outputStream);
+        return convertFilesToZip(matchingExams);
     }
 }

src/test/java/ch/puzzle/eft/controller/ExamControllerTest.java

@@ -1,8 +1,6 @@
 package ch.puzzle.eft.controller;
 
 import ch.puzzle.eft.service.ExamService;
-import ch.puzzle.eft.test.MockServletOutputStream;
-import jakarta.servlet.ServletOutputStream;
 import jakarta.servlet.http.HttpSession;
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -13,6 +11,7 @@
 import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.web.server.ResponseStatusException;
 
+import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.nio.file.Files;
 
@@ -26,7 +25,6 @@
 @WithMockUser
 class ExamControllerTest {
 
-    MockServletOutputStream outputStream = new MockServletOutputStream();
     @Autowired
     private MockMvc mockMvc;
     @MockBean
@@ -37,14 +35,13 @@ class ExamControllerTest {
     @Test
     void downloadZipShouldReturnZip() throws Exception {
         String examNumber = "11000";
-        doNothing().when(examFileService)
-                   .convertSelectedFilesToZip(examNumber, outputStream);
+        when(examFileService.convertSelectedFilesToZip(examNumber)).thenReturn(new ByteArrayOutputStream());
 
         mockMvc.perform(get("/exams/download-zip/{examNumber}", examNumber))
                .andExpect(status().isOk())
                .andExpect(header().string("Content-Disposition", "attachment; filename=11000.zip"));
 
-        verify(examFileService).convertSelectedFilesToZip(eq(examNumber), any(ServletOutputStream.class));
+        verify(examFileService).convertSelectedFilesToZip(examNumber);
     }
 
     @Test

src/test/java/ch/puzzle/eft/controller/ExceptionControllerTest.java

@@ -13,7 +13,7 @@
 import static org.mockito.Mockito.*;
 
 @SpringBootTest
-public class ExceptionControllerTest {
+class ExceptionControllerTest {
 
     @Mock
     private HttpServletRequest request;
@@ -32,7 +32,7 @@ void testHandleInternalServerError() {
         Exception ex = new Exception("Internal error message");
         when(request.getRequestURL()).thenReturn(new StringBuffer("http://localhost/test-url"));
 
-        String viewName = exceptionHandlingController.handleInternalServerError(model, request, ex, session);
+        String viewName = exceptionHandlingController.handleInternalServerError(request, ex, session);
 
         verify(session, times(1)).setAttribute(eq("errorModel"), any(ErrorModel.class));
 

src/test/java/ch/puzzle/eft/service/ExamServiceTest.java

@@ -1,7 +1,6 @@
 package ch.puzzle.eft.service;
 
 import ch.puzzle.eft.model.ExamModel;
-import ch.puzzle.eft.test.MockServletOutputStream;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.mockito.Spy;
@@ -11,6 +10,7 @@
 import org.springframework.web.server.ResponseStatusException;
 
 import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
@@ -29,7 +29,6 @@ class ExamServiceTest {
     @Spy
     ExamService examFileService;
 
-    MockServletOutputStream mockOutputStream = new MockServletOutputStream();
 
     @Autowired
     public ExamServiceTest(ExamService examFileService) {
@@ -150,9 +149,9 @@ void shouldConvertFilesToZip() throws IOException {
 
         List<ExamModel> examFileList = Arrays.asList(fileModel1, fileModel2);
 
-        examFileService.convertFilesToZip(examFileList, mockOutputStream);
+        ByteArrayOutputStream byteArrayOutputStream = examFileService.convertFilesToZip(examFileList);
 
-        ByteArrayInputStream bis = new ByteArrayInputStream(mockOutputStream.getContentAsByteArray());
+        ByteArrayInputStream bis = new ByteArrayInputStream(byteArrayOutputStream.toByteArray());
         ZipInputStream zis = new ZipInputStream(bis);
         ZipEntry entry;
 
@@ -179,12 +178,11 @@ void shouldConvertFilesToZip() throws IOException {
 
     @Test
     void shouldReturnCorrectFilesAfterZip() throws IOException {
-        MockServletOutputStream mockOutputStream = new MockServletOutputStream();
 
-        examFileService.convertSelectedFilesToZip("11000", mockOutputStream);
+        ByteArrayOutputStream byteArrayOutputStream = examFileService.convertSelectedFilesToZip("11000");
 
         // Convert the output stream's content to a ZipInputStream to read and verify the ZIP contents
-        byte[] zipContent = mockOutputStream.getContentAsByteArray();
+        byte[] zipContent = byteArrayOutputStream.toByteArray();
         ZipInputStream zipInputStream = new ZipInputStream(new java.io.ByteArrayInputStream(zipContent));
 
         ZipEntry entry;