Programming exercises: Directly link feedback to its test case (#7015)

src/main/java/de/tum/in/www1/artemis/config/migration/MigrationEntry.java

@@ -1,8 +1,6 @@
 package de.tum.in.www1.artemis.config.migration;
 
-import java.io.Serializable;
-
-public abstract class MigrationEntry implements Serializable {
+public abstract class MigrationEntry {
 
     public abstract void execute();
 

src/main/java/de/tum/in/www1/artemis/config/migration/MigrationRegistry.java

@@ -11,6 +11,7 @@
 import org.springframework.stereotype.Component;
 
 import de.tum.in.www1.artemis.config.migration.entries.MigrationEntry20230808_203400;
+import de.tum.in.www1.artemis.config.migration.entries.MigrationEntry20230810_150000;
 import de.tum.in.www1.artemis.config.migration.entries.MigrationEntry20230920_181600;
 
 /**
@@ -26,11 +27,11 @@ public class MigrationRegistry {
     private final MigrationService migrationService;
 
     public MigrationRegistry(MigrationService migrationService) {
-        // Here we define the order of the ChangeEntries
         this.migrationService = migrationService;
-
+        // Here we define the order of the ChangeEntries
         this.migrationEntryMap.put(1, MigrationEntry20230808_203400.class);
-        this.migrationEntryMap.put(2, MigrationEntry20230920_181600.class);
+        this.migrationEntryMap.put(2, MigrationEntry20230810_150000.class);
+        this.migrationEntryMap.put(3, MigrationEntry20230920_181600.class);
     }
 
     /**

src/main/java/de/tum/in/www1/artemis/config/migration/entries/MigrationEntry20230810_150000.java

@@ -0,0 +1,91 @@
+package de.tum.in.www1.artemis.config.migration.entries;
+
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.data.domain.Page;
+import org.springframework.data.domain.Pageable;
+
+import com.google.common.collect.Lists;
+
+import de.tum.in.www1.artemis.config.migration.MigrationEntry;
+import de.tum.in.www1.artemis.domain.ProgrammingExercise;
+import de.tum.in.www1.artemis.repository.ProgrammingExerciseRepository;
+import de.tum.in.www1.artemis.security.SecurityUtils;
+import de.tum.in.www1.artemis.service.hestia.ProgrammingExerciseTaskService;
+
+/**
+ * Migration for automatically updating all Programming exercise problem statements, removing their test names and replacing them by test ids.
+ */
+public class MigrationEntry20230810_150000 extends MigrationEntry {
+
+    private final Logger log = LoggerFactory.getLogger(MigrationEntry20230810_150000.class);
+
+    private final ProgrammingExerciseRepository programmingExerciseRepository;
+
+    private final ProgrammingExerciseTaskService taskService;
+
+    private static final int BATCH_SIZE = 100;
+
+    private static final int THREADS = 10;
+
+    public MigrationEntry20230810_150000(ProgrammingExerciseRepository programmingExerciseRepository, ProgrammingExerciseTaskService taskService) {
+        this.programmingExerciseRepository = programmingExerciseRepository;
+        this.taskService = taskService;
+    }
+
+    @Override
+    public void execute() {
+        long size = programmingExerciseRepository.count();
+        if (size == 0) {
+            // no exercises to change, migration complete
+            return;
+        }
+
+        log.info("Migrating all {} programming exercises. This might take a while.", size);
+
+        ExecutorService executorService = Executors.newFixedThreadPool(THREADS);
+        Pageable pageable = Pageable.ofSize(BATCH_SIZE);
+
+        while (true) {
+            Page<ProgrammingExercise> exercisePage = programmingExerciseRepository.findAll(pageable);
+            var exercisePartitions = Lists.partition(exercisePage.toList(), THREADS);
+            CompletableFuture<?>[] allFutures = new CompletableFuture[Math.min(THREADS, exercisePartitions.size())];
+
+            for (int i = 0; i < exercisePartitions.size(); i++) {
+                var partition = exercisePartitions.get(i);
+                allFutures[i] = CompletableFuture.runAsync(() -> {
+                    SecurityUtils.setAuthorizationObject();
+                    for (var exercise : partition) {
+                        log.debug("Migrating exercise {}", exercise.getId());
+                        taskService.replaceTestNamesWithIds(exercise);
+                    }
+                    programmingExerciseRepository.saveAll(partition);
+                }, executorService);
+            }
+
+            // Wait until all currently loaded exercises are migrated to avoid loading too many exercises at the same time.
+            CompletableFuture.allOf(allFutures).join();
+
+            if (exercisePage.isLast()) {
+                break;
+            }
+            pageable = pageable.next();
+        }
+
+        executorService.shutdown();
+    }
+
+    @Override
+    public String author() {
+        return "welscher";
+    }
+
+    @Override
+    public String date() {
+        return "20230810_150000";
+    }
+}

src/main/java/de/tum/in/www1/artemis/domain/Feedback.java

@@ -61,6 +61,14 @@ public class Feedback extends DomainObject {
     @Column(name = "reference", length = MAX_REFERENCE_LENGTH)
     private String reference;
 
+    /**
+     * Reference to the test case which created this feedback.
+     * null if the feedback was not created by an automatic test case.
+     */
+    @ManyToOne(fetch = FetchType.LAZY)
+    @JsonIgnoreProperties({ "tasks", "solutionEntries", "exercise", "coverageEntries" })
+    private ProgrammingExerciseTestCase testCase;
+
     /**
      * Absolute score for the assessed element (e.g. +0.5, -1.0, +2.0, etc.)
      */
@@ -220,6 +228,19 @@ public void setReference(String reference) {
         this.reference = reference;
     }
 
+    public ProgrammingExerciseTestCase getTestCase() {
+        return testCase;
+    }
+
+    public void setTestCase(ProgrammingExerciseTestCase testCase) {
+        this.testCase = testCase;
+    }
+
+    public Feedback testCase(ProgrammingExerciseTestCase testCase) {
+        setTestCase(testCase);
+        return this;
+    }
+
     public Double getCredits() {
         return credits;
     }
@@ -236,7 +257,7 @@ public void setCredits(Double credits) {
     /**
      * Returns if this is a positive feedback.
      * <p>
-     * This value can actually be {@code null} for feedbacks that are neither positive nor negative, e.g. when this is a
+     * This value can actually be {@code null} for feedbacks that are neither positive nor negative, e.g., when this is a
      * feedback for a programming exercise test case that has not been executed for the submission.
      *
      * @return true, if this is a positive feedback.

src/main/java/de/tum/in/www1/artemis/domain/ProgrammingExerciseTestCase.java

@@ -233,7 +233,7 @@ public boolean isSameTestCase(ProgrammingExerciseTestCase testCase) {
     @Override
     public String toString() {
         return "ProgrammingExerciseTestCase{" + "id=" + getId() + ", testName='" + testName + '\'' + ", weight=" + weight + ", active=" + active + ", visibility=" + visibility
-                + ", bonusMultiplier=" + bonusMultiplier + ", bonusPoints=" + bonusPoints + '}';
+                + ", bonusMultiplier=" + bonusMultiplier + ", bonusPoints=" + bonusPoints + ", type=" + type + '}';
     }
 
     /**
@@ -244,8 +244,8 @@ public String toString() {
      */
     public boolean isSuccessful(Result result) {
         return result.getFeedbacks().stream().anyMatch(feedback -> {
-            boolean testNameAreSame = feedback.getText() != null && feedback.getText().equalsIgnoreCase(this.getTestName());
-            return testNameAreSame && Boolean.TRUE.equals(feedback.isPositive());
+            boolean testsAreSame = this.equals(feedback.getTestCase());
+            return testsAreSame && Boolean.TRUE.equals(feedback.isPositive());
         });
     }
 
@@ -256,8 +256,6 @@ public boolean isSuccessful(Result result) {
      * @return true if there is no feedback for a given test.
      */
     public boolean wasNotExecuted(Result result) {
-        return result.getFeedbacks().stream().filter(feedback -> feedback.getType() == FeedbackType.AUTOMATIC)
-                .noneMatch(feedback -> feedback.getText() != null && feedback.getText().equalsIgnoreCase(this.getTestName()));
+        return result.getFeedbacks().stream().filter(feedback -> feedback.getType() == FeedbackType.AUTOMATIC).noneMatch(feedback -> this.equals(feedback.getTestCase()));
     }
-
 }

src/main/java/de/tum/in/www1/artemis/domain/Result.java

@@ -482,30 +482,62 @@ public void filterSensitiveInformation() {
     /**
      * Removes all feedback details that should not be passed to the student.
      *
-     * @param removeHiddenFeedback if feedbacks marked with visibility 'after due date' should also be removed.
+     * @param removeHiddenFeedback true if feedbacks marked with visibility 'after due date' should also be removed.
      */
     public void filterSensitiveFeedbacks(boolean removeHiddenFeedback) {
-        var filteredFeedback = createFilteredFeedbacks(removeHiddenFeedback);
+        filterSensitiveFeedbacks(removeHiddenFeedback, participation.getExercise());
+    }
+
+    /**
+     * Removes all feedback details that should not be passed to the student.
+     *
+     * @param removeHiddenFeedback true if feedbacks marked with visibility 'after due date' should also be removed.
+     * @param exercise             the exercise related to this result. Used to determine if test case names should be removed.
+     */
+    public void filterSensitiveFeedbacks(boolean removeHiddenFeedback, Exercise exercise) {
+        var filteredFeedback = createFilteredFeedbacks(removeHiddenFeedback, exercise);
         setFeedbacks(filteredFeedback);
 
-        // TODO: this is not good code!
+        if (exercise instanceof ProgrammingExercise) {
+            updateTestCaseCount();
+        }
+    }
+
+    /**
+     * Updates the testCaseCount and passedTestCaseCount attributes after filtering the feedback.
+     */
+    private void updateTestCaseCount() {
         var testCaseFeedback = feedbacks.stream().filter(Feedback::isTestFeedback).toList();
+
+        // TODO: this is not good code!
         setTestCaseCount(testCaseFeedback.size());
         setPassedTestCaseCount((int) testCaseFeedback.stream().filter(feedback -> Boolean.TRUE.equals(feedback.isPositive())).count());
     }
 
     /**
      * Returns a new list that only contains feedback that should be passed to the student.
      * Does not change the feedbacks attribute of this entity.
+     * Also removes the test names from all feedback if it should not be shown to the student.
      *
      * @see ResultDTO
      *
-     * @param removeHiddenFeedback if feedbacks marked with visibility 'after due date' should also be removed.
+     * @param removeHiddenFeedback true if feedbacks marked with visibility 'after due date' should also be removed.
+     * @param exercise             used to check if students can see the test case names
      * @return the new filtered list
      */
-    public List<Feedback> createFilteredFeedbacks(boolean removeHiddenFeedback) {
-        return feedbacks.stream().filter(feedback -> !feedback.isInvisible()).filter(feedback -> !removeHiddenFeedback || !feedback.isAfterDueDate())
+    public List<Feedback> createFilteredFeedbacks(boolean removeHiddenFeedback, Exercise exercise) {
+        var filteredFeedback = feedbacks.stream().filter(feedback -> !feedback.isInvisible()).filter(feedback -> !removeHiddenFeedback || !feedback.isAfterDueDate())
                 .collect(Collectors.toCollection(ArrayList::new));
+
+        if (exercise instanceof ProgrammingExercise programmingExercise && !Boolean.TRUE.equals(programmingExercise.getShowTestNamesToStudents())) {
+            filteredFeedback.stream().filter(Feedback::isTestFeedback).forEach(feedback -> {
+                if (feedback.getTestCase() != null) {
+                    feedback.getTestCase().setTestName(null);
+                }
+            });
+
+        }
+        return filteredFeedback;
     }
 
     /**

src/main/java/de/tum/in/www1/artemis/repository/ExerciseRepository.java

@@ -494,16 +494,17 @@ default boolean toggleSecondCorrection(Exercise exercise) {
     @Query("""
              SELECT e
              FROM Course c
-                 LEFT JOIN  c.exercises e
+                 LEFT JOIN c.exercises e
                  LEFT JOIN FETCH e.studentParticipations p
                  LEFT JOIN p.team.students students
                  LEFT JOIN FETCH p.submissions s
                  LEFT JOIN FETCH s.results r
-                 LEFT JOIN FETCH r.feedbacks
+                 LEFT JOIN FETCH r.feedbacks f
+                 LEFT JOIN FETCH f.testCase
             WHERE p.student.id = :userId
                   OR students.id = :userId
              """)
-    Set<Exercise> getAllExercisesUserParticipatedInWithEagerParticipationsSubmissionsResultsFeedbacksByUserId(long userId);
+    Set<Exercise> getAllExercisesUserParticipatedInWithEagerParticipationsSubmissionsResultsFeedbacksTestCasesByUserId(long userId);
 
     /**
      * For an explanation, see {@link de.tum.in.www1.artemis.web.rest.ExamResource#getAllExercisesWithPotentialPlagiarismForExam(long,long)}

src/main/java/de/tum/in/www1/artemis/repository/ProgrammingExerciseRepository.java

@@ -119,11 +119,13 @@ default ProgrammingExercise findOneByProjectKeyOrThrow(String projectKey, boolea
             LEFT JOIN FETCH pe.solutionParticipation sp
             LEFT JOIN FETCH tp.results AS tpr
             LEFT JOIN FETCH sp.results AS spr
-            LEFT JOIN FETCH tpr.feedbacks
-            LEFT JOIN FETCH spr.feedbacks
+            LEFT JOIN FETCH tpr.feedbacks tf
+            LEFT JOIN FETCH spr.feedbacks sf
+            LEFT JOIN FETCH tf.testCase
+            LEFT JOIN FETCH sf.testCase
             LEFT JOIN FETCH tpr.submission
             LEFT JOIN FETCH spr.submission
-            WHERE pe.id = :#{#exerciseId}
+            WHERE pe.id = :exerciseId
                 AND (tpr.id = (SELECT MAX(re1.id) FROM tp.results re1) OR tpr.id IS NULL)
                 AND (spr.id = (SELECT MAX(re2.id) FROM sp.results re2) OR spr.id IS NULL)
             """)

src/main/java/de/tum/in/www1/artemis/repository/ProgrammingExerciseStudentParticipationRepository.java

@@ -32,7 +32,8 @@ public interface ProgrammingExerciseStudentParticipationRepository extends JpaRe
             SELECT p
             FROM ProgrammingExerciseStudentParticipation p
                 LEFT JOIN FETCH p.results pr
-                LEFT JOIN FETCH pr.feedbacks
+                LEFT JOIN FETCH pr.feedbacks f
+                LEFT JOIN FETCH f.testCase
                 LEFT JOIN FETCH pr.submission
             WHERE p.id = :participationId
                 AND (pr.id = (