Skip to content

Commit

Permalink
feat(rust): adapt to zbus 5 (agama-project#1716)
Browse files Browse the repository at this point in the history
## The main goal

Although [zbus 5](https://github.com/dbus2/zbus/releases/tag/zbus-5.0.0)
5 was released a few days ago, Agama is still using zbus 3. The goal of
this PR is to adapt the code to use the latest version.

However, this task is not trivial: version 4 already introduced a good
share of breaking changes in zbus API.

## `downcast_ref` returns `Result`

The `downcast_ref` function now returns a `Result` instead of an
`Option` with, IMHO, it is the right thing to do. This change has a big
impact in our code to interact with NetworkManager, so I took the
opportunity to:

* Distinguish between a problem and a missing value (we always returned
`None` when something went wrong).
* Use `get_property` and `get_optional_property` to simplify our code a
bit.

## Other changes

- Do some refactoring and organization of D-Bus proxies.
- Improve the `network::nm::dbus` module. Let's use `get_property` and
`get_optional_property` for better readability and error handling.
Please, check [this commit
(WIP)](agama-project@c21eda8)
if you are interested in further improvements.
- Drop some unused code.
  • Loading branch information
imobachgs authored Nov 6, 2024
2 parents 5b4f014 + 904bbf5 commit a71e9e0
Show file tree
Hide file tree
Showing 68 changed files with 2,583 additions and 1,869 deletions.
348 changes: 87 additions & 261 deletions rust/Cargo.lock

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions rust/agama-cli/src/questions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
// To contact SUSE LLC about this file by physical or electronic mail, you may
// find current contact information at www.suse.com.

use agama_lib::proxies::Questions1Proxy;
use agama_lib::proxies::questions::QuestionsProxy;
use agama_lib::questions::http_client::HTTPClient;
use agama_lib::{base_http_client::BaseHTTPClient, connection, error::ServiceError};
use clap::{Args, Subcommand, ValueEnum};
Expand Down Expand Up @@ -60,14 +60,14 @@ pub enum Modes {
NonInteractive,
}

async fn set_mode(proxy: Questions1Proxy<'_>, value: Modes) -> Result<(), ServiceError> {
async fn set_mode(proxy: QuestionsProxy<'_>, value: Modes) -> Result<(), ServiceError> {
proxy
.set_interactive(value == Modes::Interactive)
.await
.map_err(|e| e.into())
}

async fn set_answers(proxy: Questions1Proxy<'_>, path: String) -> Result<(), ServiceError> {
async fn set_answers(proxy: QuestionsProxy<'_>, path: String) -> Result<(), ServiceError> {
proxy
.add_answer_file(path.as_str())
.await
Expand Down Expand Up @@ -109,7 +109,7 @@ pub async fn run(
subcommand: QuestionsCommands,
) -> Result<(), ServiceError> {
let connection = connection().await?;
let proxy = Questions1Proxy::new(&connection).await?;
let proxy = QuestionsProxy::new(&connection).await?;

match subcommand {
QuestionsCommands::Mode(value) => set_mode(proxy, value.value).await,
Expand Down
9 changes: 7 additions & 2 deletions rust/agama-lib/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,16 @@ tokio = { version = "1.40.0", features = ["macros", "rt-multi-thread"] }
tokio-stream = "0.1.16"
url = "2.5.2"
utoipa = "4.2.3"
zbus = { version = "3", default-features = false, features = ["tokio"] }
zbus = { version = "5", default-features = false, features = ["tokio"] }
# Needed to define curl error in profile errors
curl = { version = "0.4.47", features = ["protocol-ftp"] }
jsonwebtoken = "9.3.0"
chrono = { version = "0.4.38", default-features = false, features = ["now", "std", "alloc", "clock"] }
chrono = { version = "0.4.38", default-features = false, features = [
"now",
"std",
"alloc",
"clock",
] }
home = "0.5.9"
strum = { version = "0.26.3", features = ["derive"] }

Expand Down
12 changes: 7 additions & 5 deletions rust/agama-lib/src/dbus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ where
"Failed to find property '{}'",
name
)))?
.into();
.try_into()?;

T::try_from(value).map_err(|e| e.into())
}
Expand All @@ -57,7 +57,7 @@ where
<T as TryFrom<Value<'a>>>::Error: Into<zbus::zvariant::Error>,
{
if let Some(value) = properties.get(name) {
let value: Value = value.into();
let value: Value = value.try_into()?;
T::try_from(value).map(|v| Some(v)).map_err(|e| e.into())
} else {
Ok(None)
Expand All @@ -79,12 +79,14 @@ macro_rules! property_from_dbus {
/// using the newtype idiom) and offering a better API.
///
/// * `source`: hash map containing non-onwed values ([enum@zbus::zvariant::Value]).
pub fn to_owned_hash(source: &HashMap<&str, Value<'_>>) -> HashMap<String, OwnedValue> {
pub fn to_owned_hash<T: ToString>(
source: &HashMap<T, Value<'_>>,
) -> Result<HashMap<String, OwnedValue>, zvariant::Error> {
let mut owned = HashMap::new();
for (key, value) in source.iter() {
owned.insert(key.to_string(), value.into());
owned.insert(key.to_string(), value.try_into()?);
}
owned
Ok(owned)
}

/// Extracts the object ID from the path.
Expand Down
3 changes: 2 additions & 1 deletion rust/agama-lib/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ pub mod progress;
pub mod proxies;
mod store;
pub use store::Store;
use zbus::conn::Builder;
pub mod questions;
pub mod scripts;
pub mod transfer;
Expand All @@ -75,7 +76,7 @@ pub async fn connection() -> Result<zbus::Connection, ServiceError> {
}

pub async fn connection_to(address: &str) -> Result<zbus::Connection, ServiceError> {
let connection = zbus::ConnectionBuilder::address(address)?
let connection = Builder::address(address)?
.build()
.await
.map_err(|e| ServiceError::DBusConnectionError(address.to_string(), e))?;
Expand Down
57 changes: 34 additions & 23 deletions rust/agama-lib/src/localization/proxies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,47 +18,58 @@
// To contact SUSE LLC about this file by physical or electronic mail, you may
// find current contact information at www.suse.com.

use zbus::dbus_proxy;

#[dbus_proxy(
interface = "org.opensuse.Agama1.Locale",
//! # D-Bus interface proxy for: `org.opensuse.Agama1.Locale`
//!
//! This code was generated by `zbus-xmlgen` `5.0.0` from D-Bus introspection data.
//! Source: `org.opensuse.Agama1.Locale.bus.xml`.
//!
//! You may prefer to adapt it, instead of using it verbatim.
//!
//! More information can be found in the [Writing a client proxy] section of the zbus
//! documentation.
//!
//! This type implements the [D-Bus standard interfaces], (`org.freedesktop.DBus.*`) for which the
//! following zbus API can be used:
//!
//! * [`zbus::fdo::PeerProxy`]
//! * [`zbus::fdo::PropertiesProxy`]
//! * [`zbus::fdo::IntrospectableProxy`]
//!
//! Consequently `zbus-xmlgen` did not generate code for the above interfaces.
//!
//! [Writing a client proxy]: https://dbus2.github.io/zbus/client.html
//! [D-Bus standard interfaces]: https://dbus.freedesktop.org/doc/dbus-specification.html#standard-interfaces,
use zbus::proxy;
#[proxy(
default_service = "org.opensuse.Agama1",
default_path = "/org/opensuse/Agama1/Locale"
interface = "org.opensuse.Agama1.Locale",
assume_defaults = true
)]
trait Locale {
pub trait Locale {
/// Commit method
fn commit(&self) -> zbus::Result<()>;

/// ListKeymaps method
fn list_keymaps(&self) -> zbus::Result<Vec<(String, String)>>;

/// ListLocales method
fn list_locales(&self) -> zbus::Result<Vec<(String, String, String)>>;

/// ListTimezones method
fn list_timezones(&self) -> zbus::Result<Vec<(String, Vec<String>)>>;

/// Keymap property
#[dbus_proxy(property)]
#[zbus(property)]
fn keymap(&self) -> zbus::Result<String>;
#[dbus_proxy(property)]
#[zbus(property)]
fn set_keymap(&self, value: &str) -> zbus::Result<()>;

/// Locales property
#[dbus_proxy(property)]
#[zbus(property)]
fn locales(&self) -> zbus::Result<Vec<String>>;
#[dbus_proxy(property)]
#[zbus(property)]
fn set_locales(&self, value: &[&str]) -> zbus::Result<()>;

/// Timezone property
#[dbus_proxy(property)]
#[zbus(property)]
fn timezone(&self) -> zbus::Result<String>;
#[dbus_proxy(property)]
#[zbus(property)]
fn set_timezone(&self, value: &str) -> zbus::Result<()>;

/// UILocale property
#[dbus_proxy(property, name = "UILocale")]
#[zbus(property, name = "UILocale")]
fn uilocale(&self) -> zbus::Result<String>;
#[dbus_proxy(property, name = "UILocale")]
#[zbus(property, name = "UILocale")]
fn set_uilocale(&self, value: &str) -> zbus::Result<()>;
}
6 changes: 3 additions & 3 deletions rust/agama-lib/src/network/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ impl NetworkClient {

if response.is_ok() {
let path = format!("/network/connections/{id}");
self.client.put_void(&path.as_str(), &connection).await?
self.client.put_void(path.as_str(), &connection).await?
} else {
self.client
.post_void(format!("/network/connections").as_str(), &connection)
.post_void("/network/connections".to_string().as_str(), &connection)
.await?
}

Expand All @@ -84,7 +84,7 @@ impl NetworkClient {
// trying to be tricky here. If something breaks then we need a put method on
// BaseHTTPClient which doesn't require a serialiable object for the body
self.client
.post_void(&format!("/network/system/apply").as_str(), &())
.post_void("/network/system/apply".to_string().as_str(), &())
.await?;

Ok(())
Expand Down
Loading

0 comments on commit a71e9e0

Please sign in to comment.