diff --git a/nmrs/CHANGELOG.md b/nmrs/CHANGELOG.md index 61eea944..3279393c 100644 --- a/nmrs/CHANGELOG.md +++ b/nmrs/CHANGELOG.md @@ -3,6 +3,8 @@ All notable changes to the `nmrs` crate will be documented in this file. ## [Unreleased] +### Changed +- Dedupe DBus proxy construction across connection logic ([#165](https://github.com/cachebag/nmrs/pull/165)) ## [1.2.0] - 2026-01-05 ### Added diff --git a/nmrs/src/core/connection.rs b/nmrs/src/core/connection.rs index 5c4de6c6..d71b8773 100644 --- a/nmrs/src/core/connection.rs +++ b/nmrs/src/core/connection.rs @@ -11,7 +11,7 @@ use crate::core::state_wait::{wait_for_connection_activation, wait_for_device_di use crate::dbus::{NMAccessPointProxy, NMDeviceProxy, NMProxy, NMWirelessProxy}; use crate::monitoring::info::current_ssid; use crate::types::constants::{device_state, device_type, timeouts}; -use crate::util::utils::decode_ssid_or_empty; +use crate::util::utils::{decode_ssid_or_empty, nm_proxy}; use crate::Result; /// Decision on whether to reuse a saved connection or create a fresh one. @@ -210,12 +210,12 @@ pub(crate) async fn forget(conn: &Connection, ssid: &str) -> Result<()> { debug!("Starting connection deletion phase..."); - let settings: zbus::Proxy<'_> = zbus::proxy::Builder::new(conn) - .destination("org.freedesktop.NetworkManager")? - .path("/org/freedesktop/NetworkManager/Settings")? - .interface("org.freedesktop.NetworkManager.Settings")? - .build() - .await?; + let settings = nm_proxy( + conn, + "/org/freedesktop/NetworkManager/Settings", + "org.freedesktop.NetworkManager.Settings", + ) + .await?; let list_reply = settings.call_method("ListConnections", &()).await?; let conns: Vec = list_reply.body().deserialize()?; @@ -223,12 +223,12 @@ pub(crate) async fn forget(conn: &Connection, ssid: &str) -> Result<()> { let mut deleted_count = 0; for cpath in conns { - let cproxy: zbus::Proxy<'_> = zbus::proxy::Builder::new(conn) - .destination("org.freedesktop.NetworkManager")? - .path(cpath.clone())? - .interface("org.freedesktop.NetworkManager.Settings.Connection")? - .build() - .await?; + let cproxy = nm_proxy( + conn, + cpath.clone(), + "org.freedesktop.NetworkManager.Settings.Connection", + ) + .await?; if let Ok(msg) = cproxy.call_method("GetSettings", &()).await { let body = msg.body(); @@ -314,12 +314,12 @@ pub(crate) async fn disconnect_wifi_and_wait( return Ok(()); } - let raw: zbus::proxy::Proxy = zbus::proxy::Builder::new(conn) - .destination("org.freedesktop.NetworkManager")? - .path(dev_path.clone())? - .interface("org.freedesktop.NetworkManager.Device")? - .build() - .await?; + let raw = nm_proxy( + conn, + dev_path.clone(), + "org.freedesktop.NetworkManager.Device", + ) + .await?; debug!("Sending disconnect request"); let _ = raw.call_method("Disconnect", &()).await; diff --git a/nmrs/src/core/connection_settings.rs b/nmrs/src/core/connection_settings.rs index 20949386..f4c1bbfa 100644 --- a/nmrs/src/core/connection_settings.rs +++ b/nmrs/src/core/connection_settings.rs @@ -9,6 +9,7 @@ use std::collections::HashMap; use zbus::Connection; use zvariant::{OwnedObjectPath, Value}; +use crate::util::utils::nm_proxy; use crate::Result; /// Finds the D-Bus path of a saved connection by SSID. @@ -22,9 +23,8 @@ pub(crate) async fn get_saved_connection_path( conn: &Connection, ssid: &str, ) -> Result> { - let settings = zbus::proxy::Proxy::new( + let settings = nm_proxy( conn, - "org.freedesktop.NetworkManager", "/org/freedesktop/NetworkManager/Settings", "org.freedesktop.NetworkManager.Settings", ) @@ -34,9 +34,8 @@ pub(crate) async fn get_saved_connection_path( let conns: Vec = reply.body().deserialize()?; for cpath in conns { - let cproxy = zbus::proxy::Proxy::new( + let cproxy = nm_proxy( conn, - "org.freedesktop.NetworkManager", cpath.as_str(), "org.freedesktop.NetworkManager.Settings.Connection", ) @@ -70,12 +69,12 @@ pub(crate) async fn has_saved_connection(conn: &Connection, ssid: &str) -> Resul /// Calls the Delete method on the connection settings object. /// This permanently removes the saved connection from NetworkManager. pub(crate) async fn delete_connection(conn: &Connection, conn_path: OwnedObjectPath) -> Result<()> { - let cproxy: zbus::Proxy<'_> = zbus::proxy::Builder::new(conn) - .destination("org.freedesktop.NetworkManager")? - .path(conn_path.clone())? - .interface("org.freedesktop.NetworkManager.Settings.Connection")? - .build() - .await?; + let cproxy = nm_proxy( + conn, + conn_path.clone(), + "org.freedesktop.NetworkManager.Settings.Connection", + ) + .await?; cproxy.call_method("Delete", &()).await?; debug!("Deleted connection: {}", conn_path.as_str()); diff --git a/nmrs/src/core/vpn.rs b/nmrs/src/core/vpn.rs index d075b8ca..470f0928 100644 --- a/nmrs/src/core/vpn.rs +++ b/nmrs/src/core/vpn.rs @@ -21,6 +21,7 @@ use crate::api::models::{ use crate::builders::build_wireguard_connection; use crate::core::state_wait::wait_for_connection_activation; use crate::dbus::{NMActiveConnectionProxy, NMProxy}; +use crate::util::utils::nm_proxy; use crate::Result; /// Connects to a WireGuard connection. @@ -62,15 +63,15 @@ pub(crate) async fn connect_vpn(conn: &Connection, creds: VpnCredentials) -> Res // Use Settings API to add connection first, then activate separately // This avoids NetworkManager's device validation when using add_and_activate_connection - let settings_proxy: zbus::Proxy<'_> = zbus::proxy::Builder::new(conn) - .destination("org.freedesktop.NetworkManager")? - .path("/org/freedesktop/NetworkManager/Settings")? - .interface("org.freedesktop.NetworkManager.Settings")? - .build() - .await?; + let settings_api = nm_proxy( + conn, + "/org/freedesktop/NetworkManager/Settings", + "org.freedesktop.NetworkManager.Settings", + ) + .await?; debug!("Adding connection via Settings API"); - let add_reply = settings_proxy + let add_reply = settings_api .call_method("AddConnection", &(settings,)) .await?; let conn_path: OwnedObjectPath = add_reply.body().deserialize()?; @@ -148,12 +149,12 @@ pub(crate) async fn disconnect_vpn(conn: &Connection, name: &str) -> Result<()> }; for ac_path in active_conns { - let ac_proxy: zbus::Proxy<'_> = match zbus::proxy::Builder::new(conn) - .destination("org.freedesktop.NetworkManager")? - .path(ac_path.clone())? - .interface("org.freedesktop.NetworkManager.Connection.Active")? - .build() - .await + let ac_proxy = match nm_proxy( + conn, + ac_path.clone(), + "org.freedesktop.NetworkManager.Connection.Active", + ) + .await { Ok(p) => p, Err(_) => continue, @@ -167,12 +168,12 @@ pub(crate) async fn disconnect_vpn(conn: &Connection, name: &str) -> Result<()> Err(_) => continue, }; - let cproxy: zbus::Proxy<'_> = match zbus::proxy::Builder::new(conn) - .destination("org.freedesktop.NetworkManager")? - .path(conn_path.clone())? - .interface("org.freedesktop.NetworkManager.Settings.Connection")? - .build() - .await + let cproxy = match nm_proxy( + conn, + conn_path.clone(), + "org.freedesktop.NetworkManager.Settings.Connection", + ) + .await { Ok(p) => p, Err(_) => continue, @@ -227,12 +228,12 @@ pub(crate) async fn disconnect_vpn(conn: &Connection, name: &str) -> Result<()> pub(crate) async fn list_vpn_connections(conn: &Connection) -> Result> { let nm = NMProxy::new(conn).await?; - let settings: zbus::Proxy<'_> = zbus::proxy::Builder::new(conn) - .destination("org.freedesktop.NetworkManager")? - .path("/org/freedesktop/NetworkManager/Settings")? - .interface("org.freedesktop.NetworkManager.Settings")? - .build() - .await?; + let settings = nm_proxy( + conn, + "/org/freedesktop/NetworkManager/Settings", + "org.freedesktop.NetworkManager.Settings", + ) + .await?; let list_reply = settings.call_method("ListConnections", &()).await?; let saved_conns: Vec = list_reply.body().deserialize()?; @@ -242,12 +243,12 @@ pub(crate) async fn list_vpn_connections(conn: &Connection) -> Result)> = HashMap::new(); for ac_path in active_conns { - let ac_proxy: zbus::Proxy<'_> = match zbus::proxy::Builder::new(conn) - .destination("org.freedesktop.NetworkManager")? - .path(ac_path.clone())? - .interface("org.freedesktop.NetworkManager.Connection.Active")? - .build() - .await + let ac_proxy = match nm_proxy( + conn, + ac_path.clone(), + "org.freedesktop.NetworkManager.Connection.Active", + ) + .await { Ok(p) => p, Err(_) => continue, @@ -261,12 +262,12 @@ pub(crate) async fn list_vpn_connections(conn: &Connection) -> Result continue, }; - let cproxy: zbus::Proxy<'_> = match zbus::proxy::Builder::new(conn) - .destination("org.freedesktop.NetworkManager")? - .path(conn_path)? - .interface("org.freedesktop.NetworkManager.Settings.Connection")? - .build() - .await + let cproxy = match nm_proxy( + conn, + conn_path, + "org.freedesktop.NetworkManager.Settings.Connection", + ) + .await { Ok(p) => p, Err(_) => continue, @@ -314,12 +315,12 @@ pub(crate) async fn list_vpn_connections(conn: &Connection) -> Result::new(conn) - .destination("org.freedesktop.NetworkManager")? - .path(dev_path.clone())? - .interface("org.freedesktop.NetworkManager.Device")? - .build() - .await + match nm_proxy( + conn, + dev_path.clone(), + "org.freedesktop.NetworkManager.Device", + ) + .await { Ok(dev_proxy) => dev_proxy.get_property::("Interface").await.ok(), Err(_) => None, @@ -337,12 +338,12 @@ pub(crate) async fn list_vpn_connections(conn: &Connection) -> Result = match zbus::proxy::Builder::new(conn) - .destination("org.freedesktop.NetworkManager")? - .path(cpath.clone())? - .interface("org.freedesktop.NetworkManager.Settings.Connection")? - .build() - .await + let cproxy = match nm_proxy( + conn, + cpath.clone(), + "org.freedesktop.NetworkManager.Settings.Connection", + ) + .await { Ok(p) => p, Err(_) => continue, @@ -403,23 +404,23 @@ pub(crate) async fn forget_vpn(conn: &Connection, name: &str) -> Result<()> { let _ = disconnect_vpn(conn, name).await; - let settings: zbus::Proxy<'_> = zbus::proxy::Builder::new(conn) - .destination("org.freedesktop.NetworkManager")? - .path("/org/freedesktop/NetworkManager/Settings")? - .interface("org.freedesktop.NetworkManager.Settings")? - .build() - .await?; + let settings = nm_proxy( + conn, + "/org/freedesktop/NetworkManager/Settings", + "org.freedesktop.NetworkManager.Settings", + ) + .await?; let list_reply = settings.call_method("ListConnections", &()).await?; let conns: Vec = list_reply.body().deserialize()?; for cpath in conns { - let cproxy: zbus::Proxy<'_> = match zbus::proxy::Builder::new(conn) - .destination("org.freedesktop.NetworkManager")? - .path(cpath.clone())? - .interface("org.freedesktop.NetworkManager.Settings.Connection")? - .build() - .await + let cproxy = match nm_proxy( + conn, + cpath.clone(), + "org.freedesktop.NetworkManager.Settings.Connection", + ) + .await { Ok(p) => p, Err(_) => continue, @@ -469,12 +470,12 @@ pub(crate) async fn get_vpn_info(conn: &Connection, name: &str) -> Result = match zbus::proxy::Builder::new(conn) - .destination("org.freedesktop.NetworkManager")? - .path(ac_path.clone())? - .interface("org.freedesktop.NetworkManager.Connection.Active")? - .build() - .await + let ac_proxy = match nm_proxy( + conn, + ac_path.clone(), + "org.freedesktop.NetworkManager.Connection.Active", + ) + .await { Ok(p) => p, Err(_) => continue, @@ -488,12 +489,12 @@ pub(crate) async fn get_vpn_info(conn: &Connection, name: &str) -> Result continue, }; - let cproxy: zbus::Proxy<'_> = match zbus::proxy::Builder::new(conn) - .destination("org.freedesktop.NetworkManager")? - .path(conn_path)? - .interface("org.freedesktop.NetworkManager.Settings.Connection")? - .build() - .await + let cproxy = match nm_proxy( + conn, + conn_path, + "org.freedesktop.NetworkManager.Settings.Connection", + ) + .await { Ok(p) => p, Err(_) => continue, @@ -540,12 +541,12 @@ pub(crate) async fn get_vpn_info(conn: &Connection, name: &str) -> Result = ac_proxy.get_property("Devices").await?; let interface = if let Some(dev_path) = dev_paths.first() { - let dev_proxy: zbus::Proxy<'_> = zbus::proxy::Builder::new(conn) - .destination("org.freedesktop.NetworkManager")? - .path(dev_path.clone())? - .interface("org.freedesktop.NetworkManager.Device")? - .build() - .await?; + let dev_proxy = nm_proxy( + conn, + dev_path.clone(), + "org.freedesktop.NetworkManager.Device", + ) + .await?; Some(dev_proxy.get_property::("Interface").await?) } else { None @@ -574,12 +575,8 @@ pub(crate) async fn get_vpn_info(conn: &Connection, name: &str) -> Result = zbus::proxy::Builder::new(conn) - .destination("org.freedesktop.NetworkManager")? - .path(ip4_path)? - .interface("org.freedesktop.NetworkManager.IP4Config")? - .build() - .await?; + let ip4_proxy = + nm_proxy(conn, ip4_path, "org.freedesktop.NetworkManager.IP4Config").await?; let ip4_address = if let Ok(addr_array) = ip4_proxy .get_property::>>("AddressData") diff --git a/nmrs/src/util/utils.rs b/nmrs/src/util/utils.rs index 8095defe..858c4d8c 100644 --- a/nmrs/src/util/utils.rs +++ b/nmrs/src/util/utils.rs @@ -7,6 +7,7 @@ use log::warn; use std::borrow::Cow; use std::str; use zbus::Connection; +use zvariant::OwnedObjectPath; use crate::dbus::{NMAccessPointProxy, NMDeviceProxy, NMProxy, NMWirelessProxy}; use crate::types::constants::{device_type, frequency, signal_strength, wifi_mode}; @@ -143,6 +144,27 @@ where Ok(results) } +/// Helper to create a NetworkManager D-Bus proxy for a given path and interface. +/// +/// Returns a zbus Proxy instance for the specified path and interface. +pub(crate) async fn nm_proxy<'a, P>( + conn: &'a Connection, + path: P, + interface: &'a str, +) -> Result> +where + P: TryInto, + P::Error: Into, +{ + let owned_path = path.try_into().map_err(Into::into)?; + Ok(zbus::proxy::Builder::new(conn) + .destination("org.freedesktop.NetworkManager")? + .path(owned_path)? + .interface(interface)? + .build() + .await?) +} + /// Macro to convert Result to Option with error logging. /// Usage: `try_log!(result, "context message")?` #[macro_export]