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

[Aitorbp]

Related Issues

App: https://github.com/owncloud/android/issues/4175

• [x] 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 https://github.com/owncloud/android/pull/4320#issuecomment-1969283349
• [X] (2) 24 hours or 1 day https://github.com/owncloud/android/pull/4320#issuecomment-1969355907 [FIXED]
• [ ] (3) Wording in description https://github.com/owncloud/android/pull/4320#issuecomment-1970810580

[Aitorbp]

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

[Aitorbp]

I update you how is the new setting view:

[jesmrec]

(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]

(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]

(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?

[jesmrec]

(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:

1. Not cleaning the /tmp: In any case, users have the choice of removing failed uploads with CLEAR option in the uploads view.
2. Remove everything also from BD
3. 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.

[jesmrec]

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.