diff --git a/source/org/jivesoftware/smack/Roster.java b/source/org/jivesoftware/smack/Roster.java index dd30dfa12..e34490b1e 100644 --- a/source/org/jivesoftware/smack/Roster.java +++ b/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(); - connection.addPacketListener(presencePacket, presenceFilter); + presencePacketListener = new PresencePacketListener(); + 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 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 - * in any valid format (e.g. "domain/resource", "user@domain" or "user@domain/resource"). + * @param user the XMPP address of the user (eg "jsmith@example.com"). The + * 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 - * is available, such as when you are not subscribed to the user's presence updates. + * or an unavailable presence if the user is unavailable (offline) or if no presence + * 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 new HashMap(userPresences).values().iterator(); - } + return userPresences.values().iterator(); } } /** - * 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. + * Cleans up all resources used by the roster. + */ + void cleanup() { + rosterListeners.clear(); + } + + /** + * 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. - * @return the key to use in the presenceMap for the fully qualified xmpp ID. + * @param user the bare or fully qualified XMPP ID, e.g. jdoe@example.com or + * 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 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 - // for a particular user a map with the presence packets saved for each resource. + // If an "available" presence, add it to the presence map. Each presence + // map will hold for a particular user a map with the presence + // packets saved for each resource. if (presence.getType() == Presence.Type.available) { Map 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); - } + 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) { - Map userPresences = presenceMap.get(key); - synchronized (userPresences) { - userPresences.remove(StringUtils.parseResource(from)); + // If no resource, this is likely an offline presence as part of + // a roster presence flood. In that case, we store it. + if ("".equals(StringUtils.parseResource(from))) { + Map userPresences; + // Get the user presence map + if (presenceMap.get(key) == null) { + userPresences = new ConcurrentHashMap(); + 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 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 - } } \ No newline at end of file diff --git a/source/org/jivesoftware/smack/XMPPConnection.java b/source/org/jivesoftware/smack/XMPPConnection.java index 0b2b84ba0..caa3d3ede 100644 --- a/source/org/jivesoftware/smack/XMPPConnection.java +++ b/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; diff --git a/test/org/jivesoftware/smack/PresenceTest.java b/test/org/jivesoftware/smack/PresenceTest.java index 9dd6a29b7..ec33f0d3d 100644 --- a/test/org/jivesoftware/smack/PresenceTest.java +++ b/test/org/jivesoftware/smack/PresenceTest.java @@ -1,7 +1,7 @@ /** * $RCSfile$ - * $Revision: $ - * $Date: $ + * $Revision$ + * $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 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()); - presences.next(); + Presence value = 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; }