[telepathy-gabble/master] Serve disco responses from the cache

[Will Thompson]

We only serve up responses from cache entries added locally, because
only those are known to be complete. This means we can serve responses
to people discoing our old caps hashes, rather than returning an error,
which also works around an iChat issue where it reacts to disco errors
by re-sending the request again and again.
---
 src/connection.c                    |   37 ++++++++++++++++++----------------
 tests/twisted/caps/trust-thyself.py |   35 +++++++++++++++++++++++++++++++-
 2 files changed, 53 insertions(+), 19 deletions(-)

diff --git a/src/connection.c b/src/connection.c
index 392d3b8..449bbcf 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -2160,7 +2160,6 @@ connection_iq_disco_cb (LmMessageHandler *handler,
   LmMessageNode *iq, *result_iq, *query, *result_query, *identity;
   const gchar *node, *suffix;
   const GabbleCapabilitySet *features;
-  gchar *caps_hash;
 
   if (lm_message_get_sub_type (message) != LM_MESSAGE_SUB_TYPE_GET)
     return LM_HANDLER_RESULT_ALLOW_MORE_HANDLERS;
@@ -2206,26 +2205,32 @@ connection_iq_disco_cb (LmMessageHandler *handler,
   lm_message_node_set_attribute (identity, "name", PACKAGE_STRING);
   lm_message_node_set_attribute (identity, "type", "pc");
 
-  caps_hash = caps_hash_compute_from_self_presence (self);
-
+  if (node == NULL)
+    features = gabble_presence_peek_caps (self->self_presence);
   /* If node is not NULL, it can be either a caps bundle as defined in the
    * legacy XEP-0115 version 1.3 or an hash as defined in XEP-0115 version
-   * 1.5. */
-  if (node == NULL || !tp_strdiff (suffix, caps_hash))
-    features = gabble_presence_peek_caps (self->self_presence);
-  else if (!tp_strdiff (suffix, BUNDLE_VOICE_V1))
-    features = gabble_capabilities_get_bundle_voice_v1 ();
-  else if (!tp_strdiff (suffix, BUNDLE_VIDEO_V1))
-    features = gabble_capabilities_get_bundle_video_v1 ();
+   * 1.5. Let's see if it's a verification string we've told the cache about.
+   */
   else
-    features = NULL;
+    features = gabble_presence_cache_peek_own_caps (self->presence_cache,
+        suffix);
+
+  if (features == NULL)
+    {
+      /* Otherwise, is it one of the caps bundles we advertise? These are not
+       * just shoved into the cache with gabble_presence_cache_add_own_caps()
+       * because capabilities_get_features() always includes a few bonus
+       * features...
+       */
+      if (!tp_strdiff (suffix, BUNDLE_VOICE_V1))
+        features = gabble_capabilities_get_bundle_voice_v1 ();
+
+      if (!tp_strdiff (suffix, BUNDLE_VIDEO_V1))
+        features = gabble_capabilities_get_bundle_video_v1 ();
+    }
 
   if (features == NULL)
     {
-      /* Return <item-not-found>. It is possible that the remote contact
-       * requested an old version (old hash) of our capabilities. In the
-       * meantime, it will have gotten a new hash, and query the new hash
-       * anyway. */
       _gabble_connection_send_iq_error (self, message,
           XMPP_ERROR_ITEM_NOT_FOUND, NULL);
     }
@@ -2242,8 +2247,6 @@ connection_iq_disco_cb (LmMessageHandler *handler,
         }
     }
 
-  g_free (caps_hash);
-
   lm_message_unref (result);
 
   return LM_HANDLER_RESULT_REMOVE_MESSAGE;
diff --git a/tests/twisted/caps/trust-thyself.py b/tests/twisted/caps/trust-thyself.py
index 55bdd97..518a6bb 100644
--- a/tests/twisted/caps/trust-thyself.py
+++ b/tests/twisted/caps/trust-thyself.py
@@ -3,10 +3,12 @@ Test that we cache our own capabilities, so that we don't disco other people
 with the same caps hash or ext='' bundles.
 """
 from twisted.words.xish import xpath
+from twisted.words.protocols.jabber.client import IQ
 
 from gabbletest import exec_test, make_presence, sync_stream
-from servicetest import EventPattern
+from servicetest import EventPattern, assertEquals, assertNotEquals
 import ns
+import constants as cs
 
 def test(q, bus, conn, stream):
     conn.Connect()
@@ -15,6 +17,8 @@ def test(q, bus, conn, stream):
     c = xpath.queryForNodes('/presence/c', self_presence.stanza)[0]
 
     jid = 'lol at great.big/omg'
+
+    # Check that Gabble doesn't disco other clients with the same caps hash.
     p = make_presence(jid,
         caps={'node': c['node'],
               'hash': c['hash'],
@@ -23,10 +27,13 @@ def test(q, bus, conn, stream):
     stream.send(p)
 
     q.forbid_events([
-        EventPattern('stream-iq', to=jid, query_ns=ns.DISCO_INFO),
+        EventPattern('stream-iq', to=jid, iq_type='get',
+            query_ns=ns.DISCO_INFO),
     ])
     sync_stream(q, stream)
 
+    # Check that Gabble doesn't disco its own ext='' bundles (well, its own
+    # bundles as advertised by Gabbles that don't do hashed caps)
     p = make_presence(jid,
         caps={'node': c['node'],
               'ver':  c['ver'],
@@ -36,5 +43,29 @@ def test(q, bus, conn, stream):
     stream.send(p)
     sync_stream(q, stream)
 
+    # Advertise some different capabilities, to change our own caps hash.
+    add = [(cs.CHANNEL_TYPE_STREAMED_MEDIA, 2L**32-1),
+           (cs.CHANNEL_TYPE_STREAM_TUBE, 2L**32-1),
+           (cs.CHANNEL_TYPE_STREAM_TUBE, 2L**32-1)]
+    remove = []
+    caps = conn.Capabilities.AdvertiseCapabilities(add, remove)
+
+    self_presence = q.expect('stream-presence')
+    c_ = xpath.queryForNodes('/presence/c', self_presence.stanza)[0]
+    assertNotEquals(c['ver'], c_['ver'])
+
+    # But then someone asks us for our old caps
+    iq = IQ(stream, 'get')
+    iq['from'] = jid
+    query = iq.addElement((ns.DISCO_INFO, 'query'))
+    query['node'] = c['node'] + '#' + c['ver']
+    stream.send(iq)
+
+    # Gabble should still know what they are, and reply. This is actually quite
+    # important: there's a bug in iChat where if you return an error to a disco
+    # query, it just asks again, and again, and again...
+    reply = q.expect('stream-iq', to=jid)
+    assertEquals('result', reply.iq_type)
+
 if __name__ == '__main__':
     exec_test(test)
-- 
1.5.6.5