[FEATURE REQUEST] Automatic removal of downloaded files and local storage clean up #4320

Aitorbp · 2024-02-19T14:24:39Z

Related Issues

Added changelog files for the fixed issues in folder changelog/unreleased. More info here

QA

Test plan execution: https://github.com/owncloud/QA/blob/master/Mobile/Android/Executions/Release_4.3/Automatic%20Removal.md

Bugs & improvements:

(1) Translations removed [FEATURE REQUEST] Automatic removal of downloaded files and local storage clean up #4320 (comment) [FIXED]
(2) 24 hours or 1 day [FEATURE REQUEST] Automatic removal of downloaded files and local storage clean up #4320 (comment) [FIXED]
(3) Wording in description [FEATURE REQUEST] Automatic removal of downloaded files and local storage clean up #4320 (comment) [FIXED]
(4) Effects of cleaning /tmp up [FEATURE REQUEST] Automatic removal of downloaded files and local storage clean up #4320 (comment) [FIXED]

Aitorbp · 2024-02-20T15:02:59Z

For temporal files will also be deleted according to their last modification date with respect to the selected time.

Aitorbp · 2024-02-22T09:30:58Z

I update you how is the new setting view:

JuancaG05

Some changes requested here @Aitorbp

changelog/unreleased/4320

owncloudApp/src/main/java/com/owncloud/android/dependecyinjection/UseCaseModule.kt

...oudApp/src/main/java/com/owncloud/android/usecases/files/DeleteFilesOlderGivenTimeUseCase.kt

owncloudApp/src/main/res/values/strings.xml

owncloudApp/src/main/java/com/owncloud/android/presentation/settings/advanced/AdvanceUtils.kt

owncloudData/src/main/java/com/owncloud/android/data/files/datasources/LocalFileDataSource.kt

...java/com/owncloud/android/data/files/datasources/implementation/OCLocalFileDataSourceTest.kt

owncloudData/src/test/java/com/owncloud/android/data/files/repository/OCFileRepositoryTest.kt

owncloudData/src/main/java/com/owncloud/android/data/files/db/FileDao.kt

JuancaG05

Some more changes here @Aitorbp

...pp/src/main/java/com/owncloud/android/presentation/settings/advanced/AdvanceSettingsUtils.kt

...rc/main/java/com/owncloud/android/presentation/settings/advanced/SettingsAdvancedFragment.kt

...c/main/java/com/owncloud/android/presentation/settings/advanced/SettingsAdvancedViewModel.kt

...java/com/owncloud/android/workers/RemoveLocallyFilesWithLastUsageOlderThanGivenTimeWorker.kt

owncloudData/src/test/java/com/owncloud/android/data/files/repository/OCFileRepositoryTest.kt

JuancaG05

LGTM now!

jesmrec · 2024-02-28T15:52:58Z

(1) [FIXED]

I noticed you removed the release notes from previous versions from the strings.xml file. That action can lead to compilation problems in release builds, because there are translation files in several languages that contain already translated strings like:

https://github.com/owncloud/android/blob/master/owncloudApp/src/main/res/values-de-rDE/strings.xml
https://github.com/owncloud/android/blob/master/owncloudApp/src/main/res/values-en-rGB/strings.xml

...and more.

Two options: either recovering the strings to string.xml or remove all legacy strings from all translated files.

IMO these actions are not related with the current PR, and should be done in separated place.

jesmrec · 2024-02-28T16:23:07Z

(2) [FIXED]

kind of stupid question:

what's the best option to display: 24 hours or 1 day?

As reference, iOS app uses 1 day.

open question

jesmrec · 2024-02-29T10:10:19Z

(3) [FIXED]

Another minor detail: in the description (Remove downloaded files automatically...), i'd add that available offline content is not removed.

Suggestion, open for discussion/improvement:

Remove automatically downloaded and temporary files that are not available offline, when the time since their last usage exceeds the selected time

Apart of that, you called "selected time" the option to be chosen by the user, so, i'd also call it in the selection line, instead of Current status:

What do you think?

…nTimeUseCase, OCFileRepository,LocalFileDataSource and FileDao

…Files in vm, handling prefDeleteLocalFiles in fragment, added component in xml and string

…n the number values

…function cancelAllWorkByTag in workManagerProvider and changed named worker and use case

…SettingsAdvancedViewModelTest

jesmrec · 2024-03-05T13:17:46Z

