Merge branch '9-limits-on-submissions' into 'development'

Resolve "Limits on submissions" (1)

Closes #9

See merge request !7

src/main/java/nl/tudelft/submit/controller/SubmissionController.java

@@ -35,6 +35,7 @@ import nl.tudelft.labracore.lib.security.user.AuthenticatedPerson;
 import nl.tudelft.submit.dto.create.MailMessageCreateDTO;
 import nl.tudelft.submit.dto.create.SubmissionDownloadConfigCreateDTO;
 import nl.tudelft.submit.dto.create.note.SubmissionNoteCreateDTO;
+import nl.tudelft.submit.exception.DisallowedUploadException;
 import nl.tudelft.submit.model.Signature;
 import nl.tudelft.submit.service.*;
 import nl.tudelft.submit.service.AssignmentService;
@@ -110,7 +111,16 @@ public class SubmissionController {
 		Long id = submissionService.addSubmission(createSubmissionDto);
 
 		try {
-			fileService.uploadFiles(request.getFileMap(), id, submissionsDirectory);
+			AssignmentModuleDetailsDTO assignment = assignmentService
+					.getAssignmentDetails(createSubmissionDto.getAssignment().getId());
+			StudentGroupDetailsDTO group = groupService
+					.getGroupForPersonInModule(person.getId(), assignment.getModule().getId()).get();
+
+			if (!submissionService.canMakeSubmission(group.getId(), assignment.getId()))
+				throw new DisallowedUploadException("Submission can not be made");
+
+			fileService.uploadFiles(request.getFileMap(), id, assignment.getAllowedFileTypes(),
+					submissionsDirectory);
 
 			scriptService.runScriptTrainAndSaveFeedback(id);
 
@@ -122,7 +132,7 @@ public class SubmissionController {
 											.map(MultipartFile::getOriginalFilename)
 											.collect(Collectors.toList())))
 							.build());
-		} catch (Exception e) {
+		} catch (IOException e) {
 			e.printStackTrace(); // The email will not be sent, TODO a different email can be sent
 		}
 

src/main/java/nl/tudelft/submit/exception/DisallowedUploadException.java

+/*
+ * Submit
+ * Copyright (C) 2020 - Delft University of Technology
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+package nl.tudelft.submit.exception;
+
+import org.springframework.http.HttpStatus;
+import org.springframework.web.bind.annotation.ResponseStatus;
+
+@ResponseStatus(value = HttpStatus.BAD_REQUEST)
+public class DisallowedUploadException extends RuntimeException {
+
+	public DisallowedUploadException() {
+		super();
+	}
+
+	public DisallowedUploadException(String message) {
+		super(message);
+	}
+
+}

src/main/java/nl/tudelft/submit/service/FileService.java

@@ -31,6 +31,9 @@ import java.util.stream.Collectors;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipOutputStream;
 
+import nl.tudelft.submit.exception.DisallowedUploadException;
+
+import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.core.io.Resource;
 import org.springframework.core.io.UrlResource;
@@ -49,6 +52,9 @@ public class FileService {
 	@Value("${submit.filesys.level-depth}")
 	private Integer NUM_0F_LEVELS;
 
+	@Autowired
+	private AssignmentService assignmentService;
+
 	/**
 	 * Generates a path for a given Submission identifier
 	 *
@@ -84,41 +90,38 @@ public class FileService {
 	 * @param  file        the file to be uploaded
 	 * @param  entityId    the identifier of the entity
 	 * @param  directory   the name of the directory where file should be stored at the generated path
-	 * @throws Exception if file could not be saved
+	 * @throws IOException if file could not be saved
 	 */
