Merge branch 'grouped-alias-updates'

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=40943
Reviewed-by: Will Thompson <will.thompson@collabora.co.uk>

6 files changed, 145 insertions, 45 deletions

diff --git a/src/conn-aliasing.c b/src/conn-aliasing.c
index 8b77a8fd..bfc3b3ac 100644
--- a/src/conn-aliasing.c
+++ b/src/conn-aliasing.c
@@ -794,17 +794,32 @@ gabble_conn_aliasing_pep_nick_reply_handler (GabbleConnection *conn,
     }
 }
 
-
 void
 gabble_conn_aliasing_nickname_updated (GObject *object,
                                        TpHandle handle,
                                        gpointer user_data)
 {
+  GArray *handles;
+
+  handles = g_array_sized_new (FALSE, FALSE, sizeof (TpHandle), 1);
+  g_array_append_val (handles, handle);
+
+  gabble_conn_aliasing_nicknames_updated (object, handles, user_data);
+
+  g_array_free (handles, TRUE);
+}
+
+void
+gabble_conn_aliasing_nicknames_updated (GObject *object,
+                                        GArray *handles,
+                                        gpointer user_data)
+{
   GabbleConnection *conn = GABBLE_CONNECTION (user_data);
-  GabbleConnectionAliasSource signal_source, current_source;
-  gchar *alias = NULL;
+  GabbleConnectionAliasSource signal_source;
   GPtrArray *aliases;
-  GValue entry = { 0, };
+  guint i;
+
+  g_return_if_fail (handles->len > 0);
 
   if (object == user_data)
     {
@@ -829,47 +844,58 @@ gabble_conn_aliasing_nickname_updated (GObject *object,
       return;
     }
 
-  current_source = _gabble_connection_get_cached_alias (conn, handle, &alias);
-
-  g_assert (current_source != GABBLE_CONNECTION_ALIAS_NONE);
+  aliases = g_ptr_array_sized_new (handles->len);
 
-  /* if the active alias for this handle is already known and from
-   * a higher priority, this signal is not interesting so we do
-   * nothing */
-  if (signal_source < current_source)
+  for (i = 0; i < handles->len; i++)
     {
-      DEBUG ("ignoring boring alias change for handle %u, signal from %u "
-          "but source %u has alias \"%s\"", handle, signal_source,
-          current_source, alias);
-      goto OUT;
-    }
+      TpHandle handle = g_array_index (handles, TpHandle, i);
+      GabbleConnectionAliasSource current_source;
+      gchar *alias = NULL;
+      GValue entry = { 0, };
+
+      current_source = _gabble_connection_get_cached_alias (conn, handle,
+          &alias);
+      g_assert (current_source != GABBLE_CONNECTION_ALIAS_NONE);
+
+      /* if the active alias for this handle is already known and from
+       * a higher priority, this signal is not interesting so we do
+       * nothing */
+      if (signal_source < current_source)
+        {
+          DEBUG ("ignoring boring alias change for handle %u, signal from %u "
+              "but source %u has alias \"%s\"", handle, signal_source,
+              current_source, alias);
+          g_free (alias);
+          continue;
+        }
 
-  g_value_init (&entry, TP_STRUCT_TYPE_ALIAS_PAIR);
-  g_value_take_boxed (&entry, dbus_g_type_specialized_construct
-      (TP_STRUCT_TYPE_ALIAS_PAIR));
+      g_value_init (&entry, TP_STRUCT_TYPE_ALIAS_PAIR);
+      g_value_take_boxed (&entry,
+          dbus_g_type_specialized_construct (TP_STRUCT_TYPE_ALIAS_PAIR));
 
-  dbus_g_type_struct_set (&entry,
-      0, handle,
-      1, alias,
-      G_MAXUINT);
+      dbus_g_type_struct_set (&entry,
+          0, handle,
+          1, alias,
+          G_MAXUINT);
 
-  aliases = g_ptr_array_sized_new (1);
-  g_ptr_array_add (aliases, g_value_get_boxed (&entry));
+      g_ptr_array_add (aliases, g_value_get_boxed (&entry));
 
+      /* Check whether the roster has an entry for the handle and if so, set
+       * the roster alias so the vCard isn't fetched on every connect. */
+      if (signal_source < GABBLE_CONNECTION_ALIAS_FROM_ROSTER &&
+          gabble_roster_handle_has_entry (conn->roster, handle))
+        gabble_roster_handle_set_name (conn->roster, handle, alias, NULL);
 
-  tp_svc_connection_interface_aliasing_emit_aliases_changed (conn, aliases);
+      g_free (alias);
+    }
 
-  g_value_unset (&entry);
-  g_ptr_array_free (aliases, TRUE);
+  if (aliases->len > 0)
+    tp_svc_connection_interface_aliasing_emit_aliases_changed (conn, aliases);
 
-  /* Check whether the roster has an entry for the handle and if so, set the
-   * roster alias so the vCard isn't fetched on every connect. */
-  if (signal_source < GABBLE_CONNECTION_ALIAS_FROM_ROSTER &&
-      gabble_roster_handle_has_entry (conn->roster, handle))
-    gabble_roster_handle_set_name (conn->roster, handle, alias, NULL);
+  for (i = 0; i < aliases->len; i++)
+    g_boxed_free (TP_STRUCT_TYPE_ALIAS_PAIR, g_ptr_array_index (aliases, i));
 
-OUT:
-  g_free (alias);
+  g_ptr_array_free (aliases, TRUE);
 }
 
 static void
