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 `<hash>.<extension>` 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 <link2xt@testrun.org>
This commit is contained in:
Hocuri
2025-02-19 22:57:40 +01:00
committed by GitHub
parent 67f768fec0
commit 4a25860e22
3 changed files with 20 additions and 5 deletions

View File

@@ -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<String>,
file: Option<String>,
filename: Option<String>,
location: Option<(f64, f64)>,
quoted_message_id: Option<u32>,
) -> 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<String>,
file: Option<String>,
filename: Option<String>,
quoted_message_id: Option<u32>,
view_type: Option<MessageViewtype>,
) -> 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

View File

@@ -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<String>,
pub viewtype: Option<MessageViewtype>,
pub file: Option<String>,
pub filename: Option<String>,
pub location: Option<(f64, f64)>,
pub override_sender_name: Option<String>,
/// 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);

View File

@@ -124,6 +124,7 @@ class Chat:
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 @@ class Chat:
"html": html,
"viewtype": viewtype,
"file": file,
"filename": filename,
"location": location,
"overrideSenderName": override_sender_name,
"quotedMessageId": quoted_msg,
@@ -172,13 +174,14 @@ class Chat:
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."""