Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

agama config load/store for "software" uses the HTTP API #1548

Merged
merged 8 commits into from
Aug 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions rust/agama-lib/src/software.rs
Original file line number Diff line number Diff line change
@@ -1,10 +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;
63 changes: 63 additions & 0 deletions rust/agama-lib/src/software/http_client.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
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<Self, ServiceError> {
Ok(Self {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can use here new_with_base like Ok(Self.new_with_base(BaseHTTPClient::new()?))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point... IMHO my way is still not enough duplication to need the indirection :)

client: BaseHTTPClient::new()?,
})
}

pub fn new_with_base(base: BaseHTTPClient) -> Self {
Self { client: base }
}

pub async fn get_config(&self) -> Result<SoftwareConfig, ServiceError> {
self.client.get("/software/config").await
}

pub async fn set_config(&self, config: &SoftwareConfig) -> Result<(), ServiceError> {
// FIXME: test how errors come out:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, it can be improved by decoding body of error response and creating error from it, if it is also service error. I plan to do it, but other higher priorities come in the way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I thought too, but

  1. in the end the code seeing these errors does no decisions, just shows them to the user
  2. if we wanted to do some decisions, we would have to serialize them better than with Display/Debug

// 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<Vec<String>, 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<String> = 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<String, bool>,
) -> 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
}
}
11 changes: 11 additions & 0 deletions rust/agama-lib/src/software/model.rs
Original file line number Diff line number Diff line change
@@ -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<HashMap<String, bool>>,
/// Name of the product to install.
pub product: Option<String>,
}
2 changes: 1 addition & 1 deletion rust/agama-lib/src/software/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
116 changes: 109 additions & 7 deletions rust/agama-lib/src/software/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<SoftwareStore<'a>, ServiceError> {
impl SoftwareStore {
pub fn new() -> Result<SoftwareStore, ServiceError> {
Ok(Self {
software_client: SoftwareClient::new(connection.clone()).await?,
software_client: SoftwareHTTPClient::new()?,
})
}

Expand All @@ -34,3 +33,106 @@ impl<'a> SoftwareStore<'a> {
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<dyn Error>> {
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(())
}

#[test]
async fn test_setting_software_ok() -> Result<(), Box<dyn Error>> {
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(())
}

#[test]
async fn test_setting_software_err() -> Result<(), Box<dyn Error>> {
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(())
}
}
4 changes: 2 additions & 2 deletions rust/agama-lib/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand All @@ -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?,
})
}
Expand Down
10 changes: 1 addition & 9 deletions rust/agama-server/src/software/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand All @@ -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<HashMap<String, bool>>,
/// Name of the product to install.
product: Option<String>,
}

/// Returns an stream that emits software related events coming from D-Bus.
///
/// It emits the Event::ProductChanged and Event::PatternsChanged events.
Expand Down
2 changes: 1 addition & 1 deletion rust/agama-server/src/web/docs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
7 changes: 7 additions & 0 deletions rust/package/agama.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
-------------------------------------------------------------------
Mon Aug 26 11:19:27 UTC 2024 - Martin Vidner <[email protected]>

- 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 <[email protected]>

Expand Down
6 changes: 3 additions & 3 deletions setup-services.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Loading