From b7717ed2de03c5dc2335acdfc2e70d93820f07ec Mon Sep 17 00:00:00 2001 From: anhefti Date: Tue, 24 May 2022 16:05:32 +0200 Subject: [PATCH] SEBSERV-57 fixed --- .../sebserver/gbl/model/exam/Indicator.java | 1 + .../sebserver/gui/widget/ThresholdList.java | 2 +- .../servicelayer/exam/ExamAdminService.java | 46 +++++++++++++- .../weblayer/api/ExamTemplateController.java | 9 +++ .../weblayer/api/IndicatorController.java | 60 ++++--------------- 5 files changed, 67 insertions(+), 51 deletions(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/gbl/model/exam/Indicator.java b/src/main/java/ch/ethz/seb/sebserver/gbl/model/exam/Indicator.java index 2f15b2f2..21d119de 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gbl/model/exam/Indicator.java +++ b/src/main/java/ch/ethz/seb/sebserver/gbl/model/exam/Indicator.java @@ -240,6 +240,7 @@ public final class Indicator implements Entity { } } + @JsonIgnoreProperties(ignoreUnknown = true) public static final class Threshold implements Comparable { @JsonProperty(THRESHOLD.ATTR_VALUE) diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/widget/ThresholdList.java b/src/main/java/ch/ethz/seb/sebserver/gui/widget/ThresholdList.java index 57572f61..dcdfc08f 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/widget/ThresholdList.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/widget/ThresholdList.java @@ -131,7 +131,7 @@ public final class ThresholdList extends Composite { private void removeInvalidListEntries() { this.thresholds .stream() - .filter(entry -> entry.getValue() == null || StringUtils.isBlank(entry.getColor())) + .filter(entry -> entry.getValue() == null && StringUtils.isBlank(entry.getColor())) .collect(Collectors.toList()) .forEach(this::removeThreshold); } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/ExamAdminService.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/ExamAdminService.java index 61f9c246..347d383a 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/ExamAdminService.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/ExamAdminService.java @@ -8,9 +8,18 @@ package ch.ethz.seb.sebserver.webservice.servicelayer.exam; -import org.apache.commons.lang3.BooleanUtils; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; +import org.apache.commons.lang3.BooleanUtils; +import org.springframework.validation.FieldError; + +import ch.ethz.seb.sebserver.gbl.api.APIMessage; +import ch.ethz.seb.sebserver.gbl.api.APIMessage.APIMessageException; +import ch.ethz.seb.sebserver.gbl.model.Domain; import ch.ethz.seb.sebserver.gbl.model.exam.Exam; +import ch.ethz.seb.sebserver.gbl.model.exam.Indicator.Threshold; import ch.ethz.seb.sebserver.gbl.model.exam.ProctoringServiceSettings; import ch.ethz.seb.sebserver.gbl.util.Result; import ch.ethz.seb.sebserver.webservice.servicelayer.session.ExamProctoringService; @@ -88,4 +97,39 @@ public interface ExamAdminService { * @return ExamProctoringService instance */ Result getExamProctoringService(final Long examId); + /** Used to check threshold consistency for a given list of thresholds. + * Checks if all values are present (none null value) + * Checks if there are duplicates + * + * If a check fails, the methods throws a APIMessageException with a FieldError to notify the caller + * + * @param thresholds List of Threshold */ + public static void checkThresholdConsistency(final List thresholds) { + if (thresholds != null) { + final List emptyThresholds = thresholds.stream() + .filter(t -> t.getValue() == null || t.getColor() == null) + .collect(Collectors.toList()); + + if (!emptyThresholds.isEmpty()) { + throw new APIMessageException(APIMessage.fieldValidationError( + new FieldError( + Domain.EXAM.TYPE_NAME, + Domain.EXAM.ATTR_SUPPORTER, + "indicator:thresholds:thresholdEmpty"))); + } + + final Set values = thresholds.stream() + .map(t -> t.getValue()) + .collect(Collectors.toSet()); + + if (values.size() != thresholds.size()) { + throw new APIMessageException(APIMessage.fieldValidationError( + new FieldError( + Domain.EXAM.TYPE_NAME, + Domain.EXAM.ATTR_SUPPORTER, + "indicator:thresholds:thresholdDuplicate"))); + } + } + } + } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamTemplateController.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamTemplateController.java index 221d23db..3e9b237d 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamTemplateController.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamTemplateController.java @@ -49,6 +49,7 @@ import ch.ethz.seb.sebserver.webservice.servicelayer.bulkaction.BulkActionServic import ch.ethz.seb.sebserver.webservice.servicelayer.dao.ExamTemplateDAO; import ch.ethz.seb.sebserver.webservice.servicelayer.dao.ResourceNotFoundException; import ch.ethz.seb.sebserver.webservice.servicelayer.dao.UserActivityLogDAO; +import ch.ethz.seb.sebserver.webservice.servicelayer.exam.ExamAdminService; import ch.ethz.seb.sebserver.webservice.servicelayer.exam.ProctoringAdminService; import ch.ethz.seb.sebserver.webservice.servicelayer.validation.BeanValidationService; @@ -191,6 +192,10 @@ public class ExamTemplateController extends EntityController { + ExamAdminService.checkThresholdConsistency(indicator.thresholds); + return indicator; + }) .flatMap(this.examTemplateDAO::createNewIndicatorTemplate) .flatMap(this.userActivityLogDAO::logCreate) .getOrThrow(); @@ -212,6 +217,10 @@ public class ExamTemplateController extends EntityController { + ExamAdminService.checkThresholdConsistency(indicator.thresholds); + return indicator; + }) .flatMap(this.examTemplateDAO::saveIndicatorTemplate) .flatMap(this.userActivityLogDAO::logModify) .getOrThrow(); diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/IndicatorController.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/IndicatorController.java index c5e4d833..387a7f3d 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/IndicatorController.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/IndicatorController.java @@ -8,25 +8,17 @@ package ch.ethz.seb.sebserver.webservice.weblayer.api; -import java.util.List; -import java.util.Set; -import java.util.stream.Collectors; - import org.mybatis.dynamic.sql.SqlTable; -import org.springframework.validation.FieldError; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; import ch.ethz.seb.sebserver.gbl.api.API; -import ch.ethz.seb.sebserver.gbl.api.APIMessage; -import ch.ethz.seb.sebserver.gbl.api.APIMessage.APIMessageException; import ch.ethz.seb.sebserver.gbl.api.EntityType; import ch.ethz.seb.sebserver.gbl.api.POSTMapper; import ch.ethz.seb.sebserver.gbl.model.Domain; import ch.ethz.seb.sebserver.gbl.model.EntityProcessingReport; import ch.ethz.seb.sebserver.gbl.model.GrantEntity; import ch.ethz.seb.sebserver.gbl.model.exam.Indicator; -import ch.ethz.seb.sebserver.gbl.model.exam.Indicator.Threshold; import ch.ethz.seb.sebserver.gbl.profile.WebServiceProfile; import ch.ethz.seb.sebserver.gbl.util.Pair; import ch.ethz.seb.sebserver.gbl.util.Result; @@ -37,6 +29,7 @@ import ch.ethz.seb.sebserver.webservice.servicelayer.bulkaction.BulkActionServic import ch.ethz.seb.sebserver.webservice.servicelayer.dao.ExamDAO; import ch.ethz.seb.sebserver.webservice.servicelayer.dao.IndicatorDAO; import ch.ethz.seb.sebserver.webservice.servicelayer.dao.UserActivityLogDAO; +import ch.ethz.seb.sebserver.webservice.servicelayer.exam.ExamAdminService; import ch.ethz.seb.sebserver.webservice.servicelayer.session.ExamSessionService; import ch.ethz.seb.sebserver.webservice.servicelayer.validation.BeanValidationService; @@ -95,22 +88,20 @@ public class IndicatorController extends EntityController @Override protected Result validForCreate(final Indicator entity) { - final Result validForCreate = super.validForCreate(entity); - if (validForCreate.hasError()) { - return validForCreate; - } - - return checkThresholdConsistency(entity); + return super.validForCreate(entity) + .map(indicator -> { + ExamAdminService.checkThresholdConsistency(indicator.thresholds); + return indicator; + }); } @Override protected Result validForSave(final Indicator entity) { - final Result validForSave = super.validForSave(entity); - if (validForSave.hasError()) { - return validForSave; - } - - return checkThresholdConsistency(entity); + return super.validForSave(entity) + .map(indicator -> { + ExamAdminService.checkThresholdConsistency(indicator.thresholds); + return indicator; + }); } @Override @@ -156,33 +147,4 @@ public class IndicatorController extends EntityController } - private Result checkThresholdConsistency(final Indicator entity) { - if (entity != null) { - final List emptyThresholds = entity.thresholds.stream() - .filter(t -> t.getValue() == null) - .collect(Collectors.toList()); - - if (!emptyThresholds.isEmpty()) { - throw new APIMessageException(APIMessage.fieldValidationError( - new FieldError( - Domain.EXAM.TYPE_NAME, - Domain.EXAM.ATTR_SUPPORTER, - "indicator:thresholds:thresholdEmpty"))); - } - - final Set values = entity.thresholds.stream() - .map(t -> t.getValue()) - .collect(Collectors.toSet()); - - if (values.size() != entity.thresholds.size()) { - throw new APIMessageException(APIMessage.fieldValidationError( - new FieldError( - Domain.EXAM.TYPE_NAME, - Domain.EXAM.ATTR_SUPPORTER, - "indicator:thresholds:thresholdDuplicate"))); - } - } - return Result.of(entity); - } - }