all: change handling of connection.type for bluetooth NAP and in general

Branch f9b1bc16e9e691ab89caf883f33d94be72364671 added bluetooth NAP
support. A NAP connection is of connection.type "bluetooth", but it
also has a "bridge" setting. Also, it is primarily handled by NMDeviceBridge
and NMBridgeDeviceFactory (with help from NMBluezManager).

However, don't let nm_connection_get_connection_type() and
nm_connnection_is_type() lie about what the connection.type is.
The type is "bluetooth" for most purposes -- at least, as far as
the client is concerned (and the public API of libnm). This restores
previous API behavior, where nm_connection_get_connection_type()
and nm_connection_is_type() would be simple accessors to the
"connection.type" property.

Only a few places care about the bridge aspect, and those places need special
treatment. For example NMDeviceBridge needs to be fully aware that it can
handle bluetooth NAP connection. That is nothing new: if you handle a
connection of any type, you must know which fields matter and what they
mean. It's not enough that nm_connection_get_connection_type() for bluetooth
NAP connectins is claiming to be a bridge.

Counter examples, where the original behavior is right:

src/nm-manager.c-        g_set_error (error,
src/nm-manager.c-                     NM_MANAGER_ERROR,
src/nm-manager.c-                     NM_MANAGER_ERROR_FAILED,
src/nm-manager.c-                     "NetworkManager plugin for '%s' unavailable",
src/nm-manager.c:                     nm_connection_get_connection_type (connection));

the correct message is: "no bluetooth plugin available", not "bridge".

