Scoped storage migration #3396

abelgardep · 2021-09-29T10:44:52Z

Related Issues

Library PR (if needed): owncloud/android-library#436

QA

Test plan: https://github.com/owncloud/QA/blob/master/Mobile/Android/Release_2.19/Scoped%20Storage%20Migration.md

Reports:

JuancaG05

Some tiny suggestions @abelgardep, great job! 💯

...App/src/main/java/com/owncloud/android/presentation/ui/migration/StorageMigrationActivity.kt

...p/src/main/java/com/owncloud/android/presentation/viewmodels/migration/MigrationViewModel.kt

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

owncloudData/src/main/java/com/owncloud/android/data/storage/LocalStorageProvider.kt

owncloudData/src/main/java/com/owncloud/android/data/storage/LegacyStorageProvider.kt

owncloudData/src/main/java/com/owncloud/android/data/storage/ScopedStorageProvider.kt

fesave

Great job!!! Congratulations!!! 🥇

JuancaG05

💯

jesmrec · 2021-10-05T08:36:46Z

(1) [WONT FIX]

Display the wizard (by upgrading from prev version)

Current: in 1st slide, the Learn more points nothing

Expected: Learn more should point somewhere. If there is no a suitable way to point (complex or very technical information, or something not very friendly), i think we should remove it.

Pixel 2 Android 11
7253e2ae

jesmrec · 2021-10-05T08:48:49Z

(2) [WONT FIX]

How important is the Remind me later link in 1st slide? it is missing. Check here the mock up. Which is the expected behaviour? As @tbsbdr:

For those people, who really need the legacy storage-option for a longer transition period (until all apps migrate to the scoped storage) we should provide an option for them to postpone the migration to a convenient later time. I think we should be polite with our wizard and show it eg. only every 7 days (and also have the opportunity to start it directly form the settings). So the basic idea is: With postponing the migration a little bit, we buy some time for those people who still rely on legacy storage and eg. wait for soped-storage-upades for their eg. fancy pdf viewer.

How complex is this to implement? Is it technically correct keep using legacy storage when the app points to Android 11 SDK?

The "longer transition period" should not be very long, because all apps must do the update. Therefore, if we have to implement a complex logic to delay the migration, it would not be worthy because they will be updated soon.

More ideas welcome.

abelgardep · 2021-10-05T09:04:43Z

Related to (2), if we have a remind me later it adds more potential error cases.

Imagine that you do want to perform the migration later, new files will be downloaded to the scoped storage. Maybe there are some with the same hierarchy in the legacy one. After that, if we try to migrate, those files to scoped storage, some conflicts could appear.

We won't have full control over the files if we add a remind me later option

jesmrec · 2021-10-05T10:21:11Z

Related to (2), if we have a remind me later it adds more potential error cases.

Imagine that you do want to perform the migration later, new files will be downloaded to the scoped storage. Maybe there are some with the same hierarchy in the legacy one. After that, if we try to migrate, those files to scoped storage, some conflicts could appear.

We won't have full control over the files if we add a remind me later option

Is this ok for you to skip the "Remind later" option and perform migration just in the moment of the app upgrade? @theScrabi @michaelstingl @tbsbdr @JuancaG05 @fesave

Pros of removing "Remind me later": easier technically to implement (already done). Don't letting user to decide is straightforward and avoid confusions. Only one direction.

Pros of keeping "Remind me later": user decides when to upgrade.

Contras of keeping "Remind me later": More work to do, closer to the deadline. Users will have to wait for other apps to implement SAF inside (from November on, without Scoped Storage and SAF, not posible to update in Google Play) and could forget to trigger the migration. Conflicts can happen, because we will not maintain the legacy data anymore, causing a bigger problem.

michaelstingl · 2021-10-05T10:25:20Z

Pros of removing "Remind me later": easier technically to implement (already done). Don't letting user to decide is straightforward and avoid confusions. Only one direction.

I also prefer the simplicity of this approach.

jesmrec · 2021-10-05T11:57:10Z

(3)

Check this out:

When the wizard is displayed after upgrading:

If the app is killed, when it is opened again, the wizard is again there. ✅
If the device is switched off/rebooted, by opening again the app, migration is not done and wizard not displayed. New downloads are done correctly, but downloaded stuff before is lost.

As this is a corner case, it could happen only in case the device is running out of battery (we can guess the user will not power off the device at this moment). From my side, it does not make sense to develop a mechanism to prevent such scenario, but it could be interesting to add in the first slide a recommendation like "please don't switch your device off. If your battery is low, please plug it on before continuing the process"

what do you think?

jesmrec · 2021-10-05T16:42:58Z

(4) [FIXED]

Install previous version and add one account
Set some stuff as av. offline (not needed a big amount)
Migrate to new version (no matter if cleaning or keeping)

