From 1c896c827d2472f8295ba26e7af4a64627e49977 Mon Sep 17 00:00:00 2001 From: anhefti Date: Wed, 22 Jun 2022 08:51:13 +0200 Subject: [PATCH] dispose client connection token and connection token caching --- .../servicelayer/dao/ClientConnectionDAO.java | 42 +++++-------------- .../dao/impl/ClientConnectionDAOImpl.java | 26 +++++++++++- .../dao/impl/SEBClientConfigDAOImpl.java | 21 ++++++---- .../impl/SEBClientConnectionServiceImpl.java | 33 +++++++++++---- .../oauth/AuthorizationServerConfig.java | 3 +- .../oauth/WebClientDetailsService.java | 4 +- .../config/application-dev-ws.properties | 2 +- .../config/application-dev.properties | 1 + 8 files changed, 78 insertions(+), 54 deletions(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/ClientConnectionDAO.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/ClientConnectionDAO.java index 3e097324..611a8d30 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/ClientConnectionDAO.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/ClientConnectionDAO.java @@ -11,10 +11,11 @@ package ch.ethz.seb.sebserver.webservice.servicelayer.dao; import java.util.Collection; import java.util.Set; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.cache.annotation.CacheEvict; import org.springframework.cache.annotation.Cacheable; -import ch.ethz.seb.sebserver.gbl.model.EntityKey; import ch.ethz.seb.sebserver.gbl.model.exam.Exam; import ch.ethz.seb.sebserver.gbl.model.session.ClientConnection; import ch.ethz.seb.sebserver.gbl.util.Result; @@ -26,6 +27,8 @@ public interface ClientConnectionDAO extends EntityDAO, BulkActionSupportDAO { + Logger log = LoggerFactory.getLogger(ClientConnectionDAO.class); + String CONNECTION_TOKENS_CACHE = "CONNECTION_TOKENS_CACHE"; /** Get a list of all connection tokens of all connections (no matter what state) @@ -43,9 +46,13 @@ public interface ClientConnectionDAO extends unless = "#result.hasError()") Result> getConnectionTokens(Long examId); - @CacheEvict(cacheNames = CONNECTION_TOKENS_CACHE, key = "#examId") + @CacheEvict( + cacheNames = CONNECTION_TOKENS_CACHE, + key = "#examId") default void evictConnectionTokenCache(final Long examId) { - + if (log.isDebugEnabled()) { + log.debug("Evict SEB connection tokens for exam: {}", examId); + } } /** Get a list of all connection tokens of all connections of an exam @@ -101,22 +108,6 @@ public interface ClientConnectionDAO extends * @return Result refer to a collection of all ClientConnection of the room or to an error if happened */ Result> getCollectingRoomConnections(final Long examId, final String roomName); - /** Creates new ClientConnection from the given ClientConnection data. - * - * This evicts all entries from the CONNECTION_TOKENS_CACHE. - * - * TODO improvement: Use the examId to evict only the relevant cache entry - * - * @param data ClientConnection instance - * @return Result refer to the newly created ClientConnection data or to an error if happened */ - @Override - @CacheEvict(cacheNames = CONNECTION_TOKENS_CACHE, allEntries = true) - Result createNew(ClientConnection data); - - @Override - @CacheEvict(cacheNames = CONNECTION_TOKENS_CACHE, allEntries = true) - Result save(ClientConnection data); - @CacheEvict( cacheNames = ExamSessionCacheService.CACHE_NAME_ACTIVE_CLIENT_CONNECTION, key = "#connectionToken") @@ -130,19 +121,6 @@ public interface ClientConnectionDAO extends /** Used to re-mark a client connection record for room update in error case. */ Result markForProctoringUpdate(Long id); - /** Deletes the given ClientConnection data. - * - * This evicts all entries from the CONNECTION_TOKENS_CACHE. - * - * TODO improvement: Use the examId to evict only the relevant cache entry - * - * @param all Set of EntityKey for entities to delete - * @return Result refer to a collection of deleted entities or to an error if happened */ - @Override - @CacheEvict(cacheNames = CONNECTION_TOKENS_CACHE, allEntries = true) - // TODO this probably is nor working when called from BulkActionSupportDAO - Result> delete(Set all); - /** Get a ClientConnection by connection token. * * @param connectionToken the connection token diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ClientConnectionDAOImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ClientConnectionDAOImpl.java index 07be0c18..fb3732d8 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ClientConnectionDAOImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ClientConnectionDAOImpl.java @@ -24,6 +24,7 @@ import org.apache.commons.lang3.BooleanUtils; import org.mybatis.dynamic.sql.SqlBuilder; import org.mybatis.dynamic.sql.select.MyBatis3SelectModelAdapter; import org.mybatis.dynamic.sql.select.QueryExpressionDSL; +import org.springframework.cache.CacheManager; import org.springframework.context.annotation.Lazy; import org.springframework.stereotype.Component; import org.springframework.transaction.annotation.Transactional; @@ -72,6 +73,7 @@ public class ClientConnectionDAOImpl implements ClientConnectionDAO { private final ClientIndicatorRecordMapper clientIndicatorRecordMapper; private final ClientNotificationRecordMapper clientNotificationRecordMapper; private final ClientConnectionTokenMapper clientConnectionMinMapper; + private final CacheManager cacheManager; protected ClientConnectionDAOImpl( final ClientConnectionRecordMapper clientConnectionRecordMapper, @@ -79,7 +81,8 @@ public class ClientConnectionDAOImpl implements ClientConnectionDAO { final ClientInstructionRecordMapper clientInstructionRecordMapper, final ClientIndicatorRecordMapper clientIndicatorRecordMapper, final ClientNotificationRecordMapper clientNotificationRecordMapper, - final ClientConnectionTokenMapper clientConnectionMinMapper) { + final ClientConnectionTokenMapper clientConnectionMinMapper, + final CacheManager cacheManager) { this.clientConnectionRecordMapper = clientConnectionRecordMapper; this.clientEventRecordMapper = clientEventRecordMapper; @@ -87,6 +90,7 @@ public class ClientConnectionDAOImpl implements ClientConnectionDAO { this.clientIndicatorRecordMapper = clientIndicatorRecordMapper; this.clientNotificationRecordMapper = clientNotificationRecordMapper; this.clientConnectionMinMapper = clientConnectionMinMapper; + this.cacheManager = cacheManager; } @Override @@ -529,6 +533,8 @@ public class ClientConnectionDAOImpl implements ClientConnectionDAO { return Collections.emptyList(); } + ids.stream().forEach(this::clearConnecionTokenCache); + // delete all related client indicators this.clientIndicatorRecordMapper.deleteByExample() .where( @@ -879,4 +885,22 @@ public class ClientConnectionDAOImpl implements ClientConnectionDAO { return record.getConnectionToken(); } + private Long clearConnecionTokenCache(final Long id) { + + try { + + final ClientConnectionRecord rec = recordById(id) + .getOrThrow(); + + this.cacheManager + .getCache(CONNECTION_TOKENS_CACHE) + .evictIfPresent(rec.getExamId()); + + } catch (final Exception e) { + log.error("Failed to clear connection token cache: ", e); + } + + return id; + } + } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/SEBClientConfigDAOImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/SEBClientConfigDAOImpl.java index e11e4a27..6b19a497 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/SEBClientConfigDAOImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/SEBClientConfigDAOImpl.java @@ -215,6 +215,9 @@ public class SEBClientConfigDAOImpl implements SEBClientConfigDAO { return Collections.emptyList(); } + // clear caches and revoke tokens first + ids.stream().forEach(this::disposeSEBClientConfig); + final SebClientConfigRecord record = new SebClientConfigRecord( null, null, null, null, null, null, null, BooleanUtils.toIntegerObject(active), @@ -228,7 +231,6 @@ public class SEBClientConfigDAOImpl implements SEBClientConfigDAO { return ids.stream() .map(id -> new EntityKey(id, EntityType.SEB_CLIENT_CONFIGURATION)) - .map(this::revokeClientConnection) .collect(Collectors.toList()); }); } @@ -308,6 +310,9 @@ public class SEBClientConfigDAOImpl implements SEBClientConfigDAO { return Collections.emptyList(); } + // clear caches and revoke tokens first + ids.stream().forEach(this::disposeSEBClientConfig); + this.sebClientConfigRecordMapper.deleteByExample() .where(SebClientConfigRecordDynamicSqlSupport.id, isIn(ids)) .build() @@ -315,7 +320,6 @@ public class SEBClientConfigDAOImpl implements SEBClientConfigDAO { return ids.stream() .map(id -> new EntityKey(id, EntityType.SEB_CLIENT_CONFIGURATION)) - .map(this::revokeClientConnection) .collect(Collectors.toList()); }); } @@ -670,10 +674,10 @@ public class SEBClientConfigDAOImpl implements SEBClientConfigDAO { } } - private EntityKey revokeClientConnection(final EntityKey key) { + private Long disposeSEBClientConfig(final Long pk) { try { - final SebClientConfigRecord rec = recordById(Long.parseLong(key.modelId)) + final SebClientConfigRecord rec = recordById(pk) .getOrThrow(); // revoke token @@ -681,17 +685,18 @@ public class SEBClientConfigDAOImpl implements SEBClientConfigDAO { this.applicationEventPublisher .publishEvent(new RevokeExamTokenEvent(rec.getClientName())); } catch (final Exception e) { - log.error("Failed to revoke token for SEB client connection. Connection Configuration: {}", key, e); + log.error("Failed to revoke token for SEB client connection. Connection Configuration: {}", pk, e); } // clear cache - this.cacheManager.getCache(ClientConfigService.EXAM_CLIENT_DETAILS_CACHE) + this.cacheManager + .getCache(ClientConfigService.EXAM_CLIENT_DETAILS_CACHE) .evictIfPresent(rec.getClientName()); } catch (final Exception e) { - log.error("Failed to revoke SEB client connection. Connection Configuration: {}", key, e); + log.error("Failed to revoke SEB client connection. Connection Configuration: {}", pk, e); } - return key; + return pk; } } 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 0399de13..9b1658d3 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 @@ -180,6 +180,11 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic this.clientIndicatorFactory.initializeDistributedCaches(clientConnection); } + // flash connection token cache for exam if available + if (examId != null) { + this.clientConnectionDAO.evictConnectionTokenCache(examId); + } + // load client connection data into cache final ClientConnectionDataInternal activeClientConnection = this.examSessionService .getConnectionDataInternal(connectionToken); @@ -286,8 +291,9 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic this.clientIndicatorFactory.initializeDistributedCaches(clientConnection); } - final ClientConnectionDataInternal activeClientConnection = - reloadConnectionCache(connectionToken); + final ClientConnectionDataInternal activeClientConnection = reloadConnectionCache( + connectionToken, + examId); if (activeClientConnection == null) { log.warn("Failed to load ClientConnectionDataInternal into cache on update"); @@ -441,8 +447,9 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic } // flush and reload caches to work with actual connection data - final ClientConnectionDataInternal activeClientConnection = - reloadConnectionCache(connectionToken); + final ClientConnectionDataInternal activeClientConnection = reloadConnectionCache( + connectionToken, + examId); if (activeClientConnection == null) { log.warn("Failed to load ClientConnectionDataInternal into cache on update"); @@ -496,13 +503,14 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic establishedClientConnection.remoteProctoringRoomUpdate); // Update other connection with token and exam id - this.clientConnectionDAO + final ClientConnection connection = this.clientConnectionDAO .save(new ClientConnection( vdiPairCompanion.getId(), null, vdiExamId, null, null, null, null, null, null, establishedClientConnection.connectionToken, null, null, null, null, null, null, null)) .getOrThrow(); - reloadConnectionCache(vdiPairCompanion.getConnectionToken()); + + reloadConnectionCache(vdiPairCompanion.getConnectionToken(), connection.examId); return updatedConnection; } @@ -557,7 +565,7 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic .deleteIndicatorValues(updatedClientConnection.id); } - reloadConnectionCache(connectionToken); + reloadConnectionCache(connectionToken, clientConnection.examId); return updatedClientConnection; }); } @@ -619,7 +627,7 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic .deleteIndicatorValues(updatedClientConnection.id); } - reloadConnectionCache(connectionToken); + reloadConnectionCache(connectionToken, clientConnection.examId); return updatedClientConnection; }); } @@ -886,7 +894,14 @@ public class SEBClientConnectionServiceImpl implements SEBClientConnectionServic .getOrThrow(); } - private ClientConnectionDataInternal reloadConnectionCache(final String connectionToken) { + private ClientConnectionDataInternal reloadConnectionCache( + final String connectionToken, + final Long examId) { + + if (examId != null) { + // evict connection tokens for exam + this.clientConnectionDAO.evictConnectionTokenCache(examId); + } // evict cached ClientConnection this.examSessionCacheService.evictClientConnection(connectionToken); // and load updated ClientConnection into cache diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/AuthorizationServerConfig.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/AuthorizationServerConfig.java index a96d0e59..70b9f61a 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/AuthorizationServerConfig.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/AuthorizationServerConfig.java @@ -79,8 +79,7 @@ public class AuthorizationServerConfig extends AuthorizationServerConfigurerAdap response.setContentType(MediaType.APPLICATION_JSON_VALUE); response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); log.warn( - "Unauthorized Request: {}, {}", - request, + "Unauthorized Request: {}", exception != null ? exception.getMessage() : Constants.EMPTY_NOTE); response.getOutputStream().println("{ \"error\": \"" + exception.getMessage() + "\" }"); }); diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/WebClientDetailsService.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/WebClientDetailsService.java index 487163d4..4bad3ce4 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/WebClientDetailsService.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/WebClientDetailsService.java @@ -66,7 +66,9 @@ public class WebClientDetailsService implements ClientDetailsService { return getForExamClientAPI(clientId) .get(t -> { - log.error("Active ClientConfig not found: {} cause: {}", clientId, t.getMessage()); + if (log.isDebugEnabled()) { + log.warn("Active ClientConfig not found: {} cause: {}", clientId, t.getMessage()); + } throw new UsernameNotFoundException(t.getMessage()); }); } diff --git a/src/main/resources/config/application-dev-ws.properties b/src/main/resources/config/application-dev-ws.properties index 6d85c2b3..c45a499e 100644 --- a/src/main/resources/config/application-dev-ws.properties +++ b/src/main/resources/config/application-dev-ws.properties @@ -25,7 +25,7 @@ sebserver.webservice.clean-db-on-startup=false # webservice configuration sebserver.init.adminaccount.gen-on-init=false -sebserver.webservice.distributed=true +sebserver.webservice.distributed=false #sebserver.webservice.master.delay.threshold=10000 sebserver.webservice.http.external.scheme=http sebserver.webservice.http.external.servername=localhost diff --git a/src/main/resources/config/application-dev.properties b/src/main/resources/config/application-dev.properties index 649a771d..fdaf8908 100644 --- a/src/main/resources/config/application-dev.properties +++ b/src/main/resources/config/application-dev.properties @@ -22,6 +22,7 @@ logging.level.ch.ethz.seb.sebserver.webservice.weblayer.oauth=DEBUG #logging.level.ch.ethz.seb.sebserver.webservice.datalayer.batis.mapper.ExamRecordMapper=DEBUG #logging.level.ch.ethz.seb.sebserver.webservice.weblayer.api.ExamAPI_V1_Controller=TRACE logging.level.com.zaxxer.hikari=INFO +#logging.level.ch.ethz.seb.sebserver.webservice.servicelayer.dao=DEBUG sebserver.http.client.connect-timeout=15000 sebserver.http.client.connection-request-timeout=10000