Close push connection when UI is detached (#11596)

Change-Id: Ibfc5923406b386786ae399b7f53cea47ac885f48

9 files changed, 275 insertions, 4 deletions

diff --git a/client/src/com/vaadin/client/ApplicationConnection.java b/client/src/com/vaadin/client/ApplicationConnection.java
index 85cf0f0b46..d77a98a83b 100644
--- a/client/src/com/vaadin/client/ApplicationConnection.java
+++ b/client/src/com/vaadin/client/ApplicationConnection.java
@@ -1026,7 +1026,7 @@ public class ApplicationConnection {
      * @param details
      *            Optional details for debugging.
      */
-    protected void showSessionExpiredError(String details) {
+    public void showSessionExpiredError(String details) {
         VConsole.error("Session expired: " + details);
         showError(details, configuration.getSessionExpiredError());
     }
diff --git a/client/src/com/vaadin/client/ui/ui/UIConnector.java b/client/src/com/vaadin/client/ui/ui/UIConnector.java
index 601349d244..079e133438 100644
--- a/client/src/com/vaadin/client/ui/ui/UIConnector.java
+++ b/client/src/com/vaadin/client/ui/ui/UIConnector.java
@@ -69,6 +69,7 @@ import com.vaadin.shared.ui.Connect;
 import com.vaadin.shared.ui.Connect.LoadStyle;
 import com.vaadin.shared.ui.ui.PageClientRpc;
 import com.vaadin.shared.ui.ui.ScrollClientRpc;
+import com.vaadin.shared.ui.ui.UIClientRpc;
 import com.vaadin.shared.ui.ui.UIConstants;
 import com.vaadin.shared.ui.ui.UIServerRpc;
 import com.vaadin.shared.ui.ui.UIState;
@@ -115,6 +116,22 @@ public class UIConnector extends AbstractSingleComponentContainerConnector
                 getWidget().getElement().setScrollLeft(scrollLeft);
             }
         });
