From e06394258ad5c843edf7d1296f80bb9798b1c7ee Mon Sep 17 00:00:00 2001 From: anhefti Date: Tue, 24 May 2022 14:40:47 +0200 Subject: [PATCH] SEBSERV-308 test with Moodle and Open edX and fixes --- .../sebserver/gui/content/exam/ExamForm.java | 4 +- .../sebserver/gui/content/exam/ExamList.java | 2 +- .../exam/impl/ExamAdminServiceImpl.java | 2 +- .../servicelayer/lms/SEBRestrictionAPI.java | 8 ++++ .../lms/impl/LmsAPITemplateAdapter.java | 48 +++++++++++-------- .../lms/impl/SEBRestrictionServiceImpl.java | 16 +++++-- .../moodle/legacy/MoodleCourseAccess.java | 27 ++--------- 7 files changed, 56 insertions(+), 51 deletions(-) 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 d177701e..45486939 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 @@ -452,14 +452,14 @@ public class ExamForm implements TemplateComposer { .newAction(ActionDefinition.EXAM_ENABLE_SEB_RESTRICTION) .withEntityKey(entityKey) .withExec(action -> this.examSEBRestrictionSettings.setSEBRestriction(action, true, this.restService)) - .publishIf(() -> sebRestrictionAvailable && readonly && modifyGrant && !importFromQuizData + .publishIf(() -> sebRestrictionAvailable && readonly && editable && !importFromQuizData && BooleanUtils.isFalse(isRestricted)) .newAction(ActionDefinition.EXAM_DISABLE_SEB_RESTRICTION) .withConfirm(() -> ACTION_MESSAGE_SEB_RESTRICTION_RELEASE) .withEntityKey(entityKey) .withExec(action -> this.examSEBRestrictionSettings.setSEBRestriction(action, false, this.restService)) - .publishIf(() -> sebRestrictionAvailable && readonly && modifyGrant && !importFromQuizData + .publishIf(() -> sebRestrictionAvailable && readonly && editable && !importFromQuizData && BooleanUtils.isTrue(isRestricted)) .newAction(ActionDefinition.EXAM_PROCTORING_ON) diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/content/exam/ExamList.java b/src/main/java/ch/ethz/seb/sebserver/gui/content/exam/ExamList.java index 11fb823f..d5c2fd75 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/content/exam/ExamList.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/content/exam/ExamList.java @@ -279,7 +279,7 @@ public class ExamList implements TemplateComposer { return; } - if (exam.getStatus() == ExamStatus.UP_COMING || exam.getStatus() == ExamStatus.FINISHED) { + if (exam.getStatus() != ExamStatus.RUNNING) { return; } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/impl/ExamAdminServiceImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/impl/ExamAdminServiceImpl.java index 2da1e5e4..ea143926 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/impl/ExamAdminServiceImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/impl/ExamAdminServiceImpl.java @@ -114,7 +114,7 @@ public class ExamAdminServiceImpl implements ExamAdminService { return this.lmsAPIService .getLmsAPITemplate(exam.lmsSetupId) - .map(lmsAPI -> !lmsAPI.getSEBClientRestriction(exam).hasError()); + .map(lmsAPI -> !lmsAPI.hasSEBClientRestriction(exam)); } @Override diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/SEBRestrictionAPI.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/SEBRestrictionAPI.java index 681c248c..e542a684 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/SEBRestrictionAPI.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/SEBRestrictionAPI.java @@ -33,6 +33,14 @@ public interface SEBRestrictionAPI { * missing or to another exception on unexpected error case */ Result getSEBClientRestriction(Exam exam); + /** Use this to check if there is a SEB restriction available on the LMS for the specified exam. + * + * @param exam exam the exam to get the SEB restriction data for + * @return true if there is a SEB restriction set on the LMS for the exam or false otherwise */ + default boolean hasSEBClientRestriction(final Exam exam) { + return getSEBClientRestriction(exam).hasError(); + } + /** Applies SEB Client restrictions to the LMS with the given attributes. * * @param externalExamId The exam/course identifier from LMS side (Exam.externalId) 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 d5ac9019..c0846103 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 @@ -40,7 +40,7 @@ public class LmsAPITemplateAdapter implements LmsAPITemplate { private static final Logger log = LoggerFactory.getLogger(LmsAPITemplateAdapter.class); private final CourseAccessAPI courseAccessAPI; - private final SEBRestrictionAPI sebBestrictionAPI; + private final SEBRestrictionAPI sebRestrictionAPI; private final APITemplateDataSupplier apiTemplateDataSupplier; /** CircuitBreaker for protected lmsTestRequest */ @@ -64,10 +64,10 @@ public class LmsAPITemplateAdapter implements LmsAPITemplate { final Environment environment, final APITemplateDataSupplier apiTemplateDataSupplier, final CourseAccessAPI courseAccessAPI, - final SEBRestrictionAPI sebBestrictionAPI) { + final SEBRestrictionAPI sebRestrictionAPI) { this.courseAccessAPI = courseAccessAPI; - this.sebBestrictionAPI = sebBestrictionAPI; + this.sebRestrictionAPI = sebRestrictionAPI; this.apiTemplateDataSupplier = apiTemplateDataSupplier; this.lmsTestRequest = asyncService.createCircuitBreaker( @@ -82,7 +82,7 @@ public class LmsAPITemplateAdapter implements LmsAPITemplate { environment.getProperty( "sebserver.webservice.circuitbreaker.lmsTestRequest.timeToRecover", Long.class, - Constants.MINUTE_IN_MILLIS)); + 0L)); this.allQuizzesRequest = asyncService.createCircuitBreaker( environment.getProperty( @@ -96,7 +96,7 @@ public class LmsAPITemplateAdapter implements LmsAPITemplate { environment.getProperty( "sebserver.webservice.circuitbreaker.quizzesRequest.timeToRecover", Long.class, - Constants.MINUTE_IN_MILLIS)); + 0L)); this.quizzesRequest = asyncService.createCircuitBreaker( environment.getProperty( @@ -110,7 +110,7 @@ public class LmsAPITemplateAdapter implements LmsAPITemplate { environment.getProperty( "sebserver.webservice.circuitbreaker.quizzesRequest.timeToRecover", Long.class, - Constants.MINUTE_IN_MILLIS)); + 0L)); this.quizRequest = asyncService.createCircuitBreaker( environment.getProperty( @@ -124,7 +124,7 @@ public class LmsAPITemplateAdapter implements LmsAPITemplate { environment.getProperty( "sebserver.webservice.circuitbreaker.quizzesRequest.timeToRecover", Long.class, - Constants.MINUTE_IN_MILLIS)); + 0L)); this.chaptersRequest = asyncService.createCircuitBreaker( environment.getProperty( @@ -138,7 +138,7 @@ public class LmsAPITemplateAdapter implements LmsAPITemplate { environment.getProperty( "sebserver.webservice.circuitbreaker.chaptersRequest.timeToRecover", Long.class, - Constants.SECOND_IN_MILLIS * 30)); + 0L)); this.accountDetailRequest = asyncService.createCircuitBreaker( environment.getProperty( @@ -152,7 +152,7 @@ public class LmsAPITemplateAdapter implements LmsAPITemplate { environment.getProperty( "sebserver.webservice.circuitbreaker.accountDetailRequest.timeToRecover", Long.class, - Constants.SECOND_IN_MILLIS * 30)); + 0L)); this.restrictionRequest = asyncService.createCircuitBreaker( environment.getProperty( @@ -166,7 +166,7 @@ public class LmsAPITemplateAdapter implements LmsAPITemplate { environment.getProperty( "sebserver.webservice.circuitbreaker.sebrestriction.timeToRecover", Long.class, - Constants.SECOND_IN_MILLIS * 30)); + 0L)); this.releaseRestrictionRequest = asyncService.createCircuitBreaker( environment.getProperty( @@ -180,7 +180,7 @@ public class LmsAPITemplateAdapter implements LmsAPITemplate { environment.getProperty( "sebserver.webservice.circuitbreaker.sebrestriction.timeToRecover", Long.class, - Constants.SECOND_IN_MILLIS * 30)); + 0L)); } @Override @@ -363,8 +363,8 @@ public class LmsAPITemplateAdapter implements LmsAPITemplate { @Override public LmsSetupTestResult testCourseRestrictionAPI() { - if (this.sebBestrictionAPI != null) { - return this.sebBestrictionAPI.testCourseRestrictionAPI(); + if (this.sebRestrictionAPI != null) { + return this.sebRestrictionAPI.testCourseRestrictionAPI(); } if (log.isDebugEnabled()) { @@ -377,7 +377,7 @@ public class LmsAPITemplateAdapter implements LmsAPITemplate { @Override public Result getSEBClientRestriction(final Exam exam) { - if (this.sebBestrictionAPI == null) { + if (this.sebRestrictionAPI == null) { return Result.ofError( new UnsupportedOperationException("SEB Restriction API Not Supported For: " + getType().name())); } @@ -386,7 +386,7 @@ public class LmsAPITemplateAdapter implements LmsAPITemplate { log.debug("Get course restriction: {} for LMSSetup: {}", exam.externalId, lmsSetup()); } - return this.restrictionRequest.protectedRun(() -> this.sebBestrictionAPI + return this.restrictionRequest.protectedRun(() -> this.sebRestrictionAPI .getSEBClientRestriction(exam) .onError(error -> log.error( "Failed to get SEB restrictions: {}", @@ -394,12 +394,22 @@ public class LmsAPITemplateAdapter implements LmsAPITemplate { .getOrThrow()); } + @Override + public boolean hasSEBClientRestriction(final Exam exam) { + if (this.sebRestrictionAPI == null) { + return false; + } + + return this.sebRestrictionAPI + .getSEBClientRestriction(exam).hasError(); + } + @Override public Result applySEBClientRestriction( final String externalExamId, final SEBRestriction sebRestrictionData) { - if (this.sebBestrictionAPI == null) { + if (this.sebRestrictionAPI == null) { return Result.ofError( new UnsupportedOperationException("SEB Restriction API Not Supported For: " + getType().name())); } @@ -408,7 +418,7 @@ public class LmsAPITemplateAdapter implements LmsAPITemplate { log.debug("Apply course restriction: {} for LMSSetup: {}", externalExamId, lmsSetup()); } - return this.restrictionRequest.protectedRun(() -> this.sebBestrictionAPI + return this.restrictionRequest.protectedRun(() -> this.sebRestrictionAPI .applySEBClientRestriction(externalExamId, sebRestrictionData) .onError(error -> log.error( "Failed to apply SEB restrictions: {}", @@ -419,7 +429,7 @@ public class LmsAPITemplateAdapter implements LmsAPITemplate { @Override public Result releaseSEBClientRestriction(final Exam exam) { - if (this.sebBestrictionAPI == null) { + if (this.sebRestrictionAPI == null) { return Result.ofError( new UnsupportedOperationException("SEB Restriction API Not Supported For: " + getType().name())); } @@ -428,7 +438,7 @@ public class LmsAPITemplateAdapter implements LmsAPITemplate { log.debug("Release course restriction: {} for LMSSetup: {}", exam.externalId, lmsSetup()); } - return this.releaseRestrictionRequest.protectedRun(() -> this.sebBestrictionAPI + return this.releaseRestrictionRequest.protectedRun(() -> this.sebRestrictionAPI .releaseSEBClientRestriction(exam) .onError(error -> log.error( "Failed to release SEB restrictions: {}", diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/SEBRestrictionServiceImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/SEBRestrictionServiceImpl.java index 40fdd722..05c69b7f 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/SEBRestrictionServiceImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/SEBRestrictionServiceImpl.java @@ -248,13 +248,19 @@ public class SEBRestrictionServiceImpl implements SEBRestrictionService { @Override public Result releaseSEBClientRestriction(final Exam exam) { - if (log.isDebugEnabled()) { - log.debug(" *** SEB Restriction *** Release SEB Client restrictions from LMS for exam: {}", exam); - } - return this.lmsAPIService .getLmsAPITemplate(exam.lmsSetupId) - .flatMap(template -> template.releaseSEBClientRestriction(exam)); + .map(template -> { + if (template.lmsSetup().lmsType.features.contains(Features.SEB_RESTRICTION)) { + if (log.isDebugEnabled()) { + log.debug(" *** SEB Restriction *** Release SEB Client restrictions from LMS for exam: {}", + exam); + } + template.releaseSEBClientRestriction(exam); + + } + return exam; + }); } } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/legacy/MoodleCourseAccess.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/legacy/MoodleCourseAccess.java index eb4270c6..2af72e30 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/legacy/MoodleCourseAccess.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/legacy/MoodleCourseAccess.java @@ -156,30 +156,11 @@ public class MoodleCourseAccess implements CourseAccessAPI { @Override public Result> getQuizzes(final Set ids) { return Result.tryCatch(() -> { - final List cached = getCached(); - final List available = (cached != null) - ? cached - : Collections.emptyList(); - - final Map quizMapping = available - .stream() - .collect(Collectors.toMap(q -> q.id, Function.identity())); - - if (!quizMapping.keySet().containsAll(ids)) { - - final Map collect = getRestTemplate() - .map(template -> getQuizzesForIds(template, ids)) - .getOrElse(() -> Collections.emptyList()) - .stream() - .collect(Collectors.toMap(qd -> qd.id, Function.identity())); - if (collect != null) { - quizMapping.clear(); - quizMapping.putAll(collect); - } - } - - return quizMapping.values(); + return getRestTemplate() + .map(template -> getQuizzesForIds(template, ids)) + .onError(error -> log.error("Failed to get courses for: {}", ids, error)) + .getOrElse(() -> Collections.emptyList()); }); }