diff --git a/src/main/java/de/tum/in/www1/artemis/domain/Course.java b/src/main/java/de/tum/in/www1/artemis/domain/Course.java index 9ec939d891a0..875288fdd7b3 100644 --- a/src/main/java/de/tum/in/www1/artemis/domain/Course.java +++ b/src/main/java/de/tum/in/www1/artemis/domain/Course.java @@ -2,7 +2,6 @@ import static de.tum.in.www1.artemis.config.Constants.*; -import java.nio.file.Path; import java.time.ZonedDateTime; import java.util.HashSet; import java.util.Set; @@ -19,7 +18,6 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonView; -import de.tum.in.www1.artemis.config.Constants; import de.tum.in.www1.artemis.domain.competency.Competency; import de.tum.in.www1.artemis.domain.competency.LearningPath; import de.tum.in.www1.artemis.domain.enumeration.CourseInformationSharingConfiguration; @@ -713,56 +711,6 @@ public void removePrerequisite(Competency competency) { competency.getConsecutiveCourses().remove(this); } - /* - * NOTE: The file management is necessary to differentiate between temporary and used files and to delete used files when the corresponding course is deleted, or it is replaced - * by another file. The workflow is as follows 1. user uploads a file -> this is a temporary file, because at this point the corresponding course might not exist yet. 2. user - * saves the course -> now we move the temporary file which is addressed in courseIcon to a permanent location and update the value in courseIcon accordingly. => This happens - * in @PrePersist and @PostPersist 3. user might upload another file to replace the existing file -> this new file is a temporary file at first 4. user saves changes (with the - * new courseIcon pointing to the new temporary file) -> now we delete the old file in the permanent location and move the new file to a permanent location and update the value - * in courseIcon accordingly. => This happens in @PreUpdate and uses @PostLoad to know the old path 5. When course is deleted, the file in the permanent location is deleted => - * This happens in @PostRemove - */ - - /** - * Initialisation of the Course on Server start - */ - @PostLoad - public void onLoad() { - // replace placeholder with actual id if necessary (this is needed because changes made in afterCreate() are not persisted) - if (courseIcon != null && courseIcon.contains(Constants.FILEPATH_ID_PLACEHOLDER)) { - courseIcon = courseIcon.replace(Constants.FILEPATH_ID_PLACEHOLDER, getId().toString()); - } - prevCourseIcon = courseIcon; // save current path as old path (needed to know old path in onUpdate() and onDelete()) - } - - @PrePersist - public void beforeCreate() { - if (courseIcon != null) { - courseIcon = entityFileService.moveTempFileBeforeEntityPersistence(courseIcon, FilePathService.getCourseIconFilePath(), false); - } - } - - @PostPersist - public void afterCreate() { - // replace placeholder with actual id if necessary (id is no longer null at this point) - if (courseIcon != null && courseIcon.contains(Constants.FILEPATH_ID_PLACEHOLDER)) { - courseIcon = courseIcon.replace(Constants.FILEPATH_ID_PLACEHOLDER, getId().toString()); - } - } - - @PreUpdate - public void onUpdate() { - // move file and delete old file if necessary - courseIcon = entityFileService.handlePotentialFileUpdateBeforeEntityPersistence(getId(), prevCourseIcon, courseIcon, FilePathService.getCourseIconFilePath(), false); - } - - @PostRemove - public void onDelete() { - if (prevCourseIcon != null) { - fileService.schedulePathForDeletion(Path.of(prevCourseIcon), 0); - } - } - @Override public String toString() { return "Course{" + "id=" + getId() + ", title='" + getTitle() + "'" + ", description='" + getDescription() + "'" + ", shortName='" + getShortName() + "'" diff --git a/src/main/java/de/tum/in/www1/artemis/service/EntityFileService.java b/src/main/java/de/tum/in/www1/artemis/service/EntityFileService.java index 45267c6632c4..2a7b51c94743 100644 --- a/src/main/java/de/tum/in/www1/artemis/service/EntityFileService.java +++ b/src/main/java/de/tum/in/www1/artemis/service/EntityFileService.java @@ -68,7 +68,8 @@ public String moveFileBeforeEntityPersistenceWithIdIfIsTemp(@Nonnull String enti target = targetFolder.resolve(filename); } else { - target = fileService.generateFilePath(fileService.generateTargetFilenameBase(targetFolder), extension, targetFolder); + String generatedFilename = fileService.generateFilename(fileService.generateTargetFilenameBase(targetFolder), extension); + target = targetFolder.resolve(generatedFilename); } // remove target file before copying, because moveFile() ignores CopyOptions if (target.toFile().exists()) { diff --git a/src/main/java/de/tum/in/www1/artemis/service/FileService.java b/src/main/java/de/tum/in/www1/artemis/service/FileService.java index 7231856ed3dd..8902bf8449b6 100644 --- a/src/main/java/de/tum/in/www1/artemis/service/FileService.java +++ b/src/main/java/de/tum/in/www1/artemis/service/FileService.java @@ -15,6 +15,7 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; +import javax.annotation.Nonnull; import javax.annotation.Nullable; import javax.validation.constraints.NotNull; @@ -162,13 +163,8 @@ public URI handleSaveFile(MultipartFile file, boolean keepFilename, boolean mark // check for file type String filename = checkAndSanitizeFilename(file.getOriginalFilename()); - // Check the allowed file extensions + validateExtension(filename, markdown); final String fileExtension = FilenameUtils.getExtension(filename); - final Set allowedExtensions = markdown ? allowedMarkdownFileExtensions : allowedFileExtensions; - - if (allowedExtensions.stream().noneMatch(fileExtension::equalsIgnoreCase)) { - throw new BadRequestAlertException("Unsupported file type! Allowed file types: " + String.join(", ", allowedExtensions), "file", null, true); - } final String filenamePrefix = markdown ? "Markdown_" : "Temp_"; final Path path = markdown ? FilePathService.getMarkdownFilePath() : FilePathService.getTempFilePath(); @@ -178,20 +174,52 @@ public URI handleSaveFile(MultipartFile file, boolean keepFilename, boolean mark filePath = path.resolve(filename); } else { - filePath = generateFilePath(filenamePrefix, fileExtension, path); + String generatedFilename = generateFilename(filenamePrefix, fileExtension); + filePath = path.resolve(generatedFilename); } + + copyFile(file, filePath); + + String currentFilename = filePath.getFileName().toString(); + return URI.create(markdown ? MARKDOWN_FILE_SUBPATH : DEFAULT_FILE_SUBPATH).resolve(currentFilename); + } + + /** + * Saves a file to the given path using a generated filename. + * + * @param file the file to save + * @param basePath the base path to save the file to + * @return the path where the file was saved + */ + @NotNull + public Path saveFileFoo(MultipartFile file, Path basePath) { + String sanitizedFilename = checkAndSanitizeFilename(file.getOriginalFilename()); + validateExtension(sanitizedFilename, false); + String generatedFilename = generateFilename(generateTargetFilenameBase(basePath), FilenameUtils.getExtension(sanitizedFilename)); + Path savePath = basePath.resolve(generatedFilename); + copyFile(file, savePath); + return savePath; + } + + private void copyFile(MultipartFile file, Path filePath) { try { FileUtils.copyInputStreamToFile(file.getInputStream(), filePath.toFile()); - - return generateResponsePath(filePath, markdown); } catch (IOException e) { - log.error("Could not save file {}", filename, e); + log.error("Could not save file {}", filePath.getFileName(), e); throw new InternalServerErrorException("Could not create file"); } } - private String checkAndSanitizeFilename(String filename) { + /** + * Nullsafe sanitation method for filenames. + * + * @param filename the filename to sanitize + * @return the sanitized filename + * @throws IllegalArgumentException if the filename is null + */ + @Nonnull + public String checkAndSanitizeFilename(@Nullable String filename) { if (filename == null) { throw new IllegalArgumentException("Filename cannot be null"); } @@ -200,33 +228,30 @@ private String checkAndSanitizeFilename(String filename) { } /** - * Generates the API path getting returned to the client + * Validates the file extension of the given filename. If the markdown flag is set to true, only markdown file extensions are allowed. * - * @param filePath the file system path of the file - * @param markdown boolean which is set to true, when we are uploading a file in the Markdown format - * @return the API path of the file + * @param filename the filename to validate + * @param markdown whether the file is a markdown file + * @throws BadRequestAlertException if the file extension is not allowed */ - private URI generateResponsePath(Path filePath, boolean markdown) { - String filename = filePath.getFileName().toString(); - if (markdown) { - return URI.create(MARKDOWN_FILE_SUBPATH).resolve(filename); + public void validateExtension(String filename, boolean markdown) { + final String fileExtension = FilenameUtils.getExtension(filename); + final Set allowedExtensions = markdown ? allowedMarkdownFileExtensions : allowedFileExtensions; + + if (allowedExtensions.stream().noneMatch(fileExtension::equalsIgnoreCase)) { + throw new BadRequestAlertException("Unsupported file type! Allowed file types: " + String.join(", ", allowedExtensions), "file", null, true); } - return URI.create(DEFAULT_FILE_SUBPATH).resolve(filename); } /** - * Generates the path for the file to be saved to with a random file name based on the parameters. + * Generates a new filename based on the current time and a random UUID. * * @param filenamePrefix the prefix of the filename * @param fileExtension the extension of the file - * @param folder the folder to save the file to - * @return the path to save the file to + * @return the generated filename */ - public Path generateFilePath(String filenamePrefix, String fileExtension, Path folder) { - // append a timestamp and some randomness to the filename to avoid conflicts - String generatedFilename = filenamePrefix + ZonedDateTime.now().toString().substring(0, 23).replaceAll("[:.]", "-") + "_" + UUID.randomUUID().toString().substring(0, 8) - + "." + fileExtension; - return folder.resolve(generatedFilename); + public String generateFilename(String filenamePrefix, String fileExtension) { + return filenamePrefix + ZonedDateTime.now().toString().substring(0, 23).replaceAll("[:.]", "-") + "_" + UUID.randomUUID().toString().substring(0, 8) + "." + fileExtension; } /** @@ -240,7 +265,7 @@ public Path copyExistingFileToTarget(Path oldFilePath, Path targetFolder) { if (oldFilePath != null && !pathContains(oldFilePath, Path.of(("files/temp")))) { String filename = oldFilePath.getFileName().toString(); try { - Path target = generateFilePath(generateTargetFilenameBase(targetFolder), FilenameUtils.getExtension(filename), targetFolder); + Path target = targetFolder.resolve(generateFilename(generateTargetFilenameBase(targetFolder), FilenameUtils.getExtension(filename))); FileUtils.copyFile(oldFilePath.toFile(), target.toFile()); log.debug("Moved File from {} to {}", oldFilePath, target); return target; diff --git a/src/main/java/de/tum/in/www1/artemis/service/LectureUnitProcessingService.java b/src/main/java/de/tum/in/www1/artemis/service/LectureUnitProcessingService.java index a3472ae1eaf9..27cfce69b956 100644 --- a/src/main/java/de/tum/in/www1/artemis/service/LectureUnitProcessingService.java +++ b/src/main/java/de/tum/in/www1/artemis/service/LectureUnitProcessingService.java @@ -211,7 +211,7 @@ public LectureUnitInformationDTO getSplitUnitData(byte[] fileBytes) { */ public String saveTempFileForProcessing(long lectureId, MultipartFile file, int minutesUntilDeletion) throws IOException { String prefix = "Temp_" + lectureId + "_"; - Path filePath = fileService.generateFilePath(prefix, FilenameUtils.getExtension(file.getOriginalFilename()), FilePathService.getTempFilePath()); + Path filePath = FilePathService.getTempFilePath().resolve(fileService.generateFilename(prefix, FilenameUtils.getExtension(file.getOriginalFilename()))); FileUtils.copyInputStreamToFile(file.getInputStream(), filePath.toFile()); fileService.schedulePathForDeletion(filePath, minutesUntilDeletion); return filePath.getFileName().toString().substring(prefix.length()); diff --git a/src/main/java/de/tum/in/www1/artemis/service/QuizExerciseService.java b/src/main/java/de/tum/in/www1/artemis/service/QuizExerciseService.java index b19cd3c855b7..e8d1e4b39c2b 100644 --- a/src/main/java/de/tum/in/www1/artemis/service/QuizExerciseService.java +++ b/src/main/java/de/tum/in/www1/artemis/service/QuizExerciseService.java @@ -380,7 +380,7 @@ public void saveDndDragItemPicture(DragItem dragItem, Map */ private URI saveDragAndDropImage(Path basePath, MultipartFile file, @Nullable Long entityId) throws IOException { String clearFileExtension = FileService.sanitizeFilename(FilenameUtils.getExtension(Objects.requireNonNull(file.getOriginalFilename()))); - Path savePath = fileService.generateFilePath("dnd_image_", clearFileExtension, basePath); + Path savePath = basePath.resolve(fileService.generateFilename("dnd_image_", clearFileExtension)); FileUtils.copyToFile(file.getInputStream(), savePath.toFile()); return filePathService.publicPathForActualPathOrThrow(savePath, entityId); } diff --git a/src/main/java/de/tum/in/www1/artemis/web/rest/CourseResource.java b/src/main/java/de/tum/in/www1/artemis/web/rest/CourseResource.java index 8c276dfa4411..fded119806b2 100644 --- a/src/main/java/de/tum/in/www1/artemis/web/rest/CourseResource.java +++ b/src/main/java/de/tum/in/www1/artemis/web/rest/CourseResource.java @@ -114,13 +114,15 @@ public class CourseResource { private final ExamRepository examRepository; + private final FilePathService filePathService; + public CourseResource(UserRepository userRepository, CourseService courseService, CourseRepository courseRepository, ExerciseService exerciseService, Optional oAuth2JWKSService, Optional onlineCourseConfigurationService, AuthorizationCheckService authCheckService, TutorParticipationRepository tutorParticipationRepository, SubmissionService submissionService, Optional optionalVcsUserManagementService, AssessmentDashboardService assessmentDashboardService, ExerciseRepository exerciseRepository, Optional optionalCiUserManagementService, FileService fileService, TutorialGroupsConfigurationService tutorialGroupsConfigurationService, GradingScaleService gradingScaleService, CourseScoreCalculationService courseScoreCalculationService, GradingScaleRepository gradingScaleRepository, LearningPathService learningPathService, - ConductAgreementService conductAgreementService, ExamRepository examRepository) { + ConductAgreementService conductAgreementService, ExamRepository examRepository, FilePathService filePathService) { this.courseService = courseService; this.courseRepository = courseRepository; this.exerciseService = exerciseService; @@ -142,6 +144,7 @@ public CourseResource(UserRepository userRepository, CourseService courseService this.learningPathService = learningPathService; this.conductAgreementService = conductAgreementService; this.examRepository = examRepository; + this.filePathService = filePathService; } /** @@ -226,8 +229,9 @@ public ResponseEntity updateCourse(@PathVariable Long courseId, @Request courseUpdate.validateUnenrollmentEndDate(); if (file != null) { - String pathString = fileService.handleSaveFile(file, false, false).toString(); - courseUpdate.setCourseIcon(pathString); + Path basePath = FilePathService.getCourseIconFilePath(); + Path savePath = fileService.saveFileFoo(file, basePath); + courseUpdate.setCourseIcon(filePathService.publicPathForActualPathOrThrow(savePath, courseId).toString()); } if (courseUpdate.isOnlineCourse() != existingCourse.isOnlineCourse()) { diff --git a/src/main/java/de/tum/in/www1/artemis/web/rest/admin/AdminCourseResource.java b/src/main/java/de/tum/in/www1/artemis/web/rest/admin/AdminCourseResource.java index 14febfaf6f01..dd6564f47ebe 100644 --- a/src/main/java/de/tum/in/www1/artemis/web/rest/admin/AdminCourseResource.java +++ b/src/main/java/de/tum/in/www1/artemis/web/rest/admin/AdminCourseResource.java @@ -2,6 +2,7 @@ import java.net.URI; import java.net.URISyntaxException; +import java.nio.file.Path; import java.util.*; import org.slf4j.Logger; @@ -22,9 +23,7 @@ import de.tum.in.www1.artemis.repository.CourseRepository; import de.tum.in.www1.artemis.repository.UserRepository; import de.tum.in.www1.artemis.security.annotations.EnforceAdmin; -import de.tum.in.www1.artemis.service.CourseService; -import de.tum.in.www1.artemis.service.FileService; -import de.tum.in.www1.artemis.service.OnlineCourseConfigurationService; +import de.tum.in.www1.artemis.service.*; import de.tum.in.www1.artemis.service.metis.conversation.ChannelService; import de.tum.in.www1.artemis.web.rest.errors.BadRequestAlertException; import de.tum.in.www1.artemis.web.rest.util.HeaderUtil; @@ -55,8 +54,10 @@ public class AdminCourseResource { private final Optional onlineCourseConfigurationService; + private final FilePathService filePathService; + public AdminCourseResource(UserRepository userRepository, CourseService courseService, CourseRepository courseRepository, AuditEventRepository auditEventRepository, - FileService fileService, Optional onlineCourseConfigurationService, ChannelService channelService) { + FileService fileService, Optional onlineCourseConfigurationService, ChannelService channelService, FilePathService filePathService) { this.courseService = courseService; this.courseRepository = courseRepository; this.auditEventRepository = auditEventRepository; @@ -64,6 +65,7 @@ public AdminCourseResource(UserRepository userRepository, CourseService courseSe this.fileService = fileService; this.onlineCourseConfigurationService = onlineCourseConfigurationService; this.channelService = channelService; + this.filePathService = filePathService; } /** @@ -124,8 +126,9 @@ public ResponseEntity createCourse(@RequestPart Course course, @RequestP courseService.createOrValidateGroups(course); if (file != null) { - String pathString = fileService.handleSaveFile(file, false, false).toString(); - course.setCourseIcon(pathString); + Path basePath = FilePathService.getCourseIconFilePath(); + Path savePath = fileService.saveFileFoo(file, basePath); + course.setCourseIcon(filePathService.publicPathForActualPathOrThrow(savePath, course.getId()).toString()); } Course createdCourse = courseRepository.save(course); @@ -152,6 +155,7 @@ public ResponseEntity deleteCourse(@PathVariable long courseId) { log.info("User {} has requested to delete the course {}", user.getLogin(), course.getTitle()); courseService.delete(course); + fileService.schedulePathForDeletion(filePathService.actualPathForPublicPathOrThrow(URI.create(course.getCourseIcon())), 30); return ResponseEntity.ok().headers(HeaderUtil.createEntityDeletionAlert(applicationName, true, Course.ENTITY_NAME, course.getTitle())).build(); }