From 21200bd9a29b6785ebb9967fc0441e258347460e Mon Sep 17 00:00:00 2001 From: anhefti Date: Mon, 20 Mar 2023 09:20:30 +0100 Subject: [PATCH] Better course recovering handling and logging --- .../sebserver/gbl/async/CircuitBreaker.java | 4 ++ .../gbl/model/institution/LmsSetup.java | 9 +++-- .../lms/impl/LmsAPITemplateAdapter.java | 21 ++++++++-- .../session/impl/ExamUpdateHandler.java | 38 +++++++++++++++++-- 4 files changed, 62 insertions(+), 10 deletions(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/gbl/async/CircuitBreaker.java b/src/main/java/ch/ethz/seb/sebserver/gbl/async/CircuitBreaker.java index 61d85d65..89db1a7f 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gbl/async/CircuitBreaker.java +++ b/src/main/java/ch/ethz/seb/sebserver/gbl/async/CircuitBreaker.java @@ -261,6 +261,10 @@ public final class CircuitBreaker { if (log.isWarnEnabled()) { log.warn("Attempt error: {}, {}", e.getMessage(), this.state); } + final Throwable cause = e.getCause(); + if (cause != null && cause instanceof Exception) { + return Result.ofError((Exception) cause); + } return Result.ofError(e); } catch (final TimeoutException e) { future.cancel(false); 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 9fb6852e..93f4dac2 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 @@ -48,7 +48,10 @@ public final class LmsSetup implements GrantEntity, Activatable { * The SEB restriciton is usually in the form of certain hash keys and addition * restriction settings that prompt the LMS to check access on course/quiz connection and * allow only access for a dedicated SEB client with the right configuration in place. */ - SEB_RESTRICTION + SEB_RESTRICTION, + /** Indicates if the LMS integration has some process for course recovery + * after backup-restore process for example. */ + COURSE_RECOVERY } /** Defines the supported types if LMS bindings. @@ -59,9 +62,9 @@ public final class LmsSetup implements GrantEntity, Activatable { /** The Open edX LMS binding features both APIs, course access as well as SEB restriction */ OPEN_EDX(Features.COURSE_API, Features.SEB_RESTRICTION), /** The Moodle binding features only the course access API so far */ - MOODLE(Features.COURSE_API /* , Features.SEB_RESTRICTION */), + MOODLE(Features.COURSE_API, Features.COURSE_RECOVERY /* , Features.SEB_RESTRICTION */), /** The Moodle binding features with SEB Server integration plugin for fully featured */ - MOODLE_PLUGIN(Features.COURSE_API, Features.SEB_RESTRICTION), + MOODLE_PLUGIN(Features.COURSE_API, Features.COURSE_RECOVERY, Features.SEB_RESTRICTION), /** The Ans Delft binding is on the way */ ANS_DELFT(Features.COURSE_API, Features.SEB_RESTRICTION), /** The OpenOLAT binding is on the way */ diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/LmsAPITemplateAdapter.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/LmsAPITemplateAdapter.java index f1fb726f..f9d73794 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/LmsAPITemplateAdapter.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/LmsAPITemplateAdapter.java @@ -48,6 +48,8 @@ public class LmsAPITemplateAdapter implements LmsAPITemplate { private final CircuitBreaker> quizzesRequest; /** CircuitBreaker for protected quiz and course data requests */ private final CircuitBreaker quizRequest; + /** CircuitBreaker for protected quiz and course data requests */ + private final CircuitBreaker quizRecoverRequest; /** CircuitBreaker for protected chapter data requests */ private final CircuitBreaker chaptersRequest; /** CircuitBreaker for protected examinee account details requests */ @@ -109,6 +111,20 @@ public class LmsAPITemplateAdapter implements LmsAPITemplate { Long.class, 0L)); + this.quizRecoverRequest = asyncService.createCircuitBreaker( + environment.getProperty( + "sebserver.webservice.circuitbreaker.quizzesRequest.attempts", + Integer.class, + 1), + environment.getProperty( + "sebserver.webservice.circuitbreaker.quizzesRequest.blockingTime", + Long.class, + Constants.SECOND_IN_MILLIS * 10), + environment.getProperty( + "sebserver.webservice.circuitbreaker.quizzesRequest.timeToRecover", + Long.class, + 0L)); + this.chaptersRequest = asyncService.createCircuitBreaker( environment.getProperty( "sebserver.webservice.circuitbreaker.chaptersRequest.attempts", @@ -293,11 +309,8 @@ public class LmsAPITemplateAdapter implements LmsAPITemplate { log.debug("Try to recover quiz for exam {} for LMSSetup: {}", exam, lmsSetup()); } - return this.quizRequest.protectedRun(() -> this.courseAccessAPI + return this.quizRecoverRequest.protectedRun(() -> this.courseAccessAPI .tryRecoverQuizForExam(exam) - .onError(error -> log.error( - "Failed to run protectedQuizRecoverRequest: {}", - error.getMessage())) .getOrThrow()); } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamUpdateHandler.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamUpdateHandler.java index e4e60055..f76b3a67 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamUpdateHandler.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamUpdateHandler.java @@ -27,6 +27,7 @@ import ch.ethz.seb.sebserver.gbl.api.EntityType; import ch.ethz.seb.sebserver.gbl.model.exam.Exam; import ch.ethz.seb.sebserver.gbl.model.exam.Exam.ExamStatus; 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.profile.WebServiceProfile; import ch.ethz.seb.sebserver.gbl.util.Result; import ch.ethz.seb.sebserver.gbl.util.Utils; @@ -35,6 +36,7 @@ import ch.ethz.seb.sebserver.webservice.datalayer.batis.model.AdditionalAttribut import ch.ethz.seb.sebserver.webservice.servicelayer.dao.AdditionalAttributesDAO; import ch.ethz.seb.sebserver.webservice.servicelayer.dao.ExamDAO; import ch.ethz.seb.sebserver.webservice.servicelayer.lms.LmsAPIService; +import ch.ethz.seb.sebserver.webservice.servicelayer.lms.LmsAPITemplate; import ch.ethz.seb.sebserver.webservice.servicelayer.lms.SEBRestrictionService; import ch.ethz.seb.sebserver.webservice.servicelayer.session.ExamFinishedEvent; import ch.ethz.seb.sebserver.webservice.servicelayer.session.ExamResetEvent; @@ -149,6 +151,11 @@ class ExamUpdateHandler { } else { if (!exam.isLmsAvailable()) { this.examDAO.markLMSAvailability(quiz.id, true, updateId); + // delete attempts attribute + this.additionalAttributesDAO.delete( + EntityType.EXAM, + exam.id, + Exam.ADDITIONAL_ATTR_QUIZ_RECOVER_ATTEMPTS); } failedOrMissing.remove(quiz.id); log.info("Updated quiz data for exam: {}", updateQuizData.get()); @@ -351,8 +358,24 @@ class ExamUpdateHandler { !Utils.isEqualsWithEmptyCheck(exam.getDescription(), quizData.description) || !Utils.isEqualsWithEmptyCheck(exam.getStartURL(), quizData.startURL)) { - if (log.isDebugEnabled()) { - log.debug("Update difference from LMS. Exam:{}, QuizData: {}", exam, quizData); + if (!Utils.isEqualsWithEmptyCheck(exam.name, quizData.name)) { + log.info("Update name difference from LMS. Exam:{}, QuizData: {}", exam.name, quizData.name); + } + if (!Objects.equals(exam.startTime, quizData.startTime)) { + log.info("Update startTime difference from LMS. Exam:{}, QuizData: {}", exam.startTime, + quizData.startTime); + } + if (!Objects.equals(exam.endTime, quizData.endTime)) { + log.info("Update endTime difference from LMS. Exam:{}, QuizData: {}", exam.endTime, quizData.endTime); + } + if (!Utils.isEqualsWithEmptyCheck(exam.getDescription(), quizData.description)) { + log.info("Update description difference from LMS. Exam:{}, QuizData: {}", exam.getDescription(), + quizData.description); + } + if (!Utils.isEqualsWithEmptyCheck(exam.getStartURL(), quizData.startURL)) { + log.info("Update startURL difference from LMS. Exam:{}, QuizData: {}", + exam.getStartURL(), + quizData.startURL); } return true; @@ -384,6 +407,14 @@ class ExamUpdateHandler { return Result.tryCatch(() -> { + final LmsAPITemplate lmsTemplate = this.lmsAPIService + .getLmsAPITemplate(lmsSetupId) + .getOrThrow(); + + if (!lmsTemplate.getType().features.contains(Features.COURSE_RECOVERY)) { + throw new UnsupportedOperationException("No Course Recovery"); + } + final Exam exam = exams.get(quizId); final int attempts = Integer.parseInt(this.additionalAttributesDAO.getAdditionalAttribute( EntityType.EXAM, @@ -400,7 +431,8 @@ class ExamUpdateHandler { } log.info( - "Try to recover quiz data for Moodle quiz with internal identifier: {}", + "Try to recover quiz data from LMS: {} quiz with internal identifier: {}", + lmsSetupId, quizId); return this.lmsAPIService