handle course image persistence directly

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() + "'"

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()) {

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<String> 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<String> 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;

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());

src/main/java/de/tum/in/www1/artemis/service/QuizExerciseService.java

@@ -380,7 +380,7 @@ public void saveDndDragItemPicture(DragItem dragItem, Map<String, MultipartFile>
      */
     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);
     }

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> oAuth2JWKSService, Optional<OnlineCourseConfigurationService> onlineCourseConfigurationService, AuthorizationCheckService authCheckService,
             TutorParticipationRepository tutorParticipationRepository, SubmissionService submissionService, Optional<VcsUserManagementService> optionalVcsUserManagementService,
             AssessmentDashboardService assessmentDashboardService, ExerciseRepository exerciseRepository, Optional<CIUserManagementService> 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<Course> 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()) {

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,15 +54,18 @@ public class AdminCourseResource {
 
     private final Optional<OnlineCourseConfigurationService> onlineCourseConfigurationService;
 
+    private final FilePathService filePathService;
+
     public AdminCourseResource(UserRepository userRepository, CourseService courseService, CourseRepository courseRepository, AuditEventRepository auditEventRepository,
-            FileService fileService, Optional<OnlineCourseConfigurationService> onlineCourseConfigurationService, ChannelService channelService) {
+            FileService fileService, Optional<OnlineCourseConfigurationService> onlineCourseConfigurationService, ChannelService channelService, FilePathService filePathService) {
         this.courseService = courseService;
         this.courseRepository = courseRepository;
         this.auditEventRepository = auditEventRepository;
         this.userRepository = userRepository;
         this.fileService = fileService;
         this.onlineCourseConfigurationService = onlineCourseConfigurationService;
         this.channelService = channelService;
+        this.filePathService = filePathService;
     }
 
     /**
@@ -124,8 +126,9 @@ public ResponseEntity<Course> 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<Void> 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();
     }