Skip to content

Commit

Permalink
General: Improve performance of exercise participations view
Browse files Browse the repository at this point in the history
  • Loading branch information
krusche committed Nov 25, 2024
1 parent 7cad252 commit a4c674c
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ public ResponseEntity<List<StudentParticipation>> updateParticipationDueDates(@P
}

private Set<StudentParticipation> findParticipationWithLatestResults(Exercise exercise) {
// TODO: we should reduce the amount of data fetched here and sent to the client, because submissions and results are not used at all
// TODO: we should reduce the amount of data fetched here and sent to the client: double check which data is actually required in the exercise scores page
if (exercise.isTeamMode()) {
// For team exercises the students need to be eagerly fetched
return studentParticipationRepository.findByExerciseIdWithLatestAndManualResultsWithTeamInformation(exercise.getId());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,8 @@ Optional<ProgrammingExerciseStudentParticipation> findWithSubmissionsAndEagerStu
@EntityGraph(type = LOAD, attributePaths = { "submissions", "team.students" })
List<ProgrammingExerciseStudentParticipation> findWithSubmissionsById(long participationId);

@EntityGraph(type = LOAD, attributePaths = { "submissions" })
List<ProgrammingExerciseStudentParticipation> findWithSubmissionsByExerciseId(long exerciseId);
@EntityGraph(type = LOAD, attributePaths = { "submissions.results" })
List<ProgrammingExerciseStudentParticipation> findWithSubmissionsAndResultsByExerciseId(long exerciseId);

@EntityGraph(type = LOAD, attributePaths = { "submissions", "team.students" })
List<ProgrammingExerciseStudentParticipation> findWithSubmissionsAndTeamStudentsByExerciseId(long exerciseId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import java.time.ZonedDateTime;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -341,14 +342,16 @@ public Optional<ProgrammingSubmission> getLatestPendingSubmission(Long participa
* @return a Map of {[participationId]: ProgrammingSubmission | null}. Will contain an entry for every student participation of the exercise and a submission object if a
* pending submission exists or null if not.
*/
public Map<Long, Optional<ProgrammingSubmission>> getLatestPendingSubmissionsForProgrammingExercise(Long programmingExerciseId) {
List<ProgrammingExerciseStudentParticipation> participations = programmingExerciseStudentParticipationRepository.findWithSubmissionsByExerciseId(programmingExerciseId);
// TODO: find the latest pending submission directly using Java (the submissions are available now) and not with additional db queries
return participations.stream().collect(Collectors.toMap(Participation::getId, p -> findLatestPendingSubmissionForParticipation(p.getId())));
}

private Optional<ProgrammingSubmission> findLatestPendingSubmissionForParticipation(final long participationId) {
return findLatestPendingSubmissionForParticipation(participationId, false);
public Map<Long, Optional<Submission>> getLatestPendingSubmissionsForProgrammingExercise(Long programmingExerciseId) {
var participations = programmingExerciseStudentParticipationRepository.findWithSubmissionsAndResultsByExerciseId(programmingExerciseId);
return participations.stream().collect(Collectors.toMap(Participation::getId, p -> {
var latestSubmission = p.getSubmissions().stream().max(Comparator.comparing(Submission::getSubmissionDate));
if (latestSubmission.isEmpty() || latestSubmission.get().getLatestResult() != null) {
// This is not an error case, it is very likely that there is no pending submission for a participation.
return Optional.empty();
}
return latestSubmission;
}));
}

private Optional<ProgrammingSubmission> findLatestPendingSubmissionForParticipation(final long participationId, final boolean isGraded) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import de.tum.cit.aet.artemis.core.service.AuthorizationCheckService;
import de.tum.cit.aet.artemis.exam.repository.StudentExamRepository;
import de.tum.cit.aet.artemis.exam.service.ExamService;
import de.tum.cit.aet.artemis.exercise.domain.Submission;
import de.tum.cit.aet.artemis.exercise.domain.participation.Participation;
import de.tum.cit.aet.artemis.exercise.repository.ParticipationRepository;
import de.tum.cit.aet.artemis.exercise.service.ParticipationAuthorizationCheckService;
Expand Down Expand Up @@ -230,17 +231,17 @@ public ResponseEntity<ProgrammingSubmission> getLatestPendingSubmission(@PathVar
*/
@GetMapping("programming-exercises/{exerciseId}/latest-pending-submissions")
@EnforceAtLeastTutor
public ResponseEntity<Map<Long, Optional<ProgrammingSubmission>>> getLatestPendingSubmissionsByExerciseId(@PathVariable Long exerciseId) {
public ResponseEntity<Map<Long, Optional<Submission>>> getLatestPendingSubmissionsByExerciseId(@PathVariable Long exerciseId) {
ProgrammingExercise programmingExercise = programmingExerciseRepository.findByIdWithTemplateAndSolutionParticipationElseThrow(exerciseId);

if (!authCheckService.isAtLeastTeachingAssistantForExercise(programmingExercise)) {
throw new AccessForbiddenException("exercise", exerciseId);
}
// TODO: this REST call is quite slow for > 100 participations. We should consider a more efficient way to get the latest pending submissions.
Map<Long, Optional<ProgrammingSubmission>> pendingSubmissions = submissionService.getLatestPendingSubmissionsForProgrammingExercise(exerciseId);
// TODO: use a different data structure than map here
Map<Long, Optional<Submission>> pendingSubmissions = submissionService.getLatestPendingSubmissionsForProgrammingExercise(exerciseId);
// Remove unnecessary data to make response smaller (exercise, student of participation).
pendingSubmissions = pendingSubmissions.entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, entry -> {
Optional<ProgrammingSubmission> submissionOpt = entry.getValue();
Optional<Submission> submissionOpt = entry.getValue();
// Remove participation, is not needed in the response.
submissionOpt.ifPresent(submission -> submission.setParticipation(null));
return submissionOpt;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ export class ParticipationComponent implements OnInit, OnDestroy {
}

private loadParticipations(exerciseId: number) {
this.participationService.findAllParticipationsByExercise(exerciseId, true).subscribe((participationsResponse) => {
this.participationService.findAllParticipationsByExercise(exerciseId, false).subscribe((participationsResponse) => {
this.participations = participationsResponse.body!;
if (this.exercise.type === ExerciseType.PROGRAMMING) {
const programmingExercise = this.exercise as ProgrammingExercise;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ describe('ParticipationComponent', () => {
expect(exerciseFindStub).toHaveBeenCalledOnce();
expect(exerciseFindStub).toHaveBeenCalledWith(theExercise.id);
expect(participationFindStub).toHaveBeenCalledOnce();
expect(participationFindStub).toHaveBeenCalledWith(theExercise.id, true);
expect(participationFindStub).toHaveBeenCalledWith(theExercise.id, false);
}));

it('should initialize for programming exercise', fakeAsync(() => {
Expand Down Expand Up @@ -154,7 +154,7 @@ describe('ParticipationComponent', () => {
expect(exerciseFindStub).toHaveBeenCalledOnce();
expect(exerciseFindStub).toHaveBeenCalledWith(theExercise.id);
expect(participationFindStub).toHaveBeenCalledOnce();
expect(participationFindStub).toHaveBeenCalledWith(theExercise.id, true);
expect(participationFindStub).toHaveBeenCalledWith(theExercise.id, false);
expect(submissionGetStateStub).toHaveBeenCalledOnce();
expect(submissionGetStateStub).toHaveBeenCalledWith(theExercise.id);
}));
Expand Down

0 comments on commit a4c674c

Please sign in to comment.