Merge remote branch 'pwith/offline-fixes' into prep-0.1.11
[platform/upstream/folks.git] / backends / telepathy / tpf-persona-store.vala
index 621ab96..60dfc03 100644 (file)
  *
  * Authors:
  *       Travis Reitter <travis.reitter@collabora.co.uk>
+ *       Philip Withnall <philip.withnall@collabora.co.uk>
  */
 
 using GLib;
 using Gee;
-using Tp;
-using Tp.ContactFeature;
+using TelepathyGLib;
+using TelepathyGLib.ContactFeature;
 using Folks;
 
+/**
+ * A persona store which is associated with a single Telepathy account. It will
+ * create {@link Persona}s for each of the contacts in the published, stored or
+ * subscribed
+ * [[http://people.collabora.co.uk/~danni/telepathy-book/chapter.channel.html|channels]]
+ * of the account.
+ */
 public class Tpf.PersonaStore : Folks.PersonaStore
 {
   private string[] undisplayed_groups = { "publish", "stored", "subscribe" };
@@ -38,30 +46,64 @@ public class Tpf.PersonaStore : Folks.PersonaStore
   private HashMap<string, Channel> standard_channels_unready;
   private HashMap<string, Channel> group_channels_unready;
   private HashMap<string, Channel> groups;
+  /* FIXME: Should be HashSet<Handle> */
+  private HashSet<uint> favourite_handles;
   private Channel publish;
-  private Channel subscribe;
   private Channel stored;
+  private Channel subscribe;
   private Connection conn;
   private TpLowlevel ll;
   private AccountManager account_manager;
+  private Logger logger;
+
+  internal signal void group_members_changed (string group,
+      GLib.List<Persona>? added, GLib.List<Persona>? removed);
+  internal signal void group_removed (string group, GLib.Error? error);
 
+
+  /**
+   * The Telepathy account this store is based upon.
+   */
   [Property(nick = "basis account",
       blurb = "Telepathy account this store is based upon")]
   public Account account { get; construct; }
+
+  /**
+   * {@inheritDoc}
+   */
   public override string type_id { get; private set; }
+
+  /**
+   * {@inheritDoc}
+   */
   public override string id { get; private set; }
+
+  /**
+   * {@inheritDoc}
+   */
   public override HashTable<string, Persona> personas
     {
       get { return this._personas; }
     }
 
+  /**
+   * Create a new PersonaStore.
+   *
+   * Create a new persona store to store the {@link Persona}s for the contacts
+   * in the Telepathy account provided by `account`.
+   */
   public PersonaStore (Account account)
     {
       Object (account: account);
 
       this.type_id = "telepathy";
-      this.id = account.get_object_path (account);
+      this.id = account.get_object_path ();
 
+      this.reset ();
+    }
+
+  private void reset ()
+    {
       this._personas = new HashTable<string, Persona> (str_hash,
           str_equal);
       this.conn = null;
@@ -73,12 +115,17 @@ public class Tpf.PersonaStore : Folks.PersonaStore
       this.group_outgoing_removes = new HashMap<string, HashSet<Tpf.Persona>> (
           );
       this.publish = null;
-      this.subscribe = null;
       this.stored = null;
+      this.subscribe = null;
       this.standard_channels_unready = new HashMap<string, Channel> ();
       this.group_channels_unready = new HashMap<string, Channel> ();
       this.groups = new HashMap<string, Channel> ();
+      this.favourite_handles = new HashSet<uint> ();
       this.ll = new TpLowlevel ();
+    }
+
+  public override async void prepare ()
+    {
       this.account_manager = AccountManager.dup ();
 
       this.account_manager.account_disabled.connect ((a) =>
@@ -99,37 +146,194 @@ public class Tpf.PersonaStore : Folks.PersonaStore
 
       this.account.status_changed.connect (this.account_status_changed_cb);
 
-      Tp.ConnectionStatusReason reason;
+      TelepathyGLib.ConnectionStatusReason reason;
       var status = this.account.get_connection_status (out reason);
       /* immediately handle accounts which are not currently being disconnected
        */
-      if (status != Tp.ConnectionStatus.DISCONNECTED)
+      if (status != TelepathyGLib.ConnectionStatus.DISCONNECTED)
+        {
+          this.account_status_changed_cb (
+              TelepathyGLib.ConnectionStatus.DISCONNECTED, status, reason, null,
+              null);
+        }
+
+      try
+        {
+          this.logger = new Logger (this.id);
+          this.logger.invalidated.connect (() =>
+            {
+              warning ("lost connection to the telepathy-logger service");
+              this.logger = null;
+            });
+          this.logger.favourite_contacts_changed.connect (
+              this.favourite_contacts_changed_cb);
+        }
+      catch (DBus.Error e)
         {
-          this.account_status_changed_cb (Tp.ConnectionStatus.DISCONNECTED,
-              status, reason, null, null);
+          warning ("couldn't connect to the telepathy-logger service");
+          this.logger = null;
         }
     }
 
-  private void account_status_changed_cb (ConnectionStatus old_status,
-      ConnectionStatus new_status, ConnectionStatusReason reason,
-      string? dbus_error_name, GLib.HashTable? details)
+  private async void initialise_favourite_contacts ()
     {
-      if (new_status != Tp.ConnectionStatus.CONNECTED)
+      if (this.logger == null)
         return;
 
-      var conn = this.account.get_connection ();
-      conn.call_when_ready (this.connection_ready_cb);
+      /* Get an initial set of favourite contacts */
+      try
+        {
+          string[] contacts = yield this.logger.get_favourite_contacts ();
+
+          if (contacts.length == 0)
+            return;
+
+          /* Note that we don't need to release these handles, as they're
+           * also held by the relevant contact objects, and will be released
+           * as appropriate by those objects (we're circumventing tp-glib's
+           * handle reference counting). */
+          this.conn.request_handles (-1, HandleType.CONTACT, contacts,
+            (c, ht, h, i, e, w) =>
+              {
+                try
+                  {
+                    this.change_favourites_by_request_handles ((Handle[]) h, i,
+                        e, true);
+                  }
+                catch (GLib.Error e)
+                  {
+                    warning ("couldn't get list of favourite contacts: %s",
+                        e.message);
+                  }
+              },
+            this);
+          /* FIXME: Have to pass this as weak_object parameter since Vala
+           * seems to swap the order of user_data and weak_object in the
+           * callback. */
+        }
+      catch (DBus.Error e)
+        {
+          warning ("couldn't get list of favourite contacts: %s", e.message);
+        }
+    }
+
+  private void change_favourites_by_request_handles (Handle[] handles,
+      string[] ids, GLib.Error? error, bool add) throws GLib.Error
+    {
+      if (error != null)
+        throw error;
+
+      for (var i = 0; i < handles.length; i++)
+        {
+          Handle h = handles[i];
+          Persona p = this.handle_persona_map[h];
+
+          /* Add/Remove the handle to the set of favourite handles, since we
+           * might not have the corresponding contact yet */
+          if (add)
+            this.favourite_handles.add (h);
+          else
+            this.favourite_handles.remove (h);
+
+          /* If the persona isn't in the handle_persona_map yet, it's most
+           * likely because the account hasn't connected yet (and we haven't
+           * received the roster). If there are already entries in
+           * handle_persona_map, the account *is* connected and we should
+           * warn about the unknown persona. */
+          if (p == null && this.handle_persona_map.size > 0)
+            {
+              warning ("unknown persona '%s' in favourites list", ids[i]);
+              continue;
+            }
+
+          /* Mark or unmark the persona as a favourite */
+          if (p != null)
+            p.is_favourite = add;
+        }
+    }
+
+  private void favourite_contacts_changed_cb (string[] added, string[] removed)
+    {
+      /* Don't listen to favourites updates if the account is disconnected. */
+      if (this.conn == null)
+        return;
+
+      /* Add favourites */
+      if (added.length > 0)
+        {
+          this.conn.request_handles (-1, HandleType.CONTACT, added,
+              (c, ht, h, i, e, w) =>
+                {
+                  try
+                    {
+                      this.change_favourites_by_request_handles ((Handle[]) h,
+                          i, e, true);
+                    }
+                  catch (GLib.Error e)
+                    {
+                      warning ("couldn't add favourite contacts: %s",
+                          e.message);
+                    }
+                },
+              this);
+        }
+
+      /* Remove favourites */
+      if (removed.length > 0)
+        {
+          this.conn.request_handles (-1, HandleType.CONTACT, removed,
+              (c, ht, h, i, e, w) =>
+                {
+                  try
+                    {
+                      this.change_favourites_by_request_handles ((Handle[]) h,
+                          i, e, false);
+                    }
+                  catch (GLib.Error e)
+                    {
+                      warning ("couldn't remove favourite contacts: %s",
+                          e.message);
+                    }
+                },
+              this);
+        }
     }
 
-  private void connection_ready_cb (Connection conn, GLib.Error? error)
+  /* FIXME: the second generic type for details is "weak GLib.Value", but Vala
+   * doesn't accept it as a generic type */
+  private void account_status_changed_cb (uint old_status, uint new_status,
+      uint reason, string? dbus_error_name,
+      GLib.HashTable<weak string, weak void*>? details)
     {
-      this.ll.connection_connect_to_new_group_channels (conn,
-          this.new_group_channels_cb);
+      if (new_status == TelepathyGLib.ConnectionStatus.DISCONNECTED)
+        {
+          this.personas_changed (null, this._personas.get_values (), null, null,
+              0);
+          this.reset ();
+          return;
+        }
+      else if (new_status != TelepathyGLib.ConnectionStatus.CONNECTED)
+        return;
+
+      var conn = this.account.get_connection ();
+      conn.notify["connection-ready"].connect ((s, p) =>
+        {
+          var c = (Connection) s;
 
-      this.add_standard_channel (conn, "stored");
-      this.add_standard_channel (conn, "publish");
-      this.add_standard_channel (conn, "subscribe");
-      this.conn = conn;
+          this.ll.connection_connect_to_new_group_channels (c,
+              this.new_group_channels_cb);
+
+          this.add_standard_channel (c, "publish");
+          this.add_standard_channel (c, "stored");
+          this.add_standard_channel (c, "subscribe");
+          this.conn = c;
+
+          /* We can only initialise the favourite contacts once conn is prepared
+           */
+          this.initialise_favourite_contacts.begin ();
+        });
+
+      conn.prepare_async.begin (null);
     }
 
   private void new_group_channels_cb (void *data)
@@ -197,112 +401,106 @@ public class Tpf.PersonaStore : Folks.PersonaStore
             {
               this.publish = c;
 
-              c.group_members_changed.connect (
-                  this.publish_channel_group_members_changed_cb);
+              c.group_members_changed_detailed.connect (
+                  this.publish_channel_group_members_changed_detailed_cb);
             }
           else if (name == "stored")
             {
               this.stored = c;
 
-              c.group_members_changed.connect (
-                  this.stored_channel_group_members_changed_cb);
+              c.group_members_changed_detailed.connect (
+                  this.stored_channel_group_members_changed_detailed_cb);
             }
           else if (name == "subscribe")
             {
               this.subscribe = c;
 
-              c.group_members_changed.connect (
-                  this.subscribe_channel_group_members_changed_cb);
+              c.group_members_changed_detailed.connect (
+                  this.subscribe_channel_group_members_changed_detailed_cb);
             }
 
           this.standard_channels_unready.remove (name);
 
           c.invalidated.connect (this.channel_invalidated_cb);
 
-          unowned IntSet members = c.group_get_members ();
+          unowned IntSet? members = c.group_get_members ();
           if (members != null)
-            this.channel_group_pend_incoming_adds (c, members.to_array ());
+            {
+              this.channel_group_pend_incoming_adds.begin (c,
+                  members.to_array (), true);
+            }
         });
     }
 
