agama config load/store for "storage" uses the HTTP API (#1600)

## Problem

- https://trello.com/c/hvPtBtMD

To recap: 
> When moving to the new HTTP-based architecture, we took some
shortcuts. One of them was not using HTTP clients in the command-line
interface.

## Solution

- Add `StorageHTTPClient`
- This 🛑 ends D-Bus 🛑 usage from `SettingsStore`
✔️

## Testing

- Added a new unit test
- Tested manually: note that it is easy to pass invalid Storage config
in this way which will confuse the UI to the point of not rendering the
storage parts at all.


## Screenshots

No

rust/agama-cli/src/config.rs

@@ -5,9 +5,7 @@ use std::{
 };
 
 use crate::show_progress;
-use agama_lib::{
-    auth::AuthToken, connection, install_settings::InstallSettings, Store as SettingsStore,
-};
+use agama_lib::{auth::AuthToken, install_settings::InstallSettings, Store as SettingsStore};
 use anyhow::anyhow;
 use clap::Subcommand;
 use std::io::Write;
@@ -48,7 +46,7 @@ pub async fn run(subcommand: ConfigCommands) -> anyhow::Result<()> {
     };
 
     let client = agama_lib::http_client(token.as_str())?;
-    let store = SettingsStore::new(connection().await?, client).await?;
+    let store = SettingsStore::new(client).await?;
 
     match subcommand {
         ConfigCommands::Show => {

rust/agama-cli/src/profile.rs

@@ -1,6 +1,5 @@
 use agama_lib::{
     auth::AuthToken,
-    connection,
     install_settings::InstallSettings,
     profile::{AutoyastProfile, ProfileEvaluator, ProfileValidator, ValidationResult},
     Store as SettingsStore,
@@ -149,7 +148,7 @@ async fn import(url_string: String, dir: Option<PathBuf>) -> anyhow::Result<()>
 async fn store_settings<P: AsRef<Path>>(path: P) -> anyhow::Result<()> {
     let token = AuthToken::find().context("You are not logged in")?;
     let client = agama_lib::http_client(token.as_str())?;
-    let store = SettingsStore::new(connection().await?, client).await?;
+    let store = SettingsStore::new(client).await?;
     let settings = InstallSettings::from_file(&path)?;
     store.store(&settings).await?;
     Ok(())

rust/agama-lib/src/storage.rs

@@ -1,6 +1,7 @@
 //! Implements support for handling the storage settings
 
 pub mod client;
+pub mod http_client;
 pub mod model;
 pub mod proxies;
 mod settings;

rust/agama-lib/src/storage/http_client.rs

@@ -0,0 +1,28 @@
+//! Implements a client to access Agama's storage service.
+use crate::base_http_client::BaseHTTPClient;
+use crate::storage::StorageSettings;
+use crate::ServiceError;
+
+pub struct StorageHTTPClient {
+    client: BaseHTTPClient,
+}
+
+impl StorageHTTPClient {
+    pub fn new() -> Result<Self, ServiceError> {
+        Ok(Self {
+            client: BaseHTTPClient::new()?,
+        })
+    }
+
+    pub fn new_with_base(base: BaseHTTPClient) -> Self {
+        Self { client: base }
+    }
+
+    pub async fn get_config(&self) -> Result<StorageSettings, ServiceError> {
+        self.client.get("/storage/config").await
+    }
+
+    pub async fn set_config(&self, config: &StorageSettings) -> Result<(), ServiceError> {
+        self.client.put_void("/storage/config", config).await
+    }
+}

rust/agama-lib/src/storage/store.rs

@@ -1,27 +1,102 @@
 //! Implements the store for the storage settings.
 
-use super::{StorageClient, StorageSettings};
+use super::StorageSettings;
 use crate::error::ServiceError;
-use zbus::Connection;
+use crate::storage::http_client::StorageHTTPClient;
 
-/// Loads and stores the storage settings from/to the D-Bus service.
-pub struct StorageStore<'a> {
-    storage_client: StorageClient<'a>,
+/// Loads and stores the storage settings from/to the HTTP service.
+pub struct StorageStore {
+    storage_client: StorageHTTPClient,
 }
 
-impl<'a> StorageStore<'a> {
-    pub async fn new(connection: Connection) -> Result<StorageStore<'a>, ServiceError> {
+impl StorageStore {
+    pub fn new() -> Result<StorageStore, ServiceError> {
         Ok(Self {
-            storage_client: StorageClient::new(connection).await?,
+            storage_client: StorageHTTPClient::new()?,
         })
     }
 
     pub async fn load(&self) -> Result<StorageSettings, ServiceError> {
         Ok(self.storage_client.get_config().await?)
     }
 
-    pub async fn store(&self, settings: StorageSettings) -> Result<(), ServiceError> {
+    pub async fn store(&self, settings: &StorageSettings) -> Result<(), ServiceError> {
         self.storage_client.set_config(settings).await?;
         Ok(())
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use crate::base_http_client::BaseHTTPClient;
+    use httpmock::prelude::*;
+    use std::error::Error;
+    use tokio::test; // without this, "error: async functions cannot be used for tests"
+
+    fn storage_store(mock_server_url: String) -> StorageStore {
+        let mut bhc = BaseHTTPClient::default();
+        bhc.base_url = mock_server_url;
+        let client = StorageHTTPClient::new_with_base(bhc);
+        StorageStore {
+            storage_client: client,
+        }
+    }
+
+    #[test]
+    async fn test_getting_storage() -> Result<(), Box<dyn Error>> {
+        let server = MockServer::start();
+        let storage_mock = server.mock(|when, then| {
+            when.method(GET).path("/api/storage/config");
+            then.status(200)
+                .header("content-type", "application/json")
+                .body(
+                    r#"{
+                    "storage": { "some": "stuff" }
+                }"#,
+                );
+        });
+        let url = server.url("/api");
+
+        let store = storage_store(url);
+        let settings = store.load().await?;
+
+        // main assertion
+        assert_eq!(settings.storage.unwrap().get(), r#"{ "some": "stuff" }"#);
+        assert!(settings.storage_autoyast.is_none());
+
+        // Ensure the specified mock was called exactly one time (or fail with a detailed error description).
+        storage_mock.assert();
+        Ok(())
+    }
+
+    #[test]
+    async fn test_setting_storage_ok() -> Result<(), Box<dyn Error>> {
+        let server = MockServer::start();
+        let storage_mock = server.mock(|when, then| {
+            when.method(PUT)
+                .path("/api/storage/config")
+                .header("content-type", "application/json")
+                .body(r#"{"legacyAutoyastStorage":{ "some" : "data" }}"#);
+            then.status(200);
+        });
+        let url = server.url("/api");
+
+        let store = storage_store(url);
+        let boxed_raw_value =
+            serde_json::value::RawValue::from_string(r#"{ "some" : "data" }"#.to_owned())?;
+        let settings = StorageSettings {
+            storage: None,
+            storage_autoyast: Some(boxed_raw_value),
+        };
+
+        let result = store.store(&settings).await;
+
+        // main assertion
+        result?;
+
+        // Ensure the specified mock was called exactly one time (or fail with a detailed error description).
+        storage_mock.assert();
+        Ok(())
+    }
+}

rust/agama-lib/src/store.rs

@@ -7,35 +7,31 @@ use crate::{
     localization::LocalizationStore, network::NetworkStore, product::ProductStore,
     software::SoftwareStore, storage::StorageStore, users::UsersStore,
 };
-use zbus::Connection;
 
 /// Struct that loads/stores the settings from/to the D-Bus services.
 ///
 /// It is composed by a set of "stores" that are able to load/store the
 /// settings for each service.
 ///
 /// This struct uses the default connection built by [connection function](super::connection).
-pub struct Store<'a> {
+pub struct Store {
     users: UsersStore,
     network: NetworkStore,
     product: ProductStore,
     software: SoftwareStore,
-    storage: StorageStore<'a>,
+    storage: StorageStore,
     localization: LocalizationStore,
 }
 
-impl<'a> Store<'a> {
-    pub async fn new(
-        connection: Connection,
-        http_client: reqwest::Client,
-    ) -> Result<Store<'a>, ServiceError> {
+impl Store {
+    pub async fn new(http_client: reqwest::Client) -> Result<Store, ServiceError> {
         Ok(Self {
             localization: LocalizationStore::new()?,
             users: UsersStore::new()?,
             network: NetworkStore::new(http_client).await?,
             product: ProductStore::new()?,
             software: SoftwareStore::new()?,
-            storage: StorageStore::new(connection).await?,
+            storage: StorageStore::new()?,
         })
     }
 
@@ -77,7 +73,7 @@ impl<'a> Store<'a> {
             self.users.store(user).await?;
         }
         if settings.storage.is_some() || settings.storage_autoyast.is_some() {
-            self.storage.store(settings.into()).await?
+            self.storage.store(&settings.into()).await?
         }
         Ok(())
     }

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

@@ -10,12 +10,12 @@ use agama_lib::{
     storage::{
         model::{Action, Device, DeviceSid, ProposalSettings, ProposalSettingsPatch, Volume},
         proxies::Storage1Proxy,
-        StorageClient,
+        StorageClient, StorageSettings,
     },
 };
 use axum::{
     extract::{Query, State},
-    routing::{get, post},
+    routing::{get, post, put},
     Json, Router,
 };
 use serde::{Deserialize, Serialize};
@@ -71,7 +71,7 @@ struct StorageState<'a> {
     client: StorageClient<'a>,
 }
 
-/// Sets up and returns the axum service for the software module.
+/// Sets up and returns the axum service for the storage module.
 pub async fn storage_service(dbus: zbus::Connection) -> Result<Router, ServiceError> {
     const DBUS_SERVICE: &str = "org.opensuse.Agama.Storage1";
     const DBUS_PATH: &str = "/org/opensuse/Agama/Storage1";
@@ -87,6 +87,7 @@ pub async fn storage_service(dbus: zbus::Connection) -> Result<Router, ServiceEr
     let client = StorageClient::new(dbus.clone()).await?;
     let state = StorageState { client };
     let router = Router::new()
+        .route("/config", put(set_config).get(get_config))
         .route("/probe", post(probe))
         .route("/devices/dirty", get(devices_dirty))
         .route("/devices/system", get(system_devices))
@@ -109,6 +110,51 @@ pub async fn storage_service(dbus: zbus::Connection) -> Result<Router, ServiceEr
     Ok(router)
 }
 
+/// Returns the storage configuration.
+///
+/// * `state` : service state.
+#[utoipa::path(
+    get,
+    path = "/config",
+    context_path = "/api/storage",
+    operation_id = "get_storage_config",
+    responses(
+        (status = 200, description = "storage configuration", body = StorageSettings),
+        (status = 400, description = "The D-Bus service could not perform the action")
+    )
+)]
+async fn get_config(State(state): State<StorageState<'_>>) -> Result<Json<StorageSettings>, Error> {
+    // StorageSettings is just a wrapper over serde_json::value::RawValue
+    let settings = state.client.get_config().await.map_err(Error::Service)?;
+    Ok(Json(settings))
+}
+
+/// Sets the storage configuration.
+///
+/// * `state`: service state.
+/// * `config`: storage configuration.
+#[utoipa::path(
+    put,
+    path = "/config",
+    context_path = "/api/storage",
+    operation_id = "set_storage_config",
+    responses(
+        (status = 200, description = "Set the storage configuration"),
+        (status = 400, description = "The D-Bus service could not perform the action")
+    )
+)]
+async fn set_config(
+    State(state): State<StorageState<'_>>,
+    Json(settings): Json<StorageSettings>,
+) -> Result<Json<()>, Error> {
+    let _status: u32 = state
+        .client
+        .set_config(settings)
+        .await
+        .map_err(Error::Service)?;
+    Ok(Json(()))
+}
+
 /// Probes the storage devices.
 #[utoipa::path(
     post,

rust/package/agama.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Wed Sep 18 08:27:13 UTC 2024 - Martin Vidner <[email protected]>
+
+- For CLI, use HTTP clients instead of D-Bus clients,
+  final piece: Storage (gh#openSUSE/agama#1600)
+  - added StorageHTTPClient
+
 -------------------------------------------------------------------
 Wed Sep 13 12:25:28 UTC 2024 - Jorik Cronenberg <[email protected]>