diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/DndValidationResponse.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/DndValidationResponse.java new file mode 100644 index 0000000000..5ec85f779e --- /dev/null +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/DndValidationResponse.java @@ -0,0 +1,64 @@ +/* + * Copyright 2022 James Sharkey + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package uk.ac.cam.cl.dtg.isaac.dos; + +import uk.ac.cam.cl.dtg.isaac.dos.content.Choice; +import uk.ac.cam.cl.dtg.isaac.dos.content.Content; +import uk.ac.cam.cl.dtg.isaac.dos.content.DTOMapping; +import uk.ac.cam.cl.dtg.isaac.dto.DndValidationResponseDTO; + +import java.util.Date; +import java.util.Map; + + +/** + * Class for providing correctness feedback about drag and drop questions in a submitted Choice. + */ +@DTOMapping(DndValidationResponseDTO.class) +public class DndValidationResponse extends QuestionValidationResponse { + private Map dropZonesCorrect; + + /** + * Default constructor for Jackson. + */ + public DndValidationResponse() { + } + + /** + * Full constructor. + * + * @param questionId - questionId. + * @param answer - answer. + * @param correct - correct. + * @param dropZonesCorrect - map of correctness status of each submitted item. Key: dropZoneId, value: isCorrect + * @param explanation - explanation. + * @param dateAttempted - dateAttempted. + */ + public DndValidationResponse(final String questionId, final Choice answer, + final Boolean correct, final Map dropZonesCorrect, + final Content explanation, final Date dateAttempted) { + super(questionId, answer, correct, explanation, dateAttempted); + this.dropZonesCorrect = dropZonesCorrect; + } + + public Map getDropZonesCorrect() { + return dropZonesCorrect; + } + + public void setDropZonesCorrect(final Map itemsCorrect) { + this.dropZonesCorrect = itemsCorrect; + } +} diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/IsaacDndQuestion.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/IsaacDndQuestion.java index 485bf0db11..c55e239ccd 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/IsaacDndQuestion.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/IsaacDndQuestion.java @@ -17,19 +17,17 @@ import uk.ac.cam.cl.dtg.isaac.dos.content.DTOMapping; import uk.ac.cam.cl.dtg.isaac.dos.content.JsonContentType; -import uk.ac.cam.cl.dtg.isaac.dto.IsaacClozeQuestionDTO; import uk.ac.cam.cl.dtg.isaac.dto.IsaacDndQuestionDTO; -import uk.ac.cam.cl.dtg.isaac.quiz.IsaacClozeValidator; +import uk.ac.cam.cl.dtg.isaac.quiz.IsaacDndValidator; import uk.ac.cam.cl.dtg.isaac.quiz.ValidatesWith; - /** * Content DO for IsaacDndQuestions. * */ @DTOMapping(IsaacDndQuestionDTO.class) @JsonContentType("isaacDndQuestion") -@ValidatesWith(IsaacClozeValidator.class) +@ValidatesWith(IsaacDndValidator.class) public class IsaacDndQuestion extends IsaacItemQuestion { private Boolean withReplacement; diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/content/DndChoice.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/content/DndChoice.java index f337ca06bc..e300ed383f 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/content/DndChoice.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/content/DndChoice.java @@ -17,16 +17,28 @@ import uk.ac.cam.cl.dtg.isaac.dto.content.DndChoiceDTO; +import java.util.List; + /** - * Choice for Dnd Questions, containing a list of DndItems. + * Choice for Item Questions, containing a list of Items. * */ @DTOMapping(DndChoiceDTO.class) @JsonContentType("dndChoice") -public class DndChoice extends ItemChoice { +public class DndChoice extends Choice { + private List items; + /** * Default constructor required for mapping. */ public DndChoice() { } -} + + public List getItems() { + return items; + } + + public void setItems(final List items) { + this.items = items; + } +} \ No newline at end of file diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/content/ItemChoice.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/content/ItemChoice.java index 9d554a9288..0bc3345703 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/content/ItemChoice.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/content/ItemChoice.java @@ -26,7 +26,6 @@ @DTOMapping(ItemChoiceDTO.class) @JsonContentType("itemChoice") public class ItemChoice extends Choice { - private Boolean allowSubsetMatch; private List items; diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/DndValidationResponseDTO.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/DndValidationResponseDTO.java new file mode 100644 index 0000000000..35b9a159d9 --- /dev/null +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/DndValidationResponseDTO.java @@ -0,0 +1,60 @@ +/* + * Copyright 2022 James Sharkey + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package uk.ac.cam.cl.dtg.isaac.dto; + +import uk.ac.cam.cl.dtg.isaac.dto.content.ChoiceDTO; +import uk.ac.cam.cl.dtg.isaac.dto.content.ContentDTO; + +import java.util.Date; +import java.util.Map; + +/** + * Class for providing correctness feedback about drag and drop questions in a submitted Choice. + */ +public class DndValidationResponseDTO extends QuestionValidationResponseDTO { + private Map dropZonesCorrect; + + /** + * Default constructor for Jackson. + */ + public DndValidationResponseDTO() { + } + + /** + * Full constructor. + * + * @param questionId - questionId. + * @param answer - answer. + * @param correct - correct. + * @param dropZonesCorrect - map of correctness status of each submitted item. Key: dropZoneId, value: isCorrect + * @param explanation - explanation. + * @param dateAttempted - dateAttempted. + */ + public DndValidationResponseDTO(final String questionId, final ChoiceDTO answer, + final Boolean correct, final Map dropZonesCorrect, + final ContentDTO explanation, final Date dateAttempted) { + super(questionId, answer, correct, explanation, dateAttempted); + this.dropZonesCorrect = dropZonesCorrect; + } + + public Map getDropZonesCorrect() { + return dropZonesCorrect; + } + + public void setDropZonesCorrect(final Map dropZonesCorrect) { + this.dropZonesCorrect = dropZonesCorrect; + } +} diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/content/DndChoiceDTO.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/content/DndChoiceDTO.java index e32e3a80dc..5a82f67db1 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/content/DndChoiceDTO.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/content/DndChoiceDTO.java @@ -15,11 +15,14 @@ */ package uk.ac.cam.cl.dtg.isaac.dto.content; +import java.util.List; + /** * Choice for Dnd Questions, containing a list of DndItems. * */ -public class DndChoiceDTO extends ItemChoiceDTO { +public class DndChoiceDTO extends ChoiceDTO { + private List items; /** * Default constructor required for mapping. @@ -27,4 +30,11 @@ public class DndChoiceDTO extends ItemChoiceDTO { public DndChoiceDTO() { } + public List getItems() { + return items; + } + + public void setItems(final List items) { + this.items = items; + } } diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacDndValidator.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacDndValidator.java new file mode 100644 index 0000000000..789a0d5b1d --- /dev/null +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacDndValidator.java @@ -0,0 +1,268 @@ +/* + * Copyright 2021 Chris Purdy, 2022 James Sharkey + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package uk.ac.cam.cl.dtg.isaac.quiz; + +import org.apache.commons.lang3.BooleanUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import uk.ac.cam.cl.dtg.isaac.api.Constants; +import uk.ac.cam.cl.dtg.isaac.dos.DndValidationResponse; +import uk.ac.cam.cl.dtg.isaac.dos.IsaacDndQuestion; +import uk.ac.cam.cl.dtg.isaac.dos.content.Choice; +import uk.ac.cam.cl.dtg.isaac.dos.content.Content; +import uk.ac.cam.cl.dtg.isaac.dos.content.ContentBase; +import uk.ac.cam.cl.dtg.isaac.dos.content.DndChoice; +import uk.ac.cam.cl.dtg.isaac.dos.content.DndItem; +import uk.ac.cam.cl.dtg.isaac.dos.content.Figure; +import uk.ac.cam.cl.dtg.isaac.dos.content.Question; +import uk.ac.cam.cl.dtg.util.FigureRegion; + +import java.util.Comparator; +import java.util.Date; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.function.BiFunction; +import java.util.function.Predicate; +import java.util.regex.Pattern; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +/** Validator that only provides functionality to validate Drag and drop questions. */ +public class IsaacDndValidator implements IValidator { + public static final String FEEDBACK_QUESTION_INVALID_ANS = "This question contains at least one invalid answer."; + public static final String FEEDBACK_QUESTION_EMPTY_ANS = "This question contains an empty answer."; + public static final String FEEDBACK_QUESTION_UNRECOGNISED_ANS = "This question contains at least one answer in an " + + "unrecognised format."; + public static final String FEEDBACK_QUESTION_MISSING_ITEMS = "This question is missing items."; + public static final String FEEDBACK_QUESTION_NO_DZ = "This question doesn't have any drop zones."; + public static final String FEEDBACK_QUESTION_DUP_DZ = "This question contains duplicate drop zones."; + public static final String FEEDBACK_QUESTION_UNUSED_DZ = "This question contains a correct answer that doesn't use " + + "all drop zones."; + public static final String FEEDBACK_QUESTION_INVALID_DZ = "One of the answers to this question references an " + + "invalid drop zone."; + public static final String FEEDBACK_ANSWER_NOT_ENOUGH = "You did not provide a valid answer; it does not contain " + + "an item for each gap."; + public static final String FEEDBACK_ANSWER_TOO_MUCH = "You did not provide a valid answer; it contains more items " + + "than gaps."; + private static final Logger log = LoggerFactory.getLogger(IsaacDndValidator.class); + + @Override + public final DndValidationResponse validateQuestionResponse(final Question question, final Choice answer) { + return validate(question, answer) + .map(msg -> new DndValidationResponse(question.getId(), answer, false, null, new Content(msg), new Date())) + .orElseGet(() -> mark((IsaacDndQuestion) question, (DndChoice) answer)); + } + + private DndValidationResponse mark(final IsaacDndQuestion question, final DndChoice answer) { + var sortedAnswers = QuestionHelpers.getChoices(question) + .sorted(Comparator + .comparingInt((DndChoice choice) -> ChoiceHelpers.matchStrength(choice, answer)) + .thenComparing(DndChoice::isCorrect) + .reversed() + ).collect(Collectors.toList()); + var matchedAnswer = sortedAnswers.stream().filter(lhs -> ChoiceHelpers.matches(lhs, answer)).findFirst(); + var closestCorrect = sortedAnswers.stream().filter(DndChoice::isCorrect).findFirst(); + + var isCorrect = matchedAnswer.map(DndChoice::isCorrect).orElse(false); + var dropZonesCorrect = QuestionHelpers.getDetailedItemFeedback(question) + ? closestCorrect.map(correct -> ChoiceHelpers.getDropZonesCorrect(correct, answer)).orElse(null) : null; + var feedback = (Content) matchedAnswer.map(Choice::getExplanation) + .or(() -> !isCorrect && answer.getItems().size() < closestCorrect.map(c -> c.getItems().size()).orElse(0) + ? Optional.of(new Content(FEEDBACK_ANSWER_NOT_ENOUGH)) : Optional.empty()) + .or(() -> !isCorrect ? Optional.ofNullable(question.getDefaultFeedback()) : Optional.empty()) + .orElse(null); + return new DndValidationResponse(question.getId(), answer, isCorrect, dropZonesCorrect, feedback, new Date()); + } + + private Optional validate(final Question question, final Choice answer) { + Objects.requireNonNull(question); + Objects.requireNonNull(answer); + + if (!(answer instanceof DndChoice)) { + throw new IllegalArgumentException(String.format( + "This validator only works with DndChoices (%s is not DndChoice)", question.getId())); + } + + if (!(question instanceof IsaacDndQuestion)) { + throw new IllegalArgumentException(String.format( + "This validator only works with IsaacDndQuestions (%s is not IsaacDndQuestion)", question.getId())); + } + + return ValidationUtils.BiRuleValidator.of( + questionValidator(IsaacDndValidator::logIfTrue) + ) + .add(Constants.FEEDBACK_NO_ANSWER_PROVIDED, (q, a) -> a.getItems() == null || a.getItems().isEmpty()) + .add(Constants.FEEDBACK_UNRECOGNISED_FORMAT, (q, a) -> a.getItems().stream().anyMatch(i -> + i.getId() == null || i.getDropZoneId() == null + )) + .add(Constants.FEEDBACK_UNRECOGNISED_ITEMS, (q, a) -> a.getItems().stream().anyMatch(i -> + !q.getItems().contains(i) + || !QuestionHelpers.getDropZonesFromContent(q).contains(i.getDropZoneId())) + ) + .add(FEEDBACK_ANSWER_TOO_MUCH, (q, a) -> + a.getItems().size() > QuestionHelpers.getAnyCorrect(q).map(c -> c.getItems().size()).orElse(0) + ) + .check((IsaacDndQuestion) question, (DndChoice) answer); + } + + /** A validator whose .check method determines whether the given question is valid. */ + public static ValidationUtils.RuleValidator questionValidator( + final BiFunction logged + ) { + return new ValidationUtils.RuleValidator() + .add(Constants.FEEDBACK_NO_CORRECT_ANSWERS, q -> logged.apply( + q.getChoices() == null || q.getChoices().isEmpty(), + String.format("Question does not have any answers. %s src: %s", q.getId(), q.getCanonicalSourceFile()) + )) + .add(Constants.FEEDBACK_NO_CORRECT_ANSWERS, q -> logged.apply( + q.getChoices().stream().noneMatch(Choice::isCorrect), + String.format("Question does not have any correct answers. %s src: %s", q.getId(), q.getCanonicalSourceFile()) + )) + .add(FEEDBACK_QUESTION_INVALID_ANS, q -> q.getChoices().stream().anyMatch(c -> logged.apply( + !DndChoice.class.equals(c.getClass()), + String.format("Expected DndItem in question (%s), instead found %s!", q.getId(), c.getClass()) + ))) + .add(FEEDBACK_QUESTION_EMPTY_ANS, q -> QuestionHelpers.getChoices(q).anyMatch(c -> logged.apply( + c.getItems() == null || c.getItems().isEmpty(), + String.format("Expected list of DndItems, but none found in choice for question id (%s)!", q.getId()) + ))) + .add(FEEDBACK_QUESTION_INVALID_ANS, q -> QuestionHelpers.getChoices(q).anyMatch(c -> logged.apply( + c.getItems().stream().anyMatch(i -> i.getClass() != DndItem.class), + String.format("Expected list of DndItems, but something else found in choice for question id (%s)!", q.getId()) + ))) + .add(FEEDBACK_QUESTION_UNRECOGNISED_ANS, q -> QuestionHelpers.getChoices(q).anyMatch(c -> logged.apply( + c.getItems().stream().anyMatch(i -> + i.getId() == null || i.getDropZoneId() == null + || Objects.equals(i.getId(), "") || Objects.equals(i.getDropZoneId(), "") + ), String.format("Found item with missing id or drop zone id in answer for question id (%s)!", q.getId()) + ))) + .add(FEEDBACK_QUESTION_MISSING_ITEMS, q -> logged.apply( + q.getItems() == null || q.getItems().isEmpty(), + String.format("Expected items in question (%s), but didn't find any!", q.getId()) + )) + .add(FEEDBACK_QUESTION_NO_DZ, q -> logged.apply( + QuestionHelpers.getDropZonesFromContent(q).isEmpty(), + String.format("Question does not have any drop zones. %s src %s", q.getId(), q.getCanonicalSourceFile()) + )) + .add(FEEDBACK_QUESTION_DUP_DZ, q -> logged.apply( + QuestionHelpers.getDropZonesFromContent(q).size() != new HashSet<>(QuestionHelpers.getDropZonesFromContent(q)).size(), + String.format("Question contains duplicate drop zones. %s src %s", q.getId(), q.getCanonicalSourceFile()) + )) + .add(FEEDBACK_QUESTION_UNUSED_DZ, q -> QuestionHelpers.anyCorrectMatch(q, c -> logged.apply( + QuestionHelpers.getDropZonesFromContent(q).size() != c.getItems().size(), + String.format("Question contains correct answer that doesn't use all drop zones. %s src %s", + q.getId(), q.getCanonicalSourceFile()) + ))) + .add(FEEDBACK_QUESTION_INVALID_DZ, q -> QuestionHelpers.getChoices(q).anyMatch(c -> logged.apply( + c.getItems().stream().anyMatch(i -> !QuestionHelpers.getDropZonesFromContent(q).contains(i.getDropZoneId())), + String.format("Question contains invalid drop zone reference. %s src %s", q.getId(), q.getCanonicalSourceFile()) + ))); + } + + private static boolean logIfTrue(final boolean result, final String message) { + if (result) { + log.error(message); + } + return result; + } + + @SuppressWarnings("checkstyle:MissingJavadocType") + public static class QuestionHelpers { + public static Stream getChoices(final IsaacDndQuestion question) { + return question.getChoices().stream().map(c -> (DndChoice) c); + } + + public static Optional getAnyCorrect(final IsaacDndQuestion question) { + return getChoices(question).filter(DndChoice::isCorrect).findFirst(); + } + + public static boolean anyCorrectMatch(final IsaacDndQuestion question, final Predicate p) { + return getChoices(question).filter(DndChoice::isCorrect).anyMatch(p); + + } + + public static boolean getDetailedItemFeedback(final IsaacDndQuestion question) { + return BooleanUtils.isTrue(question.getDetailedItemFeedback()); + } + + /** + * Collects the drop zone ids from any content within the question. + */ + public static List getDropZonesFromContent(final IsaacDndQuestion question) { + if (question.getChildren() == null) { + return List.of(); + } + return question.getChildren().stream() + .flatMap(QuestionHelpers::getDropZonesFromContent) + .collect(Collectors.toList()); + } + + private static Stream getDropZonesFromContent(final ContentBase content) { + if (content instanceof Figure && ((Figure) content).getFigureRegions() != null) { + var figure = (Figure) content; + return figure.getFigureRegions().stream().map(FigureRegion::getId); + } + if (content instanceof Content && ((Content) content).getValue() != null) { + var textContent = (Content) content; + String expr = "\\[drop-zone:(?[a-zA-Z0-9_-]+)(?\\|(?w-\\d+?)?(?h-\\d+?)?)?]"; + Pattern dndDropZoneRegex = Pattern.compile(expr); + return dndDropZoneRegex.matcher(textContent.getValue()).results().map(mr -> mr.group(1)); + } + if (content instanceof Content && ((Content) content).getChildren() != null) { + return ((Content) content).getChildren().stream().flatMap(QuestionHelpers::getDropZonesFromContent); + } + + return Stream.of(); + } + } + + private static class ChoiceHelpers { + public static boolean matches(final DndChoice lhs, final DndChoice rhs) { + return lhs.getItems().stream().allMatch(lhsItem -> dropZoneEql(rhs, lhsItem)); + } + + public static int matchStrength(final DndChoice lhs, final DndChoice rhs) { + return lhs.getItems().stream() + .map(lhsItem -> dropZoneEql(rhs, lhsItem) ? 1 : 0) + .mapToInt(Integer::intValue) + .sum(); + } + + public static Map getDropZonesCorrect(final DndChoice lhs, final DndChoice rhs) { + return lhs.getItems().stream() + .filter(lhsItem -> getItemByDropZone(rhs, lhsItem.getDropZoneId()).isPresent()) + .collect(Collectors.toMap( + DndItem::getDropZoneId, + lhsItem -> dropZoneEql(rhs, lhsItem)) + ); + } + + private static boolean dropZoneEql(final DndChoice choice, final DndItem item) { + return getItemByDropZone(choice, item.getDropZoneId()) + .map(choiceItem -> choiceItem.getId().equals(item.getId())) + .orElse(false); + } + + private static Optional getItemByDropZone(final DndChoice choice, final String dropZoneId) { + return choice.getItems().stream() + .filter(item -> item.getDropZoneId().equals(dropZoneId)) + .findFirst(); + } + } +} diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/ValidationUtils.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/ValidationUtils.java index 9bc6a8058d..d1cda06ea2 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/ValidationUtils.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/ValidationUtils.java @@ -5,7 +5,12 @@ import java.math.BigDecimal; import java.math.MathContext; import java.math.RoundingMode; +import java.util.ArrayList; +import java.util.List; import java.util.Objects; +import java.util.Optional; +import java.util.function.BiPredicate; +import java.util.function.Predicate; import static java.lang.Math.max; import static java.lang.Math.min; @@ -279,4 +284,51 @@ public static boolean tooManySignificantFigures(final String valueToCheck, final return sigFigsFromUser.sigFigsMin > maxAllowedSigFigs; } + /** Validate a set of rules. Each rule takes two arguments. `.add` some rules, then `.check` whether they held.*/ + public static class BiRuleValidator { + protected final List> rules = new ArrayList<>(); + + /** Create a BiRuleValidator from a RuleValidator. */ + public static BiRuleValidator of(final RuleValidator validator) { + BiRuleValidator biValidator = new BiRuleValidator<>(); + validator.rules.forEach(r -> biValidator.add(r.message, (t, u) -> r.predicate.test(t, null))); + return biValidator; + } + + public BiRuleValidator add(final String message, final BiPredicate rule) { + rules.add(new Rule<>(message, rule)); + return this; + } + + /** Apply the validation rules on a set of objects. */ + public Optional check(final T t, final U u) { + return rules.stream() + .filter(r -> r.predicate.test(t, u)) + .map(r -> r.message) + .findFirst(); + } + + /** A rule used by either a BiRuleValidator or a RuleValidator. */ + protected static class Rule { + public final String message; + public final BiPredicate predicate; + + public Rule(final String message, final BiPredicate predicate) { + this.message = message; + this.predicate = predicate; + } + } + } + + /** A specialized BiRuleValidator whose rules take a single argument. */ + public static class RuleValidator extends BiRuleValidator { + public RuleValidator add(final String message, final Predicate rule) { + super.add(message, (t, ignored) -> rule.test(t)); + return this; + } + + public Optional check(final T t) { + return super.check(t, null); + } + } } diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/configuration/SegueGuiceConfigurationModule.java b/src/main/java/uk/ac/cam/cl/dtg/segue/configuration/SegueGuiceConfigurationModule.java index 17d22f969e..649ae10a61 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/configuration/SegueGuiceConfigurationModule.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/configuration/SegueGuiceConfigurationModule.java @@ -1501,6 +1501,10 @@ public static synchronized Injector getGuiceInjector() { return injector; } + public static void setInjector(final Injector newInjector) { + injector = newInjector; + } + @Provides @Singleton public static Clock getDefaultClock() { diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/ChoiceOrikaConverter.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/ChoiceOrikaConverter.java new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/ItemOrikaConverter.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/ItemOrikaConverter.java new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/users/QuestionValidationResponseDeserializer.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/users/QuestionValidationResponseDeserializer.java index de52a0b50d..137224cb63 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/users/QuestionValidationResponseDeserializer.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/users/QuestionValidationResponseDeserializer.java @@ -22,6 +22,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.module.SimpleModule; import com.fasterxml.jackson.databind.node.ObjectNode; +import uk.ac.cam.cl.dtg.isaac.dos.DndValidationResponse; import uk.ac.cam.cl.dtg.isaac.dos.ItemValidationResponse; import uk.ac.cam.cl.dtg.isaac.dos.LLMFreeTextQuestionValidationResponse; import uk.ac.cam.cl.dtg.isaac.dos.QuantityValidationResponse; @@ -87,6 +88,8 @@ public QuestionValidationResponse deserialize(final JsonParser jsonParser, return mapper.readValue(jsonString, ItemValidationResponse.class); } else if (questionResponseType.equals("llmFreeTextChoice")) { return mapper.readValue(jsonString, LLMFreeTextQuestionValidationResponse.class); + } else if (questionResponseType.equals("dndChoice")) { + return mapper.readValue(jsonString, DndValidationResponse.class); } else { return mapper.readValue(jsonString, QuestionValidationResponse.class); } diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/etl/ContentIndexer.java b/src/main/java/uk/ac/cam/cl/dtg/segue/etl/ContentIndexer.java index 3204d55092..39b33949bc 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/etl/ContentIndexer.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/etl/ContentIndexer.java @@ -21,6 +21,7 @@ import uk.ac.cam.cl.dtg.isaac.dos.IsaacCardDeck; import uk.ac.cam.cl.dtg.isaac.dos.IsaacClozeQuestion; import uk.ac.cam.cl.dtg.isaac.dos.IsaacCoordinateQuestion; +import uk.ac.cam.cl.dtg.isaac.dos.IsaacDndQuestion; import uk.ac.cam.cl.dtg.isaac.dos.IsaacEventPage; import uk.ac.cam.cl.dtg.isaac.dos.IsaacNumericQuestion; import uk.ac.cam.cl.dtg.isaac.dos.IsaacQuestionBase; @@ -45,6 +46,7 @@ import uk.ac.cam.cl.dtg.isaac.dos.content.Quantity; import uk.ac.cam.cl.dtg.isaac.dos.content.Question; import uk.ac.cam.cl.dtg.isaac.dos.content.Video; +import uk.ac.cam.cl.dtg.isaac.quiz.IsaacDndValidator; import uk.ac.cam.cl.dtg.segue.api.Constants; import uk.ac.cam.cl.dtg.segue.dao.content.ContentManagerException; import uk.ac.cam.cl.dtg.segue.dao.content.ContentSubclassMapper; @@ -67,6 +69,7 @@ import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.function.BiFunction; import java.util.stream.Collectors; import static com.google.common.collect.Maps.immutableEntry; @@ -1153,6 +1156,14 @@ private void recordContentTypeSpecificError(final String sha, final Content cont } } + if (content instanceof IsaacDndQuestion) { + BiFunction noLog = (res, msg) -> res; + IsaacDndValidator.questionValidator(noLog).check((IsaacDndQuestion) content).ifPresent(err -> { + var template = "Drag-and-drop Question: %s has a problem. %s"; + this.registerContentProblem(content, String.format(template, content.getId(), err), indexProblemCache); + }); + } + if (content instanceof IsaacCoordinateQuestion) { IsaacCoordinateQuestion q = (IsaacCoordinateQuestion) content; diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/etl/ElasticSearchIndexer.java b/src/main/java/uk/ac/cam/cl/dtg/segue/etl/ElasticSearchIndexer.java index 96ce5be527..f31d1d7205 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/etl/ElasticSearchIndexer.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/etl/ElasticSearchIndexer.java @@ -43,7 +43,7 @@ /** * Created by Ian on 17/10/2016. */ -class ElasticSearchIndexer extends ElasticSearchProvider { +public class ElasticSearchIndexer extends ElasticSearchProvider { private static final Integer BULK_REQUEST_BATCH_SIZE = 10000; // Huge requests overwhelm ES, so batch! private static final Logger log = LoggerFactory.getLogger(ElasticSearchIndexer.class); @@ -131,7 +131,7 @@ void bulkIndex(final String indexBase, final String indexType, final List> dataToIndex) + public void bulkIndexWithIDs(final String indexBase, final String indexType, final List> dataToIndex) throws SegueSearchException { Iterable>> partitions = Iterables.partition(dataToIndex, BULK_REQUEST_BATCH_SIZE); diff --git a/src/main/java/uk/ac/cam/cl/dtg/util/mappers/QuestionValidationMapper.java b/src/main/java/uk/ac/cam/cl/dtg/util/mappers/QuestionValidationMapper.java index 9e326835ff..5fb9e13269 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/util/mappers/QuestionValidationMapper.java +++ b/src/main/java/uk/ac/cam/cl/dtg/util/mappers/QuestionValidationMapper.java @@ -4,11 +4,13 @@ import org.mapstruct.Mapper; import org.mapstruct.SubclassMapping; import org.mapstruct.factory.Mappers; +import uk.ac.cam.cl.dtg.isaac.dos.DndValidationResponse; import uk.ac.cam.cl.dtg.isaac.dos.FormulaValidationResponse; import uk.ac.cam.cl.dtg.isaac.dos.ItemValidationResponse; import uk.ac.cam.cl.dtg.isaac.dos.LLMFreeTextQuestionValidationResponse; import uk.ac.cam.cl.dtg.isaac.dos.QuantityValidationResponse; import uk.ac.cam.cl.dtg.isaac.dos.QuestionValidationResponse; +import uk.ac.cam.cl.dtg.isaac.dto.DndValidationResponseDTO; import uk.ac.cam.cl.dtg.isaac.dto.FormulaValidationResponseDTO; import uk.ac.cam.cl.dtg.isaac.dto.ItemValidationResponseDTO; import uk.ac.cam.cl.dtg.isaac.dto.LLMFreeTextQuestionValidationResponseDTO; @@ -24,6 +26,7 @@ public interface QuestionValidationMapper { QuestionValidationMapper INSTANCE = Mappers.getMapper(QuestionValidationMapper.class); @SubclassMapping(source = FormulaValidationResponse.class, target = FormulaValidationResponseDTO.class) + @SubclassMapping(source = DndValidationResponse.class, target = DndValidationResponseDTO.class) @SubclassMapping(source = ItemValidationResponse.class, target = ItemValidationResponseDTO.class) @SubclassMapping(source = LLMFreeTextQuestionValidationResponse.class, target = LLMFreeTextQuestionValidationResponseDTO.class) @SubclassMapping(source = QuantityValidationResponse.class, target = QuantityValidationResponseDTO.class) diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AbstractIsaacIntegrationTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AbstractIsaacIntegrationTest.java index 576719b0ac..18db76daa4 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AbstractIsaacIntegrationTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AbstractIsaacIntegrationTest.java @@ -47,9 +47,11 @@ import uk.ac.cam.cl.dtg.segue.api.managers.UserAccountManager; import uk.ac.cam.cl.dtg.segue.api.managers.UserAssociationManager; import uk.ac.cam.cl.dtg.segue.api.managers.UserAuthenticationManager; +import uk.ac.cam.cl.dtg.segue.api.monitors.AnonQuestionAttemptMisuseHandler; import uk.ac.cam.cl.dtg.segue.api.monitors.EmailVerificationMisuseHandler; import uk.ac.cam.cl.dtg.segue.api.monitors.GroupManagerLookupMisuseHandler; import uk.ac.cam.cl.dtg.segue.api.monitors.IMisuseMonitor; +import uk.ac.cam.cl.dtg.segue.api.monitors.IPQuestionAttemptMisuseHandler; import uk.ac.cam.cl.dtg.segue.api.monitors.InMemoryMisuseMonitor; import uk.ac.cam.cl.dtg.segue.api.monitors.RegistrationMisuseHandler; import uk.ac.cam.cl.dtg.segue.api.monitors.TeacherPasswordResetMisuseHandler; @@ -81,6 +83,7 @@ import uk.ac.cam.cl.dtg.segue.dao.users.PgUsers; import uk.ac.cam.cl.dtg.segue.database.GitDb; import uk.ac.cam.cl.dtg.segue.database.PostgresSqlDb; +import uk.ac.cam.cl.dtg.segue.etl.ElasticSearchIndexer; import uk.ac.cam.cl.dtg.segue.search.ElasticSearchProvider; import uk.ac.cam.cl.dtg.util.AbstractConfigLoader; import uk.ac.cam.cl.dtg.util.YamlLoader; @@ -119,7 +122,7 @@ public class AbstractIsaacIntegrationTest { protected static AbstractConfigLoader properties; protected static Map globalTokens; protected static PostgresSqlDb postgresSqlDb; - protected static ElasticSearchProvider elasticSearchProvider; + protected static ElasticSearchIndexer elasticSearchProvider; protected static SchoolListReader schoolListReader; protected static MainMapper mainMapper; protected static ContentSummarizerService contentSummarizerService; @@ -151,6 +154,7 @@ public class AbstractIsaacIntegrationTest { protected static IQuizQuestionAttemptPersistenceManager quizQuestionAttemptPersistenceManager; protected static QuizQuestionManager quizQuestionManager; protected static PgUsers pgUsers; + protected static ContentSubclassMapper contentMapper; // Services protected static AssignmentService assignmentService; @@ -181,7 +185,7 @@ public class AbstractIsaacIntegrationTest { elasticsearch.start(); try { - elasticSearchProvider = new ElasticSearchProvider(ElasticSearchProvider.getClient( + elasticSearchProvider = new ElasticSearchIndexer(ElasticSearchProvider.getClient( "localhost", elasticsearch.getMappedPort(9200), "elastic", @@ -242,11 +246,11 @@ public static void setUpClass() throws Exception { pgAnonymousUsers = new PgAnonymousUsers(postgresSqlDb); passwordDataManager = new PgPasswordDataManager(postgresSqlDb); - ContentSubclassMapper contentMapper = new ContentSubclassMapper(new Reflections("uk.ac.cam.cl.dtg")); + contentMapper = new ContentSubclassMapper(new Reflections("uk.ac.cam.cl.dtg")); PgQuestionAttempts pgQuestionAttempts = new PgQuestionAttempts(postgresSqlDb, contentMapper); + mainMapper = MainMapper.INSTANCE; questionManager = new QuestionManager(contentMapper, mainMapper, pgQuestionAttempts); - mainMapper = MainMapper.INSTANCE; providersToRegister = new HashMap<>(); providersToRegister.put(AuthenticationProvider.RASPBERRYPI, new RaspberryPiOidcAuthenticator( @@ -317,6 +321,8 @@ public static void setUpClass() throws Exception { misuseMonitor.registerHandler(EmailVerificationMisuseHandler.class.getSimpleName(), new EmailVerificationMisuseHandler()); misuseMonitor.registerHandler(TeacherPasswordResetMisuseHandler.class.getSimpleName(), new TeacherPasswordResetMisuseHandler()); misuseMonitor.registerHandler(TokenOwnerLookupMisuseHandler.class.getSimpleName(), new TokenOwnerLookupMisuseHandler(emailManager, properties)); + misuseMonitor.registerHandler(AnonQuestionAttemptMisuseHandler.class.getSimpleName(), new AnonQuestionAttemptMisuseHandler()); + misuseMonitor.registerHandler(IPQuestionAttemptMisuseHandler.class.getSimpleName(), new IPQuestionAttemptMisuseHandler(emailManager, properties)); // todo: more handlers as required by different endpoints String someSegueAnonymousUserId = "9284723987anonymous83924923"; diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/IsaacIntegrationTestWithREST.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/IsaacIntegrationTestWithREST.java index fa5c71e7f0..6af3c54fac 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/IsaacIntegrationTestWithREST.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/IsaacIntegrationTestWithREST.java @@ -27,6 +27,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * Abstract superclass for integration test. Use when testing in the context of a REST application. This lets you @@ -140,12 +141,13 @@ public TestResponse post(final String url, final Object body) { return new TestResponse(response); } - public void loginAs(final RegisteredUser user) { + public TestClient loginAs(final RegisteredUser user) { var request = client.target(baseUrl + "/auth/SEGUE/authenticate").request(MediaType.APPLICATION_JSON); var body = new LocalAuthDTO(); body.setEmail(user.getEmail()); body.setPassword("test1234"); this.currentUser = builder.apply(request).post(Entity.json(body), RegisteredUserDTO.class); + return this; } } @@ -157,8 +159,13 @@ static class TestResponse { } void assertError(final String message, final Response.Status status) { - assertEquals(message, response.readEntity(Map.class).get("errorMessage")); assertEquals(status.getStatusCode(), response.getStatus()); + assertTrue(this.readEntityAsJsonUnchecked().getString("errorMessage").contains(message)); + } + + void assertError(final String message, final String status) { + assertEquals(status, Integer.toString(response.getStatus())); + assertTrue(this.readEntityAsJsonUnchecked().getString("errorMessage").contains(message)); } void assertNoUserLoggedIn() { @@ -181,6 +188,16 @@ T readEntity(final Class klass) { assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); return response.readEntity(klass); } + + JSONObject readEntityAsJson() { + assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); + return this.readEntityAsJsonUnchecked(); + } + + private JSONObject readEntityAsJsonUnchecked() { + String body = response.readEntity(String.class); + return new JSONObject(body); + } } interface RequestBuilder extends Function {} diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/QuestionFacadeIT.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/QuestionFacadeIT.java new file mode 100644 index 0000000000..de134f0aed --- /dev/null +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/QuestionFacadeIT.java @@ -0,0 +1,259 @@ +package uk.ac.cam.cl.dtg.isaac.api; +import com.google.inject.Injector; +import org.json.JSONArray; +import org.json.JSONObject; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import uk.ac.cam.cl.dtg.isaac.dos.IsaacDndQuestion; +import uk.ac.cam.cl.dtg.isaac.dos.content.Content; +import uk.ac.cam.cl.dtg.isaac.dos.content.DndChoice; +import uk.ac.cam.cl.dtg.isaac.quiz.IsaacDndValidator; +import uk.ac.cam.cl.dtg.isaac.quiz.IsaacStringMatchValidator; +import uk.ac.cam.cl.dtg.segue.api.QuestionFacade; +import uk.ac.cam.cl.dtg.segue.configuration.SegueGuiceConfigurationModule; + +import jakarta.ws.rs.core.Response; + +import java.util.List; +import java.util.Map; + +import static org.easymock.EasyMock.createNiceMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.testcontainers.shaded.com.google.common.collect.Maps.immutableEntry; +import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacDndValidatorTest.*; + +@SuppressWarnings("checkstyle:MissingJavadocType") +public class QuestionFacadeIT extends IsaacIntegrationTestWithREST { + @Test + public void shows400ForMissingAnswer() throws Exception { + var response = subject().client().post("/questions/no_such_question/answer", null); + response.assertError("No answer received.", Response.Status.BAD_REQUEST); + } + + @Test + public void shows404ForMissingQuestion() throws Exception { + var response = subject().client().post("/questions/no_such_question/answer", "{}"); + response.assertError("No question object found for given id: no_such_question", Response.Status.NOT_FOUND); + } + + @Nested + class StringMatchQuestion { + @Test + public void wrongAnswer() throws Exception { + var response = subject().client().post( + url("_regression_test_|acc_stringmatch_q|_regression_test_stringmatch_"), + "{\"type\": \"stringChoice\", \"value\": \"13\"}" + ).readEntityAsJson(); + + assertFalse(response.getBoolean("correct")); + assertEquals("13", response.getJSONObject("answer").getString("value")); + } + + @Test + public void rightAnswer() throws Exception { + var response = subject().client().post( + url("_regression_test_|acc_stringmatch_q|_regression_test_stringmatch_"), + "{\"type\": \"stringChoice\", \"value\": \"hello\"}" + ).readEntityAsJson(); + + assertTrue(response.getBoolean("correct")); + assertEquals("hello", response.getJSONObject("answer").getString("value")); + } + } + + @Nested + class DndQuestion { + @Test + public void invalidQuestion() throws Exception { + var question = persistJSON("i1", new JSONObject() + .put("type", "isaacDndQuestion") + .put("choices", new JSONArray().put( + new JSONObject() + .put("type", "dndChoice") + .put("correct", true) + .put("items", new JSONArray().put( + new JSONObject().put("dropZoneId", "dzid") // bad because missing id + )) + )) + ); + var answer = "{\"type\": \"dndChoice\"}"; + + var response = subject().client().post(url(question.getString("id")), answer).readEntityAsJson(); + + assertFalse(response.getBoolean("correct")); + assertEquals( + "This question contains at least one answer in an unrecognised format.", + response.getJSONObject("explanation").getString("value") + ); + } + + @ParameterizedTest + @CsvSource(value = { + "{};Unable to map response to a Choice;404", + "{\"type\": \"unknown\"};This validator only works with DndChoices;400", + "{\"type\": \"dndChoice\", \"items\": [{\"id\": \"6d3d\", \"dropZoneId\": \"leg_1\", \"a\": \"a\"}]};Unable to map response to a Choice;404", + "{\"type\": \"dndChoice\", \"items\": \"some_string\"};Unable to map response to a Choice;404", + "{\"type\": \"dndChoice\", \"items\": [{\"id\": [{}], \"dropZoneId\": \"leg_1\"}]};Unable to map response to a Choice;404" + }, delimiter = ';') + public void badRequest_ErrorReturned(final String answerStr, final String emsg, final String estate) throws Exception { + var dndQuestion = persist(createQuestion( + correct(choose(item_3cm, "leg_1")) + )); + var response = subject().client().post(url(dndQuestion.getId()), answerStr); + response.assertError(emsg, estate); + } + + @ParameterizedTest + @CsvSource(value = { + "{\"type\": \"dndChoice\", \"items\": [{\"dropZoneId\": \"leg_1\"}]};Your answer is not in a recognised format.", + "{\"type\": \"dndChoice\", \"items\": [{\"id\": \"6d3d\"}]};Your answer is not in a recognised format." + }, delimiter = ';') + public void badRequest_IncorrectReturnedWithExplanation(final String answerStr, final String emsg) throws Exception { + var dndQuestion = persist(createQuestion( + correct(choose(item_3cm, "leg_1"), choose(item_4cm, "leg_2"), choose(item_5cm, "hypothenuse")) + )); + var response = subject().client().post(url(dndQuestion.getId()), answerStr).readEntityAsJson(); + assertFalse(response.getBoolean("correct")); + assertEquals(emsg, response.getJSONObject("explanation").getString("value")); + } + + @ParameterizedTest + @CsvSource(value = { + "{\"type\": \"dndChoice\"}", + "{\"type\": \"dndChoice\", \"items\": []}" + }, delimiter = ';') + public void emptyAnswer_IncorrectReturned(final String answerStr) throws Exception { + var dndQuestion = persist(createQuestion( + correct(choose(item_3cm, "leg_1")) + )); + var response = subject().client().post(url(dndQuestion.getId()), answerStr).readEntityAsJson(); + assertFalse(response.getBoolean("correct")); + assertEquals( + readEntity(new JSONObject(answerStr), DndChoice.class), + readEntity(response.getJSONObject("answer"), DndChoice.class) + ); + } + + @ParameterizedTest + @CsvSource(value = { + "{\"type\": \"dndChoice\", \"items\": [{\"id\": \"6d3d\", \"dropZoneId\": \"leg_1\"}]}", + "{\"type\": \"dndChoice\", \"items\": [{\"id\": \"6d3d\", \"dropZoneId\": \"leg_1\", \"type\": \"dndItem\"}]}", + "{\"type\": \"dndChoice\", \"items\": [{\"id\": \"6d3d\", \"dropZoneId\": \"leg_1\", \"type\": \"unknown\"}]}" + }, delimiter = ';') + public void correctAnswer_CorrectReturned(final String answerStr) throws Exception { + var dndQuestion = createQuestion(correct(choose(item_3cm, "leg_1"))); + dndQuestion.setChildren(List.of(new Content("[drop-zone:leg_1]"))); + persist(dndQuestion); + + var response = subject().client().post(url(dndQuestion.getId()), answerStr).readEntityAsJson(); + assertTrue(response.getBoolean("correct")); + + assertEquals(readChoice(new JSONObject(answerStr)), readChoice(response.getJSONObject("answer"))); + } + + @Test + public void wrongAnswer_IncorrectReturned() throws Exception { + var dndQuestion = persist(createQuestion( + correct(choose(item_3cm, "leg_1"), choose(item_4cm, "leg_2"), choose(item_5cm, "hypothenuse")) + )); + var answer = answer(choose(item_3cm, "leg_2"), choose(item_4cm, "hypothenuse"), choose(item_5cm, "leg_1")); + + var response = subject().client().post(url(dndQuestion.getId()), answer).readEntityAsJson(); + + assertFalse(response.getBoolean("correct")); + assertEquals(answer, readEntity(response.getJSONObject("answer"), DndChoice.class)); + } + + @Test + public void answerWithMatchingExplanation_ExplanationReturned() throws Exception { + var explanation = new Content("That's right!"); + var dndQuestion = persist(createQuestion(correct( + explanation, + choose(item_3cm, "leg_1"), choose(item_4cm, "leg_2"), choose(item_5cm, "hypothenuse") + ))); + var answer = answer(choose(item_3cm, "leg_1"), choose(item_4cm, "leg_2"), choose(item_5cm, "hypothenuse")); + + var response = subject().client().post(url(dndQuestion.getId()), answer).readEntityAsJson(); + + assertTrue(response.getBoolean("correct")); + assertEquals(explanation, readEntity(response.getJSONObject("explanation"), Content.class)); + } + + @Test + public void detailedItemFeedbackRequested_DropZonesCorrectReturned() throws Exception { + var dndQuestion = createQuestion( + correct(choose(item_3cm, "leg_1"), choose(item_4cm, "leg_2"), choose(item_5cm, "hypothenuse")) + ); + dndQuestion.setDetailedItemFeedback(true); + persist(dndQuestion); + var answer = answer(choose(item_3cm, "leg_1"), choose(item_4cm, "leg_2"), choose(item_5cm, "hypothenuse")); + + var response = subject().client().post(url(dndQuestion.getId()), answer).readEntityAsJson(); + + assertTrue(response.getBoolean("correct")); + assertEquals( + new DropZonesCorrectFactory().setLeg1(true).setLeg2(true).setHypothenuse(true).build(), + readEntity(response.getJSONObject("dropZonesCorrect"), Map.class) + ); + } + } + + private IsaacDndQuestion persist(final IsaacDndQuestion question) throws Exception { + elasticSearchProvider.bulkIndexWithIDs( + "6c2ba42c5c83d8f31b3b385b3a9f9400a12807c9", + "content", + List.of(immutableEntry( + question.getId(), contentMapper.getSharedContentObjectMapper().writeValueAsString(question)) + ) + ); + return question; + } + + private JSONObject persistJSON(final String id, final JSONObject questionJSON) throws Exception { + questionJSON.put("id", id); + elasticSearchProvider.bulkIndexWithIDs( + "6c2ba42c5c83d8f31b3b385b3a9f9400a12807c9", + "content", + List.of(immutableEntry(id, questionJSON.toString())) + ); + return questionJSON; + } + + + private TestServer subject() throws Exception { + Injector testInjector = createNiceMock(Injector.class); + expect(testInjector.getInstance(IsaacStringMatchValidator.class)).andReturn(stringMatchValidator).anyTimes(); + expect(testInjector.getInstance(IsaacDndValidator.class)).andReturn(dndValidator).anyTimes(); + replay(testInjector); + SegueGuiceConfigurationModule.setInjector(testInjector); + + return startServer( + new QuestionFacade(properties, contentMapper, contentManager, userAccountManager, + userPreferenceManager, questionManager, logManager, misuseMonitor, null, userAssociationManager) + ); + } + + private String url(final String questionId) { + return String.format("/questions/%s/answer", questionId); + } + + private T readEntity(final JSONObject value, final Class klass) throws Exception { + return contentMapper.getSharedContentObjectMapper().readValue(value.toString(), klass); + } + + private String readChoice(final JSONObject value) throws Exception { + value.put("tags", new JSONArray()); + value.getJSONArray("items").getJSONObject(0).put("tags", new JSONArray()); + DndChoice parsed = contentMapper.getSharedContentObjectMapper().readValue(value.toString(), DndChoice.class); + return contentMapper.getSharedContentObjectMapper().writeValueAsString(parsed); + } + + private static final IsaacStringMatchValidator stringMatchValidator = new IsaacStringMatchValidator(); + private static final IsaacDndValidator dndValidator = new IsaacDndValidator(); +} diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/TestAppender.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/TestAppender.java new file mode 100644 index 0000000000..c23e1c018d --- /dev/null +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/TestAppender.java @@ -0,0 +1,36 @@ +package uk.ac.cam.cl.dtg.isaac.api; + +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.appender.AbstractAppender; +import org.apache.logging.log4j.core.config.Property; + +import java.util.ArrayList; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; + + +public class TestAppender extends AbstractAppender { + private final List events = new ArrayList<>(); + + public TestAppender() { + super("TestAppender", null, null, true, Property.EMPTY_ARRAY); + start(); + } + + @Override + public void append(final LogEvent event) { + events.add(event.toImmutable()); + } + + public void assertLevel(final Level level) { + assertEquals(1, this.events.size()); + assertEquals(level, this.events.get(0).getLevel()); + } + + public void assertMessage(final String message) { + assertEquals(1, this.events.size()); + assertEquals(message, this.events.get(0).getMessage().getFormattedMessage()); + } +} \ No newline at end of file diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacClozeValidatorTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacClozeValidatorTest.java index cc18f6686b..05fabe8ec5 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacClozeValidatorTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacClozeValidatorTest.java @@ -77,6 +77,7 @@ public final void setUp() { someIncorrectChoice.setExplanation(new Content(incorrectExplanation)); someSubsetChoice.setItems(ImmutableList.of(NULL_PLACEHOLDER, item3)); someSubsetChoice.setAllowSubsetMatch(true); + someSubsetChoice.setCorrect(false); someSubsetChoice.setExplanation(new Content(subsetMatchExplanation)); // Add both choices to question, incorrect first: @@ -146,7 +147,7 @@ public final void isaacClozeValidator_KnownIncorrect_IncorrectResponseShouldBeRe /* Test that known incorrect answers can be matched. -*/ + */ @Test public final void isaacClozeValidator_KnownIncorrectDetailedFeedback_IncorrectResponseShouldBeReturned() { // Set up the question object: @@ -199,6 +200,41 @@ public final void isaacClozeValidator_NotEnoughItems_IncorrectResponseShouldBeRe assertTrue(response.getExplanation().getValue().contains("does not contain an item for each gap")); } + + /* + * Test that when the user submits an answer with missing items, we show the generic + * feedback about missing items, even though we have more specific feedback about + * some of the submitted answers being wrong. + * + * I think it'd be better to show specific feedback. This test is here to prove that + * this is not how the current implementation works. + */ + @Test + public final void isaacClozeValidator_NotEnoughItemsMatchingIncorrectResponse_NotEnoughResponseShouldBeReturned_() { + // Set up the question object: + IsaacClozeQuestion clozeQuestion = new IsaacClozeQuestion(); + clozeQuestion.setItems(ImmutableList.of(item1, item2)); + + ItemChoice someCorrectAnswer = new ItemChoice(); + someCorrectAnswer.setItems(ImmutableList.of(item1, item3)); + someCorrectAnswer.setCorrect(true); + ItemChoice someIncorrectAnswer = new ItemChoice(); + + someIncorrectAnswer.setItems(ImmutableList.of(item1, NULL_PLACEHOLDER)); + someIncorrectAnswer.setCorrect(false); + someIncorrectAnswer.setExplanation(new Content("This is a very bad choice.")); + clozeQuestion.setChoices(ImmutableList.of(someCorrectAnswer, someIncorrectAnswer)); + + // Set up user answer: + ItemChoice c = new ItemChoice(); + c.setItems(ImmutableList.of(item1)); + + // Test response: + QuestionValidationResponse response = validator.validateQuestionResponse(clozeQuestion, c); + assertFalse(response.isCorrect()); + assertTrue(response.getExplanation().getValue().contains("does not contain an item for each gap")); + } + /* Test that answers with too many items are rejected. */ diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacDndValidatorTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacDndValidatorTest.java new file mode 100644 index 0000000000..bb3c163209 --- /dev/null +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacDndValidatorTest.java @@ -0,0 +1,642 @@ +/* + * Copyright 2022 James Sharkey + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package uk.ac.cam.cl.dtg.isaac.quiz; + +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; +import org.junit.experimental.theories.DataPoints; +import org.junit.experimental.theories.Theories; +import org.junit.experimental.theories.Theory; +import org.junit.runner.RunWith; +import uk.ac.cam.cl.dtg.isaac.api.Constants; +import uk.ac.cam.cl.dtg.isaac.api.TestAppender; +import uk.ac.cam.cl.dtg.isaac.dos.DndValidationResponse; +import uk.ac.cam.cl.dtg.isaac.dos.IsaacDndQuestion; +import uk.ac.cam.cl.dtg.isaac.dos.content.Choice; +import uk.ac.cam.cl.dtg.isaac.dos.content.Content; +import uk.ac.cam.cl.dtg.isaac.dos.content.ContentBase; +import uk.ac.cam.cl.dtg.isaac.dos.content.DndChoice; +import uk.ac.cam.cl.dtg.isaac.dos.content.DndItem; +import uk.ac.cam.cl.dtg.isaac.dos.content.Figure; +import uk.ac.cam.cl.dtg.isaac.dos.content.Item; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.function.Supplier; +import java.util.function.UnaryOperator; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static org.junit.Assert.assertNull; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; + +import org.apache.logging.log4j.core.Logger; +import uk.ac.cam.cl.dtg.isaac.dos.content.ParsonsChoice; +import uk.ac.cam.cl.dtg.util.FigureRegion; + +@RunWith(Theories.class) +@SuppressWarnings("checkstyle:MissingJavadocType") +public class IsaacDndValidatorTest { + public static final Item item_3cm = item("6d3d", "3 cm"); + public static final Item item_4cm = item("6d3e", "4 cm"); + public static final Item item_5cm = item("6d3f", "5 cm"); + public static final Item item_6cm = item("6d3g", "5 cm"); + public static final Item item_12cm = item("6d3h", "12 cm"); + public static final Item item_13cm = item("6d3i", "13 cm"); + + @DataPoints + public static CorrectnessTestCase[] correctnessTestCases = { + new CorrectnessTestCase().setTitle("singleCorrectMatch_Correct") + .setQuestion(correct(choose(item_3cm, "leg_1"), choose(item_4cm, "leg_2"), choose(item_5cm, "hypothenuse"))) + .setAnswer(answer(choose(item_3cm, "leg_1"), choose(item_4cm, "leg_2"), choose(item_5cm, "hypothenuse"))) + .expectCorrect(true), + new CorrectnessTestCase().setTitle("singleCorrectNotMatch_Incorrect") + .setQuestion(correct(choose(item_3cm, "leg_1"), choose(item_4cm, "leg_2"), choose(item_5cm, "hypothenuse"))) + .setAnswer(answer(choose(item_4cm, "leg_1"), choose(item_5cm, "leg_2"), choose(item_3cm, "hypothenuse"))) + .expectCorrect(false), + new CorrectnessTestCase().setTitle("singleCorrectPartialMatch_Incorrect") + .setQuestion(correct(choose(item_3cm, "leg_1"), choose(item_4cm, "leg_2"), choose(item_5cm, "hypothenuse"))) + .setAnswer(answer(choose(item_5cm, "leg_1"), choose(item_4cm, "leg_2"), choose(item_3cm, "hypothenuse"))) + .expectCorrect(false), + new CorrectnessTestCase().setTitle("sameAnswerCorrectAndIncorrect_Correct") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(incorrect(choose(item_3cm, "leg_1")), correct(choose(item_3cm, "leg_1"))) + .setAnswer(answer(choose(item_3cm, "leg_1"))) + .expectCorrect(true) + }; + + @Theory + public final void testCorrectness(final CorrectnessTestCase testCase) { + var response = testValidate(testCase.question, testCase.answer); + assertEquals(testCase.correct, response.isCorrect()); + } + + @DataPoints + public static ExplanationTestCase[] explanationTestCases = { + new ExplanationTestCase().setTitle("exactMatchIncorrect_shouldReturnMatching") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion( + correct(choose(item_3cm, "leg_1")), + incorrect(ExplanationTestCase.testFeedback, choose(item_4cm, "leg_1")) + ).setAnswer(answer(choose(item_4cm, "leg_1"))) + .expectCorrect(false), + new ExplanationTestCase().setTitle("exactMatchCorrect_shouldReturnMatching") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correct(ExplanationTestCase.testFeedback, choose(item_3cm, "leg_1"))) + .setAnswer(answer(choose(item_3cm, "leg_1"))) + .expectCorrect(true), + new ExplanationTestCase().setTitle("exactMatchIncorrect_shouldReturnDefaultFeedbackForQuestion") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correct(choose(item_3cm, "leg_1")), incorrect(choose(item_4cm, "leg_1"))) + .tapQuestion(q -> q.setDefaultFeedback(ExplanationTestCase.testFeedback)) + .setAnswer(answer(choose(item_4cm, "leg_1"))) + .expectCorrect(false), + new ExplanationTestCase().setTitle("matchIncorrectSubset_shouldReturnMatching") + .setChildren(List.of(new Content("[drop-zone:leg_1] [drop-zone:leg_2]"))) + .setQuestion( + correct(choose(item_3cm, "leg_1"), choose(item_4cm, "leg_2")), + incorrect(ExplanationTestCase.testFeedback, choose(item_5cm, "leg_1")) + ).setAnswer(answer(choose(item_5cm, "leg_1"), choose(item_6cm, "leg_2"))) + .expectCorrect(false), + new ExplanationTestCase().setTitle("multiMatchIncorrectSubset_shouldReturnMatching") + .setChildren(List.of(new Content("[drop-zone:leg_1] [drop-zone:leg_2]"))) + .setQuestion( + correct(choose(item_3cm, "leg_1"), choose(item_4cm, "leg_2")), + incorrect(new Content("leg_1 can't be 5"), choose(item_5cm, "leg_1")), + incorrect(new Content("leg_2 can't be 6"), choose(item_6cm, "leg_2")) + ).setAnswer(answer(choose(item_5cm, "leg_1"), choose(item_6cm, "leg_2"))) + .expectCorrect(false) + .expectExplanation("leg_1 can't be 5"), + new ExplanationTestCase().setTitle("unMatchedIncorrect_shouldReturnDefaultFeedbackForQuestion") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correct(choose(item_3cm, "leg_1"))) + .tapQuestion(q -> q.setDefaultFeedback(ExplanationTestCase.testFeedback)) + .setAnswer(answer(choose(item_4cm, "leg_1"))) + .expectCorrect(false), + new ExplanationTestCase().setTitle("partialMatchIncorrect_shouldReturnDefaultFeedbackForQuestion") + .setChildren(List.of(new Content("[drop-zone:leg_1] [drop-zone:leg_2]"))) + .setQuestion( + correct(choose(item_3cm, "leg_1"), choose(item_4cm, "leg_2")), + incorrect(new Content("feedback for choice"), choose(item_5cm, "leg_1"), choose(item_6cm, "leg_2")) + ).tapQuestion(q -> q.setDefaultFeedback(new Content("feedback for question"))) + .setAnswer(answer(choose(item_5cm, "leg_1"), choose(item_12cm, "leg_2"))) + .expectCorrect(false) + .expectExplanation("feedback for question"), + new ExplanationTestCase().setTitle("defaultCorrect_shouldReturnNone") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correct(choose(item_3cm, "leg_1"))) + .setAnswer(answer(choose(item_3cm, "leg_1"))) + .expectNoExplanation() + .expectCorrect(true), + new ExplanationTestCase().setTitle("noDefaultIncorrect_shouldReturnNone") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correct(choose(item_3cm, "leg_1"))) + .setAnswer(answer(choose(item_4cm, "leg_1"))) + .expectNoExplanation() + .expectCorrect(false) + }; + + + @Theory + public final void testExplanation(final ExplanationTestCase testCase) { + var response = testValidate(testCase.question, testCase.answer); + assertEquals(response.isCorrect(), testCase.correct); + if (testCase.feedback != null) { + assertEquals(testCase.feedback.getValue(), response.getExplanation().getValue()); + } else { + assertNull(response.getExplanation()); + } + } + + static Supplier disabledItemFeedbackNoDropZones = () -> new DropZonesTestCase() + .tapQuestion(q -> q.setDetailedItemFeedback(false)) + .expectNoDropZones(); + + static Supplier enabledItemFeedback = () -> new DropZonesTestCase() + .tapQuestion(q -> q.setDetailedItemFeedback(true)); + + @DataPoints + public static DropZonesTestCase[] dropZonesCorrectTestCases = { + disabledItemFeedbackNoDropZones.get().setTitle("incorrectNotRequestsed_NotReturned") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correct(choose(item_3cm, "leg_1"))) + .setAnswer(answer(choose(item_4cm, "leg_1"))) + .expectCorrect(false), + disabledItemFeedbackNoDropZones.get().setTitle("correctNotRequested_NotReturned") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correct(choose(item_3cm, "leg_1"))) + .setAnswer(answer(choose(item_3cm, "leg_1"))) + .expectCorrect(true), + enabledItemFeedback.get().setTitle("allCorrect_ShouldReturnAllCorrect") + .setQuestion(correct(choose(item_3cm, "leg_1"), choose(item_4cm, "leg_2"), choose(item_5cm, "hypothenuse"))) + .setAnswer(answer(choose(item_3cm, "leg_1"), choose(item_4cm, "leg_2"), choose(item_5cm, "hypothenuse"))) + .expectCorrect(true) + .expectDropZonesCorrect(d -> d.setLeg1(true).setLeg2(true).setHypothenuse(true)), + enabledItemFeedback.get().setTitle("someIncorrect_ShouldReturnWhetherCorrect") + .setQuestion(correct(choose(item_3cm, "leg_1"), choose(item_4cm, "leg_2"), choose(item_5cm, "hypothenuse"))) + .setAnswer(answer(choose(item_6cm, "leg_1"), choose(item_5cm, "leg_2"), choose(item_5cm, "hypothenuse"))) + .expectCorrect(false) + .expectDropZonesCorrect(d -> d.setLeg1(false).setLeg2(false).setHypothenuse(true)), + enabledItemFeedback.get().setTitle("multipleCorrectAnswers_decidesCorrectnessBasedOnClosesMatch") + .setQuestion( + correct(choose(item_3cm, "leg_1"), choose(item_4cm, "leg_2"), choose(item_5cm, "hypothenuse")), + correct(choose(item_5cm, "leg_1"), choose(item_12cm, "leg_2"), choose(item_13cm, "hypothenuse")) + ).setAnswer(answer(choose(item_5cm, "leg_1"))) + .expectCorrect(false) + .expectDropZonesCorrect(d -> d.setLeg1(true)) + }; + + @Theory + public final void testDropZonesCorrect(final DropZonesTestCase testCase) { + var response = testValidate(testCase.question, testCase.answer); + assertEquals(response.isCorrect(), testCase.correct); + assertEquals(response.getDropZonesCorrect(), testCase.dropZonesCorrect); + } + + @DataPoints + public static AnswerValidationTestCase[] answerValidationTestCases = { + new AnswerValidationTestCase().setTitle("itemsNull") + .setAnswer(answer()) + .expectExplanation(Constants.FEEDBACK_NO_ANSWER_PROVIDED), + new AnswerValidationTestCase().setTitle("itemsEmpty") + .setAnswer(new DndChoice()) + .expectExplanation(Constants.FEEDBACK_NO_ANSWER_PROVIDED), + new AnswerValidationTestCase().setTitle("itemsNotEnough") + .setChildren(List.of(new Content("[drop-zone:leg_1] [drop-zone:leg_2]"))) + .setQuestion(correct(choose(item_3cm, "leg_1"), choose(item_4cm, "leg_2"))) + .setAnswer(answer(choose(item_3cm, "leg_1"))) + .expectExplanation(IsaacDndValidator.FEEDBACK_ANSWER_NOT_ENOUGH) + .expectDropZonesCorrect(f -> f.setLeg1(true)), + new AnswerValidationTestCase().setTitle("itemsTooMany") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correct(choose(item_3cm, "leg_1"))) + .setAnswer(answer(choose(item_3cm, "leg_1"), choose(item_4cm, "leg_1"))) + .expectExplanation(IsaacDndValidator.FEEDBACK_ANSWER_TOO_MUCH), + new AnswerValidationTestCase().setTitle("itemNotOnQuestion") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correct(choose(item_3cm, "leg_1"))) + .setAnswer(answer(choose(new Item("bad_id", "some_value"), "leg_1"))) + .expectExplanation(Constants.FEEDBACK_UNRECOGNISED_ITEMS), + new AnswerValidationTestCase().setTitle("itemMissingId") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correct(choose(item_3cm, "leg_1"))) + .setAnswer(answer(choose(new Item(null, null), "leg_1"))) + .expectExplanation(Constants.FEEDBACK_UNRECOGNISED_FORMAT), + new AnswerValidationTestCase().setTitle("itemMissingDropZoneId") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correct(choose(item_3cm, "leg_1"))) + .setAnswer(answer(choose(item_3cm, null))) + .expectExplanation(Constants.FEEDBACK_UNRECOGNISED_FORMAT), + new AnswerValidationTestCase().setTitle("itemsNotEnough_providesSpecificExplanationFirst") + .setQuestion( + correct(choose(item_3cm, "leg_1"), choose(item_4cm, "leg_2"), choose(item_5cm, "hypothenuse")), + incorrect(new Content("Leg 1 should be less than 4 cm"), choose(item_4cm, "leg_1")) + ).setAnswer(answer(choose(item_4cm, "leg_1"))) + .expectExplanation("Leg 1 should be less than 4 cm") + .expectDropZonesCorrect(f -> f.setLeg1(false)), + new AnswerValidationTestCase().setTitle("unrecognized drop zone") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correct(choose(item_3cm, "leg_1"))) + .setAnswer(answer(choose(item_3cm, "leg_2"))) + .expectExplanation(Constants.FEEDBACK_UNRECOGNISED_ITEMS) + }; + + @Theory + public final void testAnswerValidation(final AnswerValidationTestCase testCase) { + testCase.question.setDetailedItemFeedback(true); + + var response = testValidate(testCase.question, testCase.answer); + + assertFalse(response.isCorrect()); + assertEquals(testCase.feedback.getValue(), response.getExplanation().getValue()); + assertEquals(testCase.dropZonesCorrect, response.getDropZonesCorrect()); + } + + static Supplier itemUnrecognisedFormatCase = () -> new QuestionValidationTestCase() + .expectExplanation(IsaacDndValidator.FEEDBACK_QUESTION_UNRECOGNISED_ANS) + .expectLogMessage(q -> String.format("Found item with missing id or drop zone id in answer for question id (%s)!", q.getId())); + + static Supplier noAnswersTestCase = () -> new QuestionValidationTestCase() + .expectExplanation(Constants.FEEDBACK_NO_CORRECT_ANSWERS) + .expectLogMessage(q -> String.format("Question does not have any answers. %s src: %s", q.getId(), q.getCanonicalSourceFile())); + + static Supplier noCorrectAnswersTestCase = () -> noAnswersTestCase.get() + .expectLogMessage(q -> String.format("Question does not have any correct answers. %s src: %s", q.getId(), q.getCanonicalSourceFile())); + + static Supplier answerEmptyItemsTestCase = () -> new QuestionValidationTestCase() + .expectExplanation(IsaacDndValidator.FEEDBACK_QUESTION_EMPTY_ANS) + .expectLogMessage(q -> String.format("Expected list of DndItems, but none found in choice for question id (%s)!", q.getId())); + + static Supplier questionEmptyAnswersTestCase = () -> new QuestionValidationTestCase() + .expectExplanation(IsaacDndValidator.FEEDBACK_QUESTION_MISSING_ITEMS) + .expectLogMessage(q -> String.format("Expected items in question (%s), but didn't find any!", q.getId())); + + @DataPoints + public static QuestionValidationTestCase[] questionValidationTestCases = { + noAnswersTestCase.get().setTitle("answers empty").setQuestion(q -> q.setChoices(List.of())), + noAnswersTestCase.get().setTitle("answers null").setQuestion(q -> q.setChoices(null)), + noCorrectAnswersTestCase.get().setTitle("only incorrect answers") + .setQuestion(incorrect(choose(item_3cm, "leg_1"))), + noCorrectAnswersTestCase.get().setTitle("answers without explicit correctness are treated as incorrect") + .setQuestion(answer(choose(item_3cm, "leg_1"))), + new QuestionValidationTestCase().setTitle("answer not for a DnD question") + .setQuestion(q -> q.setChoices(List.of(new ParsonsChoice() {{correct = true; setItems(List.of(new Item("", ""))); }}))) + .expectExplanation(IsaacDndValidator.FEEDBACK_QUESTION_INVALID_ANS) + .expectLogMessage(q -> String.format("Expected DndItem in question (%s), instead found class uk.ac.cam.cl.dtg.isaac.quiz.IsaacDndValidatorTest$1!", q.getId())), + answerEmptyItemsTestCase.get().setTitle("answer with empty items").setQuestion(correct()), + answerEmptyItemsTestCase.get().setTitle("answer with null items") + .setQuestion(q -> q.setChoices(Stream.of(new DndChoice()).peek(c -> c.setCorrect(true)).collect(Collectors.toList()))), + new QuestionValidationTestCase().setTitle("answer with non-dnd items") + .setQuestion(correct(new DndItemEx("id", "value", "dropZoneId"))) + .expectExplanation(IsaacDndValidator.FEEDBACK_QUESTION_INVALID_ANS) + .expectLogMessage(q -> String.format("Expected list of DndItems, but something else found in choice for question id (%s)!", q.getId())), + itemUnrecognisedFormatCase.get().setTitle("answer with missing item_id") + .setQuestion(correct(new DndItem(null, "value", "dropZoneId"))), + itemUnrecognisedFormatCase.get().setTitle("answer with empty item_id") + .setQuestion(correct(new DndItem("", "value", "dropZoneId"))), + itemUnrecognisedFormatCase.get().setTitle("answer with missing dropZoneId") + .setQuestion(correct(new DndItem("item_id", "value", null))), + itemUnrecognisedFormatCase.get().setTitle("answer with empty dropZoneId") + .setQuestion(correct(new DndItem("item_id", "value", ""))), + questionEmptyAnswersTestCase.get().setTitle("items is null") + .tapQuestion(q -> q.setItems(null)), + questionEmptyAnswersTestCase.get().setTitle("items is empty") + .tapQuestion(q -> q.setItems(List.of())), + new QuestionValidationTestCase().setTitle("has no drop zones") + .setChildren(null) + .expectExplanation(IsaacDndValidator.FEEDBACK_QUESTION_NO_DZ) + .expectLogMessage(q -> String.format("Question does not have any drop zones. %s src %s", q.getId(), q.getCanonicalSourceFile())), + new QuestionValidationTestCase().setTitle("has id duplication among drop zones") + .setChildren(List.of(new Content("[drop-zone:A1] [drop-zone:A1]"))) + .expectExplanation(IsaacDndValidator.FEEDBACK_QUESTION_DUP_DZ) + .expectLogMessage(q -> String.format("Question contains duplicate drop zones. %s src %s", q.getId(), q.getCanonicalSourceFile())), + new QuestionValidationTestCase().setTitle("correct answers contain a choice for each drop zone") + .setChildren(List.of(new Content("[drop-zone:leg_1] [drop-zone:leg_2]"))) + .setQuestion( + correct(choose(item_3cm, "leg_1"), choose(item_4cm, "leg_2")), + correct(choose(item_3cm, "leg_1")) + ).expectExplanation(IsaacDndValidator.FEEDBACK_QUESTION_UNUSED_DZ) + .expectLogMessage(q -> String.format("Question contains correct answer that doesn't use all drop zones. %s src %s", q.getId(), q.getCanonicalSourceFile())), + new QuestionValidationTestCase().setTitle("drop zone references must be valid") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correct(choose(item_3cm, "leg_1")), incorrect(choose(item_3cm, "leg_2"))) + .expectExplanation(IsaacDndValidator.FEEDBACK_QUESTION_INVALID_DZ) + .expectLogMessage(q -> String.format("Question contains invalid drop zone reference. %s src %s", q.getId(), q.getCanonicalSourceFile())) + }; + + @Theory + public final void testQuestionValidation(final QuestionValidationTestCase testCase) { + testCase.question.setDetailedItemFeedback(true); + + var response = testValidate(testCase.question, testCase.answer); + assertFalse(response.isCorrect()); + assertEquals(testCase.feedback.getValue(), response.getExplanation().getValue()); + assertEquals(testCase.dropZonesCorrect, response.getDropZonesCorrect()); + + var appender = testValidateWithLogs(testCase.question, testCase.answer); + appender.assertLevel(Level.ERROR); + appender.assertMessage(testCase.loggedMessage); + } + + static Supplier invalidDropZone = () -> new GetDropZonesTestCase() + .expectDropZones(); + + @DataPoints + public static GetDropZonesTestCase[] getDropZonesTestCases = { + invalidDropZone.get().setChildren(List.of(new Content(""))), + invalidDropZone.get().setChildren(List.of(new Content("no drop zone"))), + invalidDropZone.get().setChildren(List.of(new Content("[drop-zone A1]"))), + invalidDropZone.get().setChildren(List.of(new Content("[drop-zone: A1]"))), + invalidDropZone.get().setChildren(List.of(new Content("[drop-zone:A1 | w-100]"))), + invalidDropZone.get().setChildren(List.of(new Content("[drop-zone:A1|w-100 h-50]"))), + invalidDropZone.get().setChildren(List.of(new Content("[drop-zone:A1|h-100w-50]"))), + new GetDropZonesTestCase().setTitle("noContent_noDropZones").setChildren(null).expectDropZones(), + new GetDropZonesTestCase().setTitle("singleDropZoneSingleText_returnsDropZone") + .setChildren(List.of(new Content("[drop-zone:A1]"))) + .expectDropZones("A1"), + new GetDropZonesTestCase().setTitle("singleDropZoneSingleContent_returnsDropZone") + .setChildren(List.of(new Content("[drop-zone:A1|w-100]"))) + .expectDropZones("A1"), + new GetDropZonesTestCase().setTitle("singleDropZoneSingleContentHeight_returnsDropZone") + .setChildren(List.of(new Content("[drop-zone:A1|h-100]"))) + .expectDropZones("A1"), + new GetDropZonesTestCase().setTitle("singleDropZoneSingleContentWidthHeight_returnsDropZone") + .setChildren(List.of(new Content("[drop-zone:A1|w-100h-50]"))) + .expectDropZones("A1"), + new GetDropZonesTestCase().setTitle("multiDropZoneSingleContent_returnsDropZones") + .setChildren(List.of(new Content("Some text [drop-zone:A1], other text [drop-zone:A2]"))) + .expectDropZones("A1", "A2"), + new GetDropZonesTestCase().setTitle("multiDropZoneMultiContent_returnsDropZones") + .setChildren(List.of( + new Content("[drop-zone:A1] [drop-zone:A2]"), + new Content("[drop-zone:A3] [drop-zone:A4]") + )).expectDropZones("A1", "A2", "A3", "A4"), + new GetDropZonesTestCase().setTitle("singleDropZoneNestedContent_returnsDropZones") + .setChildren(new LinkedList<>(List.of(new Content(), new Content("[drop-zone:A2]")))) + .tapQuestion(q -> ((Content) q.getChildren().get(0)).setChildren(List.of(new Content("[drop-zone:A1]")))) + .expectDropZones("A1", "A2"), + new GetDropZonesTestCase().setTitle("figureContentWithoutDropZones_returnsNoZones") + .setChildren(List.of(new Figure())) + .expectDropZones(), + new GetDropZonesTestCase().setTitle("figureContent_returnsDropZones") + .setChildren(List.of(createFigure("A1", "A2"))) + .expectDropZones("A1", "A2"), + new GetDropZonesTestCase().setTitle("mixedButNoNesting_returnsDropZones") + .setChildren(new LinkedList<>(List.of(createFigure("A1", "A2"), new Content("[drop-zone:A3]")))) + .expectDropZones("A1", "A2", "A3"), + new GetDropZonesTestCase().setTitle("mixedNested_returnsDropZones") + .setChildren(new LinkedList<>(List.of(new Content(), new Content("[drop-zone:A2]")))) + .tapQuestion(q -> { + Content content = (Content) q.getChildren().get(0); + content.setChildren(List.of( + new Content("[drop-zone:A1]"), + createFigure("F1", "F2") + )); + }).expectDropZones("A1", "F1", "F2", "A2") + }; + + @Theory + public final void testGetDropZones(final GetDropZonesTestCase testCase) { + var dropZones = IsaacDndValidator.QuestionHelpers.getDropZonesFromContent(testCase.question); + assertEquals(testCase.dropZones, dropZones); + } + + private static DndValidationResponse testValidate(final IsaacDndQuestion question, final Choice choice) { + return new IsaacDndValidator().validateQuestionResponse(question, choice); + } + + private static TestAppender testValidateWithLogs(final IsaacDndQuestion question, final Choice choice) { + var appender = new TestAppender(); + Logger logger = (Logger) LogManager.getLogger(IsaacDndValidator.class); + logger.addAppender(appender); + logger.setLevel(Level.WARN); + + try { + testValidate(question, choice); + return appender; + } finally { + logger.removeAppender(new TestAppender()); + } + } + + @SuppressWarnings("checkstyle:MissingJavadocMethod") + public static DndChoice answer(final DndItem... list) { + var c = new DndChoice(); + c.setItems(List.of(list)); + c.setType("dndChoice"); + return c; + } + + @SuppressWarnings("checkstyle:MissingJavadocMethod") + public static DndItem choose(final Item item, final String dropZoneId) { + var value = new DndItem(item.getId(), item.getValue(), dropZoneId); + value.setType("dndItem"); + return value; + } + + @SuppressWarnings("checkstyle:MissingJavadocMethod") + public static IsaacDndQuestion createQuestion(final DndChoice... answers) { + var question = new IsaacDndQuestion(); + question.setId(UUID.randomUUID().toString()); + question.setItems(List.of(item_3cm, item_4cm, item_5cm, item_6cm, item_12cm, item_13cm)); + question.setChoices(List.of(answers)); + question.setType("isaacDndQuestion"); + question.setChildren(List.of(new Content("[drop-zone:leg_1] [drop-zone:leg_2] [drop-zone:hypothenuse]"))); + return question; + } + + @SuppressWarnings("checkstyle:MissingJavadocMethod") + public static DndChoice correct(final DndItem... list) { + var choice = answer(list); + choice.setCorrect(true); + return choice; + } + + @SuppressWarnings("checkstyle:MissingJavadocMethod") + public static DndChoice correct(final ContentBase explanation, final DndItem... list) { + var choice = correct(list); + choice.setExplanation(explanation); + return choice; + } + + private static DndChoice incorrect(final DndItem... list) { + var choice = answer(list); + choice.setCorrect(false); + return choice; + } + + private static DndChoice incorrect(final ContentBase explanation, final DndItem... list) { + var choice = incorrect(list); + choice.setExplanation(explanation); + return choice; + } + + private static Item item(final String id, final String value) { + Item item = new Item(id, value); + item.setType("item"); + return item; + } + + public static class DropZonesCorrectFactory { + private final Map map = new HashMap<>(); + + public DropZonesCorrectFactory setLeg1(final Boolean value) { + map.put("leg_1", value); + return this; + } + + public DropZonesCorrectFactory setLeg2(final boolean value) { + map.put("leg_2", value); + return this; + } + + public DropZonesCorrectFactory setHypothenuse(final boolean value) { + map.put("hypothenuse", value); + return this; + } + + public Map build() { + return map; + } + } + + private static Figure createFigure(final String... dropZones) { + var figure = new Figure(); + figure.setFigureRegions(new ArrayList<>(List.of())); + List.of(dropZones).forEach(dropZoneId -> { + var region = new FigureRegion(); + region.setId(dropZoneId); + figure.getFigureRegions().add(region); + }); + return figure; + } + + static class TestCase> { + public static Content testFeedback = new Content("some test feedback"); + + public IsaacDndQuestion question = createQuestion( + correct(choose(item_3cm, "leg_1"), choose(item_4cm, "leg_2"), choose(item_5cm, "hypothenuse")) + ); + public DndChoice answer = answer(); + public Content feedback = testFeedback; + public Map dropZonesCorrect; + public List dropZones; + public String loggedMessage; + public boolean correct = false; + private Function logMessageOp; + private final List> questionOps = new ArrayList<>(); + + public T setTitle(final String title) { + return self(); + } + + public T setQuestion(final DndChoice... choices) { + this.question = createQuestion(choices); + return self(); + } + + public T setQuestion(final Consumer op) { + var question = createQuestion(); + op.accept(question); + this.question = question; + return self(); + } + + public T setChildren(final List content) { + this.questionOps.add(q -> q.setChildren(content)); + return self(); + } + + public T tapQuestion(final Consumer op) { + this.questionOps.add(op); + return self(); + } + + public T setAnswer(final DndChoice answer) { + this.answer = answer; + return self(); + } + + public T expectCorrect(final boolean correct) { + this.correct = correct; + return self(); + } + + public T expectExplanation(final String feedback) { + this.feedback = new Content(feedback); + return self(); + } + + public T expectNoExplanation() { + this.feedback = null; + return self(); + } + + public T expectDropZonesCorrect(final UnaryOperator op) { + this.dropZonesCorrect = op.apply(new DropZonesCorrectFactory()).build(); + return self(); + } + + public T expectNoDropZones() { + this.dropZonesCorrect = null; + return self(); + } + + public T expectLogMessage(final Function op) { + this.logMessageOp = op; + return self(); + } + + public T expectDropZones(final String... dropZones) { + this.dropZones = List.of(dropZones); + return self(); + } + + private T self() { + this.questionOps.forEach(op -> op.accept(this.question)); + if (this.logMessageOp != null) { + this.loggedMessage = logMessageOp.apply(this.question); + } + return (T) this; + } + } + + public static class AnswerValidationTestCase extends TestCase {} + + public static class QuestionValidationTestCase extends TestCase {} + + public static class CorrectnessTestCase extends TestCase {} + + public static class ExplanationTestCase extends TestCase {} + + public static class DropZonesTestCase extends TestCase {} + + public static class GetDropZonesTestCase extends TestCase {} + + public static class DndItemEx extends DndItem { + public DndItemEx(final String id, final String value, final String dropZoneId) { + super(id, value, dropZoneId); + } + } + +} diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/etl/ContentIndexerTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/etl/ContentIndexerTest.java index ff8912e50c..90887e5524 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/etl/ContentIndexerTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/etl/ContentIndexerTest.java @@ -19,6 +19,7 @@ import static org.junit.Assert.*; import java.util.*; +import java.util.stream.Collectors; import com.google.api.client.util.Maps; import com.google.api.client.util.Sets; @@ -31,6 +32,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import uk.ac.cam.cl.dtg.isaac.dos.IsaacNumericQuestion; +import uk.ac.cam.cl.dtg.isaac.quiz.IsaacDndValidatorTest; import uk.ac.cam.cl.dtg.segue.api.Constants; import uk.ac.cam.cl.dtg.segue.dao.content.ContentSubclassMapper; import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; @@ -398,6 +400,62 @@ public void recordContentTypeSpecificError_disregardSigFigsSet_checkNoError() } } + @Test + public void recordContentTypeSpecificError_dndQuestionCorrect_checkNoError() throws Exception { + // ARRANGE + final Map> indexProblemCache = new HashMap<>(); + final List contents = new LinkedList<>(); + var dndQuestion = IsaacDndValidatorTest.createQuestion( + IsaacDndValidatorTest.correct(IsaacDndValidatorTest.choose(IsaacDndValidatorTest.item_3cm, "leg_1")) + ); + dndQuestion.setChildren(List.of(new Content("[drop-zone:leg_1]"))); + contents.add(dndQuestion); + + // ACT + for (Content content : contents) { + Whitebox.invokeMethod( + defaultContentIndexer, + "recordContentTypeSpecificError", + "", + content, + indexProblemCache + ); + } + + // ASSERT + assertEquals(0, indexProblemCache.size()); + } + + @Test + public void recordContentTypeSpecificError_dndQuestionInCorrect_checkErrorIsCorrect() throws Exception { + // ARRANGE + final Map> indexProblemCache = new HashMap<>(); + final List contents = new LinkedList<>(); + var dndQuestion = IsaacDndValidatorTest.createQuestion( + IsaacDndValidatorTest.correct(IsaacDndValidatorTest.choose(IsaacDndValidatorTest.item_3cm, "leg_1")) + ); + dndQuestion.setChildren(null); + dndQuestion.setCanonicalSourceFile(""); + contents.add(dndQuestion); + + // ACT + for (Content content : contents) { + Whitebox.invokeMethod( + defaultContentIndexer, + "recordContentTypeSpecificError", + "", + content, + indexProblemCache + ); + } + + // ASSERT + Collection> expected = List.of(List.of( + String.format("Drag-and-drop Question: %s has a problem. This question doesn't have any drop zones.", + dndQuestion.getId()))); + assertEquals(expected, List.copyOf(indexProblemCache.values())); + } + private Content createContentHierarchy(final int numLevels, final Set flatSet) { List children = new LinkedList();