From 8e083517701b92cee15b1d7164803183e9897355 Mon Sep 17 00:00:00 2001 From: anhefti Date: Wed, 16 Jun 2021 21:06:12 +0200 Subject: [PATCH] fixed notification performance --- .../content/MonitoringClientConnection.java | 20 ++++----- .../servicelayer/dao/ClientEventDAO.java | 20 +++++++++ .../dao/impl/ClientEventDAOImpl.java | 26 +++++++++++ .../session/SEBClientNotificationService.java | 3 +- .../InternalClientConnectionDataFactory.java | 2 +- .../SEBClientNotificationServiceImpl.java | 44 ++++++++++--------- 6 files changed, 82 insertions(+), 33 deletions(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/content/MonitoringClientConnection.java b/src/main/java/ch/ethz/seb/sebserver/gui/content/MonitoringClientConnection.java index e528e9b0..78d83452 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/content/MonitoringClientConnection.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/content/MonitoringClientConnection.java @@ -254,7 +254,7 @@ public class MonitoringClientConnection implements TemplateComposer { .newAction(ActionDefinition.MONITOR_EXAM_CLIENT_CONNECTION_CONFIRM_NOTIFICATION) .withParentEntityKey(parentEntityKey) .withConfirm(() -> NOTIFICATION_LIST_CONFIRM_TEXT_KEY) - .withExec(action -> this.confirmNotification(action, connectionData)) + .withExec(action -> this.confirmNotification(action, connectionData, t)) .noEventPropagation() .create()) .withSelectionListener(this.pageService.getSelectionPublisher( @@ -268,8 +268,10 @@ public class MonitoringClientConnection implements TemplateComposer { .withConfirm(() -> NOTIFICATION_LIST_CONFIRM_TEXT_KEY) .withSelect( () -> notificationTable.getSelection(), - action -> this.confirmNotification(action, connectionData), + action -> this.confirmNotification(action, connectionData, notificationTable), + NOTIFICATION_LIST_NO_SELECTION_KEY) + .noEventPropagation() .publishIf(() -> currentUser.get().hasRole(UserRole.EXAM_SUPPORTER), false); @@ -413,9 +415,10 @@ public class MonitoringClientConnection implements TemplateComposer { private PageAction confirmNotification( final PageAction pageAction, - final ClientConnectionData connectionData) { + final ClientConnectionData connectionData, + final EntityTable table) { - final EntityKey entityKey = pageAction.getSingleSelection(); + final EntityKey entityKey = table.getSingleSelection(); final EntityKey parentEntityKey = pageAction.getParentEntityKey(); this.pageService.getRestService() @@ -426,13 +429,8 @@ public class MonitoringClientConnection implements TemplateComposer { .call() .getOrThrow(); - return pageAction - .withEntityKey( - new EntityKey(connectionData.getConnectionId(), - EntityType.CLIENT_CONNECTION)) - .withAttribute( - Domain.CLIENT_CONNECTION.ATTR_CONNECTION_TOKEN, - connectionData.clientConnection.connectionToken); + table.reset(); + return pageAction; } private PageAction openExamCollectionProctorScreen( diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/ClientEventDAO.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/ClientEventDAO.java index d5d7c87d..6c8f71db 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/ClientEventDAO.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/ClientEventDAO.java @@ -10,6 +10,7 @@ package ch.ethz.seb.sebserver.webservice.servicelayer.dao; import java.util.Collection; import java.util.List; +import java.util.Set; import java.util.function.Predicate; import ch.ethz.seb.sebserver.gbl.model.session.ClientEvent; @@ -28,10 +29,29 @@ public interface ClientEventDAO extends EntityDAO { FilterMap filterMap, Predicate predicate); + /** Get a specified notification by id (PK) + * + * @param notificationId The PK of the notification + * @return Result refer to the specified ClientNotification or to an error when happened */ Result getPendingNotification(Long notificationId); + /** Get all pending notifications for a given client connection. + * + * @param clientConnectionId The client connection identifier + * @return Result refer to the list of pending notifications or to an error when happened */ Result> getPendingNotifications(Long clientConnectionId); + /** Get all identifiers (PKs) of client connections of a given exam that has any pending notification + * + * @param examId the exam identifier + * @return Result refer to a set of identifiers of client connections or to an error when happened */ + Result> getClientConnectionIdsWithPendingNotification(Long examId); + + /** Used to confirm a pending notification so that the notification is not pending anymore + * + * @param notificationId the notification identifier + * @param clientConnectionId the client connection identifier + * @return Result refer to the confirmed notification or to en error when happened */ Result confirmPendingNotification(Long notificationId, Long clientConnectionId); } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ClientEventDAOImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ClientEventDAOImpl.java index 12c747ed..74312604 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ClientEventDAOImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ClientEventDAOImpl.java @@ -27,6 +27,7 @@ import org.springframework.transaction.annotation.Transactional; import ch.ethz.seb.sebserver.gbl.api.EntityType; import ch.ethz.seb.sebserver.gbl.model.EntityKey; import ch.ethz.seb.sebserver.gbl.model.session.ClientConnection; +import ch.ethz.seb.sebserver.gbl.model.session.ClientConnection.ConnectionStatus; import ch.ethz.seb.sebserver.gbl.model.session.ClientEvent; import ch.ethz.seb.sebserver.gbl.model.session.ClientEvent.EventType; import ch.ethz.seb.sebserver.gbl.model.session.ClientNotification; @@ -212,6 +213,31 @@ public class ClientEventDAOImpl implements ClientEventDAO { .collect(Collectors.toList())); } + @Override + @Transactional(readOnly = true) + public Result> getClientConnectionIdsWithPendingNotification(final Long examId) { + return Result.tryCatch(() -> this.clientEventRecordMapper + .selectByExample() + .leftJoin(ClientConnectionRecordDynamicSqlSupport.clientConnectionRecord) + .on( + ClientConnectionRecordDynamicSqlSupport.id, + equalTo(ClientEventRecordDynamicSqlSupport.clientConnectionId)) + .where( + ClientConnectionRecordDynamicSqlSupport.examId, + isEqualToWhenPresent(examId)) + .and( + ClientConnectionRecordDynamicSqlSupport.status, + isEqualTo(ConnectionStatus.ACTIVE.name())) + .and( + ClientEventRecordDynamicSqlSupport.type, + isEqualTo(EventType.NOTIFICATION.id)) + .build() + .execute() + .stream() + .map(ClientEventRecord::getClientConnectionId) + .collect(Collectors.toSet())); + } + @Override @Transactional public Result confirmPendingNotification(final Long notificationId, diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/SEBClientNotificationService.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/SEBClientNotificationService.java index a25bb140..351f99e3 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/SEBClientNotificationService.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/SEBClientNotificationService.java @@ -10,6 +10,7 @@ package ch.ethz.seb.sebserver.webservice.servicelayer.session; import java.util.List; +import ch.ethz.seb.sebserver.gbl.model.session.ClientConnection; import ch.ethz.seb.sebserver.gbl.model.session.ClientEvent; import ch.ethz.seb.sebserver.gbl.model.session.ClientNotification; import ch.ethz.seb.sebserver.gbl.util.Result; @@ -24,7 +25,7 @@ public interface SEBClientNotificationService { * * @param clientConnectionId the client connection identifier * @return true if there is any pending notification for the specified client connection */ - Boolean hasAnyPendingNotification(Long clientConnectionId); + Boolean hasAnyPendingNotification(final ClientConnection clientConnection); Result> getPendingNotifications(Long clientConnectionId); diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/InternalClientConnectionDataFactory.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/InternalClientConnectionDataFactory.java index c93bc823..54b23f19 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/InternalClientConnectionDataFactory.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/InternalClientConnectionDataFactory.java @@ -47,7 +47,7 @@ public class InternalClientConnectionDataFactory { return new ClientConnectionDataInternal( clientConnection, () -> this.sebClientNotificationService - .hasAnyPendingNotification(clientConnection.id), + .hasAnyPendingNotification(clientConnection), this.clientIndicatorFactory.createFor(clientConnection)); } 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 93059772..fe20b329 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 @@ -20,6 +20,8 @@ import org.slf4j.LoggerFactory; import org.springframework.context.annotation.Lazy; import org.springframework.stereotype.Service; +import ch.ethz.seb.sebserver.gbl.Constants; +import ch.ethz.seb.sebserver.gbl.model.session.ClientConnection; import ch.ethz.seb.sebserver.gbl.model.session.ClientEvent; import ch.ethz.seb.sebserver.gbl.model.session.ClientInstruction; import ch.ethz.seb.sebserver.gbl.model.session.ClientInstruction.InstructionType; @@ -43,6 +45,9 @@ public class SEBClientNotificationServiceImpl implements SEBClientNotificationSe private final ClientEventDAO clientEventDAO; private final SEBClientInstructionService sebClientInstructionService; private final Set pendingNotifications = new HashSet<>(); + private final Set examUpdate = new HashSet<>(); + + private long lastUpdate = 0; public SEBClientNotificationServiceImpl( final ClientEventDAO clientEventDAO, @@ -53,26 +58,9 @@ public class SEBClientNotificationServiceImpl implements SEBClientNotificationSe } @Override - public Boolean hasAnyPendingNotification(final Long clientConnectionId) { - - if (this.pendingNotifications.add(clientConnectionId)) { - return true; - } - - final boolean hasAnyPendingNotification = !getPendingNotifications(clientConnectionId) - .getOr(Collections.emptyList()) - .isEmpty(); - - if (hasAnyPendingNotification) { - // NOTE this is a quick and dirty way to keep cache pendingNotifications cache size short. - // TODO find a better way to do this. - if (this.pendingNotifications.size() > 100) { - this.pendingNotifications.clear(); - } - this.pendingNotifications.add(clientConnectionId); - } - - return hasAnyPendingNotification; + public Boolean hasAnyPendingNotification(final ClientConnection clientConnection) { + updateCache(clientConnection.examId); + return this.pendingNotifications.contains(clientConnection.id); } @Override @@ -142,4 +130,20 @@ public class SEBClientNotificationServiceImpl implements SEBClientNotificationSe return notification; } + private final void updateCache(final Long examId) { + if (System.currentTimeMillis() - this.lastUpdate > 5 * Constants.SECOND_IN_MILLIS) { + this.examUpdate.clear(); + this.pendingNotifications.clear(); + this.lastUpdate = System.currentTimeMillis(); + } + + if (!this.examUpdate.contains(examId)) { + this.pendingNotifications.addAll( + this.clientEventDAO + .getClientConnectionIdsWithPendingNotification(examId) + .getOr(Collections.emptySet())); + this.examUpdate.add(examId); + } + } + }