(4) [FIXED]

Some regards about the /tmp cleaning up. In /tmp folder are stored those files that are being uploaded. They are pushed to the server from that location without remaining local copy.

The current behaviour cleans totally up the /tmp folder, so that, if there are failed uploads, they are removed from there. What happens if the worker removes the /tmp files corresponding to a failed upload:

If the error was caused by a lack of connection and then retried, file is uploaded but with 0B and not reproducible ❌
If the error was caused by any other error, the retries cause a Folder Error ❌ , non recoverable
The worst case is Auto-Uploads with remove behaviour. In that case, the retries cause a Folder Error (non recoverable), and in addition, the original file is lost. So, we are talking about a data loss ❌ ❌ ❌ .

Also, in every case, files are removed from the internal storage but not from DB. This is not critical but could make the DB grow in uncontrolled way.

My suggested solutions are:

Not cleaning the /tmp: In any case, users have the choice of removing failed uploads with CLEAR option in the uploads view.
Remove everything also from BD
Cleaning only the finished uploads from storage and DB together. It sounds like a DB cleaning

I like 1. the most. So, the final scope would be the real storage itself. I know that all the mentioned cases are pretty corner cases, but if conditions happen it could break the app. And data loss is never valid.

…iles

jesmrec · 2024-03-05T16:23:33Z

My suggested solutions are:

Not cleaning the /tmp: In any case, users have the choice of removing failed uploads with CLEAR option in the uploads view.
Remove everything also from BD
Cleaning only the finished uploads from storage and DB together. It sounds like a DB cleaning

Taken solution was 1). In this iteration, /tmp will not be cleaned up to avoid mess with the failed uploads. Uploads can be handled in the uploads view, by cleaning the lists, the content in the /tmp is also cleaned up but one case: when an upload fails for a reason that is not a connection fail, retries are not removing files from /tmp, even with successful outcome.

That problem will be moved to another issue inside the epic.

For the current PR, it's approved on my side.

…min by 1 hour an removed author in header

Aitorbp self-assigned this Feb 19, 2024

Aitorbp linked an issue Feb 19, 2024 that may be closed by this pull request

[FEATURE REQUEST] Automatic removal of downloaded files and local storage clean up #4175

Closed

14 tasks

jesmrec added Dependencies Pull requests that update a dependency file Estimation - 1 (XS) Sprint and removed Estimation - 1 (XS) Sprint Dependencies Pull requests that update a dependency file labels Feb 20, 2024

Aitorbp requested a review from JuancaG05 February 20, 2024 15:00

Aitorbp force-pushed the feature/automatic_removal_downloaded_files_local_storage_clean_up branch 5 times, most recently from 5b818dd to 2edfd85 Compare February 21, 2024 21:47

JuancaG05 requested changes Feb 27, 2024

View reviewed changes

Aitorbp force-pushed the feature/automatic_removal_downloaded_files_local_storage_clean_up branch from 2d4e65b to b05c8f5 Compare February 27, 2024 16:12

Aitorbp requested a review from JuancaG05 February 27, 2024 16:27

Aitorbp force-pushed the feature/automatic_removal_downloaded_files_local_storage_clean_up branch from 6829116 to bf6142f Compare February 28, 2024 07:41

JuancaG05 requested changes Feb 28, 2024

View reviewed changes

Aitorbp requested a review from JuancaG05 February 28, 2024 14:56

Aitorbp force-pushed the feature/automatic_removal_downloaded_files_local_storage_clean_up branch from 4aa5879 to 4feb544 Compare February 28, 2024 15:16

JuancaG05 approved these changes Feb 28, 2024

View reviewed changes

Aitorbp added 3 commits March 4, 2024 10:46

Created getFilesLastUsageIsOlderThanGivenTime in DeleteFilesOlderGive…

36100bc

…nTimeUseCase, OCFileRepository,LocalFileDataSource and FileDao

Created worker, function to delete tmps, function scheduleDeleteLocal…

b0a9d97

…Files in vm, handling prefDeleteLocalFiles in fragment, added component in xml and string

Handle preview files and some fixes and named changes

0e2859d

Aitorbp added 7 commits March 4, 2024 10:46

Test in OCFileRepositoryTest and OCLocalFileDataSourceTest.

fe48243

Added new parameters in SettingsAdvancedViewModelTest

e3f3751

