diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/LmsAPITemplate.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/LmsAPITemplate.java index 25b3e42f..9ac665a7 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/LmsAPITemplate.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/LmsAPITemplate.java @@ -87,9 +87,16 @@ public interface LmsAPITemplate { * @return Result refer to the given SebRestrictionData if restriction was successful or to an error if not */ Result applySebClientRestriction(SebRestrictionData sebRestrictionData); + /** Updates a SEB Client restriction within the LMS with the given attributes. + * + * @param sebRestrictionData containing all data for SEB Client restriction + * @return Result refer to the given SebRestrictionData if updating restriction was successful or to an error if + * not */ + Result updateSebClientRestriction(SebRestrictionData sebRestrictionData); + /** Releases an already applied SEB Client restriction within the LMS for a given Exam. * This completely removes the SEB Client restriction on LMS side. - * + * * @param exam the Exam to release the restriction for * @return Result refer to the given Exam if successful or to an error if not */ Result releaseSebClientRestriction(Exam exam); diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/MockupLmsAPITemplate.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/MockupLmsAPITemplate.java index 2d63743a..5b0f9f0c 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/MockupLmsAPITemplate.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/MockupLmsAPITemplate.java @@ -149,6 +149,12 @@ final class MockupLmsAPITemplate implements LmsAPITemplate { return Result.of(sebRestrictionData); } + @Override + public Result updateSebClientRestriction(final SebRestrictionData sebRestrictionData) { + log.info("Update SEB Client restriction: {}", sebRestrictionData); + return Result.of(sebRestrictionData); + } + @Override public Result releaseSebClientRestriction(final Exam exam) { log.info("Release SEB Client restriction for Exam: {}", exam); diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/OpenEdxLmsAPITemplate.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/OpenEdxLmsAPITemplate.java index b1c51836..9289093b 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/OpenEdxLmsAPITemplate.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/OpenEdxLmsAPITemplate.java @@ -191,6 +191,20 @@ final class OpenEdxLmsAPITemplate implements LmsAPITemplate { }); } + @Override + public Result updateSebClientRestriction(final SebRestrictionData sebRestrictionData) { + return Result.tryCatch(() -> { + + if (log.isDebugEnabled()) { + log.debug("Update SEB Client restriction: {}", sebRestrictionData); + } + + // TODO + + return sebRestrictionData; + }); + } + @Override public Result releaseSebClientRestriction(final Exam exam) { return Result.tryCatch(() -> { diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamConfigUpdateServiceImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamConfigUpdateServiceImpl.java index 9566b5fd..536d9d16 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamConfigUpdateServiceImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamConfigUpdateServiceImpl.java @@ -18,7 +18,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.context.annotation.Lazy; import org.springframework.stereotype.Service; -import org.springframework.transaction.annotation.Transactional; import ch.ethz.seb.sebserver.gbl.Constants; import ch.ethz.seb.sebserver.gbl.api.APIMessage; @@ -74,7 +73,6 @@ public class ExamConfigUpdateServiceImpl implements ExamConfigUpdateService { // generate the new Config Key and update the Config Key within the LMSSetup API for each exam (delete old Key and add new Key) // evict each Exam from cache and release the update-lock on DB @Override - @Transactional public Result> processExamConfigurationChange(final Long configurationNodeId) { final String updateId = this.examUpdateHandler.createUpdateId(); @@ -172,7 +170,6 @@ public class ExamConfigUpdateServiceImpl implements ExamConfigUpdateService { } @Override - @Transactional public Result processExamConfigurationMappingChange( final ExamConfigurationMap mapping, final Function> changeAction) { @@ -205,29 +202,35 @@ public class ExamConfigUpdateServiceImpl implements ExamConfigUpdateService { // check again if there are no new active client connections in the meantime checkActiveClientConnections(exam); - // apply the referenced change action + // apply the referenced change action. On error the change is rolled back and + // this processing returns immediately with the error final T result = changeAction.apply(mapping) - .getOrThrow(); - - // flush the exam cache - this.examSessionService.flushCache(exam) + .onError(t -> log.error("Fauled to save exam configuration: {}", + mapping.configurationNodeId)) .getOrThrow(); // update seb client restriction if the feature is activated if (exam.lmsSebRestriction) { final Result updateSebClientRestriction = this.updateSebClientRestriction(exam); + // if there was an error during update, it is logged but this process goes on + // and the saved changes are not rolled back if (updateSebClientRestriction.hasError()) { log.error("Failed to update SEB Client restriction on LMS for exam: {}", exam); } } - // release the lock + // flush the exam cache. If there was an error during flush, it is logged but this process goes on + // and the saved changes are not rolled back + this.examSessionService.flushCache(exam) + .onError(t -> log.error("Failed to flush cache for exam: {}", exam)); + + // release the exam lock this.examDAO.releaseLock(exam.id, updateId) - .getOrThrow(); + .onError(t -> log.error("Failed to release lock for exam: {}", exam)); return result; }) - .onError(TransactionHandler::rollback); + .onError(t -> this.examDAO.forceUnlock(mapping.examId)); } @@ -280,20 +283,17 @@ public class ExamConfigUpdateServiceImpl implements ExamConfigUpdateService { @Override public Result applySebClientRestriction(final Exam exam) { - return this.examUpdateHandler.applySebClientRestriction(exam) - .onError(error -> log.error("Failed to apply SEB Client restriction for Exam: {}", exam, error)); + return this.examUpdateHandler.applySebClientRestriction(exam); } @Override public Result updateSebClientRestriction(final Exam exam) { - return this.examUpdateHandler.updateSebClientRestriction(exam) - .onError(error -> log.error("Failed to update SEB Client restriction for Exam: {}", exam, error)); + return this.examUpdateHandler.updateSebClientRestriction(exam); } @Override public Result releaseSebClientRestriction(final Exam exam) { - return this.examUpdateHandler.releaseSebClientRestriction(exam) - .onError(error -> log.error("Failed to release SEB Client restriction for Exam: {}", exam, error)); + return this.examUpdateHandler.releaseSebClientRestriction(exam); } private void checkIntegrityDoubleCheck( 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 92c83cef..61291447 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 @@ -126,24 +126,7 @@ class ExamUpdateHandler { return Result.tryCatch(() -> { - final Collection configKeys = this.sebExamConfigService - .generateConfigKeys(exam.institutionId, exam.id) - .getOrThrow(); - - final Collection browserExamKeys = new ArrayList<>(); - final String browserExamKeysString = exam.getBrowserExamKeys(); - if (StringUtils.isNotBlank(browserExamKeysString)) { - browserExamKeys.addAll(Arrays.asList(StringUtils.split( - browserExamKeysString, - Constants.LIST_SEPARATOR))); - } - - final SebRestrictionData sebRestrictionData = new SebRestrictionData( - exam, - configKeys, - browserExamKeys, - // TODO when we have more restriction details available form the Exam, put it to the map - Collections.emptyMap()); + final SebRestrictionData sebRestrictionData = createSebRestrictionData(exam); if (log.isDebugEnabled()) { log.debug("Appling SEB Client restriction on LMS with: {}", sebRestrictionData); @@ -166,11 +149,20 @@ class ExamUpdateHandler { return Result.of(exam); } - if (log.isDebugEnabled()) { - log.debug("Update SEB Client restrictions for exam: {}", exam); - } - // TODO Auto-generated method stub - return null; + return Result.tryCatch(() -> { + + final SebRestrictionData sebRestrictionData = createSebRestrictionData(exam); + + if (log.isDebugEnabled()) { + log.debug("Update SEB Client restrictions for exam: {}", exam); + } + + return this.lmsAPIService + .getLmsAPITemplate(exam.lmsSetupId) + .flatMap(lmsTemplate -> lmsTemplate.updateSebClientRestriction(sebRestrictionData)) + .map(data -> exam) + .getOrThrow(); + }); } Result releaseSebClientRestriction(final Exam exam) { @@ -184,4 +176,39 @@ class ExamUpdateHandler { .flatMap(template -> template.releaseSebClientRestriction(exam)); } +// @Transactional +// public Result updateExamConfigurationMappingChange( +// final ExamConfigurationMap mapping, +// final Function> changeAction, +// final Exam exam) { +// +// return Result.tryCatch(() -> { +// +// return result; +// }) +// .onError(TransactionHandler::rollback); +// } + + private SebRestrictionData createSebRestrictionData(final Exam exam) { + final Collection configKeys = this.sebExamConfigService + .generateConfigKeys(exam.institutionId, exam.id) + .getOrThrow(); + + final Collection browserExamKeys = new ArrayList<>(); + final String browserExamKeysString = exam.getBrowserExamKeys(); + if (StringUtils.isNotBlank(browserExamKeysString)) { + browserExamKeys.addAll(Arrays.asList(StringUtils.split( + browserExamKeysString, + Constants.LIST_SEPARATOR))); + } + + final SebRestrictionData sebRestrictionData = new SebRestrictionData( + exam, + configKeys, + browserExamKeys, + // TODO when we have more restriction details available form the Exam, put it to the map + Collections.emptyMap()); + return sebRestrictionData; + } + } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamConfigurationMappingController.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamConfigurationMappingController.java index 5a17d279..34a29394 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamConfigurationMappingController.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamConfigurationMappingController.java @@ -194,7 +194,7 @@ public class ExamConfigurationMappingController extends EntityController pair); }