From f940d72fcd294cc6d2d3470f84fd9b4d4626f6b5 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sun, 4 May 2014 20:31:25 +0200 Subject: [PATCH] Move duplicate code into XMPPConnection from XMPPTCPConnection and XMPPBOSHConnection. --- .../smack/XMPPBOSHConnection.java | 43 +----- .../jivesoftware/smack/XMPPConnection.java | 58 +++++--- .../jivesoftware/smack/DummyConnection.java | 3 +- .../org/jivesoftware/smack/PacketReader.java | 6 +- .../jivesoftware/smack/XMPPTCPConnection.java | 129 ++---------------- 5 files changed, 62 insertions(+), 177 deletions(-) diff --git a/smack-bosh/src/main/java/org/jivesoftware/smack/XMPPBOSHConnection.java b/smack-bosh/src/main/java/org/jivesoftware/smack/XMPPBOSHConnection.java index afa81b4c5..34ab3b5a6 100644 --- a/smack-bosh/src/main/java/org/jivesoftware/smack/XMPPBOSHConnection.java +++ b/smack-bosh/src/main/java/org/jivesoftware/smack/XMPPBOSHConnection.java @@ -37,6 +37,7 @@ import org.jivesoftware.smack.Roster; import org.jivesoftware.smack.XMPPException; import org.jivesoftware.smack.packet.Packet; import org.jivesoftware.smack.packet.Presence; +import org.jivesoftware.smack.packet.Presence.Type; import org.jivesoftware.smack.util.StringUtils; import org.igniterealtime.jbosh.BOSHClient; import org.igniterealtime.jbosh.BOSHClientConfig; @@ -348,30 +349,6 @@ public class XMPPBOSHConnection extends XMPPConnection { } } - public void disconnect(Presence unavailablePresence) { - if (!connected) { - return; - } - shutdown(unavailablePresence); - - // Cleanup - // TODO still needed? Smack 4.0.0 BOSH -// if (roster != null) { -// roster.cleanup(); -// roster = null; -// } - sendListeners.clear(); - recvListeners.clear(); - collectors.clear(); - interceptors.clear(); - - // Reset the connection flags - wasAuthenticated = false; - isFirstInitialization = true; - - callConnectionClosedListener(); - } - /** * Closes the connection by setting presence to unavailable and closing the * HTTP client. The shutdown logic will be used during a planned disconnection or when @@ -379,9 +356,9 @@ public class XMPPBOSHConnection extends XMPPConnection { * BOSH packet reader and {@link Roster} will not be removed; thus * connection's state is kept. * - * @param unavailablePresence the presence packet to send during shutdown. */ - protected void shutdown(Presence unavailablePresence) { + @Override + protected void shutdown() { setWasAuthenticated(authenticated); authID = null; sessionID = null; @@ -390,6 +367,7 @@ public class XMPPBOSHConnection extends XMPPConnection { connected = false; isFirstInitialization = false; + Presence unavailablePresence = new Presence(Type.unavailable); try { client.disconnect(ComposableBody.builder() .setNamespaceDefinition("xmpp", XMPP_BOSH_NS) @@ -428,17 +406,6 @@ public class XMPPBOSHConnection extends XMPPConnection { readerConsumer = null; } - /** - * Sets whether the connection has already logged in the server. - * - * @param wasAuthenticated true if the connection has already been authenticated. - */ - private void setWasAuthenticated(boolean wasAuthenticated) { - if (!this.wasAuthenticated) { - this.wasAuthenticated = wasAuthenticated; - } - } - /** * Send a HTTP request to the connection manager with the provided body element. * @@ -537,7 +504,7 @@ public class XMPPBOSHConnection extends XMPPConnection { */ protected void notifyConnectionError(Exception e) { // Closes the connection temporary. A reconnection is possible - shutdown(new Presence(Presence.Type.unavailable)); + shutdown(); callConnectionClosedOnErrorListener(e); } diff --git a/smack-core/src/main/java/org/jivesoftware/smack/XMPPConnection.java b/smack-core/src/main/java/org/jivesoftware/smack/XMPPConnection.java index fa933064e..fa18e1f73 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/XMPPConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/XMPPConnection.java @@ -232,6 +232,17 @@ public abstract class XMPPConnection { */ private IOException connectionException; + /** + * Flag that indicates if the user is currently authenticated with the server. + */ + protected boolean authenticated = false; + + /** + * Flag that indicates if the user was authenticated with the server when the connection + * to the server was closed (abruptly or not). + */ + protected boolean wasAuthenticated = false; + /** * Create an executor to deliver incoming packets to listeners. We'll use a single thread with an unbounded queue. */ @@ -630,13 +641,8 @@ public abstract class XMPPConnection { /** * Closes the connection by setting presence to unavailable then closing the connection to * the XMPP server. The XMPPConnection can still be used for connecting to the server - * again.

- *

- * This method cleans up all resources used by the connection. Therefore, the roster, - * listeners and other stateful objects cannot be re-used by simply calling connect() - * on this connection again. This is unlike the behavior during unexpected disconnects - * (and subsequent connections). In that case, all state is preserved to allow for - * more seamless error recovery. + * again. + * * @throws NotConnectedException */ public void disconnect() throws NotConnectedException { @@ -646,21 +652,28 @@ public abstract class XMPPConnection { /** * Closes the connection. A custom unavailable presence is sent to the server, followed * by closing the stream. The XMPPConnection can still be used for connecting to the server - * again. A custom unavilable presence is useful for communicating offline presence + * again. A custom unavailable presence is useful for communicating offline presence * information such as "On vacation". Typically, just the status text of the presence * packet is set with online information, but most XMPP servers will deliver the full - * presence packet with whatever data is set.

- *

- * This method cleans up all resources used by the connection. Therefore, the roster, - * listeners and other stateful objects cannot be re-used by simply calling connect() - * on this connection again. This is unlike the behavior during unexpected disconnects - * (and subsequent connections). In that case, all state is preserved to allow for - * more seamless error recovery. + * presence packet with whatever data is set. * * @param unavailablePresence the presence packet to send during shutdown. * @throws NotConnectedException */ - public abstract void disconnect(Presence unavailablePresence) throws NotConnectedException; + public synchronized void disconnect(Presence unavailablePresence) throws NotConnectedException { + if (!isConnected()) { + return; + } + + sendPacket(unavailablePresence); + shutdown(); + callConnectionClosedListener(); + }; + + /** + * Shuts the current connection down. + */ + protected abstract void shutdown(); /** * Adds a new listener that will be notified when new Connections are created. Note @@ -1101,6 +1114,19 @@ public abstract class XMPPConnection { } } + /** + * Sets whether the connection has already logged in the server. This method assures that the + * {@link #wasAuthenticated} flag is never reset once it has ever been set. + * + * @param authenticated true if the connection has already been authenticated. + */ + protected void setWasAuthenticated(boolean authenticated) { + // Never reset the flag if the connection has ever been authenticated + if (!wasAuthenticated) { + wasAuthenticated = authenticated; + } + } + void callConnectionConnectedListener() { for (ConnectionListener listener : getConnectionListeners()) { listener.connected(this); diff --git a/smack-core/src/test/java/org/jivesoftware/smack/DummyConnection.java b/smack-core/src/test/java/org/jivesoftware/smack/DummyConnection.java index 4776b7d37..4e4fdd8e7 100644 --- a/smack-core/src/test/java/org/jivesoftware/smack/DummyConnection.java +++ b/smack-core/src/test/java/org/jivesoftware/smack/DummyConnection.java @@ -32,7 +32,6 @@ import org.jivesoftware.smack.PacketCollector; import org.jivesoftware.smack.Roster; import org.jivesoftware.smack.XMPPException; import org.jivesoftware.smack.packet.Packet; -import org.jivesoftware.smack.packet.Presence; /** * A dummy implementation of {@link XMPPConnection}, intended to be used during @@ -86,7 +85,7 @@ public class DummyConnection extends XMPPConnection { } @Override - public void disconnect(Presence unavailablePresence) { + protected void shutdown() { user = null; connectionID = null; roster = null; diff --git a/smack-tcp/src/main/java/org/jivesoftware/smack/PacketReader.java b/smack-tcp/src/main/java/org/jivesoftware/smack/PacketReader.java index f96ba8dc2..dbae2df95 100644 --- a/smack-tcp/src/main/java/org/jivesoftware/smack/PacketReader.java +++ b/smack-tcp/src/main/java/org/jivesoftware/smack/PacketReader.java @@ -114,13 +114,9 @@ class PacketReader { } /** - * Shuts the packet reader down. + * Shuts the packet reader down. This method simply sets the 'done' flag to true. */ public void shutdown() { - // Notify connection listeners of the connection closing if done hasn't already been set. - if (!done) { - connection.callConnectionClosedListener(); - } done = true; } diff --git a/smack-tcp/src/main/java/org/jivesoftware/smack/XMPPTCPConnection.java b/smack-tcp/src/main/java/org/jivesoftware/smack/XMPPTCPConnection.java index 610d3d3dc..617f89067 100644 --- a/smack-tcp/src/main/java/org/jivesoftware/smack/XMPPTCPConnection.java +++ b/smack-tcp/src/main/java/org/jivesoftware/smack/XMPPTCPConnection.java @@ -81,15 +81,6 @@ public class XMPPTCPConnection extends XMPPConnection { // by XMPPTCPConnection, PacketReader, PacketWriter private volatile boolean socketClosed = false; - /** - * Flag that indicates if the user is currently authenticated with the server. - */ - private boolean authenticated = false; - /** - * Flag that indicates if the user was authenticated with the server when the connection - * to the server was closed (abruptly or not). - */ - private boolean wasAuthenticated = false; private boolean anonymous = false; private boolean usingTLS = false; @@ -353,24 +344,11 @@ public class XMPPTCPConnection extends XMPPConnection { } /** - * Closes the connection by setting presence to unavailable then closing the stream to - * the XMPP server. The shutdown logic will be used during a planned disconnection or when - * dealing with an unexpected disconnection. Unlike {@link #disconnect()} the connection's - * packet reader, packet writer, and {@link Roster} will not be removed; thus - * connection's state is kept. - * - * @param unavailablePresence the presence packet to send during shutdown. - * @throws NotConnectedException + * Shuts the current connection down. After this method returns, the connection must be ready + * for re-use by connect. */ - protected void shutdown(Presence unavailablePresence) throws NotConnectedException { - // Set presence to offline. - if (packetWriter != null) { - sendPacket(unavailablePresence); - } - - this.setWasAuthenticated(authenticated); - authenticated = false; - + @Override + protected void shutdown() { if (packetReader != null) { packetReader.shutdown(); } @@ -378,14 +356,6 @@ public class XMPPTCPConnection extends XMPPConnection { packetWriter.shutdown(); } - // Wait 150 ms for processes to clean-up, then shutdown. - try { - Thread.sleep(150); - } - catch (Exception e) { - // Ignore. - } - // Set socketClosed to true. This will cause the PacketReader // and PacketWriter to ignore any Exceptions that are thrown // because of a read/write from/to a closed stream. @@ -396,29 +366,14 @@ public class XMPPTCPConnection extends XMPPConnection { } catch (Exception e) { LOGGER.log(Level.WARNING, "shutdown", e); } - // In most cases the close() should be successful, so set - // connected to false here. - connected = false; + setWasAuthenticated(authenticated); + authenticated = false; + connected = false; reader = null; writer = null; } - public synchronized void disconnect(Presence unavailablePresence) throws NotConnectedException { - // If not connected, ignore this request. - if (packetReader == null || packetWriter == null) { - return; - } - - if (!isConnected()) { - return; - } - - shutdown(unavailablePresence); - - wasAuthenticated = false; - } - void sendPacketInternal(Packet packet) throws NotConnectedException { packetWriter.sendPacket(packet); } @@ -520,49 +475,10 @@ public class XMPPTCPConnection extends XMPPConnection { } catch (SmackException ex) { - // An exception occurred in setting up the connection. Make sure we shut down the - // readers and writers and close the socket. - - if (packetWriter != null) { - try { - packetWriter.shutdown(); - } - catch (Throwable ignore) { /* ignore */ } - packetWriter = null; - } - if (packetReader != null) { - try { - packetReader.shutdown(); - } - catch (Throwable ignore) { /* ignore */ } - packetReader = null; - } - if (reader != null) { - try { - reader.close(); - } - catch (Throwable ignore) { /* ignore */ } - reader = null; - } - if (writer != null) { - try { - writer.close(); - } - catch (Throwable ignore) { /* ignore */} - writer = null; - } - if (socket != null) { - try { - socket.close(); - } - catch (Exception e) { /* ignore */ } - socket = null; - } - this.setWasAuthenticated(authenticated); - authenticated = false; - connected = false; - - throw ex; // Everything stoppped. Now throw the exception. + // An exception occurred in setting up the connection. + shutdown(); + // Everything stoppped. Now throw the exception. + throw ex; } } @@ -888,17 +804,6 @@ public class XMPPTCPConnection extends XMPPConnection { } } - /** - * Sets whether the connection has already logged in the server. - * - * @param wasAuthenticated true if the connection has already been authenticated. - */ - private void setWasAuthenticated(boolean wasAuthenticated) { - if (!this.wasAuthenticated) { - this.wasAuthenticated = wasAuthenticated; - } - } - /** * Sends out a notification that there was an error with the connection * and closes the connection. Also prints the stack trace of the given exception @@ -910,17 +815,9 @@ public class XMPPTCPConnection extends XMPPConnection { if ((packetReader == null || packetReader.done) && (packetWriter == null || packetWriter.done)) return; - if (packetReader != null) - packetReader.done = true; - if (packetWriter != null) - packetWriter.done = true; // Closes the connection temporary. A reconnection is possible - try { - shutdown(new Presence(Presence.Type.unavailable)); - } - catch (NotConnectedException e1) { - // Ignore - } + shutdown(); + // Notify connection listeners of the error. callConnectionClosedOnErrorListener(e); }