Skip to content

Commit

Permalink
Replace D-Bus with HTTP-based clients (#1438)
Browse files Browse the repository at this point in the history
## Problem

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

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. We
need to adapt the following clients:

-
[UsersClient](https://github.com/openSUSE/agama/blob/master/rust/agama-lib/src/users/client.rs)
-
[LocalizationClient](https://github.com/openSUSE/agama/blob/master/rust/agama-lib/src/localization/client.rs)
-
[ProductClient](https://github.com/openSUSE/agama/blob/master/rust/agama-lib/src/product/client.rs)
-
[StorageClient](https://github.com/openSUSE/agama/blob/master/rust/agama-lib/src/storage/client.rs)
-
[SoftwareClient](https://github.com/openSUSE/agama/blob/master/rust/agama-lib/src/software/client.rs)


## Solution

- `UsersClient` - still needed, because the HTTP service uses it to talk
to the Ruby backend. `UsersHTTPClient` added.
- `LocalizationClient` replaced with `LocalizationHTTPClient`

The rest will be done in subsequent PRs.


## Testing

- Tested manually with various correct and incorrect configs via `agama
config show` and `agama config load`. This is ripe for automation, even
in this PR.
- aaand: added tests that
- set up a trivial HTTP server (using
[httpmock](https://crates.io/crates/httpmock) - is the dependency size
OK?),
    -  mocking the JSON data and
    -  asserting what the Store classes make out of it


## Screenshots

*If the fix affects the UI attach some screenshots here.*
  • Loading branch information
mvidner committed Aug 9, 2024
2 parents 97c5b00 + 4817cb0 commit c992adc
Show file tree
Hide file tree
Showing 32 changed files with 1,234 additions and 255 deletions.
432 changes: 427 additions & 5 deletions rust/Cargo.lock

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion rust/agama-cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub enum ConfigCommands {

pub async fn run(subcommand: ConfigCommands) -> anyhow::Result<()> {
let Some(token) = AuthToken::find() else {
println!("You need to login for generating a valid token");
println!("You need to login for generating a valid token: agama auth login");
return Ok(());
};

Expand Down
2 changes: 1 addition & 1 deletion rust/agama-cli/src/profile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ pub enum ProfileCommands {
/// Evaluate a profile, injecting the hardware information from D-Bus
///
/// For an example of Jsonnet-based profile, see
/// https://github.com/openSUSE/agama/blob/master/rust/agama-lib/share/examples/profile.jsonnet
/// <https://github.com/openSUSE/agama/blob/master/rust/agama-lib/share/examples/profile.jsonnet>
Evaluate {
/// Path to jsonnet file.
path: PathBuf,
Expand Down
6 changes: 3 additions & 3 deletions rust/agama-cli/src/questions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub enum QuestionsCommands {
/// mode or change the answer in automatic mode.
///
/// Please check Agama documentation for more details and examples:
/// https://github.com/openSUSE/agama/blob/master/doc/questions.md
/// <https://github.com/openSUSE/agama/blob/master/doc/questions.md>
Answers {
/// Path to a file containing the answers in YAML format.
path: String,
Expand Down Expand Up @@ -55,7 +55,7 @@ async fn set_answers(proxy: Questions1Proxy<'_>, path: String) -> Result<(), Ser
}

async fn list_questions() -> Result<(), ServiceError> {
let client = HTTPClient::new().await?;
let client = HTTPClient::new()?;
let questions = client.list_questions().await?;
// FIXME: if performance is bad, we can skip converting json from http to struct and then
// serialize it, but it won't be pretty string
Expand All @@ -66,7 +66,7 @@ async fn list_questions() -> Result<(), ServiceError> {
}

async fn ask_question() -> Result<(), ServiceError> {
let client = HTTPClient::new().await?;
let client = HTTPClient::new()?;
let question = serde_json::from_reader(std::io::stdin())?;

let created_question = client.create_question(&question).await?;
Expand Down
4 changes: 4 additions & 0 deletions rust/agama-lib/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,7 @@ curl = { version = "0.4.44", features = ["protocol-ftp"] }
jsonwebtoken = "9.3.0"
chrono = { version = "0.4.38", default-features = false, features = ["now", "std", "alloc", "clock"] }
home = "0.5.9"

[dev-dependencies]
httpmock = "0.7.0"
env_logger = "0.11.5"
2 changes: 1 addition & 1 deletion rust/agama-lib/src/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ impl Display for AuthToken {

/// Claims that are included in the token.
///
/// See https://datatracker.ietf.org/doc/html/rfc7519 for reference.
/// See <https://datatracker.ietf.org/doc/html/rfc7519> for reference.
#[derive(Debug, Serialize, Deserialize)]
pub struct TokenClaims {
pub exp: i64,
Expand Down
189 changes: 131 additions & 58 deletions rust/agama-lib/src/base_http_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl BaseHTTPClient {
let mut headers = header::HeaderMap::new();
// just use generic anyhow error here as Bearer format is constructed by us, so failures can come only from token
let value = header::HeaderValue::from_str(format!("Bearer {}", token).as_str())
.map_err(|e| anyhow::Error::new(e))?;
.map_err(anyhow::Error::new)?;

headers.insert(header::AUTHORIZATION, value);

Expand All @@ -66,40 +66,36 @@ impl BaseHTTPClient {
Ok(client)
}

/// Simple wrapper around [`Response`] to get object from response.
///
/// If a complete [`Response`] is needed, use the [`Self::get_response`] method.
///
/// Arguments:
///
/// * `path`: path relative to HTTP API like `/questions`
pub async fn get<T: DeserializeOwned>(&self, path: &str) -> Result<T, ServiceError> {
let response = self.get_response(path).await?;
if response.status().is_success() {
response.json::<T>().await.map_err(|e| e.into())
} else {
Err(self.build_backend_error(response).await)
}
fn url(&self, path: &str) -> String {
self.base_url.clone() + path
}

/// Calls GET method on the given path and returns [`Response`] that can be further
/// processed.
///
/// If only simple object from JSON is required, use method get.
/// Simple wrapper around [`Response`] to get object from response.
///
/// Arguments:
///
/// * `path`: path relative to HTTP API like `/questions`
pub async fn get_response(&self, path: &str) -> Result<Response, ServiceError> {
self.client
pub async fn get<T>(&self, path: &str) -> Result<T, ServiceError>
where
T: DeserializeOwned,
{
let response: Result<_, ServiceError> = self
.client
.get(self.url(path))
.send()
.await
.map_err(|e| e.into())
.map_err(|e| e.into());
self.deserialize_or_error(response?).await
}

fn url(&self, path: &str) -> String {
self.base_url.clone() + path
pub async fn post<T>(&self, path: &str, object: &impl Serialize) -> Result<T, ServiceError>
where
T: DeserializeOwned,
{
let response = self
.request_response(reqwest::Method::POST, path, object)
.await?;
self.deserialize_or_error(response).await
}

/// post object to given path and report error if response is not success
Expand All @@ -108,77 +104,154 @@ impl BaseHTTPClient {
///
/// * `path`: path relative to HTTP API like `/questions`
/// * `object`: Object that can be serialiazed to JSON as body of request.
pub async fn post(&self, path: &str, object: &impl Serialize) -> Result<(), ServiceError> {
let response = self.post_response(path, object).await?;
if response.status().is_success() {
Ok(())
} else {
Err(self.build_backend_error(response).await)
}
pub async fn post_void(&self, path: &str, object: &impl Serialize) -> Result<(), ServiceError> {
let response = self
.request_response(reqwest::Method::POST, path, object)
.await?;
self.unit_or_error(response).await
}

/// post object to given path and returns server response. Reports error only if failed to send
/// request, but if server returns e.g. 500, it will be in Ok result.
/// put object to given path, deserializes the response
///
/// In general unless specific response handling is needed, simple post should be used.
/// Arguments:
///
/// * `path`: path relative to HTTP API like `/users/first`
/// * `object`: Object that can be serialiazed to JSON as body of request.
pub async fn put<T>(&self, path: &str, object: &impl Serialize) -> Result<T, ServiceError>
where
T: DeserializeOwned,
{
let response = self
.request_response(reqwest::Method::PUT, path, object)
.await?;
self.deserialize_or_error(response).await
}

/// put object to given path and report error if response is not success
///
/// Arguments:
///
/// * `path`: path relative to HTTP API like `/questions`
/// * `path`: path relative to HTTP API like `/users/first`
/// * `object`: Object that can be serialiazed to JSON as body of request.
pub async fn put_void(&self, path: &str, object: &impl Serialize) -> Result<(), ServiceError> {
let response = self
.request_response(reqwest::Method::PUT, path, object)
.await?;
self.unit_or_error(response).await
}

/// patch object at given path and report error if response is not success
///
/// Arguments:
///
/// * `path`: path relative to HTTP API like `/users/first`
/// * `object`: Object that can be serialiazed to JSON as body of request.
pub async fn post_response(
pub async fn patch<T>(&self, path: &str, object: &impl Serialize) -> Result<T, ServiceError>
where
T: DeserializeOwned,
{
let response = self
.request_response(reqwest::Method::PATCH, path, object)
.await?;
self.deserialize_or_error(response).await
}

pub async fn patch_void(
&self,
path: &str,
object: &impl Serialize,
) -> Result<Response, ServiceError> {
self.client
.post(self.url(path))
.json(object)
.send()
.await
.map_err(|e| e.into())
) -> Result<(), ServiceError> {
let response = self
.request_response(reqwest::Method::PATCH, path, object)
.await?;
self.unit_or_error(response).await
}

/// delete call on given path and report error if failed
///
/// Arguments:
///
/// * `path`: path relative to HTTP API like `/questions/1`
pub async fn delete(&self, path: &str) -> Result<(), ServiceError> {
let response = self.delete_response(path).await?;
if response.status().is_success() {
Ok(())
} else {
Err(self.build_backend_error(response).await)
}
pub async fn delete_void(&self, path: &str) -> Result<(), ServiceError> {
let response: Result<_, ServiceError> = self
.client
.delete(self.url(path))
.send()
.await
.map_err(|e| e.into());
self.unit_or_error(response?).await
}

/// delete call on given path and returns server response. Reports error only if failed to send
/// POST/PUT/PATCH an object to a given path and returns server response.
/// Reports Err only if failed to send
/// request, but if server returns e.g. 500, it will be in Ok result.
///
/// In general unless specific response handling is needed, simple delete should be used.
/// TODO: do not need variant with request body? if so, then create additional method.
/// In general unless specific response handling is needed, simple post should be used.
///
/// Arguments:
///
/// * `path`: path relative to HTTP API like `/questions/1`
pub async fn delete_response(&self, path: &str) -> Result<Response, ServiceError> {
/// * `method`: for example `reqwest::Method::PUT`
/// * `path`: path relative to HTTP API like `/questions`
/// * `object`: Object that can be serialiazed to JSON as body of request.
async fn request_response(
&self,
method: reqwest::Method,
path: &str,
object: &impl Serialize,
) -> Result<Response, ServiceError> {
self.client
.delete(self.url(path))
.request(method, self.url(path))
.json(object)
.send()
.await
.map_err(|e| e.into())
}

/// Return deserialized JSON body as `Ok(T)` or an `Err` with [`ServiceError::BackendError`]
async fn deserialize_or_error<T>(&self, response: Response) -> Result<T, ServiceError>
where
T: DeserializeOwned,
{
// DEBUG: This dbg is nice but it omits the body, thus we try harder below
// let response = dbg!(response);

if response.status().is_success() {
// We'd like to simply:
// response.json::<T>().await.map_err(|e| e.into())
// BUT also peek into the response text, in case something is wrong
// so this copies the implementation from the above and adds a debug part

let bytes_r: Result<_, ServiceError> = response.bytes().await.map_err(|e| e.into());
let bytes = bytes_r?;

// DEBUG: (we expect JSON so dbg! would escape too much, eprintln! is better)
// let text = String::from_utf8_lossy(&bytes);
// eprintln!("Response body: {}", text);

serde_json::from_slice(&bytes).map_err(|e| e.into())
} else {
Err(self.build_backend_error(response).await)
}
}

/// Return `Ok(())` or an `Err` with [`ServiceError::BackendError`]
async fn unit_or_error(&self, response: Response) -> Result<(), ServiceError> {
if response.status().is_success() {
Ok(())
} else {
Err(self.build_backend_error(response).await)
}
}

const NO_TEXT: &'static str = "(Failed to extract error text from HTTP response)";
/// Builds [`BackendError`] from response.
/// Builds [`ServiceError::BackendError`] from response.
///
/// It contains also processing of response body, that is why it has to be async.
///
/// Arguments:
///
/// * `response`: response from which generate error
pub async fn build_backend_error(&self, response: Response) -> ServiceError {
async fn build_backend_error(&self, response: Response) -> ServiceError {
let code = response.status().as_u16();
let text = response
.text()
Expand Down
2 changes: 1 addition & 1 deletion rust/agama-lib/src/dbus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ macro_rules! property_from_dbus {
/// NOTE: we could follow a different approach like building our own type (e.g.
/// using the newtype idiom) and offering a better API.
///
/// * `source`: hash map containing non-onwed values ([zbus::zvariant::Value]).
/// * `source`: hash map containing non-onwed values ([enum@zbus::zvariant::Value]).
pub fn to_owned_hash(source: &HashMap<&str, Value<'_>>) -> HashMap<String, OwnedValue> {
let mut owned = HashMap::new();
for (key, value) in source.iter() {
Expand Down
2 changes: 1 addition & 1 deletion rust/agama-lib/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//! This library offers an API to interact with Agama services. At this point, the library allows:
//!
//! * Reading and writing [installation settings](install_settings::InstallSettings).
//! * Monitoring the [progress](progress).
//! * Monitoring the [progress].
//! * Triggering actions through the [manager] (e.g., starting installation).
//!
//! ## Handling installation settings
Expand Down
5 changes: 3 additions & 2 deletions rust/agama-lib/src/localization.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
//! Implements support for handling the localization settings

mod client;
mod http_client;
pub mod model;
mod proxies;
mod settings;
mod store;

pub use client::LocalizationClient;
pub use http_client::LocalizationHTTPClient;
pub use proxies::LocaleProxy;
pub use settings::LocalizationSettings;
pub use store::LocalizationStore;
Loading

0 comments on commit c992adc

Please sign in to comment.