Support suspend timeout for long polling (#18550)

Change-Id: I1dc35f060b255baff4d28a815414836d4852218b

7 files changed, 98 insertions, 8 deletions

diff --git a/WebContent/WEB-INF/web.xml b/WebContent/WEB-INF/web.xml
index 51ac907843..a9d3db30b3 100644
--- a/WebContent/WEB-INF/web.xml
+++ b/WebContent/WEB-INF/web.xml
@@ -115,6 +115,16 @@
         <async-supported>true</async-supported>
     </servlet>
 
+    <servlet>
+        <servlet-name>VaadinApplicationRunnerWithPushTimeout</servlet-name>
+        <servlet-class>com.vaadin.launcher.ApplicationRunnerServlet</servlet-class>
+        <init-param>
+            <param-name>pushLongPollingSuspendTimeout</param-name>
+            <param-value>10000</param-value>
+        </init-param>
+        <async-supported>true</async-supported>
+    </servlet>
+
     <!-- For testing GAE - the deployment script changes this to use GAEVaadinServlet -->
     <servlet>
         <servlet-name>IntegrationTest</servlet-name>
@@ -160,6 +170,11 @@
     </servlet-mapping>
 
     <servlet-mapping>
+        <servlet-name>VaadinApplicationRunnerWithPushTimeout</servlet-name>
+        <url-pattern>/run-push-timeout/*</url-pattern>
+    </servlet-mapping>
+
+    <servlet-mapping>
         <servlet-name>VaadinApplicationRunnerWithJSR356</servlet-name>
         <url-pattern>/run-jsr356/*</url-pattern>
     </servlet-mapping>
diff --git a/server/src/com/vaadin/server/Constants.java b/server/src/com/vaadin/server/Constants.java
index 5a0d852299..77a1a3134e 100644
--- a/server/src/com/vaadin/server/Constants.java
+++ b/server/src/com/vaadin/server/Constants.java
@@ -137,6 +137,7 @@ public interface Constants {
     static final String SERVLET_PARAMETER_LEGACY_PROPERTY_TOSTRING = "legacyPropertyToString";
     static final String SERVLET_PARAMETER_SYNC_ID_CHECK = "syncIdCheck";
     static final String SERVLET_PARAMETER_SENDURLSASPARAMETERS = "sendUrlsAsParameters";
+    static final String SERVLET_PARAMETER_PUSH_SUSPEND_TIMEOUT_LONGPOLLING = "pushLongPollingSuspendTimeout";
 
     // Configurable parameter names
     static final String PARAMETER_VAADIN_RESOURCES = "Resources";
diff --git a/server/src/com/vaadin/server/communication/PushHandler.java b/server/src/com/vaadin/server/communication/PushHandler.java
index 01077c3f86..994415a0b4 100644
--- a/server/src/com/vaadin/server/communication/PushHandler.java
+++ b/server/src/com/vaadin/server/communication/PushHandler.java
@@ -55,6 +55,8 @@ import elemental.json.JsonException;
  */
 public class PushHandler {
 
+    private int longPollingSuspendTimeout = -1;
+
     /**
      * Callback interface used internally to process an event with the
      * corresponding UI properly locked.
@@ -107,7 +109,7 @@ public class PushHandler {
                 return;
             }
 
-            resource.suspend();
+            suspend(resource);
 
             AtmospherePushConnection connection = getConnectionForUI(ui);
             assert (connection != null);
@@ -174,6 +176,21 @@ public class PushHandler {
     }
 
     /**
+     * Suspends the given resource
+     * 
+     * @since
+     * @param resource
+     *            the resource to suspend
+     */
+    protected void suspend(AtmosphereResource resource) {
+        if (resource.transport() == TRANSPORT.LONG_POLLING) {
+            resource.suspend(getLongPollingSuspendTimeout());
+        } else {
+            resource.suspend(-1);
+        }
+    }
+
+    /**
      * Find the UI for the atmosphere resource, lock it and invoke the callback.
      * 
      * @param resource
@@ -493,4 +510,26 @@ public class PushHandler {
                 resource.transport() == TRANSPORT.WEBSOCKET);
     }
 
+    /**
+     * Sets the timeout used for suspend calls when using long polling.
+     * 
+     * If you are using a proxy with a defined idle timeout, set the suspend
+     * timeout to a value smaller than the proxy timeout so that the server is
+     * aware of a reconnect taking place.
+     * 
+     * @param suspendTimeout
+     *            the timeout to use for suspended AtmosphereResources
+     */
+    public void setLongPollingSuspendTimeout(int longPollingSuspendTimeout) {
+        this.longPollingSuspendTimeout = longPollingSuspendTimeout;
+    }
+
+    /**
+     * Gets the timeout used for suspend calls when using long polling.
+     * 
+     * @return the timeout to use for suspended AtmosphereResources
+     */
+    public int getLongPollingSuspendTimeout() {
+        return longPollingSuspendTimeout;
+    }
 }
diff --git a/server/src/com/vaadin/server/communication/PushRequestHandler.java b/server/src/com/vaadin/server/communication/PushRequestHandler.java
index c01c74e5cd..c44fcd9ef3 100644
--- a/server/src/com/vaadin/server/communication/PushRequestHandler.java
+++ b/server/src/com/vaadin/server/communication/PushRequestHandler.java
@@ -77,7 +77,7 @@ public class PushRequestHandler implements RequestHandler,
         final ServletConfig vaadinServletConfig = service.getServlet()
                 .getServletConfig();
 
-        pushHandler = new PushHandler(service);
+        pushHandler = createPushHandler(service);
 
         atmosphere = getPreInitializedAtmosphere(vaadinServletConfig);
         if (atmosphere == null) {
@@ -100,7 +100,12 @@ public class PushRequestHandler implements RequestHandler,
                     "Using pre-initialized Atmosphere for servlet "
                             + vaadinServletConfig.getServletName());
         }
