Changing the network model (#924)

## Problem

Agama's network model is heavily inspired by
[nmstate](https://github.com/nmstate/nmstate/blob/c9fd1e80d3c87e1f844edcd86e9d36ae499ce717/rust/src/lib/ifaces/ethernet.rs#L64).
Connections are represented [by an
enum](https://github.com/openSUSE/agama/blob/2bc452b39e4460122ab52c5a72ee0d1dfe154cec/rust/agama-dbus-server/src/network/model.rs#L220)
and each variant contains a struct which depends on the type (e.g., the
[WirelessConnection](https://github.com/openSUSE/agama/blob/2bc452b39e4460122ab52c5a72ee0d1dfe154cec/rust/agama-dbus-server/src/network/model.rs#L542)
struct). Those structs have a `base` field that points to a
[BaseConnection
struct](https://github.com/openSUSE/agama/blob/master/rust/agama-dbus-server/src/network/model.rs#L328)
which holds the common fields for all connection types.

And that's perfectly fine. However, we decided to introduce some methods
to hide the fact that the common information lives in a separate
`struct`. We could say it is for encapsulation reasons, but I don't know
whether it is a good idea. Even we created some *getters* and *setters*
for those fields, adding quite some
[boilerplate](https://github.com/openSUSE/agama/blob/master/rust/agama-dbus-server/src/network/model.rs#L264-L299)
[code](https://github.com/openSUSE/agama/blob/master/rust/agama-dbus-server/src/network/model.rs#L318-L324)

Additionally, until we introduced the `ConnectionBuilder` (in #885),
creating a new connection with some custom data (like an id and a UUID)
was not straightforward because you needed to make the `BaseConnection`
first (see [this
example](https://github.com/openSUSE/agama/blob/2bc452b39e4460122ab52c5a72ee0d1dfe154cec/rust/agama-dbus-server/src/network/model.rs#L136-L141).

## Proposal

Although I am not 100% sure whether it is a better idea, I am exploring
the possibility of using a simpler layout to represent a connection. In
this case, we would use a single `struct` containing the common fields
plus a `config` field for specific stuff. That field contains an `enum`
which represents each kind of connection.

```rust
/// Represents an availble network connection
#[derive(Debug, Default, Clone)]
pub struct Connection {
    pub id: String,
    pub uuid: Uuid,
    pub mac_address: MacAddress,
    pub ip_config: IpConfig,
    pub status: Status,
    pub interface: String,
    pub match_config: MatchConfig,
    pub config: ConnectionConfig,
}

#[derive(Default, Debug, PartialEq, Clone)]
pub enum ConnectionConfig {
    #[default]
    Ethernet,
    Wireless(WirelessConfig),
    Loopback,
    Dummy,
}
```

And not getters/setters as they are not needed at all.

## Testing

- Adapted the unit tested.

rust/agama-dbus-server/src/network/dbus/interfaces.rs

@@ -8,8 +8,8 @@ use crate::network::{
     action::Action,
     error::NetworkStateError,
     model::{
-        BondConnection, Connection as NetworkConnection, Device as NetworkDevice, MacAddress,
-        WirelessConnection,
+        BondConfig, Connection as NetworkConnection, ConnectionConfig, Device as NetworkDevice,
+        MacAddress, SecurityProtocol, WirelessConfig, WirelessMode,
     },
 };
 
@@ -229,7 +229,7 @@ impl Connection {
     /// now), it uses the original ID but appending a number in case the ID is duplicated.
     #[dbus_interface(property)]
     pub async fn id(&self) -> String {
-        self.get_connection().await.id().to_string()
+        self.get_connection().await.id.to_string()
     }
 
     /// Connection UUID.
@@ -238,44 +238,41 @@ impl Connection {
     /// backend.
     #[dbus_interface(property)]
     pub async fn uuid(&self) -> String {
-        self.get_connection().await.uuid().to_string()
+        self.get_connection().await.uuid.to_string()
     }
 
     #[dbus_interface(property)]
     pub async fn controller(&self) -> String {
         let connection = self.get_connection().await;
-        match connection.controller() {
+        match connection.controller {
             Some(uuid) => uuid.to_string(),
             None => "".to_string(),
         }
     }
 
     #[dbus_interface(property)]
     pub async fn interface(&self) -> String {
-        self.get_connection()
-            .await
-            .interface()
-            .unwrap_or("")
-            .to_string()
+        let connection = self.get_connection().await;
+        connection.interface.as_deref().unwrap_or("").to_string()
     }
 
     #[dbus_interface(property)]
     pub async fn set_interface(&mut self, name: &str) -> zbus::fdo::Result<()> {
         let mut connection = self.get_connection().await;
-        connection.set_interface(name);
+        connection.interface = Some(name.to_string());
         self.update_connection(connection).await
     }
 
     /// Custom mac-address
     #[dbus_interface(property)]
     pub async fn mac_address(&self) -> String {
-        self.get_connection().await.mac_address()
+        self.get_connection().await.mac_address.to_string()
     }
 
     #[dbus_interface(property)]
     pub async fn set_mac_address(&mut self, mac_address: &str) -> zbus::fdo::Result<()> {
         let mut connection = self.get_connection().await;
-        connection.set_mac_address(MacAddress::from_str(mac_address)?);
+        connection.mac_address = MacAddress::from_str(mac_address)?;
         self.update_connection(connection).await
     }
 }
@@ -339,28 +336,41 @@ impl Bond {
         }
     }
 
-    /// Gets the bond connection.
-    ///
-    /// Beware that it crashes when it is not a bond connection.
-    async fn get_bond(&self) -> BondConnection {
+    /// Gets the connection.
+    async fn get_connection(&self) -> Result<NetworkConnection, NetworkStateError> {
         let actions = self.actions.lock().await;
         let (tx, rx) = oneshot::channel();
         actions.send(Action::GetConnection(self.uuid, tx)).unwrap();
-        let connection = rx.await.unwrap();
+        rx.await
+            .unwrap()
+            .ok_or(NetworkStateError::UnknownConnection(self.uuid.to_string()))
+    }
 
-        match connection {
-            Some(NetworkConnection::Bond(config)) => config,
+    /// Gets the bonding configuration.
+    pub async fn get_config(&self) -> Result<BondConfig, NetworkStateError> {
+        let connection = self.get_connection().await?;
+        match connection.config {
+            ConnectionConfig::Bond(bond) => Ok(bond),
             _ => panic!("Not a bond connection. This is most probably a bug."),
         }
     }
 
-    /// Updates the connection data in the NetworkSystem.
+    /// Updates the bond configuration.
     ///
-    /// * `connection`: Updated connection.
-    async fn update_connection<'a>(&self, connection: BondConnection) -> zbus::fdo::Result<()> {
+    /// * `func`: function to update the configuration.
+    pub async fn update_config<F>(&self, func: F) -> Result<(), NetworkStateError>
+    where
+        F: FnOnce(&mut BondConfig),
+    {
+        let mut connection = self.get_connection().await?;
+        match &mut connection.config {
+            ConnectionConfig::Bond(bond) => func(bond),
+            _ => panic!("Not a bond connection. This is most probably a bug."),
+        }
         let actions = self.actions.lock().await;
-        let connection = NetworkConnection::Bond(connection.clone());
-        actions.send(Action::UpdateConnection(connection)).unwrap();
+        actions
+            .send(Action::UpdateConnection(connection.clone()))
+            .unwrap();
         Ok(())
     }
 }
@@ -369,30 +379,30 @@ impl Bond {
 impl Bond {
     /// Bonding mode.
     #[dbus_interface(property)]
-    pub async fn mode(&self) -> String {
-        let connection = self.get_bond().await;
-        connection.bond.mode.to_string()
+    pub async fn mode(&self) -> zbus::fdo::Result<String> {
+        let config = self.get_config().await?;
+        Ok(config.mode.to_string())
     }
 
     #[dbus_interface(property)]
     pub async fn set_mode(&mut self, mode: &str) -> zbus::fdo::Result<()> {
-        let mut connection = self.get_bond().await;
-        connection.set_mode(mode.try_into()?);
-        self.update_connection(connection).await
+        let mode = mode.try_into()?;
+        self.update_config(|c| c.mode = mode).await?;
+        Ok(())
     }
 
     /// List of bonding options.
     #[dbus_interface(property)]
-    pub async fn options(&self) -> String {
-        let connection = self.get_bond().await;
-        connection.bond.options.to_string()
+    pub async fn options(&self) -> zbus::fdo::Result<String> {
+        let config = self.get_config().await?;
+        Ok(config.options.to_string())
     }
 
     #[dbus_interface(property)]
     pub async fn set_options(&mut self, opts: &str) -> zbus::fdo::Result<()> {
-        let mut connection = self.get_bond().await;
-        connection.set_options(opts.try_into()?);
-        self.update_connection(connection).await
+        let opts = opts.try_into()?;
+        self.update_config(|c| c.options = opts).await?;
+        Ok(())
     }
 
     /// List of bond ports.
@@ -427,58 +437,54 @@ impl Match {
     #[dbus_interface(property)]
     pub async fn driver(&self) -> Vec<String> {
         let connection = self.get_connection().await;
-        connection.match_config().driver.clone()
+        connection.match_config.driver.clone()
     }
 
     #[dbus_interface(property)]
     pub async fn set_driver(&mut self, driver: Vec<String>) -> zbus::fdo::Result<()> {
         let mut connection = self.get_connection().await;
-        let config = connection.match_config_mut();
-        config.driver = driver;
+        connection.match_config.driver = driver;
         self.update_connection(connection).await
     }
 
     /// List of paths to match agains the ID_PATH udev property of devices.
     #[dbus_interface(property)]
     pub async fn path(&self) -> Vec<String> {
         let connection = self.get_connection().await;
-        connection.match_config().path.clone()
+        connection.match_config.path.clone()
     }
 
     #[dbus_interface(property)]
     pub async fn set_path(&mut self, path: Vec<String>) -> zbus::fdo::Result<()> {
         let mut connection = self.get_connection().await;
-        let config = connection.match_config_mut();
-        config.path = path;
+        connection.match_config.path = path;
         self.update_connection(connection).await
     }
     /// List of interface names to match.
     #[dbus_interface(property)]
     pub async fn interface(&self) -> Vec<String> {
         let connection = self.get_connection().await;
-        connection.match_config().interface.clone()
+        connection.match_config.interface.clone()
     }
 
     #[dbus_interface(property)]
     pub async fn set_interface(&mut self, interface: Vec<String>) -> zbus::fdo::Result<()> {
         let mut connection = self.get_connection().await;
-        let config = connection.match_config_mut();
-        config.interface = interface;
+        connection.match_config.interface = interface;
         self.update_connection(connection).await
     }
 
     /// List of kernel options to match.
     #[dbus_interface(property)]
     pub async fn kernel(&self) -> Vec<String> {
         let connection = self.get_connection().await;
-        connection.match_config().kernel.clone()
+        connection.match_config.kernel.clone()
     }
 
     #[dbus_interface(property)]
     pub async fn set_kernel(&mut self, kernel: Vec<String>) -> zbus::fdo::Result<()> {
         let mut connection = self.get_connection().await;
-        let config = connection.match_config_mut();
-        config.kernel = kernel;
+        connection.match_config.kernel = kernel;
         self.update_connection(connection).await
     }
 }
@@ -507,23 +513,29 @@ impl Wireless {
     /// Gets the wireless connection.
     ///
     /// Beware that it crashes when it is not a wireless connection.
-    async fn get_wireless(&self) -> MappedMutexGuard<WirelessConnection> {
-        MutexGuard::map(self.connection.lock().await, |c| match c {
-            NetworkConnection::Wireless(config) => config,
-            _ => panic!("Not a wireless network. This is most probably a bug."),
+    async fn get_wireless(&self) -> MappedMutexGuard<WirelessConfig> {
+        MutexGuard::map(self.connection.lock().await, |c| match &mut c.config {
+            ConnectionConfig::Wireless(config) => config,
+            _ => panic!("Not a wireless connection. This is most probably a bug."),
         })
     }
 
-    /// Updates the connection data in the NetworkSystem.
+    /// Updates the wireless configuration.
     ///
-    /// * `connection`: Updated connection.
-    async fn update_connection<'a>(
-        &self,
-        connection: MappedMutexGuard<'a, WirelessConnection>,
-    ) -> zbus::fdo::Result<()> {
+    /// * `func`: function to update the configuration.
+    pub async fn update_config<F>(&self, func: F) -> Result<(), NetworkStateError>
+    where
+        F: FnOnce(&mut WirelessConfig),
+    {
+        let mut connection = self.connection.lock().await;
+        match &mut connection.config {
+            ConnectionConfig::Wireless(wireless) => func(wireless),
+            _ => panic!("Not a wireless connection. This is most probably a bug."),
+        }
         let actions = self.actions.lock().await;
-        let connection = NetworkConnection::Wireless(connection.clone());
-        actions.send(Action::UpdateConnection(connection)).unwrap();
+        actions
+            .send(Action::UpdateConnection(connection.clone()))
+            .unwrap();
         Ok(())
     }
 }
@@ -533,15 +545,14 @@ impl Wireless {
     /// Network SSID.
     #[dbus_interface(property, name = "SSID")]
     pub async fn ssid(&self) -> Vec<u8> {
-        let connection = self.get_wireless().await;
-        connection.wireless.ssid.clone().into()
+        let wireless = self.get_wireless().await;
+        wireless.ssid.clone().into()
     }
 
     #[dbus_interface(property, name = "SSID")]
     pub async fn set_ssid(&mut self, ssid: Vec<u8>) -> zbus::fdo::Result<()> {
-        let mut connection = self.get_wireless().await;
-        connection.wireless.ssid = SSID(ssid);
-        self.update_connection(connection).await
+        self.update_config(|c| c.ssid = SSID(ssid)).await?;
+        Ok(())
     }
 
     /// Wireless connection mode.
@@ -551,37 +562,35 @@ impl Wireless {
     /// See [crate::network::model::WirelessMode].
     #[dbus_interface(property)]
     pub async fn mode(&self) -> String {
-        let connection = self.get_wireless().await;
-        connection.wireless.mode.to_string()
+        let wireless = self.get_wireless().await;
+        wireless.mode.to_string()
     }
 
     #[dbus_interface(property)]
     pub async fn set_mode(&mut self, mode: &str) -> zbus::fdo::Result<()> {
-        let mut connection = self.get_wireless().await;
-        connection.wireless.mode = mode.try_into()?;
-        self.update_connection(connection).await
+        let mode: WirelessMode = mode.try_into()?;
+        self.update_config(|c| c.mode = mode).await?;
+        Ok(())
     }
 
     /// Password to connect to the wireless network.
     #[dbus_interface(property)]
     pub async fn password(&self) -> String {
-        let connection = self.get_wireless().await;
-        connection
-            .wireless
-            .password
-            .clone()
-            .unwrap_or("".to_string())
+        let wireless = self.get_wireless().await;
+        wireless.password.clone().unwrap_or("".to_string())
     }
 
     #[dbus_interface(property)]
     pub async fn set_password(&mut self, password: String) -> zbus::fdo::Result<()> {
-        let mut connection = self.get_wireless().await;
-        connection.wireless.password = if password.is_empty() {
-            None
-        } else {
-            Some(password)
-        };
-        self.update_connection(connection).await
+        self.update_config(|c| {
+            c.password = if password.is_empty() {
+                None
+            } else {
+                Some(password)
+            };
+        })
+        .await?;
+        Ok(())
     }
 
     /// Wireless security protocol.
@@ -592,16 +601,16 @@ impl Wireless {
     /// See [crate::network::model::SecurityProtocol].
     #[dbus_interface(property)]
     pub async fn security(&self) -> String {
-        let connection = self.get_wireless().await;
-        connection.wireless.security.to_string()
+        let wireless = self.get_wireless().await;
+        wireless.security.to_string()
     }
 
     #[dbus_interface(property)]
     pub async fn set_security(&mut self, security: &str) -> zbus::fdo::Result<()> {
-        let mut connection = self.get_wireless().await;
-        connection.wireless.security = security
+        let security: SecurityProtocol = security
             .try_into()
             .map_err(|_| NetworkStateError::InvalidSecurityProtocol(security.to_string()))?;
-        self.update_connection(connection).await
+        self.update_config(|c| c.security = security).await?;
+        Ok(())
     }
 }

rust/agama-dbus-server/src/network/dbus/interfaces/ip_config.rs

@@ -58,7 +58,7 @@ impl Ip {
 impl Ip {
     /// Returns the IpConfig struct.
     async fn get_ip_config(&self) -> MappedMutexGuard<IpConfig> {
-        MutexGuard::map(self.get_connection().await, |c| c.ip_config_mut())
+        MutexGuard::map(self.get_connection().await, |c| &mut c.ip_config)
     }
 
     /// Updates the IpConfig struct.
@@ -69,7 +69,7 @@ impl Ip {
         F: Fn(&mut IpConfig),
     {
         let mut connection = self.get_connection().await;
-        func(connection.ip_config_mut());
+        func(&mut connection.ip_config);
         self.update_connection(connection).await?;
         Ok(())
     }