Current:

When the migration finishes, all av. offline files and folder are queued in uploads view to upload to the server

Expected:

Nothing changed, nothing should be uploaded

Pixel2 Android11
7253e2aee

jesmrec · 2021-10-05T17:00:40Z

Question -> Are log files migrated? @abelgardep

abelgardep · 2021-10-06T06:26:10Z

Question -> Are log files migrated? @abelgardep

Yes, they should, right? The point here is that in 2.18.1, logs are not stored inside a logs folder, they were directly in the root folder and they are migrated to the root folder instead of a logs folder and they won't be detected by the logs collector 🤔

jesmrec · 2021-10-06T07:05:53Z

This is the status after a migration:

the log file in owncloud root is migrated from the legacy storage, but, a new one is created inside /logs.

abelgardep · 2021-10-06T07:33:47Z

(4)

Install previous version and add one account

Set some stuff as av. offline (not needed a big amount)

Migrate to new version (no matter if cleaning or keeping)

Current:

When the migration finishes, all av. offline files and folder are queued in uploads view to upload to the server

Expected:

Nothing changed, nothing should be uploaded

Pixel2 Android11 7253e2aee

Migrating the files from legacy to scoped storage, at the moment, does not preserve copied file attributes such as creation/modification date, permissions, etc.

This is a problem for the AvOffline service since it compares the last modification timestamp in the local file with the last time we get data from the server. It means that it detects that something changed in the local storage and it enqueues a new upload.

There are two potential ways to fix this.

Find a way to migrate the files keeping the local attributes.
Alter the current AvOffline service so it won't need to enqueue new uploads when a local file changes.

The second one could be a good option since we will store the files in the scoped storage and other apps won't be able to modify them. The only way to modify files within scoped storage will be using the documents provider and it already performs a synchronization.

jesmrec · 2021-10-06T07:59:38Z

(5)

Download a big amount of files, so that the remaining space in the device is less than the space these files occupy
Migrate with "keep"/"clean" option -> no enough space

Current:

device runs into disk issues, and finally the app crashes

2021-10-06 09:53:19.727 29884-29925/? E/AndroidRuntime: FATAL EXCEPTION: DefaultDispatcher-worker-1
    Process: com.owncloud.android.debug, PID: 29884
    java.io.IOException: write failed: ENOSPC (No space left on device)
        at libcore.io.IoBridge.write(IoBridge.java:501)
        at java.io.FileOutputStream.write(FileOutputStream.java:316)
        at kotlin.io.ByteStreamsKt.copyTo(IOStreams.kt:108)
        at kotlin.io.FilesKt__UtilsKt.copyTo(Utils.kt:237)
        at kotlin.io.FilesKt__UtilsKt.copyTo$default(Utils.kt:217)
        at kotlin.io.FilesKt__UtilsKt.copyRecursively(Utils.kt:328)
        at kotlin.io.FilesKt__UtilsKt.copyRecursively$default(Utils.kt:291)
        at com.owncloud.android.data.storage.LocalStorageProvider.copyFileOrFolderToScopedStorage(LocalStorageProvider.kt:143)
        at com.owncloud.android.data.storage.LocalStorageProvider.copyLegacyToScopedStorage(LocalStorageProvider.kt:124)
        at com.owncloud.android.presentation.viewmodels.migration.MigrationViewModel$copyLegacyStorageToScopedStorage$1.invokeSuspend(MigrationViewModel.kt:73)
        at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
        at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
        at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:571)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:750)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:678)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:665)
     Caused by: android.system.ErrnoException: write failed: ENOSPC (No space left on device)
        at libcore.io.Posix.writeBytes(Native Method)
        at libcore.io.Posix.write(Posix.java:273)
        at libcore.io.BlockGuardOs.write(BlockGuardOs.java:319)

Is there any way to prevent the keep option if there is no enough space in the device?

7253e2ae

jesmrec · 2021-10-06T14:49:18Z

There are two potential ways to fix this.

Find a way to migrate the files keeping the local attributes.

Alter the current AvOffline service so it won't need to enqueue new uploads when a local file changes.

I'd go for 2, if the following steps pass:

Open oC file with external app that also supports SAF
Modify and save the file
File is saved in oC server

jesmrec · 2021-10-06T16:17:24Z

(6) [FIXED]

as discussed above

Yes, they should, right? The point here is that in 2.18.1, logs are not stored inside a logs folder, they were directly in the root folder and they are migrated to the root folder instead of a logs folder and they won't be detected by the logs collector 🤔

we'd go for an approach in which log files (out of the logs folder) will not be migrated. Log files are removed after a week and the content could be reproduced again. Otherwise, old logs will not be removed by the logs collector.

jesmrec · 2021-10-06T16:31:45Z

