From 68f7af533463d54e5e21ad830355c800bbe87a1d Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Tue, 20 Aug 2024 10:52:19 +0200 Subject: [PATCH 1/7] refactor(rust): make public agama_lib::software::model::SoftwareConfig to be used by SoftwareHTTPClient --- rust/agama-lib/src/software.rs | 1 + rust/agama-lib/src/software/model.rs | 11 +++++++++++ rust/agama-server/src/software/web.rs | 10 +--------- rust/agama-server/src/web/docs.rs | 2 +- 4 files changed, 14 insertions(+), 10 deletions(-) create mode 100644 rust/agama-lib/src/software/model.rs diff --git a/rust/agama-lib/src/software.rs b/rust/agama-lib/src/software.rs index 9231b0e5ba..cb41fbd47c 100644 --- a/rust/agama-lib/src/software.rs +++ b/rust/agama-lib/src/software.rs @@ -1,6 +1,7 @@ //! Implements support for handling the software settings mod client; +pub mod model; pub mod proxies; mod settings; mod store; diff --git a/rust/agama-lib/src/software/model.rs b/rust/agama-lib/src/software/model.rs new file mode 100644 index 0000000000..868afe8097 --- /dev/null +++ b/rust/agama-lib/src/software/model.rs @@ -0,0 +1,11 @@ +use serde::{Deserialize, Serialize}; +use std::collections::HashMap; + +/// Software service configuration (product, patterns, etc.). +#[derive(Clone, Serialize, Deserialize, utoipa::ToSchema)] +pub struct SoftwareConfig { + /// A map where the keys are the pattern names and the values whether to install them or not. + pub patterns: Option>, + /// Name of the product to install. + pub product: Option, +} diff --git a/rust/agama-server/src/software/web.rs b/rust/agama-server/src/software/web.rs index 5f298f4360..ab8526b151 100644 --- a/rust/agama-server/src/software/web.rs +++ b/rust/agama-server/src/software/web.rs @@ -16,6 +16,7 @@ use agama_lib::{ error::ServiceError, product::{proxies::RegistrationProxy, Product, ProductClient, RegistrationRequirement}, software::{ + model::SoftwareConfig, proxies::{Software1Proxy, SoftwareProductProxy}, Pattern, SelectedBy, SoftwareClient, UnknownSelectedBy, }, @@ -37,15 +38,6 @@ struct SoftwareState<'a> { software: SoftwareClient<'a>, } -/// Software service configuration (product, patterns, etc.). -#[derive(Clone, Serialize, Deserialize, utoipa::ToSchema)] -pub struct SoftwareConfig { - /// A map where the keys are the pattern names and the values whether to install them or not. - patterns: Option>, - /// Name of the product to install. - product: Option, -} - /// Returns an stream that emits software related events coming from D-Bus. /// /// It emits the Event::ProductChanged and Event::PatternsChanged events. diff --git a/rust/agama-server/src/web/docs.rs b/rust/agama-server/src/web/docs.rs index b913f3974c..c28827f949 100644 --- a/rust/agama-server/src/web/docs.rs +++ b/rust/agama-server/src/web/docs.rs @@ -106,7 +106,7 @@ use utoipa::OpenApi; schemas(agama_lib::questions::model::PasswordAnswer), schemas(agama_lib::questions::model::Question), schemas(agama_lib::questions::model::QuestionWithPassword), - schemas(crate::software::web::SoftwareConfig), + schemas(agama_lib::software::model::SoftwareConfig), schemas(crate::software::web::SoftwareProposal), schemas(crate::storage::web::ProductParams), schemas(crate::storage::web::iscsi::DiscoverParams), From e21574d53e8eb92e10c3ac76cb01da577517a8c4 Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Tue, 20 Aug 2024 10:53:40 +0200 Subject: [PATCH 2/7] feat(cli): agama config load/store for "software" uses the HTTP API with a new SoftwareHTTPClient. Underneath, the web service still uses the D-Bus SoftwareClient --- rust/agama-lib/src/software.rs | 2 + rust/agama-lib/src/software/http_client.rs | 65 ++++++++++++++++++++++ rust/agama-lib/src/software/store.rs | 13 ++--- rust/agama-lib/src/store.rs | 4 +- 4 files changed, 75 insertions(+), 9 deletions(-) create mode 100644 rust/agama-lib/src/software/http_client.rs diff --git a/rust/agama-lib/src/software.rs b/rust/agama-lib/src/software.rs index cb41fbd47c..98f06fd0f9 100644 --- a/rust/agama-lib/src/software.rs +++ b/rust/agama-lib/src/software.rs @@ -1,11 +1,13 @@ //! Implements support for handling the software settings mod client; +mod http_client; pub mod model; pub mod proxies; mod settings; mod store; pub use client::{Pattern, SelectedBy, SoftwareClient, UnknownSelectedBy}; +pub use http_client::SoftwareHTTPClient; pub use settings::SoftwareSettings; pub use store::SoftwareStore; diff --git a/rust/agama-lib/src/software/http_client.rs b/rust/agama-lib/src/software/http_client.rs new file mode 100644 index 0000000000..31ddbce341 --- /dev/null +++ b/rust/agama-lib/src/software/http_client.rs @@ -0,0 +1,65 @@ +use crate::software::model::SoftwareConfig; +use crate::{base_http_client::BaseHTTPClient, error::ServiceError}; +use std::collections::HashMap; + +pub struct SoftwareHTTPClient { + client: BaseHTTPClient, +} + +impl SoftwareHTTPClient { + pub fn new() -> Result { + Ok(Self { + client: BaseHTTPClient::new()?, + }) + } + + /* + pub fn new_with_base(base: BaseHTTPClient) -> Result { + Ok(Self { client: base }) + } + */ + + pub async fn get_config(&self) -> Result { + self.client.get("/software/config").await + } + + pub async fn set_config(&self, config: &SoftwareConfig) -> Result<(), ServiceError> { + // FIXME: test how errors come out: + // unknown pattern name, + // D-Bus client returns + // Err(ServiceError::UnknownPatterns(wrong_patterns)) + // CLI prints: + // Anyhow(Backend call failed with status 400 and text '{"error":"Agama service error: Failed to find these patterns: [\"no_such_pattern\"]"}') + self.client.put_void("/software/config", config).await + } + + /// Returns the ids of patterns selected by user + pub async fn user_selected_patterns(&self) -> Result, ServiceError> { + // TODO: this way we unnecessarily ask D-Bus (via web.rs) also for the product and then ignore it + let config = self.get_config().await?; + + let Some(patterns_map) = config.patterns else { + return Ok(vec![]); + }; + + let patterns: Vec = patterns_map + .into_iter() + .filter_map(|(name, is_selected)| if is_selected { Some(name) } else { None }) + .collect(); + + Ok(patterns) + } + + /// Selects patterns by user + pub async fn select_patterns( + &self, + patterns: HashMap, + ) -> Result<(), ServiceError> { + let config = SoftwareConfig { + product: None, + // TODO: SoftwareStore only passes true bools, false branch is untested + patterns: Some(patterns), + }; + self.set_config(&config).await + } +} diff --git a/rust/agama-lib/src/software/store.rs b/rust/agama-lib/src/software/store.rs index 3c72d952c2..32b11ed796 100644 --- a/rust/agama-lib/src/software/store.rs +++ b/rust/agama-lib/src/software/store.rs @@ -2,19 +2,18 @@ use std::collections::HashMap; -use super::{SoftwareClient, SoftwareSettings}; +use super::{SoftwareHTTPClient, SoftwareSettings}; use crate::error::ServiceError; -use zbus::Connection; /// Loads and stores the software settings from/to the D-Bus service. -pub struct SoftwareStore<'a> { - software_client: SoftwareClient<'a>, +pub struct SoftwareStore { + software_client: SoftwareHTTPClient, } -impl<'a> SoftwareStore<'a> { - pub async fn new(connection: Connection) -> Result, ServiceError> { +impl SoftwareStore { + pub fn new() -> Result { Ok(Self { - software_client: SoftwareClient::new(connection.clone()).await?, + software_client: SoftwareHTTPClient::new()?, }) } diff --git a/rust/agama-lib/src/store.rs b/rust/agama-lib/src/store.rs index addf98f8ae..036d6cfc9b 100644 --- a/rust/agama-lib/src/store.rs +++ b/rust/agama-lib/src/store.rs @@ -19,7 +19,7 @@ pub struct Store<'a> { users: UsersStore, network: NetworkStore, product: ProductStore<'a>, - software: SoftwareStore<'a>, + software: SoftwareStore, storage: StorageStore<'a>, localization: LocalizationStore, } @@ -34,7 +34,7 @@ impl<'a> Store<'a> { users: UsersStore::new()?, network: NetworkStore::new(http_client).await?, product: ProductStore::new(connection.clone()).await?, - software: SoftwareStore::new(connection.clone()).await?, + software: SoftwareStore::new()?, storage: StorageStore::new(connection).await?, }) } From f8b5a68dfa02b19fd54a7dd8c981e2dcdaea824d Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Thu, 22 Aug 2024 09:29:36 +0200 Subject: [PATCH 3/7] test: SoftwareStore and SoftwareHTTPClient --- rust/agama-lib/src/software/http_client.rs | 6 +-- rust/agama-lib/src/software/settings.rs | 2 +- rust/agama-lib/src/software/store.rs | 48 ++++++++++++++++++++++ 3 files changed, 51 insertions(+), 5 deletions(-) diff --git a/rust/agama-lib/src/software/http_client.rs b/rust/agama-lib/src/software/http_client.rs index 31ddbce341..b8c3ea624b 100644 --- a/rust/agama-lib/src/software/http_client.rs +++ b/rust/agama-lib/src/software/http_client.rs @@ -13,11 +13,9 @@ impl SoftwareHTTPClient { }) } - /* - pub fn new_with_base(base: BaseHTTPClient) -> Result { - Ok(Self { client: base }) + pub fn new_with_base(base: BaseHTTPClient) -> Self { + Self { client: base } } - */ pub async fn get_config(&self) -> Result { self.client.get("/software/config").await diff --git a/rust/agama-lib/src/software/settings.rs b/rust/agama-lib/src/software/settings.rs index 9601b96837..f94ae4d0e9 100644 --- a/rust/agama-lib/src/software/settings.rs +++ b/rust/agama-lib/src/software/settings.rs @@ -3,7 +3,7 @@ use serde::{Deserialize, Serialize}; /// Software settings for installation -#[derive(Debug, Default, Serialize, Deserialize)] +#[derive(Debug, Default, Serialize, Deserialize, PartialEq)] #[serde(rename_all = "camelCase")] pub struct SoftwareSettings { /// List of patterns to install. If empty use default. diff --git a/rust/agama-lib/src/software/store.rs b/rust/agama-lib/src/software/store.rs index 32b11ed796..d67172461f 100644 --- a/rust/agama-lib/src/software/store.rs +++ b/rust/agama-lib/src/software/store.rs @@ -33,3 +33,51 @@ impl SoftwareStore { 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 software_store(mock_server_url: String) -> SoftwareStore { + let mut bhc = BaseHTTPClient::default(); + bhc.base_url = mock_server_url; + let client = SoftwareHTTPClient::new_with_base(bhc); + SoftwareStore { + software_client: client, + } + } + + #[test] + async fn test_getting_software() -> Result<(), Box> { + let server = MockServer::start(); + let software_mock = server.mock(|when, then| { + when.method(GET).path("/api/software/config"); + then.status(200) + .header("content-type", "application/json") + .body( + r#"{ + "patterns": {"xfce":true}, + "product": "Tumbleweed" + }"#, + ); + }); + let url = server.url("/api"); + + let store = software_store(url); + let settings = store.load().await?; + + let expected = SoftwareSettings { + patterns: vec!["xfce".to_owned()], + }; + // main assertion + assert_eq!(settings, expected); + + // Ensure the specified mock was called exactly one time (or fail with a detailed error description). + software_mock.assert(); + Ok(()) + } +} From d3f2e0be6a63a713f2520c75b10e9ccdf17ad62f Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Fri, 23 Aug 2024 12:23:11 +0200 Subject: [PATCH 4/7] test: SoftwareStore::store happy Note the `when.(...).body("{JSON object}")` part where the JSON must match what the client sends, otherwise the mock server will respond with a 404, and the test will rightly fail with: ``` Error: BackendError(404, "{\"message\":\"Request did not match any route or mock\"}") ``` The problem is that if you compose that JSON according to your best understanding of the client but make a typo, you are still left with that unhelpful 404. These 3 parts were all need to arrive at a working string: 1. Enable httpmock logging by calling `env_logger::init();` at the start of the test function. 2. Show the request, by enabling the logging at runtime, at the right level: (`test_setting_software` is a partial match on the test name). ```console (cd rust; RUST_LOG=httpmock=debug cargo test test_setting_software) ``` If you don't like typing, the really important bits are these. httpmock will also log the more detailed Trace level but it does not get in the way ```console RUST_LOG=httpmock cargo test ``` 3. Unfortunately the request will be logged not as text but as an array of bytes shown numerically. [Process the output with this script to extract the text.][extract] [extract]: https://gist.github.com/mvidner/0e22ea82abecb44b469841929e5ec279#file-httpmock_body-rb --- rust/agama-lib/src/software/store.rs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/rust/agama-lib/src/software/store.rs b/rust/agama-lib/src/software/store.rs index d67172461f..7520d711b7 100644 --- a/rust/agama-lib/src/software/store.rs +++ b/rust/agama-lib/src/software/store.rs @@ -80,4 +80,31 @@ mod test { software_mock.assert(); Ok(()) } + + #[test] + async fn test_setting_software_ok() -> Result<(), Box> { + let server = MockServer::start(); + let software_mock = server.mock(|when, then| { + when.method(PUT) + .path("/api/software/config") + .header("content-type", "application/json") + .body(r#"{"patterns":{"xfce":true},"product":null}"#); + then.status(200); + }); + let url = server.url("/api"); + + let store = software_store(url); + let settings = SoftwareSettings { + patterns: vec!["xfce".to_owned()], + }; + + 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). + software_mock.assert(); + Ok(()) + } } From dff2aaa2702ad179994f3e8022ad451ea6c5564d Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Mon, 26 Aug 2024 11:01:02 +0200 Subject: [PATCH 5/7] running from the git checkout: fix service startup by starting NetworkManager before agama-web-server which depends on it --- setup-services.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/setup-services.sh b/setup-services.sh index 3c6534ab29..76da66bae5 100755 --- a/setup-services.sh +++ b/setup-services.sh @@ -182,6 +182,9 @@ $SUDO cp -v $MYDIR/service/share/dbus.conf /usr/share/dbus-1/agama.conf $SUDO mkdir -p /usr/share/agama/products.d $SUDO cp -f $MYDIR/products.d/*.yaml /usr/share/agama/products.d +# - Make sure NetworkManager is running +$SUDO systemctl start NetworkManager + # systemd reload and start of service ( $SUDO systemctl daemon-reload @@ -190,6 +193,3 @@ $SUDO cp -f $MYDIR/products.d/*.yaml /usr/share/agama/products.d # Start the web server $SUDO systemctl start agama-web-server.service ) - -# - Make sure NetworkManager is running -$SUDO systemctl start NetworkManager From 3c30c5d86eb667df17d9bbd2bd0f3e559b927d8c Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Mon, 26 Aug 2024 13:17:45 +0200 Subject: [PATCH 6/7] test: SoftwareStore::store error --- rust/agama-lib/src/software/store.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/rust/agama-lib/src/software/store.rs b/rust/agama-lib/src/software/store.rs index 7520d711b7..2323c553a8 100644 --- a/rust/agama-lib/src/software/store.rs +++ b/rust/agama-lib/src/software/store.rs @@ -107,4 +107,32 @@ mod test { software_mock.assert(); Ok(()) } + + #[test] + async fn test_setting_software_err() -> Result<(), Box> { + let server = MockServer::start(); + let software_mock = server.mock(|when, then| { + when.method(PUT) + .path("/api/software/config") + .header("content-type", "application/json") + .body(r#"{"patterns":{"no_such_pattern":true},"product":null}"#); + then.status(400) + .body(r#"'{"error":"Agama service error: Failed to find these patterns: [\"no_such_pattern\"]"}"#); + }); + let url = server.url("/api"); + + let store = software_store(url); + let settings = SoftwareSettings { + patterns: vec!["no_such_pattern".to_owned()], + }; + + let result = store.store(&settings).await; + + // main assertion + assert!(result.is_err()); + + // Ensure the specified mock was called exactly one time (or fail with a detailed error description). + software_mock.assert(); + Ok(()) + } } From 54d48ba28ee6579a57027fdfe6b0cd2e567b363b Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Mon, 26 Aug 2024 13:24:13 +0200 Subject: [PATCH 7/7] changelog --- rust/package/agama.changes | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/rust/package/agama.changes b/rust/package/agama.changes index cbabd66d85..a3bcd66804 100644 --- a/rust/package/agama.changes +++ b/rust/package/agama.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Mon Aug 26 11:19:27 UTC 2024 - Martin Vidner + +- For CLI, use HTTP clients instead of D-Bus clients, + for Software (gh#openSUSE/agama#1548) + - added SoftwareHTTPClient + ------------------------------------------------------------------- Thu Aug 15 08:33:02 UTC 2024 - Josef Reidinger