From 1781d62a3acde177b801f7fc43b0c5bb2f6208e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ba=C5=9Fak=20Akan?= Date: Fri, 10 Nov 2023 08:04:47 +0100 Subject: [PATCH] Quiz exercises: Fix LTI Moodle quiz participation results reflection (#7129) --- .../artemis/service/QuizStatisticService.java | 12 +- .../cache/quiz/QuizScheduleService.java | 13 +- .../www1/artemis/LtiQuizIntegrationTest.java | 219 ++++++++++++++++++ .../util/AbstractArtemisIntegrationTest.java | 4 + 4 files changed, 244 insertions(+), 4 deletions(-) create mode 100644 src/test/java/de/tum/in/www1/artemis/LtiQuizIntegrationTest.java diff --git a/src/main/java/de/tum/in/www1/artemis/service/QuizStatisticService.java b/src/main/java/de/tum/in/www1/artemis/service/QuizStatisticService.java index e70c03d620a4..472cc9c09a8c 100644 --- a/src/main/java/de/tum/in/www1/artemis/service/QuizStatisticService.java +++ b/src/main/java/de/tum/in/www1/artemis/service/QuizStatisticService.java @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Optional; import java.util.Set; import org.slf4j.Logger; @@ -10,8 +11,10 @@ import de.tum.in.www1.artemis.domain.Result; import de.tum.in.www1.artemis.domain.participation.Participation; +import de.tum.in.www1.artemis.domain.participation.StudentParticipation; import de.tum.in.www1.artemis.domain.quiz.*; import de.tum.in.www1.artemis.repository.*; +import de.tum.in.www1.artemis.service.connectors.lti.LtiNewResultService; @Service public class QuizStatisticService { @@ -30,15 +33,18 @@ public class QuizStatisticService { private final WebsocketMessagingService websocketMessagingService; + private final Optional ltiNewResultService; + public QuizStatisticService(StudentParticipationRepository studentParticipationRepository, ResultRepository resultRepository, WebsocketMessagingService websocketMessagingService, QuizPointStatisticRepository quizPointStatisticRepository, - QuizQuestionStatisticRepository quizQuestionStatisticRepository, QuizSubmissionRepository quizSubmissionRepository) { + QuizQuestionStatisticRepository quizQuestionStatisticRepository, QuizSubmissionRepository quizSubmissionRepository, Optional ltiNewResultService) { this.studentParticipationRepository = studentParticipationRepository; this.resultRepository = resultRepository; this.quizPointStatisticRepository = quizPointStatisticRepository; this.quizQuestionStatisticRepository = quizQuestionStatisticRepository; this.websocketMessagingService = websocketMessagingService; this.quizSubmissionRepository = quizSubmissionRepository; + this.ltiNewResultService = ltiNewResultService; } /** @@ -93,6 +99,10 @@ public void recalculateStatistics(QuizExercise quizExercise) { var latestUnratedSubmission = quizSubmissionRepository.findWithEagerSubmittedAnswersById(latestUnratedResult.getSubmission().getId()); quizExercise.addResultToAllStatistics(latestUnratedResult, latestUnratedSubmission); } + + if (ltiNewResultService.isPresent()) { + ltiNewResultService.get().onNewResult((StudentParticipation) participation); + } } // save changed Statistics diff --git a/src/main/java/de/tum/in/www1/artemis/service/scheduled/cache/quiz/QuizScheduleService.java b/src/main/java/de/tum/in/www1/artemis/service/scheduled/cache/quiz/QuizScheduleService.java index becdad53d952..db7940dc3cd2 100644 --- a/src/main/java/de/tum/in/www1/artemis/service/scheduled/cache/quiz/QuizScheduleService.java +++ b/src/main/java/de/tum/in/www1/artemis/service/scheduled/cache/quiz/QuizScheduleService.java @@ -43,6 +43,7 @@ import de.tum.in.www1.artemis.service.QuizMessagingService; import de.tum.in.www1.artemis.service.QuizStatisticService; import de.tum.in.www1.artemis.service.WebsocketMessagingService; +import de.tum.in.www1.artemis.service.connectors.lti.LtiNewResultService; import de.tum.in.www1.artemis.service.scheduled.cache.Cache; @Service @@ -72,9 +73,11 @@ public class QuizScheduleService { private final QuizExerciseRepository quizExerciseRepository; + private final Optional ltiNewResultService; + public QuizScheduleService(WebsocketMessagingService websocketMessagingService, StudentParticipationRepository studentParticipationRepository, UserRepository userRepository, QuizSubmissionRepository quizSubmissionRepository, HazelcastInstance hazelcastInstance, QuizExerciseRepository quizExerciseRepository, - QuizMessagingService quizMessagingService, QuizStatisticService quizStatisticService) { + QuizMessagingService quizMessagingService, QuizStatisticService quizStatisticService, Optional ltiNewResultService) { this.websocketMessagingService = websocketMessagingService; this.studentParticipationRepository = studentParticipationRepository; this.userRepository = userRepository; @@ -85,6 +88,7 @@ public QuizScheduleService(WebsocketMessagingService websocketMessagingService, this.scheduledProcessQuizSubmissions = hazelcastInstance.getCPSubsystem().getAtomicReference(HAZELCAST_PROCESS_CACHE_HANDLER); this.threadPoolTaskScheduler = hazelcastInstance.getScheduledExecutorService(Constants.HAZELCAST_QUIZ_SCHEDULER); this.quizCache = new QuizCache(hazelcastInstance); + this.ltiNewResultService = ltiNewResultService; } /** @@ -491,8 +495,11 @@ public void processCachedQuizSubmissions() { log.error("Participation is missing student (or student is missing username): {}", participation); } else { - sendQuizResultToUser(quizExerciseId, participation); - cachedQuiz.getParticipations().remove(entry.getKey()); + if(ltiNewResultService.isPresent()) { + ltiNewResultService.get().onNewResult(participation); + } + sendQuizResultToUser(quizExerciseId, participation); + cachedQuiz.getParticipations().remove(entry.getKey()); } }); if (!finishedParticipations.isEmpty()) { diff --git a/src/test/java/de/tum/in/www1/artemis/LtiQuizIntegrationTest.java b/src/test/java/de/tum/in/www1/artemis/LtiQuizIntegrationTest.java new file mode 100644 index 000000000000..1a017676fb9f --- /dev/null +++ b/src/test/java/de/tum/in/www1/artemis/LtiQuizIntegrationTest.java @@ -0,0 +1,219 @@ +package de.tum.in.www1.artemis; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import java.time.ZonedDateTime; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.parallel.Isolated; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; +import org.springframework.mock.web.MockMultipartFile; +import org.springframework.security.test.context.support.WithMockUser; +import org.springframework.test.web.servlet.MvcResult; +import org.springframework.test.web.servlet.request.MockMultipartHttpServletRequestBuilder; +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; + +import com.fasterxml.jackson.databind.ObjectMapper; + +import de.tum.in.www1.artemis.course.CourseUtilService; +import de.tum.in.www1.artemis.domain.Course; +import de.tum.in.www1.artemis.domain.enumeration.AssessmentType; +import de.tum.in.www1.artemis.domain.enumeration.QuizMode; +import de.tum.in.www1.artemis.domain.quiz.*; +import de.tum.in.www1.artemis.exercise.quizexercise.QuizExerciseFactory; +import de.tum.in.www1.artemis.participation.ParticipationUtilService; +import de.tum.in.www1.artemis.repository.CourseRepository; +import de.tum.in.www1.artemis.repository.ExerciseRepository; +import de.tum.in.www1.artemis.repository.QuizExerciseRepository; +import de.tum.in.www1.artemis.repository.SubmissionRepository; +import de.tum.in.www1.artemis.service.QuizExerciseService; +import de.tum.in.www1.artemis.user.UserUtilService; +import de.tum.in.www1.artemis.web.websocket.QuizSubmissionWebsocketService; + +@Isolated +class LtiQuizIntegrationTest extends AbstractSpringIntegrationBambooBitbucketJiraTest { + + private static final String TEST_PREFIX = "ltiquizsubmissiontest"; + + @Autowired + private CourseRepository courseRepository; + + @Autowired + private ExerciseRepository exerciseRepository; + + @Autowired + private QuizExerciseService quizExerciseService; + + @Autowired + private QuizExerciseRepository quizExerciseRepository; + + @Autowired + private QuizSubmissionWebsocketService quizSubmissionWebsocketService; + + @Autowired + private SubmissionRepository submissionRepository; + + @Autowired + private CourseUtilService courseUtilService; + + @Autowired + private UserUtilService userUtilService; + + @Autowired + private ParticipationUtilService participationUtilService; + + @Autowired + private ObjectMapper objectMapper; + + @BeforeEach + void init() { + // do not use the schedule service based on a time interval in the tests, because this would result in flaky tests that run much slower + quizScheduleService.stopSchedule(); + arrangeLtiServiceMocks(); + } + + @AfterEach + protected void resetSpyBeans() { + super.resetSpyBeans(); + } + + @ParameterizedTest(name = "{displayName} [{index}] {argumentsWithNames}") + @ValueSource(booleans = { true, false }) + @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER") + void testLtiServicesAreCalledUponQuizSubmission(boolean isSubmitted) { + + QuizExercise quizExercise = createSimpleQuizExercise(ZonedDateTime.now().minusMinutes(1), 240); + quizExercise = quizExerciseService.save(quizExercise); + + QuizSubmission quizSubmission = new QuizSubmission(); + for (var question : quizExercise.getQuizQuestions()) { + quizSubmission.addSubmittedAnswers(QuizExerciseFactory.generateSubmittedAnswerForQuizWithCorrectAndFalseAnswers(question)); + } + + userUtilService.addUsers(TEST_PREFIX, 1, 0, 0, 1); + quizSubmission.submitted(isSubmitted); + quizSubmissionWebsocketService.saveSubmission(quizExercise.getId(), quizSubmission, () -> TEST_PREFIX + "student1"); + + assertThat(submissionRepository.countByExerciseIdSubmitted(quizExercise.getId())).isZero(); + quizScheduleService.processCachedQuizSubmissions(); + + verifyNoInteractions(lti10Service); + verifyNoInteractions(lti13Service); + + // End the quiz right now + quizExercise = quizExerciseRepository.findOneWithQuestionsAndStatistics(quizExercise.getId()); + assertThat(quizExercise).isNotNull(); + quizExercise.setDueDate(ZonedDateTime.now()); + exerciseRepository.saveAndFlush(quizExercise); + + quizScheduleService.processCachedQuizSubmissions(); + + verify(lti10Service).onNewResult(any()); + verify(lti13Service).onNewResult(any()); + + } + + @Test + @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR") + void testLtiReevaluateStatistics() throws Exception { + + QuizExercise quizExercise = createQuizExercise(ZonedDateTime.now().plusHours(5)); + quizExercise.setReleaseDate(ZonedDateTime.now().minusHours(5)); + quizExercise.setDueDate(ZonedDateTime.now().minusHours(2)); + + var now = ZonedDateTime.now(); + + // generate submissions for each student + int numberOfParticipants = 10; + userUtilService.addStudents(TEST_PREFIX, 2, 14); + + for (int i = 1; i <= numberOfParticipants; i++) { + QuizSubmission quizSubmission = QuizExerciseFactory.generateSubmissionForThreeQuestions(quizExercise, i, true, now.minusHours(3)); + participationUtilService.addSubmission(quizExercise, quizSubmission, TEST_PREFIX + "student" + i); + participationUtilService.addResultToSubmission(quizSubmission, AssessmentType.AUTOMATIC, null, quizExercise.getScoreForSubmission(quizSubmission), true); + } + + // calculate statistics + QuizExercise quizExerciseWithRecalculatedStatistics = request.get("/api/quiz-exercises/" + quizExercise.getId() + "/recalculate-statistics", HttpStatus.OK, + QuizExercise.class); + + assertThat(quizExerciseWithRecalculatedStatistics.getQuizPointStatistic().getPointCounters()).hasSize(10); + assertThat(quizExerciseWithRecalculatedStatistics.getQuizPointStatistic().getParticipantsRated()).isEqualTo(numberOfParticipants); + + verify(lti10Service, times(10)).onNewResult(any()); + verify(lti13Service, times(10)).onNewResult(any()); + + } + + private QuizExercise createSimpleQuizExercise(ZonedDateTime releaseDate, int duration) { + Course course = courseUtilService.createCourse(); + course.setOnlineCourse(true); + courseRepository.save(course); + + QuizExercise quizExercise = QuizExerciseFactory.createQuiz(course, releaseDate, null, QuizMode.SYNCHRONIZED); + quizExercise.duration(duration); + return quizExercise; + } + + private void arrangeLtiServiceMocks() { + doNothing().when(lti10Service).onNewResult(any()); + doNothing().when(lti13Service).onNewResult(any()); + } + + private QuizExercise createQuizExercise(ZonedDateTime releaseDate) throws Exception { + QuizExercise quizExercise = createSimpleQuizExercise(releaseDate, 3600); + + QuizExercise quizExerciseServer = createQuizExerciseWithFiles(quizExercise, HttpStatus.CREATED); + QuizExercise quizExerciseDatabase = quizExerciseRepository.findOneWithQuestionsAndStatistics(quizExerciseServer.getId()); + assertThat(quizExerciseServer).isNotNull(); + assertThat(quizExerciseDatabase).isNotNull(); + + return quizExerciseDatabase; + } + + private QuizExercise createQuizExerciseWithFiles(QuizExercise quizExercise, HttpStatus expectedStatus) throws Exception { + var builder = MockMvcRequestBuilders.multipart(HttpMethod.POST, "/api/quiz-exercises"); + addFilesToBuilderAndModifyExercise(builder, quizExercise); + builder.file(new MockMultipartFile("exercise", "", MediaType.APPLICATION_JSON_VALUE, objectMapper.writeValueAsBytes(quizExercise))) + .contentType(MediaType.MULTIPART_FORM_DATA); + MvcResult result = request.getMvc().perform(builder).andExpect(status().is(expectedStatus.value())).andReturn(); + request.restoreSecurityContext(); + if (HttpStatus.valueOf(result.getResponse().getStatus()).is2xxSuccessful()) { + assertThat(result.getResponse().getContentAsString()).isNotBlank(); + return objectMapper.readValue(result.getResponse().getContentAsString(), QuizExercise.class); + } + return null; + } + + private void addFilesToBuilderAndModifyExercise(MockMultipartHttpServletRequestBuilder builder, QuizExercise quizExercise) { + int index = 0; + for (var question : quizExercise.getQuizQuestions()) { + if (question instanceof DragAndDropQuestion dragAndDropQuestion) { + String backgroundFileName = "backgroundImage" + index++ + ".jpg"; + dragAndDropQuestion.setBackgroundFilePath(backgroundFileName); + builder.file(new MockMultipartFile("files", backgroundFileName, MediaType.IMAGE_JPEG_VALUE, "backgroundImage".getBytes())); + + for (var dragItem : dragAndDropQuestion.getDragItems()) { + if (dragItem.getPictureFilePath() != null) { + String filename = "dragItemImage" + index++ + ".png"; + dragItem.setPictureFilePath(filename); + builder.file(new MockMultipartFile("files", filename, MediaType.IMAGE_PNG_VALUE, "dragItemImage".getBytes())); + } + } + } + } + } +} diff --git a/src/test/java/de/tum/in/www1/artemis/util/AbstractArtemisIntegrationTest.java b/src/test/java/de/tum/in/www1/artemis/util/AbstractArtemisIntegrationTest.java index 90b646bb96aa..f84835622f72 100644 --- a/src/test/java/de/tum/in/www1/artemis/util/AbstractArtemisIntegrationTest.java +++ b/src/test/java/de/tum/in/www1/artemis/util/AbstractArtemisIntegrationTest.java @@ -23,6 +23,7 @@ import de.tum.in.www1.artemis.service.*; import de.tum.in.www1.artemis.service.connectors.GitService; import de.tum.in.www1.artemis.service.connectors.lti.Lti10Service; +import de.tum.in.www1.artemis.service.connectors.lti.Lti13Service; import de.tum.in.www1.artemis.service.exam.ExamAccessService; import de.tum.in.www1.artemis.service.messaging.InstanceMessageSendService; import de.tum.in.www1.artemis.service.notifications.*; @@ -52,6 +53,9 @@ public abstract class AbstractArtemisIntegrationTest implements MockDelegate { @SpyBean protected Lti10Service lti10Service; + @SpyBean + protected Lti13Service lti13Service; + @SpyBean protected GitService gitService;