diff --git a/src/conn-aliasing.h b/src/conn-aliasing.h
index 8716bb78..353e3238 100644
--- a/src/conn-aliasing.h
+++ b/src/conn-aliasing.h
@@ -33,6 +33,9 @@ void conn_aliasing_iface_init (gpointer g_iface, gpointer iface_data);
 void gabble_conn_aliasing_nickname_updated (GObject *object,
     TpHandle handle, gpointer user_data);
 
+void gabble_conn_aliasing_nicknames_updated (GObject *object,
+    GArray *handles, gpointer user_data);
+
 GabbleConnectionAliasSource _gabble_connection_get_cached_alias (
     GabbleConnection *, TpHandle, gchar **);
 
diff --git a/src/connection.c b/src/connection.c
index dc724bd9..b06cbc10 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -311,8 +311,8 @@ _gabble_connection_create_channel_managers (TpBaseConnection *conn)
   GPtrArray *tmp;
 
   self->roster = gabble_roster_new (self);
-  g_signal_connect (self->roster, "nickname-update", G_CALLBACK
-      (gabble_conn_aliasing_nickname_updated), self);
+  g_signal_connect (self->roster, "nicknames-update", G_CALLBACK
+      (gabble_conn_aliasing_nicknames_updated), self);
   g_ptr_array_add (channel_managers, self->roster);
 
   self->priv->im_factory = g_object_new (GABBLE_TYPE_IM_FACTORY,
diff --git a/src/roster.c b/src/roster.c
index 48d2461f..939ca0c9 100644
--- a/src/roster.c
+++ b/src/roster.c
@@ -52,7 +52,7 @@
 /* signal enum */
 enum
 {
-  NICKNAME_UPDATE,
+  NICKNAMES_UPDATE,
   LAST_SIGNAL
 };
 
@@ -529,7 +529,8 @@ static GabbleRosterItem *
 _gabble_roster_item_update (GabbleRoster *roster,
                             TpHandle contact_handle,
                             WockyNode *node,
-                            gboolean google_roster_mode)
+                            gboolean google_roster_mode,
+                            gboolean *nickname_updated)
 {
   GabbleRosterPrivate *priv = roster->priv;
   GabbleRosterItem *item;
@@ -576,8 +577,12 @@ _gabble_roster_item_update (GabbleRoster *roster,
 
       DEBUG ("name for contact#%u changed to %s", contact_handle,
           name);
-      g_signal_emit (G_OBJECT (roster), signals[NICKNAME_UPDATE], 0,
-          contact_handle);
+
+      *nickname_updated = TRUE;
+    }
+  else
+    {
+      *nickname_updated = FALSE;
     }
 
   new_groups_handle_set = _parse_item_groups (node,
@@ -1089,6 +1094,7 @@ process_roster (
   TpBaseConnection *conn = (TpBaseConnection *) priv->conn;
   TpHandleRepoIface *contact_repo = tp_base_connection_get_handles (conn,
       TP_HANDLE_TYPE_CONTACT);
+  GArray *updates_nicknames = g_array_new (FALSE, FALSE, sizeof (TpHandle));
 
   /* asymmetry is because we don't get locally pending subscription
    * requests via <roster>, we get it via <presence> */
@@ -1113,6 +1119,7 @@ process_roster (
       const char *jid;
       TpHandle handle;
       GabbleRosterItem *item;
+      gboolean nickname_updated;
 
       handle = validate_roster_item (contact_repo, item_node, &jid);
 
@@ -1124,7 +1131,7 @@ process_roster (
       tp_handle_unref (contact_repo, handle);
 
       item = _gabble_roster_item_update (roster, handle, item_node,
-                                         google_roster);
+                                         google_roster, &nickname_updated);
 #ifdef ENABLE_DEBUG
       if (DEBUGGING)
         {
@@ -1134,6 +1141,9 @@ process_roster (
         }
 #endif
 
+      if (nickname_updated)
+        g_array_append_val (updates_nicknames, handle);
+
       /* handle publish list changes */
       switch (item->subscription)
         {
@@ -1307,6 +1317,9 @@ process_roster (
       _gabble_roster_item_maybe_remove (roster, handle);
     }
 
+  if (updates_nicknames)
+    g_signal_emit (roster, signals[NICKNAMES_UPDATE], 0, updates_nicknames);
+
   tp_base_contact_list_contacts_changed ((TpBaseContactList *) roster,
       changed, removed);
 
@@ -1317,6 +1330,7 @@ process_roster (
       tp_handle_set_destroy (blocking_changed);
     }
 
+  g_array_free (updates_nicknames, TRUE);
   tp_handle_set_destroy (changed);
   tp_handle_set_destroy (removed);
   tp_handle_set_destroy (referenced_handles);
@@ -3508,13 +3522,13 @@ gabble_roster_class_init (GabbleRosterClass *cls)
   base_class->dup_states = gabble_roster_dup_states;
   base_class->dup_contacts = gabble_roster_dup_contacts;
 
-  signals[NICKNAME_UPDATE] = g_signal_new (
-    "nickname-update",
+  signals[NICKNAMES_UPDATE] = g_signal_new (
+    "nicknames-update",
     G_TYPE_FROM_CLASS (cls),
     G_SIGNAL_RUN_LAST,
     0,
     NULL, NULL,
-    g_cclosure_marshal_VOID__UINT, G_TYPE_NONE, 1, G_TYPE_UINT);
+    g_cclosure_marshal_VOID__BOXED, G_TYPE_NONE, 1, DBUS_TYPE_G_UINT_ARRAY);
 }
 
 gboolean
diff --git a/tests/twisted/Makefile.am b/tests/twisted/Makefile.am
index 1b026943..6d7be144 100644
--- a/tests/twisted/Makefile.am
+++ b/tests/twisted/Makefile.am
@@ -64,6 +64,7 @@ TWISTED_TESTS = \
 	roster/ensure.py \
 	roster/groups.py \
 	roster/groups-12791.py \
+	roster/initial-aliases.py \
 	roster/push-from-contact.py \
 	roster/push-without-id.py \
 	roster/removed-from-rp-subscribe.py \
diff --git a/tests/twisted/roster/test-initial-aliases.py b/tests/twisted/roster/test-initial-aliases.py
new file mode 100644
index 00000000..d1aeb42a
--- /dev/null
+++ b/tests/twisted/roster/test-initial-aliases.py
@@ -0,0 +1,56 @@
+"""
+Test retrieving the aliases after connection.
+"""
+
+from servicetest import (
+    assertContains, assertLength
+    )
+from gabbletest import (
+    exec_test, make_result_iq
+    )
+import constants as cs
+import ns
+
+def add_contact(iq, jid, alias):
+    query = iq.firstChildElement()
+    item = query.addElement('item')
+    item['jid'] = jid
+    item['name'] = alias
+    item['subscription'] = 'both'
+
+def test(q, bus, conn, stream):
+    """
+    Check that when we receive a roster update we emit a single AliasesChanged
+    for all the contacts.
+    """
+
+    conn.Connect()
+
+    # Gabble asks the server for the initial roster
+    event = q.expect('stream-iq', iq_type='get', query_ns=ns.ROSTER)
+    result = make_result_iq(stream, event.stanza)
+
+    # We reply with a roster with two contacts
+    bob_jid = 'bob.smith@example.com'
+    bob_alias = 'Bob Smith'
+    add_contact(result, bob_jid, bob_alias)
+
+    alice_jid = 'alice@example.com'
+    alice_alias = 'Alice'
+    add_contact(result, alice_jid, alice_alias)
+
+    stream.send(result)
+
+    # Check we get a single AliasesChanged for both contacts
+    event = q.expect('dbus-signal', signal='AliasesChanged')
+    added = event.args[0]
+
+    bob_handle, alice_handle = conn.RequestHandles(cs.HT_CONTACT,
+                                                   [bob_jid, alice_jid])
+
+    assertLength(2, added)
+    assertContains((bob_handle, bob_alias), added)
+    assertContains((alice_handle, alice_alias), added)
+
+if __name__ == '__main__':
+    exec_test(test, do_connect=False)