From eee87a5615104192cbdb45f6c5d7c4f6919d9e23 Mon Sep 17 00:00:00 2001
From: Knut Anderssen <kanderssen@suse.com>
Date: Wed, 6 Mar 2024 19:56:41 +0000
Subject: [PATCH] Changes based on code review.

---
 rust/agama-server/src/network/action.rs       |  5 +--
 .../network/dbus/interfaces/connections.rs    |  2 +-
 rust/agama-server/src/network/web.rs          | 33 +++++--------------
 rust/agama-server/src/web.rs                  |  4 +--
 4 files changed, 15 insertions(+), 29 deletions(-)

diff --git a/rust/agama-server/src/network/action.rs b/rust/agama-server/src/network/action.rs
index 5a4042437..4f05960b8 100644
--- a/rust/agama-server/src/network/action.rs
+++ b/rust/agama-server/src/network/action.rs
@@ -21,7 +21,7 @@ pub enum Action {
         DeviceType,
         Responder<Result<OwnedObjectPath, NetworkStateError>>,
     ),
-    /// Gets a connection
+    /// Gets a connection by its Uuid
     GetConnection(Uuid, Responder<Option<Connection>>),
     /// Gets a connection
     GetConnections(Responder<Vec<Connection>>),
@@ -36,8 +36,9 @@ pub enum Action {
         Uuid,
         Responder<Result<ControllerConnection, NetworkStateError>>,
     ),
-    /// Gets a device
+    /// Gets a device by its name
     GetDevice(String, Responder<Option<Device>>),
+    /// Gets all the existent devices
     GetDevices(Responder<Vec<Device>>),
     /// Gets a device path
     GetDevicePath(String, Responder<Option<OwnedObjectPath>>),
diff --git a/rust/agama-server/src/network/dbus/interfaces/connections.rs b/rust/agama-server/src/network/dbus/interfaces/connections.rs
index 885aefb04..aca6bf50a 100644
--- a/rust/agama-server/src/network/dbus/interfaces/connections.rs
+++ b/rust/agama-server/src/network/dbus/interfaces/connections.rs
@@ -21,7 +21,7 @@ pub struct Connections {
 impl Connections {
     /// Creates a Connections interface object.
     ///
-    /// * `objects`: Objects paths registry.
+    /// * `actions`: sending-half of a channel to send actions.
     pub fn new(actions: UnboundedSender<Action>) -> Self {
         Self {
             actions: Arc::new(Mutex::new(actions)),
diff --git a/rust/agama-server/src/network/web.rs b/rust/agama-server/src/network/web.rs
index e63134a38..7a42977b3 100644
--- a/rust/agama-server/src/network/web.rs
+++ b/rust/agama-server/src/network/web.rs
@@ -1,6 +1,6 @@
 //! This module implements the web API for the network module.
 
-use crate::{error::Error, web::EventsSender};
+use crate::error::Error;
 use axum::{
     extract::State,
     http::StatusCode,
@@ -12,7 +12,7 @@ use axum::{
 use super::Action;
 
 use crate::network::{model::Connection, model::Device, nm::NetworkManagerAdapter, NetworkSystem};
-use agama_lib::connection;
+use agama_lib::error::ServiceError;
 
 use serde_json::json;
 use thiserror::Error;
@@ -38,22 +38,19 @@ impl IntoResponse for NetworkError {
 #[derive(Clone)]
 struct NetworkState {
     actions: UnboundedSender<Action>,
-    events: EventsSender,
 }
 
 /// Sets up and returns the axum service for the network module.
 ///
-/// * `events`: channel to send the events to the main service.
-pub async fn network_service(events: EventsSender) -> Router {
+/// * `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.");
-    let connection = connection().await.unwrap();
-    let mut network = NetworkSystem::new(connection.clone(), adapter);
+    let mut network = NetworkSystem::new(dbus.clone(), adapter);
 
     let state = NetworkState {
         actions: network.actions_tx(),
-        events,
     };
 
     tokio::spawn(async move {
@@ -65,10 +62,10 @@ pub async fn network_service(events: EventsSender) -> Router {
         network.listen().await;
     });
 
-    Router::new()
+    Ok(Router::new()
         .route("/connections", get(connections))
         .route("/devices", get(devices))
-        .with_state(state)
+        .with_state(state))
 }
 
 #[utoipa::path(get, path = "/network/devices", responses(
@@ -77,14 +74,8 @@ pub async fn network_service(events: EventsSender) -> Router {
 async fn devices(State(state): State<NetworkState>) -> Json<Vec<Device>> {
     let (tx, rx) = oneshot::channel();
     state.actions.send(Action::GetDevices(tx)).unwrap();
-    let result = rx.await.unwrap();
-    let mut devices = vec![];
 
-    for device in result {
-        devices.push(device)
-    }
-
-    Json(devices)
+    Json(rx.await.unwrap())
 }
 
 #[utoipa::path(get, path = "/network/connections", responses(
@@ -93,12 +84,6 @@ async fn devices(State(state): State<NetworkState>) -> Json<Vec<Device>> {
 async fn connections(State(state): State<NetworkState>) -> Json<Vec<Connection>> {
     let (tx, rx) = oneshot::channel();
     state.actions.send(Action::GetConnections(tx)).unwrap();
-    let result = rx.await.unwrap();
-    let mut connections = vec![];
-
-    for connection in result {
-        connections.push(connection)
-    }
 
-    Json(connections)
+    Json(rx.await.unwrap())
 }
diff --git a/rust/agama-server/src/web.rs b/rust/agama-server/src/web.rs
index 646e03b32..b0bf23233 100644
--- a/rust/agama-server/src/web.rs
+++ b/rust/agama-server/src/web.rs
@@ -49,8 +49,8 @@ where
 {
     let router = MainServiceBuilder::new(events.clone(), web_ui_dir)
         .add_service("/l10n", l10n_service(events.clone()))
-        .add_service("/software", software_service(dbus).await?)
-        .add_service("/network", network_service(events).await)
+        .add_service("/software", software_service(dbus.clone()).await?)
+        .add_service("/network", network_service(dbus).await?)
         .with_config(config)
         .build();
     Ok(router)