From 1384f1eb2c973252da27d6e379d8b863d912b22e Mon Sep 17 00:00:00 2001 From: =?utf8?q?Johannes=20Dahlstr=C3=B6m?= Date: Thu, 1 Nov 2012 15:10:40 +0200 Subject: [PATCH] When closeIdleUIs is true, close open UIs only after all UIs have been idle for longer than the session timeout * Rename closeIdleUIs to closeIdleSessions, isIdleUICleanupEnabled to isCloseIdleSessions (#10252) * Close the whole session at the same time as well * Move UI lastUidlRequestTime to VaadinSession lastRequestTimestamp (#10253) * Rename VaadinSession lastRequestTime to lastRequestDuration, totalSessionTime to cumulativeRequestDuration (#10253) * Rename UI lastHeartbeatTime to lastHeartbeatTimestamp * Show "Session Expired" notification when a heartbeat fails because of this Change-Id: If3d4bd9add9995f435c29812eec00b3a3a92a6e6 --- WebContent/WEB-INF/web.xml | 2 +- .../client/ApplicationConfiguration.java | 6 ++ .../vaadin/client/ApplicationConnection.java | 78 ++++++++++--------- .../server/AbstractCommunicationManager.java | 7 +- .../com/vaadin/server/BootstrapHandler.java | 9 +++ server/src/com/vaadin/server/Constants.java | 2 +- .../DefaultDeploymentConfiguration.java | 16 ++-- .../server/DeploymentConfiguration.java | 19 +++-- .../src/com/vaadin/server/RequestTimer.java | 2 +- .../src/com/vaadin/server/VaadinSession.java | 66 ++++++++++------ server/src/com/vaadin/ui/UI.java | 40 +++------- .../DeploymentConfigurationTest.html | 2 +- 12 files changed, 136 insertions(+), 113 deletions(-) diff --git a/WebContent/WEB-INF/web.xml b/WebContent/WEB-INF/web.xml index 820eb2cbce..cd1f0a75cc 100644 --- a/WebContent/WEB-INF/web.xml +++ b/WebContent/WEB-INF/web.xml @@ -61,7 +61,7 @@ 3601 - closeIdleUIs + closeIdleSessions true diff --git a/client/src/com/vaadin/client/ApplicationConfiguration.java b/client/src/com/vaadin/client/ApplicationConfiguration.java index 4159b38211..1f35c408ae 100644 --- a/client/src/com/vaadin/client/ApplicationConfiguration.java +++ b/client/src/com/vaadin/client/ApplicationConfiguration.java @@ -199,6 +199,7 @@ public class ApplicationConfiguration implements EntryPoint { private boolean standalone; private ErrorMessage communicationError; private ErrorMessage authorizationError; + private ErrorMessage sessionExpiredError; private int heartbeatInterval; private HashMap unknownComponents; @@ -314,6 +315,10 @@ public class ApplicationConfiguration implements EntryPoint { return authorizationError; } + public ErrorMessage getSessionExpiredError() { + return sessionExpiredError; + } + /** * Reads the configuration values defined by the bootstrap javascript. */ @@ -353,6 +358,7 @@ public class ApplicationConfiguration implements EntryPoint { communicationError = jsoConfiguration.getConfigError("comErrMsg"); authorizationError = jsoConfiguration.getConfigError("authErrMsg"); + sessionExpiredError = jsoConfiguration.getConfigError("sessExpMsg"); // boostrap sets initPending to false if it has sent the browser details if (jsoConfiguration.getConfigBoolean("initPending") == Boolean.FALSE) { diff --git a/client/src/com/vaadin/client/ApplicationConnection.java b/client/src/com/vaadin/client/ApplicationConnection.java index 23906f0e02..9133e3dfc5 100644 --- a/client/src/com/vaadin/client/ApplicationConnection.java +++ b/client/src/com/vaadin/client/ApplicationConnection.java @@ -960,9 +960,7 @@ public class ApplicationConnection { */ protected void showCommunicationError(String details, int statusCode) { VConsole.error("Communication error: " + details); - ErrorMessage communicationError = configuration.getCommunicationError(); - showError(details, communicationError.getCaption(), - communicationError.getMessage(), communicationError.getUrl()); + showError(details, configuration.getCommunicationError()); } /** @@ -973,9 +971,31 @@ public class ApplicationConnection { */ protected void showAuthenticationError(String details) { VConsole.error("Authentication error: " + details); - ErrorMessage authorizationError = configuration.getAuthorizationError(); - showError(details, authorizationError.getCaption(), - authorizationError.getMessage(), authorizationError.getUrl()); + showError(details, configuration.getAuthorizationError()); + } + + /** + * Shows the session expiration notification. + * + * @param details + * Optional details for debugging. + */ + protected void showSessionExpiredError(String details) { + VConsole.error("Session expired: " + details); + showError(details, configuration.getSessionExpiredError()); + } + + /** + * Shows an error notification. + * + * @param details + * Optional details for debugging. + * @param message + * An ErrorMessage describing the error. + */ + protected void showError(String details, ErrorMessage message) { + showError(details, message.getCaption(), message.getMessage(), + message.getUrl()); } /** @@ -1002,9 +1022,11 @@ public class ApplicationConnection { if (html.length() > 0) { // Add error description - html.append("

"); - html.append(details); - html.append("

"); + if (details != null) { + html.append("

"); + html.append(details); + html.append("

"); + } VNotification n = VNotification.createNotification(1000 * 60 * 45, uIConnector.getWidget()); @@ -1442,32 +1464,11 @@ public class ApplicationConnection { if (meta != null) { if (meta.containsKey("appError")) { ValueMap error = meta.getValueMap("appError"); - String html = ""; - if (error.containsKey("caption") - && error.getString("caption") != null) { - html += "

" + error.getAsString("caption") - + "

"; - } - if (error.containsKey("message") - && error.getString("message") != null) { - html += "

" + error.getAsString("message") - + "

"; - } - String url = null; - if (error.containsKey("url")) { - url = error.getString("url"); - } - if (html.length() != 0) { - /* 45 min */ - VNotification n = VNotification.createNotification( - 1000 * 60 * 45, uIConnector.getWidget()); - n.addEventListener(new NotificationRedirect(url)); - n.show(html, VNotification.CENTERED_TOP, - VNotification.STYLE_SYSTEM); - } else { - redirect(url); - } + showError(null, error.getString("caption"), + error.getString("message"), + error.getString("url")); + applicationRunning = false; } if (validatingLayouts) { @@ -3027,7 +3028,7 @@ public class ApplicationConnection { *

* Heartbeat requests are used to inform the server that the client-side is * still alive. If the client page is closed or the connection lost, the - * server will eventually close the inactive Root. + * server will eventually close the inactive UI. *

* TODO: Improved error handling, like in doUidlRequest(). * @@ -3051,16 +3052,17 @@ public class ApplicationConnection { // TODO Permit retry in some error situations VConsole.log("Heartbeat response OK"); scheduleHeartbeat(); + } else if (status == Response.SC_GONE) { + showSessionExpiredError(null); } else { - VConsole.error("Heartbeat request failed with status code " + VConsole.error("Failed sending heartbeat to server. Error code: " + status); } } @Override public void onError(Request request, Throwable exception) { - VConsole.error("Heartbeat request resulted in exception"); - VConsole.error(exception); + VConsole.error("Exception sending heartbeat: " + exception); } }; diff --git a/server/src/com/vaadin/server/AbstractCommunicationManager.java b/server/src/com/vaadin/server/AbstractCommunicationManager.java index 0655ad6e74..dc987530bd 100644 --- a/server/src/com/vaadin/server/AbstractCommunicationManager.java +++ b/server/src/com/vaadin/server/AbstractCommunicationManager.java @@ -583,8 +583,7 @@ public abstract class AbstractCommunicationManager implements Serializable { return; } - // Keep the UI alive - uI.setLastUidlRequestTime(System.currentTimeMillis()); + session.setLastRequestTimestamp(System.currentTimeMillis()); // Change all variables based on request parameters if (!handleVariables(request, response, callback, session, uI)) { @@ -1333,7 +1332,7 @@ public abstract class AbstractCommunicationManager implements Serializable { */ private void writePerformanceData(final PrintWriter outWriter) { outWriter.write(String.format(", \"timings\":[%d, %d]", - session.getTotalSessionTime(), session.getLastRequestTime())); + session.getCumulativeRequestDuration(), session.getLastRequestDuration())); } private void legacyPaint(PaintTarget paintTarget, @@ -2798,7 +2797,7 @@ public abstract class AbstractCommunicationManager implements Serializable { // null-check below handles this as well } if (ui != null) { - ui.setLastHeartbeatTime(System.currentTimeMillis()); + ui.setLastHeartbeatTimestamp(System.currentTimeMillis()); } else { response.sendError(HttpServletResponse.SC_NOT_FOUND, "UI not found"); } diff --git a/server/src/com/vaadin/server/BootstrapHandler.java b/server/src/com/vaadin/server/BootstrapHandler.java index 36a09253d9..1ab4390351 100644 --- a/server/src/com/vaadin/server/BootstrapHandler.java +++ b/server/src/com/vaadin/server/BootstrapHandler.java @@ -434,6 +434,15 @@ public abstract class BootstrapHandler implements RequestHandler { authErrMsg.put("url", systemMessages.getAuthenticationErrorURL()); appConfig.put("authErrMsg", authErrMsg); + + JSONObject sessExpMsg = new JSONObject(); + sessExpMsg + .put("caption", systemMessages.getSessionExpiredCaption()); + sessExpMsg + .put("message", systemMessages.getSessionExpiredMessage()); + sessExpMsg.put("url", systemMessages.getSessionExpiredURL()); + + appConfig.put("sessExpMsg", sessExpMsg); } // getStaticFileLocation documented to never end with a slash diff --git a/server/src/com/vaadin/server/Constants.java b/server/src/com/vaadin/server/Constants.java index b6bfcc0495..faf24b4bce 100644 --- a/server/src/com/vaadin/server/Constants.java +++ b/server/src/com/vaadin/server/Constants.java @@ -62,7 +62,7 @@ public interface Constants { static final String SERVLET_PARAMETER_DISABLE_XSRF_PROTECTION = "disable-xsrf-protection"; static final String SERVLET_PARAMETER_RESOURCE_CACHE_TIME = "resourceCacheTime"; static final String SERVLET_PARAMETER_HEARTBEAT_INTERVAL = "heartbeatInterval"; - static final String SERVLET_PARAMETER_CLOSE_IDLE_UIS = "closeIdleUIs"; + static final String SERVLET_PARAMETER_CLOSE_IDLE_SESSIONS = "closeIdleSessions"; static final String SERVLET_PARAMETER_UI_PROVIDER = "UIProvider"; // Configurable parameter names diff --git a/server/src/com/vaadin/server/DefaultDeploymentConfiguration.java b/server/src/com/vaadin/server/DefaultDeploymentConfiguration.java index 23392e59a8..13218f6e45 100644 --- a/server/src/com/vaadin/server/DefaultDeploymentConfiguration.java +++ b/server/src/com/vaadin/server/DefaultDeploymentConfiguration.java @@ -32,7 +32,7 @@ public class DefaultDeploymentConfiguration implements DeploymentConfiguration { private boolean xsrfProtectionEnabled; private int resourceCacheTime; private int heartbeatInterval; - private boolean idleUICleanupEnabled; + private boolean closeIdleSessions; private final Class systemPropertyBaseClass; /** @@ -54,7 +54,7 @@ public class DefaultDeploymentConfiguration implements DeploymentConfiguration { checkXsrfProtection(); checkResourceCacheTime(); checkHeartbeatInterval(); - checkIdleUICleanup(); + checkCloseIdleSessions(); } @Override @@ -168,8 +168,8 @@ public class DefaultDeploymentConfiguration implements DeploymentConfiguration { } @Override - public boolean isIdleUICleanupEnabled() { - return idleUICleanupEnabled; + public boolean isCloseIdleSessions() { + return closeIdleSessions; } /** @@ -225,10 +225,10 @@ public class DefaultDeploymentConfiguration implements DeploymentConfiguration { } } - private void checkIdleUICleanup() { - idleUICleanupEnabled = getApplicationOrSystemProperty( - Constants.SERVLET_PARAMETER_CLOSE_IDLE_UIS, "false").equals( - "true"); + private void checkCloseIdleSessions() { + closeIdleSessions = getApplicationOrSystemProperty( + Constants.SERVLET_PARAMETER_CLOSE_IDLE_SESSIONS, "false") + .equals("true"); } private Logger getLogger() { diff --git a/server/src/com/vaadin/server/DeploymentConfiguration.java b/server/src/com/vaadin/server/DeploymentConfiguration.java index 09405d6004..65f0393883 100644 --- a/server/src/com/vaadin/server/DeploymentConfiguration.java +++ b/server/src/com/vaadin/server/DeploymentConfiguration.java @@ -58,19 +58,24 @@ public interface DeploymentConfiguration extends Serializable { public int getHeartbeatInterval(); /** - * Returns whether UIs that have no other activity than heartbeat requests - * should be removed from the session after they have been idle the maximum - * inactivity time enforced by the session. + * Returns whether a session should be closed when all its open UIs have + * been idle for longer than its configured maximum inactivity time. + *

+ * A UI is idle if it is open on the client side but has no activity other + * than heartbeat requests. If {@code isCloseIdleSessions() == false}, + * heartbeat requests cause the session to stay open for as long as there + * are open UIs on the client side. If it is {@code true}, the session is + * eventually closed if the open UIs do not have any user interaction. * * @see WrappedSession#getMaxInactiveInterval() * * @since 7.0.0 * - * @return True if UIs receiving only heartbeat requests are eventually - * removed; false if heartbeat requests extend UI lifetime - * indefinitely. + * @return True if UIs and sessions receiving only heartbeat requests are + * eventually closed; false if heartbeat requests extend UI and + * session lifetime indefinitely. */ - public boolean isIdleUICleanupEnabled(); + public boolean isCloseIdleSessions(); /** * Gets the properties configured for the deployment, e.g. as init diff --git a/server/src/com/vaadin/server/RequestTimer.java b/server/src/com/vaadin/server/RequestTimer.java index 26a5689665..bfe5362afe 100644 --- a/server/src/com/vaadin/server/RequestTimer.java +++ b/server/src/com/vaadin/server/RequestTimer.java @@ -50,6 +50,6 @@ public class RequestTimer implements Serializable { // The timings must be stored in the context, since a new // RequestTimer is created for every request. - context.setLastRequestTime(time); + context.setLastRequestDuration(time); } } diff --git a/server/src/com/vaadin/server/VaadinSession.java b/server/src/com/vaadin/server/VaadinSession.java index d2eb1a436a..08b408fa28 100644 --- a/server/src/com/vaadin/server/VaadinSession.java +++ b/server/src/com/vaadin/server/VaadinSession.java @@ -113,9 +113,11 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { private AbstractCommunicationManager communicationManager; - private long totalSessionTime = 0; + private long cumulativeRequestDuration = 0; - private long lastRequestTime = -1; + private long lastRequestDuration = -1; + + private long lastRequestTimestamp = System.currentTimeMillis(); private transient WrappedSession session; @@ -167,10 +169,11 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { } /** - * @return The total time spent servicing requests in this session. + * @return The total time spent servicing requests in this session, in + * milliseconds. */ - public long getTotalSessionTime() { - return totalSessionTime; + public long getCumulativeRequestDuration() { + return cumulativeRequestDuration; } /** @@ -178,18 +181,31 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { * the total time spent servicing requests in this session. * * @param time - * the time spent in the last request. + * The time spent in the last request, in milliseconds. + */ + public void setLastRequestDuration(long time) { + lastRequestDuration = time; + cumulativeRequestDuration += time; + } + + /** + * @return The time spent servicing the last request in this session, in + * milliseconds. */ - public void setLastRequestTime(long time) { - lastRequestTime = time; - totalSessionTime += time; + public long getLastRequestDuration() { + return lastRequestDuration; } /** - * @return the time spent servicing the last request in this session. + * Sets the time when the last UIDL request was serviced in this session. + * + * @param timestamp + * The time when the last request was handled, in milliseconds + * since the epoch. + * */ - public long getLastRequestTime() { - return lastRequestTime; + public void setLastRequestTimestamp(long timestamp) { + lastRequestTimestamp = timestamp; } /** @@ -596,11 +612,17 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { */ @Deprecated public void cleanupInactiveUIs() { - for (UI ui : new ArrayList(uIs.values())) { - if (!isUIAlive(ui)) { - cleanupUI(ui); - getLogger().fine( - "Closed UI #" + ui.getUIId() + " due to inactivity"); + if (getUidlRequestTimeout() >= 0 + && System.currentTimeMillis() - lastRequestTimestamp > 1000 * getUidlRequestTimeout()) { + close(); + } else { + for (UI ui : new ArrayList(uIs.values())) { + if (!isUIAlive(ui)) { + cleanupUI(ui); + getLogger() + .fine("Closed UI #" + ui.getUIId() + + " due to inactivity"); + } } } } @@ -655,7 +677,7 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { * otherwise heartbeat requests are enough to extend UI lifetime * indefinitely. * - * @see DeploymentConfiguration#isIdleUICleanupEnabled() + * @see DeploymentConfiguration#isCloseIdleSessions() * @see #getHeartbeatTimeout() * @see #cleanupInactiveUIs() * @@ -665,7 +687,7 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { * timeout never occurs. */ protected int getUidlRequestTimeout() { - return configuration.isIdleUICleanupEnabled() ? getSession() + return configuration.isCloseIdleSessions() ? getSession() .getMaxInactiveInterval() : -1; } @@ -688,11 +710,7 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { protected boolean isUIAlive(UI ui) { long now = System.currentTimeMillis(); if (getHeartbeatTimeout() >= 0 - && now - ui.getLastHeartbeatTime() > 1000 * getHeartbeatTimeout()) { - return false; - } - if (getUidlRequestTimeout() >= 0 - && now - ui.getLastUidlRequestTime() > 1000 * getUidlRequestTimeout()) { + && now - ui.getLastHeartbeatTimestamp() > 1000 * getHeartbeatTimeout()) { return false; } return true; diff --git a/server/src/com/vaadin/ui/UI.java b/server/src/com/vaadin/ui/UI.java index 342983e8b8..83936a7478 100644 --- a/server/src/com/vaadin/ui/UI.java +++ b/server/src/com/vaadin/ui/UI.java @@ -172,9 +172,7 @@ public abstract class UI extends AbstractSingleComponentContainer implements * current time whenever the application receives a heartbeat or UIDL * request from the client for this UI. */ - private long lastHeartbeat = System.currentTimeMillis(); - - private long lastUidlRequest = System.currentTimeMillis(); + private long lastHeartbeatTimestamp = System.currentTimeMillis(); /** * Creates a new empty UI without a caption. The content of the UI must be @@ -942,43 +940,29 @@ public abstract class UI extends AbstractSingleComponentContainer implements } /** - * Returns the timestamp (milliseconds since the epoch) of the last received - * heartbeat for this UI. + * Returns the timestamp of the last received heartbeat for this UI. * * @see #heartbeat() * @see VaadinSession#cleanupInactiveUIs() * - * @return The time the last heartbeat request occurred. - */ - public long getLastHeartbeatTime() { - return lastHeartbeat; - } - - /** - * Returns the timestamp (milliseconds since the epoch) of the last received - * UIDL request for this UI. - * - * @return + * @return The time the last heartbeat request occurred, in milliseconds + * since the epoch. */ - public long getLastUidlRequestTime() { - return lastUidlRequest; + public long getLastHeartbeatTimestamp() { + return lastHeartbeatTimestamp; } /** * Sets the last heartbeat request timestamp for this UI. Called by the * framework whenever the application receives a valid heartbeat request for * this UI. + * + * @param lastHeartbeat + * The time the last heartbeat request occurred, in milliseconds + * since the epoch. */ - public void setLastHeartbeatTime(long lastHeartbeat) { - this.lastHeartbeat = lastHeartbeat; - } - - /** - * Sets the last UIDL request timestamp for this UI. Called by the framework - * whenever the application receives a valid UIDL request for this UI. - */ - public void setLastUidlRequestTime(long lastUidlRequest) { - this.lastUidlRequest = lastUidlRequest; + public void setLastHeartbeatTimestamp(long lastHeartbeat) { + lastHeartbeatTimestamp = lastHeartbeat; } /** diff --git a/uitest/src/com/vaadin/tests/application/DeploymentConfigurationTest.html b/uitest/src/com/vaadin/tests/application/DeploymentConfigurationTest.html index f5ad0987aa..2bdb7c79a5 100644 --- a/uitest/src/com/vaadin/tests/application/DeploymentConfigurationTest.html +++ b/uitest/src/com/vaadin/tests/application/DeploymentConfigurationTest.html @@ -18,7 +18,7 @@ assertTextPresent - exact:closeIdleUIs: true + exact:closeIdleSessions: true -- 2.39.5