libnm-core: clean up NMConnectionError

Rename NM_CONNECTION_ERROR_UNKNOWN to NM_CONNECTION_ERROR_FAILED,
following GError best practices.

Replace NM_CONNECTION_ERROR_CONNECTION_SETTING_NOT_FOUND ("no
NMSettingConnection") with a more generic
NM_CONNECTION_ERROR_MISSING_SETTING. Use that new code in a few places
that had previously been using NM_CONNECTION_ERROR_SETTING_NOT_FOUND,
which was supposed to mean "the setting that you asked about doesn't
exist", not "the connection is invalid because it's missing a required
setting".

Clarify that NM_CONNECTION_ERROR_INVALID_SETTING can be used for any
"invalid or inappropriate NMSetting", not just a "conflicting" one.
(But fix a case in nm_connection_update_secrets() that was returning
INVALID_SETTING when it should have been return-if-failing instead.)

For both MISSING_SETTING and INVALID_SETTING, always prefix the error
message with "setting-name: ", just like we do with the various
NMSetting MISSING_PROPERTY and INVALID_PROPERTY errors. And make sure
that the error message is marked for localization.

Drop NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID, which is pretty
pointless; it was only used in the case where connection.type was the
name of a valid setting type that is not a base setting type. Instead,
just return NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY for
connection.type in this case (which is what the code already did when
connection.type was completely unrecognized).

9 files changed, 69 insertions, 86 deletions

diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c
index 1e0cdbf934..18a21c98c8 100644
--- a/libnm-core/nm-connection.c
+++ b/libnm-core/nm-connection.c
@@ -312,10 +312,11 @@ nm_connection_replace_settings (NMConnection *connection,
 
 		type = nm_setting_lookup_type (setting_name);
 		if (type == G_TYPE_INVALID) {
-			g_set_error (error,
-			             NM_CONNECTION_ERROR,
-			             NM_CONNECTION_ERROR_INVALID_SETTING,
-			             "unknown setting name '%s'", setting_name);
+			g_set_error_literal (error,
+			                     NM_CONNECTION_ERROR,
+			                     NM_CONNECTION_ERROR_INVALID_SETTING,
+			                     _("unknown setting name"));
+			g_prefix_error (error, "%s: ", setting_name);
 			g_variant_unref (setting_dict);
 			g_slist_free_full (settings, g_object_unref);
 			return FALSE;
@@ -749,8 +750,9 @@ _nm_connection_verify (NMConnection *connection, GError **error)
 	if (!s_con) {
 		g_set_error_literal (error,
 		                     NM_CONNECTION_ERROR,
-		                     NM_CONNECTION_ERROR_CONNECTION_SETTING_NOT_FOUND,
-		                     "connection setting not found");
+		                     NM_CONNECTION_ERROR_MISSING_SETTING,
+		                     _("setting not found"));
+		g_prefix_error (error, "%s: ", NM_SETTING_CONNECTION_SETTING_NAME);
 		goto EXIT;
 	}
 
@@ -811,21 +813,23 @@ _nm_connection_verify (NMConnection *connection, GError **error)
 		if ((normalizable_error_type == NM_SETTING_VERIFY_SUCCESS ||
 		    (normalizable_error_type == NM_SETTING_VERIFY_NORMALIZABLE))  && (s_ip4 || s_ip6)) {
 			g_clear_error (&normalizable_error);
-			g_set_error (&normalizable_error,
-			             NM_CONNECTION_ERROR,
-			             NM_CONNECTION_ERROR_INVALID_SETTING,
-			             "slave connection cannot have an IP%c setting",
-			             s_ip4 ? '4' : '6');
+			g_set_error_literal (&normalizable_error,
+			                     NM_CONNECTION_ERROR,
+			                     NM_CONNECTION_ERROR_INVALID_SETTING,
+			                     _("setting not allowed in slave connection"));
+			g_prefix_error (&normalizable_error, "%s: ",
+			                s_ip4 ? NM_SETTING_IP4_CONFIG_SETTING_NAME : NM_SETTING_IP6_CONFIG_SETTING_NAME);
 			/* having a slave with IP config *was* and is a verify() error. */
 			normalizable_error_type = NM_SETTING_VERIFY_NORMALIZABLE_ERROR;
 		}
 	} else {
 		if (normalizable_error_type == NM_SETTING_VERIFY_SUCCESS && (!s_ip4 || !s_ip6)) {
-			g_set_error (&normalizable_error,
-			             NM_CONNECTION_ERROR,
-			             NM_CONNECTION_ERROR_SETTING_NOT_FOUND,
-			             "connection needs an IP%c setting",
-			             !s_ip4 ? '4' : '6');
+			g_set_error_literal (&normalizable_error,
+			                     NM_CONNECTION_ERROR,
+			                     NM_CONNECTION_ERROR_MISSING_SETTING,
+			                     _("setting is required for non-slave connections"));
+			g_prefix_error (&normalizable_error, "%s: ",
+			                !s_ip4 ? NM_SETTING_IP4_CONFIG_SETTING_NAME : NM_SETTING_IP6_CONFIG_SETTING_NAME);
 			/* having a master without IP config was not a verify() error, accept
 			 * it for backward compatibility. */
 			normalizable_error_type = NM_SETTING_VERIFY_NORMALIZABLE;
@@ -888,7 +892,7 @@ nm_connection_normalize (NMConnection *connection,
 		if (success == NM_SETTING_VERIFY_ERROR && error && !*error) {
 			g_set_error_literal (error,
 			                     NM_CONNECTION_ERROR,
-			                     NM_CONNECTION_ERROR_UNKNOWN,
+			                     NM_CONNECTION_ERROR_FAILED,
 			                     _("Unexpected failure to verify the connection"));
 			g_return_val_if_reached (FALSE);
 		}
@@ -919,7 +923,7 @@ nm_connection_normalize (NMConnection *connection,
 		if (error && !*error) {
 			g_set_error_literal (error,
 			                     NM_CONNECTION_ERROR,
-			                     NM_CONNECTION_ERROR_UNKNOWN,
+			                     NM_CONNECTION_ERROR_FAILED,
 			                     _("Unexpected failure to normalize the connection"));
 		}
 		g_return_val_if_reached (FALSE);
@@ -969,12 +973,13 @@ nm_connection_update_secrets (NMConnection *connection,
 	if (error)
 		g_return_val_if_fail (*error == NULL, FALSE);
 
+	full_connection = g_variant_is_of_type (secrets, NM_VARIANT_TYPE_CONNECTION);
+	g_return_val_if_fail (setting_name != NULL || full_connection, FALSE);
+
 	/* Empty @secrets means success */
 	if (g_variant_n_children (secrets) == 0)
 		return TRUE;
 
-	full_connection = g_variant_is_of_type (secrets, NM_VARIANT_TYPE_CONNECTION);
-
 	if (setting_name) {
 		/* Update just one setting's secrets */
 		setting = nm_connection_get_setting_by_name (connection, setting_name);
@@ -1009,14 +1014,6 @@ nm_connection_update_secrets (NMConnection *connection,
 		if (success_detail == NM_SETTING_UPDATE_SECRET_SUCCESS_MODIFIED)
 			updated = TRUE;
 	} else {
-		if (!full_connection) {
-			g_set_error_literal (error,
-			                     NM_CONNECTION_ERROR,
-			                     NM_CONNECTION_ERROR_INVALID_SETTING,
-			                     _("Update secrets expects a full connection, instead only a setting is provided."));
-			return FALSE;
-		}
-
 		/* check first, whether all the settings exist... */
 		g_variant_iter_init (&iter, secrets);
 		while (g_variant_iter_next (&iter, "{&s@a{sv}}", &key, NULL)) {
diff --git a/libnm-core/nm-connection.h b/libnm-core/nm-connection.h
index de1cd528a4..93529af2ce 100644
--- a/libnm-core/nm-connection.h
+++ b/libnm-core/nm-connection.h
@@ -72,28 +72,24 @@ G_BEGIN_DECLS
 
 /**
  * NMConnectionError:
- * @NM_CONNECTION_ERROR_UNKNOWN: unknown or unclassified error
- * @NM_CONNECTION_ERROR_CONNECTION_SETTING_NOT_FOUND: the #NMConnection object
- *   did not contain the required #NMSettingConnection object, which must be
- *   present for all connections
- * @NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID: the 'type' property of the
- *   'connection' setting did not point to a valid connection base type; ie
- *   it was not a hardware-related setting like #NMSettingWired or
- *   #NMSettingWireless.
+ * @NM_CONNECTION_ERROR_FAILED: unknown or unclassified error
  * @NM_CONNECTION_ERROR_SETTING_NOT_FOUND: the #NMConnection object
  *   did not contain the specified #NMSetting object
- *@NM_CONNECTION_ERROR_INVALID_SETTING: the #NMConnection object contains
- *   a conflicting setting object
+ * @NM_CONNECTION_ERROR_MISSING_SETTING: the #NMConnection object is missing an
+ *   #NMSetting which is required for its configuration. The error message will
+ *   always be prefixed with "<setting-name>: ", where "<setting-name>" is the
+ *   name of the setting that is missing.
+ * @NM_CONNECTION_ERROR_INVALID_SETTING: the #NMConnection object contains an
+ *   invalid or inappropriate #NMSetting. The error message will always be
+ *   prefixed with "<setting-name>: ", where "<setting-name>" is the name of the
+ *   setting that is invalid.
  *
  * Describes errors that may result from operations involving a #NMConnection.
- *
- **/
-typedef enum
-{
-	NM_CONNECTION_ERROR_UNKNOWN = 0,                  /*< nick=UnknownError >*/
-	NM_CONNECTION_ERROR_CONNECTION_SETTING_NOT_FOUND, /*< nick=ConnectionSettingNotFound >*/
-	NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID,      /*< nick=ConnectionTypeInvalid >*/
+ */
+typedef enum {
+	NM_CONNECTION_ERROR_FAILED = 0,                   /*< nick=Failed >*/
 	NM_CONNECTION_ERROR_SETTING_NOT_FOUND,            /*< nick=SettingNotFound >*/
+	NM_CONNECTION_ERROR_MISSING_SETTING,              /*< nick=MissingSetting >*/
 	NM_CONNECTION_ERROR_INVALID_SETTING,              /*< nick=InvalidSetting >*/
 } NMConnectionError;
 
diff --git a/libnm-core/nm-setting-bridge-port.c b/libnm-core/nm-setting-bridge-port.c
index ffd21638c8..7651adc233 100644
--- a/libnm-core/nm-setting-bridge-port.c
+++ b/libnm-core/nm-setting-bridge-port.c
@@ -162,7 +162,7 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 
 		s_con = NM_SETTING_CONNECTION (_nm_setting_find_in_list_required (all_settings,
 		                                                                  NM_SETTING_CONNECTION_SETTING_NAME,
-		                                                                  error, NULL, NULL));
+		                                                                  error));
 		if (!s_con)
 			 return FALSE;
 
diff --git a/libnm-core/nm-setting-connection.c b/libnm-core/nm-setting-connection.c
index 890f77d1cc..c4edd8e023 100644
--- a/libnm-core/nm-setting-connection.c
+++ b/libnm-core/nm-setting-connection.c
@@ -840,7 +840,7 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 		}
 
 		base_type = nm_setting_lookup_type (priv->type);
-		if (base_type == G_TYPE_INVALID) {
+		if (base_type == G_TYPE_INVALID || !_nm_setting_type_is_base_type (base_type)) {
 			g_set_error (error,
 			             NM_SETTING_CONNECTION_ERROR,
 			             NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY,
@@ -850,16 +850,6 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 			return FALSE;
 		}
 
-		if (!_nm_setting_type_is_base_type (base_type)) {
-			g_set_error (error,
-			             NM_CONNECTION_ERROR,
-			             NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID,
-			             _("connection type '%s' is not a valid base type"),
-			             priv->type);
-			g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_TYPE);
-			return FALSE;
-		}
-
 		/* Make sure the corresponding 'type' item is present */
 		if (   all_settings
 		    && !nm_setting_find_in_list (all_settings, priv->type)) {
diff --git a/libnm-core/nm-setting-private.h b/libnm-core/nm-setting-private.h
index e6e54983ed..7940a161e2 100644
--- a/libnm-core/nm-setting-private.h
+++ b/libnm-core/nm-setting-private.h
@@ -95,9 +95,7 @@ NMSetting *nm_setting_find_in_list (GSList *settings_list, const char *setting_n
 
 NMSetting * _nm_setting_find_in_list_required (GSList *all_settings,
                                                const char *setting_name,
-                                               GError **error,
-                                               const char *error_prefix_setting_name,
-                                               const char *error_prefix_property_name);
+                                               GError **error);
 
 NMSettingVerifyResult _nm_setting_verify_required_virtual_interface_name (GSList *all_settings,
                                                                           GError **error);
diff --git a/libnm-core/nm-setting-team-port.c b/libnm-core/nm-setting-team-port.c
index 1e04d68c69..ae50094c51 100644
--- a/libnm-core/nm-setting-team-port.c
+++ b/libnm-core/nm-setting-team-port.c
@@ -105,7 +105,7 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 
 		s_con = NM_SETTING_CONNECTION (_nm_setting_find_in_list_required (all_settings,
 		                                                                  NM_SETTING_CONNECTION_SETTING_NAME,
-		                                                                  error, NULL, NULL));
+		                                                                  error));
 		if (!s_con)
 			return FALSE;
 
diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c
index 8b2ebc925e..357ff12407 100644
--- a/libnm-core/nm-setting.c
+++ b/libnm-core/nm-setting.c
@@ -1743,28 +1743,21 @@ nm_setting_to_string (NMSetting *setting)
 NMSetting *
 _nm_setting_find_in_list_required (GSList *all_settings,
                                    const char *setting_name,
-                                   GError **error,
-                                   const char *error_prefix_setting_name,
-                                   const char *error_prefix_property_name)
+                                   GError **error)
 {
 	NMSetting *setting;
 
 	g_return_val_if_fail (!error || !*error, NULL);
 	g_return_val_if_fail (all_settings, NULL);
 	g_return_val_if_fail (setting_name, NULL);
-	g_return_val_if_fail (!error_prefix_setting_name == !error_prefix_property_name, NULL);
 
 	setting = nm_setting_find_in_list (all_settings, setting_name);
 	if (!setting) {
-		g_set_error (error,
-		             NM_CONNECTION_ERROR,
-		             !strcmp (setting_name, NM_SETTING_CONNECTION_SETTING_NAME)
-		                 ? NM_CONNECTION_ERROR_CONNECTION_SETTING_NOT_FOUND
-		                 : NM_CONNECTION_ERROR_SETTING_NOT_FOUND,
-		             _("Missing '%s' setting"),
-		             setting_name);
-		if (error_prefix_setting_name)
-			g_prefix_error (error, "%s.%s: ", error_prefix_setting_name, error_prefix_property_name);
+		g_set_error_literal (error,
+		                     NM_CONNECTION_ERROR,
+		                     NM_CONNECTION_ERROR_MISSING_SETTING,
+		                     _("missing setting"));
+		g_prefix_error (error, "%s: ", setting_name);
 	}
 	return setting;
 }
diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c
index 63811102ca..c46fc3207f 100644
--- a/libnm-core/tests/test-general.c
+++ b/libnm-core/tests/test-general.c
@@ -1965,7 +1965,8 @@ test_connection_bad_base_types (void)
 	nm_connection_add_setting (connection, setting);
 
 	success = nm_connection_verify (connection, &error);
-	g_assert_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID);
+	g_assert_error (error, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY);
+	g_assert (g_str_has_prefix (error->message, "connection.type: "));
 	g_assert (success == FALSE);
 	g_object_unref (connection);
 	g_clear_error (&error);
@@ -1979,7 +1980,8 @@ test_connection_bad_base_types (void)
 	nm_connection_add_setting (connection, setting);
 
 	success = nm_connection_verify (connection, &error);
-	g_assert_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID);
+	g_assert_error (error, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY);
+	g_assert (g_str_has_prefix (error->message, "connection.type: "));
 	g_assert (success == FALSE);
 	g_object_unref (connection);
 	g_clear_error (&error);
@@ -1993,7 +1995,8 @@ test_connection_bad_base_types (void)
 	nm_connection_add_setting (connection, setting);
 
 	success = nm_connection_verify (connection, &error);
-	g_assert_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID);
+	g_assert_error (error, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY);
+	g_assert (g_str_has_prefix (error->message, "connection.type: "));
 	g_assert (success == FALSE);
 	g_object_unref (connection);
 	g_clear_error (&error);
@@ -2005,7 +2008,8 @@ test_connection_bad_base_types (void)
 	nm_connection_add_setting (connection, setting);
 
 	success = nm_connection_verify (connection, &error);
-	g_assert_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID);
+	g_assert_error (error, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY);
+	g_assert (g_str_has_prefix (error->message, "connection.type: "));
 	g_assert (success == FALSE);
 	g_object_unref (connection);
 	g_clear_error (&error);
@@ -2017,7 +2021,8 @@ test_connection_bad_base_types (void)
 	nm_connection_add_setting (connection, setting);
 
 	success = nm_connection_verify (connection, &error);
-	g_assert_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID);
+	g_assert_error (error, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY);
+	g_assert (g_str_has_prefix (error->message, "connection.type: "));
 	g_assert (success == FALSE);
 	g_object_unref (connection);
 	g_clear_error (&error);
diff --git a/libnm-core/tests/test-secrets.c b/libnm-core/tests/test-secrets.c
index d728223b39..eec22524fd 100644
--- a/libnm-core/tests/test-secrets.c
+++ b/libnm-core/tests/test-secrets.c
@@ -607,10 +607,10 @@ test_update_secrets_whole_connection_empty_hash (void)
 	GError *error = NULL;
 	gboolean success;
 
-	/* Test that updating secrets with an empty hash returns success */
+	/* Test that updating secrets with an empty connection hash returns success */
 
 	connection = wifi_connection_new ();
-	secrets = g_variant_new_array (G_VARIANT_TYPE ("{sv}"), NULL, 0);
+	secrets = g_variant_new_array (G_VARIANT_TYPE ("{sa{sv}}"), NULL, 0);
 	success = nm_connection_update_secrets (connection, NULL, secrets, &error);
 	g_assert_no_error (error);
 	g_assert (success == TRUE);
@@ -715,25 +715,29 @@ test_update_secrets_null_setting_name_with_setting_hash (void)
 
 	secrets = build_wep_secrets (wepkey);
 
+	g_test_expect_message ("libnm", G_LOG_LEVEL_CRITICAL,
+	                       "*nm_connection_update_secrets*setting_name != NULL || full_connection*");
 	success = nm_connection_update_secrets (connection, NULL, secrets, &error);
-	g_assert_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_SETTING);
+	g_test_assert_expected_messages ();
+	g_assert_no_error (error);
 	g_assert (!success);
 
 	g_variant_unref (secrets);
 	g_object_unref (connection);
 }
 
-int main (int argc, char **argv)
+NMTST_DEFINE ();
+
+int
+main (int argc, char **argv)
 {
-	GError *error = NULL;
 	char *base;
 
 #if !GLIB_CHECK_VERSION (2, 35, 0)
 	g_type_init ();
 #endif
 
-	if (!nm_utils_init (&error))
-		FAIL ("nm-utils-init", "failed to initialize libnm: %s", error->message);
+	nmtst_init (&argc, &argv, TRUE);
 
 	/* The tests */
 	test_need_tls_secrets_path ();