Added support for offline presence, code cleanup (SMACK-74, SMACK-201).

git-svn-id: http://svn.igniterealtime.org/svn/repos/smack/trunk@7185 b35dd754-fafc-0310-a699-88a17e54d16e

source/org/jivesoftware/smack/Roster.java

@@ -47,7 +47,7 @@ import java.util.concurrent.CopyOnWriteArrayList;
  * @see XMPPConnection#getRoster()
  * @author Matt Tucker
  */
-public class Roster implements ConnectionListener {
+public class Roster {
 
     /**
      * The default subscription processing mode to use when a Roster is created. By default
@@ -64,7 +64,7 @@ public class Roster implements ConnectionListener {
     // The roster is marked as initialized when at least a single roster packet
     // has been recieved and processed.
     boolean rosterInitialized = false;
-    private PresencePacketListener presencePacket;
+    private PresencePacketListener presencePacketListener;
 
     private SubscriptionMode subscriptionMode = getDefaultSubscriptionMode();
 
@@ -109,10 +109,32 @@ public class Roster implements ConnectionListener {
         connection.addPacketListener(new RosterPacketListener(), rosterFilter);
         // Listen for any presence packets.
         PacketFilter presenceFilter = new PacketTypeFilter(Presence.class);
-        presencePacket = new PresencePacketListener();
+        presencePacketListener = new PresencePacketListener();
-        connection.addPacketListener(presencePacket, presenceFilter);
+        connection.addPacketListener(presencePacketListener, presenceFilter);
         // Listen for connection events
-        connection.addConnectionListener(this);
+        connection.addConnectionListener(new ConnectionListener() {
+            public void connectionClosed() {
+                // Changes the presence available contacts to unavailable
+                setOfflinePresences();
+            }
+
+            public void connectionClosedOnError(Exception e) {
+                // Changes the presence available contacts to unavailable
+                setOfflinePresences();
+            }
+
+            public void reconnectingIn(int seconds) {
+                // Ignore
+            }
+
+            public void reconnectionFailed(Exception e) {
+                // Ignore
+            }
+
+            public void reconnectionSuccessful() {
+                // Ignore
+            }
+        });
     }
 
     /**
@@ -302,25 +324,6 @@ public class Roster implements ConnectionListener {
         return Collections.unmodifiableCollection(allEntries);
     }
 
-    /**
-     * Changes the presence of available contacts offline by simulating an unavailable
-     * presence sent from the server. After a disconnection, every Presence is set
-     * to offline.
-     */
-    private void setOfflinePresences() {
-        Presence packetUnavailable;
-        for (String user : presenceMap.keySet()) {
-            Map<String, Presence> resources = presenceMap.get(user);
-            if (resources != null) {
-                for (String resource : resources.keySet()) {
-                    packetUnavailable = new Presence(Presence.Type.unavailable);
-                    packetUnavailable.setFrom(user + "/" + resource);
-                    presencePacket.processPacket(packetUnavailable);
-                }
-            }
-        }
-    }
-
     /**
      * Returns a count of the unfiled entries in the roster. An unfiled entry is
      * an entry that doesn't belong to any groups.
@@ -365,8 +368,9 @@ public class Roster implements ConnectionListener {
     /**
      * Returns true if the specified XMPP address is an entry in the roster.
      *
-     * @param user the XMPP address of the user (eg "jsmith@example.com"). The address could be
+     * @param user the XMPP address of the user (eg "jsmith@example.com"). The
-     * in any valid format (e.g. "domain/resource", "user@domain" or "user@domain/resource").
+     *      address could be in any valid format (e.g. "domain/resource",
+     *      "user@domain" or "user@domain/resource").
      * @return true if the XMPP address is an entry in the roster.
      */
     public boolean contains(String user) {
@@ -500,8 +504,9 @@ public class Roster implements ConnectionListener {
 
     /**
      * Returns an iterator (of Presence objects) for all of a user's current presences
-     * or an unavailable presence if the user is unavailable (offline) or if no presence information
+     * or an unavailable presence if the user is unavailable (offline) or if no presence
-     * is available, such as when you are not subscribed to the user's presence updates.
+     * information is available, such as when you are not subscribed to the user's presence
+     * updates.
      *
      * @param user a XMPP ID, e.g. jdoe@example.com.
      * @return an iterator (of Presence objects) for all the user's current presences,
@@ -515,22 +520,29 @@ public class Roster implements ConnectionListener {
             return Arrays.asList(new Presence(Presence.Type.unavailable)).iterator();
         }
         else {
-            synchronized (userPresences) {
+            return userPresences.values().iterator();
-                return new HashMap<String, Presence>(userPresences).values().iterator();
-            }
         }
     }
 
     /**
-     * Returns the key to use in the presenceMap for a fully qualified XMPP ID. The roster
+     * Cleans up all resources used by the roster.
-     * can contain any valid address format such us "domain/resource", "user@domain" or
+     */
-     * "user@domain/resource". If the roster contains an entry associated with the fully qualified
+    void cleanup() {
-     * XMPP ID then use the fully qualified XMPP ID as the key in presenceMap, otherwise use the
+        rosterListeners.clear();
-     * bare address. Note: When the key in presenceMap is a fully qualified XMPP ID, the
+    }
-     * userPresences is useless since it will always contain one entry for the user.
+
+    /**
+     * Returns the key to use in the presenceMap for a fully qualified XMPP ID.
+     * The roster can contain any valid address format such us "domain/resource",
+     * "user@domain" or "user@domain/resource". If the roster contains an entry
+     * associated with the fully qualified XMPP ID then use the fully qualified XMPP
+     * ID as the key in presenceMap, otherwise use the bare address. Note: When the
+     * key in presenceMap is a fully qualified XMPP ID, the userPresences is useless
+     * since it will always contain one entry for the user.
      *
-     * @param user the bare or fully qualified XMPP ID, e.g. jdoe@example.com or jdoe@example.com/Work.
+     * @param user the bare or fully qualified XMPP ID, e.g. jdoe@example.com or
-     * @return the key to use in the presenceMap for the fully qualified xmpp ID.
+     *      jdoe@example.com/Work.
+     * @return the key to use in the presenceMap for the fully qualified XMPP ID.
      */
     private String getPresenceMapKey(String user) {
         if (user == null) {
@@ -543,6 +555,25 @@ public class Roster implements ConnectionListener {
         return key.toLowerCase();
     }
 
+    /**
+     * Changes the presence of available contacts offline by simulating an unavailable
+     * presence sent from the server. After a disconnection, every Presence is set
+     * to offline.
+     */
+    private void setOfflinePresences() {
+        Presence packetUnavailable;
+        for (String user : presenceMap.keySet()) {
+            Map<String, Presence> resources = presenceMap.get(user);
+            if (resources != null) {
+                for (String resource : resources.keySet()) {
+                    packetUnavailable = new Presence(Presence.Type.unavailable);
+                    packetUnavailable.setFrom(user + "/" + resource);
+                    presencePacketListener.processPacket(packetUnavailable);
+                }
+            }
+        }
+    }
+
     /**
      * Fires roster changed event to roster listeners indicating that the
      * specified collections of contacts have been added, updated or deleted
@@ -614,8 +645,9 @@ public class Roster implements ConnectionListener {
             String from = presence.getFrom();
             String key = getPresenceMapKey(from);
 
-            // If an "available" packet, add it to the presence map. Each presence map will hold
+            // If an "available" presence, add it to the presence map. Each presence
-            // for a particular user a map with the presence packets saved for each resource.
+            // map will hold for a particular user a map with the presence
+            // packets saved for each resource.
             if (presence.getType() == Presence.Type.available) {
                 Map<String, Presence> userPresences;
                 // Get the user presence map
@@ -626,10 +658,11 @@ public class Roster implements ConnectionListener {
                 else {
                     userPresences = presenceMap.get(key);
                 }
+                // See if an offline presence was being stored in the map. If so, remove
+                // it since we now have an online presence.
+                userPresences.remove("");
                 // Add the new presence, using the resources as a key.
-                synchronized (userPresences) {
                 userPresences.put(StringUtils.parseResource(from), presence);
-                }
                 // If the user is in the roster, fire an event.
                 for (RosterEntry entry : entries) {
                     if (entry.getUser().equals(key)) {
@@ -637,13 +670,26 @@ public class Roster implements ConnectionListener {
                     }
                 }
             }
-            // If an "unavailable" packet, remove any entries in the presence map.
+            // If an "unavailable" packet.
             else if (presence.getType() == Presence.Type.unavailable) {
-                if (presenceMap.get(key) != null) {
+                // If no resource, this is likely an offline presence as part of
-                    Map<String, Presence> userPresences = presenceMap.get(key);
+                // a roster presence flood. In that case, we store it.
-                    synchronized (userPresences) {
+                if ("".equals(StringUtils.parseResource(from))) {
-                        userPresences.remove(StringUtils.parseResource(from));
+                    Map<String, Presence> userPresences;
+                    // Get the user presence map
+                    if (presenceMap.get(key) == null) {
+                        userPresences = new ConcurrentHashMap<String, Presence>();
+                        presenceMap.put(key, userPresences);
                     }
+                    else {
+                        userPresences = presenceMap.get(key);
+                    }
+                    userPresences.put("", presence);
+                }
+                // Otherwise, this is a normal offline presence.
+                else if (presenceMap.get(key) != null) {
+                    Map<String, Presence> userPresences = presenceMap.get(key);
+                    userPresences.remove(StringUtils.parseResource(from));
                     if (userPresences.isEmpty()) {
                         presenceMap.remove(key);
                     }
@@ -809,26 +855,4 @@ public class Roster implements ConnectionListener {
             fireRosterChangedEvent(addedEntries, updatedEntries, deletedEntries);
         }
     }
-
-    public void connectionClosed() {
-        // Changes the presence available contacts to unavailable
-        this.setOfflinePresences();
-    }
-
-    public void connectionClosedOnError(Exception e) {
-        // Changes the presence available contacts to unavailable
-        this.setOfflinePresences();
-    }
-
-    public void reconnectingIn(int seconds) {
-        // Ignore
-    }
-
-    public void reconnectionFailed(Exception e) {
-        // Ignore
-    }
-
-    public void reconnectionSuccessful() {
-        // Ignore
-    }
 }

source/org/jivesoftware/smack/XMPPConnection.java

@@ -630,11 +630,12 @@ public class XMPPConnection {
      * @param unavailablePresence the presence packet to send during shutdown. 
      */
     public void disconnect(Presence unavailablePresence) {
-        this.shutdown(unavailablePresence);
+        shutdown(unavailablePresence);
 
-        this.roster = null;
+        roster.cleanup();
+        roster = null;
 
-        this.wasAuthenticated = false;
+        wasAuthenticated = false;
 
         packetWriter.cleanup();
         packetWriter = null;

test/org/jivesoftware/smack/PresenceTest.java

@@ -1,7 +1,7 @@
 /**
  * $RCSfile$
- * $Revision: $
+ * $Revision$
- * $Date: $
+ * $Date$
  *
  * Copyright 2003-2007 Jive Software.
  *
@@ -181,21 +181,23 @@ public class PresenceTest extends SmackTestCase {
         // Wait up to 2 seconds
         long initial = System.currentTimeMillis();
         while (System.currentTimeMillis() - initial < 2000 && (
-                roster.getPresence(getBareJID(1)) == null)) {
+                !roster.getPresence(getBareJID(1)).isAvailable()))
+        {
             Thread.sleep(100);
         }
 
         // Check that a presence is returned for the new contact
         Presence presence = roster.getPresence(getBareJID(1));
-        assertNotNull("Returned a null Presence for an existing user", presence);
+        assertTrue("Returned an offline Presence for an existing user", presence.isAvailable());
 
         presence = roster.getPresenceResource(getBareJID(1) + "/Home");
-        assertNotNull("Returned a null Presence for Home resource", presence);
+        assertTrue("Returned an offline Presence for Home resource", presence.isAvailable());
 
         presence = roster.getPresenceResource(getFullJID(1));
-        assertNotNull("Returned a null Presence for Smack resource", presence);
+        assertTrue("Returned an offline Presence for Smack resource", presence.isAvailable());
 
         Iterator<Presence> presences = roster.getPresences(getBareJID(1));
+        assertTrue("Returned an offline Presence for an existing user", presence.isAvailable());
         assertNotNull("No presence was found for user1", presences);
         assertTrue("No presence was found for user1", presences.hasNext());
         presences.next();
@@ -209,21 +211,57 @@ public class PresenceTest extends SmackTestCase {
 
         // Check that a presence is returned for the new contact
         presence = roster.getPresence(getBareJID(1));
-        assertNotNull("Returned a null Presence for an existing user", presence);
+        assertTrue("Returned a null Presence for an existing user", presence.isAvailable());
 
         presence = roster.getPresenceResource(getFullJID(1));
-        assertNotNull("Returned a null Presence for Smack resource", presence);
+        assertTrue("Returned a null Presence for Smack resource", presence.isAvailable());
 
         presence = roster.getPresenceResource(getBareJID(1) + "/Home");
-        assertNull("Returned a Presence for no longer connected resource", presence);
+        assertTrue("Returned a Presence for no longer connected resource", !presence.isAvailable());
 
         presences = roster.getPresences(getBareJID(1));
         assertNotNull("No presence was found for user1", presences);
-        assertTrue("No presence was found for user1", presences.hasNext());
+        Presence value = presences.next();
-        presences.next();
+        assertTrue("No presence was found for user1", value.isAvailable());
         assertFalse("More than one presence was found for user1", presences.hasNext());
     }
 
+    /**
+     * User1 logs in, then sets offline presence information (presence with status text). User2
+     * logs in and checks to see if offline presence is returned.
+     */
+    public void testOfflineStatusPresence() throws Exception {
+        // Add a new roster entry for other user.
+        Roster roster = getConnection(0).getRoster();
+        roster.createEntry(getBareJID(1), "gato1", null);
+
+        // Wait up to 2 seconds
+        long initial = System.currentTimeMillis();
+        while (System.currentTimeMillis() - initial < 2000 && (
+                roster.getPresence(getBareJID(1)).getType().equals(Presence.Type.unavailable))) {
+            Thread.sleep(100);
+        }
+
+        // Sign out of conn0.
+        getConnection(0).disconnect();
+
+        // Sign out of conn1 with status
+        Presence offlinePresence = new Presence(Presence.Type.unavailable);
+        offlinePresence.setStatus("Offline test");
+        getConnection(1).disconnect(offlinePresence);
+
+        // See if conneciton 0 can get offline status.
+        XMPPConnection con0 = getConnection(0);
+        con0.connect();
+        con0.login(getUsername(0), getUsername(0));
+
+        // Wait 500 ms
+        Thread.sleep(500);
+        Presence presence = con0.getRoster().getPresence(getBareJID(1));
+        assertTrue("Offline presence status not received.",
+                "Offline test".equals(presence.getStatus()));
+    }
+
     protected int getMaxConnections() {
         return 2;
     }