decouple programmingsubmission & correction round

Author: Feuermagier

src/main/java/edu/kit/kastel/sdq/artemis4j/grading/PackedAssessment.java

@@ -0,0 +1,68 @@
+package edu.kit.kastel.sdq.artemis4j.grading;
+
+import edu.kit.kastel.sdq.artemis4j.ArtemisNetworkException;
+import edu.kit.kastel.sdq.artemis4j.client.ProgrammingSubmissionDTO;
+import edu.kit.kastel.sdq.artemis4j.client.ResultDTO;
+import edu.kit.kastel.sdq.artemis4j.grading.metajson.AnnotationMappingException;
+import edu.kit.kastel.sdq.artemis4j.grading.penalty.GradingConfig;
+
+import java.util.Optional;
+
+/**
+ * For API users, this is used in place of an assessment whenever we don't want to do
+ * the (expensive) deserialization of the assessment.
+ * The only semantic difference to an assessment is that this class does *not* imply a lock.
+ * <p>
+ * Internally, this is just a glorified ResultDTO wrapper because we need a place
+ * to store the submission
+ * @param result
+ * @param submission
+ */
+public record PackedAssessment(ResultDTO result, CorrectionRound round, ProgrammingSubmission submission) {
+    public boolean isSubmitted() {
+        return result.completionDate() != null;
+    }
+
+    public User getAssessor() {
+        return new User(result.assessor());
+    }
+
+    /**
+     * Locks and opens the assessment for this submission.
+     * <p>
+     * If the submission has not been assessed by you, this might not be possible.
+     *
+     * @param config the config for the exercise
+     * @return the assessment if there are results for this submission
+     * @throws AnnotationMappingException If the annotations that were already
+     *                                    present could not be mapped given the
+     *                                    gradingConfig
+     */
+    public Optional<Assessment> open(GradingConfig config) throws MoreRecentSubmissionException, ArtemisNetworkException, AnnotationMappingException {
+        return this.submission.getExercise().tryLockSubmission(this.submission.getId(), this.round, config);
+    }
+
+    /**
+     * Opens the assessment for this submission without locking it.
+     * <p>
+     * If the submission has not been assessed by you, you might not be able to
+     * change the assessment.
+     *
+     * @param config the config for the exercise
+     * @return the assessment if there are results for this submission
+     * @throws AnnotationMappingException If the annotations that were already
+     *                                    present could not be mapped given the
+     *                                    gradingConfig
+     */
+    public Assessment openWithoutLock(GradingConfig config) throws ArtemisNetworkException, AnnotationMappingException {
+        return new Assessment(this.result, config, this.submission, this.round);
+    }
+
+    public void cancel() throws ArtemisNetworkException {
+        ProgrammingSubmissionDTO.cancelAssessment(this.getConnection().getClient(), this.submission.getId());
+    }
+
+    private ArtemisConnection getConnection() {
+        return this.submission.getConnection();
+    }
+}

src/main/java/edu/kit/kastel/sdq/artemis4j/grading/ProgrammingExercise.java

@@ -74,19 +74,52 @@ public boolean hasSecondCorrectionRound() {
         return this.dto.secondCorrectionEnabled() != null && this.dto.secondCorrectionEnabled();
     }
 