+        registerRpc(UIClientRpc.class, new UIClientRpc() {
+            @Override
+            public void uiClosed(final boolean sessionExpired) {
+                Scheduler.get().scheduleDeferred(new ScheduledCommand() {
+                    @Override
+                    public void execute() {
+                        if (sessionExpired) {
+                            getConnection().showSessionExpiredError(null);
+                        } else {
+                            getState().enabled = false;
+                            updateEnabledState(getState().enabled);
+                        }
+                    }
+                });
+            }
+        });
         getWidget().addResizeHandler(new ResizeHandler() {
             @Override
             public void onResize(ResizeEvent event) {
diff --git a/server/src/com/vaadin/server/VaadinSession.java b/server/src/com/vaadin/server/VaadinSession.java
index e9b3eb6ada..9c803924e0 100644
--- a/server/src/com/vaadin/server/VaadinSession.java
+++ b/server/src/com/vaadin/server/VaadinSession.java
@@ -321,7 +321,7 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable {
     @Deprecated
     public void removeFromSession(VaadinService service) {
         assert hasLock();
-        session.setAttribute(getSessionAttributeName(service), null);
+        session.removeAttribute(getSessionAttributeName(service));
     }
 
     /**
diff --git a/server/src/com/vaadin/server/communication/AtmospherePushConnection.java b/server/src/com/vaadin/server/communication/AtmospherePushConnection.java
index a5025e2356..770b0b6a59 100644
--- a/server/src/com/vaadin/server/communication/AtmospherePushConnection.java
+++ b/server/src/com/vaadin/server/communication/AtmospherePushConnection.java
@@ -20,6 +20,7 @@ import java.io.IOException;
 import java.io.Serializable;
 import java.io.StringWriter;
 import java.io.Writer;
+import java.util.concurrent.Future;
 
 import org.atmosphere.cpr.AtmosphereResource;
 import org.json.JSONException;
@@ -37,6 +38,7 @@ public class AtmospherePushConnection implements Serializable, PushConnection {
 
     private UI ui;
     private transient AtmosphereResource resource;
+    private Future<String> lastMessage;
 
     public AtmospherePushConnection(UI ui) {
         this.ui = ui;
@@ -79,7 +81,8 @@ public class AtmospherePushConnection implements Serializable, PushConnection {
      *            The message to send
      */
     void sendMessage(String message) {
-        getResource().getBroadcaster().broadcast(message, getResource());
+        lastMessage = getResource().getBroadcaster().broadcast(message,
+                getResource());
     }
 
     /**
@@ -97,7 +100,8 @@ public class AtmospherePushConnection implements Serializable, PushConnection {
     /**
      * Returns whether this connection is currently open.
      */
-    protected boolean isConnected() {
+    @Override
+    public boolean isConnected() {
         return resource != null
                 && resource.getBroadcaster().getAtmosphereResources()
                         .contains(resource);
@@ -120,6 +124,17 @@ public class AtmospherePushConnection implements Serializable, PushConnection {
 
     @Override
     public void disconnect() {
+        if (lastMessage != null) {
+            try {
+                // Wait for the last message to be sent before closing the
+                // connection (assumes that futures are completed in order)
+                lastMessage.get();
+            } catch (Exception e) {
+                e.printStackTrace();
+            }
+            lastMessage = null;
+        }
+
         resource.resume();
         assert !resource.getBroadcaster().getAtmosphereResources()
                 .contains(resource);
diff --git a/server/src/com/vaadin/server/communication/PushConnection.java b/server/src/com/vaadin/server/communication/PushConnection.java
index eecf4d93a4..4e043f565f 100644
--- a/server/src/com/vaadin/server/communication/PushConnection.java
+++ b/server/src/com/vaadin/server/communication/PushConnection.java
@@ -40,4 +40,9 @@ public interface PushConnection {
      */
     public void disconnect();
 
+    /**
+     * Returns whether this connection is currently open.
+     */
+    public boolean isConnected();
+
 }
\ No newline at end of file
diff --git a/server/src/com/vaadin/ui/UI.java b/server/src/com/vaadin/ui/UI.java
index 961ed289f3..442edfebb1 100644
--- a/server/src/com/vaadin/ui/UI.java
+++ b/server/src/com/vaadin/ui/UI.java
@@ -42,6 +42,7 @@ import com.vaadin.shared.EventId;
 import com.vaadin.shared.MouseEventDetails;
 import com.vaadin.shared.communication.PushMode;
 import com.vaadin.shared.ui.ui.ScrollClientRpc;
+import com.vaadin.shared.ui.ui.UIClientRpc;
 import com.vaadin.shared.ui.ui.UIConstants;
 import com.vaadin.shared.ui.ui.UIServerRpc;
 import com.vaadin.shared.ui.ui.UIState;
@@ -363,6 +364,12 @@ public abstract class UI extends AbstractSingleComponentContainer implements
         } else {
             if (session == null) {
                 detach();
+                if (pushConnection != null && pushConnection.isConnected()) {
+                    // Close the push connection when UI is detached. Otherwise
+                    // the push connection and possibly VaadinSession will live
+                    // on.
+                    pushConnection.disconnect();
+                }
             }
             this.session = session;
         }
@@ -1006,6 +1013,15 @@ public abstract class UI extends AbstractSingleComponentContainer implements
      */
     public void close() {
         closing = true;
+
+        boolean sessionExpired = (session == null || session.isClosing());
+        getRpcProxy(UIClientRpc.class).uiClosed(sessionExpired);
+        if (getPushConnection() != null && getPushConnection().isConnected()) {
+            // Push the Rpc to the client. The connection will be closed when
+            // the UI is detached and cleaned up.
+            getPushConnection().push();
+        }
+
     }
 
     /**
diff --git a/shared/src/com/vaadin/shared/ui/ui/UIClientRpc.java b/shared/src/com/vaadin/shared/ui/ui/UIClientRpc.java
new file mode 100644
index 0000000000..3067b10e24
--- /dev/null
+++ b/shared/src/com/vaadin/shared/ui/ui/UIClientRpc.java
@@ -0,0 +1,34 @@
+/*
+ * Copyright 2000-2013 Vaadin Ltd.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.shared.ui.ui;
+
+import com.vaadin.shared.communication.ClientRpc;
+
+/**
+ * Server to Client RPC methods for UI
+ * 
+ * @since 7.1
+ * @author Vaadin Ltd
+ */
+public interface UIClientRpc extends ClientRpc {
+
+    /**
+     * @since
+     * @param sessionExpired
+     */
+    void uiClosed(boolean sessionExpired);
+
+}
diff --git a/uitest/src/com/vaadin/tests/applicationcontext/CloseUI.java b/uitest/src/com/vaadin/tests/applicationcontext/CloseUI.java
new file mode 100644
index 0000000000..6d8be4339d
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/applicationcontext/CloseUI.java
@@ -0,0 +1,160 @@
+/*
+ * Copyright 2012 Vaadin Ltd.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package com.vaadin.tests.applicationcontext;
+
+import com.vaadin.server.Page;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.server.VaadinService;
+import com.vaadin.server.VaadinSession;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.tests.util.Log;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.UI;
+
+public class CloseUI extends AbstractTestUI {
+    private static final String OLD_HASH_PARAM = "oldHash";
+    private static final String OLD_SESSION_ID_PARAM = "oldSessionId";
+
+    private final Log log = new Log(6);
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        System.out.println("UI " + getUIId() + " inited");
+
+        final int sessionHash = getSession().hashCode();
+        final String sessionId = request.getWrappedSession().getId();
+
+        log.log("Current session hashcode: " + sessionHash);
+        log.log("Current WrappedSession id: " + sessionId);
+
+        // Log previous values to make it easier to see what has changed
+        String oldHashValue = request.getParameter(OLD_HASH_PARAM);
+        if (oldHashValue != null) {
+            log.log("Old session hashcode: " + oldHashValue);
+            log.log("Same hash as current? "
+                    + oldHashValue.equals(Integer.toString(sessionHash)));
+        }
+
+        String oldSessionId = request.getParameter(OLD_SESSION_ID_PARAM);
+        if (oldSessionId != null) {
+            log.log("Old WrappedSession id: " + oldSessionId);
+            log.log("Same WrappedSession id? " + oldSessionId.equals(sessionId));
+        }
+
+        addComponent(log);
+        addComponent(new Button("Log 'hello'", new Button.ClickListener() {
+            @Override
+            public void buttonClick(ClickEvent event) {
+                log.log("Hello");
+            }
+        }));
+        addComponent(new Button("Close UI", new Button.ClickListener() {
+            @Override
+            public void buttonClick(ClickEvent event) {
+                close();
+            }
+        }));
+
+        addComponent(new Button("Close UI (background)",
+                new Button.ClickListener() {
+                    @Override
+                    public void buttonClick(ClickEvent event) {
+                        new UIRunSafelyThread(CloseUI.this) {
+                            @Override
+                            protected void runSafely() {
+                                close();
+                            };
+                        }.start();
+                    }
+                }));
+        addComponent(new Button(
+                "Close UI and redirect to /statictestfiles/static.html",
+                new Button.ClickListener() {
+                    @Override
+                    public void buttonClick(ClickEvent event) {
+                        getPage().setLocation("/statictestfiles/static.html");
+                        close();
+                    }
+                }));
+        addComponent(new Button(
+                "Close UI and redirect to /statictestfiles/static.html (background)",
+                new Button.ClickListener() {
+                    @Override
+                    public void buttonClick(ClickEvent event) {
+                        new UIRunSafelyThread(CloseUI.this) {
+
+                            @Override
+                            protected void runSafely() {
+                                getPage().setLocation(
+                                        "/statictestfiles/static.html");
+                                close();
+                            }
+                        };
+                    }
+                }));
+
+    }
+
+    private abstract class RunSafelyThread extends Thread {
+        private UI ui;
+
+        public RunSafelyThread(UI ui) {
+            this.ui = ui;
+        }
+
+        @Override
+        public void run() {
+            ui.runSafely(new Runnable() {
+
+                @Override
+                public void run() {
+                    runSafely();
+                }
+            });
+        }
+
+        protected abstract void runSafely();
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Test for closing the session and redirecting the user";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return Integer.valueOf(9859);
+    }
+
+    @Override
+    public void detach() {
+        super.detach();
+        log.log("Detach of " + this + " (" + getUIId() + ")");
+        boolean correctUI = (UI.getCurrent() == this);
+        boolean correctPage = (Page.getCurrent() == getPage());
+        boolean correctVaadinSession = (VaadinSession.getCurrent() == getSession());
+        boolean correctVaadinService = (VaadinService.getCurrent() == getSession()
+                .getService());
+        log.log("UI.current correct in detach: " + correctUI);
+        log.log("Page.current correct in detach: " + correctPage);
+        log.log("VaadinSession.current correct in detach: "
+                + correctVaadinSession);
+        log.log("VaadinService.current correct in detach: "
+                + correctVaadinService);
+    }
+}
diff --git a/uitest/src/com/vaadin/tests/applicationcontext/UIRunSafelyThread.java b/uitest/src/com/vaadin/tests/applicationcontext/UIRunSafelyThread.java
new file mode 100644
index 0000000000..0048cbd741
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/applicationcontext/UIRunSafelyThread.java
@@ -0,0 +1,24 @@
+package com.vaadin.tests.applicationcontext;
+
+import com.vaadin.ui.UI;
+
+public abstract class UIRunSafelyThread extends Thread {
+    private UI ui;
+
+    public UIRunSafelyThread(UI ui) {
+        this.ui = ui;
+    }
+
+    @Override
+    public void run() {
+        ui.runSafely(new Runnable() {
+
+            @Override
+            public void run() {
+                runSafely();
+            }
+        });
+    }
+
+    protected abstract void runSafely();
+}
\ No newline at end of file