Merge branch 'develop' of github.com:ls1intum/Artemis into bugfix/iri…

…s/event-service-exception

src/main/java/de/tum/cit/aet/artemis/core/config/MetricsBean.java

@@ -108,8 +108,8 @@ public class MetricsBean {
     private final Optional<SharedQueueManagementService> localCIBuildJobQueueService;
 
     /**
-     * List that stores active usernames (users with a submission within the last 14 days) which is refreshed
-     * every 60 minutes.
+     * List that stores active usernames (users with a submission within the last 14 days) which is refreshed every 60 minutes.
+     * NOTE: only active on scheduling node
      */
     private List<String> cachedActiveUserNames;
 
@@ -122,38 +122,54 @@ public class MetricsBean {
 
     private final AtomicInteger examsGauge = new AtomicInteger(0);
 
+    // NOTE: only active on scheduling node
     private MultiGauge activeUserMultiGauge;
 
+    // NOTE: only active on scheduling node
     private MultiGauge studentsCourseGauge;
 
+    // NOTE: only active on scheduling node
     private MultiGauge studentsExamGauge;
 
     // Internal metrics: Exercises
+    // NOTE: only active on scheduling node
     private MultiGauge exerciseGauge;
 
+    // NOTE: only active on scheduling node
     private MultiGauge activeExerciseGauge;
 
+    // NOTE: only active on scheduling node
     private MultiGauge dueExerciseGauge;
 
+    // NOTE: only active on scheduling node
     private MultiGauge dueExerciseStudentMultiplierGauge;
 
+    // NOTE: only active on scheduling node
     private MultiGauge dueExerciseStudentMultiplierActive14DaysGauge;
 
+    // NOTE: only active on scheduling node
     private MultiGauge releaseExerciseGauge;
 
+    // NOTE: only active on scheduling node
     private MultiGauge releaseExerciseStudentMultiplierGauge;
 
+    // NOTE: only active on scheduling node
     private MultiGauge releaseExerciseStudentMultiplierActive14DaysGauge;
 
     // Internal metrics: Exams
+    // NOTE: only active on scheduling node
     private MultiGauge dueExamGauge;
 
+    // NOTE: only active on scheduling node
     private MultiGauge dueExamStudentMultiplierGauge;
 
+    // NOTE: only active on scheduling node
     private MultiGauge releaseExamGauge;
 
+    // NOTE: only active on scheduling node
     private MultiGauge releaseExamStudentMultiplierGauge;
 
+    // NOTE: only active on scheduling node
     private MultiGauge activeAdminsGauge;
 
     private boolean scheduledMetricsEnabled = false;
@@ -213,10 +229,12 @@ public void applicationReady() {
 
         if (profileService.isSchedulingActive()) {
             // Should only be activated if the scheduling profile is present, because these metrics are the same for all instances
-            this.scheduledMetricsEnabled = true;
+            scheduledMetricsEnabled = true;
 
             registerActiveAdminMetrics();
             registerExerciseAndExamMetrics();
+            registerStudentExerciseMetrics();
+            registerStudentExamMetrics();
             registerPublicArtemisMetrics();
 
             // Initial calculation is done in constructor to ensure the values are present before the first metrics are calculated
@@ -245,6 +263,7 @@ public void applicationReady() {
         }
     }
 
+    // This is ALWAYS active on all nodes
     private void registerHealthContributors(List<HealthContributor> healthContributors) {
         // Publish the health status for each HealthContributor one Gauge with name ARTEMIS_HEALTH_NAME that has several values (one for each HealthIndicator),
         // using different values for the ARTEMIS_HEALTH_TAG tag
@@ -307,6 +326,7 @@ private static int extractMaxConcurrentBuilds(Optional<SharedQueueManagementServ
                 .reduce(0, Integer::sum)).orElse(0);
     }
 
+    // This is ALWAYS active on all nodes
     private void registerWebsocketMetrics() {
         // Publish the number of currently (via WebSockets) connected sessions
         Gauge.builder("artemis.instance.websocket.sessions", webSocketHandler, MetricsBean::extractWebsocketSessionCount).strongReference(true)
@@ -338,6 +358,7 @@ private static double extractWebsocketSubscriptionCount(SimpUserRegistry userReg
 
     /**
      * Register metrics for exercises and exams
+     * NOTE: only active on scheduling node
      */
     private void registerExerciseAndExamMetrics() {
         dueExerciseGauge = MultiGauge.builder("artemis.scheduled.exercises.due.count").description("Number of exercises ending within the next minutes").register(meterRegistry);
@@ -346,13 +367,11 @@ private void registerExerciseAndExamMetrics() {
 
         dueExamGauge = MultiGauge.builder("artemis.scheduled.exams.due.count").description("Number of exams ending within the next minutes").register(meterRegistry);
         releaseExamGauge = MultiGauge.builder("artemis.scheduled.exams.release.count").description("Number of exams starting within the next minutes").register(meterRegistry);
-
-        registerStudentExerciseMetrics();
-        registerStudentExamMetrics();
     }
 
     /**
      * Register metrics for exercises, multiplied with the student that are enrolled for the exercise
+     * NOTE: only active on scheduling node
      */
     private void registerStudentExerciseMetrics() {
         dueExerciseStudentMultiplierGauge = MultiGauge.builder("artemis.scheduled.exercises.due.student_multiplier")
@@ -369,6 +388,7 @@ private void registerStudentExerciseMetrics() {
 
     /**
      * Register metrics for exams, multiplied with the student that are enrolled for the exam
+     * NOTE: only active on scheduling node
      */
     private void registerStudentExamMetrics() {
         dueExamStudentMultiplierGauge = MultiGauge.builder("artemis.scheduled.exams.due.student_multiplier")
@@ -377,18 +397,22 @@ private void registerStudentExamMetrics() {
                 .description("Number of exams starting within the next minutes multiplied with students in the course").register(meterRegistry);
     }
 
+    // NOTE: only active on scheduling node
     private void registerActiveAdminMetrics() {
         activeAdminsGauge = MultiGauge.builder("artemis.users.admins.active").description("User logins of active admin accounts").register(meterRegistry);
     }
 
     /**
      * Calculate active users (active within the last 14 days) and store them in a List.
-     * The calculation is performed every 60 minutes.
-     * The initial calculation is done in the constructor to ensure it is done BEFORE {@link #recalculateMetrics()}
-     * is called.
+     * The calculation is performed every 60 minutes and should only be done on the scheduling node
+     * The initial calculation is done in the constructor to ensure it is done BEFORE {@link #recalculateMetrics()} is called.
+     * Only executed if the "scheduling"-profile is present.
      */
     @Scheduled(fixedRate = 60 * 60 * 1000, initialDelay = 60 * 60 * 1000) // Every 60 minutes
     public void calculateActiveUserMetrics() {
+        if (!scheduledMetricsEnabled) {
+            return;
+        }
         var startDate = System.currentTimeMillis();
 
         // The authorization object has to be set because this method is not called by a user but by the scheduler
@@ -537,6 +561,7 @@ private void updateMultiGaugeIntegerForMinuteRanges(MultiGauge multiGauge, NowEn
 
     /**
      * Register publicly exposed metrics.
+     * NOTE: only active on scheduling node
      */
     private void registerPublicArtemisMetrics() {
         SecurityUtils.setAuthorizationObject();

src/main/java/de/tum/cit/aet/artemis/exam/service/ExamAccessService.java

@@ -60,17 +60,17 @@ public ExamAccessService(ExamRepository examRepository, StudentExamRepository st
     }
 
     /**
-     * Real Exams: Checks if the current user is allowed to see the requested exam. If he is allowed the exam will be returned.
+     * TODO: we should distinguish the whole method between test exam and real exam to improve the readability of the code
+     * Real Exams: Checks if the current user is allowed to see the requested exam. If he is allowed the student exam will be returned (Fallback: create a new one)
      * Test Exams: Either retrieves an existing StudentExam from the Database or generates a new StudentExam
      *
      * @param courseId The id of the course
      * @param examId   The id of the exam
      * @return a ResponseEntity with the exam
      */
-    public StudentExam getExamInCourseElseThrow(Long courseId, Long examId) {
+    public StudentExam getOrCreateStudentExamElseThrow(Long courseId, Long examId) {
         User currentUser = userRepository.getUserWithGroupsAndAuthorities();
 
-        // TODO: we should distinguish the whole method between test exam and real exam to improve the readability of the code
         // Check that the current user is at least student in the course.
         Course course = courseRepository.findByIdElseThrow(courseId);
         authorizationCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.STUDENT, course, currentUser);
@@ -79,30 +79,39 @@ public StudentExam getExamInCourseElseThrow(Long courseId, Long examId) {
         Optional<StudentExam> optionalStudentExam = studentExamRepository.findByExamIdAndUserId(examId, currentUser.getId());
 
         StudentExam studentExam;
-        // If an studentExam can be fund, we can proceed
+        // If an studentExam can be found, we can immediately proceed
         if (optionalStudentExam.isPresent()) {
             studentExam = optionalStudentExam.get();
         }
         else {
-            Exam examWithExerciseGroupsAndExercises = examRepository.findWithExerciseGroupsAndExercisesByIdOrElseThrow(examId);
+            Exam exam = examRepository.findWithExerciseGroupsAndExercisesByIdOrElseThrow(examId);
+            ZonedDateTime now = ZonedDateTime.now();
+            ZonedDateTime unlockDate = ExamDateService.getExamProgrammingExerciseUnlockDate(exam);
+
             // An exam can be started 5 minutes before the start time, which is when programming exercises are unlocked
-            boolean canExamBeStarted = ZonedDateTime.now().isAfter(ExamDateService.getExamProgrammingExerciseUnlockDate(examWithExerciseGroupsAndExercises));
-            boolean isExamEnded = ZonedDateTime.now().isAfter(examWithExerciseGroupsAndExercises.getEndDate());
+            boolean canExamBeStarted = now.isAfter(unlockDate);
+            boolean isTestExam = exam.isTestExam();
+            boolean isUserRegistered = examRegistrationService.isUserRegisteredForExam(examId, currentUser.getId());
+            boolean isExamEnded = ZonedDateTime.now().isAfter(exam.getEndDate());
             // Generate a student exam if the following conditions are met:
             // 1. The exam has not ended.
             // 2. The exam is either a test exam, OR it is a normal exam where the user is registered and can click the start button.
             // Allowing student exams to be generated only when students can click the start button prevents inconsistencies.
             // For example, this avoids a scenario where a student generates an exam and an instructor adds an exercise group afterward.
-            if (!isExamEnded
-                    && (examWithExerciseGroupsAndExercises.isTestExam() || (examRegistrationService.isUserRegisteredForExam(examId, currentUser.getId()) && canExamBeStarted))) {
-                studentExam = studentExamService.generateIndividualStudentExam(examWithExerciseGroupsAndExercises, currentUser);
-                // For the start of the exam, the exercises are not needed. They are later loaded via StudentExamResource
-                studentExam.setExercises(null);
 
+            if (isExamEnded) {
+                throw new BadRequestAlertException("The exam has already ended. Cannot generate student exam.", ENTITY_NAME, "examEnded", true);
+            }
+            if (!isTestExam && !isUserRegistered) {
+                throw new AccessForbiddenException("User is not registered for the exam. Cannot generate student exam.");
             }
+            if (!canExamBeStarted) {
+                throw new AccessForbiddenException("The exam cannot be started yet. Cannot generate student exam.");
+            }
+            // Proceed only if the exam has not ended and the user meets the conditions
             else {
-                // We skip the alert since this can happen when a tutor sees the exam card or the user did not participate yet is registered for the exam
-                throw new BadRequestAlertException("Cannot generate student exam for exam ID " + examId + ".", ENTITY_NAME, "cannotGenerateStudentExam", true);
+                studentExam = studentExamService.generateIndividualStudentExam(exam, currentUser);
+                studentExam.setExercises(null);
             }
         }
 
@@ -111,7 +120,7 @@ public StudentExam getExamInCourseElseThrow(Long courseId, Long examId) {
         checkExamBelongsToCourseElseThrow(courseId, exam);
 
         if (!examId.equals(exam.getId())) {
-            throw new BadRequestAlertException("The provided examId does not match with the examId of the studentExam", ENTITY_NAME, "examIdMismatch");
+            throw new ConflictException("The provided examId does not match with the examId of the studentExam", ENTITY_NAME, "examIdMismatch");
         }
 
         // Check that the exam is visible

src/main/java/de/tum/cit/aet/artemis/exam/web/ExamResource.java

@@ -1101,7 +1101,7 @@ public ResponseEntity<Void> removeAllStudentsFromExam(@PathVariable Long courseI
      * GET /courses/{courseId}/exams/{examId}/own-student-exam: Get the own student exam for the exam
      * Real Exams: StudentExam needs to be generated by an instructor
      * Test Exam: StudentExam can be self-created by the user
-     * Note: The Access control is performed in the {@link ExamAccessService#getExamInCourseElseThrow(Long, Long)} to limit the DB-calls
+     * Note: The Access control is performed in the {@link ExamAccessService#getOrCreateStudentExamElseThrow(Long, Long)} to limit the DB-calls
      *
      * @param courseId the id of the course
      * @param examId   the id of the exam
@@ -1111,7 +1111,7 @@ public ResponseEntity<Void> removeAllStudentsFromExam(@PathVariable Long courseI
     @EnforceAtLeastStudent
     public ResponseEntity<StudentExam> getOwnStudentExam(@PathVariable Long courseId, @PathVariable Long examId) {
         log.debug("REST request to get exam {} for conduction", examId);
-        StudentExam exam = examAccessService.getExamInCourseElseThrow(courseId, examId);
+        StudentExam exam = examAccessService.getOrCreateStudentExamElseThrow(courseId, examId);
         exam.getUser().setVisibleRegistrationNumber();
         return ResponseEntity.ok(exam);
     }

src/test/java/de/tum/cit/aet/artemis/exam/ExamIntegrationTest.java

@@ -1040,7 +1040,7 @@ void testGetStudentExamForStart() throws Exception {
         exam.setVisibleDate(ZonedDateTime.now().minusHours(1).minusMinutes(5));
         StudentExam response = request.get("/api/courses/" + course1.getId() + "/exams/" + exam.getId() + "/own-student-exam", HttpStatus.OK, StudentExam.class);
         assertThat(response.getExam()).isEqualTo(exam);
-        verify(examAccessService).getExamInCourseElseThrow(course1.getId(), exam.getId());
+        verify(examAccessService).getOrCreateStudentExamElseThrow(course1.getId(), exam.getId());
     }
 
     @ParameterizedTest(name = "{displayName} [{index}] {argumentsWithNames}")
@@ -1460,13 +1460,13 @@ void testRetrieveOwnStudentExam_noStudentExam() throws Exception {
         examUser1 = examUserRepository.save(examUser1);
         exam.addExamUser(examUser1);
         examRepository.save(exam);
-        request.get("/api/courses/" + course1.getId() + "/exams/" + exam1.getId() + "/own-student-exam", HttpStatus.BAD_REQUEST, StudentExam.class);
+        request.get("/api/courses/" + course1.getId() + "/exams/" + exam1.getId() + "/own-student-exam", HttpStatus.FORBIDDEN, StudentExam.class);
     }
 
     @Test
     @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
     void testRetrieveOwnStudentExam_instructor() throws Exception {
-        request.get("/api/courses/" + course1.getId() + "/exams/" + exam1.getId() + "/own-student-exam", HttpStatus.BAD_REQUEST, StudentExam.class);
+        request.get("/api/courses/" + course1.getId() + "/exams/" + exam1.getId() + "/own-student-exam", HttpStatus.FORBIDDEN, StudentExam.class);
     }
 
     @Test

src/test/java/de/tum/cit/aet/artemis/exam/TestExamIntegrationTest.java

@@ -218,9 +218,20 @@ void testGetStudentExamForTestExamForStart_notVisible() throws Exception {
 
     @Test
     @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER")
-    void testGetStudentExamForTestExamForStart_ExamDoesNotBelongToCourse() throws Exception {
+    void testGetStudentExamForTestExamForStart_tooEarly() throws Exception {
         Exam testExam = examUtilService.addTestExam(course2);
+        testExam.setStartDate(now().plusMinutes(10));
+        testExam = examRepository.save(testExam);
+        // the request fails because the exam start is 10 min in the future
+        request.get("/api/courses/" + course1.getId() + "/exams/" + testExam.getId() + "/own-student-exam", HttpStatus.FORBIDDEN, StudentExam.class);
+    }
 
+    @Test
+    @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER")
+    void testGetStudentExamForTestExamForStart_ExamDoesNotBelongToCourse() throws Exception {
+        Exam testExam = examUtilService.addTestExam(course2);
+        testExam.setStartDate(now().plusMinutes(4));
+        testExam = examRepository.save(testExam);
         request.get("/api/courses/" + course1.getId() + "/exams/" + testExam.getId() + "/own-student-exam", HttpStatus.CONFLICT, StudentExam.class);
     }