better d-bus interface for attachments

kordophoned/include/net.buzzert.kordophonecd.Server.xml

@@ -78,22 +78,45 @@
 
         <!-- Attachments -->
 
-        <method name="GetAttachment">
+        <method name="GetAttachmentInfo">
             <arg type="s" name="attachment_id" direction="in"/>
-            <arg type="o" name="attachment_obj" direction="out"/>
+            <arg type="(sbu)" name="attachment_info" direction="out"/>
             <annotation name="org.freedesktop.DBus.DocString"
-                        value="Returns a D-Bus object path for accessing the requested attachment."/>
+                        value="Returns attachment info: (file_path: string, downloaded: bool, file_size: uint64)"/>
-        </method>
-    </interface>
-
-    <interface name="net.buzzert.kordophone.Attachment">
-        <property name="FilePath"   type="s" access="readonly" />
-        <property name="Downloaded" type="b" access="readonly" />
-
-        <method name="Delete">
         </method>
 
-        <signal name="DownloadedStateChanged" />
+        <method name="DownloadAttachment">
+            <arg type="s" name="attachment_id" direction="in"/>
+            <annotation name="org.freedesktop.DBus.DocString"
+                        value="Initiates download of the specified attachment if not already downloaded."/>
+        </method>
+
+        <signal name="AttachmentDownloadStarted">
+            <arg type="s" name="attachment_id"/>
+            <annotation name="org.freedesktop.DBus.DocString"
+                        value="Emitted when an attachment download begins."/>
+        </signal>
+
+        <signal name="AttachmentDownloadProgress">
+            <arg type="s" name="attachment_id"/>
+            <arg type="d" name="progress"/>
+            <annotation name="org.freedesktop.DBus.DocString"
+                        value="Emitted during attachment download. Progress is a value from 0.0 to 1.0."/>
+        </signal>
+
+        <signal name="AttachmentDownloadCompleted">
+            <arg type="s" name="attachment_id"/>
+            <arg type="s" name="file_path"/>
+            <annotation name="org.freedesktop.DBus.DocString"
+                        value="Emitted when an attachment download completes successfully."/>
+        </signal>
+
+        <signal name="AttachmentDownloadFailed">
+            <arg type="s" name="attachment_id"/>
+            <arg type="s" name="error_message"/>
+            <annotation name="org.freedesktop.DBus.DocString"
+                        value="Emitted when an attachment download fails."/>
+        </signal>
     </interface>
 
     <interface name="net.buzzert.kordophone.Settings">

kordophoned/src/daemon/attachment_store.rs

@@ -1,10 +1,10 @@
 use std::{
-    io::{BufReader, BufWriter, Read, Write},
+    io::{BufWriter, Write},
-    path::{Path, PathBuf},
+    path::PathBuf,
 };
 
-use anyhow::{Error, Result};
+use anyhow::Result;
-use futures_util::{poll, StreamExt};
+use futures_util::StreamExt;
 use kordophone::APIInterface;
 use thiserror::Error;
 use tokio::pin;
@@ -64,20 +64,23 @@ impl AttachmentStore {
         }
     }
 
