[folks] Fix phone normalization

[Xavier Claessens <xclaesse src gnome org>]

commit d118e13f9a73e9ce4a9ff806150e36888f144f17
Author: Xavier Claessens <xavier claessens collabora co uk>
Date:   Mon Oct 21 10:29:02 2013 -0400

    Fix phone normalization
    
    This make the code actually match the algorithm described on
    http://blog.barisione.org/2010-06/handling-phone-numbers/
    
    It also use a StringBuilder instead of doing tones of string copy.

 NEWS                                 |    3 +
 folks/phone-details.vala             |   70 ++++++++++++++++++++++++----------
 tests/folks/phone-field-details.vala |    3 +
 3 files changed, 56 insertions(+), 20 deletions(-)
---
diff --git a/NEWS b/NEWS
index 00fdcc9..2ad7b4f 100644
--- a/NEWS
+++ b/NEWS
@@ -20,6 +20,9 @@ Bugs fixed:
 • Bug 710178 — Skip whitespace between oFono vCards
 • Bug 682698 — const string[] not NULL terminated; crashes in GBoxed property
   getter (workaround)
+• Avoid useless string copies when normalizing phone numbers, and make the
+  implementation respect the algorithm described in
+  http://blog.barisione.org/2010-06/handling-phone-numbers
 
 API changes:
 
diff --git a/folks/phone-details.vala b/folks/phone-details.vala
index f906990..141ac07 100644
--- a/folks/phone-details.vala
+++ b/folks/phone-details.vala
@@ -35,11 +35,11 @@ using Gee;
  */
 public class Folks.PhoneFieldDetails : AbstractFieldDetails<string>
 {
-  private const string[] _extension_chars = { "p", "P", "w", "W", "x", "X" };
-  private const string[] _common_delimiters = { ",", ".", "(", ")", "-", " ",
-      "\t", "/" };
-  private const string[] _valid_digits = { "#", "*", "0", "1", "2", "3", "4",
-      "5", "6", "7", "8", "9" };
+  private const char[] _extension_chars = { 'p', 'P', 'w', 'W', 'x', 'X' };
+  private const char[] _common_delimiters = { ',', '.', '(', ')', '-', ' ',
+      '\t', '/' };
+  private const char[] _valid_digits = { '#', '*', '0', '1', '2', '3', '4',
+      '5', '6', '7', '8', '9' };
 
   private string _id;
   /**
@@ -108,7 +108,7 @@ public class Folks.PhoneFieldDetails : AbstractFieldDetails<string>
           debug ("[PhoneDetails.equal] Comparing %s with %s",
               n1_reduced, n2_reduced);
 
-          return  n1_reduced == n2_reduced;
+          return n1_reduced == n2_reduced;
         }
 
       return n1 == n2;
@@ -139,30 +139,53 @@ public class Folks.PhoneFieldDetails : AbstractFieldDetails<string>
    */
   public string get_normalised ()
     {
-      string normalised_number = "";
+      var builder = new StringBuilder ();
 
-      for (int i = 0; i < this.value.length; i++)
+      /* Based on http://blog.barisione.org/2010-06/handling-phone-numbers/ */
+      for (uint i = 0; i < this.value.length; i++)
         {
-          var digit = this.value.slice (i, i + 1);
+          var digit = this.value[i];
 
-          if (digit in PhoneFieldDetails._extension_chars ||
-              digit in PhoneFieldDetails._valid_digits ||
-              (i == 0 && digit == "+"))
+          if (digit in PhoneFieldDetails._extension_chars)
+            {
+              /* Keep the extension characters P, W and X, but be sure they are
+               * upper case. */
+               digit = digit.toupper ();
+            }
+          else if (digit == '+')
             {
-              /* lets keep valid digits */
-              normalised_number += digit;
+              /* "+" is valid only at the beginning of phone numbers or after
+               * the number suppression prefix. */
+              if (builder.str != "" &&
+                  builder.str != "*31#" &&
+                  builder.str != "#31#")
+                {
+                  /* Skip this "+". */
+                  debug ("[PhoneDetails.get_normalised] Wrong '+' in %s",
+                      this.value);
+                  continue;
+                }
             }
           else if (digit in PhoneFieldDetails._common_delimiters)
             {
+              /* Skip this delimiter. */
               continue;
             }
+          else if (digit in PhoneFieldDetails._valid_digits)
+            {
+              /* Ok, let's keep it. */
+            }
           else
             {
-              debug ("[PhoneDetails.get_normalised] unknown digit: %s", digit);
+              /* What is this? It doesn't seem valid but we just keep it */
+              debug ("[PhoneDetails.get_normalised] Unknown character '%c' in '%s'",
+                  digit, this.value);
             }
+
+         builder.append_c (digit);
        }
 
-      return normalised_number.up ();
+      return builder.str;
     }
 
   /**
@@ -176,15 +199,22 @@ public class Folks.PhoneFieldDetails : AbstractFieldDetails<string>
    */
   internal static string _drop_extension (string number)
     {
-      for (var i = 0; i < PhoneFieldDetails._extension_chars.length; i++)
+      /* Based on http://blog.barisione.org/2010-06/handling-phone-numbers/ */
+      var builder = new StringBuilder ();
+
+      for (uint i = 0; i < number.length; i++)
         {
-          if (number.index_of (PhoneFieldDetails._extension_chars[i]) >= 0)
+          var digit = number[i];
+          if (digit in PhoneFieldDetails._extension_chars)
             {
-              return number.split (PhoneFieldDetails._extension_chars[i])[0];
+              /* Extension character, drop this character and the rest of the
+               * string. */
+              break;
             }
+          builder.append_c (digit);
         }
 
-      return number;
+      return builder.str;
     }
 }
 
diff --git a/tests/folks/phone-field-details.vala b/tests/folks/phone-field-details.vala
index 02f07aa..afcb2b7 100644
--- a/tests/folks/phone-field-details.vala
+++ b/tests/folks/phone-field-details.vala
@@ -43,6 +43,9 @@ public class PhoneFieldDetailsTests : Folks.TestCase
         { "1-800-123-4567", "18001234567" },
         { "+1-800-123-4567", "+18001234567" },
         { "+1-800-123-4567P123", "+18001234567P123" },
+        { "+1-800-123-4567p123", "+18001234567P123" },
+        { "#31#+123", "#31#+123" },
+        { "*31#+123", "*31#+123" },
       };
 
       foreach (var s in normalisation_pairs)