-  private void publish_channel_group_members_changed_cb (Channel channel,
-      string message,
+  private void publish_channel_group_members_changed_detailed_cb (
+      Channel channel,
       /* FIXME: Array<uint> => Array<Handle>; parser bug */
-      Array<uint>? added,
-      Array<uint>? removed,
-      Array<uint>? local_pending,
-      Array<uint>? remote_pending,
-      uint actor,
-      uint reason)
+      Array<weak uint> added,
+      Array<weak uint> removed,
+      Array<weak uint> local_pending,
+      Array<weak uint> remote_pending,
+      HashTable details)
     {
-      if (added != null)
-        this.channel_group_pend_incoming_adds (channel, added);
+      if (added.length > 0)
+        this.channel_group_pend_incoming_adds.begin (channel, added, true);
+
+      /* we refuse to send these contacts our presence, so remove them */
+      for (var i = 0; i < removed.length; i++)
+        {
+          var handle = removed.index (i);
+          this.ignore_by_handle_if_needed (handle, details);
+        }
 
       /* FIXME: continue for the other arrays */
     }
 
-  private void stored_channel_group_members_changed_cb (Channel channel,
-      string message,
+  private void stored_channel_group_members_changed_detailed_cb (
+      Channel channel,
       /* FIXME: Array<uint> => Array<Handle>; parser bug */
-      Array<uint>? added,
-      Array<uint>? removed,
-      Array<uint>? local_pending,
-      Array<uint>? remote_pending,
-      uint actor,
-      uint reason)
+      Array<weak uint> added,
+      Array<weak uint> removed,
+      Array<weak uint> local_pending,
+      Array<weak uint> remote_pending,
+      HashTable details)
     {
-      if (added != null)
+      if (added.length > 0)
+        this.channel_group_pend_incoming_adds.begin (channel, added, true);
+
+      for (var i = 0; i < removed.length; i++)
         {
-          this.channel_group_pend_incoming_adds (channel, added);
+          var handle = removed.index (i);
+          this.ignore_by_handle_if_needed (handle, details);
         }
-
-      /* FIXME: continue for the other arrays */
     }
