From 6328436fbcf45afde2aeec769bf00c9eb938fefe Mon Sep 17 00:00:00 2001 From: Stephan Krusche Date: Tue, 19 Sep 2023 22:51:09 +0200 Subject: [PATCH 1/2] Exam mode: Allow instructors to cleanup archived exams (#7224) --- jest.config.js | 2 +- .../tum/in/www1/artemis/config/Constants.java | 4 ++ .../www1/artemis/service/CourseService.java | 18 ++++--- .../service/ExerciseDeletionService.java | 36 ++++++++------ .../artemis/service/exam/ExamService.java | 39 ++++++++++++++- .../service/exam/StudentExamService.java | 12 ++--- .../ProgrammingExerciseScheduleService.java | 2 +- .../artemis/web/rest/ComplaintResource.java | 2 +- .../www1/artemis/web/rest/CourseResource.java | 8 +-- .../www1/artemis/web/rest/ExamResource.java | 23 +++++++++ .../exam/manage/exam-management.service.ts | 4 ++ .../course-exam-archive-button.component.html | 20 +++++++- .../course-exam-archive-button.component.ts | 49 +++++++++++-------- src/main/webapp/i18n/de/exam.json | 4 ++ .../webapp/i18n/de/programmingExercise.json | 3 +- src/main/webapp/i18n/en/exam.json | 4 ++ .../webapp/i18n/en/programmingExercise.json | 5 ++ ...urse-exam-archive-button.component.spec.ts | 20 ++++---- 18 files changed, 182 insertions(+), 73 deletions(-) diff --git a/jest.config.js b/jest.config.js index 7d307042f585..8d2c47c3acf9 100644 --- a/jest.config.js +++ b/jest.config.js @@ -60,7 +60,7 @@ module.exports = { statements: 85.6, branches: 72.8, functions: 79.4, - lines: 85.9, + lines: 85.8, }, }, coverageReporters: ["clover", "json", "lcov", "text-summary"], diff --git a/src/main/java/de/tum/in/www1/artemis/config/Constants.java b/src/main/java/de/tum/in/www1/artemis/config/Constants.java index 843466925ca3..cc4cc689e911 100644 --- a/src/main/java/de/tum/in/www1/artemis/config/Constants.java +++ b/src/main/java/de/tum/in/www1/artemis/config/Constants.java @@ -176,6 +176,10 @@ public final class Constants { public static final String UNENROLL_FROM_COURSE = "UNENROLL_FROM_COURSE"; + public static final String CLEANUP_COURSE = "CLEANUP_COURSE"; + + public static final String CLEANUP_EXAM = "CLEANUP_EXAM"; + public static final String DELETE_EXERCISE = "DELETE_EXERCISE"; public static final String EDIT_EXERCISE = "EDIT_EXERCISE"; diff --git a/src/main/java/de/tum/in/www1/artemis/service/CourseService.java b/src/main/java/de/tum/in/www1/artemis/service/CourseService.java index 1779a8f796d6..20d367774f94 100644 --- a/src/main/java/de/tum/in/www1/artemis/service/CourseService.java +++ b/src/main/java/de/tum/in/www1/artemis/service/CourseService.java @@ -7,6 +7,7 @@ import java.nio.file.Files; import java.nio.file.Path; +import java.security.Principal; import java.time.DayOfWeek; import java.time.LocalDateTime; import java.time.ZoneId; @@ -782,21 +783,24 @@ public void archiveCourse(Course course) { /** * Cleans up a course by cleaning up all exercises from that course. This deletes all student - * submissions. Note that a course has to be archived first before being cleaned up. + * repositories and build plans. Note that a course has to be archived first before being cleaned up. * - * @param courseId The id of the course to clean up + * @param courseId The id of the course to clean up + * @param principal the user that wants to cleanup the course */ - public void cleanupCourse(Long courseId) { + public void cleanupCourse(Long courseId, Principal principal) { + final var auditEvent = new AuditEvent(principal.getName(), Constants.CLEANUP_COURSE, "course=" + courseId); + auditEventRepository.add(auditEvent); // Get the course with all exercises - var course = courseRepository.findByIdWithExercisesAndLecturesElseThrow(courseId); + var course = courseRepository.findByIdWithEagerExercisesElseThrow(courseId); if (!course.hasCourseArchive()) { log.info("Cannot clean up course {} because it hasn't been archived.", courseId); return; } // The Objects::nonNull is needed here because the relationship exam -> exercise groups is ordered and - // hibernate sometimes adds nulls to in the list of exercise groups to keep the order - Set examExercises = examRepository.findByCourseIdWithExerciseGroupsAndExercises(courseId).stream().map(Exam::getExerciseGroups).flatMap(Collection::stream) + // hibernate sometimes adds nulls into the list of exercise groups to keep the order + Set examExercises = examRepository.findByCourseIdWithExerciseGroupsAndExercises(courseId).stream().flatMap(e -> e.getExerciseGroups().stream()) .filter(Objects::nonNull).map(ExerciseGroup::getExercises).flatMap(Collection::stream).collect(Collectors.toSet()); var exercisesToCleanup = Stream.concat(course.getExercises().stream(), examExercises.stream()).collect(Collectors.toSet()); @@ -804,8 +808,6 @@ public void cleanupCourse(Long courseId) { if (exercise instanceof ProgrammingExercise) { exerciseDeletionService.cleanup(exercise.getId(), true); } - - // TODO: extend exerciseDeletionService.cleanup to clean up all exercise types }); log.info("The course {} has been cleaned up!", courseId); diff --git a/src/main/java/de/tum/in/www1/artemis/service/ExerciseDeletionService.java b/src/main/java/de/tum/in/www1/artemis/service/ExerciseDeletionService.java index 8a5fcdcc5c96..b14f782f8882 100644 --- a/src/main/java/de/tum/in/www1/artemis/service/ExerciseDeletionService.java +++ b/src/main/java/de/tum/in/www1/artemis/service/ExerciseDeletionService.java @@ -3,6 +3,8 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -16,7 +18,6 @@ import de.tum.in.www1.artemis.domain.metis.conversation.Channel; import de.tum.in.www1.artemis.domain.modeling.ModelingExercise; import de.tum.in.www1.artemis.domain.participation.ProgrammingExerciseStudentParticipation; -import de.tum.in.www1.artemis.domain.participation.StudentParticipation; import de.tum.in.www1.artemis.domain.quiz.QuizExercise; import de.tum.in.www1.artemis.repository.*; import de.tum.in.www1.artemis.repository.metis.conversation.ChannelRepository; @@ -89,26 +90,29 @@ public ExerciseDeletionService(ExerciseRepository exerciseRepository, ExerciseUn * @param deleteRepositories if true, the repositories gets deleted */ public void cleanup(Long exerciseId, boolean deleteRepositories) { + log.info("Cleanup all participations for exercise {} in parallel", exerciseId); Exercise exercise = exerciseRepository.findByIdWithStudentParticipationsElseThrow(exerciseId); - log.info("Request to cleanup all participations for Exercise : {}", exercise.getTitle()); + if (!(exercise instanceof ProgrammingExercise)) { + log.warn("Exercise with exerciseId {} is not an instance of ProgrammingExercise. Ignoring the request to cleanup repositories and build plan", exerciseId); + return; + } - if (exercise instanceof ProgrammingExercise) { - for (StudentParticipation participation : exercise.getStudentParticipations()) { + // Cleanup in parallel to speedup the process + var threadPool = Executors.newFixedThreadPool(10); + var futures = exercise.getStudentParticipations().stream().map(participation -> CompletableFuture.runAsync(() -> { + try { participationService.cleanupBuildPlan((ProgrammingExerciseStudentParticipation) participation); - } - - if (!deleteRepositories) { - return; // in this case, we are done - } - - for (StudentParticipation participation : exercise.getStudentParticipations()) { + if (!deleteRepositories) { + return; // in this case, we are done with the participation + } participationService.cleanupRepository((ProgrammingExerciseStudentParticipation) participation); } - - } - else { - log.warn("Exercise with exerciseId {} is not an instance of ProgrammingExercise. Ignoring the request to cleanup repositories and build plan", exerciseId); - } + catch (Exception exception) { + log.error("Failed to clean the student participation {} for programming exercise {}", participation.getId(), exerciseId); + } + }, threadPool).toCompletableFuture()).toArray(CompletableFuture[]::new); + // wait until all operations finish before returning + CompletableFuture.allOf(futures).thenRun(threadPool::shutdown).join(); } /** diff --git a/src/main/java/de/tum/in/www1/artemis/service/exam/ExamService.java b/src/main/java/de/tum/in/www1/artemis/service/exam/ExamService.java index 161c8684657d..909129ef6873 100644 --- a/src/main/java/de/tum/in/www1/artemis/service/exam/ExamService.java +++ b/src/main/java/de/tum/in/www1/artemis/service/exam/ExamService.java @@ -5,6 +5,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.security.Principal; import java.time.ZonedDateTime; import java.util.*; import java.util.stream.Collectors; @@ -16,6 +17,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.actuate.audit.AuditEvent; +import org.springframework.boot.actuate.audit.AuditEventRepository; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.scheduling.annotation.Async; @@ -23,6 +26,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; +import de.tum.in.www1.artemis.config.Constants; import de.tum.in.www1.artemis.domain.*; import de.tum.in.www1.artemis.domain.enumeration.*; import de.tum.in.www1.artemis.domain.exam.Exam; @@ -107,8 +111,12 @@ public class ExamService { private final BonusService bonusService; + private final ExerciseDeletionService exerciseDeletionService; + private final SubmittedAnswerRepository submittedAnswerRepository; + private final AuditEventRepository auditEventRepository; + private final CourseScoreCalculationService courseScoreCalculationService; private final CourseRepository courseRepository; @@ -121,8 +129,8 @@ public ExamService(ExamRepository examRepository, StudentExamRepository studentE ProgrammingExerciseRepository programmingExerciseRepository, QuizExerciseRepository quizExerciseRepository, ResultRepository resultRepository, SubmissionRepository submissionRepository, CourseExamExportService courseExamExportService, GitService gitService, GroupNotificationService groupNotificationService, GradingScaleRepository gradingScaleRepository, PlagiarismCaseRepository plagiarismCaseRepository, AuthorizationCheckService authorizationCheckService, - BonusService bonusService, SubmittedAnswerRepository submittedAnswerRepository, CourseScoreCalculationService courseScoreCalculationService, - CourseRepository courseRepository) { + BonusService bonusService, ExerciseDeletionService exerciseDeletionService, SubmittedAnswerRepository submittedAnswerRepository, + AuditEventRepository auditEventRepository, CourseScoreCalculationService courseScoreCalculationService, CourseRepository courseRepository) { this.examRepository = examRepository; this.studentExamRepository = studentExamRepository; this.userRepository = userRepository; @@ -143,7 +151,9 @@ public ExamService(ExamRepository examRepository, StudentExamRepository studentE this.plagiarismCaseRepository = plagiarismCaseRepository; this.authorizationCheckService = authorizationCheckService; this.bonusService = bonusService; + this.exerciseDeletionService = exerciseDeletionService; this.submittedAnswerRepository = submittedAnswerRepository; + this.auditEventRepository = auditEventRepository; this.courseScoreCalculationService = courseScoreCalculationService; this.courseRepository = courseRepository; this.defaultObjectMapper = new ObjectMapper(); @@ -1307,6 +1317,31 @@ public Page getAllActiveExams(final Pageable pageable, final User user) { ZonedDateTime.now().plusDays(EXAM_ACTIVE_DAYS)); } + /** + * Cleans up an exam by cleaning up all exercises from that course. This deletes all student + * repositories and build plans. Note that an exam has to be archived first before being cleaned up. + * + * @param examId The id of the exam to clean up + * @param principal the user that wants to cleanup the exam + */ + public void cleanupExam(Long examId, Principal principal) { + final var auditEvent = new AuditEvent(principal.getName(), Constants.CLEANUP_EXAM, "exam=" + examId); + auditEventRepository.add(auditEvent); + + // The Objects::nonNull is needed here because the relationship exam -> exercise groups is ordered and + // hibernate sometimes adds nulls into the list of exercise groups to keep the order + Set examExercises = examRepository.findByIdWithExamUsersExerciseGroupsAndExercisesElseThrow(examId).getExerciseGroups().stream().filter(Objects::nonNull) + .map(ExerciseGroup::getExercises).flatMap(Collection::stream).collect(Collectors.toSet()); + + examExercises.forEach(exercise -> { + if (exercise instanceof ProgrammingExercise) { + exerciseDeletionService.cleanup(exercise.getId(), true); + } + }); + + log.info("The exam {} has been cleaned up!", examId); + } + /** * A specialized BiFunction functional interface to provide a simple interface * for passing the data dependencies needed for a Bonus calculation (like source course/exam results). diff --git a/src/main/java/de/tum/in/www1/artemis/service/exam/StudentExamService.java b/src/main/java/de/tum/in/www1/artemis/service/exam/StudentExamService.java index 25a4dae659e9..59ca4e65d5e1 100644 --- a/src/main/java/de/tum/in/www1/artemis/service/exam/StudentExamService.java +++ b/src/main/java/de/tum/in/www1/artemis/service/exam/StudentExamService.java @@ -39,9 +39,7 @@ import de.tum.in.www1.artemis.service.*; import de.tum.in.www1.artemis.service.programming.ProgrammingExerciseParticipationService; import de.tum.in.www1.artemis.service.programming.ProgrammingTriggerService; -import de.tum.in.www1.artemis.service.scheduled.ProgrammingExerciseScheduleService; import de.tum.in.www1.artemis.service.util.ExamExerciseStartPreparationStatus; -import de.tum.in.www1.artemis.service.util.Tuple; import de.tum.in.www1.artemis.web.rest.errors.AccessForbiddenException; import de.tum.in.www1.artemis.web.rest.errors.EntityNotFoundException; @@ -89,8 +87,6 @@ public class StudentExamService { private final WebsocketMessagingService websocketMessagingService; - private final ProgrammingExerciseScheduleService programmingExerciseScheduleService; - private final TaskScheduler scheduler; public StudentExamService(StudentExamRepository studentExamRepository, UserRepository userRepository, ParticipationService participationService, @@ -99,7 +95,7 @@ public StudentExamService(StudentExamRepository studentExamRepository, UserRepos ProgrammingExerciseParticipationService programmingExerciseParticipationService, SubmissionService submissionService, StudentParticipationRepository studentParticipationRepository, ExamQuizService examQuizService, ProgrammingExerciseRepository programmingExerciseRepository, ProgrammingTriggerService programmingTriggerService, ExamRepository examRepository, CacheManager cacheManager, WebsocketMessagingService websocketMessagingService, - ProgrammingExerciseScheduleService programmingExerciseScheduleService, @Qualifier("taskScheduler") TaskScheduler scheduler) { + @Qualifier("taskScheduler") TaskScheduler scheduler) { this.participationService = participationService; this.studentExamRepository = studentExamRepository; this.userRepository = userRepository; @@ -117,7 +113,6 @@ public StudentExamService(StudentExamRepository studentExamRepository, UserRepos this.examRepository = examRepository; this.cacheManager = cacheManager; this.websocketMessagingService = websocketMessagingService; - this.programmingExerciseScheduleService = programmingExerciseScheduleService; this.scheduler = scheduler; } @@ -654,8 +649,9 @@ private void setUpExerciseParticipationsAndSubmissionsWithInitializationDate(Stu // Normally, the locking operation at the end of the exam gets scheduled during the initial unlocking process // (see ProgrammingExerciseScheduleService#scheduleIndividualRepositoryAndParticipationLockTasks) // Since this gets never executed here, we need to manually schedule the locking. - var tupel = new Tuple<>(studentExam.getIndividualEndDate(), programmingParticipation); - programmingExerciseScheduleService.scheduleIndividualRepositoryAndParticipationLockTasks(programmingExercise, Set.of(tupel)); + // TODO: reconsider this edge case and instead send a message to over the instanceMessageSendService + // var tupel = new Tuple<>(studentExam.getIndividualEndDate(), programmingParticipation); + // programmingExerciseScheduleService.scheduleIndividualRepositoryAndParticipationLockTasks(programmingExercise, Set.of(tupel)); } else { programmingExerciseParticipationService.lockStudentParticipation(programmingParticipation); diff --git a/src/main/java/de/tum/in/www1/artemis/service/scheduled/ProgrammingExerciseScheduleService.java b/src/main/java/de/tum/in/www1/artemis/service/scheduled/ProgrammingExerciseScheduleService.java index d2f102d65778..13868c5a89ff 100644 --- a/src/main/java/de/tum/in/www1/artemis/service/scheduled/ProgrammingExerciseScheduleService.java +++ b/src/main/java/de/tum/in/www1/artemis/service/scheduled/ProgrammingExerciseScheduleService.java @@ -803,7 +803,7 @@ public Runnable unlockAllStudentParticipationsWithEarlierStartDateAndLaterDueDat * @param exercise the programming exercise for which the lock is executed * @param individualParticipationsWithDueDates the set of student participations with their individual due dates */ - public void scheduleIndividualRepositoryAndParticipationLockTasks(ProgrammingExercise exercise, + private void scheduleIndividualRepositoryAndParticipationLockTasks(ProgrammingExercise exercise, Set> individualParticipationsWithDueDates) { // 1. Group all participations by due date // TODO: use student exams for safety if some participations are not pre-generated diff --git a/src/main/java/de/tum/in/www1/artemis/web/rest/ComplaintResource.java b/src/main/java/de/tum/in/www1/artemis/web/rest/ComplaintResource.java index 4f03d07a315d..aa44ed9df1b1 100644 --- a/src/main/java/de/tum/in/www1/artemis/web/rest/ComplaintResource.java +++ b/src/main/java/de/tum/in/www1/artemis/web/rest/ComplaintResource.java @@ -236,7 +236,7 @@ public ResponseEntity getNumberOfAllowedComplaintsInCourse(@PathVariable L * exercises * * @param exerciseId the id of the exercise we are interested in - * @param principal that wants to get complaints + * @param principal the user that wants to get complaints * @return the ResponseEntity with status 200 (OK) and a list of complaints. The list can be empty */ @GetMapping("exercises/{exerciseId}/complaints-for-test-run-dashboard") 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 7032f50e1064..a5a26daf1153 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 @@ -6,6 +6,7 @@ import java.io.FileInputStream; import java.io.FileNotFoundException; import java.nio.file.Path; +import java.security.Principal; import java.time.ZonedDateTime; import java.util.*; import java.util.stream.Collectors; @@ -769,12 +770,13 @@ public ResponseEntity downloadCourseArchive(@PathVariable Long courseI /** * DELETE /courses/:course/cleanup : Cleans up a course by deleting all student submissions. * - * @param courseId id of the course to clean up + * @param courseId id of the course to clean up + * @param principal the user that wants to cleanup the course * @return ResponseEntity with status */ @DeleteMapping("courses/{courseId}/cleanup") @EnforceAtLeastInstructor - public ResponseEntity cleanup(@PathVariable Long courseId) { + public ResponseEntity cleanup(@PathVariable Long courseId, Principal principal) { log.info("REST request to cleanup the Course : {}", courseId); final Course course = courseRepository.findByIdElseThrow(courseId); authCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.INSTRUCTOR, course, null); @@ -782,7 +784,7 @@ public ResponseEntity cleanup(@PathVariable Long courseId) { if (!course.hasCourseArchive()) { throw new BadRequestAlertException("Failed to clean up course " + courseId + " because it needs to be archived first.", Course.ENTITY_NAME, "archivenonexistant"); } - courseService.cleanupCourse(courseId); + courseService.cleanupCourse(courseId, principal); return ResponseEntity.ok().build(); } diff --git a/src/main/java/de/tum/in/www1/artemis/web/rest/ExamResource.java b/src/main/java/de/tum/in/www1/artemis/web/rest/ExamResource.java index 3bbeb1559343..9cd4cb4b817f 100644 --- a/src/main/java/de/tum/in/www1/artemis/web/rest/ExamResource.java +++ b/src/main/java/de/tum/in/www1/artemis/web/rest/ExamResource.java @@ -10,6 +10,7 @@ import java.net.URI; import java.net.URISyntaxException; import java.nio.file.Path; +import java.security.Principal; import java.time.ZonedDateTime; import java.time.temporal.ChronoUnit; import java.util.*; @@ -708,6 +709,28 @@ public ResponseEntity> getExamsWithQuizExercisesForUser(@PathVariable } } + /** + * DELETE /courses/{courseId}/exams/{examId}/cleanup : Cleans up an exam by deleting all student submissions. + * + * @param courseId id of the course which in includes the exam + * @param examId id of the exam to clean up + * @param principal the user that wants to cleanup the exam + * @return ResponseEntity with status + */ + @DeleteMapping("courses/{courseId}/exams/{examId}/cleanup") + @EnforceAtLeastInstructor + public ResponseEntity cleanup(@PathVariable Long courseId, @PathVariable Long examId, Principal principal) { + log.info("REST request to cleanup the exam : {}", examId); + final Exam exam = examRepository.findByIdElseThrow(examId); + examAccessService.checkCourseAndExamAccessForInstructorElseThrow(courseId, examId); + // Forbid cleaning the course if no archive has been created + if (!exam.hasExamArchive()) { + throw new BadRequestAlertException("Failed to clean up exam " + examId + " because it needs to be archived first.", ENTITY_NAME, "archivenonexistant"); + } + examService.cleanupExam(examId, principal); + return ResponseEntity.ok().build(); + } + /** * DELETE /courses/{courseId}/exams/{examId} : Delete the exam with the given id. * The delete operation cascades to all student exams, exercise group, exercises and their participations. diff --git a/src/main/webapp/app/exam/manage/exam-management.service.ts b/src/main/webapp/app/exam/manage/exam-management.service.ts index 0c7d40ce654c..a30435c01e4d 100644 --- a/src/main/webapp/app/exam/manage/exam-management.service.ts +++ b/src/main/webapp/app/exam/manage/exam-management.service.ts @@ -492,6 +492,10 @@ export class ExamManagementService { return this.http.put(`${this.resourceUrl}/${courseId}/exams/${examId}/archive`, {}, { observe: 'response' }); } + cleanupExam(courseId: number, examId: number): Observable> { + return this.http.delete(`${this.resourceUrl}/${courseId}/exams/${examId}/cleanup`, { observe: 'response' }); + } + private sendTitlesToEntityTitleService(exam: Exam | undefined | null) { this.entityTitleService.setTitle(EntityType.EXAM, [exam?.id], exam?.title); } diff --git a/src/main/webapp/app/shared/components/course-exam-archive-button/course-exam-archive-button.component.html b/src/main/webapp/app/shared/components/course-exam-archive-button/course-exam-archive-button.component.html index ed1e7d6e872f..3e71c9493215 100644 --- a/src/main/webapp/app/shared/components/course-exam-archive-button/course-exam-archive-button.component.html +++ b/src/main/webapp/app/shared/components/course-exam-archive-button/course-exam-archive-button.component.html @@ -96,7 +96,7 @@