From 90e8975269be56bd90b6fc5f4f38fb54929cbe18 Mon Sep 17 00:00:00 2001 From: anhefti Date: Wed, 17 Feb 2021 17:50:50 +0100 Subject: [PATCH 01/10] fixed security view gaps --- .../sql/base/V5_2__insert_new_security_settings_v.1.1.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/resources/config/sql/base/V5_2__insert_new_security_settings_v.1.1.sql b/src/main/resources/config/sql/base/V5_2__insert_new_security_settings_v.1.1.sql index 18207a50..fea8ad0e 100644 --- a/src/main/resources/config/sql/base/V5_2__insert_new_security_settings_v.1.1.sql +++ b/src/main/resources/config/sql/base/V5_2__insert_new_security_settings_v.1.1.sql @@ -8,8 +8,8 @@ INSERT IGNORE INTO configuration_attribute VALUES UPDATE orientation SET y_position='13', width='12' WHERE id='305'; -UPDATE orientation SET y_position='16', width='10' WHERE id='306'; -UPDATE orientation SET y_position='17', width='10' WHERE id='307'; +UPDATE orientation SET y_position='16', width='9' WHERE id='306'; +UPDATE orientation SET y_position='17', width='9' WHERE id='307'; UPDATE orientation SET y_position='18' WHERE id='317'; UPDATE orientation SET x_position='0', y_position='9' WHERE id='301'; UPDATE orientation SET x_position='3', y_position='10', width='4' WHERE id='501'; From 96313e41a731b59a2bd32e44a6dde4e14cfe79a9 Mon Sep 17 00:00:00 2001 From: anhefti Date: Thu, 18 Feb 2021 12:41:04 +0100 Subject: [PATCH 02/10] minor fixes --- .../servicelayer/sebconfig/impl/converter/TableConverter.java | 3 ++- .../session/impl/SEBClientConnectionServiceImpl.java | 2 -- src/main/resources/config/application-dev.properties | 1 + 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/converter/TableConverter.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/converter/TableConverter.java index e1a74c06..d252db2d 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/converter/TableConverter.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/converter/TableConverter.java @@ -113,7 +113,8 @@ public class TableConverter implements AttributeValueConverter { value.institutionId, value.configurationId, attribute.id) - .getOrThrow(); + .onError(error -> log.error("Failed to get table values for attribute: {}", attribute.name, error)) + .getOrElse(() -> Collections.emptyList()); final boolean noValues = CollectionUtils.isEmpty(values); diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/SEBClientConnectionServiceImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/SEBClientConnectionServiceImpl.java index 4b7646a7..0ced8866 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/SEBClientConnectionServiceImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/SEBClientConnectionServiceImpl.java @@ -294,8 +294,6 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic // connection integrity check if (clientConnection.status == ConnectionStatus.CONNECTION_REQUESTED) { - // TODO discuss if we need a flag on exam domain level that indicates whether unauthenticated connection - // are allowed or not log.warn("ClientConnection integrity warning: client connection is not authenticated: {}", clientConnection); } else if (clientConnection.status != ConnectionStatus.AUTHENTICATED) { diff --git a/src/main/resources/config/application-dev.properties b/src/main/resources/config/application-dev.properties index 5ec2e5a1..7a2939a8 100644 --- a/src/main/resources/config/application-dev.properties +++ b/src/main/resources/config/application-dev.properties @@ -8,6 +8,7 @@ server.tomcat.uri-encoding=UTF-8 logging.level.ch=INFO logging.level.org.springframework.cache=INFO logging.level.ch.ethz.seb.sebserver.webservice.servicelayer.lms.impl=DEBUG +logging.level.ch.ethz.seb.sebserver.webservice.servicelayer.session=DEBUG sebserver.http.client.connect-timeout=150000 sebserver.http.client.connection-request-timeout=100000 From 26a26c7989c5604d7cffc49e43b3af8ed2bb225e Mon Sep 17 00:00:00 2001 From: anhefti Date: Thu, 18 Feb 2021 13:20:52 +0100 Subject: [PATCH 03/10] investigate TableConverter error --- .../sebconfig/impl/converter/TableConverter.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/converter/TableConverter.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/converter/TableConverter.java index d252db2d..0f5960de 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/converter/TableConverter.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/converter/TableConverter.java @@ -73,6 +73,11 @@ public class TableConverter implements AttributeValueConverter { this.configurationAttributeDAO = configurationAttributeDAO; this.configurationValueDAO = configurationValueDAO; + + if (configurationValueDAO == null) { + log.error("Failed to inject ConfigurationValueDAO is {} configurationAttributeDAO is {}", + configurationValueDAO, configurationAttributeDAO); + } } @Override @@ -109,6 +114,8 @@ public class TableConverter implements AttributeValueConverter { final ConfigurationValue value, final boolean xml) throws IOException { + log.debug("Convert: {} -- {} -- {}", attribute.name, this.configurationValueDAO, value); + final List> values = this.configurationValueDAO.getOrderedTableValues( value.institutionId, value.configurationId, From 653b5a4c750e6b597e686108c0f569ad1ea368c1 Mon Sep 17 00:00:00 2001 From: anhefti Date: Thu, 18 Feb 2021 13:31:23 +0100 Subject: [PATCH 04/10] inspection --- .../servicelayer/sebconfig/impl/converter/TableConverter.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/converter/TableConverter.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/converter/TableConverter.java index 0f5960de..551641e7 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/converter/TableConverter.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/converter/TableConverter.java @@ -114,7 +114,8 @@ public class TableConverter implements AttributeValueConverter { final ConfigurationValue value, final boolean xml) throws IOException { - log.debug("Convert: {} -- {} -- {}", attribute.name, this.configurationValueDAO, value); + log.debug("******************************** Convert: {} -- {} -- {}", attribute, this.configurationValueDAO, + value); final List> values = this.configurationValueDAO.getOrderedTableValues( value.institutionId, From 8e0dd36a41757856575601dd1671e7221ab8e1a2 Mon Sep 17 00:00:00 2001 From: anhefti Date: Thu, 18 Feb 2021 13:33:31 +0100 Subject: [PATCH 05/10] invest --- .../sebconfig/impl/converter/TableConverter.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/converter/TableConverter.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/converter/TableConverter.java index 551641e7..964f2336 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/converter/TableConverter.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/converter/TableConverter.java @@ -74,10 +74,8 @@ public class TableConverter implements AttributeValueConverter { this.configurationAttributeDAO = configurationAttributeDAO; this.configurationValueDAO = configurationValueDAO; - if (configurationValueDAO == null) { - log.error("Failed to inject ConfigurationValueDAO is {} configurationAttributeDAO is {}", - configurationValueDAO, configurationAttributeDAO); - } + log.info("******************************* inject ConfigurationValueDAO is {} configurationAttributeDAO is {}", + configurationValueDAO, configurationAttributeDAO); } @Override @@ -114,7 +112,7 @@ public class TableConverter implements AttributeValueConverter { final ConfigurationValue value, final boolean xml) throws IOException { - log.debug("******************************** Convert: {} -- {} -- {}", attribute, this.configurationValueDAO, + log.info("******************************** Convert: {} -- {} -- {}", attribute, this.configurationValueDAO, value); final List> values = this.configurationValueDAO.getOrderedTableValues( From fa327e4e292cb03cce51d84596954b48e909c308 Mon Sep 17 00:00:00 2001 From: anhefti Date: Mon, 22 Feb 2021 13:05:08 +0100 Subject: [PATCH 06/10] fixed view of foreign SEB Settings --- .../service/examconfig/impl/ViewContext.java | 4 ++++ .../impl/rules/BrowserViewModeRule.java | 2 +- .../impl/rules/IgnoreSEBService.java | 4 ++++ .../sebconfig/impl/ExamConfigIO.java | 9 ++++--- .../sebconfig/impl/ExamConfigServiceImpl.java | 20 ---------------- .../impl/converter/TableConverter.java | 24 +++++++++---------- 6 files changed, 27 insertions(+), 36 deletions(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/examconfig/impl/ViewContext.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/examconfig/impl/ViewContext.java index 20e1fabf..bce56121 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/examconfig/impl/ViewContext.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/examconfig/impl/ViewContext.java @@ -64,6 +64,10 @@ public final class ViewContext { this.readonly = readonly; } + public boolean isReadonly() { + return this.readonly; + } + public I18nSupport getI18nSupport() { return this.i18nSupport; } diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/examconfig/impl/rules/BrowserViewModeRule.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/examconfig/impl/rules/BrowserViewModeRule.java index 8d7686f5..16328215 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/examconfig/impl/rules/BrowserViewModeRule.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/examconfig/impl/rules/BrowserViewModeRule.java @@ -45,7 +45,7 @@ public class BrowserViewModeRule implements ValueChangeRule { final ConfigurationAttribute attribute, final ConfigurationValue value) { - if (StringUtils.isBlank(value.value)) { + if (context.isReadonly() || StringUtils.isBlank(value.value)) { return; } diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/examconfig/impl/rules/IgnoreSEBService.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/examconfig/impl/rules/IgnoreSEBService.java index e28f0b8e..e5f8e7b5 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/examconfig/impl/rules/IgnoreSEBService.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/examconfig/impl/rules/IgnoreSEBService.java @@ -41,6 +41,10 @@ public class IgnoreSEBService implements ValueChangeRule { final ConfigurationAttribute attribute, final ConfigurationValue value) { + if (context.isReadonly()) { + return; + } + if (KEY_IGNORE_SEB_SERVICE.equals(attribute.name)) { if (BooleanUtils.toBoolean(value.value)) { context.disable(KEY_SEB_SERVICE_POLICY); diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/ExamConfigIO.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/ExamConfigIO.java index 6f60a330..9e77af50 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/ExamConfigIO.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/ExamConfigIO.java @@ -37,6 +37,7 @@ import org.xml.sax.SAXException; import ch.ethz.seb.sebserver.gbl.Constants; import ch.ethz.seb.sebserver.gbl.async.AsyncServiceSpringConfig; +import ch.ethz.seb.sebserver.gbl.model.sebconfig.Configuration; import ch.ethz.seb.sebserver.gbl.model.sebconfig.ConfigurationAttribute; import ch.ethz.seb.sebserver.gbl.model.sebconfig.ConfigurationValue; import ch.ethz.seb.sebserver.gbl.profile.WebServiceProfile; @@ -140,7 +141,7 @@ public class ExamConfigIO { .collect(Collectors.toList()); final Function configurationValueSupplier = - getConfigurationValueSupplier(institutionId, configurationId); + getConfigurationValueSupplier(configurationId); writeHeader(exportFormat, out); @@ -316,11 +317,13 @@ public class ExamConfigIO { } private Function getConfigurationValueSupplier( - final Long institutionId, final Long configurationId) { + final Configuration configuration = this.configurationDAO.byPK(configurationId) + .getOrThrow(); + final Map mapping = this.configurationValueDAO - .allRootAttributeValues(institutionId, configurationId) + .allRootAttributeValues(configuration.institutionId, configurationId) .getOrThrow() .stream() .collect(Collectors.toMap( diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/ExamConfigServiceImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/ExamConfigServiceImpl.java index 144de3f8..46870da6 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/ExamConfigServiceImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/ExamConfigServiceImpl.java @@ -267,26 +267,6 @@ public class ExamConfigServiceImpl implements ExamConfigService { log.debug("Start to stream plain JSON SEB Configuration data for Config-Key generation"); } -// if (true) { -// PipedOutputStream pout; -// PipedInputStream pin; -// try { -// pout = new PipedOutputStream(); -// pin = new PipedInputStream(pout); -// this.examConfigIO.exportPlain( -// ConfigurationFormat.JSON, -// pout, -// institutionId, -// configurationNodeId); -// -// final String json = IOUtils.toString(pin, "UTF-8"); -// -// log.trace("SEB Configuration JSON to create Config-Key: {}", json); -// } catch (final Exception e) { -// log.error("Failed to trace SEB Configuration JSON: ", e); -// } -// } - PipedOutputStream pout = null; PipedInputStream pin = null; try { diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/converter/TableConverter.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/converter/TableConverter.java index 964f2336..96c3ade4 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/converter/TableConverter.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/sebconfig/impl/converter/TableConverter.java @@ -10,6 +10,7 @@ package ch.ethz.seb.sebserver.webservice.servicelayer.sebconfig.impl.converter; import java.io.IOException; import java.io.OutputStream; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; @@ -73,9 +74,6 @@ public class TableConverter implements AttributeValueConverter { this.configurationAttributeDAO = configurationAttributeDAO; this.configurationValueDAO = configurationValueDAO; - - log.info("******************************* inject ConfigurationValueDAO is {} configurationAttributeDAO is {}", - configurationValueDAO, configurationAttributeDAO); } @Override @@ -112,15 +110,17 @@ public class TableConverter implements AttributeValueConverter { final ConfigurationValue value, final boolean xml) throws IOException { - log.info("******************************** Convert: {} -- {} -- {}", attribute, this.configurationValueDAO, - value); - - final List> values = this.configurationValueDAO.getOrderedTableValues( - value.institutionId, - value.configurationId, - attribute.id) - .onError(error -> log.error("Failed to get table values for attribute: {}", attribute.name, error)) - .getOrElse(() -> Collections.emptyList()); + final List> values = new ArrayList<>(); + if (value != null) { + values.addAll(this.configurationValueDAO.getOrderedTableValues( + value.institutionId, + value.configurationId, + attribute.id) + .onError(error -> log.error("Failed to get table values for attribute: {}", attribute.name, error)) + .getOrElse(() -> Collections.emptyList())); + } else { + log.warn("No ConfigurationValue for table: {}. Convert to empty table", attribute); + } final boolean noValues = CollectionUtils.isEmpty(values); From 290ca046c3ba4dc9fe554795fb12f47ef5c4d735 Mon Sep 17 00:00:00 2001 From: anhefti Date: Mon, 22 Feb 2021 13:05:59 +0100 Subject: [PATCH 07/10] allow updating client connection in active state --- .../impl/SEBClientConnectionServiceImpl.java | 52 +++++++++++++++++-- 1 file changed, 48 insertions(+), 4 deletions(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/SEBClientConnectionServiceImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/SEBClientConnectionServiceImpl.java index 0ced8866..5b1661fa 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/SEBClientConnectionServiceImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/SEBClientConnectionServiceImpl.java @@ -201,7 +201,12 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic final ClientConnection clientConnection = getClientConnection(connectionToken); checkInstitutionalIntegrity(institutionId, clientConnection); - checkExamIntegrity(examId, clientConnection); + checkExamIdIntegrity(examId, clientConnection); + + // If we have an active connection, update the connection data if requested + if (clientConnection.status == ConnectionStatus.ACTIVE) { + return updateActiveConnection(clientConnection, clientAddress, userSessionId); + } // connection integrity check if (clientConnection.status != ConnectionStatus.CONNECTION_REQUESTED) { @@ -262,6 +267,45 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic }); } + private ClientConnection updateActiveConnection( + final ClientConnection clientConnection, + final String clientAddress, + final String userSessionId) { + + final String virtualClientAddress = getVirtualClientAddress( + clientConnection.examId, + clientAddress, + clientConnection.clientAddress); + + final ClientConnection updatedClientConnection = this.clientConnectionDAO + .save(new ClientConnection( + clientConnection.id, + null, + null, + null, + null, + (StringUtils.isNotBlank(userSessionId)) ? userSessionId : null, + (StringUtils.isNotBlank(clientAddress)) ? clientAddress : null, + virtualClientAddress, + null, + null, + null, + null)) + .getOrThrow(); + + final ClientConnectionDataInternal activeClientConnection = + reloadConnectionCache(clientConnection.connectionToken); + + if (activeClientConnection == null) { + log.warn("Failed to load ClientConnectionDataInternal into cache on update"); + } else if (log.isDebugEnabled()) { + log.debug("SEB client connection, successfully updated ClientConnection: {}", + updatedClientConnection); + } + + return updatedClientConnection; + } + @Override public Result establishClientConnection( final String connectionToken, @@ -289,7 +333,7 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic ClientConnection clientConnection = getClientConnection(connectionToken); checkInstitutionalIntegrity(institutionId, clientConnection); - checkExamIntegrity(examId, clientConnection); + checkExamIdIntegrity(examId, clientConnection); clientConnection = updateUserSessionId(userSessionId, clientConnection, examId); // connection integrity check @@ -624,7 +668,7 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic .getOrThrow().institutionId; } - private void checkExamIntegrity(final Long examId, final ClientConnection clientConnection) { + private void checkExamIdIntegrity(final Long examId, final ClientConnection clientConnection) { if (examId != null && clientConnection.examId != null && !examId.equals(clientConnection.examId)) { @@ -703,7 +747,7 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic "Exam is currently on update and locked for new SEB Client connections"); } - // check Exam has an default SEB Exam configuration attached + // check Exam has a default SEB Exam configuration attached if (!this.examSessionService.hasDefaultConfigurationAttached(examId)) { throw new APIConstraintViolationException( "Exam is currently running but has no default SEB Exam configuration attached"); From 0383aebcbfe91c211cdf1619a22005d5637f3f00 Mon Sep 17 00:00:00 2001 From: anhefti Date: Mon, 1 Mar 2021 20:07:00 +0100 Subject: [PATCH 08/10] fix: take also finished exams into running check --- .../sebserver/webservice/servicelayer/dao/impl/ExamDAOImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ExamDAOImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ExamDAOImpl.java index b92f3dbf..3ded3a19 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ExamDAOImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ExamDAOImpl.java @@ -431,7 +431,7 @@ public class ExamDAOImpl implements ExamDAO { isEqualTo(BooleanUtils.toInteger(true))) .and( ExamRecordDynamicSqlSupport.status, - isEqualTo(ExamStatus.UP_COMING.name())) + isNotEqualTo(ExamStatus.RUNNING.name())) .and( ExamRecordDynamicSqlSupport.updating, isEqualTo(BooleanUtils.toInteger(false))) From 527c005702cd7488d8f490036f776d3ff21efa64 Mon Sep 17 00:00:00 2001 From: anhefti Date: Mon, 1 Mar 2021 20:21:12 +0100 Subject: [PATCH 09/10] fixed update id --- .../webservice/servicelayer/session/impl/ExamUpdateHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 c6568112..c83b9d60 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 @@ -45,7 +45,7 @@ class ExamUpdateHandler { this.examDAO = examDAO; this.sebRestrictionService = sebRestrictionService; - this.updatePrefix = webserviceInfo.getHostAddress() + this.updatePrefix = webserviceInfo.getLocalHostAddress() + "_" + webserviceInfo.getServerPort() + "_"; this.examTimeSuffix = examTimeSuffix; } From 2f8913f1297ae9a970302006ec8cb922ce0aa586 Mon Sep 17 00:00:00 2001 From: anhefti Date: Wed, 3 Mar 2021 13:28:33 +0100 Subject: [PATCH 10/10] improved LMS fail handling on exams 1. if LMS is not available the exams gets a state override and is not running 2. if LMS lms is available but the course id is invalid, the exams gets a state override and is not running --- .../seb/sebserver/gbl/api/APIMessage.java | 5 +- .../seb/sebserver/gbl/model/exam/Exam.java | 4 +- .../model/institution/LmsSetupTestResult.java | 4 ++ .../seb/sebserver/gui/content/ExamForm.java | 38 +++++++------- .../gui/content/ExamFormConfigs.java | 19 +++---- .../gui/content/ExamFormIndicators.java | 12 ++--- .../seb/sebserver/gui/content/ExamList.java | 2 +- .../content/ExamSEBRestrictionSettings.java | 20 ++++++-- .../servicelayer/dao/impl/ExamDAOImpl.java | 50 ++++++++++++++++--- .../lms/impl/moodle/MoodleCourseAccess.java | 45 ++++++++--------- .../lms/impl/moodle/MoodleLmsAPITemplate.java | 2 +- .../moodle/MoodleRestTemplateFactory.java | 12 +++-- .../session/ExamSessionService.java | 3 +- .../session/impl/ExamSessionCacheService.java | 3 +- .../session/impl/ExamSessionServiceImpl.java | 16 ++++-- .../api/ExamAdministrationController.java | 2 +- src/main/resources/messages.properties | 4 ++ 17 files changed, 150 insertions(+), 91 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 85410d7f..ae3f96b9 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 @@ -52,7 +52,10 @@ public class APIMessage implements Serializable { EXAM_CONSISTENCY_VALIDATION_CONFIG("1401", HttpStatus.OK, "No SEB Exam Configuration defined for the Exam"), EXAM_CONSISTENCY_VALIDATION_SEB_RESTRICTION("1402", HttpStatus.OK, "SEB restriction API available but Exam not restricted on LMS side yet"), - EXAM_CONSISTENCY_VALIDATION_INDICATOR("1403", HttpStatus.OK, "No Indicator defined for the Exam"); + 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"); public final String messageCode; public final HttpStatus httpStatus; diff --git a/src/main/java/ch/ethz/seb/sebserver/gbl/model/exam/Exam.java b/src/main/java/ch/ethz/seb/sebserver/gbl/model/exam/Exam.java index 193cbab1..d9a73d4c 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gbl/model/exam/Exam.java +++ b/src/main/java/ch/ethz/seb/sebserver/gbl/model/exam/Exam.java @@ -59,7 +59,9 @@ public final class Exam implements GrantEntity { public enum ExamStatus { UP_COMING, RUNNING, - FINISHED + FINISHED, + CORRUPT_NO_LMS_CONNECTION, + CORRUPT_INVALID_ID } public enum ExamType { diff --git a/src/main/java/ch/ethz/seb/sebserver/gbl/model/institution/LmsSetupTestResult.java b/src/main/java/ch/ethz/seb/sebserver/gbl/model/institution/LmsSetupTestResult.java index fe631d8b..f2a661f8 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gbl/model/institution/LmsSetupTestResult.java +++ b/src/main/java/ch/ethz/seb/sebserver/gbl/model/institution/LmsSetupTestResult.java @@ -82,6 +82,10 @@ public final class LmsSetupTestResult { .anyMatch(error -> error.errorType == type); } + public boolean hasAnyError() { + return !this.errors.isEmpty(); + } + @Override public String toString() { final StringBuilder builder = new StringBuilder(); 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 b4af2310..45ea48ff 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 @@ -35,11 +35,8 @@ 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.ProctoringSettings; import ch.ethz.seb.sebserver.gbl.model.exam.QuizData; -import ch.ethz.seb.sebserver.gbl.model.institution.LmsSetup; -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; import ch.ethz.seb.sebserver.gui.content.action.ActionDefinition; @@ -62,7 +59,6 @@ import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.exam.CheckSEBRest import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.exam.GetExam; import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.exam.GetProctoringSettings; 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; @@ -79,7 +75,7 @@ public class ExamForm implements TemplateComposer { private static final Logger log = LoggerFactory.getLogger(ExamForm.class); protected static final String ATTR_READ_GRANT = "ATTR_READ_GRANT"; - protected static final String ATTR_MODIFY_GRANT = "ATTR_MODIFY_GRANT"; + protected static final String ATTR_EDITABLE = "ATTR_EDITABLE"; protected static final String ATTR_EXAM_STATUS = "ATTR_EXAM_STATUS"; public static final LocTextKey EXAM_FORM_TITLE_KEY = @@ -118,6 +114,10 @@ public class ExamForm implements TemplateComposer { new LocTextKey("sebserver.exam.consistency.missing-config"); private final static LocTextKey CONSISTENCY_MESSAGE_MISSING_SEB_RESTRICTION = new LocTextKey("sebserver.exam.consistency.missing-seb-restriction"); + private final static LocTextKey CONSISTENCY_MESSAGE_VALIDATION_LMS_CONNECTION = + new LocTextKey("sebserver.exam.consistency.no-lms-connection"); + private final static LocTextKey CONSISTENCY_MESSAGEINVALID_ID_REFERENCE = + new LocTextKey("sebserver.exam.consistency.invalid-lms-id"); private final Map consistencyMessageMapping; private final PageService pageService; @@ -166,6 +166,12 @@ public class ExamForm implements TemplateComposer { this.consistencyMessageMapping.put( APIMessage.ErrorMessage.EXAM_CONSISTENCY_VALIDATION_SEB_RESTRICTION.messageCode, CONSISTENCY_MESSAGE_MISSING_SEB_RESTRICTION); + this.consistencyMessageMapping.put( + APIMessage.ErrorMessage.EXAM_CONSISTENCY_VALIDATION_LMS_CONNECTION.messageCode, + CONSISTENCY_MESSAGE_VALIDATION_LMS_CONNECTION); + this.consistencyMessageMapping.put( + APIMessage.ErrorMessage.EXAM_CONSISTENCY_VALIDATION_INVALID_ID_REFERENCE.messageCode, + CONSISTENCY_MESSAGEINVALID_ID_REFERENCE); } @Override @@ -218,9 +224,8 @@ public class ExamForm implements TemplateComposer { final boolean modifyGrant = userGrantCheck.m(); final boolean writeGrant = userGrantCheck.w(); final ExamStatus examStatus = exam.getStatus(); - final boolean editable = examStatus == ExamStatus.UP_COMING - || examStatus == ExamStatus.RUNNING - && currentUser.get().hasRole(UserRole.EXAM_ADMIN); + final boolean editable = modifyGrant && (examStatus == ExamStatus.UP_COMING || + examStatus == ExamStatus.RUNNING); final boolean sebRestrictionAvailable = testSEBRestrictionAPI(exam); final boolean isRestricted = readonly && sebRestrictionAvailable && this.restService .getBuilder(CheckSEBRestriction.class) @@ -387,12 +392,7 @@ public class ExamForm implements TemplateComposer { .newAction(ActionDefinition.EXAM_MODIFY_SEB_RESTRICTION_DETAILS) .withEntityKey(entityKey) .withExec(this.examSEBRestrictionSettings.settingsFunction(this.pageService)) - .withAttribute( - ExamSEBRestrictionSettings.PAGE_CONTEXT_ATTR_LMS_TYPE, - this.restService.getBuilder(GetLmsSetup.class) - .withURIVariable(API.PARAM_MODEL_ID, String.valueOf(exam.lmsSetupId)) - .call() - .getOrThrow().lmsType.name()) + .withAttribute(ExamSEBRestrictionSettings.PAGE_CONTEXT_ATTR_LMS_ID, String.valueOf(exam.lmsSetupId)) .withAttribute(PageContext.AttributeKeys.FORCE_READ_ONLY, String.valueOf(!modifyGrant)) .noEventPropagation() .publishIf(() -> sebRestrictionAvailable && readonly) @@ -433,7 +433,7 @@ public class ExamForm implements TemplateComposer { formContext .copyOf(content) .withAttribute(ATTR_READ_GRANT, String.valueOf(userGrantCheck.r())) - .withAttribute(ATTR_MODIFY_GRANT, String.valueOf(modifyGrant)) + .withAttribute(ATTR_EDITABLE, String.valueOf(editable)) .withAttribute(ATTR_EXAM_STATUS, examStatus.name())); // Indicators @@ -441,7 +441,7 @@ public class ExamForm implements TemplateComposer { formContext .copyOf(content) .withAttribute(ATTR_READ_GRANT, String.valueOf(userGrantCheck.r())) - .withAttribute(ATTR_MODIFY_GRANT, String.valueOf(modifyGrant)) + .withAttribute(ATTR_EDITABLE, String.valueOf(editable)) .withAttribute(ATTR_EXAM_STATUS, examStatus.name())); } } @@ -467,11 +467,7 @@ public class ExamForm implements TemplateComposer { } private boolean testSEBRestrictionAPI(final Exam exam) { - final Result lmsSetupCall = this.restService.getBuilder(GetLmsSetup.class) - .withURIVariable(API.PARAM_MODEL_ID, String.valueOf(exam.lmsSetupId)) - .call(); - - if (!lmsSetupCall.hasError() && !lmsSetupCall.get().lmsType.features.contains(Features.SEB_RESTRICTION)) { + if (exam.status == ExamStatus.CORRUPT_NO_LMS_CONNECTION || exam.status == ExamStatus.CORRUPT_INVALID_ID) { return false; } diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/content/ExamFormConfigs.java b/src/main/java/ch/ethz/seb/sebserver/gui/content/ExamFormConfigs.java index b7c20c20..a8e54d38 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/content/ExamFormConfigs.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/content/ExamFormConfigs.java @@ -26,7 +26,6 @@ import ch.ethz.seb.sebserver.gbl.model.Domain; import ch.ethz.seb.sebserver.gbl.model.EntityKey; 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.user.UserRole; import ch.ethz.seb.sebserver.gbl.profile.GuiProfile; import ch.ethz.seb.sebserver.gui.content.action.ActionDefinition; import ch.ethz.seb.sebserver.gui.service.ResourceService; @@ -41,7 +40,6 @@ import ch.ethz.seb.sebserver.gui.service.remote.download.DownloadService; import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.RestService; import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.exam.DeleteExamConfigMapping; import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.exam.GetExamConfigMappingsPage; -import ch.ethz.seb.sebserver.gui.service.remote.webservice.auth.CurrentUser; import ch.ethz.seb.sebserver.gui.table.ColumnDefinition; import ch.ethz.seb.sebserver.gui.table.EntityTable; import ch.ethz.seb.sebserver.gui.widget.WidgetFactory; @@ -89,20 +87,15 @@ public class ExamFormConfigs implements TemplateComposer { @Override public void compose(final PageContext pageContext) { - final CurrentUser currentUser = this.resourceService.getCurrentUser(); final Composite content = pageContext.getParent(); - final EntityKey entityKey = pageContext.getEntityKey(); - final boolean modifyGrant = BooleanUtils.toBoolean( - pageContext.getAttribute(ExamForm.ATTR_MODIFY_GRANT)); + final boolean editable = BooleanUtils.toBoolean( + pageContext.getAttribute(ExamForm.ATTR_EDITABLE)); final boolean readGrant = BooleanUtils.toBoolean( pageContext.getAttribute(ExamForm.ATTR_READ_GRANT)); final ExamStatus examStatus = ExamStatus.valueOf( pageContext.getAttribute(ExamForm.ATTR_EXAM_STATUS)); final boolean isExamRunning = examStatus == ExamStatus.RUNNING; - final boolean editable = examStatus == ExamStatus.UP_COMING - || examStatus == ExamStatus.RUNNING - && currentUser.get().hasRole(UserRole.EXAM_ADMIN); // List of SEB Configuration this.widgetFactory.addFormSubContextHeader( @@ -134,7 +127,7 @@ public class ExamFormConfigs implements TemplateComposer { this.resourceService::localizedExamConfigStatusName) .widthProportion(1)) .withDefaultActionIf( - () -> true, + () -> readGrant, this::viewExamConfigPageAction) .withSelectionListener(this.pageService.getSelectionPublisher( @@ -162,12 +155,12 @@ public class ExamFormConfigs implements TemplateComposer { .withParentEntityKey(entityKey) .withExec(this.examToConfigBindingForm.bindFunction()) .noEventPropagation() - .publishIf(() -> modifyGrant && editable && !configurationTable.hasAnyContent()) + .publishIf(() -> editable && !configurationTable.hasAnyContent()) .newAction(ActionDefinition.EXAM_CONFIGURATION_EXAM_CONFIG_VIEW_PROP) .withParentEntityKey(entityKey) .withEntityKey(configMapKey) - .publishIf(() -> modifyGrant && configurationTable.hasAnyContent(), false) + .publishIf(() -> readGrant && configurationTable.hasAnyContent(), false) .newAction(ActionDefinition.EXAM_CONFIGURATION_DELETE_FROM_LIST) .withEntityKey(entityKey) @@ -181,7 +174,7 @@ public class ExamFormConfigs implements TemplateComposer { } return null; }) - .publishIf(() -> modifyGrant && configurationTable.hasAnyContent() && editable, false) + .publishIf(() -> editable && configurationTable.hasAnyContent() && editable, false) .newAction(ActionDefinition.EXAM_CONFIGURATION_GET_CONFIG_KEY) .withSelect( diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/content/ExamFormIndicators.java b/src/main/java/ch/ethz/seb/sebserver/gui/content/ExamFormIndicators.java index ece120cf..6c0b858a 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/content/ExamFormIndicators.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/content/ExamFormIndicators.java @@ -72,8 +72,8 @@ public class ExamFormIndicators implements TemplateComposer { public void compose(final PageContext pageContext) { final Composite content = pageContext.getParent(); final EntityKey entityKey = pageContext.getEntityKey(); - final boolean modifyGrant = BooleanUtils.toBoolean( - pageContext.getAttribute(ExamForm.ATTR_MODIFY_GRANT)); + final boolean editable = BooleanUtils.toBoolean( + pageContext.getAttribute(ExamForm.ATTR_EDITABLE)); // List of Indicators this.widgetFactory.addFormSubContextHeader( @@ -111,7 +111,7 @@ public class ExamFormIndicators implements TemplateComposer { .asMarkup() .widthProportion(4)) .withDefaultActionIf( - () -> modifyGrant, + () -> editable, () -> actionBuilder .newAction(ActionDefinition.EXAM_INDICATOR_MODIFY_FROM_LIST) .withParentEntityKey(entityKey) @@ -132,7 +132,7 @@ public class ExamFormIndicators implements TemplateComposer { indicatorTable::getSelection, PageAction::applySingleSelectionAsEntityKey, INDICATOR_EMPTY_SELECTION_TEXT_KEY) - .publishIf(() -> modifyGrant && indicatorTable.hasAnyContent(), false) + .publishIf(() -> editable && indicatorTable.hasAnyContent(), false) .newAction(ActionDefinition.EXAM_INDICATOR_DELETE_FROM_LIST) .withEntityKey(entityKey) @@ -140,11 +140,11 @@ public class ExamFormIndicators implements TemplateComposer { indicatorTable::getSelection, this::deleteSelectedIndicator, INDICATOR_EMPTY_SELECTION_TEXT_KEY) - .publishIf(() -> modifyGrant && indicatorTable.hasAnyContent(), false) + .publishIf(() -> editable && indicatorTable.hasAnyContent(), false) .newAction(ActionDefinition.EXAM_INDICATOR_NEW) .withParentEntityKey(entityKey) - .publishIf(() -> modifyGrant); + .publishIf(() -> editable); } diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/content/ExamList.java b/src/main/java/ch/ethz/seb/sebserver/gui/content/ExamList.java index e7be7a46..4b066b8c 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/content/ExamList.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/content/ExamList.java @@ -254,7 +254,7 @@ public class ExamList implements TemplateComposer { final Exam exam, final PageService pageService) { - if (exam.getStatus() != ExamStatus.RUNNING) { + if (exam.getStatus() == ExamStatus.UP_COMING || exam.getStatus() == ExamStatus.FINISHED) { return; } 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 ac63677c..78e99f4e 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 @@ -51,6 +51,7 @@ import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.exam.DeactivateSE import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.exam.GetCourseChapters; import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.exam.GetSEBRestrictionSettings; import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.exam.SaveSEBRestriction; +import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.lmssetup.GetLmsSetup; @Lazy @Component @@ -81,7 +82,7 @@ public class ExamSEBRestrictionSettings { private final static LocTextKey SEB_RESTRICTION_FORM_EDX_USER_BANNING_ENABLED = new LocTextKey("sebserver.exam.form.sebrestriction.USER_BANNING_ENABLED"); - static final String PAGE_CONTEXT_ATTR_LMS_TYPE = "ATTR_LMS_TYPE"; + static final String PAGE_CONTEXT_ATTR_LMS_ID = "ATTR_LMS_ID"; Function settingsFunction(final PageService pageService) { @@ -126,7 +127,7 @@ public class ExamSEBRestrictionSettings { } final EntityKey entityKey = pageContext.getEntityKey(); - final LmsType lmsType = getLmsType(pageContext); + final LmsType lmsType = getLmsType(pageService, pageContext.getAttribute(PAGE_CONTEXT_ATTR_LMS_ID)); SEBRestriction bodyValue = null; try { final Form form = formHandle.getForm(); @@ -194,7 +195,9 @@ public class ExamSEBRestrictionSettings { final RestService restService = this.pageService.getRestService(); final ResourceService resourceService = this.pageService.getResourceService(); final EntityKey entityKey = this.pageContext.getEntityKey(); - final LmsType lmsType = getLmsType(this.pageContext); + final LmsType lmsType = getLmsType( + this.pageService, + this.pageContext.getAttribute(PAGE_CONTEXT_ATTR_LMS_ID)); final boolean isReadonly = BooleanUtils.toBoolean( this.pageContext.getAttribute(PageContext.AttributeKeys.FORCE_READ_ONLY)); @@ -305,9 +308,16 @@ public class ExamSEBRestrictionSettings { } - private LmsType getLmsType(final PageContext pageContext) { + private LmsType getLmsType(final PageService pageService, final String lmsSetupId) { try { - return LmsType.valueOf(pageContext.getAttribute(PAGE_CONTEXT_ATTR_LMS_TYPE)); + + return pageService + .getRestService() + .getBuilder(GetLmsSetup.class) + .withURIVariable(API.PARAM_MODEL_ID, lmsSetupId) + .call() + .getOrThrow().lmsType; + } catch (final Exception e) { return null; } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ExamDAOImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ExamDAOImpl.java index 3ded3a19..9f9f355c 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ExamDAOImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ExamDAOImpl.java @@ -102,7 +102,7 @@ public class ExamDAOImpl implements ExamDAO { @Transactional(readOnly = true) public Result examGrantEntityByPK(final Long id) { return recordById(id) - .map(record -> toDomainModel(record, null).getOrThrow()); + .map(record -> toDomainModel(record, null, null).getOrThrow()); } @Override @@ -111,7 +111,7 @@ public class ExamDAOImpl implements ExamDAO { return Result.tryCatch(() -> this.clientConnectionRecordMapper .selectByPrimaryKey(connectionId)) .flatMap(ccRecord -> recordById(ccRecord.getExamId())) - .map(record -> toDomainModel(record, null).getOrThrow()); + .map(record -> toDomainModel(record, null, null).getOrThrow()); } @Override @@ -793,17 +793,48 @@ public class ExamDAOImpl implements ExamDAO { final Map quizzes = this.lmsAPIService .getLmsAPITemplate(lmsSetupId) .map(template -> getQuizzesFromLMS(template, recordMapping.keySet(), cached)) - .getOrThrow() + .onError(error -> log.error("Failed to get quizzes for exams: ", error)) + .getOr(Collections.emptyList()) .stream() .flatMap(Result::skipOnError) .collect(Collectors.toMap(q -> q.id, Function.identity())); + if (records.size() != quizzes.size()) { + + // Check if we have LMS connection to verify the source of the exam quiz mismatch + final LmsAPITemplate lmsSetup = this.lmsAPIService + .getLmsAPITemplate(lmsSetupId) + .getOrThrow(); + + if (log.isDebugEnabled()) { + log.debug("Quizzes size mismatch detected by getting exams quiz data from LMS: {}", lmsSetup); + } + + if (lmsSetup.testCourseAccessAPI().hasAnyError()) { + // No course access on the LMS. This means we can't get any quizzes from this LMSSetup at the moment + // All exams are marked as corrupt because of LMS Setup failure + + log.warn("Failed to get quizzes form LMS Setup. No access to LMS {}", lmsSetup); + + return recordMapping.entrySet() + .stream() + .map(entry -> toDomainModel( + entry.getValue(), + null, + ExamStatus.CORRUPT_NO_LMS_CONNECTION) + .getOr(null)) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + } + } + // collect Exam's return recordMapping.entrySet() .stream() .map(entry -> toDomainModel( entry.getValue(), - getQuizData(quizzes, entry.getKey(), entry.getValue())) + getQuizData(quizzes, entry.getKey(), entry.getValue()), + ExamStatus.CORRUPT_INVALID_ID) .onError(error -> log.error( "Failed to get quiz data from remote LMS for exam: ", error)) @@ -813,8 +844,11 @@ public class ExamDAOImpl implements ExamDAO { }); } - private Collection> getQuizzesFromLMS(final LmsAPITemplate template, final Set ids, + private Collection> getQuizzesFromLMS( + final LmsAPITemplate template, + final Set ids, final boolean cached) { + try { return (cached) ? template.getQuizzesFromCache(ids) @@ -840,6 +874,7 @@ public class ExamDAOImpl implements ExamDAO { // a case by using the short name of the quiz and search for the quiz within the course with this // short name. If one quiz has been found that matches all criteria, we adapt the internal id // mapping to this quiz. + // If recovering fails, this returns null and the calling side must handle the lack of quiz data try { final LmsSetup lmsSetup = this.lmsAPIService .getLmsSetup(record.getLmsSetupId()) @@ -914,7 +949,8 @@ public class ExamDAOImpl implements ExamDAO { private Result toDomainModel( final ExamRecord record, - final QuizData quizData) { + final QuizData quizData, + final ExamStatus statusOverride) { return Result.tryCatch(() -> { @@ -943,7 +979,7 @@ public class ExamDAOImpl implements ExamDAO { ExamType.valueOf(record.getType()), record.getOwner(), supporter, - status, + (quizData != null) ? status : (statusOverride != null) ? statusOverride : status, record.getBrowserKeys(), BooleanUtils.toBooleanObject((quizData != null) ? record.getActive() : null), record.getLastupdate()); diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleCourseAccess.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleCourseAccess.java index 9b2d7298..2d192400 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleCourseAccess.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleCourseAccess.java @@ -191,7 +191,7 @@ public class MoodleCourseAccess extends CourseAccess { if (restTemplateRequest.hasError()) { final String message = "Failed to gain access token from Moodle Rest API:\n tried token endpoints: " + this.moodleRestTemplateFactory.knownTokenAccessPaths; - log.error(message + " cause: ", restTemplateRequest.getError()); + log.error(message + " cause: {}", restTemplateRequest.getError().getMessage()); return LmsSetupTestResult.ofTokenRequestError(message); } @@ -213,14 +213,14 @@ public class MoodleCourseAccess extends CourseAccess { protected Supplier> quizzesSupplier(final Set ids) { return () -> getRestTemplate() .map(template -> getQuizzesForIds(template, ids)) - .getOrThrow(); + .getOr(Collections.emptyList()); } protected Supplier> allQuizzesSupplier(final FilterMap filterMap) { return () -> getRestTemplate() .map(template -> collectAllQuizzes(template, filterMap)) - .getOrThrow(); + .getOr(Collections.emptyList()); } @Override @@ -380,17 +380,7 @@ public class MoodleCourseAccess extends CourseAccess { return Collections.emptyList(); } - if (courseQuizData.warnings != null && !courseQuizData.warnings.isEmpty()) { - log.warn( - "There are warnings from Moodle response: Moodle: {} request: {} warnings: {} warning sample: {}", - this.lmsSetup, - MoodleCourseAccess.MOODLE_QUIZ_API_FUNCTION_NAME, - courseQuizData.warnings.size(), - courseQuizData.warnings.iterator().next().toString()); - if (log.isTraceEnabled()) { - log.trace("All warnings from Moodle: {}", courseQuizData.warnings.toString()); - } - } + logMoodleWarnings(courseQuizData.warnings); if (courseQuizData.quizzes == null || courseQuizData.quizzes.isEmpty()) { log.error("No quizzes found for ids: {} on LMS; {}", quizIds, this.lmsSetup.name); @@ -461,17 +451,7 @@ public class MoodleCourseAccess extends CourseAccess { Collections.emptyList(); } - if (courses.warnings != null && !courses.warnings.isEmpty()) { - log.warn( - "There are warnings from Moodle response: Moodle: {} request: {} warnings: {} warning sample: {}", - this.lmsSetup, - MoodleCourseAccess.MOODLE_COURSE_BY_FIELD_API_FUNCTION_NAME, - courses.warnings.size(), - courses.warnings.iterator().next().toString()); - if (log.isTraceEnabled()) { - log.trace("All warnings from Moodle: {}", courses.warnings.toString()); - } - } + logMoodleWarnings(courses.warnings); if (courses.courses == null || courses.courses.isEmpty()) { log.error("No courses found for ids: {} on LMS: {}", ids, this.lmsSetup.name); @@ -643,6 +623,21 @@ public class MoodleCourseAccess extends CourseAccess { return idNumber.equals(Constants.EMPTY_NOTE) ? null : idNumber; } + private void logMoodleWarnings(final Collection warnings) { + if (warnings != null && !warnings.isEmpty()) { + if (log.isDebugEnabled()) { + log.debug( + "There are warnings from Moodle response: Moodle: {} request: {} warnings: {} warning sample: {}", + this.lmsSetup, + MoodleCourseAccess.MOODLE_QUIZ_API_FUNCTION_NAME, + warnings.size(), + warnings.iterator().next().toString()); + } else if (log.isTraceEnabled()) { + log.trace("All warnings from Moodle: {}", warnings.toString()); + } + } + } + private static final Pattern ACCESS_DENIED_PATTERN_1 = Pattern.compile(Pattern.quote("No access rights"), Pattern.CASE_INSENSITIVE); private static final Pattern ACCESS_DENIED_PATTERN_2 = 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 8cd903bc..e37ad8a1 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 @@ -117,7 +117,7 @@ public class MoodleLmsAPITemplate implements LmsAPITemplate { @Override public Result getSEBClientRestriction(final Exam exam) { if (log.isDebugEnabled()) { - log.debug("Get SEB Client restriction for Exam: {}", exam); + log.debug("Get SEB Client restriction for Exam: {}", exam.externalId); } return this.moodleCourseRestriction diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleRestTemplateFactory.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleRestTemplateFactory.java index 13cbd60f..605bc37b 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleRestTemplateFactory.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleRestTemplateFactory.java @@ -128,14 +128,18 @@ class MoodleRestTemplateFactory { .map(this::createRestTemplate) .map(result -> { if (result.hasError()) { - log.error("Failed to get access token: ", result.getError()); + log.warn("Failed to get access token for LMS: {}({})", + this.lmsSetup.name, + this.lmsSetup.id); } return result; }) .filter(Result::hasValue) .findFirst() .orElse(Result.ofRuntimeError( - "Failed to gain any access on paths: " + this.knownTokenAccessPaths)); + "Failed to gain any access for LMS " + + this.lmsSetup.name + "(" + this.lmsSetup.id + + ") on paths: " + this.knownTokenAccessPaths)); } Result createRestTemplate(final String accessTokenPath) { @@ -146,7 +150,9 @@ class MoodleRestTemplateFactory { final CharSequence accessToken = template.getAccessToken(); if (accessToken == null) { - throw new RuntimeException("Failed to gain access token on path: " + accessTokenPath); + throw new RuntimeException("Failed to get access token for LMS " + + this.lmsSetup.name + "(" + this.lmsSetup.id + + ") on path: " + accessTokenPath); } return template; diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/ExamSessionService.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/ExamSessionService.java index 7163b65c..afdcd698 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/ExamSessionService.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/ExamSessionService.java @@ -64,6 +64,7 @@ public interface ExamSessionService { /** Use this to check the consistency of a running Exam. * Current consistency checks are: * - Check if there is at least one Exam supporter attached to the Exam + * - Check if we have access to LMS for the exam * - Check if there is one default SEB Exam Configuration attached to the Exam * - Check if SEB restriction API is available and the exam is running but not yet restricted on LMS side * - Check if there is at least one Indicator defined for the monitoring of the Exam @@ -71,7 +72,7 @@ public interface ExamSessionService { * @param examId the identifier of the Exam to check * @return Result of one APIMessage per consistency check if the check failed. An empty Collection of everything is * okay. */ - Result> checkRunningExamConsistency(Long examId); + Result> checkExamConsistency(Long examId); /** Use this to check if a specified Exam has currently active SEB Client connections. * diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamSessionCacheService.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamSessionCacheService.java index 67a531f1..b26c2434 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamSessionCacheService.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamSessionCacheService.java @@ -133,7 +133,8 @@ public class ExamSessionCacheService { case RUNNING: { return true; } - case UP_COMING: { + case UP_COMING: + case FINISHED: { return this.examUpdateHandler.updateRunning(exam.id) .map(e -> e.status == ExamStatus.RUNNING) .getOr(false); diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamSessionServiceImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamSessionServiceImpl.java index f940f8c6..edd56c82 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamSessionServiceImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamSessionServiceImpl.java @@ -121,14 +121,22 @@ public class ExamSessionServiceImpl implements ExamSessionService { } @Override - public Result> checkRunningExamConsistency(final Long examId) { + public Result> checkExamConsistency(final Long examId) { return Result.tryCatch(() -> { final Collection result = new ArrayList<>(); - if (isExamRunning(examId)) { - final Exam exam = getRunningExam(examId) - .getOrThrow(); + final Exam exam = this.examDAO.byPK(examId) + .getOrThrow(); + // check lms connection + if (exam.status == ExamStatus.CORRUPT_NO_LMS_CONNECTION) { + result.add(ErrorMessage.EXAM_CONSISTENCY_VALIDATION_LMS_CONNECTION.of(exam.getModelId())); + } + if (exam.status == ExamStatus.CORRUPT_INVALID_ID) { + result.add(ErrorMessage.EXAM_CONSISTENCY_VALIDATION_INVALID_ID_REFERENCE.of(exam.getModelId())); + } + + if (exam.status == ExamStatus.RUNNING) { // check exam supporter if (exam.getSupporter().isEmpty()) { result.add(ErrorMessage.EXAM_CONSISTENCY_VALIDATION_SUPPORTER.of(exam.getModelId())); diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamAdministrationController.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamAdministrationController.java index 8d328d45..6e8d9e56 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamAdministrationController.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamAdministrationController.java @@ -246,7 +246,7 @@ public class ExamAdministrationController extends EntityController { checkReadPrivilege(institutionId); return this.examSessionService - .checkRunningExamConsistency(modelId) + .checkExamConsistency(modelId) .getOrThrow(); } diff --git a/src/main/resources/messages.properties b/src/main/resources/messages.properties index d7ea6256..5c0437a7 100644 --- a/src/main/resources/messages.properties +++ b/src/main/resources/messages.properties @@ -430,6 +430,8 @@ sebserver.exam.consistency.missing-supporter= - There are no Exam Supporter defi sebserver.exam.consistency.missing-indicator= - There is no indicator defined for this exam. Use 'Add Indicator" on the right to add an indicator. sebserver.exam.consistency.missing-config= - There is no configuration defined for this exam. Use 'Add Configuration' to attach one. sebserver.exam.consistency.missing-seb-restriction= - There is currently no SEB restriction applied on the LMS side. Use 'Enable SEB Restriction' on the right to activate auto-restriction.
Or if this is not possible consider doing it manually on the LMS. +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.action.list=Exam @@ -530,6 +532,8 @@ sebserver.exam.type.VDI.tooltip=Exam type specified for Virtual Desktop Infrastr sebserver.exam.status.UP_COMING=Up Coming sebserver.exam.status.RUNNING=Running sebserver.exam.status.FINISHED=Finished +sebserver.exam.status.CORRUPT_NO_LMS_CONNECTION=Corrupt (No LMS Connection) +sebserver.exam.status.CORRUPT_INVALID_ID=Corrupt (Invalid Identifier) sebserver.exam.configuration.list.actions= sebserver.exam.configuration.list.title=Exam Configuration