Add support for heartbeat not extending session (#9266)

10 files changed, 147 insertions, 33 deletions

diff --git a/client/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java b/client/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java
index b4635c6d80..299278269c 100644
--- a/client/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java
+++ b/client/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java
@@ -2611,7 +2611,6 @@ public class ApplicationConnection {
      * <b>TODO</b>: Improved error handling, like in doUidlRequest().
      * 
      * @see #scheduleHeartbeat()
-     * @see com.vaadin.ui.Root#heartbeat()
      */
     protected void sendHeartbeat() {
         final String uri = addGetParameters(
diff --git a/server/src/com/vaadin/Application.java b/server/src/com/vaadin/Application.java
index c832645405..8efcadb667 100644
--- a/server/src/com/vaadin/Application.java
+++ b/server/src/com/vaadin/Application.java
@@ -2424,30 +2424,28 @@ public class Application implements Terminal.ErrorListener, Serializable {
     }
 
     /**
-     * Removes all those roots from the application whose last heartbeat
-     * occurred more than {@link #getHeartbeatTimeout()} seconds ago. Close
-     * events are fired for the removed roots. If
-     * <code>getHeartbeatTimeout()</code> returns a nonpositive number, no
-     * cleanup is performed.
+     * Removes all those roots from the application for whom
+     * {@link #isRootAlive} returns false. Close events are fired for the
+     * removed roots.
      * <p>
      * Called by the framework at the end of every request.
      * 
      * @see Root.CloseEvent
      * @see Root.CloseListener
-     * @see #getHeartbeatTimeout()
+     * @see #isRootAlive(Root)
      * 
      * @since 7.0.0
      */
     public void closeInactiveRoots() {
-        if (getHeartbeatTimeout() > 0) {
-            long now = System.currentTimeMillis();
-            for (Iterator<Root> i = roots.values().iterator(); i.hasNext();) {
-                Root root = i.next();
-                if (now - root.getLastHeartbeat() > 1000 * getHeartbeatTimeout()) {
-                    i.remove();
-                    retainOnRefreshRoots.values().remove(root.getRootId());
-                    root.fireCloseEvent();
-                }
+        for (Iterator<Root> i = roots.values().iterator(); i.hasNext();) {
+            Root root = i.next();
+            if (!isRootAlive(root)) {
+                i.remove();
+                retainOnRefreshRoots.values().remove(root.getRootId());
+                root.fireCloseEvent();
+                getLogger().info(
+                        "Closed root #" + root.getRootId()
+                                + " due to inactivity");
             }
         }
     }
@@ -2456,15 +2454,69 @@ public class Application implements Terminal.ErrorListener, Serializable {
      * Returns the number of seconds that must pass without a valid heartbeat or
      * UIDL request being received from a root before that root is removed from
      * the application. This is a lower bound; it might take longer to close an
-     * inactive root.
+     * inactive root. Returns a negative number if heartbeat is disabled and
+     * timeout never occurs.
+     * 
+     * @see #getUidlRequestTimeout()
+     * @see #closeInactiveRoots()
+     * @see DeploymentConfiguration#getHeartbeatInterval()
      * 
      * @since 7.0.0
      * 
-     * @return The heartbeat timeout in seconds or a nonpositive number if
-     *         timeout never occurs.
+     * @return The heartbeat timeout in seconds or a negative number if timeout
+     *         never occurs.
      */
-    public int getHeartbeatTimeout() {
+    protected int getHeartbeatTimeout() {
         // Permit three missed heartbeats before closing the root
         return (int) (configuration.getHeartbeatInterval() * (3.1));
     }
+
+    /**
+     * Returns the number of seconds that must pass without a valid UIDL request
+     * being received from a root before the root is removed from the
+     * application, even though heartbeat requests are received. This is a lower
+     * bound; it might take longer to close an inactive root. Returns a negative
+     * number if
+     * <p>
+     * This timeout only has effect if cleanup of inactive roots is enabled;
+     * otherwise heartbeat requests are enough to extend root lifetime
+     * indefinitely.
+     * 
+     * @see DeploymentConfiguration#isIdleRootCleanupEnabled()
+     * @see #getHeartbeatTimeout()
+     * @see #closeInactiveRoots()
+     * 
+     * @since 7.0.0
+     * 
+     * @return The UIDL request timeout in seconds, or a negative number if
+     *         timeout never occurs.
+     */
+    protected int getUidlRequestTimeout() {
+        return configuration.isIdleRootCleanupEnabled() ? getContext()
+                .getMaxInactiveInterval() : -1;
+    }
+
+    /**
+     * Returns whether the given root is alive (the client-side actively
+     * communicates with the server) or whether it can be removed from the
+     * application and eventually collected.
+     * 
+     * @since 7.0.0
+     * 
+     * @param root
+     *            The Root whose status to check
+     * @return true if the root is alive, false if it could be removed.
+     */
+    protected boolean isRootAlive(Root root) {
+        long now = System.currentTimeMillis();
+        if (getHeartbeatTimeout() >= 0
+                && now - root.getLastHeartbeatTime() > 1000 * getHeartbeatTimeout()) {
+            return false;
+        }
+        if (getUidlRequestTimeout() >= 0
+                && now - root.getLastUidlRequestTime() > 1000 * getUidlRequestTimeout()) {
+            return false;
+        }
+        return true;
+    }
 }
diff --git a/server/src/com/vaadin/service/ApplicationContext.java b/server/src/com/vaadin/service/ApplicationContext.java
index c6116d6e73..55495dcd5c 100644
--- a/server/src/com/vaadin/service/ApplicationContext.java
+++ b/server/src/com/vaadin/service/ApplicationContext.java
@@ -80,6 +80,12 @@ public interface ApplicationContext extends Serializable {
     public void removeTransactionListener(TransactionListener listener);
 
     /**
+     * Returns the time between requests, in seconds, before this context is
+     * invalidated. A negative time indicates the context should never timeout.
+     */
+    public int getMaxInactiveInterval();
+
+    /**
      * Generate a URL that can be used as the relative location of e.g. an
      * {@link ApplicationResource}.
      * 
diff --git a/server/src/com/vaadin/terminal/DeploymentConfiguration.java b/server/src/com/vaadin/terminal/DeploymentConfiguration.java
index 8d5686ebbd..d3fd4567f2 100644
--- a/server/src/com/vaadin/terminal/DeploymentConfiguration.java
+++ b/server/src/com/vaadin/terminal/DeploymentConfiguration.java
@@ -23,6 +23,7 @@ import java.util.Properties;
 import javax.portlet.PortletContext;
 import javax.servlet.ServletContext;
 
+import com.vaadin.service.ApplicationContext;
 import com.vaadin.terminal.gwt.server.AddonContext;
 import com.vaadin.terminal.gwt.server.AddonContextListener;
 
@@ -162,11 +163,26 @@ public interface DeploymentConfiguration extends Serializable {
 
     /**
      * Returns the number of seconds between heartbeat requests of a root, or a
-     * non-negative number if heartbeat is disabled.
+     * non-positive number if heartbeat is disabled.
      * 
      * @since 7.0.0
      * 
-     * @return
+     * @return The time between heartbeats.
      */
     public int getHeartbeatInterval();
+
+    /**
+     * Returns whether roots that have no other activity than heartbeat requests
+     * should be closed after they have been idle the maximum inactivity time
+     * enforced by the session.
+     * 
+     * @see ApplicationContext#getMaxInactiveInterval()
+     * 
+     * @since 7.0.0
+     * 
+     * @return True if roots receiving only heartbeat requests are eventually
+     *         closed; false if heartbeat requests extend root lifetime
+     *         indefinitely.
+     */
+    public boolean isIdleRootCleanupEnabled();
 }
diff --git a/server/src/com/vaadin/terminal/gwt/server/AbstractCommunicationManager.java b/server/src/com/vaadin/terminal/gwt/server/AbstractCommunicationManager.java
index 8bb14af45f..b5ea6a8735 100644
--- a/server/src/com/vaadin/terminal/gwt/server/AbstractCommunicationManager.java
+++ b/server/src/com/vaadin/terminal/gwt/server/AbstractCommunicationManager.java
@@ -581,7 +581,7 @@ public abstract class AbstractCommunicationManager implements Serializable {
             }
 
             // Keep the root alive
-            root.heartbeat();
+            root.setLastUidlRequestTime(System.currentTimeMillis());
 
             // Change all variables based on request parameters
             if (!handleVariables(request, response, callback, application, root)) {
@@ -2683,7 +2683,7 @@ public abstract class AbstractCommunicationManager implements Serializable {
             // null-check below handles this as well
         }
         if (root != null) {
-            root.heartbeat();
+            root.setLastHeartbeatTime(System.currentTimeMillis());
         } else {
             response.sendError(HttpServletResponse.SC_NOT_FOUND,
                     "Root not found");
diff --git a/server/src/com/vaadin/terminal/gwt/server/AbstractDeploymentConfiguration.java b/server/src/com/vaadin/terminal/gwt/server/AbstractDeploymentConfiguration.java
index 1895560209..30ba82f664 100644
--- a/server/src/com/vaadin/terminal/gwt/server/AbstractDeploymentConfiguration.java
+++ b/server/src/com/vaadin/terminal/gwt/server/AbstractDeploymentConfiguration.java
@@ -34,6 +34,7 @@ public abstract class AbstractDeploymentConfiguration implements
     private boolean xsrfProtectionEnabled;
     private int resourceCacheTime;
     private int heartbeatInterval;
+    private boolean idleRootCleanupEnabled;
 
     public AbstractDeploymentConfiguration(Class<?> systemPropertyBaseClass,
             Properties applicationProperties) {
@@ -44,6 +45,7 @@ public abstract class AbstractDeploymentConfiguration implements
         checkXsrfProtection();
         checkResourceCacheTime();
         checkHeartbeatInterval();
+        checkIdleRootCleanup();
     }
 
     @Override
@@ -204,6 +206,11 @@ public abstract class AbstractDeploymentConfiguration implements
         return heartbeatInterval;
     }
 
+    @Override
+    public boolean isIdleRootCleanupEnabled() {
+        return idleRootCleanupEnabled;
+    }
+
     /**
      * Log a warning if Vaadin is not running in production mode.
      */
@@ -256,6 +263,12 @@ public abstract class AbstractDeploymentConfiguration implements
         }
     }
 
+    private void checkIdleRootCleanup() {
+        idleRootCleanupEnabled = getApplicationOrSystemProperty(
+                Constants.SERVLET_PARAMETER_CLOSE_IDLE_ROOTS, "false").equals(
+                "true");
+    }
+
     private Logger getLogger() {
         return Logger.getLogger(getClass().getName());
     }
diff --git a/server/src/com/vaadin/terminal/gwt/server/Constants.java b/server/src/com/vaadin/terminal/gwt/server/Constants.java
index a04288055e..adf26bf7f7 100644
--- a/server/src/com/vaadin/terminal/gwt/server/Constants.java
+++ b/server/src/com/vaadin/terminal/gwt/server/Constants.java
@@ -65,6 +65,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_RATE = "heartbeatRate";
+    static final String SERVLET_PARAMETER_CLOSE_IDLE_ROOTS = "closeIdleRoots";
 
     // Configurable parameter names
     static final String PARAMETER_VAADIN_RESOURCES = "Resources";
@@ -95,5 +96,4 @@ public interface Constants {
     static final String PORTAL_PARAMETER_VAADIN_WIDGETSET = "vaadin.widgetset";
     static final String PORTAL_PARAMETER_VAADIN_RESOURCE_PATH = "vaadin.resources.path";
     static final String PORTAL_PARAMETER_VAADIN_THEME = "vaadin.theme";
-
 }
diff --git a/server/src/com/vaadin/terminal/gwt/server/PortletApplicationContext2.java b/server/src/com/vaadin/terminal/gwt/server/PortletApplicationContext2.java
index eba7d6e3a3..a7ee5be96d 100644
--- a/server/src/com/vaadin/terminal/gwt/server/PortletApplicationContext2.java
+++ b/server/src/com/vaadin/terminal/gwt/server/PortletApplicationContext2.java
@@ -404,6 +404,11 @@ public class PortletApplicationContext2 extends AbstractWebApplicationContext {
         }
     }
 
+    @Override
+    public int getMaxInactiveInterval() {
+        return getPortletSession().getMaxInactiveInterval();
+    }
+
     private Logger getLogger() {
         return Logger.getLogger(PortletApplicationContext2.class.getName());
     }
diff --git a/server/src/com/vaadin/terminal/gwt/server/WebApplicationContext.java b/server/src/com/vaadin/terminal/gwt/server/WebApplicationContext.java
index 4cc0ed188d..bfcc0c1038 100644
--- a/server/src/com/vaadin/terminal/gwt/server/WebApplicationContext.java
+++ b/server/src/com/vaadin/terminal/gwt/server/WebApplicationContext.java
@@ -187,4 +187,8 @@ public class WebApplicationContext extends AbstractWebApplicationContext {
         return mgr;
     }
 
+    @Override
+    public int getMaxInactiveInterval() {
+        return getHttpSession().getMaxInactiveInterval();
+    }
 }
diff --git a/server/src/com/vaadin/ui/Root.java b/server/src/com/vaadin/ui/Root.java
index 14abf0f24f..98c200cd34 100644
--- a/server/src/com/vaadin/ui/Root.java
+++ b/server/src/com/vaadin/ui/Root.java
@@ -488,6 +488,8 @@ public abstract class Root extends AbstractComponentContainer implements
      */
     private long lastHeartbeat = System.currentTimeMillis();
 
+    private long lastUidlRequest = System.currentTimeMillis();
+
     /**
      * Creates a new empty root without a caption. This root will have a
      * {@link VerticalLayout} with margins enabled as its content.
@@ -1319,26 +1321,43 @@ public abstract class Root extends AbstractComponentContainer implements
     }
 
     /**
-     * Returns the timestamp (millisecond since the epoch) of the last received
+     * Returns the timestamp (milliseconds since the epoch) of the last received
      * heartbeat for this Root.
      * 
      * @see #heartbeat()
      * @see Application#closeInactiveRoots()
      * 
-     * @return The time
+     * @return The time the last heartbeat request occurred.
      */
-    public long getLastHeartbeat() {
+    public long getLastHeartbeatTime() {
         return lastHeartbeat;
     }
 
     /**
-     * Updates the heartbeat timestamp of this Root to the current time. Called
-     * by the framework whenever the application receives a valid heartbeat or
+     * Returns the timestamp (milliseconds since the epoch) of the last received
      * UIDL request for this Root.
      * 
-     * @see java.lang.System#currentTimeMillis()
+     * @return
+     */
+    public long getLastUidlRequestTime() {
+        return lastUidlRequest;
+    }
+
+    /**
+     * Sets the last heartbeat request timestamp for this Root. Called by the
+     * framework whenever the application receives a valid heartbeat request for
+     * this Root.
+     */
+    public void setLastHeartbeatTime(long lastHeartbeat) {
+        this.lastHeartbeat = lastHeartbeat;
+    }
+
+    /**
+     * Sets the last UIDL request timestamp for this Root. Called by the
+     * framework whenever the application receives a valid UIDL request for this
+     * Root.
      */
-    public void heartbeat() {
-        this.lastHeartbeat = System.currentTimeMillis();
+    public void setLastUidlRequestTime(long lastUidlRequest) {
+        this.lastUidlRequest = lastUidlRequest;
     }
 }