-
-  private void subscribe_channel_group_members_changed_cb (Channel channel,
-      string message,
+  private void subscribe_channel_group_members_changed_detailed_cb (
+      Channel channel,
       /* FIXME: Array<uint> => Array<Handle>; parser bug */
-      Array<uint>? added,
-      Array<uint>? removed,
-      Array<uint>? local_pending,
-      Array<uint>? remote_pending,
-      uint actor,
-      uint reason)
+      Array<weak uint> added,
+      Array<weak uint> removed,
+      Array<weak uint> local_pending,
+      Array<weak uint> remote_pending,
+      HashTable details)
     {
-      if (added != null)
+      if (added.length > 0)
         {
-          this.channel_group_pend_incoming_adds (channel, added);
+          this.channel_group_pend_incoming_adds.begin (channel, added, true);
 
           /* expose ourselves to anyone we can see */
           if (this.publish != null)
-            this.channel_group_pend_incoming_adds (this.publish, added);
+            {
+              this.channel_group_pend_incoming_adds.begin (this.publish, added,
+                  true);
+            }
         }
 
-      /* FIXME: continue for the other arrays */
-    }
-
-  private void set_up_new_group_channel (Channel channel)
-    {
-      /* hold a ref to the channel here until it's ready, so it doesn't
-       * disappear */
-      this.group_channels_unready[channel.get_identifier ()] = channel;
-
-      channel.notify["channel-ready"].connect ((s, p) =>
+      /* these contacts refused to send us their presence, so remove them */
+      for (var i = 0; i < removed.length; i++)
         {
-          var c = (Channel) s;
-          var name = c.get_identifier ();
-
-          this.groups[name] = c;
-          this.group_channels_unready.remove (name);
-
-          c.invalidated.connect (this.channel_invalidated_cb);
-          c.group_members_changed.connect (
-            this.group_channel_group_members_changed_cb);
+          var handle = removed.index (i);
+          this.ignore_by_handle_if_needed (handle, details);
+        }
 
-          unowned IntSet members = c.group_get_members ();
-          if (members != null)
-            this.channel_group_pend_incoming_adds (c, members.to_array ());
-        });
+      /* FIXME: continue for the other arrays */
     }
 
   private void channel_invalidated_cb (Proxy proxy, uint domain, int code,
@@ -319,22 +517,156 @@ public class Tpf.PersonaStore : Folks.PersonaStore
         this.subscribe = null;
       else
         {
-          var error = new GLib.Error ((Quark) domain, code, message);
+          var error = new GLib.Error ((Quark) domain, code, "%s", message);
           var name = channel.get_identifier ();
           this.group_removed (name, error);
           this.groups.remove (name);
         }
     }
 