-	private void uploadFile(MultipartFile file, Long entityId, String directory) throws Exception {
-		try {
+	private void uploadFile(MultipartFile file, Long entityId, String directory) throws IOException {
 		Path path = Paths
 				.get("./" + directory + File.separator
 						+ StringUtils.cleanPath(getDirectory(entityId) + file.getOriginalFilename()));
 		File f = new File(String.valueOf(path));
 		f.mkdirs();
 		Files.copy(file.getInputStream(), path, StandardCopyOption.REPLACE_EXISTING);
-		} catch (Exception e) {
-			e.printStackTrace();
-			throw new Exception("Could not store file " + file.getOriginalFilename()
-					+ ". Please try again!");
-		}
 	}
 
 	/**
 	 * Upload multiple files to the local system.
 	 *
 	 * @param  fileMap      a map containing files received from the requests
-	 * @param  entityId  the identifier of the entity
-	 * @param  directory the name of the directory where file should be stored at the generated path (only the
-	 *                   name, not the complete path)
-	 * @throws Exception if file could not be saved
+	 * @param  submissionId the id of the submission
+	 * @param  assignmentId the id of the assignment
+	 * @param  directory    the name of the directory where file should be stored at the generated path (only
+	 *                      the name, not the complete path)
+	 * @throws IOException  if file could not be saved
 	 */
-	public void uploadFiles(Map<String, MultipartFile> fileMap, Long entityId, String directory)
-			throws Exception {
-		fileMap.forEach((p, f) -> {
-			try {
-				uploadFile(f, entityId, directory);
-			} catch (Exception e) {
-				e.printStackTrace();
+	public void uploadFiles(Map<String, MultipartFile> fileMap, Long submissionId,
+			List<String> allowedFileTypes,
+			String directory) throws IOException {
+		for (MultipartFile file : fileMap.values()) {
+			if (!allowedFileTypes.isEmpty() && allowedFileTypes.stream()
+					.noneMatch(ext -> file.getResource().getFilename().endsWith(ext))) {
+				throw new DisallowedUploadException(
+						"File " + file.getResource().getFilename() + " is not allowed");
+			}
+			uploadFile(file, submissionId, directory);
 		}
-		});
 	}
 
 	/**

src/main/java/nl/tudelft/submit/service/SubmissionService.java

@@ -172,13 +172,10 @@ public class SubmissionService {
 	 * @return              boolean whether anyone from that group can make a submission
 	 */
 	public boolean canMakeSubmission(Long assignmentId, Long groupId) {
-		AssignmentModuleDetailsDTO assignment = assignmentService.getAssignmentDetails(assignmentId);
-
-		Integer submissionLimit = assignment.getSubmissionLimit();
-		if (submissionLimit != null
-				&& getSubmissionsOfAssignmentForGroup(assignmentId, groupId).size() >= submissionLimit) {
+		if (!submissionApi.canMakeSubmission(groupId, assignmentId).block())
 			return false;
-		}
+
+		AssignmentModuleDetailsDTO assignment = assignmentService.getAssignmentDetails(assignmentId);
 
 		LocalDateTime deadline = assignmentService.getDeadlineForGroup(assignmentId, groupId);
 		LocalDateTime currentTime = LocalDateTime.now();

src/main/resources/templates/submission/dropzone.html

@@ -18,28 +18,25 @@
 
 -->
 <!DOCTYPE html>
-<html lang="en" xmlns:th="http://www.thymeleaf.org" xmlns:layout="http://www.ultraq.net.nz/thymeleaf/layout"
-      layout:decorate="~{layout}">
+<html lang="en" xmlns:th="http://www.thymeleaf.org" xmlns:layout="http://www.ultraq.net.nz/thymeleaf/layout">
 <body>
 
-<script th:inline="javascript" layout:fragment="submission-dropzone">
+<script th:inline="javascript" layout:fragment="submission-dropzone" th:with="fileTypes = ${assignment.allowedFileTypes.toString()}">
     /*<![CDATA[*/
-    var assignment_id = /*[[${assignment.getId()}]]*/ "";
-    var dz_id = "sub-form-" + assignment_id;
-    new Dropzone("#" + dz_id, {
+    new Dropzone("#sub-form-" + [[${assignment.id}]], {
         url: "/submission/",
         autoProcessQueue: false,
         autoQueue: true,
-        acceptedFiles: ".pdf",
+        [[${fileTypes != null} ? 'acceptedFiles: ' + ${#strings.substring(fileTypes, 1, #strings.length(fileTypes)-1)} + ',' : '']]
         uploadMultiple: true,
         parallelUploads : 5,
         maxFiles : 5,
         addRemoveLinks : true,
         init: function() {
-            var button_id = "sub-button-" + assignment_id;
-            var submitButton = document.getElementById(button_id);
+            let button_id = "sub-button-" + [[${assignment.id}]];
+            let submitButton = document.getElementById(button_id);
 
-            var dz = this;
+            let dz = this;
 
             submitButton.addEventListener("click", function() {
                 dz.processQueue();
@@ -57,22 +54,20 @@
 
 <script th:inline="javascript" layout:fragment="assignment-dropzone">
     /*<![CDATA[*/
-    var assignment_id = /*[[${assignment.getId()}]]*/ "";
-    var dz_id = "submissionForm";
-    new Dropzone("#" + dz_id, {
+    new Dropzone("#sub-form-" + [[${assignment.id}]], {
         url: "/submission/",
         autoProcessQueue: false,
         autoQueue: true,
-        acceptedFiles: ".pdf",
+        [[${fileTypes != null} ? 'acceptedFiles: ' + ${#strings.substring(fileTypes, 1, #strings.length(fileTypes)-1)} + ',' : '']]
         uploadMultiple: true,
         parallelUploads : 5,
         maxFiles : 5,
         addRemoveLinks : true,
         init: function() {
-            var button_id = "submit-button";
-            var submitButton = document.getElementById(button_id);
+            let button_id = "submit-button";
+            let submitButton = document.getElementById(button_id);
 
-            var dz = this;
+            let dz = this;
 
             submitButton.addEventListener("click", function() {
                 dz.processQueue();

src/test/java/nl/tudelft/submit/controller/SubmissionControllerTest.java

@@ -25,6 +25,7 @@ import static org.springframework.test.web.servlet.request.MockMvcRequestBuilder
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
 
 import java.io.File;
+import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Paths;
 import java.time.LocalDateTime;
@@ -78,11 +79,14 @@ class SubmissionControllerTest {
 	private static final Long SUBMISSION_ID = 123L;
 	private static final Long GROUP_ID = 69L;
 	private static final Long ASSIGNMENT_ID = 420L;
+	private static final Long MODULE_ID = 412312L;
 	private static final Long PERSON_ID = 42L;
 	private static final Long NOTE_ID = 4876L;
 
 	private static final AssignmentSummaryDTO ASSIGNMENT_SUMMARY_DTO = new AssignmentSummaryDTO()
 			.id(ASSIGNMENT_ID);
+	private static final AssignmentModuleDetailsDTO ASSIGNMENT_DETAILS_DTO = new AssignmentModuleDetailsDTO()
+			.id(ASSIGNMENT_ID).module(new ModuleLayer1DTO().id(MODULE_ID));
 	private static final SubmissionSummaryDTO SUB_VIEW_DTO = new SubmissionSummaryDTO().id(111L);
 	private static final SubmissionMemberDetailsDTO SUB_DETAILED_MEM_VIEW_DTO = new SubmissionMemberDetailsDTO()
 			.id(777L)
@@ -136,8 +140,12 @@ class SubmissionControllerTest {
 
 		when(authService.isMemberOfGroup(anyLong())).thenReturn(true);
 		when(submissionService.addSubmission(SUB_CREATE_DTO)).thenReturn(SUBMISSION_ID);
+		when(submissionService.canMakeSubmission(anyLong(), anyLong())).thenReturn(true);
 		when(userSettingsService.getOrCreateUserSettings(any())).thenReturn(settings);
-		doNothing().when(fileService).uploadFiles(any(), anyLong(), anyString());
+		when(assignmentService.getAssignmentDetails(anyLong())).thenReturn(ASSIGNMENT_DETAILS_DTO);
+		when(groupService.getGroupForPersonInModule(anyLong(), anyLong()))
+				.thenReturn(Optional.ofNullable(new StudentGroupDetailsDTO().id(GROUP_ID)));
+		doNothing().when(fileService).uploadFiles(any(), anyLong(), anyList(), anyString());
 		doNothing().when(scriptService).runScriptTrainAndSaveFeedback(anyLong());
 
 		mockMvc.perform(MockMvcRequestBuilders.multipart("/submission")
@@ -146,7 +154,7 @@ class SubmissionControllerTest {
 				.flashAttr("createSubmissionDto", SUB_CREATE_DTO))
 				.andExpect(status().is3xxRedirection());
 
-		verify(fileService).uploadFiles(any(), anyLong(), anyString());
+		verify(fileService).uploadFiles(any(), anyLong(), any(), anyString());
 		verify(authService).isMemberOfGroup(anyLong());
 		verify(submissionService).addSubmission(SUB_CREATE_DTO);
 		verify(emailService).sendEmailToAuthPerson(any(MailMessageCreateDTO.class));
@@ -160,7 +168,11 @@ class SubmissionControllerTest {
 
 		when(authService.isMemberOfGroup(anyLong())).thenReturn(true);
 		when(submissionService.addSubmission(SUB_CREATE_DTO)).thenReturn(SUBMISSION_ID);
-		doThrow(new Exception()).when(fileService).uploadFiles(any(), anyLong(), anyString());
+		when(submissionService.canMakeSubmission(anyLong(), anyLong())).thenReturn(true);
+		when(assignmentService.getAssignmentDetails(anyLong())).thenReturn(ASSIGNMENT_DETAILS_DTO);
+		when(groupService.getGroupForPersonInModule(anyLong(), anyLong()))
+				.thenReturn(Optional.ofNullable(new StudentGroupDetailsDTO().id(GROUP_ID)));
+		doThrow(new IOException()).when(fileService).uploadFiles(any(), anyLong(), any(), anyString());
 		doNothing().when(scriptService).runScriptTrainAndSaveFeedback(anyLong());
 
 		mockMvc.perform(MockMvcRequestBuilders.multipart("/submission")
@@ -169,7 +181,7 @@ class SubmissionControllerTest {
 				.flashAttr("createSubmissionDto", SUB_CREATE_DTO))
 				.andExpect(status().is3xxRedirection());
 
-		verify(fileService).uploadFiles(any(), anyLong(), anyString());
+		verify(fileService).uploadFiles(any(), anyLong(), any(), anyString());
 		verify(authService).isMemberOfGroup(anyLong());
 		verify(submissionService).addSubmission(SUB_CREATE_DTO);
 		verify(emailService, never()).sendEmailToAuthPerson(any(MailMessageCreateDTO.class));

src/test/java/nl/tudelft/submit/service/FileServiceTest.java

@@ -23,6 +23,7 @@ import java.io.File;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 import nl.tudelft.submit.TestSubmitApplication;
@@ -67,13 +68,13 @@ class FileServiceTest {
 
 	@Test
 	void uploadFile() throws Exception {
-		MockMultipartFile file = new MockMultipartFile("file", "filename", "multipart/form-data",
+		MockMultipartFile file = new MockMultipartFile("file", "filename.pdf", "multipart/form-data",
 				"submission".getBytes());
 
 		Map<String, MultipartFile> map = new HashMap<>();
 		map.put("param", file);
 
-		fileService.uploadFiles(map, SUB_ID_1, "submissions");
+		fileService.uploadFiles(map, SUB_ID_1, List.of("pdf"), "submissions");
 
 		String path = "./" + submissionsDirectory + File.separator + fileService.getDirectory(SUB_ID_1);
 

src/test/java/nl/tudelft/submit/service/SubmissionServiceTest.java

@@ -146,17 +146,12 @@ class SubmissionServiceTest {
 
 	@Test
 	void canMakeSubmissionOverSubmissionLimit() {
-		AssignmentModuleDetailsDTO assignment = new AssignmentModuleDetailsDTO().submissionLimit(2)
-				.acceptLateSubmissions(false);
-
-		when(assignmentService.getAssignmentDetails(anyLong())).thenReturn(assignment);
 		when(submissionApi.getSubmissionsForGroupAndAssignment(anyLong(), anyLong()))
 				.thenReturn(Flux.just(new SubmissionDetailsDTO(), new SubmissionDetailsDTO()));
+		when(submissionApi.canMakeSubmission(anyLong(), anyLong())).thenReturn(Mono.just(false));
 
 		assertThat(submissionService.canMakeSubmission(ASSIGNMENT_ID, GROUP_ID)).isFalse();
 
-		verify(assignmentService).getAssignmentDetails(ASSIGNMENT_ID);
-		verify(submissionApi).getSubmissionsForGroupAndAssignment(GROUP_ID, ASSIGNMENT_ID);
 		verify(assignmentService, never()).getDeadlineForGroup(anyLong(), anyLong());
 	}
 
@@ -166,6 +161,7 @@ class SubmissionServiceTest {
 
 		when(assignmentService.getAssignmentDetails(anyLong())).thenReturn(assignment);
 		when(assignmentService.getDeadlineForGroup(anyLong(), anyLong())).thenReturn(null);
+		when(submissionApi.canMakeSubmission(anyLong(), anyLong())).thenReturn(Mono.just(true));
 
 		assertThat(submissionService.canMakeSubmission(ASSIGNMENT_ID, GROUP_ID)).isTrue();
 
@@ -180,6 +176,7 @@ class SubmissionServiceTest {
 
 		when(assignmentService.getAssignmentDetails(anyLong())).thenReturn(assignment);
 		when(assignmentService.getDeadlineForGroup(anyLong(), anyLong())).thenReturn(FUTURE_DEADLINE);
+		when(submissionApi.canMakeSubmission(anyLong(), anyLong())).thenReturn(Mono.just(true));
 
 		assertThat(submissionService.canMakeSubmission(ASSIGNMENT_ID, GROUP_ID)).isTrue();
 
@@ -195,6 +192,7 @@ class SubmissionServiceTest {
 
 		when(assignmentService.getAssignmentDetails(anyLong())).thenReturn(assignment);
 		when(assignmentService.getDeadlineForGroup(anyLong(), anyLong())).thenReturn(PAST_DEADLINE);
+		when(submissionApi.canMakeSubmission(anyLong(), anyLong())).thenReturn(Mono.just(true));
 
 		assertThat(submissionService.canMakeSubmission(ASSIGNMENT_ID, GROUP_ID)).isTrue();
 
@@ -210,6 +208,7 @@ class SubmissionServiceTest {
 
 		when(assignmentService.getAssignmentDetails(anyLong())).thenReturn(assignment);
 		when(assignmentService.getDeadlineForGroup(anyLong(), anyLong())).thenReturn(PAST_DEADLINE);
+		when(submissionApi.canMakeSubmission(anyLong(), anyLong())).thenReturn(Mono.just(true));
 
 		assertThat(submissionService.canMakeSubmission(ASSIGNMENT_ID, GROUP_ID)).isFalse();
 
@@ -224,6 +223,7 @@ class SubmissionServiceTest {
 
 		when(assignmentService.getAssignmentDetails(anyLong())).thenReturn(assignment);
 		when(assignmentService.getDeadlineForGroup(anyLong(), anyLong())).thenReturn(PAST_DEADLINE);
+		when(submissionApi.canMakeSubmission(anyLong(), anyLong())).thenReturn(Mono.just(true));
 
 		assertThat(submissionService.canMakeSubmission(ASSIGNMENT_ID, GROUP_ID)).isFalse();
 
@@ -238,6 +238,7 @@ class SubmissionServiceTest {
 
 		when(assignmentService.getAssignmentDetails(anyLong())).thenReturn(assignment);
 		when(assignmentService.getDeadlineForGroup(anyLong(), anyLong())).thenReturn(FUTURE_DEADLINE);
+		when(submissionApi.canMakeSubmission(anyLong(), anyLong())).thenReturn(Mono.just(true));
 
 		assertThat(submissionService.canMakeSubmission(ASSIGNMENT_ID, GROUP_ID)).isTrue();