-    /**
-     * Fetches all submissions for this exercise. This may fetch *many* submissions,
-     * and does not cache the result, so be careful.
-     *
-     * @param correctionRound       The correction round to fetch submissions for
-     * @param filterAssessedByTutor Whether to only fetch submissions that the
-     *                              current user has assessed
-     * @return a list of submissions
-     * @throws ArtemisNetworkException if the request fails
-     */
-    public List<ProgrammingSubmission> fetchSubmissions(CorrectionRound correctionRound, boolean filterAssessedByTutor)
+    // /**
+    //  * Fetches all submissions for this exercise. This may fetch *many* submissions,
+    //  * and does not cache the result, so be careful.
+    //  *
+    //  * @param correctionRound       The correction round to fetch submissions for
+    //  * @param filterAssessedByTutor Whether to only fetch submissions that the
+    //  *                              current user has assessed
+    //  * @return a list of submissions
+    //  * @throws ArtemisNetworkException if the request fails
+    //  */
+    // public List<ProgrammingSubmission> fetchSubmissions(CorrectionRound correctionRound, boolean filterAssessedByTutor)
+    //         throws ArtemisNetworkException {
+    //
+    //     if (correctionRound == CorrectionRound.SECOND && !this.hasSecondCorrectionRound()) {
+    //         throw new IllegalArgumentException("This exercise does not have a second correction round");
+    //     }
+    //
+    //     if (correctionRound == CorrectionRound.REVIEW) {
+    //         throw new IllegalArgumentException("Can't fetch submissions for the review 'round'");
+    //     }
+    //
+    //     return ProgrammingSubmissionDTO.fetchAll(
+    //                     this.getConnection().getClient(),
+    //                     this.getId(),
+    //                     correctionRound.toArtemis(),
+    //                     filterAssessedByTutor)
+    //             .stream()
+    //             .map(submissionDto -> new ProgrammingSubmission(submissionDto, this))
+    //             .toList();
+    // }
+
+    public List<ProgrammingSubmissionWithResults> fetchAllSubmissions()
             throws ArtemisNetworkException {
+        // Artemis ignores the correction round since assessedByTutor is false
+        return ProgrammingSubmissionDTO.fetchAll(
+                        this.getConnection().getClient(),
+                        this.getId(),
+                        0,
+                        true)
+                .stream()
+                .map(dto -> new ProgrammingSubmissionWithResults(new ProgrammingSubmission(dto, this)))
+                .toList();
+    }
 
+    public List<PackedAssessment> fetchMyAssessments(CorrectionRound correctionRound)
+            throws ArtemisNetworkException {
         if (correctionRound == CorrectionRound.SECOND && !this.hasSecondCorrectionRound()) {
             throw new IllegalArgumentException("This exercise does not have a second correction round");
         }
@@ -95,33 +128,25 @@ public List<ProgrammingSubmission> fetchSubmissions(CorrectionRound correctionRo
             throw new IllegalArgumentException("Can't fetch submissions for the review 'round'");
         }
 
-        return ProgrammingSubmissionDTO.fetchAll(
+        var submissions = ProgrammingSubmissionDTO.fetchAll(
                         this.getConnection().getClient(),
                         this.getId(),
                         correctionRound.toArtemis(),
-                        filterAssessedByTutor)
+                        true)
                 .stream()
-                .map(submissionDto -> new ProgrammingSubmission(submissionDto, this, correctionRound))
+                .map(submissionDto -> new ProgrammingSubmission(submissionDto, this))
                 .toList();
-    }
 
