From eda937e5c5beb11a632b760505d00f70eea69ab6 Mon Sep 17 00:00:00 2001
From: Kubat <mael.martin31@gmail.com>
Date: Sat, 4 Feb 2023 18:20:08 +0100
Subject: [PATCH] MPRIS: Try to make the crate more digest

- Use a builder to not handle multiple optional things and hide the
  multiple server definitions
- Rename types to have shorter names
- Remove unused error types
- Use references and small strings to avoid allocations as much as
  possible
- Set DBus names manually for signals/properties/methods
---
 src/rust/mpris/src/error.rs  |  12 ---
 src/rust/mpris/src/lib.rs    |   5 +-
 src/rust/mpris/src/main.rs   |   5 +-
 src/rust/mpris/src/server.rs | 158 +++++++++++++++++++++--------------
 src/rust/mpris/src/types.rs  |  48 ++++++-----
 5 files changed, 133 insertions(+), 95 deletions(-)
 delete mode 100644 src/rust/mpris/src/error.rs

diff --git a/src/rust/mpris/src/error.rs b/src/rust/mpris/src/error.rs
deleted file mode 100644
index cb9f10ed..00000000
--- a/src/rust/mpris/src/error.rs
+++ /dev/null
@@ -1,12 +0,0 @@
-use zbus::DBusError;
-
-#[derive(DBusError, Debug)]
-#[dbus_error(prefix = "org.mpris.MediaPlayer2")]
-pub enum Error {
-    #[dbus_error(zbus_error)]
-    ZBus(zbus::Error),
-
-    NoFile(String),
-}
-
-pub type MPRISResult<T> = std::result::Result<T, Error>;
diff --git a/src/rust/mpris/src/lib.rs b/src/rust/mpris/src/lib.rs
index 5449773c..400b69b4 100644
--- a/src/rust/mpris/src/lib.rs
+++ b/src/rust/mpris/src/lib.rs
@@ -1,3 +1,4 @@
-pub mod error;
-pub mod server;
 pub mod types;
