From fa0715b673fd33d54fc30b4de41b9363f81e9a47 Mon Sep 17 00:00:00 2001 From: anhefti Date: Tue, 21 Sep 2021 14:38:45 +0200 Subject: [PATCH] SEBSERV-162 fixes and better error handling --- .../seb/sebserver/gbl/api/APIMessage.java | 22 ++++++- .../sebserver/gui/content/exam/ExamForm.java | 61 +++++++++++++++---- .../seb/sebserver/gui/form/FormHandle.java | 5 ++ .../gui/service/page/impl/PageAction.java | 2 +- .../remote/webservice/api/RestCallError.java | 6 ++ .../exam/ExamTemplateService.java | 6 ++ .../exam/impl/ExamTemplateServiceImpl.java | 57 +++++++++++------ .../indicator/AbstractClientIndicator.java | 8 ++- .../config/application-ws.properties | 7 +++ src/main/resources/messages.properties | 3 +- .../resources/application-test.properties | 7 ++- 11 files changed, 148 insertions(+), 36 deletions(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/gbl/api/APIMessage.java b/src/main/java/ch/ethz/seb/sebserver/gbl/api/APIMessage.java index 9c4bafb2..1fd2e9ec 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gbl/api/APIMessage.java +++ b/src/main/java/ch/ethz/seb/sebserver/gbl/api/APIMessage.java @@ -43,6 +43,7 @@ public class APIMessage implements Serializable { RESOURCE_NOT_FOUND("1002", HttpStatus.NOT_FOUND, "resource not found"), ILLEGAL_API_ARGUMENT("1010", HttpStatus.BAD_REQUEST, "Illegal API request argument"), UNEXPECTED("1100", HttpStatus.INTERNAL_SERVER_ERROR, "Unexpected internal server-side error"), + FIELD_VALIDATION("1200", HttpStatus.BAD_REQUEST, "Field validation error"), INTEGRITY_VALIDATION("1201", HttpStatus.BAD_REQUEST, "Action would lied to an integrity violation"), PASSWORD_MISMATCH("1300", HttpStatus.BAD_REQUEST, "new password do not match confirmed password"), @@ -57,7 +58,12 @@ public class APIMessage implements Serializable { EXAM_CONSISTENCY_VALIDATION_INDICATOR("1403", HttpStatus.OK, "No Indicator defined for the Exam"), EXAM_CONSISTENCY_VALIDATION_LMS_CONNECTION("1404", HttpStatus.OK, "No Connection To LMS"), EXAM_CONSISTENCY_VALIDATION_INVALID_ID_REFERENCE("1405", HttpStatus.OK, - "There seems to be an invalid exam - course identifier reference. The course cannot be found"); + "There seems to be an invalid exam - course identifier reference. The course cannot be found"), + + EXAM_IMPORT_ERROR_AUTO_CONFIG("1500", HttpStatus.BAD_REQUEST, + "Failed to automatically create and link exam configuration from exam template"), + EXAM_IMPORT_ERROR_AUTO_CONFIG_LINKING("1500", HttpStatus.BAD_REQUEST, + "Failed to automatically link auto-generated exam configuration"); public final String messageCode; public final HttpStatus httpStatus; @@ -246,6 +252,20 @@ public class APIMessage implements Serializable { this.apiMessages = Arrays.asList(errorMessage.of(detail, attributes)); } + public APIMessageException(final ErrorMessage errorMessage, final Exception errorCause) { + super(errorMessage.systemMessage); + this.apiMessages = Arrays.asList(errorMessage.of(errorCause)); + } + + public APIMessageException( + final ErrorMessage errorMessage, + final Exception errorCause, + final String... attributes) { + + super(errorMessage.systemMessage); + this.apiMessages = Arrays.asList(errorMessage.of(errorCause, attributes)); + } + @Override public Collection getAPIMessages() { return this.apiMessages; diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/content/exam/ExamForm.java b/src/main/java/ch/ethz/seb/sebserver/gui/content/exam/ExamForm.java index 382e0632..152b1e72 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/content/exam/ExamForm.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/content/exam/ExamForm.java @@ -11,6 +11,7 @@ package ch.ethz.seb.sebserver.gui.content.exam; import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.function.BooleanSupplier; @@ -45,6 +46,7 @@ import ch.ethz.seb.sebserver.gui.content.action.ActionDefinition; import ch.ethz.seb.sebserver.gui.form.Form; import ch.ethz.seb.sebserver.gui.form.FormBuilder; import ch.ethz.seb.sebserver.gui.form.FormHandle; +import ch.ethz.seb.sebserver.gui.form.FormPostException; import ch.ethz.seb.sebserver.gui.service.ResourceService; import ch.ethz.seb.sebserver.gui.service.i18n.I18nSupport; import ch.ethz.seb.sebserver.gui.service.i18n.LocTextKey; @@ -56,6 +58,7 @@ import ch.ethz.seb.sebserver.gui.service.page.TemplateComposer; import ch.ethz.seb.sebserver.gui.service.page.event.ActionEvent; import ch.ethz.seb.sebserver.gui.service.page.impl.PageAction; import ch.ethz.seb.sebserver.gui.service.remote.download.DownloadService; +import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.RestCallError; import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.RestService; import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.exam.CheckExamConsistency; import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.exam.CheckSEBRestriction; @@ -127,6 +130,11 @@ public class ExamForm implements TemplateComposer { private final static LocTextKey CONSISTENCY_MESSAGEINVALID_ID_REFERENCE = new LocTextKey("sebserver.exam.consistency.invalid-lms-id"); + private final static LocTextKey AUTO_GEN_CONFIG_ERROR_TITLE = + new LocTextKey("sebserver.exam.autogen.error.config.title"); + private final static LocTextKey AUTO_GEN_CONFIG_ERROR_TEXT = + new LocTextKey("sebserver.exam.autogen.error.config.text"); + private final Map consistencyMessageMapping; private final PageService pageService; private final ResourceService resourceService; @@ -493,19 +501,50 @@ public class ExamForm implements TemplateComposer { final FormHandle formHandle, final boolean applySEBRestriction) { - // process normal save first - final PageAction processFormSave = formHandle.processFormSave(action); + try { + // process normal save first + final PageAction processFormSave = formHandle.processFormSave(action); - // when okay and the exam sebRestriction is true - if (applySEBRestriction) { - this.examSEBRestrictionSettings.setSEBRestriction( - processFormSave, - true, - this.restService, - t -> log.error("Failed to initially restrict the course for SEB on LMS: {}", t.getMessage())); + // when okay and the exam sebRestriction is true + if (applySEBRestriction) { + this.examSEBRestrictionSettings.setSEBRestriction( + processFormSave, + true, + this.restService, + t -> log.error("Failed to initially restrict the course for SEB on LMS: {}", t.getMessage())); + } + + return processFormSave; + + } catch (final Exception e) { + // try to geht the created exam id + Throwable error = e; + if (e instanceof FormPostException) { + error = ((FormPostException) e).getCause(); + } + if (error instanceof RestCallError) { + final List apiMessages = ((RestCallError) error).getAPIMessages(); + if (apiMessages != null && !apiMessages.isEmpty()) { + final APIMessage apiMessage = apiMessages.get(0); + final String examIdAttr = apiMessage.attributes + .stream() + .filter(attr -> attr.startsWith(API.PARAM_MODEL_ID)) + .findFirst().orElse(null); + if (examIdAttr != null) { + final String[] split = StringUtils.split( + examIdAttr, + Constants.FORM_URL_ENCODED_NAME_VALUE_SEPARATOR); + if (API.PARAM_MODEL_ID.equals(split[0])) { + action.pageContext().publishPageMessage( + AUTO_GEN_CONFIG_ERROR_TITLE, + AUTO_GEN_CONFIG_ERROR_TEXT); + return action.withEntityKey(new EntityKey(split[1], EntityType.EXAM)); + } + } + } + } + throw e; } - - return processFormSave; } private boolean testSEBRestrictionAPI(final Exam exam) { diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/form/FormHandle.java b/src/main/java/ch/ethz/seb/sebserver/gui/form/FormHandle.java index 3b27f860..3cc5a147 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/form/FormHandle.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/form/FormHandle.java @@ -177,6 +177,11 @@ public class FormHandle { private void handleUnexpectedError(final Exception error) { try { + + if (error instanceof RestCallError && !((RestCallError) error).isUnexpectedError()) { + return; + } + if (error instanceof TooManyRequests) { final TooManyRequests.Code code = ((TooManyRequests) error).code; if (code != null) { diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/page/impl/PageAction.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/page/impl/PageAction.java index afc90d83..5535c524 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/page/impl/PageAction.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/page/impl/PageAction.java @@ -198,7 +198,7 @@ public final class PageAction { PageAction.this.pageContext.publishPageMessage(pme); return Result.ofError(pme); } catch (final RestCallError restCallError) { - if (!restCallError.isFieldValidationError()) { + if (restCallError.isUnexpectedError()) { log.error("Failed to execute action: {} | error: {} | cause: {}", PageAction.this.getName(), restCallError.getMessage(), diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/remote/webservice/api/RestCallError.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/remote/webservice/api/RestCallError.java index 1ed226ce..076f70c6 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/remote/webservice/api/RestCallError.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/remote/webservice/api/RestCallError.java @@ -52,6 +52,12 @@ public class RestCallError extends RuntimeException implements APIMessageError { .anyMatch(APIMessage.ErrorMessage.FIELD_VALIDATION::isOf); } + public boolean isUnexpectedError() { + return this.errors + .stream() + .anyMatch(error -> Integer.valueOf(error.messageCode) < 1200); + } + @Override public String toString() { return "RestCallError [errors=" + this.errors + "]"; diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/ExamTemplateService.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/ExamTemplateService.java index 41f45fdf..106e8d83 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/ExamTemplateService.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/ExamTemplateService.java @@ -13,12 +13,18 @@ import ch.ethz.seb.sebserver.gbl.util.Result; public interface ExamTemplateService { + /** Exam start-date placeholder for autogenerated exam configurations name and description texts */ String VAR_START_DATE = "__startDate__"; + /** Exam current-date placeholder for autogenerated exam configurations name and description texts */ String VAR_CURRENT_DATE = "__currentDate__"; + /** Exam exam-name placeholder for autogenerated exam configurations name and description texts */ String VAR_EXAM_NAME = "__examName__"; + /** Exam exam-template-name placeholder for autogenerated exam configurations name and description texts */ String VAR_EXAM_TEMPLATE_NAME = "__examTemplateName__"; + /** Default name text for autogenerated exam configurations */ String DEFAULT_EXAM_CONFIG_NAME_TEMPLATE = VAR_START_DATE + " " + VAR_EXAM_NAME; + /** Default description text for autogenerated exam configurations */ String DEFAULT_EXAM_CONFIG_DESC_TEMPLATE = "This has automatically been created from the exam template: " + VAR_EXAM_TEMPLATE_NAME + " at: " diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/impl/ExamTemplateServiceImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/impl/ExamTemplateServiceImpl.java index 57fdadf1..08c9b90b 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/impl/ExamTemplateServiceImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/impl/ExamTemplateServiceImpl.java @@ -24,6 +24,9 @@ import org.springframework.stereotype.Service; import com.fasterxml.jackson.core.type.TypeReference; import ch.ethz.seb.sebserver.gbl.Constants; +import ch.ethz.seb.sebserver.gbl.api.API; +import ch.ethz.seb.sebserver.gbl.api.APIMessage.APIMessageException; +import ch.ethz.seb.sebserver.gbl.api.APIMessage.ErrorMessage; import ch.ethz.seb.sebserver.gbl.api.EntityType; import ch.ethz.seb.sebserver.gbl.api.JSONMapper; import ch.ethz.seb.sebserver.gbl.model.exam.Exam; @@ -77,11 +80,11 @@ public class ExamTemplateServiceImpl implements ExamTemplateService { final IndicatorDAO indicatorDAO, final JSONMapper jsonMapper, - @Value("${sebserver.webservice.api.exam.indicator.name:Ping}") final String defaultIndicatorName, - @Value("${sebserver.webservice.api.exam.indicator.type:LAST_PING}") final String defaultIndicatorType, - @Value("${sebserver.webservice.api.exam.indicator.color:b4b4b4}") final String defaultIndicatorColor, - @Value("${sebserver.webservice.api.exam.indicator.thresholds:[{\"value\":2000.0,\"color\":\"22b14c\"},{\"value\":5000.0,\"color\":\"ff7e00\"},{\"value\":10000.0,\"color\":\"ed1c24\"}]}") final String defaultIndicatorThresholds, - @Value("${sebserver.webservice.configtemplate.examconfig.default.name:") final String defaultExamConfigNameTemplate, + @Value("${sebserver.webservice.api.exam.indicator.name:}") final String defaultIndicatorName, + @Value("${sebserver.webservice.api.exam.indicator.type:}") final String defaultIndicatorType, + @Value("${sebserver.webservice.api.exam.indicator.color:}") final String defaultIndicatorColor, + @Value("${sebserver.webservice.api.exam.indicator.thresholds:}") final String defaultIndicatorThresholds, + @Value("${sebserver.webservice.configtemplate.examconfig.default.name:}") final String defaultExamConfigNameTemplate, @Value("${sebserver.webservice.configtemplate.examconfig.default.description:}") final String defaultExamConfigDescTemplate) { this.examTemplateDAO = examTemplateDAO; @@ -173,22 +176,28 @@ public class ExamTemplateServiceImpl implements ExamTemplateService { if (examTemplate.configTemplateId != null) { // create new exam configuration for the exam + final ConfigurationNode configurationNode = new ConfigurationNode( + null, + exam.institutionId, + examTemplate.configTemplateId, + replaceVars(this.defaultExamConfigNameTemplate, exam, examTemplate), + replaceVars(this.defaultExamConfigDescTemplate, exam, examTemplate), + ConfigurationType.EXAM_CONFIG, + exam.owner, + ConfigurationStatus.CONSTRUCTION); + final ConfigurationNode examConfig = this.configurationNodeDAO - .createNew(new ConfigurationNode( - null, - exam.institutionId, - examTemplate.configTemplateId, - replaceVars(this.defaultExamConfigNameTemplate, exam, examTemplate), - replaceVars(this.defaultExamConfigDescTemplate, exam, examTemplate), - ConfigurationType.EXAM_CONFIG, - exam.owner, - ConfigurationStatus.CONSTRUCTION)) + .createNew(configurationNode) .onError(error -> log.error( - "Failed to create exam configuration for exam: {} from template: {}", - exam, - examTemplate, + "Failed to create exam configuration for exam: {} from template: {} examConfig: {}", + exam.name, + examTemplate.name, + configurationNode, error)) - .getOrThrow(); + .getOrThrow(error -> new APIMessageException( + ErrorMessage.EXAM_IMPORT_ERROR_AUTO_CONFIG, + error, + API.PARAM_MODEL_ID + Constants.FORM_URL_ENCODED_NAME_VALUE_SEPARATOR + exam.id)); // map the exam configuration to the exam this.examConfigurationMapDAO.createNew(new ExamConfigurationMap( @@ -201,7 +210,10 @@ public class ExamTemplateServiceImpl implements ExamTemplateService { exam, examConfig, error)) - .getOrThrow(); + .getOrThrow(error -> new APIMessageException( + ErrorMessage.EXAM_IMPORT_ERROR_AUTO_CONFIG_LINKING, + error, + API.PARAM_MODEL_ID + Constants.FORM_URL_ENCODED_NAME_VALUE_SEPARATOR + exam.id)); } } else { @@ -269,6 +281,13 @@ public class ExamTemplateServiceImpl implements ExamTemplateService { private Result addDefaultIndicator(final Exam exam) { return Result.tryCatch(() -> { + if (StringUtils.isBlank(this.defaultIndicatorName)) { + if (log.isDebugEnabled()) { + log.debug("No default indicator defined for exam: {}", exam.externalId); + } + return exam; + } + if (log.isDebugEnabled()) { log.debug("Init default indicator for exam: {}", exam.externalId); } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/indicator/AbstractClientIndicator.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/indicator/AbstractClientIndicator.java index 9790c05a..c5fa6bfb 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/indicator/AbstractClientIndicator.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/indicator/AbstractClientIndicator.java @@ -36,8 +36,12 @@ public abstract class AbstractClientIndicator implements ClientIndicator { final boolean active, final boolean cachingEnabled) { - this.indicatorId = (indicatorDefinition != null) ? indicatorDefinition.id : -1; - this.examId = (indicatorDefinition != null) ? indicatorDefinition.examId : -1; + this.indicatorId = (indicatorDefinition != null && indicatorDefinition.id != null) + ? indicatorDefinition.id + : -1; + this.examId = (indicatorDefinition != null && indicatorDefinition.examId != null) + ? indicatorDefinition.examId + : -1; this.connectionId = connectionId; this.active = active; this.cachingEnabled = cachingEnabled; diff --git a/src/main/resources/config/application-ws.properties b/src/main/resources/config/application-ws.properties index 9f4eb807..b289725e 100644 --- a/src/main/resources/config/application-ws.properties +++ b/src/main/resources/config/application-ws.properties @@ -71,6 +71,13 @@ sebserver.webservice.proctoring.resetBroadcastOnLeav=true sebserver.webservice.proctoring.zoom.enableWaitingRoom=false sebserver.webservice.proctoring.zoom.sendRejoinForCollectingRoom=true +# Default indicator example: +#sebserver.webservice.api.exam.indicator.name=Ping +#sebserver.webservice.api.exam.indicator.type=LAST_PING +#sebserver.webservice.api.exam.indicator.color=b4b4b4 +#sebserver.webservice.api.exam.indicator.thresholds=[{'value':5000.0,'color':'22b14c'},{'value':10000.0,'color':'ff7e00'},{'value':15000.0,'color':'ed1c24'}] + +# Default name and description template for auto-generated exam configuration sebserver.webservice.configtemplate.examconfig.default.name=__startDate__ __examName__ sebserver.webservice.configtemplate.examconfig.default.description=This has automatically been created from the exam template: __examTemplateName__ at: __currentDate__ diff --git a/src/main/resources/messages.properties b/src/main/resources/messages.properties index db2251da..fec5fc40 100644 --- a/src/main/resources/messages.properties +++ b/src/main/resources/messages.properties @@ -445,6 +445,8 @@ sebserver.exam.consistency.missing-seb-restriction= - There is currently no SEB sebserver.exam.consistency.no-lms-connection= - Failed to connect to the LMS Setup of this exam yet.
Please check the LMS connection within the LMS Setup. sebserver.exam.consistency.invalid-lms-id= - The referencing course identifier seems to be invalid.
Please check if the course for this exam still exists on the LMS and the course identifier has not changed. sebserver.exam.confirm.remove-config=This exam is current running. The remove of the attached configuration will led to an invalid state
where connecting SEB clients cannot download the configuration for the exam.

Are you sure to remove the configuration? +sebserver.exam.autogen.error.config.title=Exam Import with Template +sebserver.exam.autogen.error.config.text=There was an unexpected error while auto-create exam configuration.
Please add an exam configuration manually. sebserver.exam.action.list=Exam sebserver.exam.action.list.view=View Exam @@ -534,7 +536,6 @@ sebserver.exam.form.sebrestriction.permissions.CHECK_CONFIG_KEY.tooltip=Always c sebserver.exam.form.sebrestriction.permissions.CHECK_BROWSER_EXAM_OR_CONFIG_KEY=Check Browser Exam-, Or Config Key sebserver.exam.form.sebrestriction.permissions.CHECK_BROWSER_EXAM_OR_CONFIG_KEY.tooltip=Always check either SEB Browser Exam Key or SEB Config Key with the defined ones for every request - sebserver.exam.type.UNDEFINED=Not Defined sebserver.exam.type.UNDEFINED.tooltip=No exam type specified sebserver.exam.type.MANAGED=Managed Devices diff --git a/src/test/resources/application-test.properties b/src/test/resources/application-test.properties index 4f74dda7..6c4bd19e 100644 --- a/src/test/resources/application-test.properties +++ b/src/test/resources/application-test.properties @@ -41,4 +41,9 @@ sebserver.webservice.api.redirect.unauthorized=none sebserver.webservice.lms.openedx.api.token.request.paths=/oauth2/access_token sebserver.webservice.lms.moodle.api.token.request.paths -management.endpoints.web.base-path=/actuator \ No newline at end of file +management.endpoints.web.base-path=/actuator + +sebserver.webservice.api.exam.indicator.name=Ping +sebserver.webservice.api.exam.indicator.type=LAST_PING +sebserver.webservice.api.exam.indicator.color=b4b4b4 +sebserver.webservice.api.exam.indicator.thresholds=[{"value":5000.0,"color":"22b14c"},{"value":10000.0,"color":"ff7e00"},{"value":15000.0,"color":"ed1c24"}]