-    public List<ProgrammingSubmission> fetchSubmissions(CorrectionRound correctionRound)
-            throws ArtemisNetworkException {
-        return this.fetchSubmissions(
-                correctionRound,
-                !this.getCourse().isInstructor(this.getConnection().getAssessor()));
-    }
-
-    /**
-     * Fetches all submissions from correction round 1 and 2 (if enabled).
-     */
-    public List<ProgrammingSubmission> fetchSubmissions() throws ArtemisNetworkException {
-        List<ProgrammingSubmission> submissions = new ArrayList<>(this.fetchSubmissions(CorrectionRound.FIRST));
-        if (this.hasSecondCorrectionRound()) {
-            submissions.addAll(this.fetchSubmissions(CorrectionRound.SECOND));
+        var assessments = new ArrayList<PackedAssessment>(submissions.size());
+        for (var submission : submissions) {
+            // TODO this may return more than one result for instructors
+            var results = submission.getDTO().nonAutomaticResults();
+            if (results.size() != 1) {
+                throw new IllegalStateException("Too many non-automatic results");
+            }
+            assessments.add(new PackedAssessment(results.get(0), correctionRound, submission));
         }
-
-        return submissions;
+        return assessments;
     }
 
     /**
@@ -178,9 +203,8 @@ public Optional<Assessment> tryLockSubmission(
                 this.getConnection().getClient(), submissionId, correctionRound.toArtemis());
 
         if (locked.id() != submissionId) {
-            // Artemis automatically returns the most recent submission associated with the
-            // same participation
-            // as the requested submission
+            // Artemis automatically returns the most recent submission that is associated with the
+            // participation of the requested submission
             throw new MoreRecentSubmissionException(
                     submissionId, locked.id(), locked.participation().id());
         }
@@ -199,16 +223,16 @@ public Optional<Assessment> tryLockSubmission(
             return Optional.empty();
         }
 
-        var submission = new ProgrammingSubmission(locked, this, correctionRound);
+        var submission = new ProgrammingSubmission(locked, this);
         return Optional.of(new Assessment(result, gradingConfig, submission, correctionRound));
     }
 
-    public int fetchOwnSubmissionCount(CorrectionRound correctionRound) throws ArtemisNetworkException {
-        return this.fetchSubmissions(correctionRound, true).size();
+    public int fetchOwnAssessmentCount(CorrectionRound correctionRound) throws ArtemisNetworkException {
+        return this.fetchMyAssessments(correctionRound).size();
     }
 
     public int fetchLockedSubmissionCount(CorrectionRound correctionRound) throws ArtemisNetworkException {
-        return (int) this.fetchSubmissions(correctionRound, true).stream()
+        return (int) this.fetchMyAssessments(correctionRound).stream()
                 .filter(s -> !s.isSubmitted())
                 .count();
     }

src/main/java/edu/kit/kastel/sdq/artemis4j/grading/ProgrammingSubmission.java

@@ -1,19 +1,18 @@
 /* Licensed under EPL-2.0 2024. */
 package edu.kit.kastel.sdq.artemis4j.grading;
 
-import java.nio.file.Path;
-import java.time.ZonedDateTime;
-import java.util.Objects;
-import java.util.Optional;
-
 import edu.kit.kastel.sdq.artemis4j.ArtemisClientException;
 import edu.kit.kastel.sdq.artemis4j.ArtemisNetworkException;
-import edu.kit.kastel.sdq.artemis4j.client.AssessmentType;
 import edu.kit.kastel.sdq.artemis4j.client.ProgrammingSubmissionDTO;
 import edu.kit.kastel.sdq.artemis4j.client.ResultDTO;
 import edu.kit.kastel.sdq.artemis4j.grading.metajson.AnnotationMappingException;
 import edu.kit.kastel.sdq.artemis4j.grading.penalty.GradingConfig;
 
+import java.nio.file.Path;
+import java.time.ZonedDateTime;
+import java.util.Objects;
+import java.util.Optional;
+
 /**
  * A student's programming submission. A submission essentially consists of the
  * URL of a student's Git repository, along with a commit hash. We do not model
@@ -22,12 +21,11 @@
 public class ProgrammingSubmission extends ArtemisConnectionHolder {
     private final ProgrammingSubmissionDTO dto;
 
-    private final CorrectionRound correctionRound;
     private final User student;
     private final ProgrammingExercise exercise;
 
     public ProgrammingSubmission(
-            ProgrammingSubmissionDTO dto, ProgrammingExercise exercise, CorrectionRound correctionRound) {
+            ProgrammingSubmissionDTO dto, ProgrammingExercise exercise) {
         super(exercise);
 
         this.dto = dto;
@@ -40,8 +38,6 @@ public ProgrammingSubmission(
         } else {
             this.student = null;
         }
-
-        this.correctionRound = correctionRound;
     }
 
     public long getId() {
@@ -79,10 +75,6 @@ public ProgrammingExercise getExercise() {
         return exercise;
     }
 
-    public CorrectionRound getCorrectionRound() {
-        return this.correctionRound;
-    }
-
     public ZonedDateTime getSubmissionDate() {
         return this.dto.submissionDate();
     }
@@ -95,17 +87,6 @@ public Optional<ResultDTO> getLatestResult() {
         }
     }
 
-    /**
-     * Get the assessor of this assessment.
-     * <p>
-     * This is the user who has locked the submission.
-     *
-     * @return the assessor or empty if the submission has not been assessed
-     */
-    public Optional<User> getAssessor() {
-        return this.getRelevantResult().map(ResultDTO::assessor).map(User::new);
-    }
-
     /**
      * Clones the submission, including the test repository, into the given path,
      * and checks out the submitted commit. This method uses the user's VCS access token, potentially creating a new one.
@@ -131,7 +112,9 @@ public ClonedProgrammingSubmission cloneViaSSHInto(Path target) throws ArtemisCl
     }
 
     /**
-     * Tries to lock this submission. Locking is reentrant.
+     * Prefer the methods on PackedAssessment!
+     * <p>
+     * Tries to lock this submission for a given correction round. Locking is reentrant.
      *
      * @return An empty optional if a *different* user has already locked the
      *         submission, otherwise the assessment
@@ -144,46 +127,21 @@ public ClonedProgrammingSubmission cloneViaSSHInto(Path target) throws ArtemisCl
      *                                       corresponding student (i.e.
      *                                       participation)
      */
-    public Optional<Assessment> tryLock(GradingConfig gradingConfig)
+    public Optional<Assessment> tryLock(GradingConfig gradingConfig, CorrectionRound correctionRound)
             throws AnnotationMappingException, ArtemisNetworkException, MoreRecentSubmissionException {
-        return this.exercise.tryLockSubmission(this.getId(), this.getCorrectionRound(), gradingConfig);
+        return this.exercise.tryLockSubmission(this.getId(), correctionRound, gradingConfig);
     }
 
-    public boolean isSubmitted() {
-        var result = this.getRelevantResult();
-        if (result.isEmpty() || result.get().completionDate() == null) {
-            return false;
-        }
-
-        var assessmentType = result.get().assessmentType();
-        return assessmentType == AssessmentType.MANUAL || assessmentType == AssessmentType.SEMI_AUTOMATIC;
+    public boolean isBuildFailed() {
+        return this.dto.buildFailed();
     }
 
     /**
-     * Opens the assessment for this submission.
-     * <p>
-     * If the submission has not been assessed by you, you might not be able to
-     * change the assessment.
-     *
-     * @param config the config for the exercise
-     * @return the assessment if there are results for this submission
-     * @throws AnnotationMappingException If the annotations that were already
-     *                                    present could not be mapped given the
-     *                                    gradingConfig
+     * Be VERY careful with the dto's results, since their number may differ depending on the source of the dto.
+     * @return the corresponding dto
      */
-    public Optional<Assessment> openAssessment(GradingConfig config)
-            throws AnnotationMappingException, ArtemisNetworkException {
-        ResultDTO resultDTO = this.getRelevantResult().orElse(null);
-
-        if (resultDTO != null) {
-            return Optional.of(new Assessment(resultDTO, config, this, this.correctionRound));
-        }
-
-        return Optional.empty();
-    }
-
-    public boolean isBuildFailed() {
-        return this.dto.buildFailed();
+    public ProgrammingSubmissionDTO getDTO() {
+        return this.dto;
     }
 
     @Override
@@ -198,30 +156,4 @@ public boolean equals(Object o) {
     public int hashCode() {
         return Objects.hashCode(this.getId());
     }
-
-    /**
-     * Returns the relevant result for this submission.
-     * <p>
-     * The difference between this method and {@link #getLatestResult()} is that
-     * when a submission has multiple results from different correction rounds, this
-     * method will return the result for the current correction round. If you want the
-     * latest result regardless of the correction round, use {@link #getLatestResult()}.
-     *
-     * @return the relevant result, if present
-     */
-    public Optional<ResultDTO> getRelevantResult() {
-        var results = this.dto.nonAutomaticResults();
-
-        if (results.isEmpty()) {
-            return Optional.empty();
-        } else if (results.size() == 1) {
-            // We only have one result, so the submission has
-            // probably been created for a specific correction round,
-            // or we only have one correction round
-            return Optional.of(results.get(0));
-        } else {
-            // More than one result, so probably multiple correction rounds
-            return Optional.of(results.get(this.correctionRound.toArtemis()));
-        }
-    }
 }