libebook: Report missing phone number support by GErroropenismus-work-error-args

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.

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 b8f7c2b76..55ef37cd2 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 17d2d1b41..1870f660d 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 33f4a46e4..486c1313e 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 63b8c395d..5939c7dfa 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);
 }