-    pub async fn download_attachent<C, F>(
+    pub async fn download_attachment<C, F, Fut>(
         &mut self,
         attachment: &Attachment,
         mut client_factory: F,
     ) -> Result<()>
     where
         C: APIInterface,
-        F: AsyncFnMut() -> Result<C>,
+        F: FnMut() -> Fut,
+        Fut: std::future::Future<Output = Result<C>>,
     {
         if attachment.downloaded {
-            log::error!(target: target::ATTACHMENTS, "Attempted to download existing attachment.");
+            log::info!(target: target::ATTACHMENTS, "Attachment already downloaded: {}", attachment.guid);
             return Err(AttachmentStoreError::AttachmentAlreadyDownloaded.into());
         }
 
+        log::info!(target: target::ATTACHMENTS, "Starting download for attachment: {}", attachment.guid);
+
         // Create temporary file first, we'll atomically swap later.
         assert!(!std::fs::exists(&attachment.path).unwrap());
         let file = std::fs::File::create(&attachment.path)?;
@@ -85,7 +88,7 @@ impl AttachmentStore {
 
         log::trace!(target: target::ATTACHMENTS, "Created attachment file at {}", &attachment.path.display());
 
-        let mut client = (client_factory)().await?;
+        let mut client = client_factory().await?;
         let stream = client
             .fetch_attachment_data(&attachment.guid)
             .await
@@ -99,6 +102,13 @@ impl AttachmentStore {
             writer.write(data.as_ref())?;
         }
 
+        log::info!(target: target::ATTACHMENTS, "Completed download for attachment: {}", attachment.guid);
         Ok(())
     }
+    
+    /// Check if an attachment should be downloaded
+    pub fn should_download(&self, attachment_id: &str) -> bool {
+        let attachment = self.get_attachment(&attachment_id.to_string());
+        !attachment.downloaded
+    }
 }

kordophoned/src/daemon/events.rs

@@ -62,6 +62,12 @@ pub enum Event {
     /// - reply: Reply of the attachment object, if known.
     GetAttachment(String, Reply<Attachment>),
 
+    /// Downloads an attachment from the server.
+    /// Parameters:
+    /// - attachment_id: The attachment ID to download
+    /// - reply: Reply indicating success or failure
+    DownloadAttachment(String, Reply<()>),
+
     /// Delete all conversations from the database.
     DeleteAllConversations(Reply<()>),
 }

kordophoned/src/daemon/mod.rs

@@ -60,6 +60,7 @@ pub mod target {
     pub static EVENT: &str = "event";
     pub static SETTINGS: &str = "settings";
     pub static UPDATES: &str = "updates";
+    pub static ATTACHMENTS: &str = "attachments";
 }
 
 pub struct Daemon {
@@ -284,6 +285,12 @@ impl Daemon {
                 let attachment = self.attachment_store.get_attachment(&guid);
                 reply.send(attachment).unwrap();
             }
+
+            Event::DownloadAttachment(attachment_id, reply) => {
+                // For now, just return success - we'll implement the actual download logic later
+                log::info!(target: target::ATTACHMENTS, "Download requested for attachment: {}", attachment_id);
+                reply.send(()).unwrap();
+            }
         }
     }
 

kordophoned/src/dbus/server_impl.rs

@@ -12,7 +12,6 @@ use crate::daemon::{
 };
 
 use crate::dbus::endpoint::DbusRegistry;
-use crate::dbus::interface::NetBuzzertKordophoneAttachment as DbusAttachment;
 use crate::dbus::interface::NetBuzzertKordophoneRepository as DbusRepository;
 use crate::dbus::interface::NetBuzzertKordophoneSettings as DbusSettings;
 
@@ -159,27 +158,32 @@ impl DbusRepository for ServerImpl {
             .map(|uuid| uuid.to_string())
     }
 
-    fn get_attachment(
+    fn get_attachment_info(
         &mut self,
         attachment_id: String,
-    ) -> Result<dbus::Path<'static>, dbus::MethodErr> {
+    ) -> Result<(String, bool, u32), dbus::MethodErr> {
-        use crate::dbus::interface;
+        self.send_event_sync(|r| Event::GetAttachment(attachment_id, r))
+            .map(|attachment| {
+                let file_size = if attachment.downloaded {
+                    std::fs::metadata(&attachment.path)
+                        .map(|m| m.len() as u32)
+                        .unwrap_or(0)
+                } else {
+                    0
+                };
                 
-        self.send_event_sync(|r| Event::GetAttachment(attachment_id.clone(), r))
+                (
-            .and_then(|attachment| {
+                    attachment.path.to_string_lossy().to_string(),
-                let id: &str = attachment_id.split("-").take(1).last().unwrap();
+                    attachment.downloaded,
-                let obj_path = format!("/net/buzzert/kordophonecd/attachments/{}", &id);
+                    file_size,
-                log::trace!("Registering attachment at path: {}", &obj_path);
+                )
-
-                self.dbus_registry.register_object(
-                    &obj_path,
-                    attachment,
-                    |cr| vec![interface::register_net_buzzert_kordophone_attachment(cr)]
-                );
-
-                Ok(obj_path.into())
             })
     }
+
+    fn download_attachment(&mut self, attachment_id: String) -> Result<(), dbus::MethodErr> {
+        // For now, just trigger the download event - we'll implement the actual download logic later
+        self.send_event_sync(|r| Event::DownloadAttachment(attachment_id, r))
+    }
 }
 
 impl DbusSettings for ServerImpl {
@@ -233,21 +237,6 @@ impl DbusSettings for ServerImpl {
     }
 }
 
-impl DbusAttachment for Attachment {
-    fn file_path(&self) -> Result<String, dbus::MethodErr> {
-        Ok(self.path.as_os_str().to_os_string().into_string().unwrap())
-    }
-
-    fn downloaded(&self) -> Result<bool, dbus::MethodErr> {
-        Ok(self.downloaded)
-    }
-
-    fn delete(&mut self) -> Result<(), dbus::MethodErr> {
-        // Mostly a placeholder method because dbuscodegen for some reason barfs on this
-        // if there are no methods defined.
-        todo!()
-    }
-}
 
 fn run_sync_future<F, T>(f: F) -> Result<T, MethodErr>
 where