Move duplicate code into XMPPConnection

from XMPPTCPConnection and XMPPBOSHConnection.

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);
     }
 

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.<p>
+     * again.
-     * <p/>
+     *
-     * 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.
      * @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.<p>
+     * presence packet with whatever data is set.
-     * <p/>
-     * 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.
      * 
      * @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);

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;

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;
     }
 

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
+     * Shuts the current connection down. After this method returns, the connection must be ready
-     * the XMPP server. The shutdown logic will be used during a planned disconnection or when
+     * for re-use by connect.
-     * 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 
      */
-    protected void shutdown(Presence unavailablePresence) throws NotConnectedException {
+    @Override
-        // Set presence to offline.
+    protected void shutdown() {
-        if (packetWriter != null) {
-                sendPacket(unavailablePresence);
-        }
-
-        this.setWasAuthenticated(authenticated);
-        authenticated = false;
-
         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
+            // An exception occurred in setting up the connection.
-            // readers and writers and close the socket.
+            shutdown();
-
+            // Everything stoppped. Now throw the exception.
-            if (packetWriter != null) {
+            throw ex;
-                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.
         }
     }
 
@@ -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();
-            shutdown(new Presence(Presence.Type.unavailable));
+
-        }
-        catch (NotConnectedException e1) {
-            // Ignore
-        }
         // Notify connection listeners of the error.
         callConnectionClosedOnErrorListener(e);
     }