[folks] Ensure the IMable.im_addresses property does not contain duplicates

[Travis Reitter <treitter src gnome org>]

commit ae5928a23c08501f0f47db8e29f51f5f6a838889
Author: Philip Withnall <philip withnall collabora co uk>
Date:   Fri Aug 20 11:53:18 2010 +0100

    Ensure the IMable.im_addresses property does not contain duplicates
    
    This can cause Personas to appear multiple times in a linked Individual,
    leading to bad state. Helps: bgo#626544

 backends/key-file/kf-persona.vala |   14 ++++++++++++--
 folks/imable.vala                 |   12 +++++++++---
 folks/individual-aggregator.vala  |   22 ++++++++++++++++++++--
 3 files changed, 41 insertions(+), 7 deletions(-)
---
diff --git a/backends/key-file/kf-persona.vala b/backends/key-file/kf-persona.vala
index f610f43..6f69c33 100644
--- a/backends/key-file/kf-persona.vala
+++ b/backends/key-file/kf-persona.vala
@@ -19,6 +19,7 @@
  */
 
 using GLib;
+using Gee;
 using Folks;
 using Folks.Backends.Kf;
 
@@ -110,11 +111,20 @@ public class Folks.Backends.Kf.Persona : Folks.Persona,
 
               /* FIXME: We have to convert our nice efficient string[] to a
                * GenericArray<string> because Vala doesn't like null-terminated
-               * arrays as generic types. */
+               * arrays as generic types.
+               * We can take this opportunity to remove duplicates. */
+              HashSet<string> address_set = new HashSet<string> ();
               GenericArray<string> im_address_array =
                   new GenericArray<string> ();
+
               foreach (string address in im_addresses)
-                im_address_array.add (address);
+                {
+                  if (!address_set.contains (address))
+                    {
+                      im_address_array.add (address);
+                      address_set.add (address);
+                    }
+                }
 
               this._im_addresses.insert (protocol, im_address_array);
             }
diff --git a/folks/imable.vala b/folks/imable.vala
index db2b28f..c5cced5 100644
--- a/folks/imable.vala
+++ b/folks/imable.vala
@@ -26,14 +26,20 @@ using GLib;
 public interface Folks.IMable : Object
 {
   /* FIXME: We have to use GenericArray<string> here rather than string[] as
-   * null-terminated arrays aren't supported as generic types yet. */
+   * null-terminated arrays aren't supported as generic types yet. It would be
+   * best if we changed to using a proper ordered set datatype, which inherently
+   * disallows duplicates, while retaining the ordering of its members.
+   * (bgo#627483) */
   /**
-   * A mapping of IM protocol to an ordered list of IM addresses.
+   * A mapping of IM protocol to an ordered set of IM addresses.
    *
-   * Each mapping is from an arbitrary protocol identifier to a list of IM
+   * Each mapping is from an arbitrary protocol identifier to a set of IM
    * addresses on that protocol for the contact, listed in preference order.
    * The most-preferred IM address for each protocol comes first in that
    * protocol's list.
+   *
+   * There must be no duplicate IM addresses in each ordered set, though a given
+   * IM address may be present in the sets for different protocols.
    */
   public abstract HashTable<string, GenericArray<string>> im_addresses
     {
diff --git a/folks/individual-aggregator.vala b/folks/individual-aggregator.vala
index a33aa33..4c0c4e1 100644
--- a/folks/individual-aggregator.vala
+++ b/folks/individual-aggregator.vala
@@ -621,8 +621,14 @@ public class Folks.IndividualAggregator : Object
       /* FIXME: We hardcode this to use the key-file backend for now */
       assert (this.writeable_store.type_id == "key-file");
 
+      /* `protocols` will be passed to the new Kf.Persona, whereas `_protocols`
+       * is used to ensure we don't get duplicate IM addresses in the ordered
+       * set of addresses for each protocol in `protocols`. It's temporary. */
       HashTable<string, GenericArray<string>> protocols =
           new HashTable<string, GenericArray<string>> (str_hash, str_equal);
+      HashTable<string, HashSet<string>> _protocols =
+          new HashTable<string, HashSet<string>> (str_hash, str_equal);
+
       personas.foreach ((p) =>
         {
           unowned Persona persona = (Persona) p;
@@ -637,16 +643,28 @@ public class Folks.IndividualAggregator : Object
 
               GenericArray<string> existing_addresses =
                   protocols.lookup (protocol);
-              if (existing_addresses == null)
+              HashSet<string> address_set = _protocols.lookup (protocol);
+
+              if (existing_addresses == null || address_set == null)
                 {
                   existing_addresses = new GenericArray<string> ();
+                  address_set = new HashSet<string> ();
+
                   protocols.insert (protocol, existing_addresses);
+                  _protocols.insert (protocol, address_set);
                 }
 
               addresses.foreach ((a) =>
                 {
                   unowned string address = (string) a;
-                  existing_addresses.add (address);
+
+                  /* Only add the IM address to the ordered set if it isn't
+                   * already a member. */
+                  if (!address_set.contains (address))
+                    {
+                      existing_addresses.add (address);
+                      address_set.add (address);
+                    }
                 });
             });
         });