From a59cfd6b41018c46e6313f31b7122f020f883c02 Mon Sep 17 00:00:00 2001
From: Marco Barisione <marco@barisione.org>
Date: Wed, 31 Aug 2011 17:31:24 +0100
Subject: [PATCH 1/5] Roster: don't hide local pending contacts from stored

---
 src/roster.c                               |   13 ++++++---
 tests/twisted/roster/test-google-roster.py |   41 ++++++++++++++++++++++++++-
 2 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/src/roster.c b/src/roster.c
index 8d8c3b7..3e1b1e2 100644
--- a/src/roster.c
+++ b/src/roster.c
@@ -1247,12 +1247,17 @@ process_roster (
         case GABBLE_ROSTER_SUBSCRIPTION_FROM:
         case GABBLE_ROSTER_SUBSCRIPTION_BOTH:
           if (google_roster &&
-              /* Don't hide contacts from stored if they're remote pending.
-               * This works around Google Talk flickering ask="subscribe"
-               * when you try to subscribe to someone; see
-               * test-google-roster.py.
+              /* Don't hide contacts from stored if they're pending.
+               * This works around two Google Talk issues:
+               * - When you try to subscribe to someone, you get a flickering
+               *   ask="subscribe";
+               * - When somebody tries to subscribe to you, you get a presence
+               *   with type="subscribe" followed by a roster update with
+               *   subscribe="none".
+               * See test-google-roster.py for more details.
                */
               item->subscribe != TP_SUBSCRIPTION_STATE_ASK &&
+              item->publish != TP_SUBSCRIPTION_STATE_ASK &&
               !_google_roster_item_should_keep (jid, item))
             {
               tp_handle_set_remove (changed, handle);
diff --git a/tests/twisted/roster/test-google-roster.py b/tests/twisted/roster/test-google-roster.py
index 630e3c3..790501c 100644
--- a/tests/twisted/roster/test-google-roster.py
+++ b/tests/twisted/roster/test-google-roster.py
@@ -131,8 +131,9 @@ def test_inital_roster(q, bus, conn, stream):
 
 def test_flickering(q, bus, conn, stream, subscribe):
     """
-    Google's server is buggy, and subscription state transitions "flicker"
-    sometimes. Here, we test that Gabble is suppressing the flickers.
+    Google's server is buggy; when asking to subscribe to somebody, the
+    subscription state transitions "flicker" sometimes. Here, we test that
+    Gabble is suppressing the flickers.
     """
 
     self_handle = conn.GetSelfHandle()
@@ -264,6 +265,41 @@ def test_flickering(q, bus, conn, stream, subscribe):
     sync_dbus(bus, q, conn)
     q.unforbid_events(change_events)
 
+def test_local_pending(q, bus, conn, stream, subscribe):
+    """
+    When somebody asks to subscribe to us, Google sends the subscription
+    request and then a roster update saying there is no subscription.
+    This causes the contact to appear in local pending and then disappear.
+    Here, we test that Gabble is suppressing the flickers.
+    """
+
+    self_handle = conn.GetSelfHandle()
+    contact = 'alice@foo.com'
+    handle = conn.RequestHandles(cs.HT_CONTACT, [contact])[0]
+
+    # We add Alice
+    presence = domish.Element(('jabber:client', 'presence'))
+    presence['from'] = contact
+    presence['type'] = 'subscribe'
+    stream.send(presence)
+
+    q.expect('dbus-signal', signal='ContactsChanged',
+            args=[{handle: (cs.SUBSCRIPTION_STATE_NO,
+                cs.SUBSCRIPTION_STATE_ASK, '')}, []])
+
+    # Now we send the spurious roster update with subscribe="none" and verify
+    # that nothing happens in reaction to that
+    change_event = EventPattern('dbus-signal', signal='MembersChanged')
+    q.forbid_events([change_event])
+
+    iq = make_set_roster_iq(stream, 'test@localhost/Resource', contact,
+            "none", False)
+    stream.send(iq)
+
+    sync_stream(q, stream)
+    sync_dbus(bus, q, conn)
+    q.unforbid_events([change_event])
+
 # This event is forbidden in all of the deny tests!
 remove_events = [
     EventPattern('stream-iq', query_ns=ns.ROSTER,
@@ -520,6 +556,7 @@ def test(q, bus, conn, stream):
     publish, subscribe, stored, deny = test_inital_roster(q, bus, conn, stream)
 
     test_flickering(q, bus, conn, stream, subscribe)
+    test_local_pending(q, bus, conn, stream, subscribe)
     test_deny_simple(q, bus, conn, stream, stored, deny)
     test_deny_overlap_one(q, bus, conn, stream, subscribe, stored, deny)
     test_deny_overlap_two(q, bus, conn, stream,
-- 
1.7.7