Port Individual.personas to be a Set<Persona>

This is a major user of GLib.List, and porting it should make porting a lot
of other stuff a lot easier.

Helps: bgo#640092

2 files changed, 64 insertions, 68 deletions

diff --git a/folks/individual-aggregator.vala b/folks/individual-aggregator.vala
index 05bdc35..98a70d4 100644
--- a/folks/individual-aggregator.vala
+++ b/folks/individual-aggregator.vala
@@ -413,7 +413,7 @@ public class Folks.IndividualAggregator : Object
            * in the `final_individual`. */
           HashSet<Individual> candidate_inds = new HashSet<Individual> ();
 
-          GLib.List<Persona> final_personas = new GLib.List<Persona> ();
+          var final_personas = new HashSet<Persona> ();
           Individual final_individual = null;
 
           debug ("Aggregating persona '%s' on '%s'.", persona.uid, persona.iid);
@@ -482,8 +482,8 @@ public class Folks.IndividualAggregator : Object
                 }
             }
 
-          /* Ensure the original persona makes it into the final persona */
-          final_personas.prepend (persona);
+          /* Ensure the original persona makes it into the final individual */
+          final_personas.add (persona);
 
           if (candidate_inds.size > 0 && this._linking_enabled == true)
             {
@@ -494,17 +494,7 @@ public class Folks.IndividualAggregator : Object
                * Individuals so that the Individuals themselves are removed. */
               foreach (var individual in candidate_inds)
                 {
-                  /* FIXME: It would be faster to prepend a reversed copy of
-                   * `individual.personas`, then reverse the entire
-                   * `final_personas` list afterwards, but Vala won't let us.
-                   * We also have to reference each of `individual.personas`
-                   * manually, since copy() doesn't do that for us. */
-                  individual.personas.foreach ((p) =>
-                    {
-                      ((Persona) p).ref ();
-                    });
-
-                  final_personas.concat (individual.personas.copy ());
+                  final_personas.add_all (individual.personas);
                 }
             }
           else if (candidate_inds.size > 0)
@@ -520,9 +510,9 @@ public class Folks.IndividualAggregator : Object
           final_individual = new Individual (final_personas);
           debug ("    Created new individual '%s' with personas:",
               final_individual.id);
-          final_personas.foreach ((i) =>
+          foreach (var p in final_personas)
             {
-              var final_persona = (Persona) i;
+              var final_persona = (Persona) p;
 
               debug ("        %s (is user: %s, IID: %s)", final_persona.uid,
                   final_persona.is_user ? "yes" : "no", final_persona.iid);
@@ -567,7 +557,7 @@ public class Folks.IndividualAggregator : Object
                         });
                     }
                 }
-            });
+            }
 
           /* Remove the old Individuals. This has to be done here, as we need
            * the final_individual. */
@@ -807,7 +797,7 @@ public class Folks.IndividualAggregator : Object
         }
 
       /* If the individual has 0 personas, we've already signaled removal */
-      if (i.personas.length () > 0)
+      if (i.personas.size > 0)
         this.individuals_changed (null, i_list, null, null, 0);
       this.individuals.remove (i.id);
     }
@@ -872,10 +862,7 @@ public class Folks.IndividualAggregator : Object
 
       if (parent != null && persona != null)
         {
-          var personas = parent.personas.copy ();
-
-          personas.append (persona);
-          parent.personas = personas;
+          parent.personas.add (persona);
         }
 
       return persona;
@@ -902,17 +889,16 @@ public class Folks.IndividualAggregator : Object
    */
   public async void remove_individual (Individual individual) throws GLib.Error
     {
-      /* We have to iterate manually since using foreach() requires a sync
-       * lambda function, meaning we can't yield on the remove_persona() call.
-       *
-       * Furthermore, removing personas changes the persona list so we need to
-       * make a copy first */
-      var personas = individual.personas.copy ();
-      unowned GLib.List<unowned Persona> i;
+      /* Removing personas changes the persona set so we need to make a copy
+       * first */
+      var personas = new HashSet<Persona> ();
+      foreach (var p in individual.personas)
+        {
+          personas.add (p);
+        }
 
-      for (i = personas; i != null; i = i.next)
+      foreach (var persona in personas)
         {
-          var persona = (Persona) i.data;
           yield persona.store.remove_persona (persona);
         }
     }
@@ -1085,18 +1071,19 @@ public class Folks.IndividualAggregator : Object
           return;
         }
 
-      /* Remove all the Personas from writeable PersonaStores
-       * We have to iterate manually since using foreach() requires a sync
-       * lambda function, meaning we can't yield on the remove_persona() call */
       debug ("Unlinking Individual '%s', deleting Personas:", individual.id);
 
-      /* We have to take a copy of the Persona list before removing the
+      /* Remove all the Personas from writeable PersonaStores.
+       *
+       * We have to take a copy of the Persona list before removing the
        * Personas, as _personas_changed_cb() (which is called as a result of
        * calling _writeable_store.remove_persona()) messes around with Persona
        * lists. */
-      var personas = individual.personas.copy ();
-      foreach (var p in personas)
-        p.ref ();
+      var personas = new HashSet<Persona> ();
+      foreach (var p in individual.personas)
+        {
+          personas.add (p);
+        }
 
       foreach (var persona in personas)
         {
diff --git a/folks/individual.vala b/folks/individual.vala
index 563ddd7..9e9e6d0 100644
--- a/folks/individual.vala
+++ b/folks/individual.vala
@@ -429,6 +429,10 @@ public class Folks.Individual : Object,
   /**
    * The set of {@link Persona}s encapsulated by this Individual.
    *
+   * No order is specified over the set of personas, as such an order may be
+   * different across each of the properties implemented by the personas (e.g.
+   * should they be ordered by presence, name, star sign, etc.?).
+   *
    * Changing the set of personas may cause updates to the aggregated properties
    * provided by the Individual, resulting in property notifications for them.
    *
@@ -437,8 +441,10 @@ public class Folks.Individual : Object,
    * {@link IndividualAggregator.link_personas} or
    * {@link IndividualAggregator.unlink_individual}, which will ensure the link
    * changes are written to the appropriate backend.
+   *
+   * @since UNRELEASED
    */
-  public GLib.List<Persona> personas
+  public Set<Persona> personas
     {
       get { return this._persona_set; }
       set { this._set_personas (value, null); }
@@ -586,8 +592,10 @@ public class Folks.Individual : Object,
    * @param personas a list of {@link Persona}s to initialise the
    * {@link Individual} with, or `null`
    * @return a new Individual
+   *
+   * @since UNRELEASED
    */
-  public Individual (GLib.List<Persona>? personas)
+  public Individual (Set<Persona>? personas)
     {
       this._im_addresses = new HashMultiMap<string, string> ();
       this._web_service_addresses = new HashMultiMap<string, string> ();
@@ -779,7 +787,7 @@ public class Folks.Individual : Object,
             {
               favourite = ((FavouriteDetails) p).is_favourite;
               if (favourite == true)
-                return;
+                break;
             }
         }
 
@@ -1358,45 +1366,46 @@ public class Folks.Individual : Object,
       this.notify_property ("notes");
     }
 
-  private void _set_personas (GLib.List<Persona>? persona_list,
+  private void _set_personas (Set<Persona>? personas,
       Individual? replacement_individual)
     {
-      var persona_set = new HashSet<Persona> (null, null);
       GLib.List<Persona> added = null;
       GLib.List<Persona> removed = null;
 
-      /* Determine which Personas have been added */
-      foreach (var p in persona_list)
+      /* Determine which Personas have been added. If personas == null, we
+       * assume it's an empty set. */
+      if (personas != null)
         {
-          if (!this._persona_set.contains (p))
+          foreach (var p in personas)
             {
-              /* Keep track of how many Personas are users */
-              if (p.is_user)
-                this._persona_user_count++;
+              if (!this._persona_set.contains (p))
+                {
+                  /* Keep track of how many Personas are users */
+                  if (p.is_user)
+                    this._persona_user_count++;
 
-              added.prepend (p);
+                  added.prepend (p);
 
-              this._persona_set.add (p);
-              this._connect_to_persona (p);
+                  this._persona_set.add (p);
+                  this._connect_to_persona (p);
 
-              /* Increment the Persona count for this PersonaStore */
-              var store = p.store;
-              var num_from_store = this._stores.get (store);
-              if (num_from_store == 0)
-                {
-                  this._stores.set (store, num_from_store + 1);
-                }
-              else
-                {
-                  this._stores.set (store, 1);
+                  /* Increment the Persona count for this PersonaStore */
+                  var store = p.store;
+                  var num_from_store = this._stores.get (store);
+                  if (num_from_store == 0)
+                    {
+                      this._stores.set (store, num_from_store + 1);
+                    }
+                  else
+                    {
+                      this._stores.set (store, 1);
 
-                  store.removed.connect (this._store_removed_cb);
-                  store.personas_changed.connect (
-                      this._store_personas_changed_cb);
+                      store.removed.connect (this._store_removed_cb);
+                      store.personas_changed.connect (
+                          this._store_personas_changed_cb);
+                    }
                 }
             }
-
-          persona_set.add (p);
         }
 
       /* Determine which Personas have been removed */
@@ -1405,7 +1414,7 @@ public class Folks.Individual : Object,
         {
           var p = iter.get ();
 
-          if (!persona_set.contains (p))
+          if (personas == null || !personas.contains (p))
             {
               /* Keep track of how many Personas are users */
               if (p.is_user)