From 4a25860e22e55eb16731d57e7af4161c052dc568 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 19 Feb 2025 22:57:40 +0100 Subject: [PATCH] feat: Deduplicate blob files in the JsonRPC API (#6470) This makes it so that files will be deduplicated when using the JsonRPC API. @nicodh and @WofWca you know the Desktop code and how it is using the API, so, you can probably tell me whether this is a good way of changing the JsonRPC code - feel free to push changes directly to this PR here! This PR here changes the existing functions instead of creating new ones; we can alternatively create new ones if it allows for a smoother transition. This brings a few changes: - If you pass a file that is already in the blobdir, it will be renamed to `.` immediately (previously, the filename on the disk stayed the same) - If you pass a file that's not in the blobdir yet, it will be copied to the blobdir immediately (previously, it was copied to the blobdir later, when sending) - If you create a file and then pass it to `create_message()`, it's better to directly create it in the blobdir, since it doesn't need to be copied. - You must not write to the files after they were passed to core, because otherwise, the hash will be wrong. So, if Desktop recodes videos or so, then the video file mustn't just be overwritten. What you can do instead is write the recoded video to a file with a random name in the blobdir and then create a new message with the new attachment. If needed, we can also create a JsonRPC for `set_file_and_deduplicate()` that replaces the file on an existing message. In order to test whether everything still works, the desktop issue has a list of things to test: https://github.com/deltachat/deltachat-desktop/issues/4498 Core issue: #6265 --------- Co-authored-by: l --- deltachat-jsonrpc/src/api.rs | 10 +++++++--- deltachat-jsonrpc/src/api/types/message.rs | 10 +++++++++- deltachat-rpc-client/src/deltachat_rpc_client/chat.py | 5 ++++- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/deltachat-jsonrpc/src/api.rs b/deltachat-jsonrpc/src/api.rs index a3ab5c2e97..3c60855a51 100644 --- a/deltachat-jsonrpc/src/api.rs +++ b/deltachat-jsonrpc/src/api.rs @@ -1950,7 +1950,7 @@ impl CommandApi { let ctx = self.get_context(account_id).await?; let mut msg = Message::new(Viewtype::Sticker); - msg.set_file(&sticker_path, None); + msg.set_file_and_deduplicate(&ctx, Path::new(&sticker_path), None, None)?; // JSON-rpc does not need heuristics to turn [Viewtype::Sticker] into [Viewtype::Image] msg.force_sticker(); @@ -2170,12 +2170,14 @@ impl CommandApi { // mimics the old desktop call, will get replaced with something better in the composer rewrite, // the better version will just be sending the current draft, though there will be probably something similar with more options to this for the corner cases like setting a marker on the map + #[expect(clippy::too_many_arguments)] async fn misc_send_msg( &self, account_id: u32, chat_id: u32, text: Option, file: Option, + filename: Option, location: Option<(f64, f64)>, quoted_message_id: Option, ) -> Result<(u32, MessageObject)> { @@ -2187,7 +2189,7 @@ impl CommandApi { }); message.set_text(text.unwrap_or_default()); if let Some(file) = file { - message.set_file(file, None); + message.set_file_and_deduplicate(&ctx, Path::new(&file), filename.as_deref(), None)?; } if let Some((latitude, longitude)) = location { message.set_location(latitude, longitude); @@ -2215,12 +2217,14 @@ impl CommandApi { // the better version should support: // - changing viewtype to enable/disable compression // - keeping same message id as long as attachment does not change for webxdc messages + #[expect(clippy::too_many_arguments)] async fn misc_set_draft( &self, account_id: u32, chat_id: u32, text: Option, file: Option, + filename: Option, quoted_message_id: Option, view_type: Option, ) -> Result<()> { @@ -2237,7 +2241,7 @@ impl CommandApi { )); draft.set_text(text.unwrap_or_default()); if let Some(file) = file { - draft.set_file(file, None); + draft.set_file_and_deduplicate(&ctx, Path::new(&file), filename.as_deref(), None)?; } if let Some(id) = quoted_message_id { draft diff --git a/deltachat-jsonrpc/src/api/types/message.rs b/deltachat-jsonrpc/src/api/types/message.rs index aff2186fea..d47b61c2b2 100644 --- a/deltachat-jsonrpc/src/api/types/message.rs +++ b/deltachat-jsonrpc/src/api/types/message.rs @@ -1,3 +1,5 @@ +use std::path::Path; + use crate::api::VcardContact; use anyhow::{Context as _, Result}; use deltachat::chat::Chat; @@ -607,6 +609,7 @@ pub struct MessageData { pub html: Option, pub viewtype: Option, pub file: Option, + pub filename: Option, pub location: Option<(f64, f64)>, pub override_sender_name: Option, /// Quoted message id. Takes preference over `quoted_text` (see below). @@ -631,7 +634,12 @@ impl MessageData { message.set_override_sender_name(self.override_sender_name); } if let Some(file) = self.file { - message.set_file(file, None); + message.set_file_and_deduplicate( + context, + Path::new(&file), + self.filename.as_deref(), + None, + )?; } if let Some((latitude, longitude)) = self.location { message.set_location(latitude, longitude); diff --git a/deltachat-rpc-client/src/deltachat_rpc_client/chat.py b/deltachat-rpc-client/src/deltachat_rpc_client/chat.py index 9d33a5a0f4..3522e88992 100644 --- a/deltachat-rpc-client/src/deltachat_rpc_client/chat.py +++ b/deltachat-rpc-client/src/deltachat_rpc_client/chat.py @@ -124,6 +124,7 @@ def send_message( html: Optional[str] = None, viewtype: Optional[ViewType] = None, file: Optional[str] = None, + filename: Optional[str] = None, location: Optional[tuple[float, float]] = None, override_sender_name: Optional[str] = None, quoted_msg: Optional[Union[int, Message]] = None, @@ -137,6 +138,7 @@ def send_message( "html": html, "viewtype": viewtype, "file": file, + "filename": filename, "location": location, "overrideSenderName": override_sender_name, "quotedMessageId": quoted_msg, @@ -172,13 +174,14 @@ def set_draft( self, text: Optional[str] = None, file: Optional[str] = None, + filename: Optional[str] = None, quoted_msg: Optional[int] = None, viewtype: Optional[str] = None, ) -> None: """Set draft message.""" if isinstance(quoted_msg, Message): quoted_msg = quoted_msg.id - self._rpc.misc_set_draft(self.account.id, self.id, text, file, quoted_msg, viewtype) + self._rpc.misc_set_draft(self.account.id, self.id, text, file, filename, quoted_msg, viewtype) def remove_draft(self) -> None: """Remove draft message."""