From a896737f5bdc595de78c776dd30f9877e92754b9 Mon Sep 17 00:00:00 2001 From: anhefti Date: Tue, 9 May 2023 14:13:09 +0200 Subject: [PATCH 1/3] patch fixes monitoring sorting and zoom token refresh --- .../session/ClientConnectionTable.java | 35 +++++++++++------- .../gui/service/session/ColorData.java | 11 +----- .../proctoring/ZoomProctoringService.java | 37 +++++++++++++++++-- 3 files changed, 58 insertions(+), 25 deletions(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/session/ClientConnectionTable.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/session/ClientConnectionTable.java index 581da9f0..9e766723 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/session/ClientConnectionTable.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/session/ClientConnectionTable.java @@ -15,6 +15,7 @@ import java.util.Comparator; import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Set; @@ -69,8 +70,6 @@ public final class ClientConnectionTable implements FullPageMonitoringGUIUpdate private static final Logger log = LoggerFactory.getLogger(ClientConnectionTable.class); private static final int BOTTOM_PADDING = 20; - //private static final int[] TABLE_PROPORTIONS = new int[] { 3, 3, 2, 1 }; - //private static final int NUMBER_OF_NONE_INDICATOR_COLUMNS = 3; private static final String INDICATOR_NAME_TEXT_KEY_PREFIX = "sebserver.exam.indicator.type.description."; @@ -101,12 +100,13 @@ public final class ClientConnectionTable implements FullPageMonitoringGUIUpdate private final Map clientGroupMapping; private final Table table; private final ColorData colorData; + private final List sortList = new ArrayList<>(); private final Function localizedClientConnectionStatusNameFunction; private Consumer selectionListener; private int tableWidth; private boolean needsSort = false; - private LinkedHashMap tableMapping; + private final LinkedHashMap tableMapping; private final Set toDelete = new HashSet<>(); private final Set toUpdateStatic = new HashSet<>(); private final Set duplicates = new HashSet<>(); @@ -432,13 +432,15 @@ public final class ClientConnectionTable implements FullPageMonitoringGUIUpdate } private void sortTable() { - this.tableMapping = this.tableMapping.entrySet() - .stream() - .sorted(Entry.comparingByValue()) - .collect(Collectors.toMap( - Entry::getKey, - Entry::getValue, - (e1, e2) -> e1, LinkedHashMap::new)); + this.sortList.clear(); + this.sortList.addAll(this.tableMapping.values()); + Collections.sort(this.sortList); + this.tableMapping.clear(); + final Iterator iterator = this.sortList.iterator(); + while (iterator.hasNext()) { + final UpdatableTableItem item = iterator.next(); + this.tableMapping.put(item.connectionId, item); + } } private void notifySelectionChange() { @@ -494,7 +496,7 @@ public final class ClientConnectionTable implements FullPageMonitoringGUIUpdate @Override public boolean sebVersionDenied() { - return this.monitoringData.sebVersionDenied; + return (this.monitoringData == null) ? false : this.monitoringData.sebVersionDenied; } @Override @@ -559,7 +561,7 @@ public final class ClientConnectionTable implements FullPageMonitoringGUIUpdate } private void updateNotifications(final TableItem tableItem) { - if (BooleanUtils.isTrue(this.monitoringData.pendingNotification)) { + if (this.monitoringData != null && BooleanUtils.isTrue(this.monitoringData.pendingNotification)) { tableItem.setImage(0, WidgetFactory.ImageIcon.NOTIFICATION.getImage(ClientConnectionTable.this.table.getDisplay())); tableItem.setBackground(0, ClientConnectionTable.this.colorData.color2); @@ -595,7 +597,7 @@ public final class ClientConnectionTable implements FullPageMonitoringGUIUpdate final Long id = entry.getKey(); final String displayValue = entry.getValue(); final IndicatorData indicatorData = ClientConnectionTable.this.indicatorMapping.get(id); - if (indicatorData == null) { + if (indicatorData == null || this.monitoringData == null) { return; } @@ -654,6 +656,9 @@ public final class ClientConnectionTable implements FullPageMonitoringGUIUpdate } int notificationWeight() { + if (this.monitoringData == null) { + return 0; + } return BooleanUtils.isTrue(this.monitoringData.pendingNotification) || (this.monitoringData.status.establishedStatus && this.marked) ? -1 : 0; } @@ -663,6 +668,9 @@ public final class ClientConnectionTable implements FullPageMonitoringGUIUpdate } int thresholdsWeight() { + if (this.monitoringData != null && !this.monitoringData.status.clientActiveStatus) { + return 0; + } return -this.thresholdsWeight; } @@ -690,6 +698,7 @@ public final class ClientConnectionTable implements FullPageMonitoringGUIUpdate } String getConnectionIdentifier() { + if (this.staticData != null && this.staticData.userSessionId != null) { return this.staticData.userSessionId; } diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/session/ColorData.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/session/ColorData.java index 52bf90b6..dd857e6f 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/session/ColorData.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/session/ColorData.java @@ -69,20 +69,13 @@ public class ColorData { switch (entry.getStatus()) { case CONNECTION_REQUESTED: case AUTHENTICATED: { - if (entry.incidentFlag() > 0) { - return -1; - } - return 1; + return 0; } case ACTIVE: { - final int incidentFlag = entry.incidentFlag(); - if (incidentFlag > 0) { - return -incidentFlag; - } return 2; } case CLOSED: - return 4; + return 3; default: return 5; } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/proctoring/ZoomProctoringService.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/proctoring/ZoomProctoringService.java index 5206c7b0..38b3168c 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/proctoring/ZoomProctoringService.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/proctoring/ZoomProctoringService.java @@ -1016,6 +1016,37 @@ public class ZoomProctoringService implements ExamProctoringService { } } + @Override + boolean isValid(final ProctoringServiceSettings proctoringSettings) { + final boolean valid = super.isValid(proctoringSettings); + if (!valid) { + return false; + } + + try { + final OAuth2RestTemplate oAuth2RestTemplate = (OAuth2RestTemplate) super.restTemplate; + final OAuth2AccessToken accessToken = oAuth2RestTemplate.getAccessToken(); + if (accessToken == null) { + return false; + } + + final boolean expired = accessToken.isExpired(); + if (expired) { + return false; + } + + final int expiresIn = accessToken.getExpiresIn(); + if (expiresIn < 60) { + return false; + } + + return true; + } catch (final Exception e) { + log.error("Failed to verify Zoom OAuth2RestTemplate status", e); + return false; + } + } + @Override public HttpHeaders getHeaders() { final HttpHeaders httpHeaders = new HttpHeaders(); @@ -1123,14 +1154,15 @@ public class ZoomProctoringService implements ExamProctoringService { @Override public boolean supportsRefresh(final OAuth2ProtectedResourceDetails resource) { - return false; + return true; } @Override public OAuth2AccessToken refreshAccessToken(final OAuth2ProtectedResourceDetails resource, final OAuth2RefreshToken refreshToken, final AccessTokenRequest request) throws UserRedirectRequiredException { - return null; + + return this.obtainAccessToken(resource, request); } @Override @@ -1140,7 +1172,6 @@ public class ZoomProctoringService implements ExamProctoringService { final ClientCredentialsResourceDetails resource = (ClientCredentialsResourceDetails) details; return retrieveToken(request, resource, getParametersForTokenRequest(resource), new HttpHeaders()); - } private MultiValueMap getParametersForTokenRequest( From d50e818d6e79b72492483db2bad493893cf1ac7b Mon Sep 17 00:00:00 2001 From: anhefti Date: Thu, 11 May 2023 11:05:23 +0200 Subject: [PATCH 2/3] SEBSERV-444 fixed --- .../gui/service/i18n/impl/PolyglotPageServiceImpl.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/i18n/impl/PolyglotPageServiceImpl.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/i18n/impl/PolyglotPageServiceImpl.java index 1ddf4989..c642653e 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/i18n/impl/PolyglotPageServiceImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/i18n/impl/PolyglotPageServiceImpl.java @@ -11,7 +11,9 @@ package ch.ethz.seb.sebserver.gui.service.i18n.impl; import java.util.Locale; import java.util.function.Consumer; +import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.text.StringEscapeUtils; import org.eclipse.rap.rwt.RWT; import org.eclipse.swt.SWT; import org.eclipse.swt.widgets.Button; @@ -269,8 +271,14 @@ public final class PolyglotPageServiceImpl implements PolyglotPageService { final I18nSupport i18nSupport) { return label -> { + if (locTextKey != null) { - label.setText(i18nSupport.getText(locTextKey)); + final String text = i18nSupport.getText(locTextKey); + if (BooleanUtils.toBoolean((Boolean) label.getData(RWT.MARKUP_ENABLED))) { + label.setText(StringEscapeUtils.escapeHtml4(text)); + } else { + label.setText(text); + } } if (i18nSupport.hasText(locToolTipKey)) { label.setToolTipText(Utils.formatLineBreaks(i18nSupport.getText(locToolTipKey))); From 4e182c94e3e7966404e123cf87297c016c4599f1 Mon Sep 17 00:00:00 2001 From: anhefti Date: Mon, 15 May 2023 09:06:13 +0200 Subject: [PATCH 3/3] better error handling and logging --- .../gui/service/push/UpdateErrorHandler.java | 4 ++-- .../session/impl/ExamSessionServiceImpl.java | 4 +++- .../session/impl/SEBClientConnectionServiceImpl.java | 4 ++-- .../impl/SEBClientNotificationServiceImpl.java | 5 ++--- .../webservice/weblayer/api/APIExceptionHandler.java | 11 ++++++++++- .../weblayer/api/ExamMonitoringController.java | 5 ++++- .../gui/integration/UseCasesIntegrationTest.java | 4 ++-- 7 files changed, 25 insertions(+), 12 deletions(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/push/UpdateErrorHandler.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/push/UpdateErrorHandler.java index c453c9bc..35790109 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/push/UpdateErrorHandler.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/push/UpdateErrorHandler.java @@ -56,14 +56,14 @@ public final class UpdateErrorHandler implements Function { } catch (final Exception ee) { log.warn("Unable to auto-logout: ", ee.getMessage()); } - return true; + return false; } } @Override public Boolean apply(final Exception error) { this.errors++; - log.error("Failed to update server push: {}", error.getMessage(), error); + log.warn("Failed to update server push: {}", error.getMessage()); if (this.errors > 5) { checkUserSession(); } 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 d49e8c4d..d8d89ff9 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 @@ -241,7 +241,9 @@ public class ExamSessionServiceImpl implements ExamSessionService { flushCache(exam); } - log.info("Exam {} is not currently running", examId); + if (log.isDebugEnabled()) { + log.info("Exam {} is not currently running", examId); + } return Result.ofError(new NoSuchElementException( "No currently running exam found for id: " + examId)); 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 ab726f6c..d5a80ccd 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 @@ -329,7 +329,7 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic if (StringUtils.isNoneBlank(clientAddress) && StringUtils.isNotBlank(clientConnection.clientAddress) && !clientAddress.equals(clientConnection.clientAddress)) { - log.error( + log.warn( "ClientConnection integrity violation: client address mismatch: {}, {}", clientAddress, clientConnection.clientAddress); @@ -337,7 +337,7 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic "ClientConnection integrity violation: client address mismatch"); } } else if (!clientConnection.status.clientActiveStatus) { - log.error("ClientConnection integrity violation: client connection is not in expected state: {}", + log.warn("ClientConnection integrity violation: client connection is not in expected state: {}", clientConnection); throw new IllegalArgumentException( "ClientConnection integrity violation: client connection is not in expected state"); diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/SEBClientNotificationServiceImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/SEBClientNotificationServiceImpl.java index 89a1c31f..6c2decb6 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/SEBClientNotificationServiceImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/SEBClientNotificationServiceImpl.java @@ -93,7 +93,7 @@ public class SEBClientNotificationServiceImpl implements SEBClientNotificationSe this.clientEventDAO.getPendingNotificationByValue(clientConnection.id, notificationId) .flatMap(notification -> this.clientEventDAO.confirmPendingNotification(notification.id)) .map(this::removeFromCache) - .onError(error -> log.error("Failed to confirm pending notification: {}", event, error)); + .getOrThrow(); } catch (final Exception e) { log.error( @@ -110,8 +110,7 @@ public class SEBClientNotificationServiceImpl implements SEBClientNotificationSe return this.clientEventDAO.getPendingNotification(notificationId) .map(notification -> this.confirmClientSide(notification, examId, connectionToken)) .flatMap(notification -> this.clientEventDAO.confirmPendingNotification(notificationId)) - .map(this::removeFromCache) - .onError(error -> log.error("Failed to confirm pending notification: {}", notificationId, error)); + .map(this::removeFromCache); } @Override diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/APIExceptionHandler.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/APIExceptionHandler.java index 4d0581d2..d8f02488 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/APIExceptionHandler.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/APIExceptionHandler.java @@ -149,6 +149,15 @@ public class APIExceptionHandler extends ResponseEntityExceptionHandler { HttpStatus.BAD_REQUEST); } + @ExceptionHandler(IllegalArgumentException.class) + public ResponseEntity handleIllegalArgumentException( + final IllegalArgumentException ex, + final WebRequest request) { + + log.warn("Illegal argument or state detected: {}\n send 400 Bad Request response", ex.getMessage()); + return new ResponseEntity<>(HttpStatus.BAD_REQUEST); + } + @ExceptionHandler(ResourceNotFoundException.class) public ResponseEntity handleResourceNotFoundException( final ResourceNotFoundException ex, @@ -182,7 +191,7 @@ public class APIExceptionHandler extends ResponseEntityExceptionHandler { final ExamNotRunningException ex, final WebRequest request) { - log.info("{}", ex.getMessage()); + log.debug("{}", ex.getMessage()); return APIMessage.ErrorMessage.INTEGRITY_VALIDATION .createErrorResponse(ex.getMessage()); } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamMonitoringController.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamMonitoringController.java index 81c790e9..36aec030 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamMonitoringController.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamMonitoringController.java @@ -425,7 +425,10 @@ public class ExamMonitoringController { notificationId, examId, connectionToken) - .getOrThrow(); + .onError(error -> { + log.error("Failed to confirm pending notification: {} for exam {}, cause: {}", + notificationId, examId, error.getMessage()); + }); } @RequestMapping( diff --git a/src/test/java/ch/ethz/seb/sebserver/gui/integration/UseCasesIntegrationTest.java b/src/test/java/ch/ethz/seb/sebserver/gui/integration/UseCasesIntegrationTest.java index e8d72124..6d62f73d 100644 --- a/src/test/java/ch/ethz/seb/sebserver/gui/integration/UseCasesIntegrationTest.java +++ b/src/test/java/ch/ethz/seb/sebserver/gui/integration/UseCasesIntegrationTest.java @@ -1578,7 +1578,7 @@ public class UseCasesIntegrationTest extends GuiIntegrationTest { fail("Exception expected here"); } catch (final Exception e) { - assertEquals("Unexpected error while rest call", e.getMessage()); + assertEquals("argument \"content\" is null", e.getMessage()); } // test follow-up integrity violation @@ -1596,7 +1596,7 @@ public class UseCasesIntegrationTest extends GuiIntegrationTest { fail("Exception expected here"); } catch (final Exception e) { - assertEquals("Unexpected error while rest call", e.getMessage()); + assertEquals("argument \"content\" is null", e.getMessage()); } final ConfigurationTableValues newTableValue = new ConfigurationTableValues(