Changes based on code review

rust/agama-server/src/network/adapter.rs

@@ -1,4 +1,4 @@
-use crate::network::model::NetworkStateItems;
+use crate::network::model::StateConfig;
 use crate::network::NetworkState;
 use agama_lib::error::ServiceError;
 use async_trait::async_trait;
@@ -17,10 +17,7 @@ pub enum NetworkAdapterError {
 /// A trait for the ability to read/write from/to a network service
 #[async_trait]
 pub trait Adapter {
-    async fn read(
-        &self,
-        items: Vec<NetworkStateItems>,
-    ) -> Result<NetworkState, NetworkAdapterError>;
+    async fn read(&self, config: StateConfig) -> Result<NetworkState, NetworkAdapterError>;
     async fn write(&self, network: &NetworkState) -> Result<(), NetworkAdapterError>;
 }
 

rust/agama-server/src/network/model.rs

@@ -20,11 +20,22 @@ use uuid::Uuid;
 use zbus::zvariant::Value;
 
 #[derive(PartialEq)]
-pub enum NetworkStateItems {
-    AccessPoints,
-    Devices,
-    Connections,
-    GeneralState,
+pub struct StateConfig {
+    pub access_points: bool,
+    pub devices: bool,
+    pub connections: bool,
+    pub general_state: bool,
+}
+
+impl Default for StateConfig {
+    fn default() -> Self {
+        Self {
+            access_points: true,
+            devices: true,
+            connections: true,
+            general_state: true,
+        }
+    }
 }
 
 #[derive(Default, Clone, Debug, utoipa::ToSchema)]

rust/agama-server/src/network/nm/adapter.rs

@@ -1,5 +1,5 @@
 use crate::network::{
-    model::{Connection, NetworkState, NetworkStateItems},
+    model::{Connection, NetworkState, StateConfig},
     nm::NetworkManagerClient,
     Adapter, NetworkAdapterError,
 };
@@ -31,68 +31,47 @@ impl<'a> NetworkManagerAdapter<'a> {
 
 #[async_trait]
 impl<'a> Adapter for NetworkManagerAdapter<'a> {
-    async fn read(
-        &self,
-        items: Vec<NetworkStateItems>,
-    ) -> Result<NetworkState, NetworkAdapterError> {
-        let items = if items.is_empty() {
-            vec![
-                NetworkStateItems::AccessPoints,
-                NetworkStateItems::Connections,
-                NetworkStateItems::Devices,
-                NetworkStateItems::GeneralState,
-            ]
-        } else {
-            items
-        };
-
+    async fn read(&self, config: StateConfig) -> Result<NetworkState, NetworkAdapterError> {
         let general_state = self
             .client
             .general_state()
             .await
             .map_err(NetworkAdapterError::Read)?;
 
-        let devices = if items.contains(&NetworkStateItems::Devices) {
-            self.client
+        let mut state = NetworkState::default();
+
+        if config.devices {
+            state.devices = self
+                .client
                 .devices()
                 .await
-                .map_err(NetworkAdapterError::Read)?
-        } else {
-            vec![]
-        };
+                .map_err(NetworkAdapterError::Read)?;
+        }
 
-        let connections = if items.contains(&NetworkStateItems::Connections) {
-            self.client
+        if config.connections {
+            state.connections = self
+                .client
                 .connections()
                 .await
-                .map_err(NetworkAdapterError::Read)?
-        } else {
-            vec![]
-        };
-
-        let access_points =
-            if items.contains(&NetworkStateItems::AccessPoints) && general_state.wireless_enabled {
-                if items.len() == 1 {
-                    self.client
-                        .request_scan()
-                        .await
-                        .map_err(NetworkAdapterError::Read)?;
-                    thread::sleep(time::Duration::from_secs(1));
-                };
+                .map_err(NetworkAdapterError::Read)?;
+        }
+
+        if config.access_points && general_state.wireless_enabled {
+            if !config.devices && !config.connections {
                 self.client
-                    .access_points()
+                    .request_scan()
                     .await
-                    .map_err(NetworkAdapterError::Read)?
-            } else {
-                vec![]
+                    .map_err(NetworkAdapterError::Read)?;
+                thread::sleep(time::Duration::from_secs(1));
             };
+            state.access_points = self
+                .client
+                .access_points()
+                .await
+                .map_err(NetworkAdapterError::Read)?;
+        }
 
-        Ok(NetworkState::new(
-            general_state,
-            access_points,
-            devices,
-            connections,
-        ))
+        Ok(state)
     }
 
     /// Writes the connections to NetworkManager.
@@ -103,7 +82,7 @@ impl<'a> Adapter for NetworkManagerAdapter<'a> {
     ///
     /// * `network`: network model.
     async fn write(&self, network: &NetworkState) -> Result<(), NetworkAdapterError> {
-        let old_state = self.read(vec![]).await?;
+        let old_state = self.read(StateConfig::default()).await?;
         let checkpoint = self
             .client
             .create_checkpoint()

rust/agama-server/src/network/system.rs

@@ -1,4 +1,4 @@
-use super::{error::NetworkStateError, model::NetworkStateItems, NetworkAdapterError};
+use super::{error::NetworkStateError, model::StateConfig, NetworkAdapterError};
 use crate::network::{dbus::Tree, model::Connection, Action, Adapter, NetworkState};
 use agama_lib::network::types::DeviceType;
 use std::{error::Error, sync::Arc};
@@ -37,7 +37,7 @@ impl<T: Adapter> NetworkSystem<T> {
     /// Writes the network configuration.
     pub async fn write(&mut self) -> Result<(), NetworkAdapterError> {
         self.adapter.write(&self.state).await?;
-        self.state = self.adapter.read(vec![]).await?;
+        self.state = self.adapter.read(StateConfig::default()).await?;
         Ok(())
     }
 
@@ -50,7 +50,7 @@ impl<T: Adapter> NetworkSystem<T> {
 
     /// Populates the D-Bus tree with the known devices and connections.
     pub async fn setup(&mut self) -> Result<(), Box<dyn Error>> {
-        self.state = self.adapter.read(vec![]).await?;
+        self.state = self.adapter.read(StateConfig::default()).await?;
         let mut tree = self.tree.lock().await;
         tree.set_connections(&mut self.state.connections).await?;
         tree.set_devices(&self.state.devices).await?;
@@ -78,7 +78,10 @@ impl<T: Adapter> NetworkSystem<T> {
             Action::RefreshScan(tx) => {
                 let state = self
                     .adapter
-                    .read(vec![NetworkStateItems::AccessPoints])
+                    .read(StateConfig {
+                        access_points: true,
+                        ..Default::default()
+                    })
                     .await?;
                 self.state.general_state = state.general_state;
                 self.state.access_points = state.access_points;

rust/agama-server/src/network/web.rs

@@ -9,9 +9,9 @@ use axum::{
     Json, Router,
 };
 
-use super::{error::NetworkStateError, model::GeneralState, Action};
+use super::{error::NetworkStateError, model::GeneralState, Action, Adapter};
 
-use crate::network::{model::Connection, model::Device, nm::NetworkManagerAdapter, NetworkSystem};
+use crate::network::{model::Connection, model::Device, NetworkSystem};
 use agama_lib::error::ServiceError;
 use agama_lib::network::settings::NetworkConnection;
 use uuid::Uuid;
@@ -26,9 +26,11 @@ pub enum NetworkError {
     UnknownConnection(String),
     #[error("Cannot translate: {0}")]
     CannotTranslate(#[from] Error),
+    #[error("Cannot update configuration: {0}")]
+    CannotUpdate(Uuid),
     #[error("Cannot apply configuration")]
     CannotApplyConfig,
-    #[error("Network states error: {0}")]
+    #[error("Network state error: {0}")]
     Error(#[from] NetworkStateError),
 }
 
@@ -49,10 +51,10 @@ struct NetworkState {
 /// Sets up and returns the axum service for the network module.
 ///
 /// * `dbus`: zbus Connection.
-pub async fn network_service(dbus: zbus::Connection) -> Result<Router, ServiceError> {
-    let adapter = NetworkManagerAdapter::from_system()
-        .await
-        .expect("Could not connect to NetworkManager to read the configuration.");
+pub async fn network_service<T: Adapter + std::marker::Send + 'static>(
+    dbus: zbus::Connection,
+    adapter: T,
+) -> Result<Router, ServiceError> {
     let mut network = NetworkSystem::new(dbus.clone(), adapter);
 
     let state = NetworkState {
@@ -69,7 +71,7 @@ pub async fn network_service(dbus: zbus::Connection) -> Result<Router, ServiceEr
     });
 
     Ok(Router::new()
-        .route("/state", get(general_state).post(update_general_state))
+        .route("/state", get(general_state).put(update_general_state))
         .route("/connections", get(connections).post(add_connection))
         .route(
             "/connections/:id",
@@ -93,7 +95,7 @@ async fn general_state(State(state): State<NetworkState>) -> Json<GeneralState>
     Json(state)
 }
 
-#[utoipa::path(post, path = "/network/state", responses(
+#[utoipa::path(put, path = "/network/state", responses(
   (status = 200, description = "Update general network config", body = GenereralState)
 ))]
 async fn update_general_state(
@@ -224,7 +226,7 @@ async fn update_connection(
 
     state
         .actions
-        .send(Action::UpdateConnection(Box::new(conn.clone())))
+        .send(Action::UpdateConnection(Box::new(conn)))
         .unwrap();
 
     Ok(Json(()))

rust/agama-server/src/web.rs

@@ -8,7 +8,7 @@ use crate::{
     error::Error,
     l10n::web::l10n_service,
     manager::web::{manager_service, manager_stream},
-    network::web::network_service,
+    network::{web::network_service, NetworkManagerAdapter},
     software::web::{software_service, software_stream},
     web::common::{issues_stream, progress_stream, service_status_stream},
 };
@@ -48,11 +48,15 @@ pub async fn service<P>(
 where
     P: AsRef<Path>,
 {
+    let network_adapter = NetworkManagerAdapter::from_system()
+        .await
+        .expect("Could not connect to NetworkManager to read the configuration");
+
     let router = MainServiceBuilder::new(events.clone(), web_ui_dir)
         .add_service("/l10n", l10n_service(events.clone()))
         .add_service("/manager", manager_service(dbus.clone()).await?)
         .add_service("/software", software_service(dbus.clone()).await?)
-        .add_service("/network", network_service(dbus).await?)
+        .add_service("/network", network_service(dbus, network_adapter).await?)
         .with_config(config)
         .build();
     Ok(router)

rust/agama-server/tests/network.rs

@@ -8,7 +8,7 @@ use agama_lib::network::{
 };
 use agama_server::network::{
     self,
-    model::{self, GeneralState, Ipv4Method, Ipv6Method, NetworkStateItems},
+    model::{self, GeneralState, Ipv4Method, Ipv6Method, StateConfig},
     Adapter, NetworkAdapterError, NetworkService, NetworkState,
 };
 use async_trait::async_trait;
@@ -21,10 +21,7 @@ pub struct NetworkTestAdapter(network::NetworkState);
 
 #[async_trait]
 impl Adapter for NetworkTestAdapter {
-    async fn read(
-        &self,
-        _: Vec<NetworkStateItems>,
-    ) -> Result<network::NetworkState, NetworkAdapterError> {
+    async fn read(&self, _: StateConfig) -> Result<network::NetworkState, NetworkAdapterError> {
         Ok(self.0.clone())
     }
 
@@ -37,11 +34,7 @@ impl Adapter for NetworkTestAdapter {
 async fn test_read_connections() -> Result<(), Box<dyn Error>> {
     let mut server = DBusServer::new().start().await?;
 
-    let general_state = GeneralState {
-        wireless_enabled: false,
-        connectivity: true,
-        networking_enabled: true,
-    };
+    let general_state = GeneralState::default();
 
     let device = model::Device {
         name: String::from("eth0"),
@@ -152,12 +145,7 @@ async fn test_add_bond_connection() -> Result<(), Box<dyn Error>> {
 async fn test_update_connection() -> Result<(), Box<dyn Error>> {
     let mut server = DBusServer::new().start().await?;
 
-    let general_state = GeneralState {
-        wireless_enabled: false,
-        connectivity: true,
-        networking_enabled: true,
-    };
-
+    let general_state = GeneralState::default();
     let device = model::Device {
         name: String::from("eth0"),
         type_: DeviceType::Ethernet,