-
+        pushHandler
+                .setLongPollingSuspendTimeout(atmosphere
+                        .getAtmosphereConfig()
+                        .getInitParameter(
+                                com.vaadin.server.Constants.SERVLET_PARAMETER_PUSH_SUSPEND_TIMEOUT_LONGPOLLING,
+                                -1));
         for (AtmosphereHandlerWrapper handlerWrapper : atmosphere
                 .getAtmosphereHandlers().values()) {
             AtmosphereHandler handler = handlerWrapper.atmosphereHandler;
@@ -113,6 +118,22 @@ public class PushRequestHandler implements RequestHandler,
         }
     }
 
+    /**
+     * Creates a push handler for this request handler.
+     * <p>
+     * Create your own request handler and override this method if you want to
+     * customize the {@link PushHandler}, e.g. to dynamically decide the suspend
+     * timeout.
+     * 
+     * @since
+     * @param service
+     *            the vaadin service
+     * @return the push handler to use for this service
+     */
+    protected PushHandler createPushHandler(VaadinServletService service) {
+        return new PushHandler(service);
+    }
+
     private static final Logger getLogger() {
         return Logger.getLogger(PushRequestHandler.class.getName());
     }
diff --git a/uitest/src/com/vaadin/tests/push/BasicPushLongPollingTest.java b/uitest/src/com/vaadin/tests/push/BasicPushLongPollingTest.java
index b404747c80..a060d5a57a 100644
--- a/uitest/src/com/vaadin/tests/push/BasicPushLongPollingTest.java
+++ b/uitest/src/com/vaadin/tests/push/BasicPushLongPollingTest.java
@@ -15,5 +15,19 @@
  */
 package com.vaadin.tests.push;
 
+import org.junit.Test;
+
 public class BasicPushLongPollingTest extends BasicPushTest {
+
+    @Test
+    public void pushAfterServerTimeout() throws InterruptedException {
+        getDriver().get(
+                getTestUrl().replace("/run/", "/run-push-timeout/")
+                        + "?debug=push");
+        sleep(11000); // Wait for server timeout (10s)
+
+        getServerCounterStartButton().click();
+        waitUntilServerCounterChanges();
+    }
+
 }
diff --git a/uitest/src/com/vaadin/tests/push/BasicPushTest.java b/uitest/src/com/vaadin/tests/push/BasicPushTest.java
index 5bac54f0f7..f176008eb0 100644
--- a/uitest/src/com/vaadin/tests/push/BasicPushTest.java
+++ b/uitest/src/com/vaadin/tests/push/BasicPushTest.java
@@ -57,11 +57,11 @@ public abstract class BasicPushTest extends MultiBrowserTest {
         return Integer.parseInt(clientCounterElem.getText());
     }
 
-    private WebElement getIncrementButton() {
+    protected WebElement getIncrementButton() {
         return getIncrementButton(this);
     }
 
-    private WebElement getServerCounterStartButton() {
+    protected WebElement getServerCounterStartButton() {
         return getServerCounterStartButton(this);
     }
 
@@ -83,7 +83,7 @@ public abstract class BasicPushTest extends MultiBrowserTest {
         return t.vaadinElementById(BasicPush.INCREMENT_BUTTON_ID);
     }
 
-    private void waitUntilClientCounterChanges(final int expectedValue) {
+    protected void waitUntilClientCounterChanges(final int expectedValue) {
         waitUntil(new ExpectedCondition<Boolean>() {
 
             @Override
@@ -93,7 +93,7 @@ public abstract class BasicPushTest extends MultiBrowserTest {
         }, 10);
     }
 
-    private void waitUntilServerCounterChanges() {
+    protected void waitUntilServerCounterChanges() {
         final int counter = BasicPushTest.getServerCounter(this);
         waitUntil(new ExpectedCondition<Boolean>() {
 
diff --git a/uitest/src/com/vaadin/tests/tb3/AbstractTB3Test.java b/uitest/src/com/vaadin/tests/tb3/AbstractTB3Test.java
index 2e3854cb2b..d76cd616b1 100644
--- a/uitest/src/com/vaadin/tests/tb3/AbstractTB3Test.java
+++ b/uitest/src/com/vaadin/tests/tb3/AbstractTB3Test.java
@@ -381,7 +381,7 @@ public abstract class AbstractTB3Test extends ParallelTest {
      * @return
      */
     public WebElement vaadinElementById(String id) {
-        return driver.findElement(vaadinLocatorById(id));
+        return driver.findElement(By.id(id));
     }
 
     /**