Skip to content

Commit

Permalink
General: Fix performance issues when opening course archives
Browse files Browse the repository at this point in the history
  • Loading branch information
krusche committed Oct 24, 2024
1 parent d3ddb92 commit 62ef9f5
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import de.tum.cit.aet.artemis.core.domain.CourseInformationSharingConfiguration;
import de.tum.cit.aet.artemis.core.domain.Organization;
import de.tum.cit.aet.artemis.core.domain.User;
import de.tum.cit.aet.artemis.core.dto.CourseForArchiveDTO;
import de.tum.cit.aet.artemis.core.dto.StatisticsEntry;
import de.tum.cit.aet.artemis.core.exception.EntityNotFoundException;
import de.tum.cit.aet.artemis.core.repository.base.ArtemisJpaRepository;
Expand Down Expand Up @@ -547,25 +548,25 @@ SELECT COUNT(c) > 0
* or if they are an admin. Filters out any courses that do not belong to
* a specific semester (i.e., have a null semester).
*
* @param userId The id of the user whose courses are being retrieved
* @param isAdmin A boolean flag indicating whether the user is an admin
* @param groups A set of groups that the user belongs to
* @param now The current time to check if the course is still active
* @return A set of courses that the user has access to and belong to a specific semester
*/
@Query("""
SELECT DISTINCT c
SELECT new de.tum.cit.aet.artemis.core.dto.CourseForArchiveDTO(c.id, c.title, c.semester, c.color, c.courseIcon)
FROM Course c
LEFT JOIN UserGroup ug ON ug.group IN (
c.studentGroupName,
c.teachingAssistantGroupName,
c.editorGroupName,
c.instructorGroupName
)
WHERE (:isAdmin = TRUE OR ug.userId = :userId)
AND c.semester IS NOT NULL
AND c.endDate IS NOT NULL
AND c.endDate < :now
WHERE (:isAdmin = TRUE
OR c.studentGroupName in :groups
OR c.teachingAssistantGroupName in :groups
OR c.editorGroupName in :groups
OR c.instructorGroupName in :groups
)
AND c.semester IS NOT NULL
AND c.endDate IS NOT NULL
AND c.endDate < :now
""")
Set<Course> findInactiveCoursesForUserRolesWithNonNullSemester(@Param("userId") long userId, @Param("isAdmin") boolean isAdmin, @Param("now") ZonedDateTime now);
Set<CourseForArchiveDTO> findInactiveCoursesForUserRolesWithNonNullSemester(@Param("isAdmin") boolean isAdmin, @Param("groups") Set<String> groups,
@Param("now") ZonedDateTime now);

}
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
import de.tum.cit.aet.artemis.core.domain.User;
import de.tum.cit.aet.artemis.core.dto.CourseContentCount;
import de.tum.cit.aet.artemis.core.dto.CourseDeletionSummaryDTO;
import de.tum.cit.aet.artemis.core.dto.CourseForArchiveDTO;
import de.tum.cit.aet.artemis.core.dto.CourseManagementDetailViewDTO;
import de.tum.cit.aet.artemis.core.dto.DueDateStat;
import de.tum.cit.aet.artemis.core.dto.SearchResultPageDTO;
Expand Down Expand Up @@ -698,10 +699,10 @@ public List<Course> getAllCoursesForManagementOverview(boolean onlyActive) {
*
* @return A list of courses for the course archive.
*/
public Set<Course> getAllCoursesForCourseArchive() {
public Set<CourseForArchiveDTO> getAllCoursesForCourseArchive() {
var user = userRepository.getUserWithGroupsAndAuthorities();
boolean isAdmin = authCheckService.isAdmin(user);
return courseRepository.findInactiveCoursesForUserRolesWithNonNullSemester(user.getId(), isAdmin, ZonedDateTime.now());
return courseRepository.findInactiveCoursesForUserRolesWithNonNullSemester(isAdmin, user.getGroups(), ZonedDateTime.now());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -568,15 +568,11 @@ public ResponseEntity<Set<CourseForArchiveDTO>> getCoursesForArchive() {
long start = System.nanoTime();
User user = userRepository.getUserWithGroupsAndAuthorities();
log.debug("REST request to get all inactive courses from previous semesters user {} has access to", user.getLogin());
Set<Course> courses = courseService.getAllCoursesForCourseArchive();
Set<CourseForArchiveDTO> courses = courseService.getAllCoursesForCourseArchive();
log.debug("courseService.getAllCoursesForCourseArchive done");

final Set<CourseForArchiveDTO> dto = courses.stream()
.map(course -> new CourseForArchiveDTO(course.getId(), course.getTitle(), course.getSemester(), course.getColor(), course.getCourseIcon()))
.collect(Collectors.toSet());

log.debug("GET /courses/for-archive took {} for {} courses for user {}", TimeLogUtil.formatDurationFrom(start), courses.size(), user.getLogin());
return ResponseEntity.ok(dto);
log.info("GET /courses/for-archive took {} for {} courses for user {}", TimeLogUtil.formatDurationFrom(start), courses.size(), user.getLogin());
return ResponseEntity.ok(courses);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3400,9 +3400,7 @@ public void testGetAllCoursesForCourseArchiveWithNonNullSemestersAndEndDate() th
expectedOldCourses.get(2).setEndDate(ZonedDateTime.now().minusDays(10));
expectedOldCourses.get(3).setSemester(null); // will be filtered out

for (Course oldCourse : expectedOldCourses) {
courseRepo.save(oldCourse);
}
courseRepo.saveAll(expectedOldCourses);

final Set<CourseForArchiveDTO> actualOldCourses = request.getSet("/api/courses/for-archive", HttpStatus.OK, CourseForArchiveDTO.class);
assertThat(actualOldCourses).as("Course archive has 3 courses").hasSize(3);
Expand Down

0 comments on commit 62ef9f5

Please sign in to comment.