From ac7533aedc1992997327c9907ade574f0ce8b736 Mon Sep 17 00:00:00 2001 From: Lucas <24826124+Luro02@users.noreply.github.com> Date: Wed, 22 Jan 2025 14:23:12 +0100 Subject: [PATCH] Implement some things for intelligrade (#165) * expose `GradingConfigDTO` to implement #164 * implement configuring highlighting #160 * apply spotless --- .../grading/penalty/GradingConfig.java | 76 +++++++++++-------- .../grading/penalty/MistakeType.java | 33 +++++++- .../kit/kastel/sdq/artemis4j/End2EndTest.java | 45 +++++++---- src/test/resources/config.json | 20 ++++- 4 files changed, 121 insertions(+), 53 deletions(-) diff --git a/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/penalty/GradingConfig.java b/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/penalty/GradingConfig.java index a52cb40..dafb4a9 100644 --- a/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/penalty/GradingConfig.java +++ b/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/penalty/GradingConfig.java @@ -1,4 +1,4 @@ -/* Licensed under EPL-2.0 2024. */ +/* Licensed under EPL-2.0 2024-2025. */ package edu.kit.kastel.sdq.artemis4j.grading.penalty; import java.util.Collections; @@ -29,49 +29,52 @@ private GradingConfig( this.positiveFeedbackAllowed = positiveFeedbackAllowed; } - public static GradingConfig readFromString(String configString, ProgrammingExercise exercise) + public static GradingConfig fromDTO(GradingConfigDTO configDTO, ProgrammingExercise exercise) throws InvalidGradingConfigException { - ObjectMapper mapper = new ObjectMapper(); + if (!configDTO.isAllowedForExercise(exercise.getId())) { + throw new InvalidGradingConfigException( + "Grading config is not valid for exercise with id " + exercise.getId()); + } - try { - var configDTO = mapper.readValue(configString, GradingConfigDTO.class); + var ratingGroups = + configDTO.ratingGroups().stream().map(RatingGroup::new).toList(); + var ratingGroupsById = ratingGroups.stream().collect(Collectors.toMap(RatingGroup::getId, Function.identity())); - // no allowed exercises means it is valid for all exercises - if (configDTO.allowedExercises() != null - && !configDTO.allowedExercises().isEmpty() - && !configDTO.allowedExercises().contains(exercise.getId())) { - throw new InvalidGradingConfigException( - "Grading config is not valid for exercise with id " + exercise.getId()); + for (MistakeType.MistakeTypeDTO dto : configDTO.mistakeTypes()) { + if (!StringUtil.matchMaybe(exercise.getShortName(), dto.enabledForExercises())) { + continue; } - var ratingGroups = - configDTO.ratingGroups().stream().map(RatingGroup::new).toList(); - var ratingGroupsById = - ratingGroups.stream().collect(Collectors.toMap(RatingGroup::getId, Function.identity())); + MistakeType.createAndAddToGroup( + dto, + StringUtil.matchMaybe(exercise.getShortName(), dto.enabledPenaltyForExercises()), + ratingGroupsById.get(dto.appliesTo())); + } - for (MistakeType.MistakeTypeDTO dto : configDTO.mistakeTypes()) { - if (!StringUtil.matchMaybe(exercise.getShortName(), dto.enabledForExercises())) { - continue; - } + var config = new GradingConfig( + configDTO.shortName(), configDTO.positiveFeedbackAllowed(), ratingGroups, exercise.getId()); + log.info( + "Parsed grading config for exercise '{}' and found {} mistake types", + config.getShortName(), + config.getMistakeTypes().size()); + return config; + } - MistakeType.createAndAddToGroup( - dto, - StringUtil.matchMaybe(exercise.getShortName(), dto.enabledPenaltyForExercises()), - ratingGroupsById.get(dto.appliesTo())); - } + public static GradingConfigDTO readDTOFromString(String configString) throws InvalidGradingConfigException { + ObjectMapper mapper = new ObjectMapper(); - var config = new GradingConfig( - configDTO.shortName(), configDTO.positiveFeedbackAllowed(), ratingGroups, exercise.getId()); - log.info( - "Parsed grading config for exercise '{}' and found {} mistake types", - config.getShortName(), - config.getMistakeTypes().size()); - return config; + try { + return mapper.readValue(configString, GradingConfigDTO.class); } catch (JsonProcessingException e) { throw new InvalidGradingConfigException(e); } } + public static GradingConfig readFromString(String configString, ProgrammingExercise exercise) + throws InvalidGradingConfigException { + return fromDTO(readDTOFromString(configString), exercise); + } + public List getMistakeTypes() { return this.streamMistakeTypes().toList(); } @@ -108,10 +111,17 @@ private Stream streamMistakeTypes() { return ratingGroups.stream().map(RatingGroup::getMistakeTypes).flatMap(List::stream); } - record GradingConfigDTO( + public record GradingConfigDTO( String shortName, @JsonProperty(defaultValue = "true") boolean positiveFeedbackAllowed, List allowedExercises, List ratingGroups, - List mistakeTypes) {} + List mistakeTypes) { + public boolean isAllowedForExercise(long exerciseId) { + // no allowed exercises means it is valid for all exercises + return this.allowedExercises() == null + || this.allowedExercises().isEmpty() + || this.allowedExercises().contains(exerciseId); + } + } } diff --git a/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/penalty/MistakeType.java b/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/penalty/MistakeType.java index 1718e1a..02f53ba 100644 --- a/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/penalty/MistakeType.java +++ b/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/penalty/MistakeType.java @@ -1,4 +1,4 @@ -/* Licensed under EPL-2.0 2024. */ +/* Licensed under EPL-2.0 2024-2025. */ package edu.kit.kastel.sdq.artemis4j.grading.penalty; import java.util.Collections; @@ -6,6 +6,8 @@ import java.util.Map; import java.util.Objects; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonValue; import edu.kit.kastel.sdq.artemis4j.i18n.FormatString; import edu.kit.kastel.sdq.artemis4j.i18n.TranslatableString; @@ -17,6 +19,7 @@ public final class MistakeType { private final FormatString buttonTexts; private final MistakeReportingState reporting; private final List autograderProblemTypes; + private final Highlight highlight; static void createAndAddToGroup(MistakeTypeDTO dto, boolean shouldScore, RatingGroup ratingGroup) { var mistakeType = new MistakeType(dto, shouldScore, ratingGroup); @@ -41,6 +44,8 @@ private MistakeType(MistakeTypeDTO dto, boolean shouldScore, RatingGroup ratingG } else { this.reporting = MistakeReportingState.REPORT; } + + this.highlight = dto.highlight() == null ? Highlight.DEFAULT : dto.highlight(); } public String getId() { @@ -79,6 +84,15 @@ public List getAutograderProblemTypes() { return Collections.unmodifiableList(autograderProblemTypes); } + /** + * Returns how the mistake type should be highlighted. + * + * @return the highlight for the mistake + */ + public Highlight getHighlight() { + return this.highlight; + } + @Override public boolean equals(Object o) { if (this == o) return true; @@ -101,7 +115,19 @@ public String toString() { + message.getDefaultTranslationPattern() + ", buttonTexts=" + buttonTexts.getDefaultTranslationPattern() + ", reporting=" + reporting + ", autograderProblemTypes=" - + autograderProblemTypes + '}'; + + autograderProblemTypes + ", highlight=" + + highlight + '}'; + } + + public enum Highlight { + NONE, + DEFAULT; + + @JsonValue + @Override + public String toString() { + return this.name().toLowerCase(); + } } record MistakeTypeDTO( @@ -114,5 +140,6 @@ record MistakeTypeDTO( String enabledPenaltyForExercises, Map additionalButtonTexts, Map additionalMessages, - List autograderProblemTypes) {} + List autograderProblemTypes, + @JsonProperty(defaultValue = "Highlight.DEFAULT") Highlight highlight) {} } diff --git a/src/test/java/edu/kit/kastel/sdq/artemis4j/End2EndTest.java b/src/test/java/edu/kit/kastel/sdq/artemis4j/End2EndTest.java index d56e009..77c7b87 100644 --- a/src/test/java/edu/kit/kastel/sdq/artemis4j/End2EndTest.java +++ b/src/test/java/edu/kit/kastel/sdq/artemis4j/End2EndTest.java @@ -1,6 +1,8 @@ -/* Licensed under EPL-2.0 2023-2024. */ +/* Licensed under EPL-2.0 2023-2025. */ package edu.kit.kastel.sdq.artemis4j; +import static org.junit.jupiter.api.Assertions.assertEquals; + import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -108,10 +110,10 @@ void testCreationOfSimpleAnnotations() throws ArtemisClientException { this.assessment = this.programmingSubmission.tryLock(this.gradingConfig).orElseThrow(); List tests = this.assessment.getTestResults(); - Assertions.assertEquals(13, tests.size()); + assertEquals(13, tests.size()); - Assertions.assertEquals(1, this.assessment.getAnnotations().size()); - Assertions.assertEquals( + assertEquals(1, this.assessment.getAnnotations().size()); + assertEquals( AnnotationSource.MANUAL_FIRST_ROUND, this.assessment.getAnnotations().get(0).getSource()); } @@ -128,13 +130,13 @@ void testCreationOfCustomAnnotation() throws ArtemisClientException { this.assessment = this.programmingSubmission.tryLock(this.gradingConfig).orElseThrow(); List tests = this.assessment.getTestResults(); - Assertions.assertEquals(13, tests.size()); + assertEquals(13, tests.size()); - Assertions.assertEquals(1, this.assessment.getAnnotations().size()); + assertEquals(1, this.assessment.getAnnotations().size()); var annotation = this.assessment.getAnnotations().get(0); - Assertions.assertEquals(AnnotationSource.MANUAL_FIRST_ROUND, annotation.getSource()); - Assertions.assertEquals(Optional.of(-2.0), annotation.getCustomScore()); - Assertions.assertEquals(Optional.of(FEEDBACK_TEXT), annotation.getCustomMessage()); + assertEquals(AnnotationSource.MANUAL_FIRST_ROUND, annotation.getSource()); + assertEquals(Optional.of(-2.0), annotation.getCustomScore()); + assertEquals(Optional.of(FEEDBACK_TEXT), annotation.getCustomMessage()); } @Test @@ -142,16 +144,16 @@ void testExportImport() throws ArtemisClientException { MistakeType mistakeType = this.gradingConfig.getMistakeTypes().get(1); this.assessment.addPredefinedAnnotation(mistakeType, "src/edu/kit/informatik/BubbleSort.java", 1, 2, null); - Assertions.assertEquals(1, this.assessment.getAnnotations().size()); + assertEquals(1, this.assessment.getAnnotations().size()); var oldAnnotations = this.assessment.getAnnotations(); String exportedAssessment = this.assessment.exportAssessment(); this.assessment.clearAnnotations(); - Assertions.assertEquals(0, this.assessment.getAnnotations().size()); + assertEquals(0, this.assessment.getAnnotations().size()); this.assessment.importAssessment(exportedAssessment); - Assertions.assertEquals(oldAnnotations, this.assessment.getAnnotations()); + assertEquals(oldAnnotations, this.assessment.getAnnotations()); } @Test @@ -176,11 +178,11 @@ void testAssessmentFetchesFeedbacks() throws ArtemisClientException { } } - Assertions.assertEquals(this.programmingSubmission, updatedSubmission); + assertEquals(this.programmingSubmission, updatedSubmission); Assessment newAssessment = updatedSubmission.openAssessment(this.gradingConfig).orElseThrow(); - Assertions.assertEquals(1, newAssessment.getAnnotations().size()); + assertEquals(1, newAssessment.getAnnotations().size()); } @Test @@ -338,7 +340,7 @@ void testAnnotationMerging() throws ArtemisClientException { feedbackTexts.add(feedbackDTO.detailText()); } - Assertions.assertEquals( + assertEquals( List.of( // other feedback is 5 annotations in MergeSort and 5 in Client that should be merged "[Funktionalität:Custom Penalty] Other Feedback 0 (0P)", @@ -358,4 +360,17 @@ void testAnnotationMerging() throws ArtemisClientException { "[Funktionalität:JavaDoc Leer] JavaDoc ist leer oder nicht vorhanden\nExplanation: Has used last annotation for message. Weitere Probleme in L12."), feedbackTexts); } + + @Test + void testHighlight() { + assertEquals( + MistakeType.Highlight.DEFAULT, + this.gradingConfig.getMistakeTypeById("custom").getHighlight()); + assertEquals( + MistakeType.Highlight.DEFAULT, + this.gradingConfig.getMistakeTypeById("jdEmpty").getHighlight()); + assertEquals( + MistakeType.Highlight.NONE, + this.gradingConfig.getMistakeTypeById("magicLiteral").getHighlight()); + } } diff --git a/src/test/resources/config.json b/src/test/resources/config.json index da00df6..bf44af6 100644 --- a/src/test/resources/config.json +++ b/src/test/resources/config.json @@ -37,7 +37,8 @@ "threshold": 1, "penalty": 5 }, - "appliesTo": "functionality" + "appliesTo": "functionality", + "highlight": "default" }, { "shortName": "jdTrivial", @@ -49,6 +50,21 @@ "penalty": 5 }, "appliesTo": "functionality" + }, + { + "shortName": "magicLiteral", + "message": "Literal wird nicht als Konstante ausgelagert", + "button": "Magic Literal", + "penaltyRule": { + "shortName": "thresholdPenalty", + "threshold": 1, + "penalty": 1.0 + }, + "appliesTo": "functionality", + "autograderProblemTypes": [ + "MAGIC_LITERAL" + ], + "highlight": "none" } ] -} \ No newline at end of file +}