diff options
10 files changed, 376 insertions, 27 deletions
diff --git a/client/src/com/vaadin/client/ApplicationConnection.java b/client/src/com/vaadin/client/ApplicationConnection.java index 04e3ab9dc6..bc8d82cd30 100644 --- a/client/src/com/vaadin/client/ApplicationConnection.java +++ b/client/src/com/vaadin/client/ApplicationConnection.java @@ -455,7 +455,7 @@ public class ApplicationConnection { scheduleHeartbeat(); - initializePush(); + setPushEnabled(getConfiguration().getPushMode().isEnabled()); Window.addWindowClosingHandler(new ClosingHandler() { @Override @@ -3393,8 +3393,19 @@ public class ApplicationConnection { focusedElement); } - private void initializePush() { - if (getConfiguration().getPushMode().isEnabled()) { + /** + * Sets the status for the push connection. + * + * @param enabled + * <code>true</code> to enable the push connection; + * <code>false</code> to disable the push connection. + */ + public void setPushEnabled(boolean enabled) { + if (enabled && push == null) { + /* + * TODO support for loading atmosphere.js on demand will be added in + * another commit. + */ push = GWT.create(PushConnection.class); push.init(this); @@ -3410,6 +3421,9 @@ public class ApplicationConnection { push.connect(pushUri); } }); + } else if (!enabled && push != null) { + push.disconnect(); + push = null; } } diff --git a/client/src/com/vaadin/client/communication/PushConnection.java b/client/src/com/vaadin/client/communication/PushConnection.java index f72066c149..8619cd00d2 100644 --- a/client/src/com/vaadin/client/communication/PushConnection.java +++ b/client/src/com/vaadin/client/communication/PushConnection.java @@ -34,16 +34,47 @@ import com.vaadin.client.VConsole; */ public class PushConnection { + protected enum State { + /** + * Connection is newly created and has not yet been started. + */ + NEW, + + /** + * Opening request has been sent, but still waiting for confirmation + */ + CONNECT_PENDING, + + /** + * Connection is open and ready to use. + */ + CONNECTED, + + /** + * Connection was disconnected while the connection was pending. Wait + * for the connection to get established before closing it. No new + * messages are accepted, but pending messages will still be delivered. + */ + DISCONNECT_PENDING, + + /** + * Connection has been disconnected and should not be used any more. + */ + DISCONNECTED; + } + private ApplicationConnection connection; private JavaScriptObject socket; private ArrayList<String> messageQueue = new ArrayList<String>(); - private boolean connected = false; + private State state = State.NEW; private AtmosphereConfiguration config; + private String uri; + public PushConnection() { } @@ -58,17 +89,33 @@ public class PushConnection { } public void connect(String uri) { + if (state != State.NEW) { + throw new IllegalStateException( + "Connection has already been connected."); + } + + state = State.CONNECT_PENDING; + // uri is needed to identify the right connection when closing + this.uri = uri; VConsole.log("Establishing push connection"); socket = doConnect(uri, getConfig()); } public void push(String message) { - if (!connected) { + switch (state) { + case CONNECT_PENDING: VConsole.log("Queuing push message: " + message); messageQueue.add(message); - } else { + break; + case CONNECTED: VConsole.log("Sending push message: " + message); doPush(socket, message); + break; + case NEW: + throw new IllegalStateException("Can not push before connecting"); + case DISCONNECT_PENDING: + case DISCONNECTED: + throw new IllegalStateException("Can not push after disconnecting"); } } @@ -82,11 +129,54 @@ public class PushConnection { protected void onOpen(AtmosphereResponse response) { VConsole.log("Push connection established using " + response.getTransport()); - connected = true; for (String message : messageQueue) { - push(message); + doPush(socket, message); } messageQueue.clear(); + + switch (state) { + case CONNECT_PENDING: + state = State.CONNECTED; + break; + case DISCONNECT_PENDING: + // Set state to connected to make disconnect close the connection + state = State.CONNECTED; + disconnect(); + break; + case CONNECTED: + // IE likes to open the same connection multiple times, just ignore + break; + default: + throw new IllegalStateException( + "Got onOpen event when conncetion state is " + state + + ". This should never happen."); + } + } + + /** + * Closes the push connection. + */ + public void disconnect() { + switch (state) { + case NEW: + // Nothing to close up, just update state + state = State.DISCONNECTED; + break; + case CONNECT_PENDING: + // Wait until connection is established before closing it + state = State.DISCONNECT_PENDING; + break; + case CONNECTED: + // Normal disconnect + VConsole.log("Closing push connection"); + doDisconnect(uri); + state = State.DISCONNECTED; + break; + case DISCONNECT_PENDING: + case DISCONNECTED: + // Nothing more to do + break; + } } protected void onMessage(AtmosphereResponse response) { @@ -231,4 +321,9 @@ public class PushConnection { /*-{ socket.push(message); }-*/; + + private static native void doDisconnect(String url) + /*-{ + $wnd.jQueryVaadin.atmosphere.unsubscribeUrl(url); + }-*/; } diff --git a/client/src/com/vaadin/client/ui/ui/UIConnector.java b/client/src/com/vaadin/client/ui/ui/UIConnector.java index f3b4f36670..26ca6b559a 100644 --- a/client/src/com/vaadin/client/ui/ui/UIConnector.java +++ b/client/src/com/vaadin/client/ui/ui/UIConnector.java @@ -588,6 +588,10 @@ public class UIConnector extends AbstractSingleComponentContainerConnector if (stateChangeEvent.hasPropertyChanged("pollInterval")) { configurePolling(); } + + if (stateChangeEvent.hasPropertyChanged("pushMode")) { + getConnection().setPushEnabled(getState().pushMode.isEnabled()); + } } private void configurePolling() { diff --git a/server/src/com/vaadin/server/communication/AtmospherePushConnection.java b/server/src/com/vaadin/server/communication/AtmospherePushConnection.java index 9b98153a23..a5025e2356 100644 --- a/server/src/com/vaadin/server/communication/AtmospherePushConnection.java +++ b/server/src/com/vaadin/server/communication/AtmospherePushConnection.java @@ -117,4 +117,12 @@ public class AtmospherePushConnection implements Serializable, PushConnection { protected AtmosphereResource getResource() { return resource; } + + @Override + public void disconnect() { + resource.resume(); + assert !resource.getBroadcaster().getAtmosphereResources() + .contains(resource); + resource = null; + } } diff --git a/server/src/com/vaadin/server/communication/PushConnection.java b/server/src/com/vaadin/server/communication/PushConnection.java index 590219b1b5..eecf4d93a4 100644 --- a/server/src/com/vaadin/server/communication/PushConnection.java +++ b/server/src/com/vaadin/server/communication/PushConnection.java @@ -35,4 +35,9 @@ public interface PushConnection { */ public void push(); + /** + * Disconnects the connection. + */ + public void disconnect(); + }
\ No newline at end of file diff --git a/server/src/com/vaadin/server/communication/PushHandler.java b/server/src/com/vaadin/server/communication/PushHandler.java index a9e6c17751..bdc8c723a5 100644 --- a/server/src/com/vaadin/server/communication/PushHandler.java +++ b/server/src/com/vaadin/server/communication/PushHandler.java @@ -38,6 +38,7 @@ import com.vaadin.server.VaadinServletRequest; import com.vaadin.server.VaadinServletService; import com.vaadin.server.VaadinSession; import com.vaadin.server.WebBrowser; +import com.vaadin.shared.communication.PushMode; import com.vaadin.ui.UI; /** @@ -138,6 +139,46 @@ public class PushHandler implements AtmosphereHandler { } }; + /** + * Callback used when a connection is closed by the client. + */ + PushEventCallback disconnectCallback = new PushEventCallback() { + @Override + public void run(AtmosphereResource resource, UI ui) throws IOException { + PushMode pushMode = ui.getPushMode(); + AtmospherePushConnection pushConnection = getConnectionForUI(ui); + + String id = resource.uuid(); + + if (pushConnection == null) { + getLogger() + .log(Level.WARNING, + "Could not find push connection to close: {0} with transport {1}", + new Object[] { id, resource.transport() }); + } else { + if (!pushMode.isEnabled()) { + /* + * The client is expected to close the connection after push + * mode has been set to disabled, just clean up some stuff + * and be done with it + */ + getLogger().log(Level.FINEST, + "Connection closed for resource {0}", id); + } else { + /* + * Unexpected cancel, e.g. if the user closes the browser + * tab. + */ + getLogger() + .log(Level.FINE, + "Connection unexpectedly closed for resource {0} with transport {1}", + new Object[] { id, resource.transport() }); + } + ui.setPushConnection(null); + } + } + }; + private static final String LONG_PADDING; static { @@ -234,10 +275,7 @@ public class PushHandler implements AtmosphereHandler { String id = resource.uuid(); if (event.isCancelled()) { - // The client closed the connection. - // TODO Do some cleanup - getLogger().log(Level.FINER, "Connection closed for resource {0}", - id); + callWithUi(resource, disconnectCallback); } else if (event.isResuming()) { // A connection that was suspended earlier was resumed (committed to // the client.) Should only happen if the transport is JSONP or diff --git a/server/src/com/vaadin/ui/UI.java b/server/src/com/vaadin/ui/UI.java index 4eff7645e2..e1646a5ac5 100644 --- a/server/src/com/vaadin/ui/UI.java +++ b/server/src/com/vaadin/ui/UI.java @@ -130,8 +130,6 @@ public abstract class UI extends AbstractSingleComponentContainer implements */ private int scrollLeft = 0; - private PushMode pushMode; - private UIServerRpc rpc = new UIServerRpc() { @Override public void click(MouseEventDetails mouseDetails) { @@ -1189,11 +1187,19 @@ public abstract class UI extends AbstractSingleComponentContainer implements * Sets the internal push connection object used by this UI. This method * should only be called by the framework. */ - public void setPushConnection(PushConnection connection) { - assert pushConnection == null; - assert connection != null; - pushConnection = connection; - if (hasPendingPush) { + public void setPushConnection(PushConnection pushConnection) { + assert (pushConnection != null) == (getPushMode().isEnabled()); + + if (pushConnection == this.pushConnection) { + return; + } + + if (this.pushConnection != null) { + this.pushConnection.disconnect(); + } + + this.pushConnection = pushConnection; + if (pushConnection != null && hasPendingPush) { hasPendingPush = false; pushConnection.push(); } @@ -1232,12 +1238,12 @@ public abstract class UI extends AbstractSingleComponentContainer implements * @return The push mode. */ public PushMode getPushMode() { - return pushMode; + return getState(false).pushMode; } /** * Sets the mode of bidirectional ("push") communication that should be used - * in this UI. Set once on UI creation and cannot be changed afterwards. + * in this UI. * * @param pushMode * The push mode to use. @@ -1245,16 +1251,13 @@ public abstract class UI extends AbstractSingleComponentContainer implements * @throws IllegalArgumentException * if the argument is null. * @throws IllegalStateException - * if the mode is already set or if push support is not - * available. + * if push support is not available. */ public void setPushMode(PushMode pushMode) { if (pushMode == null) { throw new IllegalArgumentException("Push mode cannot be null"); } - if (this.pushMode != null) { - throw new IllegalStateException("Push mode already set"); - } + if (pushMode.isEnabled()) { VaadinSession session = getSession(); if (session != null && !session.getService().ensurePushAvailable()) { @@ -1262,7 +1265,13 @@ public abstract class UI extends AbstractSingleComponentContainer implements "Push is not available. See previous log messages for more information."); } } - this.pushMode = pushMode; + + /* + * Client-side will open a new connection or disconnect the old + * connection, so there's nothing more to do on the server at this + * point. + */ + getState().pushMode = pushMode; } } diff --git a/shared/src/com/vaadin/shared/ui/ui/UIState.java b/shared/src/com/vaadin/shared/ui/ui/UIState.java index fbb6427c6f..589744a48c 100644 --- a/shared/src/com/vaadin/shared/ui/ui/UIState.java +++ b/shared/src/com/vaadin/shared/ui/ui/UIState.java @@ -17,6 +17,7 @@ package com.vaadin.shared.ui.ui; import java.io.Serializable; +import com.vaadin.shared.communication.PushMode; import com.vaadin.shared.ui.TabIndexState; public class UIState extends TabIndexState { @@ -24,6 +25,8 @@ public class UIState extends TabIndexState { public LoadingIndicatorConfiguration loadingIndicatorConfiguration = new LoadingIndicatorConfiguration(); public int pollInterval = -1; + public PushMode pushMode = PushMode.DISABLED; + public static class LoadingIndicatorConfiguration implements Serializable { public int initialDelay = 300; public int delayStateDelay = 1500; diff --git a/uitest/src/com/vaadin/tests/push/TogglePush.html b/uitest/src/com/vaadin/tests/push/TogglePush.html new file mode 100644 index 0000000000..b752d2120c --- /dev/null +++ b/uitest/src/com/vaadin/tests/push/TogglePush.html @@ -0,0 +1,91 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> +<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en"> +<head profile="http://selenium-ide.openqa.org/profiles/test-case"> +<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" /> +<link rel="selenium.base" href="" /> +<title>New Test</title> +</head> +<body> +<table cellpadding="1" cellspacing="1" border="1"> +<thead> +<tr><td rowspan="1" colspan="3">New Test</td></tr> +</thead><tbody> +<tr> + <td>open</td> + <td>/run-push/com.vaadin.tests.push.TogglePush?restartApplication</td> + <td></td> +</tr> +<tr> + <td>click</td> + <td>vaadin=runpushcomvaadintestspushTogglePush::/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[3]/VButton[0]/domChild[0]/domChild[0]</td> + <td></td> +</tr> +<tr> + <td>pause</td> + <td>2000</td> + <td></td> +</tr> +<!--Push is enabled, so text gets updated--> +<tr> + <td>assertText</td> + <td>vaadin=runpushcomvaadintestspushTogglePush::/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[0]/VLabel[0]</td> + <td>Counter has been updated 1 times</td> +</tr> +<tr> + <td>mouseClick</td> + <td>vaadin=runpushcomvaadintestspushTogglePush::/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[1]/VCheckBox[0]/domChild[0]</td> + <td>61,6</td> +</tr> +<tr> + <td>click</td> + <td>vaadin=runpushcomvaadintestspushTogglePush::/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[3]/VButton[0]/domChild[0]/domChild[0]</td> + <td></td> +</tr> +<tr> + <td>pause</td> + <td>2000</td> + <td></td> +</tr> +<!--Push is disabled, so text is not updated--> +<tr> + <td>assertText</td> + <td>vaadin=runpushcomvaadintestspushTogglePush::/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[0]/VLabel[0]</td> + <td>Counter has been updated 1 times</td> +</tr> +<tr> + <td>click</td> + <td>vaadin=runpushcomvaadintestspushTogglePush::/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[2]/VButton[0]/domChild[0]/domChild[0]</td> + <td></td> +</tr> +<!--Direct update is visible, and includes previous update--> +<tr> + <td>assertText</td> + <td>vaadin=runpushcomvaadintestspushTogglePush::/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[0]/VLabel[0]</td> + <td>Counter has been updated 3 times</td> +</tr> +<tr> + <td>mouseClick</td> + <td>vaadin=runpushcomvaadintestspushTogglePush::/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[1]/VCheckBox[0]/domChild[0]</td> + <td>61,3</td> +</tr> +<tr> + <td>click</td> + <td>vaadin=runpushcomvaadintestspushTogglePush::/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[3]/VButton[0]/domChild[0]/domChild[0]</td> + <td></td> +</tr> +<tr> + <td>pause</td> + <td>2000</td> + <td></td> +</tr> +<!--Push is enabled again, so text gets updated--> +<tr> + <td>assertText</td> + <td>vaadin=runpushcomvaadintestspushTogglePush::/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[0]/VLabel[0]</td> + <td>Counter has been updated 4 times</td> +</tr> + +</tbody></table> +</body> +</html> diff --git a/uitest/src/com/vaadin/tests/push/TogglePush.java b/uitest/src/com/vaadin/tests/push/TogglePush.java new file mode 100644 index 0000000000..8e9eafeabb --- /dev/null +++ b/uitest/src/com/vaadin/tests/push/TogglePush.java @@ -0,0 +1,82 @@ +package com.vaadin.tests.push; + +import java.util.Timer; +import java.util.TimerTask; + +import com.vaadin.data.Property.ValueChangeEvent; +import com.vaadin.data.Property.ValueChangeListener; +import com.vaadin.server.VaadinRequest; +import com.vaadin.shared.communication.PushMode; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.Button; +import com.vaadin.ui.Button.ClickEvent; +import com.vaadin.ui.CheckBox; +import com.vaadin.ui.Label; + +public class TogglePush extends AbstractTestUI { + private final Label counterLabel = new Label(); + private int counter = 0; + + @Override + protected void setup(VaadinRequest request) { + updateCounter(); + addComponent(counterLabel); + + CheckBox pushSetting = new CheckBox("Push enabled"); + pushSetting.setValue(Boolean.valueOf(getPushMode().isEnabled())); + pushSetting.setImmediate(true); + pushSetting.addValueChangeListener(new ValueChangeListener() { + @Override + public void valueChange(ValueChangeEvent event) { + if (event.getProperty().getValue() == Boolean.TRUE) { + setPushMode(PushMode.AUTOMATIC); + } else { + setPushMode(PushMode.DISABLED); + } + } + }); + addComponent(pushSetting); + + addComponent(new Button("Update counter now", + new Button.ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + updateCounter(); + } + })); + + addComponent(new Button("Update counter in 1 sec", + new Button.ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + new Timer().schedule(new TimerTask() { + @Override + public void run() { + runSafely(new Runnable() { + @Override + public void run() { + updateCounter(); + } + }); + } + }, 1000); + } + })); + } + + public void updateCounter() { + counterLabel.setValue("Counter has been updated " + counter++ + + " times"); + } + + @Override + protected String getTestDescription() { + return "Basic test for enabling and disabling push on the fly."; + } + + @Override + protected Integer getTicketNumber() { + return Integer.valueOf(11506); + } + +} |