refactor: upgrade to zbus v5 (#14)

* api update * result wrapper for dbusmenu * update ownedvalueext * item error unify * all wrapped with ok * remove ref for some basic type * item * fix clippy * get_update_event return Result
JakeStanger · Feb 8, 2025 · 6d727c6 · 6d727c6
1 parent a3d8421
commit 6d727c6
Show file tree

Hide file tree

Showing 10 changed files with 489 additions and 1,063 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -13,14 +13,14 @@ default = []
 dbusmenu-gtk3 = ["dep:gtk", "dep:dbusmenu-gtk3-sys"]
 
 [dependencies]
-zbus = { version = "3.15.2", features = ["tokio"] }
+zbus = { version = "5.3", default-features = false, features = ["tokio"] }
 tracing = "0.1.41"
-serde = { version = "1.0.216", features = ["derive"] }
-tokio = { version = "1.42.0", features = ["rt", "sync", "macros", "time"] }
-thiserror = "2.0.7"
+serde = { version = "1", features = ["derive"] }
+tokio = { version = "1", features = ["rt", "sync", "macros", "time"] }
+thiserror = "2.0.11"
 
-gtk = {  version = "0.18.1", optional = true }
-dbusmenu-gtk3-sys = {  version = "0.1.0", optional = true }
+gtk = { version = "0.18.1", optional = true }
+dbusmenu-gtk3-sys = { version = "0.1.0", optional = true }
 
 [dev-dependencies]
-tokio = { version = "1.41.1", features = ["rt-multi-thread"] }
+tokio = { version = "1.41.1", features = ["rt-multi-thread"] }
diff --git a/src/client.rs b/src/client.rs
@@ -3,7 +3,7 @@ use crate::dbus::notifier_item_proxy::StatusNotifierItemProxy;
 use crate::dbus::notifier_watcher_proxy::StatusNotifierWatcherProxy;
 use crate::dbus::status_notifier_watcher::StatusNotifierWatcher;
 use crate::dbus::{self, OwnedValueExt};
-use crate::error::Error;
+use crate::error::{Error, Result};
 use crate::item::{self, Status, StatusNotifierItem, Tooltip};
 use crate::menu::{MenuDiff, TrayMenu};
 use crate::names;
@@ -362,14 +362,20 @@ impl Client {
         let dbus_proxy = DBusProxy::new(connection).await?;
 
         let mut disconnect_stream = dbus_proxy.receive_name_owner_changed().await?;
-        let mut props_changed = notifier_item_proxy.receive_all_signals().await?;
+        let mut props_changed = notifier_item_proxy.inner().receive_all_signals().await?;
 
         loop {
             tokio::select! {
                 Some(change) = props_changed.next() => {
-                    if let Some(event) = Self::get_update_event(change, &properties_proxy).await {
-                        debug!("[{destination}{path}] received property change: {event:?}");
-                        tx.send(Event::Update(destination.to_string(), event))?;
+                    match Self::get_update_event(change, &properties_proxy).await {
+                        Ok(Some(event)) => {
+                                debug!("[{destination}{path}] received property change: {event:?}");
+                                tx.send(Event::Update(destination.to_string(), event))?;
+                            }
+                        Err(e) => {
+                            error!("Error parsing update properties from {destination}{path}: {e:?}");
+                        }
+                        _ => {}
                     }
                 }
                 Some(signal) = disconnect_stream.next() => {
@@ -402,10 +408,13 @@ impl Client {
 
     /// Gets the update event for a `DBus` properties change message.
     async fn get_update_event(
-        change: Arc<Message>,
+        change: Message,
         properties_proxy: &PropertiesProxy<'_>,
-    ) -> Option<UpdateEvent> {
-        let member = change.member()?;
+    ) -> Result<Option<UpdateEvent>> {
+        let header = change.header();
+        let member = header
+            .member()
+            .ok_or(Error::InvalidData("Update message header missing `member`"))?;
 
         let property_name = match member.as_str() {
             "NewAttentionIcon" => "AttentionIconName",
@@ -417,47 +426,37 @@ impl Client {
             _ => &member.as_str()["New".len()..],
         };
 
-        let res = properties_proxy
+        let property = properties_proxy
             .get(
                 InterfaceName::from_static_str(PROPERTIES_INTERFACE)
                     .expect("to be valid interface name"),
                 property_name,
             )
-            .await;
-
-        let property = match res {
-            Ok(property) => property,
-            Err(err) => {
-                error!("error fetching property '{property_name}': {err:?}");
-                return None;
-            }
-        };
+            .await?;
 
         debug!("received tray item update: {member} -> {property:?}");
 
         use UpdateEvent::*;
-        match member.as_str() {
-            "NewAttentionIcon" => Some(AttentionIcon(property.to_string())),
-            "NewIcon" => Some(Icon(property.to_string())),
-            "NewOverlayIcon" => Some(OverlayIcon(property.to_string())),
+        Ok(match member.as_str() {
+            "NewAttentionIcon" => Some(AttentionIcon(property.to_string().ok())),
+            "NewIcon" => Some(Icon(property.to_string().ok())),
+            "NewOverlayIcon" => Some(OverlayIcon(property.to_string().ok())),
             "NewStatus" => Some(Status(
-                property
-                    .downcast_ref::<str>()
-                    .map(item::Status::from)
-                    .unwrap_or_default(),
+                property.downcast_ref::<&str>().map(item::Status::from)?,
             )),
-            "NewTitle" => Some(Title(property.to_string())),
-            "NewToolTip" => Some(Tooltip(
+            "NewTitle" => Some(Title(property.to_string().ok())),
+            "NewToolTip" => Some(Tooltip({
                 property
-                    .downcast_ref::<Structure>()
-                    .map(crate::item::Tooltip::try_from)?
-                    .ok(),
-            )),
+                    .downcast_ref::<&Structure>()
+                    .ok()
+                    .map(crate::item::Tooltip::try_from)
+                    .transpose()?
+            })),
             _ => {
                 warn!("received unhandled update event: {member}");
                 None
             }
-        }
+        })
     }
 
     /// Watches the `DBusMenu` associated with an SNI item.
@@ -541,7 +540,8 @@ impl Client {
                     ))?;
                 }
                 Some(change) = properties_updated.next() => {
-                    let update = change.body::<PropertiesUpdate>()?;
+                    let body = change.message().body();
+                    let update: PropertiesUpdate= body.deserialize::<PropertiesUpdate>()?;
                     let diffs = Vec::try_from(update)?;
 
                     tx.send(Event::Update(

diff --git a/src/dbus/dbus_menu_proxy.rs b/src/dbus/dbus_menu_proxy.rs
@@ -12,7 +12,7 @@
 
 use std::collections::HashMap;
 
-use zbus::dbus_proxy;
+use zbus::proxy;
 use zbus::zvariant::{OwnedValue, Value};
 
 use serde::{Deserialize, Serialize};
@@ -55,8 +55,8 @@ pub struct RemovedProps<'a> {
     pub(crate) fields: Vec<&'a str>,
 }
 
-#[dbus_proxy(interface = "com.canonical.dbusmenu", assume_defaults = true)]
-trait DBusMenu {
+#[proxy(interface = "com.canonical.dbusmenu", assume_defaults = true)]
+pub trait DBusMenu {
     fn about_to_show(&self, id: i32) -> zbus::Result<bool>;
 
     fn event(
@@ -82,22 +82,22 @@ trait DBusMenu {
 
     fn get_property(&self, id: i32, name: &str) -> zbus::Result<zbus::zvariant::OwnedValue>;
 
-    #[dbus_proxy(signal)]
+    #[zbus(signal)]
     fn item_activation_requested(&self, id: i32, timestamp: u32) -> zbus::Result<()>;
 
-    #[dbus_proxy(signal)]
+    #[zbus(signal)]
     fn items_properties_updated(
         &self,
         updated_props: Vec<(i32, HashMap<&str, Value<'_>>)>,
         removed_props: Vec<(i32, Vec<&str>)>,
     ) -> zbus::Result<()>;
 
-    #[dbus_proxy(signal)]
+    #[zbus(signal)]
     fn layout_updated(&self, revision: u32, parent: i32) -> zbus::Result<()>;
 
-    #[dbus_proxy(property)]
+    #[zbus(property)]
     fn status(&self) -> zbus::Result<String>;
 
-    #[dbus_proxy(property)]
+    #[zbus(property)]
     fn version(&self) -> zbus::Result<u32>;
 }
diff --git a/src/dbus/mod.rs b/src/dbus/mod.rs
@@ -1,3 +1,4 @@
+use crate::error::Result;
 use std::collections::HashMap;
 use std::ops::Deref;
 use zbus::zvariant::{ObjectPath, OwnedValue, Value};
@@ -13,36 +14,42 @@ pub(crate) struct DBusProps(pub HashMap<String, OwnedValue>);
 impl DBusProps {
     /// Gets `key` from the map if present,
     /// downcasting it to type `T`.
-    pub fn get<'a, T>(&'a self, key: &str) -> Option<&'a T>
+    pub fn get<'a, T>(&'a self, key: &str) -> Option<Result<&'a T>>
     where
         T: ?Sized,
         &'a T: TryFrom<&'a Value<'a>>,
+        <&'a T as TryFrom<&'a Value<'a>>>::Error: Into<zbus::zvariant::Error>,
     {
-        self.0.get(key).and_then(|value| value.downcast_ref::<T>())
+        self.0
+            .get(key)
+            .map(|v| v.downcast_ref().map_err(Into::into))
     }
 
     /// Gets `key` from the map if present,
     /// interpreting it as a `str`
     /// and converting it to a string.
-    pub fn get_string(&self, key: &str) -> Option<String> {
-        self.get::<str>(key).map(ToString::to_string)
+    pub fn get_string(&self, key: &str) -> Option<Result<String>> {
+        self.get::<str>(key).map(|res| res.map(ToString::to_string))
     }
 
     /// Gets `key` from the map if present,
     /// interpreting it as an `ObjectPath`,
     /// and converting it to a string.
-    pub fn get_object_path(&self, key: &str) -> Option<String> {
-        self.get::<ObjectPath>(key).map(ToString::to_string)
+    pub fn get_object_path(&self, key: &str) -> Option<Result<String>> {
+        self.get::<ObjectPath>(key)
+            .map(|res| res.map(ToString::to_string))
     }
 }
 
 pub(crate) trait OwnedValueExt {
-    fn to_string(&self) -> Option<String>;
+    fn to_string(&self) -> Result<String>;
 }
 
 impl OwnedValueExt for OwnedValue {
-    fn to_string(&self) -> Option<String> {
-        self.downcast_ref::<str>().map(ToString::to_string)
+    fn to_string(&self) -> Result<String> {
+        self.downcast_ref::<&str>()
+            .map(ToString::to_string)
+            .map_err(Into::into)
     }
 }
 

diff --git a/src/dbus/notifier_item_proxy.rs b/src/dbus/notifier_item_proxy.rs
@@ -10,13 +10,13 @@
 //! section of the zbus documentation.
 //!
 
-use zbus::dbus_proxy;
+use zbus::proxy;
 
 #[allow(dead_code)]
 type ToolTip = (String, Vec<(i32, i32, Vec<u8>)>);
 
-#[dbus_proxy(interface = "org.kde.StatusNotifierItem", assume_defaults = true)]
-trait StatusNotifierItem {
+#[proxy(interface = "org.kde.StatusNotifierItem", assume_defaults = true)]
+pub trait StatusNotifierItem {
     /// Activate method
     fn activate(&self, x: i32, y: i32) -> zbus::Result<()>;
 
@@ -30,86 +30,86 @@ trait StatusNotifierItem {
     fn secondary_activate(&self, x: i32, y: i32) -> zbus::Result<()>;
 
     /// NewAttentionIcon signal
-    #[dbus_proxy(signal)]
+    #[zbus(signal)]
     fn new_attention_icon(&self) -> zbus::Result<()>;
 
     /// NewIcon signal
-    #[dbus_proxy(signal)]
+    #[zbus(signal)]
     fn new_icon(&self) -> zbus::Result<()>;
 
     /// NewOverlayIcon signal
-    #[dbus_proxy(signal)]
+    #[zbus(signal)]
     fn new_overlay_icon(&self) -> zbus::Result<()>;
 
     /// NewStatus signal
-    #[dbus_proxy(signal)]
+    #[zbus(signal)]
     fn new_status(&self, status: &str) -> zbus::Result<()>;
 
     /// NewTitle signal
-    #[dbus_proxy(signal)]
+    #[zbus(signal)]
     fn new_title(&self) -> zbus::Result<()>;
 
     /// NewToolTip signal
-    #[dbus_proxy(signal)]
+    #[zbus(signal)]
     fn new_tool_tip(&self) -> zbus::Result<()>;
 
     /// AttentionIconName property
-    #[dbus_proxy(property)]
+    #[zbus(property)]
     fn attention_icon_name(&self) -> zbus::Result<String>;
 
     /// AttentionIconPixmap property
-    #[dbus_proxy(property)]
+    #[zbus(property)]
     fn attention_icon_pixmap(&self) -> zbus::Result<Vec<(i32, i32, Vec<u8>)>>;
 
     /// AttentionMovieName property
-    #[dbus_proxy(property)]
+    #[zbus(property)]
     fn attention_movie_name(&self) -> zbus::Result<String>;
 
     /// Category property
-    #[dbus_proxy(property)]
+    #[zbus(property)]
     fn category(&self) -> zbus::Result<String>;
 
     /// IconName property
-    #[dbus_proxy(property)]
+    #[zbus(property)]
     fn icon_name(&self) -> zbus::Result<String>;
 
     /// IconPixmap property
-    #[dbus_proxy(property)]
+    #[zbus(property)]
     fn icon_pixmap(&self) -> zbus::Result<Vec<(i32, i32, Vec<u8>)>>;
 
     /// IconThemePath property
-    #[dbus_proxy(property)]
+    #[zbus(property)]
     fn icon_theme_path(&self) -> zbus::Result<String>;
 
     /// Id property
-    #[dbus_proxy(property)]
+    #[zbus(property)]
     fn id(&self) -> zbus::Result<String>;
 
     /// ItemIsMenu property
-    #[dbus_proxy(property)]
+    #[zbus(property)]
     fn item_is_menu(&self) -> zbus::Result<bool>;
 
     /// Menu property
-    #[dbus_proxy(property)]
+    #[zbus(property)]
     fn menu(&self) -> zbus::Result<zbus::zvariant::OwnedObjectPath>;
 
     /// OverlayIconName property
-    #[dbus_proxy(property)]
+    #[zbus(property)]
     fn overlay_icon_name(&self) -> zbus::Result<String>;
 
     /// OverlayIconPixmap property
-    #[dbus_proxy(property)]
+    #[zbus(property)]
     fn overlay_icon_pixmap(&self) -> zbus::Result<Vec<(i32, i32, Vec<u8>)>>;
 
     /// Status property
-    #[dbus_proxy(property)]
+    #[zbus(property)]
     fn status(&self) -> zbus::Result<String>;
 
     /// Title property
-    #[dbus_proxy(property)]
+    #[zbus(property)]
     fn title(&self) -> zbus::Result<String>;
 
     /// ToolTip property
-    #[dbus_proxy(property)]
+    #[zbus(property)]
     fn tool_tip(&self) -> zbus::Result<ToolTip>;
 }