From 2b5df53cc3ac4f0e7b8ca5ad8f76a52444368090 Mon Sep 17 00:00:00 2001 From: James Magahern Date: Mon, 26 May 2025 16:19:26 -0700 Subject: [PATCH] better d-bus interface for attachments --- .../net.buzzert.kordophonecd.Server.xml | 47 ++++++++++++---- kordophoned/src/daemon/attachment_store.rs | 26 ++++++--- kordophoned/src/daemon/events.rs | 6 ++ kordophoned/src/daemon/mod.rs | 7 +++ kordophoned/src/dbus/server_impl.rs | 55 ++++++++----------- 5 files changed, 88 insertions(+), 53 deletions(-) diff --git a/kordophoned/include/net.buzzert.kordophonecd.Server.xml b/kordophoned/include/net.buzzert.kordophonecd.Server.xml index b29ed0c..59ddae6 100644 --- a/kordophoned/include/net.buzzert.kordophonecd.Server.xml +++ b/kordophoned/include/net.buzzert.kordophonecd.Server.xml @@ -78,22 +78,45 @@ - + - + - - - - - - - - + value="Returns attachment info: (file_path: string, downloaded: bool, file_size: uint64)"/> - + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/kordophoned/src/daemon/attachment_store.rs b/kordophoned/src/daemon/attachment_store.rs index 38f8324..33e8510 100644 --- a/kordophoned/src/daemon/attachment_store.rs +++ b/kordophoned/src/daemon/attachment_store.rs @@ -1,10 +1,10 @@ use std::{ - io::{BufReader, BufWriter, Read, Write}, - path::{Path, PathBuf}, + io::{BufWriter, Write}, + path::PathBuf, }; -use anyhow::{Error, Result}; -use futures_util::{poll, StreamExt}; +use anyhow::Result; +use futures_util::StreamExt; use kordophone::APIInterface; use thiserror::Error; use tokio::pin; @@ -64,20 +64,23 @@ impl AttachmentStore { } } - pub async fn download_attachent( + pub async fn download_attachment( &mut self, attachment: &Attachment, mut client_factory: F, ) -> Result<()> where C: APIInterface, - F: AsyncFnMut() -> Result, + F: FnMut() -> Fut, + Fut: std::future::Future>, { 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 + } } diff --git a/kordophoned/src/daemon/events.rs b/kordophoned/src/daemon/events.rs index 287f188..633d119 100644 --- a/kordophoned/src/daemon/events.rs +++ b/kordophoned/src/daemon/events.rs @@ -62,6 +62,12 @@ pub enum Event { /// - reply: Reply of the attachment object, if known. GetAttachment(String, Reply), + /// 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<()>), } diff --git a/kordophoned/src/daemon/mod.rs b/kordophoned/src/daemon/mod.rs index 5cee910..7012bf0 100644 --- a/kordophoned/src/daemon/mod.rs +++ b/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(); + } } } diff --git a/kordophoned/src/dbus/server_impl.rs b/kordophoned/src/dbus/server_impl.rs index cb94674..3ac69e6 100644 --- a/kordophoned/src/dbus/server_impl.rs +++ b/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::MethodErr> { - use crate::dbus::interface; - - self.send_event_sync(|r| Event::GetAttachment(attachment_id.clone(), r)) - .and_then(|attachment| { - let id: &str = attachment_id.split("-").take(1).last().unwrap(); - let obj_path = format!("/net/buzzert/kordophonecd/attachments/{}", &id); - 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()) + ) -> Result<(String, bool, u32), dbus::MethodErr> { + 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 + }; + + ( + attachment.path.to_string_lossy().to_string(), + attachment.downloaded, + file_size, + ) }) } + + 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 { - Ok(self.path.as_os_str().to_os_string().into_string().unwrap()) - } - - fn downloaded(&self) -> Result { - 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: F) -> Result where