From 6f9daddf5378c10d20b0909fa493feb059a86cb4 Mon Sep 17 00:00:00 2001 From: anhefti Date: Wed, 5 Aug 2020 11:14:39 +0200 Subject: [PATCH] SEBSERV-75 error handling for SEB Restriction --- .../gbl/model/institution/LmsSetup.java | 2 +- .../seb/sebserver/gui/content/ExamForm.java | 19 ++-- .../content/ExamSEBRestrictionSettings.java | 13 +-- .../sebserver/gui/content/LmsSetupForm.java | 8 +- .../lms/impl/NoSEBRestrictionException.java | 4 + .../impl/moodle/MoodleCourseRestriction.java | 93 ++++++++++++++++++- .../lms/impl/moodle/MoodleLmsAPITemplate.java | 2 +- src/main/resources/messages.properties | 1 + .../impl/moodle/MoodleCourseAccessTest.java | 1 - 9 files changed, 122 insertions(+), 21 deletions(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/gbl/model/institution/LmsSetup.java b/src/main/java/ch/ethz/seb/sebserver/gbl/model/institution/LmsSetup.java index 78c7d1ea..9e112c9c 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gbl/model/institution/LmsSetup.java +++ b/src/main/java/ch/ethz/seb/sebserver/gbl/model/institution/LmsSetup.java @@ -46,7 +46,7 @@ public final class LmsSetup implements GrantEntity, Activatable { public enum LmsType { MOCKUP(Features.COURSE_API), OPEN_EDX(Features.COURSE_API, Features.SEB_RESTRICTION), - MOODLE(Features.COURSE_API); + MOODLE(Features.COURSE_API, Features.SEB_RESTRICTION); public final EnumSet features; diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/content/ExamForm.java b/src/main/java/ch/ethz/seb/sebserver/gui/content/ExamForm.java index 1516a3aa..d87c3229 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/content/ExamForm.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/content/ExamForm.java @@ -43,7 +43,8 @@ import ch.ethz.seb.sebserver.gbl.model.exam.Exam.ExamStatus; import ch.ethz.seb.sebserver.gbl.model.exam.ExamConfigurationMap; import ch.ethz.seb.sebserver.gbl.model.exam.Indicator; import ch.ethz.seb.sebserver.gbl.model.exam.QuizData; -import ch.ethz.seb.sebserver.gbl.model.institution.LmsSetup.Features; +import ch.ethz.seb.sebserver.gbl.model.institution.LmsSetupTestResult; +import ch.ethz.seb.sebserver.gbl.model.institution.LmsSetupTestResult.ErrorType; import ch.ethz.seb.sebserver.gbl.model.user.UserRole; import ch.ethz.seb.sebserver.gbl.profile.GuiProfile; import ch.ethz.seb.sebserver.gbl.util.Result; @@ -73,6 +74,7 @@ import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.exam.GetExamConfi import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.exam.GetIndicatorPage; import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.exam.SaveExam; import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.lmssetup.GetLmsSetup; +import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.lmssetup.TestLmsSetup; import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.quiz.GetQuizData; import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.quiz.ImportAsExam; import ch.ethz.seb.sebserver.gui.service.remote.webservice.auth.CurrentUser; @@ -620,12 +622,17 @@ public class ExamForm implements TemplateComposer { } private boolean testSEBRestrictionAPI(final Exam exam) { - return this.restService.getBuilder(GetLmsSetup.class) + // Call the testing endpoint with the specified data to test + final Result result = this.restService.getBuilder(TestLmsSetup.class) .withURIVariable(API.PARAM_MODEL_ID, String.valueOf(exam.lmsSetupId)) - .call() - .onError(t -> log.error("Failed to check SEB restriction API: ", t)) - .map(lmsSetup -> lmsSetup.lmsType.features.contains(Features.SEB_RESTRICTION)) - .getOr(false); + .call(); + + if (result.hasError()) { + return false; + } + + final LmsSetupTestResult lmsSetupTestResult = result.get(); + return !lmsSetupTestResult.hasError(ErrorType.QUIZ_RESTRICTION_API_REQUEST); } private void showConsistencyChecks(final Collection result, final Composite parent) { diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/content/ExamSEBRestrictionSettings.java b/src/main/java/ch/ethz/seb/sebserver/gui/content/ExamSEBRestrictionSettings.java index f3909586..01ef10ba 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/content/ExamSEBRestrictionSettings.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/content/ExamSEBRestrictionSettings.java @@ -200,12 +200,13 @@ public class ExamSEBRestrictionSettings { .call() .getOrThrow(); - final Chapters chapters = restService - .getBuilder(GetCourseChapters.class) - .withURIVariable(API.PARAM_MODEL_ID, entityKey.modelId) - .call() - .onError(t -> t.printStackTrace()) - .getOr(null); + final Chapters chapters = (lmsType == LmsType.OPEN_EDX) + ? restService.getBuilder(GetCourseChapters.class) + .withURIVariable(API.PARAM_MODEL_ID, entityKey.modelId) + .call() + .onError(t -> t.printStackTrace()) + .getOr(null) + : null; final PageContext formContext = this.pageContext .copyOf(content) diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/content/LmsSetupForm.java b/src/main/java/ch/ethz/seb/sebserver/gui/content/LmsSetupForm.java index 5ac43cf3..bcb9d82b 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/content/LmsSetupForm.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/content/LmsSetupForm.java @@ -457,9 +457,11 @@ public class LmsSetupForm implements TemplateComposer { Utils.formatHTMLLinesForceEscaped(Utils.escapeHTML_XML_EcmaScript(error.message)))); } case QUIZ_RESTRICTION_API_REQUEST: { - // NOTE: quiz restriction is not mandatory for functional LmsSetup - // so this error is ignored here - break; + + throw new PageMessageException(new LocTextKey( + "sebserver.lmssetup.action.test.features.error", + "OK", + Utils.formatHTMLLinesForceEscaped(Utils.escapeHTML_XML_EcmaScript(error.message)))); } default: { throw new PageMessageException(new LocTextKey( diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/NoSEBRestrictionException.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/NoSEBRestrictionException.java index c4551cfe..dacc3243 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/NoSEBRestrictionException.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/NoSEBRestrictionException.java @@ -19,4 +19,8 @@ public class NoSEBRestrictionException extends RuntimeException { super(cause); } + public NoSEBRestrictionException(final String message) { + super(message); + } + } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleCourseRestriction.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleCourseRestriction.java index bf06d33b..22b7b62c 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleCourseRestriction.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleCourseRestriction.java @@ -16,12 +16,16 @@ import org.slf4j.LoggerFactory; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.core.type.TypeReference; import ch.ethz.seb.sebserver.gbl.api.JSONMapper; import ch.ethz.seb.sebserver.gbl.model.exam.MoodleSEBRestriction; import ch.ethz.seb.sebserver.gbl.model.institution.LmsSetupTestResult; import ch.ethz.seb.sebserver.gbl.util.Result; +import ch.ethz.seb.sebserver.webservice.servicelayer.lms.impl.NoSEBRestrictionException; import ch.ethz.seb.sebserver.webservice.servicelayer.lms.impl.moodle.MoodleRestTemplateFactory.MoodleAPIRestTemplate; /** GET: @@ -80,8 +84,27 @@ public class MoodleCourseRestriction { } LmsSetupTestResult initAPIAccess() { - // TODO test availability - return LmsSetupTestResult.ofQuizRestrictionAPIError("not available yet"); + // try to call the SEB Restrictions API + try { + + final MoodleAPIRestTemplate template = getRestTemplate() + .getOrThrow(); + + final String jsonResponse = template.callMoodleAPIFunction( + MOODLE_DEFAULT_COURSE_RESTRICTION_WS_FUNCTION, + new LinkedMultiValueMap<>(), + null); + + final Error checkError = this.checkError(jsonResponse); + if (checkError != null) { + return LmsSetupTestResult.ofQuizRestrictionAPIError(checkError.exception); + } + + } catch (final Exception e) { + log.debug("Moodle SEB restriction API not available: ", e); + return LmsSetupTestResult.ofQuizRestrictionAPIError(e.getMessage()); + } + return LmsSetupTestResult.ofOkay(); } Result getSEBRestriction( @@ -125,6 +148,12 @@ public class MoodleCourseRestriction { queryParams, null); + final Error error = this.checkError(resultJSON); + if (error != null) { + log.error("Failed to get SEB restriction: {}", error.toString()); + throw new NoSEBRestrictionException("Failed to get SEB restriction: " + error.exception); + } + final MoodleSEBRestriction restrictiondata = this.jsonMapper.readValue( resultJSON, new TypeReference() { @@ -241,11 +270,17 @@ public class MoodleCourseRestriction { queryParams.add(MOODLE_DEFAULT_COURSE_RESTRICTION_COURSE_ID, courseId); queryParams.add(MOODLE_DEFAULT_COURSE_RESTRICTION_QUIZ_ID, quizId); - template.callMoodleAPIFunction( + final String jsonResponse = template.callMoodleAPIFunction( MOODLE_DEFAULT_COURSE_RESTRICTION_WS_FUNCTION_DELETE, queryParams, null); + final Error error = this.checkError(jsonResponse); + if (error != null) { + log.error("Failed to delete SEB restriction: {}", error.toString()); + return false; + } + return true; }); } @@ -291,6 +326,12 @@ public class MoodleCourseRestriction { queryParams, queryAttributes); + final Error error = this.checkError(resultJSON); + if (error != null) { + log.error("Failed to post SEB restriction: {}", error.toString()); + throw new NoSEBRestrictionException("Failed to post SEB restriction: " + error.exception); + } + final MoodleSEBRestriction restrictiondata = this.jsonMapper.readValue( resultJSON, new TypeReference() { @@ -300,4 +341,50 @@ public class MoodleCourseRestriction { }); } + public Error checkError(final String jsonResponse) { + if (jsonResponse.contains("exception") || jsonResponse.contains("errorcode")) { + try { + return this.jsonMapper.readValue( + jsonResponse, + new TypeReference() { + }); + } catch (final Exception e) { + log.error("Failed to parse error response: {} cause: ", jsonResponse, e); + return null; + } + } + + return null; + } + + @JsonIgnoreProperties(ignoreUnknown = true) + private static class Error { + public final String exception; + public final String errorcode; + public final String message; + + @JsonCreator + Error( + @JsonProperty(value = "exception") final String exception, + @JsonProperty(value = "errorcode") final String errorcode, + @JsonProperty(value = "message") final String message) { + this.exception = exception; + this.errorcode = errorcode; + this.message = message; + } + + @Override + public String toString() { + final StringBuilder builder = new StringBuilder(); + builder.append("Error [exception="); + builder.append(this.exception); + builder.append(", errorcode="); + builder.append(this.errorcode); + builder.append(", message="); + builder.append(this.message); + builder.append("]"); + return builder.toString(); + } + } + } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleLmsAPITemplate.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleLmsAPITemplate.java index 4f0fdebc..3c4d5523 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleLmsAPITemplate.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleLmsAPITemplate.java @@ -57,7 +57,7 @@ public class MoodleLmsAPITemplate implements LmsAPITemplate { @Override public LmsSetupTestResult testCourseRestrictionAPI() { - return LmsSetupTestResult.ofQuizRestrictionAPIError("Not available yet"); + return this.moodleCourseRestriction.initAPIAccess(); } @Override diff --git a/src/main/resources/messages.properties b/src/main/resources/messages.properties index e9ea1cea..13713ae0 100644 --- a/src/main/resources/messages.properties +++ b/src/main/resources/messages.properties @@ -307,6 +307,7 @@ sebserver.lmssetup.action.test.ok=Successfully connected to the course API sebserver.lmssetup.action.test.tokenRequestError=The API access was denied:
{0}

Please check the LMS connection details. sebserver.lmssetup.action.test.quizRequestError=Unable to request courses or exams from the course API of the LMS. {0} sebserver.lmssetup.action.test.quizRestrictionError=Unable to access course restriction API of the LMS. {0} +sebserver.lmssetup.action.test.features.error=The API access was granted but there is some missing functionality:

- Course Access: {0}
- SEB Restriction: {1} sebserver.lmssetup.action.test.missingParameter=There is one or more missing connection parameter.
Please check the connection parameter for this LMS Setup sebserver.lmssetup.action.test.unknownError=An unexpected error happened while trying to connect to the LMS course API. {0} sebserver.lmssetup.action.save=Save LMS Setup diff --git a/src/test/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleCourseAccessTest.java b/src/test/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleCourseAccessTest.java index 05a678fa..2dd01bb8 100644 --- a/src/test/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleCourseAccessTest.java +++ b/src/test/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleCourseAccessTest.java @@ -152,7 +152,6 @@ public class MoodleCourseAccessTest { final MoodleRestTemplateFactory moodleRestTemplateFactory = mock(MoodleRestTemplateFactory.class); final MoodleAPIRestTemplate moodleAPIRestTemplate = mock(MoodleAPIRestTemplate.class); when(moodleRestTemplateFactory.createRestTemplate()).thenReturn(Result.of(moodleAPIRestTemplate)); - //doThrow(RuntimeException.class).when(moodleAPIRestTemplate).testAPIConnection(any()); when(moodleRestTemplateFactory.test()).thenReturn(LmsSetupTestResult.ofOkay()); final MoodleCourseAccess moodleCourseAccess = new MoodleCourseAccess(