Add support for "Caps Optimizations"

Smack's previous entity caps implementation assumed that an entity lost its entity caps feature as soon as a presence without caps from that entity was received. But according to XEP-0115 § 8.4, this is a perfectly normal optimization technique. We now reset the caps state after an available presence becomes unavailable. Also introduce PresenceEventListener, which is required for this feature. Also make Roster.preApprove() take a BareJid as argument. Fixes SMACK-723.
2016-06-30 17:01:46 +02:00 · 2016-06-30 17:01:46 +02:00 · d07ed60737
parent 4248fbbb89
commit d07ed60737
8 changed files with 280 additions and 22 deletions
--- a/smack-extensions/src/main/java/org/jivesoftware/smackx/caps/EntityCapsManager.java
+++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/caps/EntityCapsManager.java
@ -27,9 +27,10 @@ import org.jivesoftware.smack.XMPPConnectionRegistry;
 import org.jivesoftware.smack.XMPPException.XMPPErrorException;
 import org.jivesoftware.smack.packet.IQ;
 import org.jivesoftware.smack.packet.Stanza;
+import org.jivesoftware.smack.roster.AbstractPresenceEventListener;
+import org.jivesoftware.smack.roster.Roster;
 import org.jivesoftware.smack.packet.ExtensionElement;
 import org.jivesoftware.smack.packet.Presence;
-import org.jivesoftware.smack.filter.NotFilter;
 import org.jivesoftware.smack.filter.PresenceTypeFilter;
 import org.jivesoftware.smack.filter.StanzaFilter;
 import org.jivesoftware.smack.filter.AndFilter;
@ -47,6 +48,7 @@ import org.jivesoftware.smackx.disco.packet.DiscoverInfo.Identity;
 import org.jivesoftware.smackx.xdata.FormField;
 import org.jivesoftware.smackx.xdata.packet.DataForm;
 import org.jxmpp.jid.DomainBareJid;
+import org.jxmpp.jid.FullJid;
 import org.jxmpp.jid.Jid;
 import org.jxmpp.util.cache.LruCache;

