From 1d7bd02382f2e5d578a7bf95ac88583837f3b785 Mon Sep 17 00:00:00 2001 From: anhefti Date: Tue, 17 Nov 2020 17:04:01 +0100 Subject: [PATCH 1/2] fixed concurrent user login within ServerPushService --- .../content/MonitoringClientConnection.java | 5 +- .../gui/content/MonitoringRunningExam.java | 32 +++++++- .../gui/service/push/ServerPushContext.java | 19 +++-- .../gui/service/push/ServerPushService.java | 76 ++++++++++++------- .../session/ClientConnectionTable.java | 5 +- .../gui/widget/ImageUploadSelection.java | 8 +- 6 files changed, 104 insertions(+), 41 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 310e3b3a..bba8e7c0 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 @@ -163,7 +163,10 @@ public class MonitoringClientConnection implements TemplateComposer { indicators); this.serverPushService.runServerPush( - new ServerPushContext(content, Utils.truePredicate()), + new ServerPushContext( + content, + Utils.truePredicate(), + MonitoringRunningExam.createServerPushUpdateErrorHandler(this.pageService, pageContext)), this.pollInterval, context1 -> clientConnectionDetails.updateData(), context -> clientConnectionDetails.updateGUI()); diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/content/MonitoringRunningExam.java b/src/main/java/ch/ethz/seb/sebserver/gui/content/MonitoringRunningExam.java index ae8c311d..25537a62 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/content/MonitoringRunningExam.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/content/MonitoringRunningExam.java @@ -19,6 +19,7 @@ import org.eclipse.swt.SWT; import org.eclipse.swt.layout.GridData; import org.eclipse.swt.layout.GridLayout; import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.MessageBox; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Value; @@ -57,6 +58,7 @@ import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.session.GetClient import ch.ethz.seb.sebserver.gui.service.remote.webservice.auth.CurrentUser; import ch.ethz.seb.sebserver.gui.service.session.ClientConnectionTable; import ch.ethz.seb.sebserver.gui.service.session.InstructionProcessor; +import ch.ethz.seb.sebserver.gui.widget.Message; @Lazy @Component @@ -149,7 +151,9 @@ public class MonitoringRunningExam implements TemplateComposer { ActionDefinition.MONITOR_EXAM_DISABLE_SELECTED_CONNECTION)); this.serverPushService.runServerPush( - new ServerPushContext(content, Utils.truePredicate()), + new ServerPushContext( + content, Utils.truePredicate(), + createServerPushUpdateErrorHandler(this.pageService, pageContext)), this.pollInterval, context -> clientTable.updateValues(), updateTableGUI(clientTable)); @@ -360,4 +364,30 @@ public class MonitoringRunningExam implements TemplateComposer { }; } + static final Function createServerPushUpdateErrorHandler( + final PageService pageService, + final PageContext pageContext) { + + return error -> { + log.error("Fialed to update server push: {}", error.getMessage()); + try { + pageService.getCurrentUser().get(); + } catch (final Exception e) { + log.error("Failed to verify current user after server push error: {}", e.getMessage()); + log.info("Force logout and session cleanup..."); + pageContext.forwardToLoginPage(); + final MessageBox logoutSuccess = new Message( + pageContext.getShell(), + pageService.getI18nSupport().getText("sebserver.logout"), + Utils.formatLineBreaks( + pageService.getI18nSupport().getText("sebserver.logout.invalid-session.message")), + SWT.ICON_INFORMATION, + pageService.getI18nSupport()); + logoutSuccess.open(null); + return true; + } + return false; + }; + } + } diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/push/ServerPushContext.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/push/ServerPushContext.java index d4309ce3..63261278 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/push/ServerPushContext.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/push/ServerPushContext.java @@ -8,6 +8,7 @@ package ch.ethz.seb.sebserver.gui.service.push; +import java.util.function.Function; import java.util.function.Predicate; import org.eclipse.swt.widgets.Composite; @@ -18,23 +19,25 @@ import org.eclipse.swt.widgets.Display; * @author anhefti */ public final class ServerPushContext { - private final Composite anchor; - private final Predicate runAgain; - - public ServerPushContext(final Composite anchor) { - this(anchor, context -> false); - } + public final Composite anchor; + public final Predicate runAgain; + public final Function errorHandler; + boolean internalStop = false; public ServerPushContext( final Composite anchor, - final Predicate runAgain) { + final Predicate runAgain, + final Function errorHandler) { + this.errorHandler = errorHandler != null + ? errorHandler + : error -> true; this.anchor = anchor; this.runAgain = runAgain; } public boolean runAgain() { - return this.runAgain.test(this); + return !this.internalStop && this.runAgain.test(this); } public boolean isDisposed() { diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/push/ServerPushService.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/push/ServerPushService.java index fb4b5e6a..902f6bb5 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/push/ServerPushService.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/push/ServerPushService.java @@ -54,43 +54,43 @@ public class ServerPushService { } } - if (business != null) { - try { - log.trace("Call business on Server Push Session on: {}", Thread.currentThread().getName()); - business.accept(context); - } catch (final Exception e) { - log.error("Unexpected error while do business for server push service", e); - if (context.runAgain()) { - continue; - } else { - return; - } - } + if (context.isDisposed()) { + continue; } - if (!context.isDisposed()) { - - log.trace("Call update on Server Push Session on: {}", Thread.currentThread().getName()); - - context.getDisplay().asyncExec(() -> { + context.getDisplay().asyncExec(() -> { + if (business != null) { try { - update.accept(context); + + if (log.isTraceEnabled()) { + log.trace("Call business on Server Push Session on: {}", + Thread.currentThread().getName()); + } + + business.accept(context); + updateGUI(context, update); + } catch (final Exception e) { - log.warn( - "Failed to update on Server Push Session {}. It seems that the UISession is not available anymore. " - + "This may source from a connection interruption. cause: {}", - Thread.currentThread().getName(), e.getMessage()); + log.error("Unexpected error while do business for server push service", e); + context.internalStop = context.errorHandler.apply(e); } - }); - } + } else { + updateGUI(context, update); + } + }); + } + + if (log.isInfoEnabled()) { + log.info("Stop Server Push Session on: {}", Thread.currentThread().getName()); } - log.info("Stop Server Push Session on: {}", Thread.currentThread().getName()); try { pushSession.stop(); } catch (final Exception e) { log.warn( - "Failed to stop Server Push Session on: {}. It seems that the UISession is not available anymore. This may source from a connection interruption", + "Failed to stop Server Push Session on: {}. " + + "It seems that the UISession is not available anymore. " + + "This may source from a connection interruption", Thread.currentThread().getName(), e); } @@ -101,4 +101,28 @@ public class ServerPushService { bgThread.setDaemon(true); bgThread.start(); } + + private void updateGUI( + final ServerPushContext context, + final Consumer update) { + + if (!context.isDisposed()) { + + if (log.isTraceEnabled()) { + log.trace("Call update on Server Push Session on: {}", + Thread.currentThread().getName()); + } + + try { + update.accept(context); + } catch (final Exception e) { + log.warn( + "Failed to update on Server Push Session {}. " + + "It seems that the UISession is not available anymore. " + + "This may source from a connection interruption. cause: {}", + Thread.currentThread().getName(), e.getMessage()); + context.internalStop = context.errorHandler.apply(e); + } + } + } } 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 70b53006..f1232557 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 @@ -303,10 +303,7 @@ public final class ClientConnectionTable { this.restCallBuilder .withHeader(API.EXAM_MONITORING_STATE_FILTER, this.statusFilterParam) .call() - .get(error -> { - log.error("Error poll connection data: ", error); - return Collections.emptyList(); - }) + .getOrThrow() .forEach(data -> { final UpdatableTableItem tableItem = this.tableMapping.computeIfAbsent( data.getConnectionId(), diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/widget/ImageUploadSelection.java b/src/main/java/ch/ethz/seb/sebserver/gui/widget/ImageUploadSelection.java index 6b7cf37e..a6b1a7e9 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/widget/ImageUploadSelection.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/widget/ImageUploadSelection.java @@ -113,7 +113,13 @@ public final class ImageUploadSelection extends Composite { ImageUploadSelection.this.fileUpload.submit(uploadHandler.getUploadUrl()); ImageUploadSelection.this.serverPushService.runServerPush( - new ServerPushContext(ImageUploadSelection.this, ImageUploadSelection::uploadInProgress), + new ServerPushContext( + ImageUploadSelection.this, + ImageUploadSelection::uploadInProgress, + error -> { + log.error("Failed to upload image: {}", error.getMessage()); + return false; + }), 200, ImageUploadSelection::update); }); From 797ebdd200d14a035d1fdd8e2c70baf851e0e78a Mon Sep 17 00:00:00 2001 From: anhefti Date: Tue, 17 Nov 2020 17:05:25 +0100 Subject: [PATCH 2/2] changed internal method name --- .../seb/sebserver/gui/service/push/ServerPushService.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/push/ServerPushService.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/push/ServerPushService.java index 902f6bb5..8fb7269b 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/push/ServerPushService.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/push/ServerPushService.java @@ -68,14 +68,14 @@ public class ServerPushService { } business.accept(context); - updateGUI(context, update); + doUpdate(context, update); } catch (final Exception e) { log.error("Unexpected error while do business for server push service", e); context.internalStop = context.errorHandler.apply(e); } } else { - updateGUI(context, update); + doUpdate(context, update); } }); } @@ -102,7 +102,7 @@ public class ServerPushService { bgThread.start(); } - private void updateGUI( + private void doUpdate( final ServerPushContext context, final Consumer update) {