-  private void channel_group_pend_incoming_adds (Channel channel,
-      Array<uint> adds)
+  private void ignore_by_handle_if_needed (uint handle,
+      HashTable<string, HashTable<string, Value?>> details)
+    {
+      unowned TelepathyGLib.IntSet members;
+
+      if (this.subscribe != null)
+        {
+          members = this.subscribe.group_get_members ();
+          if (members.is_member (handle))
+            return;
+
+          members = this.subscribe.group_get_remote_pending ();
+          if (members.is_member (handle))
+            return;
+        }
+
+      if (this.publish != null)
+        {
+          members = this.publish.group_get_members ();
+          if (members.is_member (handle))
+            return;
+        }
+
+      string? message = TelepathyGLib.asv_get_string (details, "message");
+      bool valid;
+      Persona? actor = null;
+      uint32 actor_handle = TelepathyGLib.asv_get_uint32 (details, "actor",
+          out valid);
+      if (actor_handle > 0 && valid)
+        actor = this.handle_persona_map[actor_handle];
+
+      Groups.ChangeReason reason = Groups.ChangeReason.NONE;
+      uint32 tp_reason = TelepathyGLib.asv_get_uint32 (details, "change-reason",
+          out valid);
+      if (valid)
+        reason = change_reason_from_tp_reason (tp_reason);
+
+      this.ignore_by_handle (handle, message, actor, reason);
+    }
+
+  private Groups.ChangeReason change_reason_from_tp_reason (uint reason)
+    {
+      return (Groups.ChangeReason) reason;
+    }
+
+  private void ignore_by_handle (uint handle, string? message, Persona? actor,
+      Groups.ChangeReason reason)
+    {
+      var persona = this.handle_persona_map[handle];
+
+      /*
+       * remove all handle-keyed entries
+       */
+      this.handle_persona_map.remove (handle);
+
+      /* skip channel_group_incoming_adds because they occurred after removal */
+
+      if (persona == null)
+        return;
+
+      /*
+       * remove all persona-keyed entries
+       */
+      foreach (var entry in this.channel_group_personas_map)
+        {
+          var channel = (Channel) entry.key;
+          var members = this.channel_group_personas_map[channel];
+          if (members != null)
+            members.remove (persona);
+        }
+
+      foreach (var entry in this.group_outgoing_adds)
+        {
+          var name = (string) entry.key;
+          var members = this.group_outgoing_adds[name];
+          if (members != null)
+            members.remove (persona);
+        }
+
+      var personas = new GLib.List<Persona> ();
+      personas.append (persona);
+      this.personas_changed (null, personas, message, actor, reason);
+      this._personas.remove (persona.iid);
+    }
+
+  /**
+   * {@inheritDoc}
+   */
+  public override async void remove_persona (Folks.Persona persona)
+    {
+      var tp_persona = (Tpf.Persona) persona;
+
+      try
+        {
+          this.ll.channel_group_change_membership (this.stored,
+              (Handle) tp_persona.contact.handle, false);
+        }
+      catch (GLib.Error e1)
+        {
+          warning ("failed to remove persona '%s' (%s) from stored list: %s",
+              tp_persona.uid, tp_persona.alias, e1.message);
+        }
+
+      try
+        {
+          this.ll.channel_group_change_membership (this.subscribe,
+              (Handle) tp_persona.contact.handle, false);
+        }
+      catch (GLib.Error e2)
+        {
+          warning ("failed to remove persona '%s' (%s) from subscribe list: %s",
+              tp_persona.uid, tp_persona.alias, e2.message);
+        }
+
+      try
+        {
+          this.ll.channel_group_change_membership (this.publish,
+              (Handle) tp_persona.contact.handle, false);
+        }
+      catch (GLib.Error e3)
+        {
+          warning ("failed to remove persona '%s' (%s) from publish list: %s",
+              tp_persona.uid, tp_persona.alias, e3.message);
+        }
+
+      /* the contact will be actually removed (and signaled) when we hear back
+       * from the server */
+    }
+
+  /* Only non-group contact list channels should use create_personas == true,
+   * since the exposed set of Personas are meant to be filtered by them */
+  private async void channel_group_pend_incoming_adds (Channel channel,
+      Array<uint> adds,
+      bool create_personas)
     {
       var adds_length = adds != null ? adds.length : 0;
       if (adds_length >= 1)
         {
-          /* this won't complete before we would add the personas to the group,
-           * so we have to buffer the contact handles below */
-          this.create_personas_from_channel_handles_async (channel, adds);
+          if (create_personas)
+            {
+              yield this.create_personas_from_channel_handles_async (channel,
+                  adds);
+            }
 
           for (var i = 0; i < adds.length; i++)
             {
@@ -360,23 +692,48 @@ public class Tpf.PersonaStore : Folks.PersonaStore
       this.channel_groups_add_new_personas ();
     }
 
-  private void group_channel_group_members_changed_cb (Channel channel,
-      string message,
+  private void set_up_new_group_channel (Channel channel)
+    {
+      /* hold a ref to the channel here until it's ready, so it doesn't
+       * disappear */
+      this.group_channels_unready[channel.get_identifier ()] = channel;
+
+      channel.notify["channel-ready"].connect ((s, p) =>
+        {
+          var c = (Channel) s;
+          var name = c.get_identifier ();
+
+          this.groups[name] = c;
+          this.group_channels_unready.remove (name);
+
+          c.invalidated.connect (this.channel_invalidated_cb);
+          c.group_members_changed_detailed.connect (
+            this.channel_group_members_changed_detailed_cb);
+
+          unowned IntSet members = c.group_get_members ();
+          if (members != null)
+            {
+              this.channel_group_pend_incoming_adds.begin (c,
+                members.to_array (), false);
+            }
+        });
+    }
+
+  private void channel_group_members_changed_detailed_cb (Channel channel,
       /* FIXME: Array<uint> => Array<Handle>; parser bug */
-      Array<uint>? added,
-      Array<uint>? removed,
-      Array<uint>? local_pending,
-      Array<uint>? remote_pending,
-      uint actor,
-      uint reason)
+      Array<weak uint> added,
+      Array<weak uint> removed,
+      Array<weak uint> local_pending,
+      Array<weak uint> remote_pending,
+      HashTable details)
     {
       if (added != null)
-        this.channel_group_pend_incoming_adds (channel, added);
+        this.channel_group_pend_incoming_adds.begin (channel, added, false);
 
       /* FIXME: continue for the other arrays */
     }
 
-  public override async void change_group_membership (Folks.Persona persona,
+  internal async void change_group_membership (Folks.Persona persona,
       string group, bool is_member)
     {
       var tp_persona = (Tpf.Persona) persona;
@@ -406,8 +763,8 @@ public class Tpf.PersonaStore : Folks.PersonaStore
         }
     }
 
-  private void change_standard_contact_list_membership (Tp.Channel channel,
-      Folks.Persona persona, bool is_member)
+  private void change_standard_contact_list_membership (
+      TelepathyGLib.Channel channel, Folks.Persona persona, bool is_member)
     {
       var tp_persona = (Tpf.Persona) persona;
 
@@ -450,7 +807,8 @@ public class Tpf.PersonaStore : Folks.PersonaStore
     }
 
   /* FIXME: Array<uint> => Array<Handle>; parser bug */
-  private void create_personas_from_channel_handles_async (Channel channel,
+  private async void create_personas_from_channel_handles_async (
+      Channel channel,
       Array<uint> channel_handles)
     {
       ContactFeature[] features =
@@ -460,7 +818,7 @@ public class Tpf.PersonaStore : Folks.PersonaStore
           PRESENCE
         };
 
-      Handle[] contact_handles = {};
+      uint[] contact_handles = {};
       for (var i = 0; i < channel_handles.length; i++)
         {
           var channel_handle = (Handle) channel_handles.index (i);
@@ -470,36 +828,35 @@ public class Tpf.PersonaStore : Folks.PersonaStore
             contact_handles += contact_handle;
         }
 
-      /* FIXME: we have to use 'this' as the weak object because the
-        * weak object gets passed into the underlying callback as the
-        * object instance; there may be a way to fix this with the
-        * instance_pos directive, but I couldn't get it to work */
-      if (contact_handles.length > 0)
-        this.conn.get_contacts_by_handle (contact_handles, features,
-            this.get_contacts_by_handle_cb, this);
-    }
+      try
+        {
+          if (contact_handles.length < 1)
+            return;
 
-  private void get_contacts_by_handle_cb (Connection connection,
-      uint n_contacts,
-      [CCode (array_length = false)]
-      Contact[] contacts,
-      uint n_failed,
-      [CCode (array_length = false)]
-      Handle[] failed,
-      GLib.Error error,
-      GLib.Object weak_object)
-    {
-      if (n_failed >= 1)
-        warning ("failed to retrieve contacts for handles:");
+          unowned GLib.List<TelepathyGLib.Contact> contacts =
+              yield this.ll.connection_get_contacts_by_handle_async (
+                  this.conn, contact_handles, (uint[]) features);
 
-      for (var i = 0; i < n_failed; i++)
+          if (contacts == null || contacts.length () < 1)
+            return;
+
+          var contacts_array = new TelepathyGLib.Contact[contacts.length ()];
+          var j = 0;
+          unowned GLib.List<TelepathyGLib.Contact> l = contacts;
+          for (; l != null; l = l.next)
+            {
+              contacts_array[j] = l.data;
+              j++;
+            }
+
+          this.add_new_personas_from_contacts (contacts_array);
+        }
+      catch (GLib.Error e)
         {
-          Handle h = failed[i];
-          warning ("    %u", (uint) h);
+          warning ("failed to create personas from incoming contacts in " +
+              "channel '%s': %s",
+              channel.get_identifier (), e.message);
         }
-
-      /* we have to manually pass the length since we don't get it */
-      this.add_new_personas_from_contacts (contacts, n_contacts);
     }
 
   private async GLib.List<Tpf.Persona>? create_personas_from_contact_ids (
@@ -514,23 +871,24 @@ public class Tpf.PersonaStore : Folks.PersonaStore
 
       if (contact_ids.length > 0)
         {
-          unowned GLib.List<Tp.Contact> contacts =
+          unowned GLib.List<TelepathyGLib.Contact> contacts =
               yield this.ll.connection_get_contacts_by_id_async (
-                  this.conn, contact_ids, features);
+                  this.conn, contact_ids, (uint[]) features);
 
           GLib.List<Persona> personas = new GLib.List<Persona> ();
           uint err_count = 0;
           string err_format = "";
-          unowned GLib.List<Tp.Contact> l;
+          unowned GLib.List<TelepathyGLib.Contact> l;
           for (l = contacts; l != null; l = l.next)
             {
               var contact = l.data;
               try
                 {
-                  var persona = new Tpf.Persona (contact, this);
-                  personas.prepend (persona);
+                  var persona = this.add_persona_from_contact (contact);
+                  if (persona != null)
+                    personas.prepend (persona);
                 }
-              catch (Tp.Error e)
+              catch (Tpf.PersonaError e)
                 {
                   if (err_count == 0)
                     err_format = "failed to create %u personas:\n";
@@ -547,32 +905,51 @@ public class Tpf.PersonaStore : Folks.PersonaStore
                   err_count);
             }
 
+          if (personas != null)
+            this.personas_changed (personas, null, null, null, 0);
+
           return personas;
         }
 
       return null;
     }
 
-  private void add_new_personas_from_contacts (Contact[] contacts,
-      uint n_contacts)
+  private Tpf.Persona? add_persona_from_contact (Contact contact)
+      throws Tpf.PersonaError
     {
-      var personas_new = new HashTable<string, Persona> (str_hash, str_equal);
-      for (var i = 0; i < n_contacts; i++)
+      var h = contact.get_handle ();
+      if (this.handle_persona_map[h] == null)
         {
-          var contact = contacts[i];
+          var persona = new Tpf.Persona (contact, this);
+
+          this._personas.insert (persona.iid, persona);
+          this.handle_persona_map[h] = persona;
+
+          /* If the handle is a favourite, ensure the persona's marked
+           * as such. This deals with the case where we receive a
+           * contact _after_ we've discovered that they're a
+           * favourite. */
+          persona.is_favourite = this.favourite_handles.contains (h);
+
+          return persona;
+        }
+
+      return null;
+    }
 
+
+  private void add_new_personas_from_contacts (Contact[] contacts)
+    {
+      GLib.List<Persona> personas = new GLib.List<Persona> ();
+      foreach (Contact contact in contacts)
+        {
           try
             {
-              var persona = new Tpf.Persona (contact, this);
-              if (this._personas.lookup (persona.iid) == null)
-                {
-                  personas_new.insert (persona.iid, persona);
-
-                  this._personas.insert (persona.iid, persona);
-                  this.handle_persona_map[contact.get_handle ()] = persona;
-                }
+              var persona = this.add_persona_from_contact (contact);
+              if (persona != null)
+                personas.prepend (persona);
             }
-          catch (Tp.Error e)
+          catch (Tpf.PersonaError e)
             {
               warning ("failed to create persona from contact '%s' (%p)",
                   contact.alias, contact);
@@ -581,11 +958,8 @@ public class Tpf.PersonaStore : Folks.PersonaStore
 
       this.channel_groups_add_new_personas ();
 
-      if (personas_new.size () >= 1)
-        {
-          GLib.List<Persona> personas = personas_new.get_values ();
-          this.personas_added (personas);
-        }
+      if (personas != null)
+        this.personas_changed (personas, null, null, null, 0);
     }
 
   private void channel_groups_add_new_personas ()
@@ -642,10 +1016,13 @@ public class Tpf.PersonaStore : Folks.PersonaStore
       return true;
     }
 
+  /**
+   * {@inheritDoc}
+   */
   public override async Folks.Persona? add_persona_from_details (
-      HashTable<string, string> details) throws Folks.PersonaStoreError
+      HashTable<string, Value?> details) throws Folks.PersonaStoreError
     {
-      var contact_id = details.lookup ("contact");
+      var contact_id = TelepathyGLib.asv_get_string (details, "contact");
       if (contact_id == null)
         {
           throw new PersonaStoreError.INVALID_ARGUMENT (
@@ -696,4 +1073,47 @@ public class Tpf.PersonaStore : Folks.PersonaStore
 
       return null;
     }
+
+  /**
+   * Change the favourite status of a persona in this store.
+   *
+   * This function is idempotent, but relies upon having a connection to the
+   * Telepathy logger service, so may fail if that connection is not present.
+   */
+  internal async void change_is_favourite (Folks.Persona persona,
+      bool is_favourite)
+    {
+      /* It's possible for us to not be able to connect to the logger;
+       * see connection_ready_cb() */
+      if (this.logger == null)
+        {
+          warning ("failed to change favourite without connection to the " +
+                   "telepathy-logger service");
+          return;
+        }
+
+      try
+        {
+          /* Add or remove the persona to the list of favourites as
+           * appropriate. */
+          var id = ((Tpf.Persona) persona).contact.get_identifier ();
+
+          if (is_favourite)
+            yield this.logger.add_favourite_contact (id);
+          else
+            yield this.logger.remove_favourite_contact (id);
+        }
+      catch (DBus.Error e)
+        {
+          warning ("failed to change a persona's favourite status");
+        }
+    }
+
+  internal async void change_alias (Tpf.Persona persona, string alias)
+    {
+      debug ("Changing alias of persona %u to '%s'.", persona.contact.handle,
+          alias);
+      this.ll.connection_set_contact_alias (this.conn,
+          (Handle) persona.contact.handle, alias);
+    }
 }