Development: Log the duration of the getCompetency REST call

ls1intum · Oct 29, 2023 · 7cb5f1c · 7cb5f1c
1 parent 5b9e4f9
commit 7cb5f1c
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 29 deletions.
diff --git a/src/main/java/de/tum/in/www1/artemis/repository/CompetencyRepository.java b/src/main/java/de/tum/in/www1/artemis/repository/CompetencyRepository.java
@@ -38,14 +38,6 @@ public interface CompetencyRepository extends JpaRepository<Competency, Long> {
             """)
     Set<Competency> findAllForCourseWithProgressForUser(@Param("courseId") Long courseId, @Param("userId") Long userId);
 
-    @Query("""
-            SELECT c
-            FROM Competency c
-                LEFT JOIN FETCH c.exercises ex
-            WHERE c.id = :#{#competencyId}
-            """)
-    Optional<Competency> findByIdWithExercises(@Param("competencyId") long competencyId);
-
     @Query("""
             SELECT c
             FROM Competency c
@@ -61,8 +53,8 @@ public interface CompetencyRepository extends JpaRepository<Competency, Long> {
                 LEFT JOIN FETCH c.exercises
                 LEFT JOIN FETCH c.lectureUnits lu
                 LEFT JOIN FETCH lu.completedUsers
-                LEFT JOIN FETCH lu.lecture l
-                LEFT JOIN FETCH lu.exercise e
+                LEFT JOIN FETCH lu.lecture
+                LEFT JOIN FETCH lu.exercise
             WHERE c.id = :competencyId
             """)
     Optional<Competency> findByIdWithExercisesAndLectureUnits(@Param("competencyId") Long competencyId);
@@ -154,10 +146,6 @@ default Competency findByIdWithLectureUnitsAndCompletionsElseThrow(long competen
         return findByIdWithLectureUnitsAndCompletions(competencyId).orElseThrow(() -> new EntityNotFoundException("Competency", competencyId));
     }
 
-    default Competency findByIdWithExercisesAndLectureUnitsAndCompletionsElseThrow(long competencyId) {
-        return findByIdWithExercisesAndLectureUnitsAndCompletions(competencyId).orElseThrow(() -> new EntityNotFoundException("Competency", competencyId));
-    }
-
     default Competency findByIdWithExercisesAndLectureUnitsBidirectionalElseThrow(long competencyId) {
         return findByIdWithExercisesAndLectureUnitsBidirectional(competencyId).orElseThrow(() -> new EntityNotFoundException("Competency", competencyId));
     }
@@ -178,9 +166,5 @@ default Competency findByIdWithExercisesAndLectureUnitsElseThrow(Long competency
         return findByIdWithExercisesAndLectureUnits(competencyId).orElseThrow(() -> new EntityNotFoundException("Competency", competencyId));
     }
 
-    default Competency findByIdWithExercisesElseThrow(Long competencyId) {
-        return findByIdWithExercises(competencyId).orElseThrow(() -> new EntityNotFoundException("Competency", competencyId));
-    }
-
     long countByCourse(Course course);
 }
diff --git a/src/main/java/de/tum/in/www1/artemis/web/rest/CompetencyResource.java b/src/main/java/de/tum/in/www1/artemis/web/rest/CompetencyResource.java
@@ -30,6 +30,7 @@
 import de.tum.in.www1.artemis.service.CompetencyService;
 import de.tum.in.www1.artemis.service.learningpath.LearningPathService;
 import de.tum.in.www1.artemis.service.util.RoundingUtil;
+import de.tum.in.www1.artemis.service.util.TimeLogUtil;
 import de.tum.in.www1.artemis.web.rest.dto.CourseCompetencyProgressDTO;
 import de.tum.in.www1.artemis.web.rest.dto.PageableSearchDTO;
 import de.tum.in.www1.artemis.web.rest.dto.SearchResultPageDTO;
@@ -131,7 +132,8 @@ public ResponseEntity<List<Competency>> getCompetencies(@PathVariable Long cours
     }
 
     /**
-     * GET /courses/:courseId/competencies/:competencyId : gets the competency with the specified id
+     * GET /courses/:courseId/competencies/:competencyId : gets the competency with the specified id including its related exercises and lecture units
+     * This method also calculates the user progress
      *
      * @param competencyId the id of the competency to retrieve
      * @param courseId     the id of the course to which the competency belongs
@@ -140,21 +142,23 @@ public ResponseEntity<List<Competency>> getCompetencies(@PathVariable Long cours
     @GetMapping("/courses/{courseId}/competencies/{competencyId}")
     @EnforceAtLeastStudent
     public ResponseEntity<Competency> getCompetency(@PathVariable Long competencyId, @PathVariable Long courseId) {
-        log.debug("REST request to get Competency : {}", competencyId);
-        var user = userRepository.getUserWithGroupsAndAuthorities();
+        log.info("REST request to get Competency : {}", competencyId);
+        long start = System.nanoTime();
+        var currentUser = userRepository.getUserWithGroupsAndAuthorities();
         var course = courseRepository.findByIdElseThrow(courseId);
+        // TODO: this database request is quite intense and might lead to memory issues
+        // it loads many completed user objects even if those are not necessary which could include are large object graph
         var competency = competencyRepository.findByIdWithExercisesAndLectureUnitsElseThrow(competencyId);
         checkAuthorizationForCompetency(Role.STUDENT, course, competency);
 
-        competency.setUserProgress(competencyProgressRepository.findByCompetencyIdAndUserId(competencyId, user.getId()).map(Set::of).orElse(Set.of()));
+        competency.setUserProgress(competencyProgressRepository.findByCompetencyIdAndUserId(competencyId, currentUser.getId()).map(Set::of).orElse(Set.of()));
         // Set completion status and remove exercise units (redundant as we also return all exercises)
         competency.setLectureUnits(competency.getLectureUnits().stream().filter(lectureUnit -> !(lectureUnit instanceof ExerciseUnit))
-                .filter(lectureUnit -> authorizationCheckService.isAllowedToSeeLectureUnit(lectureUnit, user)).map(lectureUnit -> {
-                    lectureUnit.setCompleted(lectureUnit.isCompletedFor(user));
-                    return lectureUnit;
-                }).collect(Collectors.toSet()));
-        competency
-                .setExercises(competency.getExercises().stream().filter(exercise -> authorizationCheckService.isAllowedToSeeExercise(exercise, user)).collect(Collectors.toSet()));
+                .filter(lectureUnit -> authorizationCheckService.isAllowedToSeeLectureUnit(lectureUnit, currentUser))
+                .peek(lectureUnit -> lectureUnit.setCompleted(lectureUnit.isCompletedFor(currentUser))).collect(Collectors.toSet()));
+        competency.setExercises(
+                competency.getExercises().stream().filter(exercise -> authorizationCheckService.isAllowedToSeeExercise(exercise, currentUser)).collect(Collectors.toSet()));
+        log.info("getCompetency took {}", TimeLogUtil.formatDurationFrom(start));
         return ResponseEntity.ok().body(competency);
     }
 
@@ -173,7 +177,7 @@ public ResponseEntity<Competency> updateCompetency(@PathVariable Long courseId,
             throw new BadRequestException();
         }
         var course = courseRepository.findByIdElseThrow(courseId);
-        var existingCompetency = this.competencyRepository.findByIdWithLectureUnitsElseThrow(competency.getId());
+        var existingCompetency = competencyRepository.findByIdWithLectureUnitsElseThrow(competency.getId());
         checkAuthorizationForCompetency(Role.INSTRUCTOR, course, existingCompetency);
 
         existingCompetency.setTitle(competency.getTitle());