(7) [FIXED]

Small typo?:

safer would sound better. ;)

jesmrec · 2021-10-06T16:45:24Z

(8)

Finished uploads are lost in the uploads view after migrating, that means:

Install previous version
Perform some correct uploads -> they will appear as finished in the uploads view
Upgrade to this branch

Current:

Uploads view is empty

Expected:

Finished uploads in the list

Is there any regard in the migration? afaik, the list of finished uploads is stored in DB.

Nexus 6P v7
7253e2ae

jesmrec · 2021-10-07T06:42:29Z

About (6)

With commit bb166e2 , separated files are not migrated anymore. But, i saw that the migrated folders do not follow the expected hierarchy. Check the 10GB_10files folder inside the Scoped Storage:

folder with all the stuff hangs from owncloud instead from the account folder.

abelgardep · 2021-10-07T07:20:27Z

(8)

Finished uploads are lost in the uploads view after migrating, that means:

Install previous version

Perform some correct uploads -> they will appear as finished in the uploads view

Upgrade to this branch

Current:

Uploads view is empty

Expected:

Finished uploads in the list

Is there any regard in the migration? afaik, the list of finished uploads is stored in DB.

Nexus 6P v7 7253e2ae

Yes, I clean the finished uploads when migration is done. That way I just need to update where the pending uploads are stored.

abelgardep · 2021-10-18T09:43:13Z

Related to (5): Two scenarios

No local space to migrate and keep For example, 20 Gb of owncloud data and 12Gb free space left. We are not able to keep legacy and scoped storage. No Migrate and Keep option could do the work.
No local space to move a huge file in the Migrate and Clean option. For example, a 7Gb video, total owncloud data: 10Gb but 5Gb free space. We could try to migrate everything but the huge file.

…nside folders

… information

…migration

… space in device

tbsbdr · 2021-10-27T12:01:43Z

Text for version without "Keep old data"

Plaintext

(thx @mcarioscio-ownCloud for your review!)
Here comes some polished text. Also if we dont offer an option to keep old files, it must say "... files will be cleaned ..." (2nd screen)

Updated Text is italic

Text:

Your app has been updated. This app update migrates your files to a more secure location on your device. This one-time data migration is required and can take up to a minute or more.
Found 2.6 GB of data on your external storage. It will be moved to the safe storage on your device. Remaining files will be cleaned from your external storage after the migration to avoid duplicates and vulnerability.
Migrating your files. Please don’t turn your device off.
Migration completed successfully. Your files are now safer than ever before.

@abelgardep if the text is okay for you, could you please update the text in the wizard?

jesmrec · 2021-10-28T12:21:09Z

Regarding (5), i ran into space issues

Device has 8 GB free
Download 5GB inside the oC account (now 3GB left). I used 5 files 1GB each
Perform migration

Current: app crashes. After the crash:

Legacy storage: all the files are there.
Scoped Storage: 3 files copied

It seems that the old files were not removed at the time they were totally copied.

Expected: every file is copied to Scoped Storage and then, removed from legacy storage.

Nexus6P Android7
01a9fba8e

jesmrec · 2021-10-29T08:38:46Z

(11)

Set two accounts in old version
Download files in both accounts
Migrate to this version

Current: some downloaded files are displayed as repeated.
Expected: no repetitions

Pixel2 v11
Nexus6P v7

723f3bd8

…e space is not enough during migration.

jesmrec · 2021-11-02T15:44:24Z

Regarding (5)

With the latest commit 135e682 i have performed the following steps:

9GB available in device
Download a 5GB file and two 1GB files

So, we have only ~2GB available

Migration

After migration:

5GB file is downloaded (green mark)
1GB files are not downloaded (no green mark)

But, checking the Device explorer:

1GB files are complete
5GB file is not complete (1.8GB)

is there any way to remove pieces of file that were not totally migrated?
why is green mark in the file that was not totally migrated and not in the ones migrated?

…when migration fails.

… scoped storage

sonarqubecloud · 2021-11-02T18:15:26Z

SonarCloud Quality Gate failed.

721 Bugs
18 Vulnerabilities
26 Security Hotspots
4139 Code Smells

0.0% Coverage
2.5% Duplication

jesmrec · 2021-11-03T09:04:53Z

Everything fixed, ready to go!

abelgardep requested review from JuancaG05, fesave and theScrabi September 29, 2021 10:44

abelgardep self-assigned this Sep 29, 2021

abelgardep linked an issue Sep 29, 2021 that may be closed by this pull request

[FEATURE REQUEST] Scoped Storage Migration from legacy storage #3327

Closed

14 tasks

JuancaG05 requested changes Sep 29, 2021

View reviewed changes

fesave approved these changes Sep 30, 2021

View reviewed changes

JuancaG05 approved these changes Sep 30, 2021

