Rework Roster's SubscribeListener

allow multiple of them to be installed, instead of at most one. Fixes
deadlock in LowLevelRosterIntegration test because
IoTProvisioningManager's SubscribeListener would not come up with a
decission.

smack-experimental/src/main/java/org/jivesoftware/smackx/iot/provisioning/IoTProvisioningManager.java

@@ -151,7 +151,7 @@ public final class IoTProvisioningManager extends Manager {
                         });
 
         roster = Roster.getInstanceFor(connection);
-        roster.setSubscribeListener(new SubscribeListener() {
+        roster.addSubscribeListener(new SubscribeListener() {
             @Override
             public SubscribeAnswer processSubscribe(Jid from, Presence subscribeRequest) {
                 // First check if the subscription request comes from a known registry and accept the request if so.

smack-im/src/main/java/org/jivesoftware/smack/roster/Roster.java

@@ -199,7 +199,9 @@ public final class Roster extends Manager {
 
     private SubscriptionMode subscriptionMode = getDefaultSubscriptionMode();
 
-    private SubscribeListener subscribeListener;
+    private final Set<SubscribeListener> subscribeListeners = new CopyOnWriteArraySet<>();
+
+    private SubscriptionMode previousSubscriptionMode;
 
     /**
      * Returns the default subscription processing mode to use when a new Roster is created. The
@@ -249,11 +251,12 @@ public final class Roster extends Manager {
                 SubscribeAnswer subscribeAnswer = null;
                 switch (subscriptionMode) {
                 case manual:
-                    final SubscribeListener subscribeListener = Roster.this.subscribeListener;
+                    for (SubscribeListener subscribeListener : subscribeListeners) {
-                    if (subscribeListener == null) {
-                        return;
-                    }
                         subscribeAnswer = subscribeListener.processSubscribe(from, presence);
+                        if (subscribeAnswer != null) {
+                            break;
+                        }
+                    }
                     if (subscribeAnswer == null) {
                         return;
                     }
@@ -666,19 +669,37 @@ public final class Roster extends Manager {
     }
 
     /**
-     * Set the subscribe listener, which is invoked on incoming subscription requests and if
+     * Add a subscribe listener, which is invoked on incoming subscription requests and if
-     * {@link SubscriptionMode} is set to {@link SubscriptionMode#manual}. If
+     * {@link SubscriptionMode} is set to {@link SubscriptionMode#manual}. This also sets subscription
-     * <code>subscribeListener</code> is not <code>null</code>, then this also sets subscription
      * mode to {@link SubscriptionMode#manual}.
      * 
-     * @param subscribeListener the subscribe listener to set.
+     * @param subscribeListener the subscribe listener to add.
+     * @return <code>true</code> if the listener was not already added.
      * @since 4.2
      */
-    public void setSubscribeListener(SubscribeListener subscribeListener) {
+    public boolean addSubscribeListener(SubscribeListener subscribeListener) {
-        if (subscribeListener != null) {
+        Objects.requireNonNull(subscribeListener, "SubscribeListener argument must not be null");
-            setSubscriptionMode(SubscriptionMode.manual);
+        if (subscriptionMode != SubscriptionMode.manual) {
+            previousSubscriptionMode = subscriptionMode;
         }
-        this.subscribeListener = subscribeListener;
+        return subscribeListeners.add(subscribeListener);
+    }
+
+    /**
+     * Remove a subscribe listener. Also restores the previous subscription mode
+     * state, if the last listener got removed.
+     *
+     * @param subscribeListener
+     *            the subscribe listener to remove.
+     * @return <code>true</code> if the listener registered and got removed.
+     * @since 4.2
+     */
+    public boolean removeSubscribeListener(SubscribeListener subscribeListener) {
+        boolean removed = subscribeListeners.remove(subscribeListener);
+        if (removed && subscribeListeners.isEmpty()) {
+            setSubscriptionMode(previousSubscriptionMode);
+        }
+        return removed;
     }
 
     /**

smack-integration-test/src/main/java/org/jivesoftware/smack/roster/RosterIntegrationTest.java

@@ -50,7 +50,7 @@ public class RosterIntegrationTest extends AbstractSmackIntegrationTest {
     public void subscribeRequestListenerTest() throws TimeoutException, Exception {
         ensureBothAccountsAreNotInEachOthersRoster(conOne, conTwo);
 
-        rosterTwo.setSubscribeListener(new SubscribeListener() {
+        final SubscribeListener subscribeListener = new SubscribeListener() {
             @Override
             public SubscribeAnswer processSubscribe(Jid from, Presence subscribeRequest) {
                 if (from.equals(conOne.getUser().asBareJid())) {
@@ -58,7 +58,8 @@ public class RosterIntegrationTest extends AbstractSmackIntegrationTest {
                 }
                 return SubscribeAnswer.Deny;
             }
-        });
+        };
+        rosterTwo.addSubscribeListener(subscribeListener);
 
         final String conTwosRosterName = "ConTwo " + testRunId;
         final SimpleResultSyncPoint addedAndSubscribed = new SimpleResultSyncPoint();
@@ -88,10 +89,16 @@ public class RosterIntegrationTest extends AbstractSmackIntegrationTest {
                 }
             }
         });
+
+        try {
             rosterOne.createEntry(conTwo.getUser().asBareJid(), conTwosRosterName, null);
 
             assertTrue(addedAndSubscribed.waitForResult(2 * connection.getPacketReplyTimeout()));
         }
+        finally {
+            rosterTwo.removeSubscribeListener(subscribeListener);
+        }
+    }
 
     public static void ensureBothAccountsAreNotInEachOthersRoster(XMPPConnection conOne, XMPPConnection conTwo) throws NotLoggedInException,
                     NoResponseException, XMPPErrorException, NotConnectedException,
@@ -124,7 +131,7 @@ public class RosterIntegrationTest extends AbstractSmackIntegrationTest {
             return;
         }
 
-        rosterOne.setSubscribeListener(new SubscribeListener() {
+        final SubscribeListener subscribeListener = new SubscribeListener() {
             @Override
             public SubscribeAnswer processSubscribe(Jid from, Presence subscribeRequest) {
                 if (from.equals(conTwo.getUser().asBareJid())) {
@@ -132,7 +139,8 @@ public class RosterIntegrationTest extends AbstractSmackIntegrationTest {
                 }
                 return SubscribeAnswer.Deny;
             }
-        });
+        };
+        rosterOne.addSubscribeListener(subscribeListener);
 
         final SimpleResultSyncPoint syncPoint = new SimpleResultSyncPoint();
         rosterTwo.addPresenceEventListener(new AbstractPresenceEventListener() {
@@ -149,7 +157,7 @@ public class RosterIntegrationTest extends AbstractSmackIntegrationTest {
         try {
             syncPoint.waitForResult(timeout);
         } finally {
-            rosterOne.setSubscribeListener(null);
+            rosterOne.removeSubscribeListener(subscribeListener);
         }
     }
 }