Add summary in ListPreference and updated release notes

b0b600f

Changed named of functions and made the separation with underscores i…

b5e25b8

…n the number values

Get millisecond value from preferences and not by parameter, created …

a521f96

…function cancelAllWorkByTag in workManagerProvider and changed named worker and use case

Updated tests in OCFileRepositoryTest, OCLocalFileDataSourceTest and …

3d4e328

…SettingsAdvancedViewModelTest

Update release notes and summary

7e80fab

Aitorbp force-pushed the feature/automatic_removal_downloaded_files_local_storage_clean_up branch from 59f2f64 to aa43a74 Compare March 4, 2024 10:47

JuancaG05 mentioned this pull request Mar 5, 2024

[FEATURE REQUEST] Automatic removal of downloaded files and local storage clean up #4175

Closed

14 tasks

Aitorbp added 7 commits March 5, 2024 14:17

Changed unit tests and added cancelAllWorkByTag before enqueue worker

0da5782

Changed named and changed func scheduleDeleteLocalFiles

6f41ae3

Changed 24 hours for 1 day in selector

4883b67

Changed string prefs_delete_local_files_summary

5461581

Removed all legacy strings from all translated files

9550d81

Changed repeatInterval worker to 60L

f5fa86b

Deleted clearTemporalFiles func and changed strings about temporary f…

a67d598

…iles

Aitorbp force-pushed the feature/automatic_removal_downloaded_files_local_storage_clean_up branch from ca3855a to a67d598 Compare March 5, 2024 14:18

Aitorbp and others added 2 commits March 5, 2024 14:19

Added calens

f004f01

Calens changelog updated

f3303b8

jesmrec added the Approved by qa label Mar 5, 2024

jesmrec mentioned this pull request Mar 5, 2024

[BUG] Retried successful uploads don't free space in /tmp folder #4335

Closed

Correcting format in strings, changed release note title, changed 60 …

d6162a3

…min by 1 hour an removed author in header

Aitorbp requested a review from JuancaG05 March 6, 2024 08:02

Aitorbp merged commit 28a602d into master Mar 6, 2024
5 checks passed

Aitorbp deleted the feature/automatic_removal_downloaded_files_local_storage_clean_up branch March 6, 2024 09:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FEATURE REQUEST] Automatic removal of downloaded files and local storage clean up #4320

[FEATURE REQUEST] Automatic removal of downloaded files and local storage clean up #4320

Aitorbp commented Feb 19, 2024 •

edited by jesmrec

Loading

Aitorbp commented Feb 20, 2024

Aitorbp commented Feb 22, 2024

JuancaG05 left a comment

JuancaG05 left a comment

JuancaG05 left a comment

jesmrec commented Feb 28, 2024 •

edited

Loading

jesmrec commented Feb 28, 2024 •

edited

Loading

jesmrec commented Feb 29, 2024 •

edited

Loading

jesmrec commented Mar 5, 2024 •

edited

Loading

jesmrec commented Mar 5, 2024 •

edited

Loading

[FEATURE REQUEST] Automatic removal of downloaded files and local storage clean up #4320

[FEATURE REQUEST] Automatic removal of downloaded files and local storage clean up #4320

Conversation

Aitorbp commented Feb 19, 2024 • edited by jesmrec Loading

Related Issues

QA

Aitorbp commented Feb 20, 2024

Aitorbp commented Feb 22, 2024

JuancaG05 left a comment

Choose a reason for hiding this comment

JuancaG05 left a comment

Choose a reason for hiding this comment

JuancaG05 left a comment

Choose a reason for hiding this comment

jesmrec commented Feb 28, 2024 • edited Loading

(1) [FIXED]

jesmrec commented Feb 28, 2024 • edited Loading

(2) [FIXED]

jesmrec commented Feb 29, 2024 • edited Loading

(3) [FIXED]

jesmrec commented Mar 5, 2024 • edited Loading

(4) [FIXED]

jesmrec commented Mar 5, 2024 • edited Loading

Aitorbp commented Feb 19, 2024 •

edited by jesmrec

Loading

jesmrec commented Feb 28, 2024 •

edited

Loading

jesmrec commented Feb 28, 2024 •

edited

Loading

jesmrec commented Feb 29, 2024 •

edited

Loading

jesmrec commented Mar 5, 2024 •

edited

Loading

jesmrec commented Mar 5, 2024 •

edited

Loading