settings: use delegation instead of inheritance for NMSettingsConnection and NMConnectionth/settings-connection-delegate

NMConnection is an interface, which is both implemented by
NMSimpleConnection (libnm-core), NMSettingsConnection (src) and
NMRemoteConnection (libnm).

NMSettingsConnection does a lot of things

  1) it is an NMDBusObject and exports the API of a connection profile
     on D-Bus
  2) it interacts with NMSettings and contains functionality
     for tracking the profiles.
  3) it is the base-class of types like NMSKeyfileConnection and
     NMIfcfgConnection. These determine how to persist the profile
     on disk.
  4) it implements NMConnection interface, to itself track the
     settings of the profile.

3) and 4) would be better implemented via delegation than inheritance.

Don't let NMSettingsConnection implemente the NMConnection interface.
Instead, it references now a NMSimpleConnection instance, to which it
delegates for keeping the actual profiles.

Advantages:

  - by delegating, there is a clearer separation of what
    NMSettingsConnection does. For example, in C we often required
    casts from NMSettingsConnection to NMConnection. NMConnection
    is a very trivial object with very little logic. When we have
    a NMConnection instance at hand, it's good to know that it is
    *only* that simple.

  - NMConnection is implemented as an interface and we create
    NMSimpleConnection instances whenever we need a real instance.
    In GLib, interfaces have a performance overhead, that we needlessly
    pay all the time. With this change, we could compile a version of
    libnm-core for the daemon, where NMConnection is not an interface but
    a GObject implementation akin to NMSimpleConnection.

  - In the previous implementation, we cannot do copy-on-write.
    For example, when NMDevice needs a snapshot of the activated
    profile as applied-connection, all it can do is clone the
    NMSettingsConnection as a NMSimpleConnection.
    Likewise, when we get a NMConnection instance and want to keep
    a reference to it, we cannot do that, because we never know
    who also references and modifies the instance.
    By separating NMSettingsConnection we could have NMConnection
    immutable and copy-on-write, to avoid all unnecessary clones.

6 files changed, 122 insertions, 138 deletions

diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c
index a3bd601359..1404854d71 100644
--- a/src/NetworkManagerUtils.c
+++ b/src/NetworkManagerUtils.c
@@ -805,9 +805,11 @@ nm_utils_match_connection (NMConnection *const*connections,
 		return NULL;
 
 	for (; *connections; connections++) {
-		NMConnection *candidate = NM_CONNECTION (*connections);
+		NMConnection *candidate = *connections;
 		GHashTable *diffs = NULL;
 
+		nm_assert (NM_IS_CONNECTION (candidate));
+
 		if (match_filter_func) {
 			if (!match_filter_func (candidate, match_filter_data))
 				continue;
diff --git a/src/devices/bluetooth/nm-bluez-device.c b/src/devices/bluetooth/nm-bluez-device.c
index d8e40d6f38..e988d74e2f 100644
--- a/src/devices/bluetooth/nm-bluez-device.c
+++ b/src/devices/bluetooth/nm-bluez-device.c
@@ -94,7 +94,7 @@ typedef struct {
 	NMSettings *settings;
 	GSList *connections;
 
-	NMConnection *pan_connection;
+	NMSettingsConnection *pan_connection;
 	gboolean pan_connection_no_autocreate;
 } NMBluezDevicePrivate;
 
@@ -114,8 +114,9 @@ G_DEFINE_TYPE (NMBluezDevice, nm_bluez_device, G_TYPE_OBJECT)
 /*****************************************************************************/
 
 static void cp_connection_added (NMSettings *settings,
-                                 NMConnection *connection, NMBluezDevice *self);
-static gboolean connection_compatible (NMBluezDevice *self, NMConnection *connection);
+                                 NMSettingsConnection *set_con,
+                                 NMBluezDevice *self);
+static gboolean connection_compatible (NMBluezDevice *self, NMSettingsConnection *set_con);
 
 /*****************************************************************************/
 
@@ -181,10 +182,10 @@ nm_bluez_device_get_connected (NMBluezDevice *self)
 static void
 pan_connection_check_create (NMBluezDevice *self)
 {
-	NMConnection *connection;
-	NMConnection *added;
+	gs_unref_object NMConnection *connection = NULL;
+	NMSettingsConnection *added;
 	NMSetting *setting;
-	char *id;
+	gs_free char *id = NULL;
 	char uuid[37];
 	GError *error = NULL;
 	NMBluezDevicePrivate *priv = NM_BLUEZ_DEVICE_GET_PRIVATE (self);
@@ -246,15 +247,14 @@ pan_connection_check_create (NMBluezDevice *self)
 	 * which then already finds the suitable connection in priv->connections. This is confusing,
 	 * so block the signal. check_emit_usable will succeed after this function call returns. */
 	g_signal_handlers_block_by_func (priv->settings, cp_connection_added, self);
-	added = NM_CONNECTION (nm_settings_add_connection (priv->settings, connection, FALSE, &error));
+	added = nm_settings_add_connection (priv->settings, connection, FALSE, &error);
 	g_signal_handlers_unblock_by_func (priv->settings, cp_connection_added, self);
 
 	if (added) {
-		g_assert (!g_slist_find (priv->connections, added));
-		g_assert (connection_compatible (self, added));
-		g_assert (nm_connection_compare (added, connection, NM_SETTING_COMPARE_FLAG_EXACT));
+		nm_assert (!g_slist_find (priv->connections, added));
+		nm_assert (connection_compatible (self, added));
 
-		nm_settings_connection_set_flags (NM_SETTINGS_CONNECTION (added), NM_SETTINGS_CONNECTION_INT_FLAGS_NM_GENERATED, TRUE);
+		nm_settings_connection_set_flags (added, NM_SETTINGS_CONNECTION_INT_FLAGS_NM_GENERATED, TRUE);
 
 		priv->connections = g_slist_prepend (priv->connections, g_object_ref (added));
 		priv->pan_connection = added;
@@ -263,11 +263,7 @@ pan_connection_check_create (NMBluezDevice *self)
 		nm_log_warn (LOGD_BT, "bluez[%s] couldn't add new Bluetooth connection for NAP device: '%s' (%s): %s",
 		             priv->path, id, uuid, error->message);
 		g_clear_error (&error);
-
 	}
-	g_object_unref (connection);
-
-	g_free (id);
 }
 
 static gboolean
@@ -321,9 +317,10 @@ check_emit_usable_schedule (NMBluezDevice *self)
 /*****************************************************************************/
 
 static gboolean
-connection_compatible (NMBluezDevice *self, NMConnection *connection)
+connection_compatible (NMBluezDevice *self, NMSettingsConnection *set_con)
 {
 	NMBluezDevicePrivate *priv = NM_BLUEZ_DEVICE_GET_PRIVATE (self);
+	NMConnection *connection = nm_settings_connection_get_connection (set_con);
 	NMSettingBluetooth *s_bt;
 	const char *bt_type;
 	const char *bdaddr;
@@ -361,22 +358,24 @@ connection_compatible (NMBluezDevice *self, NMConnection *connection)
 }
 
 static gboolean
-_internal_track_connection (NMBluezDevice *self, NMConnection *connection, gboolean tracked)
+_internal_track_connection (NMBluezDevice *self,
+                            NMSettingsConnection *set_con,
+                            gboolean tracked)
 {
 	NMBluezDevicePrivate *priv = NM_BLUEZ_DEVICE_GET_PRIVATE (self);
 	gboolean was_tracked;
 
-	was_tracked = !!g_slist_find (priv->connections, connection);
+	was_tracked = !!g_slist_find (priv->connections, set_con);
 	if (was_tracked == !!tracked)
 		return FALSE;
 
 	if (tracked)
-		priv->connections = g_slist_prepend (priv->connections, g_object_ref (connection));
+		priv->connections = g_slist_prepend (priv->connections, g_object_ref (set_con));
 	else {
-		priv->connections = g_slist_remove (priv->connections, connection);
-		if (priv->pan_connection == connection)
+		priv->connections = g_slist_remove (priv->connections, set_con);
+		if (priv->pan_connection == set_con)
 			priv->pan_connection = NULL;
-		g_object_unref (connection);
+		g_object_unref (set_con);
 	}
 
 	return TRUE;
@@ -384,32 +383,32 @@ _internal_track_connection (NMBluezDevice *self, NMConnection *connection, gbool
 
 static void
 cp_connection_added (NMSettings *settings,
-                     NMConnection *connection,
+                     NMSettingsConnection *set_con,
                      NMBluezDevice *self)
 {
-	if (connection_compatible (self, connection)) {
-		if (_internal_track_connection (self, connection, TRUE))
+	if (connection_compatible (self, set_con)) {
+		if (_internal_track_connection (self, set_con, TRUE))
 			check_emit_usable (self);
 	}
 }
 
 static void
 cp_connection_removed (NMSettings *settings,
-                       NMConnection *connection,
+                       NMSettingsConnection *set_con,
                        NMBluezDevice *self)
 {
-	if (_internal_track_connection (self, connection, FALSE))
+	if (_internal_track_connection (self, set_con, FALSE))
 		check_emit_usable (self);
 }
 
 static void
 cp_connection_updated (NMSettings *settings,
-                       NMConnection *connection,
+                       NMSettingsConnection *set_con,
                        gboolean by_user,
                        NMBluezDevice *self)
 {
-	if (_internal_track_connection (self, connection,
-	                                connection_compatible (self, connection)))
+	if (_internal_track_connection (self, set_con,
+	                                connection_compatible (self, set_con)))
 		check_emit_usable_schedule (self);
 }
 
@@ -423,10 +422,8 @@ load_connections (NMBluezDevice *self)
 
 	connections = nm_settings_get_connections (priv->settings, NULL);
 	for (i = 0; connections[i]; i++) {
-		NMConnection *connection = (NMConnection *) connections[i];
-
-		if (connection_compatible (self, connection))
-			changed |= _internal_track_connection (self, connection, TRUE);
+		if (connection_compatible (self, connections[i]))
+			changed |= _internal_track_connection (self, connections[i], TRUE);
 	}
 	if (changed)
 		check_emit_usable (self);
@@ -1178,14 +1175,14 @@ dispose (GObject *object)
 {
 	NMBluezDevice *self = NM_BLUEZ_DEVICE (object);
 	NMBluezDevicePrivate *priv = NM_BLUEZ_DEVICE_GET_PRIVATE (self);
-	NMConnection *to_delete = NULL;
+	NMSettingsConnection *to_delete = NULL;
 
 	nm_clear_g_source (&priv->check_emit_usable_id);
 
 	if (priv->pan_connection) {
 		/* Check whether we want to remove the created connection. If so, we take a reference
 		 * and delete it at the end of dispose(). */
-		if (NM_FLAGS_HAS (nm_settings_connection_get_flags (NM_SETTINGS_CONNECTION (priv->pan_connection)),
+		if (NM_FLAGS_HAS (nm_settings_connection_get_flags (priv->pan_connection),
 		                  NM_SETTINGS_CONNECTION_INT_FLAGS_NM_GENERATED))
 			to_delete = g_object_ref (priv->pan_connection);
 
@@ -1219,8 +1216,8 @@ dispose (GObject *object)
 
 	if (to_delete) {
 		nm_log_dbg (LOGD_BT, "bluez[%s] removing Bluetooth connection for NAP device: '%s' (%s)", priv->path,
-		            nm_connection_get_id (to_delete), nm_connection_get_uuid (to_delete));
-		nm_settings_connection_delete (NM_SETTINGS_CONNECTION (to_delete), NULL);
+		            nm_settings_connection_get_id (to_delete), nm_settings_connection_get_uuid (to_delete));
+		nm_settings_connection_delete (to_delete, NULL);
 		g_object_unref (to_delete);
 	}
 
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index b1faf13e15..1306761822 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -1736,7 +1736,7 @@ nm_device_parent_find_for_connection (NMDevice *self,
 		                                                        current_setting_parent);
 		if (   parent_connection
 		    && nm_device_check_connection_compatible (parent_device,
-		                                              NM_CONNECTION (parent_connection),
+		                                              nm_settings_connection_get_connection (parent_connection),
 		                                              NULL))
 			return current_setting_parent;
 	}
@@ -11883,7 +11883,8 @@ nm_device_set_ip_config (NMDevice *self,
 		                     NM_SETTINGS_CONNECTION_INT_FLAGS_NM_GENERATED)
 		    && nm_active_connection_get_activation_type (NM_ACTIVE_CONNECTION (priv->act_request.obj)) == NM_ACTIVATION_TYPE_EXTERNAL) {
 			g_object_freeze_notify (G_OBJECT (settings_connection));
-			nm_connection_add_setting (NM_CONNECTION (settings_connection),
+			/* FIXME(copy-on-write-connection): avoid modifying NMConnection instances and share them via copy-on-write. */
+			nm_connection_add_setting (nm_settings_connection_get_connection (settings_connection),
 			                           IS_IPv4
 			                             ? nm_ip4_config_create_setting (priv->ip_config_4)
 			                             : nm_ip6_config_create_setting (priv->ip_config_6));
@@ -13732,7 +13733,7 @@ nm_device_get_best_connection (NMDevice *self,
 		 */
 		if (    specific_object /* << Optimization: we know that the connection is available without @specific_object.  */
 		    && !nm_device_check_connection_available (self,
-		                                              NM_CONNECTION (candidate),
+		                                              nm_settings_connection_get_connection (candidate),
 		                                              _NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST,
 		                                              specific_object,
 		                                              NULL))
diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c
index 6c5a4723fb..35d46529e4 100644
--- a/src/devices/wifi/nm-device-wifi.c
+++ b/src/devices/wifi/nm-device-wifi.c
@@ -1264,14 +1264,15 @@ check_scanning_prohibited (NMDeviceWifi *self, gboolean periodic)
 
 static gboolean
 hidden_filter_func (NMSettings *settings,
-                    NMSettingsConnection *connection,
+                    NMSettingsConnection *set_con,
                     gpointer user_data)
 {
+	NMConnection *connection = nm_settings_connection_get_connection (set_con);
 	NMSettingWireless *s_wifi;
 
-	if (!nm_connection_is_type (NM_CONNECTION (connection), NM_SETTING_WIRELESS_SETTING_NAME))
+	if (!nm_connection_is_type (connection, NM_SETTING_WIRELESS_SETTING_NAME))
 		return FALSE;
-	s_wifi = nm_connection_get_setting_wireless (NM_CONNECTION (connection));
+	s_wifi = nm_connection_get_setting_wireless (connection);
 	if (!s_wifi)
 		return FALSE;
 	if (nm_streq0 (nm_setting_wireless_get_mode (s_wifi), NM_SETTING_WIRELESS_MODE_AP))
diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c
index c09f680400..670a70e39a 100644
--- a/src/settings/nm-settings-connection.c
+++ b/src/settings/nm-settings-connection.c
@@ -49,8 +49,6 @@
 
 /*****************************************************************************/
 
-static void nm_settings_connection_connection_interface_init (NMConnectionInterface *iface);
-
 NM_GOBJECT_PROPERTIES_DEFINE (NMSettingsConnection,
 	PROP_UNSAVED,
 	PROP_READY,
@@ -87,6 +85,8 @@ typedef struct _NMSettingsConnectionPrivate {
 
 	CList call_ids_lst_head; /* in-progress secrets requests */
 
+	NMConnection *connection;
+
 	/* Caches secrets from on-disk connections; were they not cached any
 	 * call to nm_connection_clear_secrets() wipes them out and we'd have
 	 * to re-read them from disk which defeats the purpose of having the
@@ -115,9 +115,7 @@ typedef struct _NMSettingsConnectionPrivate {
 
 } NMSettingsConnectionPrivate;
 
-G_DEFINE_TYPE_WITH_CODE (NMSettingsConnection, nm_settings_connection, NM_TYPE_DBUS_OBJECT,
-                         G_IMPLEMENT_INTERFACE (NM_TYPE_CONNECTION, nm_settings_connection_connection_interface_init)
-                         )
+G_DEFINE_TYPE (NMSettingsConnection, nm_settings_connection, NM_TYPE_DBUS_OBJECT)
 
 #define NM_SETTINGS_CONNECTION_GET_PRIVATE(self) _NM_GET_PRIVATE_PTR (self, NMSettingsConnection, NM_IS_SETTINGS_CONNECTION)
 
@@ -152,6 +150,16 @@ static const NMDBusInterfaceInfoExtended interface_info_settings_connection;
 
 /*****************************************************************************/
 
+NMConnection *
+nm_settings_connection_get_connection (NMSettingsConnection *self)
+{
+	nm_assert (NM_IS_SETTINGS_CONNECTION (self));
+
+	return NM_SETTINGS_CONNECTION_GET_PRIVATE (self)->connection;
+}
+
+/*****************************************************************************/
+
 gboolean
 nm_settings_connection_has_unmodified_applied_connection (NMSettingsConnection *self,
                                                           NMConnection *applied_connection,
@@ -163,7 +171,8 @@ nm_settings_connection_has_unmodified_applied_connection (NMSettingsConnection *
 	/* for convenience, we *always* ignore certain settings. */
 	compare_flags |= NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS | NM_SETTING_COMPARE_FLAG_IGNORE_TIMESTAMP;
 
-	return nm_connection_compare (NM_CONNECTION (self), applied_connection, compare_flags);
+	return nm_connection_compare (nm_settings_connection_get_connection (self),
+	                              applied_connection, compare_flags);
 }
 
 /*****************************************************************************/
@@ -332,8 +341,7 @@ nm_settings_connection_recheck_visibility (NMSettingsConnection *self)
 
 	priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self);
 
-	s_con = nm_connection_get_setting_connection (NM_CONNECTION (self));
-	g_assert (s_con);
+	s_con = nm_connection_get_setting_connection (nm_settings_connection_get_connection (self));
 
 	/* Check every user in the ACL for a session */
 	num = nm_setting_connection_get_num_permissions (s_con);
@@ -390,8 +398,7 @@ nm_settings_connection_check_permission (NMSettingsConnection *self,
 	                   NM_SETTINGS_CONNECTION_INT_FLAGS_VISIBLE))
 		return FALSE;
 
-	s_con = nm_connection_get_setting_connection (NM_CONNECTION (self));
-	g_assert (s_con);
+	s_con = nm_connection_get_setting_connection (nm_settings_connection_get_connection (self));
 
 	/* Check every user in the ACL for a session */
 	num = nm_setting_connection_get_num_permissions (s_con);
@@ -447,7 +454,7 @@ update_system_secrets_cache (NMSettingsConnection *self)
 
 	if (priv->system_secrets)
 		g_object_unref (priv->system_secrets);
-	priv->system_secrets = nm_simple_connection_new_clone (NM_CONNECTION (self));
+	priv->system_secrets = nm_simple_connection_new_clone (nm_settings_connection_get_connection (self));
 
 	/* Clear out non-system-owned and not-saved secrets */
 	nm_connection_clear_secrets_with_flags (priv->system_secrets,
@@ -636,7 +643,7 @@ nm_settings_connection_update (NMSettingsConnection *self,
 
 	if (persist_mode == NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK) {
 		if (!klass->commit_changes (self,
-		                            new_connection ?: NM_CONNECTION (self),
+		                            new_connection ?: nm_settings_connection_get_connection (self),
 		                            commit_reason,
 		                            &reread_connection,
 		                            &logmsg_change,
@@ -659,26 +666,26 @@ nm_settings_connection_update (NMSettingsConnection *self,
 
 	/* Do nothing if there's nothing to update */
 	if (   replace_connection
-	    && !nm_connection_compare (NM_CONNECTION (self),
+	    && !nm_connection_compare (nm_settings_connection_get_connection (self),
 	                               replace_connection,
 	                               NM_SETTING_COMPARE_FLAG_EXACT)) {
 		gs_unref_object NMConnection *simple = NULL;
 
 		if (log_diff_name) {
-			nm_utils_log_connection_diff (replace_connection, NM_CONNECTION (self), LOGL_DEBUG, LOGD_CORE, log_diff_name, "++ ",
+			nm_utils_log_connection_diff (replace_connection, nm_settings_connection_get_connection (self), LOGL_DEBUG, LOGD_CORE, log_diff_name, "++ ",
 			                              nm_dbus_object_get_path (NM_DBUS_OBJECT (self)));
 		}
 
 		/* Make a copy of agent-owned secrets because they won't be present in
 		 * the connection returned by plugins, as plugins return only what was
 		 * reread from the file. */
-		simple = nm_simple_connection_new_clone (NM_CONNECTION (self));
+		simple = nm_simple_connection_new_clone (nm_settings_connection_get_connection (self));
 		nm_connection_clear_secrets_with_flags (simple,
 		                                        secrets_filter_cb,
 		                                        GUINT_TO_POINTER (NM_SETTING_SECRET_FLAG_AGENT_OWNED));
 		con_agent_secrets = nm_connection_to_dbus (simple, NM_CONNECTION_SERIALIZE_ONLY_SECRETS);
 
-		nm_connection_replace_settings_from_connection (NM_CONNECTION (self), replace_connection);
+		nm_connection_replace_settings_from_connection (nm_settings_connection_get_connection (self), replace_connection);
 
 		replaced = TRUE;
 	}
@@ -701,12 +708,12 @@ nm_settings_connection_update (NMSettingsConnection *self,
 
 			dict = nm_connection_to_dbus (priv->agent_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS);
 			if (dict) {
-				(void) nm_connection_update_secrets (NM_CONNECTION (self), NULL, dict, NULL);
+				(void) nm_connection_update_secrets (nm_settings_connection_get_connection (self), NULL, dict, NULL);
 				g_variant_unref (dict);
 			}
 		}
 		if (con_agent_secrets)
-			(void) nm_connection_update_secrets (NM_CONNECTION (self), NULL, con_agent_secrets, NULL);
+			(void) nm_connection_update_secrets (nm_settings_connection_get_connection (self), NULL, con_agent_secrets, NULL);
 	}
 
 	nm_settings_connection_recheck_visibility (self);
@@ -800,7 +807,7 @@ nm_settings_connection_delete (NMSettingsConnection *self,
 	set_visible (self, FALSE);
 
 	/* Tell agents to remove secrets for this connection */
-	for_agents = nm_simple_connection_new_clone (NM_CONNECTION (self));
+	for_agents = nm_simple_connection_new_clone (nm_settings_connection_get_connection (self));
 	nm_connection_clear_secrets (for_agents);
 	nm_agent_manager_delete_secrets (priv->agent_mgr,
 	                                 nm_dbus_object_get_path (NM_DBUS_OBJECT (self)),
@@ -920,7 +927,7 @@ get_cmp_flags (NMSettingsConnection *self, /* only needed for logging */
                gboolean *agent_had_system,
                ForEachSecretFlags *cmp_flags)
 {
-	gboolean is_self = (((NMConnection *) self) == connection);
+	gboolean is_self = (nm_settings_connection_get_connection (self) == connection);
 
 	g_return_if_fail (secrets);
 
@@ -1001,7 +1008,7 @@ nm_settings_connection_new_secrets (NMSettingsConnection *self,
 		return FALSE;
 	}
 
-	if (!nm_connection_update_secrets (NM_CONNECTION (self), setting_name, secrets, error))
+	if (!nm_connection_update_secrets (nm_settings_connection_get_connection (self), setting_name, secrets, error))
 		return FALSE;
 
 	update_system_secrets_cache (self);
@@ -1073,7 +1080,7 @@ get_secrets_done_cb (NMAgentManager *manager,
 		goto out;
 	}
 
-	if (!nm_connection_get_setting_by_name (NM_CONNECTION (self), setting_name)) {
+	if (!nm_connection_get_setting_by_name (nm_settings_connection_get_connection (self), setting_name)) {
 		g_set_error (&local, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_SETTING_NOT_FOUND,
 		             "Connection didn't have requested setting '%s'.",
 		             setting_name);
@@ -1083,7 +1090,7 @@ get_secrets_done_cb (NMAgentManager *manager,
 
 	get_cmp_flags (self,
 	               call_id,
-	               NM_CONNECTION (self),
+	               nm_settings_connection_get_connection (self),
 	               agent_dbus_owner,
 	               agent_has_modify,
 	               setting_name,
@@ -1100,8 +1107,8 @@ get_secrets_done_cb (NMAgentManager *manager,
 		dict = nm_connection_to_dbus (priv->system_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS);
 
 	/* Update the connection with our existing secrets from backing storage */
-	nm_connection_clear_secrets (NM_CONNECTION (self));
-	if (!dict || nm_connection_update_secrets (NM_CONNECTION (self), setting_name, dict, &local)) {
+	nm_connection_clear_secrets (nm_settings_connection_get_connection (self));
+	if (!dict || nm_connection_update_secrets (nm_settings_connection_get_connection (self), setting_name, dict, &local)) {
 		GVariant *filtered_secrets;
 
 		/* Update the connection with the agent's secrets; by this point if any
@@ -1109,8 +1116,8 @@ get_secrets_done_cb (NMAgentManager *manager,
 		 * will have been authenticated, so those secrets can replace the existing
 		 * system secrets.
 		 */
-		filtered_secrets = for_each_secret (NM_CONNECTION (self), secrets, TRUE, validate_secret_flags, &cmp_flags);
-		if (nm_connection_update_secrets (NM_CONNECTION (self), setting_name, filtered_secrets, &local)) {
+		filtered_secrets = for_each_secret (nm_settings_connection_get_connection (self), secrets, TRUE, validate_secret_flags, &cmp_flags);
+		if (nm_connection_update_secrets (nm_settings_connection_get_connection (self), setting_name, filtered_secrets, &local)) {
 			/* Now that all secrets are updated, copy and cache new secrets,
 			 * then save them to backing storage.
 			 */
@@ -1250,7 +1257,7 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self,
 	g_return_val_if_fail (NM_IS_SETTINGS_CONNECTION (self), NULL);
 	g_return_val_if_fail (   !applied_connection
 	                      || (   NM_IS_CONNECTION (applied_connection)
-	                          && (((NMConnection *) self) != applied_connection)), NULL);
+	                          && (nm_settings_connection_get_connection (self) != applied_connection)), NULL);
 
 	call_id = g_slice_new0 (NMSettingsConnectionCallId);
 	call_id->self = self;
@@ -1264,7 +1271,7 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self,
 	c_list_link_tail (&priv->call_ids_lst_head, &call_id->call_ids_lst);
 
 	/* Make sure the request actually requests something we can return */
-	if (!nm_connection_get_setting_by_name (NM_CONNECTION (self), setting_name)) {
+	if (!nm_connection_get_setting_by_name (nm_settings_connection_get_connection (self), setting_name)) {
 		g_set_error (&local, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_SETTING_NOT_FOUND,
 		             "Connection didn't have requested setting '%s'.",
 		             setting_name);
@@ -1295,7 +1302,7 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self,
 
 	call_id_a = nm_agent_manager_get_secrets (priv->agent_mgr,
 	                                          nm_dbus_object_get_path (NM_DBUS_OBJECT (self)),
-	                                          NM_CONNECTION (self),
+	                                          nm_settings_connection_get_connection (self),
 	                                          subject,
 	                                          existing_secrets,
 	                                          setting_name,
@@ -1465,7 +1472,7 @@ auth_start (NMSettingsConnection *self,
 	nm_assert (G_IS_DBUS_METHOD_INVOCATION (invocation));
 	nm_assert (NM_IS_AUTH_SUBJECT (subject));
 
-	if (!nm_auth_is_subject_in_acl_set_error (NM_CONNECTION (self),
+	if (!nm_auth_is_subject_in_acl_set_error (nm_settings_connection_get_connection (self),
 	                                          subject,
 	                                          NM_SETTINGS_ERROR,
 	                                          NM_SETTINGS_ERROR_PERMISSION_DENIED,
@@ -1539,15 +1546,14 @@ get_settings_auth_cb (NMSettingsConnection *self,
 	if (error)
 		g_dbus_method_invocation_return_gerror (context, error);
 	else {
+		gs_unref_object NMConnection *dupl_con = NULL;
 		GVariant *settings;
-		NMConnection *dupl_con;
 		NMSettingConnection *s_con;
 		NMSettingWireless *s_wifi;
 		guint64 timestamp = 0;
-		char **bssids;
+		gs_free char **bssids = NULL;
 
-		dupl_con = nm_simple_connection_new_clone (NM_CONNECTION (self));
-		g_assert (dupl_con);
+		dupl_con = nm_simple_connection_new_clone (nm_settings_connection_get_connection (self));
 
 		/* Timestamp is not updated in connection's 'timestamp' property,
 		 * because it would force updating the connection and in turn
@@ -1557,8 +1563,7 @@ get_settings_auth_cb (NMSettingsConnection *self,
 		 */
 		nm_settings_connection_get_timestamp (self, &timestamp);
 		if (timestamp) {
-			s_con = nm_connection_get_setting_connection (NM_CONNECTION (dupl_con));
-			g_assert (s_con);
+			s_con = nm_connection_get_setting_connection (dupl_con);
 			g_object_set (s_con, NM_SETTING_CONNECTION_TIMESTAMP, timestamp, NULL);
 		}
 		/* Seen BSSIDs are not updated in 802-11-wireless 'seen-bssids' property
@@ -1566,20 +1571,17 @@ get_settings_auth_cb (NMSettingsConnection *self,
 		 * return settings too.
 		 */
 		bssids = nm_settings_connection_get_seen_bssids (self);
-		s_wifi = nm_connection_get_setting_wireless (NM_CONNECTION (dupl_con));
+		s_wifi = nm_connection_get_setting_wireless (dupl_con);
 		if (bssids && bssids[0] && s_wifi)
 			g_object_set (s_wifi, NM_SETTING_WIRELESS_SEEN_BSSIDS, bssids, NULL);
-		g_free (bssids);
 
 		/* Secrets should *never* be returned by the GetSettings method, they
 		 * get returned by the GetSecrets method which can be better
 		 * protected against leakage of secrets to unprivileged callers.
 		 */
-		settings = nm_connection_to_dbus (NM_CONNECTION (dupl_con), NM_CONNECTION_SERIALIZE_NO_SECRETS);
-		g_assert (settings);
+		settings = nm_connection_to_dbus (dupl_con, NM_CONNECTION_SERIALIZE_NO_SECRETS);
 		g_dbus_method_invocation_return_value (context,
 		                                       g_variant_new ("(@a{sa{sv}})", settings));
-		g_object_unref (dupl_con);
 	}
 }
 
@@ -1734,7 +1736,7 @@ update_auth_cb (NMSettingsConnection *self,
 			gs_unref_hashtable GHashTable *diff = NULL;
 			gboolean same;
 
-			same = nm_connection_diff (NM_CONNECTION (self), info->new_settings,
+			same = nm_connection_diff (nm_settings_connection_get_connection (self), info->new_settings,
 			                           NM_SETTING_COMPARE_FLAG_EXACT |
 			                           NM_SETTING_COMPARE_FLAG_DIFF_RESULT_NO_DEFAULT,
 			                           &diff);
@@ -1745,7 +1747,7 @@ update_auth_cb (NMSettingsConnection *self,
 
 	commit_reason = NM_SETTINGS_CONNECTION_COMMIT_REASON_USER_ACTION;
 	if (   info->new_settings
-	    && !nm_streq0 (nm_connection_get_id (NM_CONNECTION (self)),
+	    && !nm_streq0 (nm_connection_get_id (nm_settings_connection_get_connection (self)),
 	                   nm_connection_get_id (info->new_settings)))
 		commit_reason |= NM_SETTINGS_CONNECTION_COMMIT_REASON_ID_CHANGED;
 
@@ -1791,7 +1793,7 @@ update_auth_cb (NMSettingsConnection *self,
 		 * as agent-owned secrets are the only ones we send back be saved.
 		 * Only send secrets to agents of the same UID that called update too.
 		 */
-		for_agent = nm_simple_connection_new_clone (NM_CONNECTION (self));
+		for_agent = nm_simple_connection_new_clone (nm_settings_connection_get_connection (self));
 		nm_connection_clear_secrets_with_flags (for_agent,
 		                                        secrets_filter_cb,
 		                                        GUINT_TO_POINTER (NM_SETTING_SECRET_FLAG_AGENT_OWNED));
@@ -1811,11 +1813,9 @@ get_update_modify_permission (NMConnection *old, NMConnection *new)
 	guint32 orig_num = 0, new_num = 0;
 
 	s_con = nm_connection_get_setting_connection (old);
-	g_assert (s_con);
 	orig_num = nm_setting_connection_get_num_permissions (s_con);
 
 	s_con = nm_connection_get_setting_connection (new);
-	g_assert (s_con);
 	new_num = nm_setting_connection_get_num_permissions (s_con);
 
 	/* If the caller is the only user in either connection's permissions, then
@@ -1848,7 +1848,7 @@ settings_connection_update (NMSettingsConnection *self,
 	 * the problem (ex a system settings plugin that can't write connections out)
 	 * instead of over D-Bus.
 	 */
-	if (!check_writable (NM_CONNECTION (self), &error))
+	if (!check_writable (nm_settings_connection_get_connection (self), &error))
 		goto error;
 
 	/* Check if the settings are valid first */
@@ -1894,7 +1894,7 @@ settings_connection_update (NMSettingsConnection *self,
 	info->flags = flags;
 	info->new_settings = tmp;
 
-	permission = get_update_modify_permission (NM_CONNECTION (self),
+	permission = get_update_modify_permission (nm_settings_connection_get_connection (self),
 	                                           tmp ?: NM_CONNECTION(self));
 	auth_start (self, context, subject, permission, update_auth_cb, info);
 	return;
@@ -2066,8 +2066,7 @@ get_modify_permission_basic (NMSettingsConnection *self)
 	 * we use the 'modify.own' permission instead of 'modify.system'.  If the
 	 * request affects more than just the caller, require 'modify.system'.
 	 */
-	s_con = nm_connection_get_setting_connection (NM_CONNECTION (self));
-	nm_assert (s_con);
+	s_con = nm_connection_get_setting_connection (nm_settings_connection_get_connection (self));
 	if (nm_setting_connection_get_num_permissions (s_con) == 1)
 		return NM_AUTH_PERMISSION_SETTINGS_MODIFY_OWN;
 
@@ -2087,7 +2086,7 @@ impl_settings_connection_delete (NMDBusObject *obj,
 	gs_unref_object NMAuthSubject *subject = NULL;
 	GError *error = NULL;
 
-	if (!check_writable (NM_CONNECTION (self), &error))
+	if (!check_writable (nm_settings_connection_get_connection (self), &error))
 		goto err;
 
 	subject = _new_auth_subject (invocation, &error);
@@ -2122,7 +2121,7 @@ dbus_get_agent_secrets_cb (NMSettingsConnection *self,
 		 * secrets from backing storage and those returned from the agent
 		 * by the time we get here.
 		 */
-		dict = nm_connection_to_dbus (NM_CONNECTION (self), NM_CONNECTION_SERIALIZE_ONLY_SECRETS);
+		dict = nm_connection_to_dbus (nm_settings_connection_get_connection (self), NM_CONNECTION_SERIALIZE_ONLY_SECRETS);
 		if (!dict)
 			dict = g_variant_new_array (G_VARIANT_TYPE ("{sa{sv}}"), NULL, 0);
 		g_dbus_method_invocation_return_value (context, g_variant_new ("(@a{sa{sv}})", dict));
@@ -2204,7 +2203,7 @@ dbus_clear_secrets_auth_cb (NMSettingsConnection *self,
 	}
 
 	/* Clear secrets in connection and caches */
-	nm_connection_clear_secrets (NM_CONNECTION (self));
+	nm_connection_clear_secrets (nm_settings_connection_get_connection (self));
 	if (priv->system_secrets)
 		nm_connection_clear_secrets (priv->system_secrets);
 	if (priv->agent_secrets)
@@ -2213,7 +2212,7 @@ dbus_clear_secrets_auth_cb (NMSettingsConnection *self,
 	/* Tell agents to remove secrets for this connection */
 	nm_agent_manager_delete_secrets (priv->agent_mgr,
 	                                 nm_dbus_object_get_path (NM_DBUS_OBJECT (self)),
-	                                 NM_CONNECTION (self));
+	                                 nm_settings_connection_get_connection (self));
 
 	nm_settings_connection_update (self,
 	                               NULL,
@@ -2388,15 +2387,8 @@ _cmp_timestamp (NMSettingsConnection *a, NMSettingsConnection *b)
 static int
 _cmp_last_resort (NMSettingsConnection *a, NMSettingsConnection *b)
 {
-	int c;
-
-	nm_assert (NM_IS_SETTINGS_CONNECTION (a));
-	nm_assert (NM_IS_SETTINGS_CONNECTION (b));
-
-	c = g_strcmp0 (nm_connection_get_uuid (NM_CONNECTION (a)),
-	               nm_connection_get_uuid (NM_CONNECTION (b)));
-	if (c)
-		return c;
+	NM_CMP_DIRECT_STRCMP0 (nm_settings_connection_get_uuid (a),
+	                       nm_settings_connection_get_uuid (b));
 
 	/* hm, same UUID. Use their pointer value to give them a stable
 	 * order. */
@@ -2411,19 +2403,11 @@ _cmp_last_resort (NMSettingsConnection *a, NMSettingsConnection *b)
 int
 nm_settings_connection_cmp_timestamp (NMSettingsConnection *a, NMSettingsConnection *b)
 {
-	int c;
+	NM_CMP_SELF (a, b);
 
-	if (a == b)
-		return 0;
-	if (!a)
-		return 1;
-	if (!b)
-		return -1;
-
-	if ((c = _cmp_timestamp (a, b)))
-		return c;
-	if ((c = nm_utils_cmp_connection_by_autoconnect_priority (NM_CONNECTION (a), NM_CONNECTION (b))))
-		return c;
+	NM_CMP_RETURN (_cmp_timestamp (a, b));
+	NM_CMP_RETURN (nm_utils_cmp_connection_by_autoconnect_priority (nm_settings_connection_get_connection (a),
+	                                                                nm_settings_connection_get_connection (b)));
 	return _cmp_last_resort (a, b);
 }
 
@@ -2437,14 +2421,11 @@ nm_settings_connection_cmp_timestamp_p_with_data (gconstpointer pa, gconstpointe
 int
 nm_settings_connection_cmp_autoconnect_priority (NMSettingsConnection *a, NMSettingsConnection *b)
 {
-	int c;
-
 	if (a == b)
 		return 0;
-	if ((c = nm_utils_cmp_connection_by_autoconnect_priority (NM_CONNECTION (a), NM_CONNECTION (b))))
-		return c;
-	if ((c = _cmp_timestamp (a, b)))
-		return c;
+	NM_CMP_RETURN (nm_utils_cmp_connection_by_autoconnect_priority (nm_settings_connection_get_connection (a),
+	                                                                nm_settings_connection_get_connection (b)));
+	NM_CMP_RETURN (_cmp_timestamp (a, b));
 	return _cmp_last_resort (a, b);
 }
 
@@ -2727,7 +2708,7 @@ nm_settings_connection_read_and_fill_seen_bssids (NMSettingsConnection *self)
 		 * seen-bssids list from the deprecated seen-bssids property of the
 		 * wifi setting.
 		 */
-		s_wifi = nm_connection_get_setting_wireless (NM_CONNECTION (self));
+		s_wifi = nm_connection_get_setting_wireless (nm_settings_connection_get_connection (self));
 		if (s_wifi) {
 			len = nm_setting_wireless_get_num_seen_bssids (s_wifi);
 			for (i = 0; i < len; i++) {
@@ -2747,7 +2728,7 @@ _autoconnect_retries_initial (NMSettingsConnection *self)
 	NMSettingConnection *s_con;
 	int retries = -1;
 
-	s_con = nm_connection_get_setting_connection ((NMConnection *) self);
+	s_con = nm_connection_get_setting_connection (nm_settings_connection_get_connection (self));
 	if (s_con)
 		retries = nm_setting_connection_get_autoconnect_retries (s_con);
 
@@ -2959,13 +2940,13 @@ nm_settings_connection_get_filename (NMSettingsConnection *self)
 const char *
 nm_settings_connection_get_id (NMSettingsConnection *self)
 {
-	return nm_connection_get_id (NM_CONNECTION (self));
+	return nm_connection_get_id (nm_settings_connection_get_connection (self));
 }
 
 const char *
 nm_settings_connection_get_uuid (NMSettingsConnection *self)
 {
-	return nm_connection_get_uuid (NM_CONNECTION (self));
+	return nm_connection_get_uuid (nm_settings_connection_get_connection (self));
 }
 
 /*****************************************************************************/
@@ -2995,6 +2976,8 @@ nm_settings_connection_init (NMSettingsConnection *self)
 
 	priv->autoconnect_retries = AUTOCONNECT_RETRIES_UNSET;
 
+	priv->connection = nm_simple_connection_new ();
+
 	g_signal_connect (self, NM_CONNECTION_SECRETS_CLEARED, G_CALLBACK (secrets_cleared_cb), NULL);
 	g_signal_connect (self, NM_CONNECTION_CHANGED, G_CALLBACK (connection_changed_cb), NULL);
 }
@@ -3034,7 +3017,11 @@ dispose (GObject *object)
 	g_signal_handlers_disconnect_by_func (self, G_CALLBACK (secrets_cleared_cb), NULL);
 	g_signal_handlers_disconnect_by_func (self, G_CALLBACK (connection_changed_cb), NULL);
 
-	nm_connection_clear_secrets (NM_CONNECTION (self));
+	if (priv->connection) {
+		nm_connection_clear_secrets (priv->connection);
+		g_clear_object (&priv->connection);
+	}
+
 	g_clear_object (&priv->system_secrets);
 	g_clear_object (&priv->agent_secrets);
 
@@ -3264,9 +3251,3 @@ nm_settings_connection_class_init (NMSettingsConnectionClass *klass)
 	                  g_cclosure_marshal_VOID__VOID,
 	                  G_TYPE_NONE, 0);
 }
-
-static void
-nm_settings_connection_connection_interface_init (NMConnectionInterface *iface)
-{
-}
-
diff --git a/src/settings/nm-settings-connection.h b/src/settings/nm-settings-connection.h
index cd64f9762d..5b484f8460 100644
--- a/src/settings/nm-settings-connection.h
+++ b/src/settings/nm-settings-connection.h
@@ -139,6 +139,8 @@ struct _NMSettingsConnectionClass {
 
 GType nm_settings_connection_get_type (void);
 
+NMConnection *nm_settings_connection_get_connection (NMSettingsConnection *self);
+
 guint64 nm_settings_connection_get_last_secret_agent_version_id (NMSettingsConnection *self);
 
 gboolean nm_settings_connection_has_unmodified_applied_connection (NMSettingsConnection *self,