[evolution-data-server/openismus-work-error-args: 2/2] libebook: Report missing phone number support by GError

[Mathias Hasselmann <hasselmm src gnome org>]

commit b6b7263803ed39d9f6a2835da6b100a85f8e0f38
Author: Mathias Hasselmann <mathias openismus com>
Date:   Thu Mar 7 13:25:01 2013 +0100

    libebook: Report missing phone number support by GError
    
    Add GError arguments to e_phone_number_get_default_region() and
    e_phone_number_get_country_code_for_region() to properly report
    missing phone number: Warnings cannot be handled programatically
    and make GTest based unit tests failed.

 addressbook/libebook-contacts/e-phone-number.c     |   26 +++++++---
 addressbook/libebook-contacts/e-phone-number.h     |    5 +-
 .../libedata-book/e-book-backend-sqlitedb.c        |   15 ++++--
 tests/libebook/test-ebook-phone-number.c           |   54 ++++++++++++++++++--
 4 files changed, 81 insertions(+), 19 deletions(-)
---
diff --git a/addressbook/libebook-contacts/e-phone-number.c b/addressbook/libebook-contacts/e-phone-number.c
index b8f7c2b..55ef37c 100644
--- a/addressbook/libebook-contacts/e-phone-number.c
+++ b/addressbook/libebook-contacts/e-phone-number.c
@@ -96,6 +96,7 @@ e_phone_number_is_supported (void)
  * e_phone_number_get_country_code_for_region:
  * @region_code: (allow-none): a two-letter country code, a locale name, or
  * %NULL
+ * @error: (out): a #GError to set an error, if any
  *
  * Retrieves the preferred country calling code for @region_code,
  * e.g. 358 for "fi" or 1 for "en_US UTF-8".
@@ -109,7 +110,8 @@ e_phone_number_is_supported (void)
  * Since: 3.8
  */
 gint