@ -95,8 +97,6 @@ public final class EntityCapsManager extends Manager {

    private static final StanzaFilter PRESENCES_WITH_CAPS = new AndFilter(new StanzaTypeFilter(Presence.class), new StanzaExtensionFilter(
                    ELEMENT, NAMESPACE));
-    private static final StanzaFilter PRESENCES_WITHOUT_CAPS = new AndFilter(new StanzaTypeFilter(Presence.class), new NotFilter(new StanzaExtensionFilter(
-                    ELEMENT, NAMESPACE)));

    /**
     * Map of "node + '#' + hash" to DiscoverInfo data
@ -328,15 +328,12 @@ public final class EntityCapsManager extends Manager {

        }, PRESENCES_WITH_CAPS);

-        connection.addAsyncStanzaListener(new StanzaListener() {
+        Roster.getInstanceFor(connection).addPresenceEventListener(new AbstractPresenceEventListener() {
            @Override
-            public void processPacket(Stanza packet) {
-                // always remove the JID from the map, even if entityCaps are
-                // disabled
-                Jid from = packet.getFrom();
+            public void presenceUnavailable(FullJid from, Presence presence) {
                JID_TO_NODEVER_CACHE.remove(from);
            }
-        }, PRESENCES_WITHOUT_CAPS);
+        });

        connection.addPacketSendingListener(new StanzaListener() {
            @Override
--- a/smack-im/src/main/java/org/jivesoftware/smack/roster/AbstractPresenceEventListener.java
+++ b/smack-im/src/main/java/org/jivesoftware/smack/roster/AbstractPresenceEventListener.java
@ -0,0 +1,46 @@
+/**
+ *
+ * Copyright 2016 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 org.jivesoftware.smack.packet.Presence;
+import org.jxmpp.jid.BareJid;
+import org.jxmpp.jid.FullJid;
+import org.jxmpp.jid.Jid;
+
+public abstract class AbstractPresenceEventListener implements PresenceEventListener {
+
+    @Override
+    public void presenceAvailable(FullJid address, Presence availablePresence) {
+    }
+
+    @Override
+    public void presenceUnavailable(FullJid address, Presence presence) {
+    }
+
+    @Override
+    public void presenceError(Jid address, Presence errorPresence) {
+    }
+
+    @Override
+    public void presenceSubscribed(BareJid address, Presence subscribedPresence) {
+    }
+
+    @Override
+    public void presenceUnsubscribed(BareJid address, Presence unsubscribedPresence) {
+    }
+
+}
--- a/smack-im/src/main/java/org/jivesoftware/smack/roster/PresenceEventListener.java
+++ b/smack-im/src/main/java/org/jivesoftware/smack/roster/PresenceEventListener.java
@ -0,0 +1,35 @@
+/**
+ *
+ * Copyright 2016 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 org.jivesoftware.smack.packet.Presence;
+import org.jxmpp.jid.BareJid;
+import org.jxmpp.jid.FullJid;
+import org.jxmpp.jid.Jid;
+
+public interface PresenceEventListener {
+
+    public void presenceAvailable(FullJid address, Presence availablePresence);
+
+    public void presenceUnavailable(FullJid address, Presence presence);
+
+    public void presenceError(Jid address, Presence errorPresence);
+
+    public void presenceSubscribed(BareJid address, Presence subscribedPresence);
+
+    public void presenceUnsubscribed(BareJid address, Presence unsubscribedPresence);
+}
--- a/smack-im/src/main/java/org/jivesoftware/smack/roster/Roster.java
+++ b/smack-im/src/main/java/org/jivesoftware/smack/roster/Roster.java
@ -1,6 +1,6 @@
 /**
 *
- * Copyright 2003-2007 Jive Software.
+ * Copyright 2003-2007 Jive Software, 2016 Florian Schmaus.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
@ -151,6 +151,8 @@ public final class Roster extends Manager {
    private final Set<RosterEntry> unfiledEntries = new CopyOnWriteArraySet<>();
    private final Set<RosterListener> rosterListeners = new LinkedHashSet<>();

+    private final Set<PresenceEventListener> presenceEventListeners = new CopyOnWriteArraySet<>();
+
    /**
     * A map of JIDs to another Map of Resourceparts to Presences. The 'inner' map may contain
     * {@link Resourcepart#EMPTY} if there are no other Presences available.
@ -289,6 +291,11 @@ public final class Roster extends Manager {
                if (resumed) {
                    return;
                }
+
+                // Ensure that all available presences received so far in a eventually existing previous session are
+                // marked 'offline'.
+                setOfflinePresencesAndResetLoaded();
+
                try {
                    Roster.this.reload();
                }
@ -532,6 +539,14 @@ public final class Roster extends Manager {
        }
    }

+    public boolean addPresenceEventListener(PresenceEventListener presenceEventListener) {
+        return presenceEventListeners.add(presenceEventListener);
+    }
+
+    public boolean removePresenceEventListener(PresenceEventListener presenceEventListener) {
+        return presenceEventListeners.remove(presenceEventListener);
+    }
+
    /**
     * Creates a new group.
     * <p>
@ -584,10 +599,7 @@ public final class Roster extends Manager {
        rosterPacket.addRosterItem(item);
        connection.createPacketCollectorAndSend(rosterPacket).nextResultOrThrow();

-        // Create a presence subscription packet and send.
-        Presence presencePacket = new Presence(Presence.Type.subscribe);
-        presencePacket.setTo(user);
-        connection.sendStanza(presencePacket);
+        sendSubscriptionRequest(user);
    }

    /**
@ -621,7 +633,7 @@ public final class Roster extends Manager {
     * @throws FeatureNotSupportedException if pre-approving is not supported.
     * @since 4.2
     */
-    public void preApprove(Jid user) throws NotLoggedInException, NotConnectedException, InterruptedException, FeatureNotSupportedException {
+    public void preApprove(BareJid user) throws NotLoggedInException, NotConnectedException, InterruptedException, FeatureNotSupportedException {
        final XMPPConnection connection = connection();
        if (!isSubscriptionPreApprovalSupported()) {
            throw new FeatureNotSupportedException("Pre-approving");
@ -644,6 +656,15 @@ public final class Roster extends Manager {
        return connection.hasFeature(SubscriptionPreApproval.ELEMENT, SubscriptionPreApproval.NAMESPACE);
    }

+    public void sendSubscriptionRequest(BareJid jid) throws NotLoggedInException, NotConnectedException, InterruptedException {
+        final XMPPConnection connection = getAuthenticatedConnectionOrThrow();
+
+        // Create a presence subscription packet and send.
+        Presence presencePacket = new Presence(Presence.Type.subscribe);
+        presencePacket.setTo(jid);
+        connection.sendStanza(presencePacket);
+    }
+
    /**
     * Set the subscribe listener, which is invoked on incoming subscription requests and if
     * {@link SubscriptionMode} is set to {@link SubscriptionMode#manual}. If
@ -1363,12 +1384,21 @@ public final class Roster extends Manager {
            Presence presence = (Presence) packet;
            Jid from = presence.getFrom();
            Resourcepart fromResource = Resourcepart.EMPTY;
+            BareJid bareFrom = null;
+            FullJid fullFrom = null;
            if (from != null) {
                fromResource = from.getResourceOrNull();
                if (fromResource == null) {
                    fromResource = Resourcepart.EMPTY;
+                    bareFrom = from.asBareJid();
+                }
+                else {
+                    fullFrom = from.asFullJidIfPossible();
+                    // We know that this must be a full JID in this case.
+                    assert (fullFrom != null);
                }
            }
+
            BareJid key = from != null ? from.asBareJid() : null;
            Map<Resourcepart, Presence> userPresences;

@ -1388,6 +1418,9 @@ public final class Roster extends Manager {
                if (contains(key)) {
                    fireRosterPresenceEvent(presence);
                }
+                for (PresenceEventListener presenceEventListener : presenceEventListeners) {
+                    presenceEventListener.presenceAvailable(fullFrom, presence);
+                }
                break;
            // If an "unavailable" packet.
            case unavailable:
@ -1409,6 +1442,23 @@ public final class Roster extends Manager {
                if (contains(key)) {
                    fireRosterPresenceEvent(presence);
                }
+
+                // Ensure that 'from' is a full JID before invoking the presence unavailable
+                // listeners. Usually unavailable presences always have a resourcepart, i.e. are
+                // full JIDs, but RFC 6121 § 4.5.4 has an implementation note that unavailable
+                // presences from a bare JID SHOULD be treated as applying to all resources. I don't
+                // think any client or server ever implemented that, I do think that this
+                // implementation note is a terrible idea since it adds another corner case in
+                // client code, instead of just having the invariant
+                // "unavailable presences are always from the full JID".
+                if (fullFrom != null) {
+                    for (PresenceEventListener presenceEventListener : presenceEventListeners) {
+                        presenceEventListener.presenceUnavailable(fullFrom, presence);
+                    }
+                } else {
+                    LOGGER.fine("Unavailable presence from bare JID: " + presence);
+                }
+
                break;
            // Error presence packets from a bare JID mean we invalidate all existing
            // presence info for the user.
@ -1429,6 +1479,19 @@ public final class Roster extends Manager {
                if (contains(key)) {
                    fireRosterPresenceEvent(presence);
                }
+                for (PresenceEventListener presenceEventListener : presenceEventListeners) {
+                    presenceEventListener.presenceError(from, presence);
+                }
+                break;
+            case subscribed:
+                for (PresenceEventListener presenceEventListener : presenceEventListeners) {
+                    presenceEventListener.presenceSubscribed(bareFrom, presence);
+                }
+                break;
+            case unsubscribed:
+                for (PresenceEventListener presenceEventListener : presenceEventListeners) {
+                    presenceEventListener.presenceUnsubscribed(bareFrom, presence);
+                }
                break;
            default:
                break;
@ -1624,4 +1687,5 @@ public final class Roster extends Manager {
    public void setNonRosterPresenceMapMaxSize(int maximumSize) {
        nonRosterPresenceMap.setMaxCacheSize(maximumSize);
    }
+
 }
--- a/smack-im/src/test/java/org/jivesoftware/smack/roster/SubscriptionPreApprovalTest.java
+++ b/smack-im/src/test/java/org/jivesoftware/smack/roster/SubscriptionPreApprovalTest.java
@ -73,7 +73,7 @@ public class SubscriptionPreApprovalTest extends InitSmackIm {
    @Test(expected=FeatureNotSupportedException.class)
    public void testPreApprovalNotSupported() throws Throwable {
        final Jid contactJID = JidCreate.from("preapproval@example.com");
-        roster.preApprove(contactJID);
+        roster.preApprove(contactJID.asBareJid());
    }

    @Test
--- a/smack-integration-test/src/main/java/org/igniterealtime/smack/inttest/AbstractSmackIntegrationTest.java
+++ b/smack-integration-test/src/main/java/org/igniterealtime/smack/inttest/AbstractSmackIntegrationTest.java
@ -35,10 +35,10 @@ public abstract class AbstractSmackIntegrationTest extends AbstractSmackIntTest
     */
    protected final XMPPConnection connection;

-    protected final long defaultTimeout;
-
    protected final String testRunId;

+    protected final long defaultTimeout;
+
    public AbstractSmackIntegrationTest(SmackIntegrationTestEnvironment environment) {
        this.connection = this.conOne = environment.conOne;
        this.conTwo = environment.conTwo;
--- a/smack-integration-test/src/main/java/org/jivesoftware/smack/roster/LowLevelRosterIntegrationTest.java
+++ b/smack-integration-test/src/main/java/org/jivesoftware/smack/roster/LowLevelRosterIntegrationTest.java
@ -0,0 +1,73 @@
+/**
+ *
+ * Copyright 2016 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.concurrent.TimeoutException;
+
+import org.igniterealtime.smack.inttest.AbstractSmackLowLevelIntegrationTest;
+import org.igniterealtime.smack.inttest.Configuration;
+import org.igniterealtime.smack.inttest.SmackIntegrationTest;
+import org.igniterealtime.smack.inttest.util.SimpleResultSyncPoint;
+import org.jivesoftware.smack.packet.Presence;
+import org.jivesoftware.smack.tcp.XMPPTCPConnection;
+import org.jxmpp.jid.FullJid;
+
+public class LowLevelRosterIntegrationTest extends AbstractSmackLowLevelIntegrationTest {
+
+    public LowLevelRosterIntegrationTest(Configuration configuration, String testRunId) {
+        super(configuration, testRunId);
+    }
+
+    @SmackIntegrationTest
+    public void testPresenceEventListenersOffline(final XMPPTCPConnection conOne, final XMPPTCPConnection conTwo) throws TimeoutException, Exception {
+        RosterIntegrationTest.ensureBothAccountsAreNotInEachOthersRoster(conOne, conTwo);
+
+        final Roster rosterOne = Roster.getInstanceFor(conOne);
+        final Roster rosterTwo = Roster.getInstanceFor(conTwo);
+
+        // TODO create Roster.createEntry() with boolean flag for subscribe or not.
+        rosterOne.createEntry(conTwo.getUser().asBareJid(), "Con Two", null);
+        rosterTwo.createEntry(conOne.getUser().asBareJid(), "Con One", null);
+
+        // TODO Change timeout form '5000' to something configurable.
+        final long timeout = 5000;
+        RosterIntegrationTest.ensureBothAccountsAreSubscribedToEachOther(conOne, conTwo, timeout);
+
+        final SimpleResultSyncPoint offlineTriggered = new SimpleResultSyncPoint();
+
+        rosterOne.addPresenceEventListener(new AbstractPresenceEventListener() {
+            @Override
+            public void presenceUnavailable(FullJid jid, Presence presence) {
+                if (!jid.equals(conTwo.getUser())) {
+                    return;
+                }
+                offlineTriggered.signal();
+            }
+        });
+
+        // Disconnect conTwo, this should cause an 'unavilable' presence to be send from conTwo to
+        // conOne.
+        conTwo.disconnect();
+
+        Boolean result = offlineTriggered.waitForResult(timeout);
+        if (!result) {
+            throw new Exception("presenceUnavailable() was not called");
+        }
+    }
+
+}
--- a/smack-integration-test/src/main/java/org/jivesoftware/smack/roster/RosterIntegrationTest.java
+++ b/smack-integration-test/src/main/java/org/jivesoftware/smack/roster/RosterIntegrationTest.java
@ -1,6 +1,6 @@
 /**
 *
- * Copyright 2015 Florian Schmaus
+ * Copyright 2015-2016 Florian Schmaus
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
@ -32,6 +32,7 @@ import org.jivesoftware.smack.XMPPConnection;
 import org.jivesoftware.smack.XMPPException.XMPPErrorException;
 import org.jivesoftware.smack.packet.Presence;
 import org.jivesoftware.smack.roster.packet.RosterPacket.ItemType;
+import org.jxmpp.jid.BareJid;
 import org.jxmpp.jid.Jid;

 public class RosterIntegrationTest extends AbstractSmackIntegrationTest {
@ -47,7 +48,7 @@ public class RosterIntegrationTest extends AbstractSmackIntegrationTest {

    @SmackIntegrationTest
    public void subscribeRequestListenerTest() throws TimeoutException, Exception {
-        ensureBothAccountsAreNotInEachOthersRoster();
+        ensureBothAccountsAreNotInEachOthersRoster(conOne, conTwo);

        rosterTwo.setSubscribeListener(new SubscribeListener() {
            @Override
@ -92,14 +93,14 @@ public class RosterIntegrationTest extends AbstractSmackIntegrationTest {
        assertTrue(addedAndSubscribed.waitForResult(2 * connection.getPacketReplyTimeout()));
    }

-    private void ensureBothAccountsAreNotInEachOthersRoster() throws NotLoggedInException,
+    public static void ensureBothAccountsAreNotInEachOthersRoster(XMPPConnection conOne, XMPPConnection conTwo) throws NotLoggedInException,
                    NoResponseException, XMPPErrorException, NotConnectedException,
                    InterruptedException {
        notInRoster(conOne, conTwo);
        notInRoster(conTwo, conOne);
    }

-    private void notInRoster(XMPPConnection c1, XMPPConnection c2) throws NotLoggedInException,
+    private static void notInRoster(XMPPConnection c1, XMPPConnection c2) throws NotLoggedInException,
                    NoResponseException, XMPPErrorException, NotConnectedException,
                    InterruptedException {
        Roster roster = Roster.getInstanceFor(c1);
@ -109,4 +110,46 @@ public class RosterIntegrationTest extends AbstractSmackIntegrationTest {
        }
        roster.removeEntry(c2Entry);
    }
+
+    public static void ensureBothAccountsAreSubscribedToEachOther(XMPPConnection conOne, XMPPConnection conTwo, long timeout) throws TimeoutException, Exception {
+        ensureSubscribedTo(conOne, conTwo, timeout);
+        ensureSubscribedTo(conTwo, conOne, timeout);
+    }
+
+    private static void ensureSubscribedTo(final XMPPConnection conOne, final XMPPConnection conTwo, long timeout) throws TimeoutException, Exception {
+        Roster rosterOne = Roster.getInstanceFor(conOne);
+        Roster rosterTwo = Roster.getInstanceFor(conTwo);
+
+        if (rosterOne.isSubscribedToMyPresence(conTwo.getUser())) {
+            return;
+        }
+
+        rosterOne.setSubscribeListener(new SubscribeListener() {
+            @Override
+            public SubscribeAnswer processSubscribe(Jid from, Presence subscribeRequest) {
+                if (from.equals(conTwo.getUser().asBareJid())) {
+                    return SubscribeAnswer.Approve;
+                }
+                return SubscribeAnswer.Deny;
+            }
+        });
+
+        final SimpleResultSyncPoint syncPoint = new SimpleResultSyncPoint();
+        rosterTwo.addPresenceEventListener(new AbstractPresenceEventListener() {
+            @Override
+            public void presenceSubscribed(BareJid address, Presence subscribedPresence) {
+                if (!address.equals(conOne.getUser().asBareJid())) {
+                    return;
+                }
+                syncPoint.signal();
+            }
+        });
+        rosterTwo.sendSubscriptionRequest(conOne.getUser().asBareJid());
+
+        try {
+            syncPoint.waitForResult(timeout);
+        } finally {
+            rosterOne.setSubscribeListener(null);
+        }
+    }
 }