From c9fa6eabf31af49bbc021bfd77106523b2608e89 Mon Sep 17 00:00:00 2001 From: Tobias Lippert <84102468+tobias-lippert@users.noreply.github.com> Date: Thu, 13 Jul 2023 12:10:26 +0200 Subject: [PATCH] Programming exercises: Delete directory for import from file if error occurs (#6864) --- ...grammingExerciseImportFromFileService.java | 37 +++++++++++-------- ...xerciseBitbucketBambooIntegrationTest.java | 9 +++++ ...gExerciseGitlabJenkinsIntegrationTest.java | 13 +++++++ .../ProgrammingExerciseTestService.java | 11 ++++++ 4 files changed, 55 insertions(+), 15 deletions(-) diff --git a/src/main/java/de/tum/in/www1/artemis/service/programming/ProgrammingExerciseImportFromFileService.java b/src/main/java/de/tum/in/www1/artemis/service/programming/ProgrammingExerciseImportFromFileService.java index 5db948e27ffa..2151f85aff20 100644 --- a/src/main/java/de/tum/in/www1/artemis/service/programming/ProgrammingExerciseImportFromFileService.java +++ b/src/main/java/de/tum/in/www1/artemis/service/programming/ProgrammingExerciseImportFromFileService.java @@ -67,22 +67,29 @@ public ProgrammingExercise importProgrammingExerciseFromFile(ProgrammingExercise if (!"zip".equals(FileNameUtils.getExtension(zipFile.getOriginalFilename()))) { throw new BadRequestAlertException("The file is not a zip file", "programmingExercise", "fileNotZip"); } - Path importExerciseDir = Files.createTempDirectory("imported-exercise-dir"); - Path exerciseFilePath = Files.createTempFile(importExerciseDir, "exercise-for-import", ".zip"); - - zipFile.transferTo(exerciseFilePath); - zipFileService.extractZipFileRecursively(exerciseFilePath); - checkRepositoriesExist(importExerciseDir); - var oldShortName = getProgrammingExerciseFromDetailsFile(importExerciseDir).getShortName(); - programmingExerciseService.validateNewProgrammingExerciseSettings(programmingExerciseForImport, course); - var importedProgrammingExercise = programmingExerciseService.createProgrammingExercise(programmingExerciseForImport); - if (Boolean.TRUE.equals(programmingExerciseForImport.isStaticCodeAnalysisEnabled())) { - staticCodeAnalysisService.createDefaultCategories(importedProgrammingExercise); + Path importExerciseDir = null; + ProgrammingExercise importedProgrammingExercise; + try { + importExerciseDir = Files.createTempDirectory("imported-exercise-dir"); + Path exerciseFilePath = Files.createTempFile(importExerciseDir, "exercise-for-import", ".zip"); + + zipFile.transferTo(exerciseFilePath); + zipFileService.extractZipFileRecursively(exerciseFilePath); + checkRepositoriesExist(importExerciseDir); + var oldShortName = getProgrammingExerciseFromDetailsFile(importExerciseDir).getShortName(); + programmingExerciseService.validateNewProgrammingExerciseSettings(programmingExerciseForImport, course); + importedProgrammingExercise = programmingExerciseService.createProgrammingExercise(programmingExerciseForImport); + if (Boolean.TRUE.equals(programmingExerciseForImport.isStaticCodeAnalysisEnabled())) { + staticCodeAnalysisService.createDefaultCategories(importedProgrammingExercise); + } + copyEmbeddedFiles(exerciseFilePath.toAbsolutePath().getParent().resolve(FileNameUtils.getBaseName(exerciseFilePath.toString()))); + importRepositoriesFromFile(importedProgrammingExercise, importExerciseDir, oldShortName); + importedProgrammingExercise.setCourse(course); + } + finally { + // want to make sure the directories are deleted, even if an exception is thrown + fileService.scheduleForDirectoryDeletion(importExerciseDir, 5); } - copyEmbeddedFiles(exerciseFilePath.toAbsolutePath().getParent().resolve(FileNameUtils.getBaseName(exerciseFilePath.toString()))); - importRepositoriesFromFile(importedProgrammingExercise, importExerciseDir, oldShortName); - importedProgrammingExercise.setCourse(course); - fileService.scheduleForDirectoryDeletion(importExerciseDir, 5); return importedProgrammingExercise; } diff --git a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseBitbucketBambooIntegrationTest.java b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseBitbucketBambooIntegrationTest.java index 42d7736181f2..af230abc0aff 100644 --- a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseBitbucketBambooIntegrationTest.java +++ b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseBitbucketBambooIntegrationTest.java @@ -6,6 +6,7 @@ import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.verify; import java.io.IOException; import java.net.URISyntaxException; @@ -341,6 +342,14 @@ void importExerciseFromFileMissingExerciseDetailsJson_badRequest() throws Except programmingExerciseTestService.importFromFile_missingExerciseDetailsJson_badRequest(); } + @Test + @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR") + void importExerciseFromFile_exception_directoryDeleted() throws Exception { + doThrow(new RuntimeException("Error")).when(zipFileService).extractZipFileRecursively(any(Path.class)); + programmingExerciseTestService.importFromFile_exception_DirectoryDeleted(); + verify(fileService).scheduleForDirectoryDeletion(any(Path.class), eq(5L)); + } + @Test @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR") void importExerciseFromFileFile_NoZip_badRequest() throws Exception { diff --git a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseGitlabJenkinsIntegrationTest.java b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseGitlabJenkinsIntegrationTest.java index 1b0fc5facaf7..1c8872c95f8b 100644 --- a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseGitlabJenkinsIntegrationTest.java +++ b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseGitlabJenkinsIntegrationTest.java @@ -3,7 +3,12 @@ import static de.tum.in.www1.artemis.domain.enumeration.ProgrammingLanguage.*; import static de.tum.in.www1.artemis.exercise.programmingexercise.ProgrammingExerciseTestService.studentLogin; import static de.tum.in.www1.artemis.exercise.programmingexercise.ProgrammingSubmissionConstants.GITLAB_PUSH_EVENT_REQUEST; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.verify; +import java.nio.file.Path; import java.util.Arrays; import java.util.stream.Stream; @@ -180,6 +185,14 @@ void importExerciseFromFile_NoZip_badRequest() throws Exception { programmingExerciseTestService.importFromFile_fileNoZip_badRequest(); } + @Test + @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR") + void importExerciseFromFile_exception_directoryDeleted() throws Exception { + doThrow(new RuntimeException("Error")).when(zipFileService).extractZipFileRecursively(any(Path.class)); + programmingExerciseTestService.importFromFile_exception_DirectoryDeleted(); + verify(fileService).scheduleForDirectoryDeletion(any(Path.class), eq(5L)); + } + @Test @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR") void importExerciseFromFileMissingRepository_badRequest() throws Exception { diff --git a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseTestService.java b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseTestService.java index cf7411781199..f578b01365c4 100644 --- a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseTestService.java +++ b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseTestService.java @@ -534,6 +534,17 @@ void importFromFile_missingRepository_BadRequest() throws Exception { ProgrammingExercise.class, HttpStatus.BAD_REQUEST); } + public void importFromFile_exception_DirectoryDeleted() throws Exception { + mockDelegate.mockConnectorRequestForImportFromFile(exercise); + Resource resource = new ClassPathResource("test-data/import-from-file/valid-import.zip"); + + var file = new MockMultipartFile("file", "test.zip", "application/zip", resource.getInputStream()); + var course = courseUtilService.addEmptyCourse(); + exercise.setChannelName("testchannel-pe"); + request.postWithMultipartFile(ROOT + "/courses/" + course.getId() + "/programming-exercises/import-from-file", exercise, "programmingExercise", file, + ProgrammingExercise.class, HttpStatus.INTERNAL_SERVER_ERROR); + } + // TEST void createProgrammingExercise_validExercise_withStaticCodeAnalysis(ProgrammingLanguage language, ProgrammingLanguageFeature programmingLanguageFeature) throws Exception { exercise.setStaticCodeAnalysisEnabled(true);