-e_phone_number_get_country_code_for_region (const gchar *region_code)
+e_phone_number_get_country_code_for_region (const gchar *region_code,
+                                            GError **error)
 {
 #ifdef ENABLE_PHONENUMBER
 
@@ -117,7 +119,7 @@ e_phone_number_get_country_code_for_region (const gchar *region_code)
 
 #else /* ENABLE_PHONENUMBER */
 
-       g_warning ("%s: The library was built without phone number support.", G_STRFUNC);
+       _e_phone_number_set_error (error, E_PHONE_NUMBER_ERROR_NOT_IMPLEMENTED);
        return 0;
 
 #endif /* ENABLE_PHONENUMBER */
@@ -125,6 +127,7 @@ e_phone_number_get_country_code_for_region (const gchar *region_code)
 
 /**
  * e_phone_number_get_default_region:
+ * @error: (out): a #GError to set an error, if any
  *
  * Retrieves the current two-letter country code that's used by default for
  * parsing phone numbers in e_phone_number_from_string(). It can be useful
@@ -141,7 +144,7 @@ e_phone_number_get_country_code_for_region (const gchar *region_code)
  * Since: 3.8
  */
 gchar *
-e_phone_number_get_default_region (void)
+e_phone_number_get_default_region (GError **error)
 {
 #ifdef ENABLE_PHONENUMBER
 
@@ -149,7 +152,7 @@ e_phone_number_get_default_region (void)
 
 #else /* ENABLE_PHONENUMBER */
 
-       g_warning ("%s: The library was built without phone number support.", G_STRFUNC);
+       _e_phone_number_set_error (error, E_PHONE_NUMBER_ERROR_NOT_IMPLEMENTED);
        return NULL;
 
 #endif /* ENABLE_PHONENUMBER */
@@ -216,6 +219,9 @@ e_phone_number_to_string (const EPhoneNumber *phone_number,
 
 #else /* ENABLE_PHONENUMBER */
 
+       /* The EPhoneNumber instance must be invalid. We'd also bail out with
+        * a warning if phone numbers are supported. Any code triggering this
+        * is broken and should be fixed. */
        g_warning ("%s: The library was built without phone number support.", G_STRFUNC);
        return NULL;
 
@@ -245,6 +251,9 @@ e_phone_number_get_country_code (const EPhoneNumber *phone_number,
 
 #else /* ENABLE_PHONENUMBER */
 
+       /* The EPhoneNumber instance must be invalid. We'd also bail out with
+        * a warning if phone numbers are supported. Any code triggering this
+        * is broken and should be fixed. */
        g_warning ("%s: The library was built without phone number support.", G_STRFUNC);
        return 0;
 
@@ -272,6 +281,9 @@ e_phone_number_get_national_number (const EPhoneNumber *phone_number)
 
 #else /* ENABLE_PHONENUMBER */
 
+       /* The EPhoneNumber instance must be invalid. We'd also bail out with
+        * a warning if phone numbers are supported. Any code triggering this
+        * is broken and should be fixed. */
        g_warning ("%s: The library was built without phone number support.", G_STRFUNC);
        return NULL;
 
@@ -299,9 +311,9 @@ e_phone_number_compare (const EPhoneNumber *first_number,
 
 #else /* ENABLE_PHONENUMBER */
 
-       /* NOTE: This calls for a dedicated return value, but I sense broken
-        * client code that only checks for E_PHONE_NUMBER_MATCH_NONE and then
-        * treats the "not-implemented" return value as a match */
+       /* The EPhoneNumber instance must be invalid. We'd also bail out with
+        * a warning if phone numbers are supported. Any code triggering this
+        * is broken and should be fixed. */
        g_warning ("%s: The library was built without phone number support.", G_STRFUNC);
        return E_PHONE_NUMBER_MATCH_NONE;
 
diff --git a/addressbook/libebook-contacts/e-phone-number.h b/addressbook/libebook-contacts/e-phone-number.h
index 17d2d1b..1870f66 100644
--- a/addressbook/libebook-contacts/e-phone-number.h
+++ b/addressbook/libebook-contacts/e-phone-number.h
@@ -209,9 +209,10 @@ GQuark                     e_phone_number_error_quark      (void);
 
 gboolean               e_phone_number_is_supported     (void) G_GNUC_CONST;
 gint                   e_phone_number_get_country_code_for_region
-                                                       (const gchar *region_code);
+                                                       (const gchar *region_code,
+                                                        GError **error);
 gchar *                        e_phone_number_get_default_region
-                                                       (void);
+                                                       (GError **error);
 
 EPhoneNumber *         e_phone_number_from_string      (const gchar *phone_number,
                                                         const gchar *region_code,
diff --git a/addressbook/libedata-book/e-book-backend-sqlitedb.c 
b/addressbook/libedata-book/e-book-backend-sqlitedb.c
index 33f4a46..76e8dd9 100644
--- a/addressbook/libedata-book/e-book-backend-sqlitedb.c
+++ b/addressbook/libedata-book/e-book-backend-sqlitedb.c
@@ -1194,7 +1194,7 @@ create_collation (gpointer data,
                        db, name, SQLITE_UTF8, GINT_TO_POINTER (country_code),
                        ixphone_compare_for_country);
        } else if (strcmp (name, "ixphone_national") == 0) {
-               country_code = e_phone_number_get_country_code_for_region (NULL);
+               country_code = e_phone_number_get_country_code_for_region (NULL, NULL);
 
                ret = sqlite3_create_collation_v2 (
                        db, name, SQLITE_UTF8,
@@ -2042,8 +2042,10 @@ e_book_backend_sqlitedb_new_contacts (EBookBackendSqliteDB *ebsdb,
                return FALSE;
        }
 
-       if (e_phone_number_is_supported ())
-               default_region = e_phone_number_get_default_region ();
+       default_region = e_phone_number_get_default_region (error);
+
+       if (default_region == NULL)
+               success = FALSE;
 
        for (l = contacts; success && l != NULL; l = g_slist_next (l)) {
                EContact *contact = (EContact *) l->data;
@@ -3170,7 +3172,7 @@ field_name_and_query_term (EBookBackendSqliteDB *ebsdb,
                         *  o Normalize the string
                         *  o Check the E.164 column instead
                         */
-                       const gint country_code = e_phone_number_get_country_code_for_region (region);
+                       const gint country_code = e_phone_number_get_country_code_for_region (region, NULL);
 
                        if (ebsdb->priv->summary_fields[summary_index].type == E_TYPE_CONTACT_ATTR_LIST) {
                                field_name = g_strdup ("multi.value_phone");
@@ -4624,7 +4626,10 @@ upgrade_contacts_table (EBookBackendSqliteDB *ebsdb,
 
        if (e_phone_number_is_supported ()) {
                g_message ("The phone number indexes' format has changed. Rebuilding them.");
-               default_region = e_phone_number_get_default_region ();
+               default_region = e_phone_number_get_default_region (error);
+
+               if (default_region == NULL)
+                       success = FALSE;
        }
 
        for (l = vcard_data; success && l; l = l->next) {
diff --git a/tests/libebook/test-ebook-phone-number.c b/tests/libebook/test-ebook-phone-number.c
index 63b8c39..5939c7d 100644
--- a/tests/libebook/test-ebook-phone-number.c
+++ b/tests/libebook/test-ebook-phone-number.c
@@ -339,22 +339,66 @@ test_supported (void)
 static void
 test_country_code_for_region (void)
 {
+       GError *error = NULL;
+       gint code;
+
        g_assert_cmpstr (setlocale (LC_ADDRESS, NULL), ==, "en_US.UTF-8");
-       g_assert_cmpint (e_phone_number_get_country_code_for_region ("CH"), ==, 41);
-       g_assert_cmpint (e_phone_number_get_country_code_for_region (NULL), ==, 1);
-       g_assert_cmpint (e_phone_number_get_country_code_for_region ("C"), ==, 0);
-       g_assert_cmpint (e_phone_number_get_country_code_for_region (""), ==, 1);
+
+#ifdef ENABLE_PHONENUMBER
+
+       code = e_phone_number_get_country_code_for_region ("CH", &error);
+       g_assert_cmpstr (error ? error->message : NULL, ==, NULL);
+       g_assert_cmpint (code, ==, 41);
+
+       code = e_phone_number_get_country_code_for_region (NULL, &error);
+       g_assert_cmpstr (error ? error->message : NULL, ==, NULL);
+       g_assert_cmpint (code, ==, 1);
+
+       code = e_phone_number_get_country_code_for_region ("C", &error);
+       g_assert_cmpstr (error ? error->message : NULL, ==, NULL);
+       g_assert_cmpint (code, ==, 0);
+
+       code = e_phone_number_get_country_code_for_region ("", &error);
+       g_assert_cmpstr (error ? error->message : NULL, ==, NULL);
+       g_assert_cmpint (code, ==, 1);
+
+#else /* ENABLE_PHONENUMBER */
+
+       code = e_phone_number_get_country_code_for_region ("CH", &error);
+
+       g_assert (error != NULL);
+       g_assert (error->domain == E_PHONE_NUMBER_ERROR);
+       g_assert (error->code == E_PHONE_NUMBER_ERROR_NOT_IMPLEMENTED);
+       g_assert (error->message != NULL);
+       g_assert_cmpint (code, ==, 0);
+
+#endif /* ENABLE_PHONENUMBER */
 }
 
 static void
 test_default_region (void)
 {
+       GError *error = NULL;
        gchar *country;
 
        g_assert_cmpstr (setlocale (LC_ADDRESS, NULL), ==, "en_US.UTF-8");
+       country = e_phone_number_get_default_region (&error);
+
+#ifdef ENABLE_PHONENUMBER
 
-       country = e_phone_number_get_default_region ();
+       g_assert_cmpstr (error ? error->message : NULL, ==, NULL);
        g_assert_cmpstr (country, ==, "US");
+
+#else /* ENABLE_PHONENUMBER */
+
+       g_assert (error != NULL);
+       g_assert (error->domain == E_PHONE_NUMBER_ERROR);
+       g_assert (error->code == E_PHONE_NUMBER_ERROR_NOT_IMPLEMENTED);
+       g_assert (error->message != NULL);
+       g_assert_cmpstr (country, ==, NULL);
+
+#endif /* ENABLE_PHONENUMBER */
+
        g_free (country);
 }