diff --git a/smack-im/src/main/java/org/jivesoftware/smack/roster/Roster.java b/smack-im/src/main/java/org/jivesoftware/smack/roster/Roster.java index ce8ae40f1..3cf51bab3 100644 --- a/smack-im/src/main/java/org/jivesoftware/smack/roster/Roster.java +++ b/smack-im/src/main/java/org/jivesoftware/smack/roster/Roster.java @@ -59,6 +59,7 @@ import org.jivesoftware.smack.roster.packet.RosterPacket; import org.jivesoftware.smack.roster.packet.RosterVer; import org.jivesoftware.smack.roster.packet.RosterPacket.Item; import org.jivesoftware.smack.roster.rosterstore.RosterStore; +import org.jivesoftware.smack.util.Objects; import org.jxmpp.util.XmppStringUtils; /** @@ -123,11 +124,22 @@ public class Roster extends Manager { private RosterStore rosterStore; private final Map groups = new ConcurrentHashMap(); + + /** + * Concurrent hash map from JID to its roster entry. + */ private final Map entries = new ConcurrentHashMap(); + private final Set unfiledEntries = new CopyOnWriteArraySet<>(); private final Set rosterListeners = new LinkedHashSet<>(); private final Map> presenceMap = new ConcurrentHashMap>(); + /** + * Mutually exclude roster listener invocation and changing the {@link entries} map. Also used + * to synchronize access to either the roster listeners or the entries map. + */ + private final Object rosterListenersAndEntriesLock = new Object(); + // The roster is marked as initialized when at least a single roster packet // has been received and processed. private boolean loaded = false; @@ -354,9 +366,12 @@ public class Roster extends Manager { * * @param rosterListener a roster listener. * @return true if the listener was not already added. + * @see #getEntriesAndAddListener(RosterListener, RosterEntries) */ public boolean addRosterListener(RosterListener rosterListener) { - return rosterListeners.add(rosterListener); + synchronized (rosterListenersAndEntriesLock) { + return rosterListeners.add(rosterListener); + } } /** @@ -367,7 +382,9 @@ public class Roster extends Manager { * @return true if the listener was active and got removed. */ public boolean removeRosterListener(RosterListener rosterListener) { - return rosterListeners.remove(rosterListener); + synchronized (rosterListenersAndEntriesLock) { + return rosterListeners.remove(rosterListener); + } } /** @@ -481,6 +498,33 @@ public class Roster extends Manager { return getEntries().size(); } + /** + * Add a roster listener and invoke the roster entries with all entries of the roster. + *

+ * The method guarantees that the listener is only invoked after + * {@link RosterEntries#rosterEntires(Collection)} has been invoked, and that all roster events + * that happen while rosterEntires(Collection) is called are queued until the + * method returns. + *

+ *

+ * This guarantee makes this the ideal method to e.g. populate a UI element with the roster while + * installing a {@link RosterListener} to listen for subsequent roster events. + *

+ * + * @param rosterListener the listener to install + * @param rosterEntries the roster entries callback interface + * @since 4.1 + */ + public void getEntriesAndAddListener(RosterListener rosterListener, RosterEntries rosterEntries) { + Objects.requireNonNull(rosterListener, "listener must not be null"); + Objects.requireNonNull(rosterEntries, "rosterEntries must not be null"); + + synchronized (rosterListenersAndEntriesLock) { + rosterEntries.rosterEntires(entries.values()); + addRosterListener(rosterListener); + } + } + /** * Returns a set of all entries in the roster, including entries * that don't belong to any groups. @@ -488,14 +532,13 @@ public class Roster extends Manager { * @return all entries in the roster. */ public Set getEntries() { - Set allEntries = new HashSet(); - // Loop through all roster groups and add their entries to the answer - for (RosterGroup rosterGroup : getGroups()) { - allEntries.addAll(rosterGroup.getEntries()); + Set allEntries; + synchronized (rosterListenersAndEntriesLock) { + allEntries = new HashSet<>(entries.size()); + for (RosterEntry entry : entries.values()) { + allEntries.add(entry); + } } - // Add the roster unfiled entries to the answer - allEntries.addAll(unfiledEntries); - return allEntries; } @@ -896,15 +939,17 @@ public class Roster extends Manager { */ private void fireRosterChangedEvent(final Collection addedEntries, final Collection updatedEntries, final Collection deletedEntries) { - for (RosterListener listener : rosterListeners) { - if (!addedEntries.isEmpty()) { - listener.entriesAdded(addedEntries); - } - if (!updatedEntries.isEmpty()) { - listener.entriesUpdated(updatedEntries); - } - if (!deletedEntries.isEmpty()) { - listener.entriesDeleted(deletedEntries); + synchronized (rosterListenersAndEntriesLock) { + for (RosterListener listener : rosterListeners) { + if (!addedEntries.isEmpty()) { + listener.entriesAdded(addedEntries); + } + if (!updatedEntries.isEmpty()) { + listener.entriesUpdated(updatedEntries); + } + if (!deletedEntries.isEmpty()) { + listener.entriesDeleted(deletedEntries); + } } } } @@ -915,14 +960,19 @@ public class Roster extends Manager { * @param presence the presence change. */ private void fireRosterPresenceEvent(final Presence presence) { - for (RosterListener listener : rosterListeners) { - listener.presenceChanged(presence); + synchronized (rosterListenersAndEntriesLock) { + for (RosterListener listener : rosterListeners) { + listener.presenceChanged(presence); + } } } private void addUpdateEntry(Collection addedEntries, Collection updatedEntries, Collection unchangedEntries, RosterPacket.Item item, RosterEntry entry) { - RosterEntry oldEntry = entries.put(item.getUser(), entry); + RosterEntry oldEntry; + synchronized (rosterListenersAndEntriesLock) { + oldEntry = entries.put(item.getUser(), entry); + } if (oldEntry == null) { addedEntries.add(item.getUser()); } diff --git a/smack-im/src/main/java/org/jivesoftware/smack/roster/RosterEntries.java b/smack-im/src/main/java/org/jivesoftware/smack/roster/RosterEntries.java new file mode 100644 index 000000000..b15659c02 --- /dev/null +++ b/smack-im/src/main/java/org/jivesoftware/smack/roster/RosterEntries.java @@ -0,0 +1,25 @@ +/** + * + * Copyright 2015 Florian Schmaus + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jivesoftware.smack.roster; + +import java.util.Collection; + +public interface RosterEntries { + + public void rosterEntires(Collection rosterEntries); + +}