>From e7e2bd8b43d6110723fb36d53c6523fbff38bd3e Mon Sep 17 00:00:00 2001 From: Mathias Hasselmann Date: Thu, 7 Mar 2013 13:25:01 +0100 Subject: [PATCH] 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 | 17 ++++-- tests/libebook/test-ebook-phone-number.c | 54 ++++++++++++++++++-- 4 files changed, 83 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..486c131 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,12 @@ e_book_backend_sqlitedb_new_contacts (EBookBackendSqliteDB *ebsdb, return FALSE; } - if (e_phone_number_is_supported ()) - default_region = e_phone_number_get_default_region (); + if (e_phone_number_is_supported ()) { + 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 +3174,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 +4628,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); } -- 1.7.10.4