src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c:   if (   (   nm_connection_is_type (connection, NM_SETTING_WIRED_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c:           && !nm_connection_get_setting_pppoe (connection))
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c:       || nm_connection_is_type (connection, NM_SETTING_VLAN_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c:       || nm_connection_is_type (connection, NM_SETTING_WIRELESS_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c:       || nm_connection_is_type (connection, NM_SETTING_INFINIBAND_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c:       || nm_connection_is_type (connection, NM_SETTING_BOND_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c:       || nm_connection_is_type (connection, NM_SETTING_TEAM_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c:       || nm_connection_is_type (connection, NM_SETTING_BRIDGE_SETTING_NAME))
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c-        return TRUE;

the correct behavior is for ifcfg-rh plugin to reject bluetooth NAP
connections, not proceed and store it.

7 files changed, 103 insertions, 109 deletions

diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c
index 97bbf0d02a..0faf68a567 100644
--- a/clients/common/nm-secret-agent-simple.c
+++ b/clients/common/nm-secret-agent-simple.c
@@ -543,23 +543,30 @@ request_secrets_from_ui (NMSecretAgentSimpleRequest *request)
 		                                            TRUE);
 		g_ptr_array_add (secrets, secret);
 	} else if (nm_connection_is_type (request->connection, NM_SETTING_BLUETOOTH_SETTING_NAME)) {
-		NMSetting *setting;
-
-		setting = nm_connection_get_setting_by_name (request->connection, NM_SETTING_GSM_SETTING_NAME);
-		if (!setting)
-			setting = nm_connection_get_setting_by_name (request->connection, NM_SETTING_CDMA_SETTING_NAME);
+		NMSetting *setting = NULL;
+
+		setting = nm_connection_get_setting_by_name (request->connection, NM_SETTING_BLUETOOTH_SETTING_NAME);
+		if (   setting
+		    && !nm_streq0 (nm_setting_bluetooth_get_connection_type (NM_SETTING_BLUETOOTH (setting)), NM_SETTING_BLUETOOTH_TYPE_NAP)) {
+			setting = nm_connection_get_setting_by_name (request->connection, NM_SETTING_GSM_SETTING_NAME);
+			if (!setting)
+				setting = nm_connection_get_setting_by_name (request->connection, NM_SETTING_CDMA_SETTING_NAME);
+		}
 
-		title = _("Mobile broadband network password");
-		msg = g_strdup_printf (_("A password is required to connect to '%s'."),
-		                       nm_connection_get_id (request->connection));
+		if (setting) {
+			title = _("Mobile broadband network password");
+			msg = g_strdup_printf (_("A password is required to connect to '%s'."),
+			                       nm_connection_get_id (request->connection));
 
-		secret = nm_secret_agent_simple_secret_new (_("Password"),
-		                                            setting,
-		                                            "password",
-		                                            NULL,
-		                                            NULL,
-		                                            TRUE);
-		g_ptr_array_add (secrets, secret);
+			secret = nm_secret_agent_simple_secret_new (_("Password"),
+			                                            setting,
+			                                            "password",
+			                                            NULL,
+			                                            NULL,
+			                                            TRUE);
+			g_ptr_array_add (secrets, secret);
+		} else
+			ok = FALSE;
 	} else if (nm_connection_is_type (request->connection, NM_SETTING_VPN_SETTING_NAME)) {
 		NMSettingConnection *s_con;
 
diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c
index e2f51dfb96..a5a4643750 100644
--- a/libnm-core/nm-connection.c
+++ b/libnm-core/nm-connection.c
@@ -1664,16 +1664,9 @@ nm_connection_to_dbus (NMConnection *connection,
 gboolean
 nm_connection_is_type (NMConnection *connection, const char *type)
 {
-	NMSetting *setting;
-
-	g_return_val_if_fail (NM_IS_CONNECTION (connection), FALSE);
-	g_return_val_if_fail (type != NULL, FALSE);
+	g_return_val_if_fail (type, FALSE);
 
-	setting = nm_connection_get_setting_by_name (connection, type);
-	if (!setting)
-		return FALSE;
-
-	return _nm_setting_get_base_type_priority (setting) != NM_SETTING_PRIORITY_INVALID;
+	return nm_streq0 (type, nm_connection_get_connection_type (connection));
 }
 
 static int
@@ -1902,19 +1895,22 @@ nm_connection_get_id (NMConnection *connection)
  * nm_connection_get_connection_type:
  * @connection: the #NMConnection
  *
- * Returns: the connection's base type.
+ * A shortcut to return the type from the connection's #NMSettingConnection.
+ *
+ * Returns: the type from the connection's 'connection' setting
  **/
 const char *
 nm_connection_get_connection_type (NMConnection *connection)
 {
-	NMSetting *setting;
+	NMSettingConnection *s_con;
 
 	g_return_val_if_fail (NM_IS_CONNECTION (connection), NULL);
 
-	setting = _nm_connection_find_base_type_setting (connection);
-	if (!setting)
+	s_con = nm_connection_get_setting_connection (connection);
+	if (!s_con)
 		return NULL;
-	return nm_setting_get_name (setting);
+
+	return nm_setting_connection_get_connection_type (s_con);
 }
 
 /**
@@ -1949,10 +1945,12 @@ nm_connection_is_virtual (NMConnection *connection)
 		NMSettingInfiniband *s_ib;
 
 		s_ib = nm_connection_get_setting_infiniband (connection);
-		g_return_val_if_fail (s_ib != NULL, FALSE);
-		return nm_setting_infiniband_get_virtual_interface_name (s_ib) != NULL;
+		return s_ib && nm_setting_infiniband_get_virtual_interface_name (s_ib);
 	}
 
+	if (nm_streq (type, NM_SETTING_BLUETOOTH_SETTING_NAME))
+		return !!_nm_connection_get_setting_bluetooth_for_nap (connection);
+
 	return FALSE;
 }
 
diff --git a/libnm/nm-device-bridge.c b/libnm/nm-device-bridge.c
index 55f09a20fa..27f362b2d6 100644
--- a/libnm/nm-device-bridge.c
+++ b/libnm/nm-device-bridge.c
@@ -107,9 +107,14 @@ connection_compatible (NMDevice *device, NMConnection *connection, GError **erro
 		return FALSE;
 
 	if (!nm_connection_is_type (connection, NM_SETTING_BRIDGE_SETTING_NAME)) {
-		g_set_error_literal (error, NM_DEVICE_ERROR, NM_DEVICE_ERROR_INCOMPATIBLE_CONNECTION,
-		                     _("The connection was not a bridge connection."));
-		return FALSE;
+		if (   _nm_connection_get_setting_bluetooth_for_nap (connection)
+		    && nm_connection_is_type (connection, NM_SETTING_BLUETOOTH_SETTING_NAME)) {
+			/* a bluetooth NAP setting is a compatible connection for a bridge. */
+		} else {
+			g_set_error_literal (error, NM_DEVICE_ERROR, NM_DEVICE_ERROR_INCOMPATIBLE_CONNECTION,
+			                     _("The connection was not a bridge connection."));
+			return FALSE;
+		}
 	}
 
 	/* FIXME: check ports? */
diff --git a/libnm/nm-device-bt.c b/libnm/nm-device-bt.c
index 0fcd28b9e8..a47e39580f 100644
--- a/libnm/nm-device-bt.c
+++ b/libnm/nm-device-bt.c
@@ -131,12 +131,19 @@ connection_compatible (NMDevice *device, NMConnection *connection, GError **erro
 	if (!NM_DEVICE_CLASS (nm_device_bt_parent_class)->connection_compatible (device, connection, error))
 		return FALSE;
 
-	if (!nm_connection_is_type (connection, NM_SETTING_BLUETOOTH_SETTING_NAME)) {
+	if (   !nm_connection_is_type (connection, NM_SETTING_BLUETOOTH_SETTING_NAME)
+	    || !(s_bt = nm_connection_get_setting_bluetooth (connection))) {
 		g_set_error (error, NM_DEVICE_ERROR, NM_DEVICE_ERROR_INCOMPATIBLE_CONNECTION,
 		             _("The connection was not a Bluetooth connection."));
 		return FALSE;
 	}
 
+	if (nm_streq0 (nm_setting_bluetooth_get_connection_type (s_bt), NM_SETTING_BLUETOOTH_TYPE_NAP)) {
+		g_set_error (error, NM_DEVICE_ERROR, NM_DEVICE_ERROR_INCOMPATIBLE_CONNECTION,
+		             _("The connection is of Bluetooth NAP type."));
+		return FALSE;
+	}
+
 	/* Check BT address */
 	hw_addr = nm_device_bt_get_hw_address (NM_DEVICE_BT (device));
 	if (hw_addr) {
@@ -145,7 +152,6 @@ connection_compatible (NMDevice *device, NMConnection *connection, GError **erro
 			                     _("Invalid device Bluetooth address."));
 			return FALSE;
 		}
-		s_bt = nm_connection_get_setting_bluetooth (connection);
 		setting_addr = nm_setting_bluetooth_get_bdaddr (s_bt);
 		if (setting_addr && !nm_utils_hwaddr_matches (setting_addr, -1, hw_addr, -1)) {
 			g_set_error_literal (error, NM_DEVICE_ERROR, NM_DEVICE_ERROR_INCOMPATIBLE_CONNECTION,
diff --git a/src/devices/bluetooth/nm-bluez-device.c b/src/devices/bluetooth/nm-bluez-device.c
index e42f6533da..af3ab77f9f 100644
--- a/src/devices/bluetooth/nm-bluez-device.c
+++ b/src/devices/bluetooth/nm-bluez-device.c
@@ -343,6 +343,10 @@ connection_compatible (NMBluezDevice *self, NMConnection *connection)
 		return FALSE;
 
 	bt_type = nm_setting_bluetooth_get_connection_type (s_bt);
+
+	if (nm_streq (bt_type, NM_SETTING_BLUETOOTH_TYPE_NAP))
+		return FALSE;
+
 	if (   g_str_equal (bt_type, NM_SETTING_BLUETOOTH_TYPE_DUN)
 	    && !(priv->capabilities & NM_BT_CAPABILITY_DUN))
 		return FALSE;
diff --git a/src/devices/nm-device-bridge.c b/src/devices/nm-device-bridge.c
index 166e1bdd27..6a0d383c3c 100644
--- a/src/devices/nm-device-bridge.c
+++ b/src/devices/nm-device-bridge.c
@@ -136,9 +136,17 @@ check_connection_compatible (NMDevice *device, NMConnection *connection)
 		return FALSE;
 
 	s_bridge = nm_connection_get_setting_bridge (connection);
-	if (!s_bridge || !nm_connection_is_type (connection, NM_SETTING_BRIDGE_SETTING_NAME))
+	if (!s_bridge)
 		return FALSE;
 
+	if (!nm_connection_is_type (connection, NM_SETTING_BRIDGE_SETTING_NAME)) {
+		if (   nm_connection_is_type (connection, NM_SETTING_BLUETOOTH_SETTING_NAME)
+		    && _nm_connection_get_setting_bluetooth_for_nap (connection)) {
+			/* a bluetooth NAP connection is handled by the bridge */
+		} else
+			return FALSE;
+	}
+
 	mac_address = nm_setting_bridge_get_mac_address (s_bridge);
 	if (mac_address && nm_device_is_real (device)) {
 		const char *hw_addr;
diff --git a/src/devices/nm-device-factory.c b/src/devices/nm-device-factory.c
index c09212cddb..d0ad791acb 100644
--- a/src/devices/nm-device-factory.c
+++ b/src/devices/nm-device-factory.c
@@ -30,6 +30,8 @@
 
 #include "platform/nm-platform.h"
 #include "nm-utils.h"
+#include "nm-core-internal.h"
+#include "nm-setting-bluetooth.h"
 
 #define PLUGIN_PREFIX "libnm-device-plugin-"
 
@@ -90,56 +92,26 @@ nm_device_factory_create_device (NMDeviceFactory *factory,
                                  GError **error)
 {
 	NMDeviceFactoryClass *klass;
-	const NMLinkType *link_types = NULL;
-	const char *const*setting_types = NULL;
-	int i;
 	NMDevice *device;
 	gboolean ignore = FALSE;
 
 	g_return_val_if_fail (factory, NULL);
 	g_return_val_if_fail (iface && *iface, NULL);
-	g_return_val_if_fail (plink || connection, NULL);
-	g_return_val_if_fail (!plink || !connection, NULL);
-
-	nm_device_factory_get_supported_types (factory, &link_types, &setting_types);
-
-	NM_SET_OUT (out_ignore, FALSE);
-
 	if (plink) {
+		g_return_val_if_fail (!connection, NULL);
 		g_return_val_if_fail (strcmp (iface, plink->name) == 0, NULL);
-
-		for (i = 0; link_types[i] > NM_LINK_TYPE_UNKNOWN; i++) {
-			if (plink->type == link_types[i])
-				break;
-		}
-
-		if (link_types[i] == NM_LINK_TYPE_UNKNOWN) {
-			g_set_error (error, NM_DEVICE_ERROR, NM_DEVICE_ERROR_CREATION_FAILED,
-			             "Device factory %s does not support link type %s (%d)",
-			             G_OBJECT_TYPE_NAME (factory),
-			             plink->kind, plink->type);
-			return NULL;
-		}
-	} else if (connection) {
-		for (i = 0; setting_types && setting_types[i]; i++) {
-			if (nm_connection_is_type (connection, setting_types[i]))
-				break;
-		}
-
-		if (!setting_types[i]) {
-			g_set_error (error, NM_DEVICE_ERROR, NM_DEVICE_ERROR_INCOMPATIBLE_CONNECTION,
-			             "Device factory %s does not support connection type %s",
-			             G_OBJECT_TYPE_NAME (factory),
-			             nm_connection_get_connection_type (connection));
-			return NULL;
-		}
-	}
+		nm_assert (factory == nm_device_factory_manager_find_factory_for_link_type (plink->type));
+	} else if (connection)
+		nm_assert (factory == nm_device_factory_manager_find_factory_for_connection (connection));
+	else
+		g_return_val_if_reached (NULL);
 
 	klass = NM_DEVICE_FACTORY_GET_CLASS (factory);
 	if (!klass->create_device) {
 		g_set_error (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_FAILED,
 		             "Device factory %s cannot manage new devices",
 		             G_OBJECT_TYPE_NAME (factory));
+		NM_SET_OUT (out_ignore, FALSE);
 		return NULL;
 	}
 
@@ -252,51 +224,45 @@ _cleanup (void)
 	g_clear_pointer (&factories_by_setting, g_hash_table_unref);
 }
 
-static NMDeviceFactory *
-find_factory (const NMLinkType *needle_link_types,
-              const char *const*needle_setting_types)
-{
-	NMDeviceFactory *found;
-	guint i;
-
-	g_return_val_if_fail (factories_by_link, NULL);
-	g_return_val_if_fail (factories_by_setting, NULL);
-
-	/* NMLinkType search */
-	for (i = 0; needle_link_types && needle_link_types[i] > NM_LINK_TYPE_UNKNOWN; i++) {
-		found = g_hash_table_lookup (factories_by_link, GUINT_TO_POINTER (needle_link_types[i]));
-		if (found)
-			return found;
-	}
-
-	/* NMSetting name search */
-	for (i = 0; needle_setting_types && needle_setting_types[i]; i++) {
-		found = g_hash_table_lookup (factories_by_setting, needle_setting_types[i]);
-		if (found)
-			return found;
-	}
-
-	return NULL;
-}
-
 NMDeviceFactory *
 nm_device_factory_manager_find_factory_for_link_type (NMLinkType link_type)
 {
-	const NMLinkType ltypes[2] = { link_type, NM_LINK_TYPE_NONE };
+	g_return_val_if_fail (factories_by_link, NULL);
 
-	if (link_type == NM_LINK_TYPE_UNKNOWN)
-		return NULL;
-	g_return_val_if_fail (link_type > NM_LINK_TYPE_UNKNOWN, NULL);
-	return find_factory (ltypes, NULL);
+	return g_hash_table_lookup (factories_by_link, GUINT_TO_POINTER (link_type));
 }
 
 NMDeviceFactory *
 nm_device_factory_manager_find_factory_for_connection (NMConnection *connection)
 {
-	const char *const stypes[2] = { nm_connection_get_connection_type (connection), NULL };
+	const char *type;
+
+	g_return_val_if_fail (factories_by_setting, NULL);
+
+	type = nm_connection_get_connection_type (connection);
+
+	if (   nm_streq (type, NM_SETTING_BLUETOOTH_SETTING_NAME)
+	    && _nm_connection_get_setting_bluetooth_for_nap (connection)) {
+		/* for Bluetooth NAP connections, we return the bridge factory
+		 * instead of the bluetooth factory.
+		 *
+		 * In a way, this is a hack. The more orthodox solution would
+		 * be that device factories don't only announce supported setting
+		 * types, but instead match on a full fledged NMConnection.
+		 *
+		 * However, our device-factories are known at compile time.
+		 * There is no need to keep this generic. We *know* which
+		 * factory to choose. Making this generic would not make it
+		 * cleaner. */
+		if (!g_hash_table_lookup (factories_by_setting, type)) {
+			/* we need both the bluetooth and the bridge factory
+			 * to make this work. */
+			return NULL;
+		}
+		type = NM_SETTING_BRIDGE_SETTING_NAME;
+	}
 
-	g_assert (stypes[0]);
-	return find_factory (NULL, stypes);
+	return g_hash_table_lookup (factories_by_setting, type);
 }
 
 void