Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Deduplicate blob files in the JsonRPC API #6470

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Hocuri
Copy link
Collaborator

@Hocuri Hocuri commented Jan 24, 2025

This makes it so that files will be deduplicated when using the JsonRPC API.

Desktop issue: deltachat/deltachat-desktop#4498

@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!

There is no need to rush merging this PR; if you want, you can first concentrate on the tasks described in the desktop issue.

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() and similar, 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.

Not sure if there is any documentation of the JsonRPC API that needs to be changed?

Core issue: #6265

@Hocuri Hocuri changed the title feat: Deduplicate blob files in the JsonRPC API [WIP] feat: Deduplicate blob files in the JsonRPC API Jan 24, 2025
@Hocuri Hocuri force-pushed the hoc/add-deduplication-jsonrpc branch from 0d5c3ad to 7663144 Compare January 24, 2025 10:22
@@ -2157,12 +2157,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
#[allow(clippy::too_many_arguments)]
async fn misc_send_msg(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure whether it's necessary to change this function and misc_set_draft() since it's going to be replaced anyway?

Is this function and misc_set_draft() still in use in the Desktop UI?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently it is still used, did a quick grep for miscSetDraft.

@Hocuri Hocuri changed the title [WIP] feat: Deduplicate blob files in the JsonRPC API feat: Deduplicate blob files in the JsonRPC API Jan 24, 2025
@Hocuri Hocuri requested review from nicodh and WofWca January 24, 2025 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants