Skip to content

Commit

Permalink
General: Improve performance and appearance (#6073)
Browse files Browse the repository at this point in the history
  • Loading branch information
Stephan Krusche authored Jan 15, 2023
1 parent e1a06ea commit 2cd54c2
Show file tree
Hide file tree
Showing 30 changed files with 225 additions and 156 deletions.
4 changes: 2 additions & 2 deletions src/main/java/de/tum/in/www1/artemis/domain/BaseExercise.java
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,9 @@ public void setExampleSolutionPublicationDate(@Nullable ZonedDateTime exampleSol
* @return true, if students are allowed to see this exercise, otherwise false
*/
@JsonView(QuizView.Before.class)
public Boolean isVisibleToStudents() {
public boolean isVisibleToStudents() {
if (releaseDate == null) { // no release date means the exercise is visible to students
return Boolean.TRUE;
return true;
}
return releaseDate.isBefore(ZonedDateTime.now());
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/de/tum/in/www1/artemis/domain/Exercise.java
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ else if (participation.getExercise() instanceof ModelingExercise || participatio
* @return the latest relevant result in the given participation, or null, if none exist
*/
@Nullable
public Submission findLatestSubmissionWithRatedResultWithCompletionDate(Participation participation, Boolean ignoreAssessmentDueDate) {
public Submission findLatestSubmissionWithRatedResultWithCompletionDate(Participation participation, boolean ignoreAssessmentDueDate) {
// for most types of exercises => return latest result (all results are relevant)
Submission latestSubmission = null;
// we get the results over the submissions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ public String getType() {
* @return true if quiz has started, false otherwise
*/
@JsonView(QuizView.Before.class)
public Boolean isQuizStarted() {
public boolean isQuizStarted() {
return isVisibleToStudents();
}

Expand All @@ -174,7 +174,7 @@ public Boolean isQuizStarted() {
* @return true if quiz has ended, false otherwise
*/
@JsonView(QuizView.Before.class)
public Boolean isQuizEnded() {
public boolean isQuizEnded() {
return getDueDate() != null && ZonedDateTime.now().isAfter(getDueDate());
}

Expand All @@ -184,7 +184,7 @@ public Boolean isQuizEnded() {
* @return true if quiz should be filtered, false otherwise
*/
@JsonIgnore
public Boolean shouldFilterForStudents() {
public boolean shouldFilterForStudents() {
return !isQuizEnded();
}

Expand All @@ -194,7 +194,7 @@ public Boolean shouldFilterForStudents() {
* @return true if the quiz is valid, otherwise false
*/
@JsonIgnore
public Boolean isValid() {
public boolean isValid() {
// check title
if (getTitle() == null || getTitle().isEmpty()) {
return false;
Expand Down Expand Up @@ -353,7 +353,7 @@ public StudentParticipation findRelevantParticipation(List<StudentParticipation>

@Override
@Nullable
public Submission findLatestSubmissionWithRatedResultWithCompletionDate(Participation participation, Boolean ignoreAssessmentDueDate) {
public Submission findLatestSubmissionWithRatedResultWithCompletionDate(Participation participation, boolean ignoreAssessmentDueDate) {
// The shouldFilterForStudents() method uses the exercise release/due dates, not the ones of the exam, therefor we can only use them if this exercise is not part of an exam
// In exams, all results should be seen as relevant as they will only be created once the exam is over
if (shouldFilterForStudents() && !isExamExercise()) {
Expand Down Expand Up @@ -712,7 +712,7 @@ else if (batch != null && batch.isSubmissionAllowed()) {
public void validateDates() {
super.validateDates();
quizBatches.forEach(quizBatch -> {
if (quizBatch.getStartTime().isBefore(getReleaseDate())) {
if (quizBatch.getStartTime() != null && quizBatch.getStartTime().isBefore(getReleaseDate())) {
throw new BadRequestAlertException("Start time must not be before release date!", getTitle(), "noValidDates");
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,44 +58,43 @@ public interface CourseRepository extends JpaRepository<Course, Long> {
""")
List<Course> findAllActive(@Param("now") ZonedDateTime now);

// Note: you should not add exercises or exercises+categories here, because this would make the query too complex and would take significantly longer
@EntityGraph(type = LOAD, attributePaths = { "lectures", "lectures.attachments", "exams" })
@Query("""
SELECT DISTINCT c FROM Course c
WHERE (c.startDate <= :now
OR c.startDate IS NULL)
AND (c.endDate >= :now
OR c.endDate IS NULL)
SELECT DISTINCT c
FROM Course c
WHERE (c.startDate <= :now OR c.startDate IS NULL)
AND (c.endDate >= :now OR c.endDate IS NULL)
""")
List<Course> findAllActiveWithLecturesAndExams(@Param("now") ZonedDateTime now);

// Note: you should not add exercises or exercises+categories here, because this would make the query too complex and would take significantly longer
@EntityGraph(type = LOAD, attributePaths = { "lectures", "lectures.attachments", "exams" })
Optional<Course> findWithLecturesAndExamsById(long courseId);

@Query("""
SELECT DISTINCT c FROM Course c
LEFT JOIN FETCH c.tutorialGroups tutorialGroups
LEFT JOIN FETCH tutorialGroups.teachingAssistant tutor
LEFT JOIN FETCH tutorialGroups.registrations registrations
LEFT JOIN FETCH registrations.student student
WHERE (c.startDate <= :#{#now}
OR c.startDate IS NULL)
AND (c.endDate >= :#{#now}
OR c.endDate IS NULL)
AND (student.id = :#{#userId} OR tutor.id = :#{#userId})
SELECT DISTINCT c
FROM Course c
LEFT JOIN FETCH c.tutorialGroups tutorialGroups
LEFT JOIN FETCH tutorialGroups.teachingAssistant tutor
LEFT JOIN FETCH tutorialGroups.registrations registrations
LEFT JOIN FETCH registrations.student student
WHERE (c.startDate <= :now OR c.startDate IS NULL)
AND (c.endDate >= :now OR c.endDate IS NULL)
AND (student.id = :userId OR tutor.id = :userId)
""")
List<Course> findAllActiveWithTutorialGroupsWhereUserIsRegisteredOrTutor(@Param("now") ZonedDateTime now, @Param("userId") Long userId);

@EntityGraph(type = LOAD, attributePaths = { "lectures", "lectures.attachments", "exams" })
Optional<Course> findWithEagerLecturesAndExamsById(long courseId);

// Note: this is currently only used for testing purposes
@Query("""
SELECT DISTINCT c FROM Course c
LEFT JOIN FETCH c.exercises exercises
LEFT JOIN FETCH c.lectures lectures
LEFT JOIN FETCH lectures.attachments
SELECT DISTINCT c
FROM Course c
LEFT JOIN FETCH c.exercises exercises
LEFT JOIN FETCH c.lectures lectures
LEFT JOIN FETCH lectures.attachments
LEFT JOIN FETCH exercises.categories
WHERE (c.startDate <= :now
OR c.startDate IS NULL)
AND (c.endDate >= :now
OR c.endDate IS NULL)
WHERE (c.startDate <= :now OR c.startDate IS NULL)
AND (c.endDate >= :now OR c.endDate IS NULL)
""")
List<Course> findAllActiveWithEagerExercisesAndLectures(@Param("now") ZonedDateTime now);

Expand Down Expand Up @@ -283,7 +282,7 @@ default List<Course> findAllCurrentlyActiveNotOnlineAndRegistrationEnabledWithOr
*/
@NotNull
default Course findByIdWithLecturesAndExamsElseThrow(long courseId) {
return findWithEagerLecturesAndExamsById(courseId).orElseThrow(() -> new EntityNotFoundException("Course", courseId));
return findWithLecturesAndExamsById(courseId).orElseThrow(() -> new EntityNotFoundException("Course", courseId));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,35 +28,48 @@
public interface ExerciseRepository extends JpaRepository<Exercise, Long> {

@Query("""
SELECT e FROM Exercise e
LEFT JOIN FETCH e.categories
WHERE e.course.id = :#{#courseId}
SELECT e
FROM Exercise e
LEFT JOIN FETCH e.categories
WHERE e.course.id = :courseId
""")
Set<Exercise> findByCourseIdWithCategories(@Param("courseId") Long courseId);

@Query("""
SELECT e
FROM Exercise e LEFT JOIN FETCH e.categories WHERE
e.id IN :exerciseIds
SELECT e
FROM Exercise e
LEFT JOIN FETCH e.categories
WHERE e.course.id IN :courseIds
""")
Set<Exercise> findByExerciseIdWithCategories(@Param("exerciseIds") Set<Long> exerciseIds);
Set<Exercise> findByCourseIdsWithCategories(@Param("courseIds") Set<Long> courseIds);

@Query("""
SELECT e FROM Exercise e
WHERE e.course.id = :#{#courseId}
SELECT e
FROM Exercise e
LEFT JOIN FETCH e.categories
WHERE e.id IN :exerciseIds
""")
Set<Exercise> findByExerciseIdsWithCategories(@Param("exerciseIds") Set<Long> exerciseIds);

@Query("""
SELECT e
FROM Exercise e
WHERE e.course.id = :courseId
AND e.mode = 'TEAM'
""")
Set<Exercise> findAllTeamExercisesByCourseId(@Param("courseId") Long courseId);

@Query("""
SELECT e FROM Exercise e
WHERE e.course.id = :#{#courseId}
SELECT e
FROM Exercise e
WHERE e.course.id = :courseId
""")
Set<Exercise> findAllExercisesByCourseId(@Param("courseId") Long courseId);

@Query("""
SELECT e FROM Exercise e
LEFT JOIN FETCH e.learningGoals
SELECT e
FROM Exercise e
LEFT JOIN FETCH e.learningGoals
WHERE e.id = :exerciseId
""")
Optional<Exercise> findByIdWithLearningGoals(@Param("exerciseId") Long exerciseId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ public interface QuizBatchRepository extends JpaRepository<QuizBatch, Long> {
FROM QuizBatch quizBatch
JOIN QuizSubmission submission ON quizBatch.id = submission.quizBatch
JOIN TREAT(submission.participation AS StudentParticipation) participation
WHERE participation.exercise.id = :#{#quizExercise.id}
AND participation.student.login = :#{#studentLogin}
WHERE participation.exercise = :quizExercise
AND participation.student.login = :studentLogin
""")
Set<QuizBatch> findAllByQuizExerciseAndStudentLogin(@Param("quizExercise") QuizExercise quizExercise, @Param("studentLogin") String studentLogin);

Expand Down
37 changes: 24 additions & 13 deletions src/main/java/de/tum/in/www1/artemis/service/CourseService.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@

import de.tum.in.www1.artemis.config.Constants;
import de.tum.in.www1.artemis.domain.*;
import de.tum.in.www1.artemis.domain.enumeration.ExerciseMode;
import de.tum.in.www1.artemis.domain.enumeration.IncludedInOverallScore;
import de.tum.in.www1.artemis.domain.enumeration.NotificationType;
import de.tum.in.www1.artemis.domain.exam.Exam;
Expand All @@ -50,6 +49,7 @@
import de.tum.in.www1.artemis.service.notifications.GroupNotificationService;
import de.tum.in.www1.artemis.service.tutorialgroups.TutorialGroupService;
import de.tum.in.www1.artemis.service.user.UserService;
import de.tum.in.www1.artemis.service.util.TimeLogUtil;
import de.tum.in.www1.artemis.web.rest.dto.CourseManagementDetailViewDTO;
import de.tum.in.www1.artemis.web.rest.dto.DueDateStat;
import de.tum.in.www1.artemis.web.rest.dto.StatsForDashboardDTO;
Expand Down Expand Up @@ -187,9 +187,8 @@ public CourseService(Environment env, ArtemisAuthenticationProvider artemisAuthe
*
* @param courses the courses for which the participations should be fetched
* @param user the user for which the participations should be fetched
* @param startTimeInMillis start time for logging purposes
*/
public void fetchParticipationsWithSubmissionsAndResultsForCourses(List<Course> courses, User user, long startTimeInMillis) {
public void fetchParticipationsWithSubmissionsAndResultsForCourses(List<Course> courses, User user) {
Set<Exercise> exercises = courses.stream().flatMap(course -> course.getExercises().stream()).collect(Collectors.toSet());
List<StudentParticipation> participationsOfUserInExercises = studentParticipationRepository.getAllParticipationsOfUserInExercises(user, exercises);
if (participationsOfUserInExercises.isEmpty()) {
Expand All @@ -206,11 +205,6 @@ public void fetchParticipationsWithSubmissionsAndResultsForCourses(List<Course>
}
}
}
Map<ExerciseMode, List<Exercise>> exercisesGroupedByExerciseMode = exercises.stream().collect(Collectors.groupingBy(Exercise::getMode));
int noOfIndividualExercises = Objects.requireNonNullElse(exercisesGroupedByExerciseMode.get(ExerciseMode.INDIVIDUAL), List.of()).size();
int noOfTeamExercises = Objects.requireNonNullElse(exercisesGroupedByExerciseMode.get(ExerciseMode.TEAM), List.of()).size();
log.info("/courses/for-dashboard.done in {}ms for {} courses with {} individual exercises and {} team exercises for user {}",
System.currentTimeMillis() - startTimeInMillis, courses.size(), noOfIndividualExercises, noOfTeamExercises, user.getLogin());
}

/**
Expand All @@ -225,7 +219,10 @@ public Course findOneWithExercisesAndLecturesAndExamsAndLearningGoalsAndTutorial
if (!authCheckService.isAtLeastStudentInCourse(course, user)) {
throw new AccessForbiddenException();
}
course.setExercises(exerciseService.findAllForCourse(course, user));
// Load exercises with categories separately because this is faster than loading them with lectures and exam above (the query would become too complex)
course.setExercises(exerciseRepository.findByCourseIdWithCategories(course.getId()));
course.setExercises(exerciseService.filterExercisesForCourse(course, user));
exerciseService.loadExerciseDetailsIfNecessary(course, user);
course.setLectures(lectureService.filterActiveAttachments(course.getLectures(), user));
course.setLearningGoals(learningGoalService.findAllForCourse(course, user));
course.setPrerequisites(learningGoalService.findAllPrerequisitesForCourse(course, user));
Expand Down Expand Up @@ -255,21 +252,35 @@ public List<Course> findAllActiveForUser(User user) {
* @return an unmodifiable list of all courses including exercises, lectures and exams for the user
*/
public List<Course> findAllActiveWithExercisesAndLecturesAndExamsForUser(User user) {
long start = System.nanoTime();
var userVisibleCourses = courseRepository.findAllActiveWithLecturesAndExams().stream()
// remove old courses that have already finished
.filter(course -> course.getEndDate() == null || course.getEndDate().isAfter(ZonedDateTime.now()))
// remove courses the user should not be able to see
.filter(course -> isCourseVisibleForUser(user, course));
.filter(course -> isCourseVisibleForUser(user, course)).toList();

log.debug("Find user visible courses finished after {}", TimeLogUtil.formatDurationFrom(start));

// TODO: find all exercises for all courses of the user in one db call to improve the performance, then we would need to map them to the correct course
long startFindAllExercises = System.nanoTime();
var courseIds = userVisibleCourses.stream().map(DomainObject::getId).collect(Collectors.toSet());
Set<Exercise> allExercises = exerciseRepository.findByCourseIdsWithCategories(courseIds);
log.debug("findAllExercisesByCourseIdsWithCategories finished with {} exercises after {}", allExercises.size(), TimeLogUtil.formatDurationFrom(startFindAllExercises));

return userVisibleCourses.peek(course -> {
course.setExercises(exerciseService.findAllForCourse(course, user));
long startFilterAll = System.nanoTime();
var courses = userVisibleCourses.stream().peek(course -> {
// connect the exercises with the course
course.setExercises(allExercises.stream().filter(ex -> ex.getCourseViaExerciseGroupOrCourseMember().getId().equals(course.getId())).collect(Collectors.toSet()));
course.setExercises(exerciseService.filterExercisesForCourse(course, user));
exerciseService.loadExerciseDetailsIfNecessary(course, user);
course.setLectures(lectureService.filterActiveAttachments(course.getLectures(), user));
if (authCheckService.isOnlyStudentInCourse(course, user)) {
course.setExams(examRepository.filterVisibleExams(course.getExams()));
}
}).toList();

log.debug("all {} filterExercisesForCourse individually finished together after {}", courses.size(), TimeLogUtil.formatDurationFrom(startFilterAll));
log.debug("Filter exercises, lectures, and exams finished after {}", TimeLogUtil.formatDurationFrom(start));
return courses;
}

private boolean isCourseVisibleForUser(User user, Course course) {
Expand Down
Loading

0 comments on commit 2cd54c2

Please sign in to comment.