+
+mod server;
+pub use server::{Server, ServerBuilder};
diff --git a/src/rust/mpris/src/main.rs b/src/rust/mpris/src/main.rs
index cd23ce4e..a3c7f150 100644
--- a/src/rust/mpris/src/main.rs
+++ b/src/rust/mpris/src/main.rs
@@ -1,6 +1,9 @@
 #[tokio::main]
 async fn main() -> Result<(), Box<dyn std::error::Error>> {
-    let _ = mpris::server::build_session("mpris").await?;
+    let _server = mpris::Server::builder("mpris")
+        .desktop_entry("MPRIS Tester for Amadeus")
+        .try_build()
+        .await?;
 
     // Do other things or go to wait forever
     std::future::pending::<()>().await;
diff --git a/src/rust/mpris/src/server.rs b/src/rust/mpris/src/server.rs
index 4b2f86b7..36b3bdb1 100644
--- a/src/rust/mpris/src/server.rs
+++ b/src/rust/mpris/src/server.rs
@@ -2,31 +2,56 @@
 
 use crate::types::*;
 use commons::log;
+use smallstring::SmallString;
 use zbus::{dbus_interface, Connection, ConnectionBuilder, SignalContext as ZSignalCtx};
 
-macro_rules! str {
-    ($str: literal) => {
-        String::from($str)
-    };
+pub struct Server {
+    connexion: Connection,
 }
 
-pub async fn build_session(identity: impl ToString) -> zbus::Result<Connection> {
-    let main = Main {
-        identity: identity.to_string(),
-    };
-    let player = Player {};
-    let track_list = TrackList {};
-    ConnectionBuilder::session()?
-        .name("org.mpris.MediaPlayer2.mpris")?
-        .serve_at("/org/mpris/MediaPlayer2", main)?
-        .serve_at("/org/mpris/MediaPlayer2", player)?
-        .serve_at("/org/mpris/MediaPlayer2", track_list)?
-        .build()
-        .await
+pub struct ServerBuilder {
+    identity: SmallString,
+    desktop_entry: Option<SmallString>,
+}
+
+impl Server {
+    pub fn builder(identity: impl AsRef<str>) -> ServerBuilder {
+        ServerBuilder {
+            identity: identity.as_ref().into(),
+            desktop_entry: None,
+        }
+    }
+}
+
+impl ServerBuilder {
+    pub fn desktop_entry(mut self, desktop_entry: impl AsRef<str>) -> Self {
+        self.desktop_entry = Some(desktop_entry.as_ref().into());
+        self
+    }
+
+    /// Try to build the DBus server.
+    pub async fn try_build(self) -> zbus::Result<Server> {
+        let desktop_entry = self.desktop_entry.unwrap_or_else(|| self.identity.clone());
+        let main = Main {
+            identity: self.identity,
+            desktop_entry,
+        };
+        let player = Player {};
+        let track_list = TrackList {};
+        let connexion = ConnectionBuilder::session()?
+            .name("org.mpris.MediaPlayer2.mpris")?
+            .serve_at("/org/mpris/MediaPlayer2", main)?
+            .serve_at("/org/mpris/MediaPlayer2", player)?
+            .serve_at("/org/mpris/MediaPlayer2", track_list)?
+            .build()
+            .await?;
+        Ok(Server { connexion })
+    }
 }
 
 struct Main {
-    identity: String,
+    identity: SmallString,
+    desktop_entry: SmallString,
 }
 
 struct Player {}
@@ -63,8 +88,8 @@ impl Main {
 
     /// DesktopEntry property
     #[dbus_interface(property, name = "DesktopEntry")]
-    fn desktop_entry(&self) -> String {
-        self.identity.clone()
+    fn desktop_entry(&self) -> &str {
+        &self.desktop_entry
     }
 
     /// Fullscreen property
@@ -78,105 +103,115 @@ impl Main {
     /// HasTrackList property
     #[dbus_interface(property, name = "HasTrackList")]
     fn has_track_list(&self) -> bool {
-        false
+        true
     }
 
     /// Identity property
     #[dbus_interface(property, name = "Identity")]
-    fn identity(&self) -> String {
-        self.identity.clone()
+    fn identity(&self) -> &str {
+        &self.identity
     }
 
     /// SupportedMimeTypes property
     #[dbus_interface(property, name = "SupportedMimeTypes")]
-    fn supported_mime_types(&self) -> Vec<String> {
-        vec![str!("video/mkv")]
+    fn supported_mime_types(&self) -> Vec<&str> {
+        vec!["video/mkv"]
     }
 
     /// SupportedUriSchemes property. It should be `id` and `file` for lektord.
     /// Here we support `file` for only files in the database to enable the user
     /// to search the kara folder and drop the files to play them imediatly.
     #[dbus_interface(property, name = "SupportedUriSchemes")]
-    fn supported_uri_schemes(&self) -> Vec<String> {
-        vec![str!("id"), str!("file")]
+    fn supported_uri_schemes(&self) -> Vec<&str> {
+        vec!["id", "file"]
     }
 }
 
 #[dbus_interface(name = "org.mpris.MediaPlayer2.Player")]
 impl Player {
     /// Next method
+    #[dbus_interface(name = "Next")]
     fn next(&self) {}
 
     /// Pause method
+    #[dbus_interface(name = "Pause")]
     fn pause(&self) {
         todo!()
     }
 
     /// Play method
+    #[dbus_interface(name = "Play")]
     fn play(&self) {
         todo!()
     }
 
     /// OpenUri method
+    #[dbus_interface(name = "OpenUri")]
     fn open_uri(&self, uri: &str) {
         todo!("handle open uri {uri}")
     }
 
+    /// Toggle play pause status.
+    #[dbus_interface(name = "PlayPause")]
     fn play_pause(&self) {
         todo!()
     }
 
     /// Previous method
+    #[dbus_interface(name = "Previous")]
     fn previous(&self) {
         todo!()
     }
 
     /// Seek method
+    #[dbus_interface(name = "Seek")]
     fn seek(&self, offset: TimeMicroSec) {
         todo!("handle seek offset {offset:?}")
     }
 
     /// SetPosition method
-    fn set_position(&self, track_id: MediaPlayerObjectPath, position: TimeMicroSec) {
+    #[dbus_interface(name = "SetPosition")]
+    fn set_position(&self, track_id: ObjectPath, position: TimeMicroSec) {
         todo!("handle set position {track_id:?} {position:?}")
     }
 
     /// Stop method
+    #[dbus_interface(name = "Stop")]
     fn stop(&self) {
-        todo!()
+        log::warn!("ignore stop command")
     }
 
     /// CanControl property
-    #[dbus_interface(property)]
+    #[dbus_interface(property, name = "CanControl")]
     fn can_control(&self) -> bool {
         true
     }
 
     /// CanPause property
-    #[dbus_interface(property)]
+    #[dbus_interface(property, name = "CanPause")]
     fn can_pause(&self) -> bool {
         true
     }
 
     /// CanPlay property
-    #[dbus_interface(property)]
+    #[dbus_interface(property, name = "CanPlay")]
     fn can_play(&self) -> bool {
         true
     }
 
     /// CanSeek property
-    #[dbus_interface(property)]
+    #[dbus_interface(property, name = "CanSeek")]
     fn can_seek(&self) -> bool {
         false
     }
 
     /// LoopStatus property
     #[dbus_interface(property, name = "LoopStatus")]
-    fn loop_status(&self) -> MediaPlayerLoopStatus {
-        MediaPlayerLoopStatus::None
+    fn loop_status(&self) -> LoopStatus {
+        LoopStatus::None
     }
     #[dbus_interface(property, name = "LoopStatus")]
-    fn set_loop_status(&self, loop_status: MediaPlayerLoopStatus) {
+    fn set_loop_status(&self, loop_status: LoopStatus) {
         todo!("handle loop status {loop_status:?}")
     }
 
@@ -209,8 +244,8 @@ impl Player {
 
     /// PlaybackStatus property
     #[dbus_interface(property, name = "PlaybackStatus")]
-    fn playback_status(&self) -> MediaPlayerPlaybackStatus {
-        MediaPlayerPlaybackStatus::Stopped
+    fn playback_status(&self) -> PlaybackStatus {
+        PlaybackStatus::Stopped
     }
 
     /// Position property
@@ -223,76 +258,77 @@ impl Player {
     /// least a `mpris:trackid` value of type `o` which is the object path of
     /// the current kara id.
     #[dbus_interface(property, name = "Metadata")]
-    fn metadata(&self) -> MediaPlayerMetadata {
-        MediaPlayerMetadata::from_iter([])
+    fn metadata(&self) -> TrackMetadata {
+        TrackMetadata::from_iter([])
     }
 }
 
 #[dbus_interface(name = "org.mpris.MediaPlayer2.TrackList")]
 impl TrackList {
     /// AddTrack method
-    fn add_track(&self, uri: &str, after: MediaPlayerObjectPath, set_as_current: bool) {
+    #[dbus_interface(name = "AddTrack")]
+    fn add_track(&self, uri: &str, after: ObjectPath, set_as_current: bool) {
         todo!("handle add {uri} after {after:?} with set as current: {set_as_current}")
     }
 
     /// GetTracksMetadata method
-    fn get_tracks_metadata(
-        &self,
-        tracks_ids: Vec<MediaPlayerObjectPath>,
-    ) -> Vec<MediaPlayerMetadata> {
+    #[dbus_interface(name = "GetTracksMetadata")]
+    fn get_tracks_metadata(&self, tracks_ids: Vec<ObjectPath>) -> Vec<TrackMetadata> {
         todo!("handle track ids: {tracks_ids:?}")
     }
 
     /// GoTo method
-    fn go_to(&self, track_id: MediaPlayerObjectPath) {
+    #[dbus_interface(name = "GoTo")]
+    fn go_to(&self, track_id: ObjectPath) {
         todo!("handle track id {track_id:?}")
     }
 
     /// RemoveTrack method
-    fn remove_track(&self, track_id: MediaPlayerObjectPath) {
+    #[dbus_interface(name = "RemoveTrack")]
+    fn remove_track(&self, track_id: ObjectPath) {
         todo!("handle track id {track_id:?}")
     }
 
     /// CanEditTracks property
-    #[dbus_interface(property)]
+    #[dbus_interface(property, name = "CanEditTracks")]
     fn can_edit_tracks(&self) -> bool {
         true
     }
 
     /// Tracks property
-    #[dbus_interface(property)]
-    fn tracks(&self) -> Vec<MediaPlayerObjectPath> {
+    #[dbus_interface(property, name = "Tracks")]
+    fn tracks(&self) -> Vec<ObjectPath> {
         vec![]
     }
 
     /// TrackAdded signal
-    #[dbus_interface(signal)]
+    #[dbus_interface(signal, name = "TrackAdded")]
     async fn track_added(
         #[zbus(signal_context)] ctxt: ZSignalCtx<'_>,
-        metadata: MediaPlayerMetadata,
-        after: MediaPlayerObjectPath,
+        metadata: TrackMetadata,
+        after: ObjectPath,
     ) -> zbus::Result<()>;
 
     /// TrackListReplaced signal
-    #[dbus_interface(signal)]
+    #[dbus_interface(signal, name = "TrackListReplaced")]
     async fn track_list_replaced(
         #[zbus(signal_context)] ctxt: ZSignalCtx<'_>,
-        tracks: Vec<MediaPlayerObjectPath>,
-        current: MediaPlayerObjectPath,
+        tracks: Vec<ObjectPath>,
+        current: ObjectPath,
     ) -> zbus::Result<()>;
 
     /// TrackMetadataChanged signal
-    #[dbus_interface(signal)]
+    #[dbus_interface(signal, name = "TrackMetadataChanged")]
     async fn track_metadata_changed(
         #[zbus(signal_context)] ctxt: ZSignalCtx<'_>,
-        track_id: MediaPlayerObjectPath,
-        metadata: MediaPlayerMetadata,
+        track_id: ObjectPath,
+        metadata: TrackMetadata,
     ) -> zbus::Result<()>;
 
     /// TrackRemoved signal
-    #[dbus_interface(signal)]
+    #[dbus_interface(signal, name = "TrackRemoved")]
     async fn track_removed(
         #[zbus(signal_context)] ctxt: ZSignalCtx<'_>,
-        track_id: MediaPlayerObjectPath,
+        track_id: ObjectPath,
     ) -> zbus::Result<()>;
 }
diff --git a/src/rust/mpris/src/types.rs b/src/rust/mpris/src/types.rs
index d89d6e11..d4ce97fa 100644
--- a/src/rust/mpris/src/types.rs
+++ b/src/rust/mpris/src/types.rs
@@ -5,7 +5,7 @@ use zbus::zvariant::{Dict as ZDict, ObjectPath as ZObjectPath, Type as ZType, Va
 /// The loop status.
 #[derive(Debug, Deserialize, Serialize, ZType, Clone, Copy, PartialEq, Eq)]
 #[zvariant(signature = "s")]
-pub enum MediaPlayerLoopStatus {
+pub enum LoopStatus {
     None,
     Track,
     Playlist,
@@ -14,7 +14,7 @@ pub enum MediaPlayerLoopStatus {
 /// The playback status.
 #[derive(Debug, Serialize, Deserialize, ZType, Clone, Copy, PartialEq, Eq)]
 #[zvariant(signature = "s")]
-pub enum MediaPlayerPlaybackStatus {
+pub enum PlaybackStatus {
     Playing,
     Paused,
     Stopped,
@@ -28,7 +28,7 @@ pub struct TimeMicroSec(i64);
 
 #[derive(Debug, Serialize, Deserialize, ZType, Clone, PartialEq, Eq)]
 #[zvariant(signature = "a{sv}")]
-pub struct MediaPlayerMetadata(HashMap<String, String>);
+pub struct TrackMetadata(HashMap<String, String>);
 
 /// A valid path must be prefiexed by `/org/mpris/MediaPlayer2`. After that we
 /// have the variant name, then a `/`, then the ascii only value. For example
@@ -38,7 +38,7 @@ pub struct MediaPlayerMetadata(HashMap<String, String>);
 ///     - `/org/mpris/MediaPlayer2/TrackList/NoTrack` is a special case...
 #[derive(Debug, Serialize, Deserialize, ZType, Clone, Copy, PartialEq, Eq, Default)]
 #[zvariant(signature = "o")]
-pub enum MediaPlayerObjectPath {
+pub enum ObjectPath {
     /// Not an object, or nothing.
     #[default]
     None,
@@ -48,11 +48,14 @@ pub enum MediaPlayerObjectPath {
 
     /// An id.
     Id(i64),
+
+    /// A position in the queue.
+    Position(i64),
 }
 
 // Implementations
 
-impl FromIterator<(String, String)> for MediaPlayerMetadata {
+impl FromIterator<(String, String)> for TrackMetadata {
     fn from_iter<T: IntoIterator<Item = (String, String)>>(iter: T) -> Self {
         Self(iter.into_iter().collect())
     }
@@ -71,7 +74,7 @@ impl From<ZValue<'_>> for TimeMicroSec {
     }
 }
 
-impl From<ZValue<'_>> for MediaPlayerLoopStatus {
+impl From<ZValue<'_>> for LoopStatus {
     fn from(value: ZValue<'_>) -> Self {
         let value: String = value.try_into().unwrap_or_default();
         match &value[..] {
@@ -83,7 +86,7 @@ impl From<ZValue<'_>> for MediaPlayerLoopStatus {
     }
 }
 
-impl From<ZValue<'_>> for MediaPlayerPlaybackStatus {
+impl From<ZValue<'_>> for PlaybackStatus {
     fn from(value: ZValue<'_>) -> Self {
         let value: String = value.try_into().unwrap_or_default();
         match &value[..] {
@@ -95,13 +98,17 @@ impl From<ZValue<'_>> for MediaPlayerPlaybackStatus {
     }
 }
 
-impl From<ZValue<'_>> for MediaPlayerObjectPath {
+impl From<ZValue<'_>> for ObjectPath {
     fn from(value: ZValue<'_>) -> Self {
         let value: ZObjectPath = value.try_into().unwrap_or_default();
         let value = value.trim_start_matches('/');
         match value.split('/').collect::<Vec<_>>()[..] {
             ["org", "mpris", "MediaPlayer2", "TrackList", "NoTrack"] => Self::NoTrack,
             ["org", "mpris", "MediaPlayer2", "None"] => Self::None,
+            ["org", "mpris", "MediaPlayer2", "Position", position] => position
+                .parse::<i64>()
+                .map(Self::Position)
+                .unwrap_or_default(),
             ["org", "mpris", "MediaPlayer2", "Id", id] => {
                 id.parse::<i64>().map(Self::Id).unwrap_or_default()
             }
@@ -110,23 +117,26 @@ impl From<ZValue<'_>> for MediaPlayerObjectPath {
     }
 }
 
-impl From<MediaPlayerObjectPath> for ZValue<'_> {
-    fn from(value: MediaPlayerObjectPath) -> Self {
-        use MediaPlayerObjectPath::*;
+impl From<ObjectPath> for ZValue<'_> {
+    fn from(value: ObjectPath) -> Self {
+        use ObjectPath::*;
         ZValue::ObjectPath(
             match value {
                 None => ZObjectPath::try_from("/org/mpris/MediaPlayer2/None"),
                 NoTrack => ZObjectPath::try_from("/org/mpris/MediaPlayer2/TrackList/NoTrack"),
                 Id(id) => ZObjectPath::try_from(format!("/org/mpris/MediaPlayer2/Id/{id}")),
+                Position(position) => {
+                    ZObjectPath::try_from(format!("/org/mpris/MediaPlayer2/Position/{position}"))
+                }
             }
             .expect("incorrect serialization"),
         )
     }
 }
 
-impl From<MediaPlayerLoopStatus> for ZValue<'_> {
-    fn from(value: MediaPlayerLoopStatus) -> Self {
-        use MediaPlayerLoopStatus::*;
+impl From<LoopStatus> for ZValue<'_> {
+    fn from(value: LoopStatus) -> Self {
+        use LoopStatus::*;
         match value {
             None => "None",
             Track => "Track",
@@ -136,9 +146,9 @@ impl From<MediaPlayerLoopStatus> for ZValue<'_> {
     }
 }
 
-impl From<MediaPlayerPlaybackStatus> for ZValue<'_> {
-    fn from(value: MediaPlayerPlaybackStatus) -> Self {
-        use MediaPlayerPlaybackStatus::*;
+impl From<PlaybackStatus> for ZValue<'_> {
+    fn from(value: PlaybackStatus) -> Self {
+        use PlaybackStatus::*;
         match value {
             Paused => "Paused",
             Playing => "Playing",
@@ -154,8 +164,8 @@ impl From<TimeMicroSec> for ZValue<'_> {
     }
 }
 
-impl From<MediaPlayerMetadata> for ZValue<'_> {
-    fn from(MediaPlayerMetadata(value): MediaPlayerMetadata) -> Self {
+impl From<TrackMetadata> for ZValue<'_> {
+    fn from(TrackMetadata(value): TrackMetadata) -> Self {
         let mut dict = ZDict::new(
             "s".try_into().expect("invalid signature"),
             "v".try_into().expect("invalid signature"),
-- 
GitLab