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

Only Download Necessary Files #13683

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

alperozturk96
Copy link
Collaborator

@alperozturk96 alperozturk96 commented Oct 2, 2024

  • Tests written, or not not needed

Problem

Downloaded files are re-downloading, and notifications appear for files that have already been downloaded. The sync should only trigger downloads for new or editable files, as media files cannot be changed.

The two-way sync triggers the FileDownloadWorker every 15 minutes. If a folder contains multiple files, even if all files are already downloaded, the worker will still show notifications for each file and attempt to re-download them again.

Note: Images can be edited, but any edited image will create a copy rather than modifying the original file. Thus, no need to download original image.

Changes

  • Download only files that haven't been downloaded and aren't media files.
  • Return early if requestDownloads is empty.

Demo

test.mp4

Copy link

github-actions bot commented Oct 2, 2024

Codacy

Lint

TypemasterPR
Warnings5959
Errors33

SpotBugs

CategoryBaseNew
Bad practice6464
Correctness6363
Dodgy code296296
Experimental11
Internationalization77
Malicious code vulnerability11
Multithreaded correctness66
Performance5353
Security1818
Total509509

@tobiasKaminsky
Copy link
Member

Can you explain reason behind filtering media files?

This worker is also used when exporting files (FilesExportWork.kt), so this would then not work if you want to export a pdf file?

Copy link
Member

@tobiasKaminsky tobiasKaminsky left a comment

Choose a reason for hiding this comment

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

see comment

@alperozturk96
Copy link
Collaborator Author

Can you explain reason behind filtering media files?

This worker is also used when exporting files (FilesExportWork.kt), so this would then not work if you want to export a pdf file?

Screenshot 2024-10-08 at 11 49 09

FilesExportWork only triggers the FileDownloadWorker when a file hasn’t been downloaded, so the filter introduced in this PR won’t be applied.

Here’s a demo of FilesExportWork with already downloaded files.

exp.mp4

You can check the PR description for more details on the purpose of this PR.

@fegauthier
Copy link

I'm eagerly waiting for this PR. I have a Google Pixel 8, and files are constantly resyncing every 15 minutes. It drains the battery significantly and uses bandwidth unnecessarily. Do you guys know when it will be merged?

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants