Change FieldDetails.parameters to be a MultiMap<string, string>

Helps: bgo#640092

6 files changed, 73 insertions, 90 deletions

diff --git a/NEWS b/NEWS
index dde2f31..7a06e05 100644
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,9 @@ API changes:
 * Removed LinkedHashSet in favour of Gee.HashSet
 * Backend.persona_stores is now of type Map<string, PersonaStore>
 * GroupDetails.groups is now of type Set<string>
+* FieldDetails.parameters is now of type MultiMap<string, string>
+* FieldDetails.get_parameter_values() now returns type Collection<string>
+* FieldDetails.extend_parameters() now takes type MultiMap<string, string>
 
 Overview of changes from libfolks 0.4.0 to libfolks 0.5.0
 =========================================================
diff --git a/backends/tracker/lib/trf-persona-store.vala b/backends/tracker/lib/trf-persona-store.vala
index cca6488..9a4fb8f 100644
--- a/backends/tracker/lib/trf-persona-store.vala
+++ b/backends/tracker/lib/trf-persona-store.vala
@@ -2331,18 +2331,14 @@ public class Trf.PersonaStore : Folks.PersonaStore
           else if (what == Trf.Attrib.URLS)
             {
               fd = (FieldDetails) p;
-              unowned GLib.List<string> type_p =
-                fd.get_parameter_values ("type");
-              if (type_p.length () > 0)
+              var type_p = fd.get_parameter_values ("type");
+              if (type_p.contains ("blog"))
                 {
-                  if (type_p.nth_data (0) == "blog")
-                    {
-                      related_connection = Trf.OntologyDefs.NCO_BLOG;
-                    }
-                  else if (type_p.nth_data (0) == "website")
-                    {
-                      related_connection = Trf.OntologyDefs.NCO_WEBSITE;
-                    }
+                  related_connection = Trf.OntologyDefs.NCO_BLOG;
+                }
+              else if (type_p.contains ("website"))
+                {
+                  related_connection = Trf.OntologyDefs.NCO_WEBSITE;
                 }
               attr = "'%s'".printf (fd.value);
             }
diff --git a/backends/tracker/lib/trf-persona.vala b/backends/tracker/lib/trf-persona.vala
index 2e97827..91e8919 100644
--- a/backends/tracker/lib/trf-persona.vala
+++ b/backends/tracker/lib/trf-persona.vala
@@ -957,9 +957,7 @@ public class Trf.Persona : Folks.Persona,
 
       foreach (var p in this._phone_numbers)
         {
-          var current_tracker_id =
-              p.get_parameter_values ("tracker_id").nth_data (0);
-          if (tracker_id == current_tracker_id)
+          if (p.get_parameter_values ("tracker_id").contains (tracker_id))
             {
               found = true;
               break;
@@ -983,9 +981,7 @@ public class Trf.Persona : Folks.Persona,
 
       foreach (var p in this._phone_numbers)
         {
-          var current_tracker_id = p.get_parameter_values
-              ("tracker_id").nth_data(0);
-          if (tracker_id == current_tracker_id)
+          if (p.get_parameter_values ("tracker_id").contains (tracker_id))
             {
               this._phone_numbers.remove (p);
               found = true;
@@ -1007,9 +1003,7 @@ public class Trf.Persona : Folks.Persona,
 
       foreach (var e in this._email_addresses)
         {
-          var current_tracker_id =
-              e.get_parameter_values ("tracker_id").nth_data (0);
-          if (tracker_id == current_tracker_id)
+          if (e.get_parameter_values ("tracker_id").contains (tracker_id))
             {
               found = true;
               break;
@@ -1033,9 +1027,7 @@ public class Trf.Persona : Folks.Persona,
 
       foreach (var e in this._email_addresses)
         {
-          var current_tracker_id =
-              e.get_parameter_values ("tracker_id").nth_data(0);
-          if (tracker_id == current_tracker_id)
+          if (e.get_parameter_values ("tracker_id").contains (tracker_id))
             {
               this._email_addresses.remove (e);
               found = true;
@@ -1130,8 +1122,7 @@ public class Trf.Persona : Folks.Persona,
 
       foreach (var p in this._urls)
         {
-          var t_id = p.get_parameter_values ("tracker_id").nth_data (0);
-          if (tracker_id == t_id)
+          if (p.get_parameter_values ("tracker_id").contains (tracker_id))
             {
               found = true;
               break;
@@ -1156,9 +1147,7 @@ public class Trf.Persona : Folks.Persona,
 
       foreach (var u in this._urls)
         {
-          var current_tracker_id = u.get_parameter_values
-              ("tracker_id").nth_data(0);
-          if (tracker_id == current_tracker_id)
+          if (u.get_parameter_values ("tracker_id").contains (tracker_id))
             {
               this._urls.remove (u);
               found = true;
diff --git a/folks/field-details.vala b/folks/field-details.vala
index 7cd7464..c3b9afe 100644
--- a/folks/field-details.vala
+++ b/folks/field-details.vala
@@ -19,6 +19,7 @@
  */
 
 using GLib;
+using Gee;
 
 /**
  * Object representing a string value that can have some parameters
@@ -51,9 +52,9 @@ public class Folks.FieldDetails : Object
    * {@link Folks.FieldDetails.value}. The keys are the names of the
    * parameters, while the values are a list of strings.
    *
-   * @since 0.3.5
+   * @since UNRELEASED
    */
-  public HashTable<string, List<string>> parameters { get; set; }
+  public MultiMap<string, string> parameters { get; set; }
 
   /**
    * Create a new FieldDetails.
@@ -66,22 +67,26 @@ public class Folks.FieldDetails : Object
   public FieldDetails (string value)
     {
       this.value = value;
-      this.parameters = new HashTable<string, List<string>> (str_hash,
-          str_equal);
+      this.parameters = new HashMultiMap<string, string> ();
     }
 
   /**
    * Get the values for a parameter
    *
    * @param parameter_name the parameter name
-   * @return a list of values for `parameter_name` or `null` (i.e. an empty
-   * list) if there are no such parameters.
+   * @return a collection of values for `parameter_name` or `null` (i.e. no
+   * collection) if there are no such parameters.
    *
-   * @since 0.3.5
+   * @since UNRELEASED
    */
-  public unowned List<string> get_parameter_values (string parameter_name)
+  public Collection<string>? get_parameter_values (string parameter_name)
     {
-      return this.parameters.lookup (parameter_name);
+      if (this.parameters.contains (parameter_name) == false)
+        {
+          return null;
+        }
+
+      return this.parameters.get (parameter_name);
     }
 
   /**
@@ -97,18 +102,7 @@ public class Folks.FieldDetails : Object
    */
   public void add_parameter (string parameter_name, string parameter_value)
     {
-      unowned List<string> values = this.parameters.lookup (parameter_name);
-      if (values == null)
-        {
-          var new_values = new List<string> ();
-          new_values.append (parameter_value);
-          this.parameters.insert (parameter_name, (owned) new_values);
-        }
-      else
-        {
-          if (values.find_custom (parameter_value, strcmp) == null)
-            values.append (parameter_value);
-        }
+      this.parameters.set (parameter_name, parameter_value);
     }
 
   /**
@@ -124,9 +118,8 @@ public class Folks.FieldDetails : Object
    */
   public void set_parameter (string parameter_name, string parameter_value)
     {
-      var values = new List<string> ();
-      values.append (parameter_value);
-      this.parameters.insert (parameter_name, (owned) values);
+      this.parameters.remove_all (parameter_name);
+      this.parameters.set (parameter_name, parameter_value);
     }
 
   /**
@@ -136,17 +129,17 @@ public class Folks.FieldDetails : Object
    *
    * @param additional the parameters to add
    *
-   * @since 0.3.5
+   * @since UNRELEASED
    */
-  public void extend_parameters (HashTable<string, List<string>> additional)
+  public void extend_parameters (MultiMap<string, string> additional)
     {
-      var iter = HashTableIter<string, List<string>> (additional);
-      unowned string name;
-      unowned List<string> values;
-      while (iter.next (out name, out values))
+      foreach (var name in additional.get_keys ())
         {
-          foreach (unowned string val in values)
-            this.add_parameter (name, val);
+          var values = additional.get (name);
+          foreach (var val in values)
+            {
+              this.add_parameter (name, val);
+            }
         }
     }
 
@@ -159,6 +152,6 @@ public class Folks.FieldDetails : Object
    */
   public void remove_parameter_all (string parameter_name)
     {
-      this.parameters.remove (parameter_name);
+      this.parameters.remove_all (parameter_name);
     }
 }
diff --git a/tests/folks/field-details.vala b/tests/folks/field-details.vala
index 113ed13..819bf98 100644
--- a/tests/folks/field-details.vala
+++ b/tests/folks/field-details.vala
@@ -1,3 +1,4 @@
+using Gee;
 using Folks;
 
 public class FieldDetailsTests : Folks.TestCase
@@ -22,8 +23,7 @@ public class FieldDetailsTests : Folks.TestCase
       string[] values_1 = {"foo", "bar", "baz"};
       string[] values_2 = {"qux", "quxx"};
       FieldDetails details;
-      uint i;
-      unowned GLib.List<string> values;
+      Collection<string> values;
 
       details = new FieldDetails (param_name);
 
@@ -31,19 +31,16 @@ public class FieldDetailsTests : Folks.TestCase
         details.add_parameter (param_name, val);
 
       /* populate with first list of param values */
-      i = 0;
       values = details.get_parameter_values (param_name);
-      assert (values.length () == values_1.length);
-      for (unowned List<string> l = values; l != null; l = l.next, i++)
-        assert (l.data == values_1[i]);
+      assert (values.size == values_1.length);
+      foreach (var val in values_1)
+        assert (values.contains (val));
 
       /* replace the list of param values */
-      i = 0;
       details.set_parameter (param_name, values_2[0]);
       values = details.get_parameter_values (param_name);
-      assert (values.length () == 1);
-      for (unowned List<string> l = values; l != null; l = l.next, i++)
-          assert (l.data == values_2[i]);
+      assert (values.size == 1);
+      assert (values.contains (values_2[0]));
 
       /* clear the list */
       details.remove_parameter_all (param_name);
@@ -58,18 +55,17 @@ public class FieldDetailsTests : Folks.TestCase
       foreach (var val in values_2)
         values_2_list.append (val);
 
-      var param_table = new HashTable<string, unowned List<string>> (str_hash,
-          str_equal);
-      param_table.insert (param_name, values_2_list);
+      var param_table = new HashMultiMap<string, string> ();
+      foreach (var val in values_2_list)
+        param_table.set (param_name, val);
 
       details.extend_parameters (param_table);
       values = details.get_parameter_values (param_name);
-      assert (values.length () == (values_1.length + values_2.length));
-      i = 0;
-      for (; i < values_1.length; i++)
-        assert (values.nth_data (i) == values_1[i]);
-      for (; i < values_2.length; i++)
-        assert (values.nth_data (i) == values_2[i]);
+      assert (values.size == (values_1.length + values_2.length));
+      foreach (var val in values_1)
+        assert (values.contains (val));
+      foreach (var val in values_2)
+          assert (values.contains (val));
     }
 }
 
diff --git a/tests/tracker/set-urls.vala b/tests/tracker/set-urls.vala
index d7e56e7..ed3744d 100644
--- a/tests/tracker/set-urls.vala
+++ b/tests/tracker/set-urls.vala
@@ -134,17 +134,23 @@ public class SetURLsTests : Folks.TestCase
         {
           foreach (unowned FieldDetails p in i.urls)
             {
-              unowned GLib.List<string> type_p =
-                p.get_parameter_values ("type");
-              string type = type_p.nth_data (0);
-
-              if (type == "blog" && p.value == this._urls.get ("blog"))
-                this._urls.unset ("blog");
-              else if (type == "website" &&
+              var type_p = p.get_parameter_values ("type");
+
+              if (type_p.contains ("blog") &&
+                  p.value == this._urls.get ("blog"))
+                {
+                  this._urls.unset ("blog");
+                }
+              else if (type_p.contains ("website") &&
                   p.value == this._urls.get ("website"))
-                this._urls.unset ("website");
-              else if (type == "url" && p.value == this._urls.get ("url"))
-                this._urls.unset ("url");
+                {
+                  this._urls.unset ("website");
+                }
+              else if (type_p.contains ("url") &&
+                  p.value == this._urls.get ("url"))
+                {
+                  this._urls.unset ("url");
+                }
             }
         }