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

Adapt to file deduplication #4498

Closed
1 of 2 tasks
Hocuri opened this issue Jan 16, 2025 · 4 comments · Fixed by #4604
Closed
1 of 2 tasks

Adapt to file deduplication #4498

Hocuri opened this issue Jan 16, 2025 · 4 comments · Fixed by #4604
Assignees
Labels
new DC feature New features in core that need adaptions in desktop

Comments

@Hocuri
Copy link

Hocuri commented Jan 16, 2025

We want to deduplicate blob files by always setting the file name to be the hash of the file content. Core issue: chatmail/core#6265, core PR: chatmail/core#6332

To be done:

  • When the user opens an attachment, copy it to a temp file with the correct name first Attachmentfiles on devices are rw - changes to attachements are not signaled to the user by DeltaChat #4352
  • Make sure that the actual name of the file isn't shown to the user or used to determine the file type (via the extension), instead dc_msg_get_filename() (C-FFI) or MessageObject.file_name (JsonRPC) needs to be used, similar to the Android PR at fix: Use getFilename() instead of the actual filename on disk deltachat-android#3521.
    • Search for usages of MessageObject.file, check where it's passed, and make sure that all it's never used to check the file extension, to pass it to an external application, or to show it to the user.
    • Look at places in the code where a filename is user-visible, like when viewing it in the UI or passing it to an external application (especially when sharing or opening it). Check where the filename comes from, and make sure that it comes from filename, not from file.
    • Test that
    1. sharing to and from DC

    2. opening

    3. saving ("Export attachment")

    4. drafting and re-entering the chat

    5. opening the drafted file once more

    6. sending

      still works for a) Images b) Videos c) vCards (attached contacts) d) attached files e) webxdc's. Also that editing and previewing drafted images works, and that previewing drafted videos works. (I hope I didn't forget to test any combination)

Then Desktop will be ready for using core with chatmail/core#6332 merged. Received files will be deduplicated then, but outgoing files not yet.

For deduplicating outgoing files, I will need to add a new JsonRPC API or modify the existing one to use set_file_and_deduplicate() instead of set_file() (or alternatively, one of you could do that). I will need to ask one of you how you are using the API so that I can design it in a way that works well for you. And then of course, you will need to create a Desktop PR similar to deltachat/deltachat-android#3513 that uses this new/changed API.

@link2xt
Copy link
Collaborator

link2xt commented Jan 25, 2025

This will need to be re-tested after merging chatmail/core#6470, but as long as it is not merged everything seems to be fine correctly. I am using desktop with the main core. Incoming attachments are deduplicated, filenames displayed and suggested for saving are all correct, vCards incoming and outgoing work as expected as well.

@nicodh
Copy link
Member

nicodh commented Jan 27, 2025

With #4564 file deduplication is in core and now in desktop also. Which means, opening an attachment will reveal the "real" filename, but since the file extension is kept " determine the file type (via the extension" is still working.

We should nevertheless implement the copy to tmp solution asap.

@nicodh nicodh self-assigned this Jan 27, 2025
@Hocuri
Copy link
Author

Hocuri commented Jan 27, 2025

since the file extension is kept

FTR, only the last part of the file extension is kept, e.g. in archive.tar.xz only the .xz extension will be kept.

@nicodh
Copy link
Member

nicodh commented Feb 1, 2025

Concerning "copy file to tmp dir" I doubt if that is the best solution.
I added a comment here #4352

Hocuri added a commit to chatmail/core that referenced this issue Feb 19, 2025
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:
deltachat/deltachat-desktop#4498
Core issue: #6265

---------

Co-authored-by: l <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new DC feature New features in core that need adaptions in desktop
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants