Fix offline contacts showing up as unknown, not offline

Due to a weird interaction between the presence cache, IM channels, and
scraping nicknames out of <message/>s, receiving a message from an
offline contact before the roster is received would cause their status
to erroneously show up as unknown, not offline.

This fix is a bit of a hack, but it is much smaller than refactoring to
make the IM channel store the alias (which would allow us to expunge
keep_unavailable).

This regressed as a side-effect of e0cda61.

Fixes: <https://bugs.freedesktop.org/show_bug.cgi?id=41743>

2 files changed, 32 insertions, 4 deletions

diff --git a/src/roster.c b/src/roster.c
index 8d8c3b7b8..d7c05272c 100644
--- a/src/roster.c
+++ b/src/roster.c
@@ -1398,11 +1398,29 @@ got_roster_iq (GabbleRoster *roster,
         {
           GabbleRosterItem *item = v;
           TpHandle contact = GPOINTER_TO_UINT (k);
+          GabblePresence *presence = gabble_presence_cache_get (
+              priv->conn->presence_cache, contact);
 
           if (item->subscribe == TP_SUBSCRIPTION_STATE_YES &&
-              gabble_presence_cache_get (roster->priv->conn->presence_cache,
-                  contact) == NULL)
-            g_array_append_val (members, contact);
+              (presence == NULL || presence->status == GABBLE_PRESENCE_UNKNOWN))
+            {
+              /* The contact might be in the presence cache with UNKNOWN
+               * presence if we've received a message from them before the
+               * roster arrived: an item is forcibly added to stash the
+               * nickname which might have been included in the <message/> in
+               * the presence cache. (This seems like a rather illogical place
+               * to stash such nicknames—if anything, they should live in
+               * GabbleImFactory—but there we go.)
+               *
+               * So if this is the case, we flip their status to OFFLINE. We
+               * don't use gabble_presence_update() because we want to signal
+               * all the unknown→offline transitions together.
+               */
+              if (presence != NULL)
+                presence->status = GABBLE_PRESENCE_OFFLINE;
+
+              g_array_append_val (members, contact);
+            }
 
           if (item->unsent_edits != NULL)
             edited_items = g_slist_prepend (edited_items, item);
diff --git a/tests/twisted/presence/initial-contact-presence.py b/tests/twisted/presence/initial-contact-presence.py
index ab5288199..881316c73 100644
--- a/tests/twisted/presence/initial-contact-presence.py
+++ b/tests/twisted/presence/initial-contact-presence.py
@@ -8,7 +8,7 @@ This serves as a regression test for
 <https://bugs.freedesktop.org/show_bug.cgi?id=38603>, among other bugs.
 """
 
-from gabbletest import exec_test, make_presence, sync_stream
+from gabbletest import exec_test, make_presence, sync_stream, elem
 from servicetest import assertEquals, EventPattern, sync_dbus
 
 import constants as cs
@@ -58,6 +58,16 @@ def test(q, bus, conn, stream):
     stream.send(make_presence('eve@foo.com'))
     q.expect('dbus-signal', signal='PresencesChanged', args=[{eve: AVAILABLE}])
 
+    # We also get a message from a contact before we get the roster (presumably
+    # they sent this while we were offline?). This shouldn't affect the contact
+    # being reported as offline when we finally do get the roster, but it used
+    # to: <https://bugs.freedesktop.org/show_bug.cgi?id=41743>.
+    stream.send(
+        elem('message', from_='amy@foo.com', type='chat')(
+          elem('body')(u'why are you never online?')
+        ))
+    q.expect('dbus-signal', signal='MessageReceived')
+
     event.stanza['type'] = 'result'
     event.query.addChild(make_roster_item('amy@foo.com', 'both'))
     event.query.addChild(make_roster_item('bob@foo.com', 'from'))