From 9fd0961c32e538cdd721119394f4adf75b4c0928 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Tue, 29 Apr 2014 14:12:49 +0200 Subject: [PATCH] Simplified Roster Removed some throw declarations from methods, so some try/catch blocks could be removed. Use switch/case instead of if/else if. Make members final when possible. Add logger statements in case of error IQ result and remove superfluous check of instanceof IQ. --- .../java/org/jivesoftware/smack/Roster.java | 89 +++++++------------ .../jivesoftware/smack/RosterOfflineTest.java | 5 -- 2 files changed, 34 insertions(+), 60 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/Roster.java b/smack-core/src/main/java/org/jivesoftware/smack/Roster.java index c2515fbad..790e0cc5d 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/Roster.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/Roster.java @@ -71,15 +71,15 @@ public class Roster { private final XMPPConnection connection; private final RosterStore rosterStore; - private final Map groups; - private final Map entries; - private final List unfiledEntries; - private final List rosterListeners; - private Map> presenceMap; + private final Map groups = new ConcurrentHashMap(); + private final Map entries = new ConcurrentHashMap(); + private final List unfiledEntries = new CopyOnWriteArrayList(); + private final List rosterListeners = new CopyOnWriteArrayList(); + private final Map> presenceMap = new ConcurrentHashMap>(); // The roster is marked as initialized when at least a single roster packet // has been received and processed. boolean rosterInitialized = false; - private PresencePacketListener presencePacketListener; + private final PresencePacketListener presencePacketListener = new PresencePacketListener(); private SubscriptionMode subscriptionMode = getDefaultSubscriptionMode(); @@ -115,17 +115,11 @@ public class Roster { Roster(final XMPPConnection connection) { this.connection = connection; rosterStore = connection.getConfiguration().getRosterStore(); - groups = new ConcurrentHashMap(); - unfiledEntries = new CopyOnWriteArrayList(); - entries = new ConcurrentHashMap(); - rosterListeners = new CopyOnWriteArrayList(); - presenceMap = new ConcurrentHashMap>(); // Listen for any roster packets. PacketFilter rosterFilter = new PacketTypeFilter(RosterPacket.class); connection.addPacketListener(new RosterPushListener(), rosterFilter); // Listen for any presence packets. PacketFilter presenceFilter = new PacketTypeFilter(Presence.class); - presencePacketListener = new PresencePacketListener(); connection.addPacketListener(presencePacketListener, presenceFilter); // Listen for connection events @@ -226,7 +220,6 @@ public class Roster { throw new IllegalStateException("Anonymous users can't have a roster."); } - RosterPacket packet = new RosterPacket(); if (rosterStore != null && connection.isRosterVersioningSupported()) { packet.setVersion(rosterStore.getRosterVersion()); @@ -266,13 +259,9 @@ public class Roster { * * @param name the name of the group. * @return a new group, or null if the group already exists - * @throws NotLoggedInException If not logged in. * @throws IllegalStateException if logged in anonymously */ - public RosterGroup createGroup(String name) throws NotLoggedInException { - if (!connection.isAuthenticated()) { - throw new NotLoggedInException(); - } + public RosterGroup createGroup(String name) { if (connection.isAnonymous()) { throw new IllegalStateException("Anonymous users can't have a roster."); } @@ -293,12 +282,10 @@ public class Roster { * @param name the nickname of the user. * @param groups the list of group names the entry will belong to, or null if the * the roster entry won't belong to a group. - * @throws NotLoggedInException * @throws NoResponseException if there was no response from the server. * @throws XMPPErrorException if an XMPP exception occurs. * @throws NotLoggedInException If not logged in. * @throws NotConnectedException - * @throws IllegalStateException if logged in anonymously */ public void createEntry(String user, String name, String[] groups) throws NotLoggedInException, NoResponseException, XMPPErrorException, NotConnectedException { if (!connection.isAuthenticated()) { @@ -695,7 +682,7 @@ public class Roster { private void addUpdateEntry(Collection addedEntries, Collection updatedEntries, RosterPacket.Item item, - RosterEntry entry) throws SmackException { + RosterEntry entry){ RosterEntry oldEntry = entries.put(item.getUser(), entry); if (oldEntry == null) { addedEntries.add(item.getUser()); @@ -889,19 +876,25 @@ public class Roster { } } else if (presence.getType() == Presence.Type.subscribe) { - if (subscriptionMode == SubscriptionMode.accept_all) { + Presence response = null; + switch (subscriptionMode) { + case accept_all: // Accept all subscription requests. - Presence response = new Presence(Presence.Type.subscribed); - response.setTo(presence.getFrom()); - connection.sendPacket(response); - } - else if (subscriptionMode == SubscriptionMode.reject_all) { + response = new Presence(Presence.Type.subscribed); + break; + case reject_all: // Reject all subscription requests. - Presence response = new Presence(Presence.Type.unsubscribed); + response = new Presence(Presence.Type.unsubscribed); + break; + case manual: + default: + // Otherwise, in manual mode so ignore. + break; + } + if (response != null) { response.setTo(presence.getFrom()); connection.sendPacket(response); } - // Otherwise, in manual mode so ignore. } else if (presence.getType() == Presence.Type.unsubscribe) { if (subscriptionMode != SubscriptionMode.manual) { @@ -951,11 +944,10 @@ public class Roster { @Override public void processPacket(Packet packet) { connection.removePacketListener(this); - if (!(packet instanceof IQ)) { - return; - } + IQ result = (IQ)packet; if (!result.getType().equals(IQ.Type.RESULT)) { + LOGGER.severe("Roster result IQ not of type result"); return; } @@ -963,14 +955,11 @@ public class Roster { Collection updatedEntries = new ArrayList(); Collection deletedEntries = new ArrayList(); - try { - if (packet instanceof RosterPacket) { - nonemptyResult((RosterPacket) packet, addedEntries, updatedEntries, deletedEntries); - } else { - emptyResult(addedEntries, updatedEntries); - } - } catch (SmackException e) { - return; + if (packet instanceof RosterPacket) { + nonemptyResult((RosterPacket) packet, addedEntries, updatedEntries, deletedEntries); + } + else { + emptyResult(addedEntries, updatedEntries); } synchronized (Roster.this) { @@ -981,7 +970,7 @@ public class Roster { fireRosterChangedEvent(addedEntries, updatedEntries, deletedEntries); } - private void emptyResult(Collection addedEntries, Collection updatedEntries) throws SmackException { + private void emptyResult(Collection addedEntries, Collection updatedEntries) { for(RosterPacket.Item item : rosterStore.getEntries()){ RosterEntry entry = new RosterEntry(item.getUser(), item.getName(), item.getItemType(), item.getItemStatus(), Roster.this, connection); @@ -990,7 +979,7 @@ public class Roster { } private void addEntries(Collection addedEntries, Collection updatedEntries, - Collection deletedEntries, String version, Collection items) throws SmackException { + Collection deletedEntries, String version, Collection items) { for (RosterPacket.Item item : items) { RosterEntry entry = new RosterEntry(item.getUser(), item.getName(), item.getItemType(), item.getItemStatus(), Roster.this, connection); @@ -1014,8 +1003,7 @@ public class Roster { } private void nonemptyResult(RosterPacket packet, Collection addedEntries, - Collection updatedEntries, Collection deletedEntries) - throws SmackException { + Collection updatedEntries, Collection deletedEntries) { RosterPacket rosterPacket = (RosterPacket) packet; String version = rosterPacket.getVersion(); @@ -1032,8 +1020,6 @@ public class Roster { removeEmptyGroups(); } - - } /** @@ -1068,12 +1054,7 @@ public class Roster { Collection deletedEntries = new ArrayList(); Item item = items.iterator().next(); - try { - processPushItem(addedEntries, updatedEntries, deletedEntries, version, item); - } - catch (SmackException e) { - return; - } + processPushItem(addedEntries, updatedEntries, deletedEntries, version, item); connection.sendPacket(IQ.createResultIQ(rosterPacket)); @@ -1084,7 +1065,7 @@ public class Roster { } private void processPushItem(Collection addedEntries, Collection updatedEntries, - Collection deletedEntries, String version, Item item) throws SmackException { + Collection deletedEntries, String version, Item item) { RosterEntry entry = new RosterEntry(item.getUser(), item.getName(), item.getItemType(), item.getItemStatus(), Roster.this, connection); @@ -1101,7 +1082,5 @@ public class Roster { } } } - } - } diff --git a/smack-tcp/src/test/java/org/jivesoftware/smack/RosterOfflineTest.java b/smack-tcp/src/test/java/org/jivesoftware/smack/RosterOfflineTest.java index df1c0116f..48f602b73 100644 --- a/smack-tcp/src/test/java/org/jivesoftware/smack/RosterOfflineTest.java +++ b/smack-tcp/src/test/java/org/jivesoftware/smack/RosterOfflineTest.java @@ -46,11 +46,6 @@ public class RosterOfflineTest { roster.createEntry("test", "test", null); } - @Test(expected = SmackException.class) - public void shouldThrowExceptionOnCreateGroup() throws Exception { - roster.createGroup("test"); - } - @Test(expected = SmackException.class) public void shouldThrowExceptionOnReload() throws Exception { roster.reload();