Quiz exercises: Fix an issue with quiz participations on multi node environments (#12578)

krusche · web-flow · commit 4f33aa774448 · 2026-04-23T12:03:38.000+02:00
diff --git a/src/main/java/de/tum/cit/aet/artemis/quiz/domain/DragAndDropSubmittedAnswer.java b/src/main/java/de/tum/cit/aet/artemis/quiz/domain/DragAndDropSubmittedAnswer.java
@@ -11,9 +11,6 @@
 import jakarta.persistence.JoinColumn;
 import jakarta.persistence.OneToMany;
 
-import org.hibernate.annotations.Cache;
-import org.hibernate.annotations.CacheConcurrencyStrategy;
-
 import com.fasterxml.jackson.annotation.JsonInclude;
 
 import de.tum.cit.aet.artemis.quiz.domain.compare.DnDMapping;
@@ -26,10 +23,11 @@
 @JsonInclude(JsonInclude.Include.NON_EMPTY)
 public class DragAndDropSubmittedAnswer extends SubmittedAnswer {
 
-    // NOTE: this relation cannot be bidirectional, because it would otherwise be ManyToMany
+    // NOTE: this relation cannot be bidirectional, because it would otherwise be ManyToMany.
+    // No @Cache here on purpose — same reason as MultipleChoiceSubmittedAnswer.selectedOptions (see #12574): NONSTRICT_READ_WRITE on an
+    // actively-mutated collection produced non-deterministic reads under concurrent autosave / evaluation activity on the clustered setup.
     @OneToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER, orphanRemoval = true)
     @JoinColumn(name = "submitted_answer_id")
-    @Cache(usage = CacheConcurrencyStrategy.NONSTRICT_READ_WRITE)
     private Set<DragAndDropMapping> mappings = new HashSet<>();
 
     public Set<DragAndDropMapping> getMappings() {
diff --git a/src/main/java/de/tum/cit/aet/artemis/quiz/domain/MultipleChoiceSubmittedAnswer.java b/src/main/java/de/tum/cit/aet/artemis/quiz/domain/MultipleChoiceSubmittedAnswer.java
@@ -12,9 +12,6 @@
 import jakarta.persistence.JoinTable;
 import jakarta.persistence.ManyToMany;
 
-import org.hibernate.annotations.Cache;
-import org.hibernate.annotations.CacheConcurrencyStrategy;
-
 import com.fasterxml.jackson.annotation.JsonInclude;
 
 import de.tum.cit.aet.artemis.core.domain.DomainObject;
@@ -27,8 +24,10 @@
 @JsonInclude(JsonInclude.Include.NON_EMPTY)
 public class MultipleChoiceSubmittedAnswer extends SubmittedAnswer {
 
+    // No @Cache here on purpose. A second-level cache with NONSTRICT_READ_WRITE used to produce partial / stale selected-option collections
+    // under concurrent activity (autosave, evaluation) on a clustered setup, see Artemis issue #12574. Selected options are small and
+    // rarely re-read for the same submission, so always fetching from the database is both simpler and deterministic.
     @ManyToMany(fetch = FetchType.EAGER)
-    @Cache(usage = CacheConcurrencyStrategy.NONSTRICT_READ_WRITE)
     @JoinTable(name = "multiple_choice_submitted_answer_selected_options", joinColumns = @JoinColumn(name = "multiple_choice_submitted_answers_id", referencedColumnName = "id"), inverseJoinColumns = @JoinColumn(name = "selected_options_id", referencedColumnName = "id"))
     private Set<AnswerOption> selectedOptions = new HashSet<>();
 
diff --git a/src/main/java/de/tum/cit/aet/artemis/quiz/domain/ShortAnswerSubmittedAnswer.java b/src/main/java/de/tum/cit/aet/artemis/quiz/domain/ShortAnswerSubmittedAnswer.java
@@ -12,9 +12,6 @@
 import jakarta.persistence.OneToMany;
 import jakarta.validation.Valid;
 
-import org.hibernate.annotations.Cache;
-import org.hibernate.annotations.CacheConcurrencyStrategy;
-
 import com.fasterxml.jackson.annotation.JsonInclude;
 
 import de.tum.cit.aet.artemis.quiz.domain.compare.SAMapping;
@@ -27,10 +24,11 @@
 @JsonInclude(JsonInclude.Include.NON_EMPTY)
 public class ShortAnswerSubmittedAnswer extends SubmittedAnswer {
 
-    // NOTE: this relation cannot be bidirectional, because it would otherwise be ManyToMany
+    // NOTE: this relation cannot be bidirectional, because it would otherwise be ManyToMany.
+    // No @Cache here on purpose — same reason as MultipleChoiceSubmittedAnswer.selectedOptions (see #12574): NONSTRICT_READ_WRITE on an
+    // actively-mutated collection produced non-deterministic reads under concurrent autosave / evaluation activity on the clustered setup.
     @OneToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER, orphanRemoval = true)
     @JoinColumn(name = "submitted_answer_id")
-    @Cache(usage = CacheConcurrencyStrategy.NONSTRICT_READ_WRITE)
     @Valid
     private Set<ShortAnswerSubmittedText> submittedTexts = new HashSet<>();
 
diff --git a/src/main/java/de/tum/cit/aet/artemis/quiz/repository/QuizSubmissionRepository.java b/src/main/java/de/tum/cit/aet/artemis/quiz/repository/QuizSubmissionRepository.java
@@ -51,7 +51,7 @@ public interface QuizSubmissionRepository extends ArtemisJpaRepository<QuizSubmi
     List<QuizSubmission> findWithEagerSubmittedAnswersByParticipationId(long participationId);
 
     @Query("""
-            SELECT submission
+            SELECT DISTINCT submission
             FROM QuizSubmission submission
                 LEFT JOIN FETCH submission.submittedAnswers
                 JOIN submission.results r
diff --git a/src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizSubmissionService.java b/src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizSubmissionService.java
@@ -279,13 +279,21 @@ public QuizSubmission saveSubmissionForLiveMode(Long exerciseId, QuizSubmission
         String logText = submitted ? "submit quiz in live mode:" : "save quiz in live mode:";
 
         long start = System.nanoTime();
-        var quizExercise = quizExerciseRepository.findByIdElseThrow(exerciseId);
+        // Load questions eagerly so we can sanitize the client-submitted references below (answer options, drag items,
+        // drop locations, short-answer spots). Without this, a stale id from the client causes Hibernate's merge to fail
+        // the whole save with ObjectNotFoundException — see #12584.
+        var quizExercise = quizExerciseRepository.findByIdWithQuestionsElseThrow(exerciseId);
         quizExercise.setQuizBatches(null);
         // A submission always exists because the user has to start the participation before submitting, which creates a submission
         var existingSubmission = quizSubmissionRepository.findByExerciseIdAndStudentLogin(quizExercise.getId(), userLogin)
                 .orElseThrow(() -> new EntityNotFoundException("Cannot find quiz submission for exercise " + exerciseId + " and user " + userLogin));
         checkSubmissionForLiveModeOrThrow(quizExercise, existingSubmission, userLogin, logText, start);
 
+        // Replace client-supplied references with server-managed ones, and drop any that no longer exist. This protects
+        // the downstream Hibernate merge from stale ids (e.g. after a quiz was re-imported) that would otherwise trigger
+        // ObjectNotFoundException and abort the entire save.
+        sanitizeSubmittedAnswersAgainstQuestions(quizSubmission, quizExercise);
+
         // TODO: ideally we only save if something has changed, we can use "if (!isContentEqualTo(existingSubmission, quizSubmission))"
 
         // recreate pointers back to submission in each submitted answer
@@ -369,6 +377,123 @@ private void checkSubmissionForLiveModeOrThrow(QuizExercise quizExercise, QuizSu
         // but for performance reasons the checks may have to be done in the quiz submission service where no feedback for the students can be generated
     }
 
+    /**
+     * Replace every client-supplied reference inside the submitted answers (question, answer option, drag item, drop location, short-answer spot) with the server-side managed
+     * instance loaded from {@code quizExercise}. Any reference that does not exist server-side is dropped. Submitted answers whose question is no longer part of the quiz are
+     * removed entirely.
+     *
+     * <p>
+     * This is a defensive sanitization: the live-submit endpoint still accepts a raw {@link QuizSubmission} from the client (see the TODO on
+     * {@code QuizSubmissionResource.saveOrSubmitForLiveMode}). Without this step a single stale id — e.g. after a quiz was re-imported between tab open and submit — causes the
+     * downstream Hibernate merge to abort the whole request with {@code ObjectNotFoundException} when it tries to resolve the reference (#12584).
+     */
+    private void sanitizeSubmittedAnswersAgainstQuestions(QuizSubmission quizSubmission, QuizExercise quizExercise) {
+        // Normalize a null collection to empty so downstream iterations (e.g. setSubmission on each answer) stay null-safe.
+        if (quizSubmission.getSubmittedAnswers() == null) {
+            quizSubmission.setSubmittedAnswers(new HashSet<>());
+            return;
+        }
+        if (quizSubmission.getSubmittedAnswers().isEmpty()) {
+            return;
+        }
+        Map<Long, QuizQuestion> questionsById = quizExercise.getQuizQuestions().stream().filter(question -> question.getId() != null)
+                .collect(Collectors.toMap(QuizQuestion::getId, Function.identity()));
+        Set<SubmittedAnswer> sanitizedAnswers = new HashSet<>();
+        for (SubmittedAnswer submittedAnswer : quizSubmission.getSubmittedAnswers()) {
+            QuizQuestion clientQuestion = submittedAnswer.getQuizQuestion();
+            if (clientQuestion == null || clientQuestion.getId() == null) {
+                continue;
+            }
+            QuizQuestion serverQuestion = questionsById.get(clientQuestion.getId());
+            if (serverQuestion == null) {
+                continue;
+            }
+            // Drop any submitted answer whose runtime subtype does not match the server-side question type — otherwise
+            // we would attach e.g. an MC answer to a ShortAnswer question and pass an inconsistent entity to merge.
+            if (submittedAnswer instanceof MultipleChoiceSubmittedAnswer mcAnswer && serverQuestion instanceof MultipleChoiceQuestion mcQuestion) {
+                submittedAnswer.setQuizQuestion(serverQuestion);
+                sanitizeMultipleChoiceSelectedOptions(mcAnswer, mcQuestion);
+                sanitizedAnswers.add(submittedAnswer);
+            }
+            else if (submittedAnswer instanceof DragAndDropSubmittedAnswer dndAnswer && serverQuestion instanceof DragAndDropQuestion dndQuestion) {
+                submittedAnswer.setQuizQuestion(serverQuestion);
+                sanitizeDragAndDropMappings(dndAnswer, dndQuestion);
+                sanitizedAnswers.add(submittedAnswer);
+            }
+            else if (submittedAnswer instanceof ShortAnswerSubmittedAnswer saAnswer && serverQuestion instanceof ShortAnswerQuestion saQuestion) {
+                submittedAnswer.setQuizQuestion(serverQuestion);
+                sanitizeShortAnswerSubmittedTexts(saAnswer, saQuestion);
+                sanitizedAnswers.add(submittedAnswer);
+            }
+        }
+        quizSubmission.setSubmittedAnswers(sanitizedAnswers);
+    }
+
+    private void sanitizeMultipleChoiceSelectedOptions(MultipleChoiceSubmittedAnswer submittedAnswer, MultipleChoiceQuestion question) {
+        Map<Long, AnswerOption> validOptions = question.getAnswerOptions().stream().filter(option -> option.getId() != null)
+                .collect(Collectors.toMap(AnswerOption::getId, Function.identity()));
+        Set<AnswerOption> clientOptions = submittedAnswer.getSelectedOptions();
+        Set<AnswerOption> sanitized = new HashSet<>();
+        if (clientOptions != null) {
+            for (AnswerOption clientOption : clientOptions) {
+                if (clientOption == null || clientOption.getId() == null) {
+                    continue;
+                }
+                AnswerOption serverOption = validOptions.get(clientOption.getId());
+                if (serverOption != null) {
+                    sanitized.add(serverOption);
+                }
+            }
+        }
+        submittedAnswer.setSelectedOptions(sanitized);
+    }
+
+    private void sanitizeDragAndDropMappings(DragAndDropSubmittedAnswer submittedAnswer, DragAndDropQuestion question) {
+        Map<Long, DragItem> validDragItems = question.getDragItems().stream().filter(item -> item.getId() != null).collect(Collectors.toMap(DragItem::getId, Function.identity()));
+        Map<Long, DropLocation> validDropLocations = question.getDropLocations().stream().filter(location -> location.getId() != null)
+                .collect(Collectors.toMap(DropLocation::getId, Function.identity()));
+        Set<DragAndDropMapping> clientMappings = submittedAnswer.getMappings();
+        Set<DragAndDropMapping> sanitized = new HashSet<>();
+        if (clientMappings != null) {
+            for (DragAndDropMapping mapping : clientMappings) {
+                if (mapping == null || mapping.getDragItem() == null || mapping.getDropLocation() == null || mapping.getDragItem().getId() == null
+                        || mapping.getDropLocation().getId() == null) {
+                    continue;
+                }
+                DragItem serverDragItem = validDragItems.get(mapping.getDragItem().getId());
+                DropLocation serverDropLocation = validDropLocations.get(mapping.getDropLocation().getId());
+                if (serverDragItem == null || serverDropLocation == null) {
+                    continue;
+                }
+                mapping.setDragItem(serverDragItem);
+                mapping.setDropLocation(serverDropLocation);
+                sanitized.add(mapping);
+            }
+        }
+        submittedAnswer.setMappings(sanitized);
+    }
+
+    private void sanitizeShortAnswerSubmittedTexts(ShortAnswerSubmittedAnswer submittedAnswer, ShortAnswerQuestion question) {
+        Map<Long, ShortAnswerSpot> validSpots = question.getSpots().stream().filter(spot -> spot.getId() != null)
+                .collect(Collectors.toMap(ShortAnswerSpot::getId, Function.identity()));
+        Set<ShortAnswerSubmittedText> clientTexts = submittedAnswer.getSubmittedTexts();
+        Set<ShortAnswerSubmittedText> sanitized = new HashSet<>();
+        if (clientTexts != null) {
+            for (ShortAnswerSubmittedText submittedText : clientTexts) {
+                if (submittedText == null || submittedText.getSpot() == null || submittedText.getSpot().getId() == null) {
+                    continue;
+                }
+                ShortAnswerSpot serverSpot = validSpots.get(submittedText.getSpot().getId());
+                if (serverSpot == null) {
+                    continue;
+                }
+                submittedText.setSpot(serverSpot);
+                sanitized.add(submittedText);
+            }
+        }
+        submittedAnswer.setSubmittedTexts(sanitized);
+    }
+
     /**
      * Find StudentParticipation of the given quizExercise that was done by the given user
      *
diff --git a/src/main/webapp/app/quiz/manage/update/quiz-exercise-update.component.spec.ts b/src/main/webapp/app/quiz/manage/update/quiz-exercise-update.component.spec.ts
@@ -1289,6 +1289,46 @@ describe('QuizExerciseUpdateComponent', () => {
                 expect(quizExerciseServiceCreateStub).toHaveBeenCalled();
             });
 
+            it('should keep the edit screen visible after saving a not-yet-started quiz when the server response omits isEditable', () => {
+                // Server DTOs use @JsonInclude(NON_EMPTY), so create/update responses do not carry the isEditable flag.
+                // The component must recompute it client-side; otherwise the "quiz has started" banner appears incorrectly.
+                const savedResponse = new QuizExercise(course, undefined);
+                savedResponse.id = 456;
+                savedResponse.title = 'test';
+                savedResponse.duration = 600;
+                savedResponse.quizMode = QuizMode.SYNCHRONIZED;
+                savedResponse.status = QuizStatus.VISIBLE;
+                savedResponse.quizEnded = false;
+                savedResponse.quizBatches = [];
+                savedResponse.isAtLeastEditor = true;
+                savedResponse.isEditable = undefined;
+                quizExerciseServiceUpdateStub.mockReturnValue(of(new HttpResponse<QuizExercise>({ body: savedResponse })));
+
+                saveQuizWithPendingChangesCache();
+
+                expect(comp.quizExercise.isEditable).toBe(true);
+            });
+
+            it('should respect a server-provided isEditable=false flag after save (e.g. exam-date-aware)', () => {
+                // When the server authoritatively says the quiz is no longer editable (e.g. exam start date has passed),
+                // the client must not override it to true via the local check.
+                const savedResponse = new QuizExercise(course, undefined);
+                savedResponse.id = 456;
+                savedResponse.title = 'test';
+                savedResponse.duration = 600;
+                savedResponse.quizMode = QuizMode.SYNCHRONIZED;
+                savedResponse.status = QuizStatus.VISIBLE;
+                savedResponse.quizEnded = false;
+                savedResponse.quizBatches = [];
+                savedResponse.isAtLeastEditor = true;
+                savedResponse.isEditable = false;
+                quizExerciseServiceUpdateStub.mockReturnValue(of(new HttpResponse<QuizExercise>({ body: savedResponse })));
+
+                saveQuizWithPendingChangesCache();
+
+                expect(comp.quizExercise.isEditable).toBe(false);
+            });
+
             it('should call alert service with specific error title if provided on save error', () => {
                 comp.quizExercise.id = undefined;
                 const mockErrorResponse = {
diff --git a/src/main/webapp/app/quiz/manage/update/quiz-exercise-update.component.ts b/src/main/webapp/app/quiz/manage/update/quiz-exercise-update.component.ts
@@ -598,8 +598,10 @@ export class QuizExerciseUpdateComponent extends QuizExerciseValidationDirective
         this.reconcileMappingReferences(quizExercise);
         this.prepareEntity(quizExercise);
         this.quizExercise = quizExercise;
-        // Respect the server-provided isEditable flag (accounts for exam dates), combined with local check
-        this.quizExercise.isEditable = this.quizExercise.isEditable && isQuizEditable(this.quizExercise);
+        // Prefer the server-provided editability flag (e.g. exam-date-aware) when present; otherwise fall back to the
+        // local check. The create/update endpoints currently omit the field, which is why the unconditional overwrite
+        // by the previous implementation flipped the banner on for fresh, not-yet-started quizzes.
+        this.quizExercise.isEditable = this.quizExercise.isEditable ?? isQuizEditable(this.quizExercise);
         this.exerciseService.validateDate(this.quizExercise);
         this.savedEntity = cloneDeep(this.quizExercise);
         this.changeDetector.detectChanges();
diff --git a/src/test/java/de/tum/cit/aet/artemis/quiz/QuizSubmissionIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/quiz/QuizSubmissionIntegrationTest.java
diff --git a/src/test/playwright/e2e/exercise/quiz-exercise/QuizExerciseParticipation.spec.ts b/src/test/playwright/e2e/exercise/quiz-exercise/QuizExerciseParticipation.spec.ts