View reviewed changes

abelgardep force-pushed the feature/scoped_storage branch from 2a6a99a to 46f0deb Compare October 7, 2021 07:27

abelgardep force-pushed the scoped_storage_migration branch from bb166e2 to 818a5a9 Compare October 7, 2021 07:34

abelgardep force-pushed the feature/scoped_storage branch from 46f0deb to 24621a6 Compare October 22, 2021 10:13

abelgardep and others added 7 commits October 26, 2021 13:04

Do not migrate independant files in root folder. Important ones are i…

deaf824

…nside folders

Fix a little typo

4ae7be0

Respect previous hierarchy when migrating files

9523af6

Remove Learn more text since we don't have a proper link to show more…

ace589d

… information

Update local path after migration. Do not upload the file after this …

69790c0

…migration

First approximation of moveRecursively() extension performed.

a702167

"Migrate and keep old files" will only be available if there's enough…

bd89c41

… space in device

abelgardep force-pushed the scoped_storage_migration branch from 43168bd to bd89c41 Compare October 26, 2021 11:05

abelgardep added 4 commits October 26, 2021 13:06

Remove unnecessary code after rebasing

d4a3f8c

Move files properly

2423f63

Remove media scan stuff, no need after moving to scoped storage

02f839c

Pleasure the ktlint

da8cdf6

Update wording and remove the option to keep the legacy storage

01a9fba

jesmrec mentioned this pull request Oct 28, 2021

[BUG] Files whose names differ only in case may point to the same one when available offline #2690

Closed

Fix moveRecursively function

723f3bd

fesave and others added 4 commits November 2, 2021 09:29

An insufficient space warning has been added before migrating files.

8742bc1

An insufficient space warning has been improved before migrating files.

3f035cf

The application has been improved, now it does not crash when the fre…

5cb842c

…e space is not enough during migration.

Fix duplications after migrating the files to scoped storage

135e682

fesave and others added 2 commits November 2, 2021 18:06

The application has been improved, now an incomplete file is deleted …

79dbafa

…when migration fails.

Delete recursively the legacy storage after a successful migration to…

c60fba6

… scoped storage

jesmrec added the Approved by qa label Nov 3, 2021

abelgardep merged commit 6fcf701 into feature/scoped_storage Nov 3, 2021

abelgardep deleted the scoped_storage_migration branch November 3, 2021 17:06

Scoped storage migration #3396

Scoped storage migration #3396

Uh oh!

Conversation

abelgardep commented Sep 29, 2021 • edited by jesmrec Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related Issues

QA

Uh oh!

JuancaG05 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fesave left a comment

Choose a reason for hiding this comment

Uh oh!

JuancaG05 left a comment

Choose a reason for hiding this comment

Uh oh!

jesmrec commented Oct 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

(1) [WONT FIX]

Uh oh!

jesmrec commented Oct 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

(2) [WONT FIX]

Uh oh!

abelgardep commented Oct 5, 2021

Uh oh!

jesmrec commented Oct 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michaelstingl commented Oct 5, 2021

Uh oh!

jesmrec commented Oct 5, 2021

(3)

Uh oh!

jesmrec commented Oct 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

(4) [FIXED]

Uh oh!

jesmrec commented Oct 5, 2021

Uh oh!

abelgardep commented Oct 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jesmrec commented Oct 6, 2021

Uh oh!

abelgardep commented Oct 6, 2021

(4)

Uh oh!

jesmrec commented Oct 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

(5)

Uh oh!

jesmrec commented Oct 6, 2021

Uh oh!

jesmrec commented Oct 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

(6) [FIXED]

Uh oh!

jesmrec commented Oct 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

(7) [FIXED]

Uh oh!

jesmrec commented Oct 6, 2021

(8)

Uh oh!

jesmrec commented Oct 7, 2021

Uh oh!

abelgardep commented Oct 7, 2021

(8)

Uh oh!

abelgardep commented Oct 18, 2021

Uh oh!

abelgardep commented Sep 29, 2021 •

edited by jesmrec

Loading

jesmrec commented Oct 5, 2021 •

edited

Loading

jesmrec commented Oct 5, 2021 •

edited

Loading

jesmrec commented Oct 5, 2021 •

edited

Loading

jesmrec commented Oct 5, 2021 •

edited

Loading

abelgardep commented Oct 6, 2021 •

edited

Loading

jesmrec commented Oct 6, 2021 •

edited

Loading

jesmrec commented Oct 6, 2021 •

edited

Loading

jesmrec commented Oct 6, 2021 •

edited

Loading

tbsbdr commented Oct 27, 2021 •

edited

Loading

jesmrec commented Oct 28, 2021 •

edited

Loading

jesmrec commented Nov 2, 2021 •

edited

Loading