all: change handling of connection.type for bluetooth NAP and in generalth/bluetooth-nap

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, 106 insertions, 80 deletions

diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c
index 89cf156bda..fce4840f16 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 f8ff46a614..641ec87637 100644
--- a/libnm-core/nm-connection.c
+++ b/libnm-core/nm-connection.c
@@ -1633,16 +1633,19 @@ nm_connection_to_dbus (NMConnection *connection,
 gboolean
 nm_connection_is_type (NMConnection *connection, const char *type)
 {
-	NMSetting *setting;
+	NMSettingConnection *s_con;
+	const char *type2;
 
 	g_return_val_if_fail (NM_IS_CONNECTION (connection), FALSE);
 	g_return_val_if_fail (type != NULL, FALSE);
 
-	setting = nm_connection_get_setting_by_name (connection, type);
-	if (!setting)
+	s_con = nm_connection_get_setting_connection (connection);
+	if (!s_con)
 		return FALSE;
 
-	return _nm_setting_get_base_type_priority (setting) != NM_SETTING_PRIORITY_INVALID;
+	type2 = nm_setting_connection_get_connection_type (s_con);
+
+	return (g_strcmp0 (type2, type) == 0);
 }
 
 static int
@@ -1871,19 +1874,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);
 }
 
 /**
@@ -1918,10 +1924,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 c281dbad0c..afed25563c 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..10a2ce2d04 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-"
 
@@ -92,9 +94,9 @@ nm_device_factory_create_device (NMDeviceFactory *factory,
 	NMDeviceFactoryClass *klass;
 	const NMLinkType *link_types = NULL;
 	const char *const*setting_types = NULL;
-	int i;
 	NMDevice *device;
 	gboolean ignore = FALSE;
+	NMDeviceFactory *factory_rematch;
 
 	g_return_val_if_fail (factory, NULL);
 	g_return_val_if_fail (iface && *iface, NULL);
@@ -108,12 +110,8 @@ nm_device_factory_create_device (NMDeviceFactory *factory,
 	if (plink) {
 		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) {
+		factory_rematch = nm_device_factory_manager_find_factory_for_link_type (plink->type);
+		if (factory_rematch != factory) {
 			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),
@@ -121,12 +119,8 @@ nm_device_factory_create_device (NMDeviceFactory *factory,
 			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]) {
+		factory_rematch = nm_device_factory_manager_find_factory_for_connection (connection);
+		if (factory_rematch != factory) {
 			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),
@@ -252,51 +246,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