Update targetSdkversion to 29

[grzesiek2010]

Problem description

Working on #3189 we wanted to bump targetSdkversion directly to 29 but it turned out to be not that easy.

this doc seems to be the most relevant https://developer.android.com/about/versions/10/privacy/changes

We would need fixes at least for:

• External storage access scoped to app files and media
  
• Restriction on non-resettable device identifiers

[getodk-bot]

Hello @grzesiek2010, you have been unassigned from this issue because you have not updated this issue or any referenced pull requests for over 15 days.

You can reclaim this issue or claim any other issue by commenting @opendatakit-bot claim on that issue.

Thanks for your contributions, and hope to see you again soon!

[grzesiek2010]

The biggest problem for us is scoped storage and I'm in the 99% of people that hate it. Today starts Android Dev Summit 2019 and probably I'm naive but I hope they will say something like:

Hey, guys, it was a mistake we are going to let you use the storage, in the same way, like before.

If not we will have more info about it at least so I'm putting it on hold and will be back next week.

[lognaturel]

Makes sense! We're lucky that @seadowg will be at the Dev Summit getting information first hand. I hope you're right...

[lognaturel]

@seadowg, @grzesiek2010 and I discussed scoped storage on a call.

• primary reason Google has given for this change is to reduce "clutter" so that by default files get cleaned up when an app is removed
• @seadowg confirms everyone at dev summit hates scoped storage but that it's definitely coming into effect when targetSDK goes to 29
• Google has now said they'll require targeting the SDK version before the latest release (e.g. when API 30 is released, apps will need to target 29)
• Given prior Android version release pace, we can expect API 30 in fall 2020
• April 2019 blog post: "Scoped Storage will be required in next year’s major platform release for all apps, independent of target SDK level"
• Apparently it's still possible to use Context#getExternalFilesDir to get a directory that a user could access from adb but it will have a path like Android/data/org.odk.collect.android/files (will be cleared on app delete)

We agreed the next steps are:

• @lognaturel Draft up a message to users explaining the change and its implications
• @grzesiek2010 Confirm that using Context#getExternalFilesDir means files are accessible through adb
• @grzesiek2010 Confirm what happens when an app that uses Context#getExternalFilesDir is moved between SD card and internal storage. We expect files get moved over and relative paths just work
• @grzesiek2010 Migrate paths stored in the instances and forms databases to relative paths
• @grzesiek2010 Ensure all base paths are pulled out

Then, when we're ready to target API 29 and fully make the switch, all we should need to do is switch to Context#getExternalFilesDir and move /sdcard/odk there.

[grzesiek2010]

Confirm that using Context#getExternalFilesDir means files are accessible through adb

I can confirm that I was able to access files using eg.
adb push testfile /storage/emulated/0/Android/data/org.odk.collect.android/files

Confirm what happens when an app that uses Context#getExternalFilesDir is moved between SD card and internal storage. We expect files get moved over and relative paths just work

I had hard time trying to do such a migration and to no avail. I used various devices and every time receiving an exception like: Not enough storage space which wasn't true, on another device it's Unable to move the app...
but obviously it's how it should work so I'm 99% sure it will be ok (I'll try with more devices later).
The only thing we need is add android:installLocation="auto" to the Manifest file because by default apps are not movable.

[EDIT] Finally I managed to test moving apps to sd card. Looks as if the problem was caused by the fact I installed the sample app via USB, everything was fine using an apk file. I don't know why but it fixed the problem.

[getodk-bot]

Hello @grzesiek2010, you have been unassigned from this issue because you have not updated this issue or any referenced pull requests for over 15 days.

You can reclaim this issue or claim any other issue by commenting @opendatakit-bot claim on that issue.

Thanks for your contributions, and hope to see you again soon!

[lognaturel]

@grzesiek2010 has done the important exploration and learning tasks that we set out to do for v1.25. We've decided to wait on code changes so I'm removing from milestone.

[getodk-bot]

Hello @grzesiek2010, you have been unassigned from this issue because you have not updated this issue or any referenced pull requests for over 15 days.

You can reclaim this issue or claim any other issue by commenting @opendatakit-bot claim on that issue.

Thanks for your contributions, and hope to see you again soon!

[lognaturel]

There have been some exploration of approaches to getting the scoped storage migration started at #3444, #3460. I want to summarize at a high-level and hopefully figure out a direction.

The only thing we need is add android:installLocation="auto" to the Manifest file because by default apps are not movable.

I read more into app install location and I now think it's not relevant to us. I thought that moving the app would also move its data files but it does not. I think we can ignore this.

Some other questions that have come up for me:

• Is there really a benefit to using Context#getExternalFilesDir over Context#getFilesDir? Is it really the case that files from getFilesDir can't be accessed from adb? The docs state that the differences are that (1) getExternalFilesDir may not always be available since it could be removed by the user. This seems scary and undesirable. (2) that other apps can write to the directory if they have the Manifest.permission.WRITE_EXTERNAL_STORAGE permission. This seems useful but not a must-have, I think.

The #1 priority is that users can't lose saved and filled forms when they upgrade to the scoped storage version. This is non-negotiable. To achieve this, we need to move everything from /sdcard/odk/ to Context#getExternalFilesDir/odk. This needs to happen BEFORE we are forced to switch to scoped storage to be in the Play Store because at that point we won't be able to access /sdcard/odk/ from Collect. While we'd like to delay this change because it will be inconvenient to users, we can't delay it too much because we then risk not being able to do it at all.

"Moving everything" is tricky because it involves both form files and database files that refer to those form files. Currently the references in the database use absolute paths. If those paths were relative, then we could move the whole directory on upgrade and everything should just work. This is where the idea of changing paths in the databases to be relative came from. However, there are two problems:

• downgrading from versions with relative paths in the db to versions with absolute paths in the db would lead to broken behavior because old code can't handle relative paths.
• users don't have to install every version so in the version where we move the directory, we'd also need to first make sure that all paths in the databases are relative. This means that changing paths in a prior version is not very useful.

This leads me to think that for the upgrade path, we need to do the following all in the same new version:

• change paths in the database to be relative
• move all files currently in /sdcard/odk/ to Context#getExternalFilesDir/odk
• use Context#getExternalFilesDir to root paths in the database

Sadly I no longer see any way to break this up. I also realize that we probably need to design a user experience around this because we will need to block the UI while the file copy happens and there could potentially be large media files involved.

@grzesiek2010's exploration has focused around what to do if a user wants to downgrade from the Collect version that requires scoped storage version. Unfortunately, we've realized that because one of the side-effects of using Context#getExternalFilesDir is that those files get deleted on uninstall (without using the -r flag), there's not much point to having a downgrade path. This makes it especially important to get the user messaging right. We've talked about pairing this change with a major version bump and visual refresh. I still think that's probably the way to go but we need to be very careful because if we make overly dramatic or controversial changes, users will want to downgrade.

@grzesiek2010 can you please review this analysis carefully and see if I've overlooked anything?

[lognaturel]

Sadly I no longer see any way to break this up.

Luckily, @seadowg was feeling a little more creative than me and has suggested that we start in v1.26 with a change that makes installs that don't yet have an /sdcard/odk folder use Context#getExternalFilesDir/odk. That means those users will never need to switch over. Collect will need to be able to look for files in one of two places depending on whether /sdcard/odk/ exists or not. In the case where /sdcard/odk is used, paths in databases will be interpreted as absolute paths. In the case where Context#getExternalFilesDir/odk is used, paths in databases will be interpreted as relative paths. If this general concept seems reasonable, I'll file an issue with a more detailed specification.

I also realize that we probably need to design a user experience around this

In discussion with @seadowg, we realized it would be better to let the user drive when the change happens. That is, we'd have some new button or something that describes the change that needs to happen and lets the user trigger a migration of their files. That would let them choose a time when they don't have data on their device or make a backup first. Then at some point in the summer, we'd force the migration on upgrade and block the UI. We could add analytics to track the migration completion rate.

One additional thing to note is that even if we time this well, we're not guaranteed that users will upgrade to one of the versions before the the one that requires scoped storage. Folks who go straight from v1.25 to that version, for example, will not get a migration (because again, it will be impossible to do in a scoped storage world). Luckily, in that case, we can tell users how to move the files via adb and the sync tasks will take care of populating the databases.

[grzesiek2010]

we start in v1.26 with a change that makes installs that don't yet have an /sdcard/odk folder use Context#getExternalFilesDir/odk

you mean fresh installs won't use sd card right? I'm wondering in how many cases we can avoid copying files thanks to that, you know whether the game is worth the candle 😄
According to google play it's about 1k new installs every day compared to 280k of all active devices.

Assuming we for example add it in 1.26 and then force users to use Context#getExternalFilesDir in v1.27 we would have 2 months between those two releases and we could save ~60k out of ~300k users from copying files that's 20%. Of course if we have a longer break between v1.26 and scoped storage the number will increase.

The only thing we need is add android:installLocation="auto" to the Manifest file because by default apps are not movable.
I read more into app install location and I now think it's not relevant to us. I thought that moving the app would also move its data files but it does not. I think we can ignore this.

from testing a sample app I know that thing is needed to enable moving the app between internal and external storage, otherwise the Change button is not visible:

One additional thing to note is that even if we time this well, we're not guaranteed that users will upgrade to one of the versions before the the one that requires scoped storage. Folks who go straight from v1.25 to that version, for example, will not get a migration (because again, it will be impossible to do in a scoped storage world). Luckily, in that case, we can tell users how to move the files via adb and the sync tasks will take care of populating the databases.

That's right and that's the one of the few things we can agree on in 100% besides of the fact that we need to start using scope storage in the future.

[grzesiek2010]

we could save ~60k out of ~300k users from copying files that's 20%

to be honest that number 20% would be too big we have about 1.2k new installs every day but (fortunately) the number of uninstalls is lower (thanks to that our audience is growing) so in a few month the audience might be much bigger, but still.

[seadowg]

Assuming we for example add it in 1.26 and then force users to use Context#getExternalFilesDir in v1.27 we would have 2 months between those two releases and we could save ~60k out of ~300k users from copying files

@grzesiek2010 this is a great point. I guess Collect is a little strange in having this behaviour but it will definitely benefit us here!

[lognaturel]

Thanks so much for all the continued thought that has gone into this. I hope that together we can get it right for users! I've written separate forum posts about no longer being able to access IMEIs and needing to move off /sdcard/odk to get broader feedback. I'm continuing the conversation here for now but there may be crossover and we may choose to move the rest of the decision-making on the forum depending on what others have to say and who engages in the conversation.

from testing a sample app I know that thing is needed to enable moving the app between internal and external storage, otherwise the Change button is not visible:

Yes. What I was saying is that from what I now understand, the path represented by Context#getExternalFilesDir doesn't change in that case so it's not very helpful to users (because the app is ~8mb and it's the data that's big) and doesn't require any different approach for targeting API 29. The docs are quite explicit: "all private user data, databases, optimized .dex files, and extracted native code are saved on the internal device memory." That said, could you please quickly verify this with your test app and removable media? The test app would need to change the manifest flag and write a file to Context#getExternalFilesDir. Then you could move the app and see whether the file also moved or not.

you mean fresh installs won't use sd card right? I'm wondering in how many cases we can avoid copying files thanks to that, you know whether the game is worth the candle

The big thing that I hadn't considered in @seadowg's concept is letting users manually trigger the migration rather than doing it on install. This is better because it's less likely that users will be disrupted by or interrupt the migration if they trigger it themselves. I've since realized that we'd probably need to provide a manual trigger in case something went wrong like the phone running out of batteries part way through the data migration so it's work we'd need to do anyway. That means Collect needs to be able to handle both storage locations in the same version.

If we're going to do that anyway, we might as well give new users the best possible experience by setting them up with the new location right away. Does that reasoning make sense? As we discussed above, we will need to force a migration before August while we still have access to /sdcard/odk but hopefully by then most active users will be on the new storage location already (and we'll know because of analytics).

I previously thought that we were going to have to say that other applications could no longer access form definition or submission XML files. However, I think it's too useful and that we need to provide a mechanism for this (e.g. see https://github.com/opendatakit/skunkworks-crow or https://github.com/posm/OpenMapKitAndroid). I think we can do so by reviewing the existing FileProvider declaration in the manifest and adding externally-published activities that can be used to get content URIs for form definitions and submissions. Then this should be explicitly documented as part of the app's API.

[seadowg]

@lognaturel that makes sense to me. Thanks for taking the time to get that all down! The addition of the flag for migration started is a nice catch so we can handle recovery from any disaster scenarios.

I'm definitely in support of pulling out an interface that's responsible for returning file paths. That sounds like a nice first step that gets us ready withoyut any change in behaviour.

[grzesiek2010]

I agree that keeping file names instead of paths would be better but I'm a bit confused...
Do you think it's better to change all existed paths now (during copying files) or keep everything as is and handle everything in that new class you mentioned?

[lognaturel]

keeping file names instead of paths

In what I've described in the first bullet list here, what's stored are still paths. For example, for instance files, the instance directory and instance filename will both be included (identify-user-audit-false_2020-01-10_22-21-42/identify-user-audit-false_2020-01-10_22-21-42.xml). They're redundant but like I mentioned, that means the database schema doesn't have to change which seems simpler. We could just store the filename and derive the folder name from it but that doesn't feel like a big win to me and means some additional code changes. Does that make sense to you?

As a side note, I spent some time seeing whether we could derive that instance filename with other existing columns. It's in the structure form_name_creation_date so it seems like we should be able to. But it's not so straightforward because the database date field is converted from a unix timestamp to a formatted date using the device timezone at the time of writing the file. So that's another reason I don't think we should bother changing the schema -- we'd have to rename existing files which adds even more complexity.

Do you think it's better to change all existed paths now (during copying files) or keep everything as is and handle everything in that new class you mentioned?

Do you mean that we could keep the database files exactly as they are, move them, and then translate paths every time we read them from the databases? That is certainly possible but it feels really strange to me. I think that as part of the file moving operation, the databases should be modified to use relative paths. In the second bulleted list above, I suggested doing that as a separate operation from the file copy but it could possibly be simpler to do as files are copied, that will be up to you to discover. The important thing is that the existing database files are not modified. That way, if something goes wrong part way through the migration, everything still works with the original paths.

For paths from the database, I'm imagining that they would go through methods like getAbsoluteInstancePath or getAbsoluteCachePath. Currently, those methods would just return the same path that was passed in. Eventually, they would check whether the migration was run, and if it was, they would root them appropriately.

[grzesiek2010]

In what I've described in the first bullet list here, what's stored are still paths

Yeah, it will be a path in case of instances but in other cases forms/cache it will be just a file name.

As a side note, I spent some time seeing whether we could derive that instance filename with other existing columns. It's in the structure form_name_creation_date so it seems like we should be able to. But it's not so straightforward because the database date field is converted from a unix timestamp to a formatted date using the device timezone at the time of writing the file. So that's another reason I don't think we should bother changing the schema -- we'd have to rename existing files which adds even more complexity.

I agree. We could do even more then like remove formMediaPath column because it's always displayName + -media. Such improvements we can add in the future.

Do you mean that we could keep the database files exactly as they are, move them, and then translate paths every time we read them from the databases?

I'm in favor of converting existing paths so I'm happy we agree. I just misunderstood you and asked for clarification.

Ok no more questions for now. Thanks!

[getodk-bot]

Hello @grzesiek2010, you have been unassigned from this issue because you have not updated this issue or any referenced pull requests for over 15 days.

You can reclaim this issue or claim any other issue by commenting @opendatakit-bot claim on that issue.

Thanks for your contributions, and hope to see you again soon!

[lognaturel]

For anyone who might be following this issue, the scoped storage migration is now implemented and available to beta test: https://forum.opendatakit.org/t/odk-collect-v1-26-beta-1-scoped-storage-migration/25313. There is documentation on the forum: https://forum.opendatakit.org/t/odk-collect-v1-26-storage-migration/25268.

The migration is currently opt-in and will be forced in a release right before the requirement to target SDK 29 comes into effect.

Additionally, the beta includes a new installID preference which will take the place of the IMEI as the deviceID in the first release that targets SDK 29.

We will delay targeting SDK 29 until we are required to.

[lognaturel]

We've been talking about August 2020 as the time when we need to target 29 but I now think we may have until November. From https://support.google.com/googleplay/android-developer/answer/113469#targetsdk:

• August 3, 2020: Required for new apps
• November 2, 2020: Required for app updates

Does that sound right? I don't necessarily thing we should delay much beyond August but it'd be good to make sure we have the right "last chance" date in mind.

[seadowg]

Yeah that seems right. Given it looks like we'll have a September release maybe we should target that?

[lognaturel]

In v1.29 we will:

• Replace value of deviceID with existing value of installID, remove installId from settings menu
• Remove SIM serial and subscriber ID from settings menu~, remove READ_PHONE_STATE permission~ (still needed for phone number)
• Set target API to 29 and request to keep legacy storage on
• Force storage migration on first launch if it hasn't already been done. Try again on subsequent launch in case of failure. New installs should go straight to using scoped storage.

Those feel like different PRs to me but that'll be up to the person doing the implementation (likely @grzesiek2010). We should write acceptance criteria for the last point.

We don't know how long we will have with the requestLegacyExternalStorage flag being respected (and it's never guaranteed to be). I think we can continue targeting API 29 with the flag and forcing the migration on first launch until we need to target API 30 (likely Nov 2021). At that point we can remove the flag, remove the migration code, and only use scoped storage.

[lognaturel]

There have been further changes to the APIs and some new documentation released since I last checked. I skimmed and don't see anything that changes our plans but here are a few resources: FAQ, migration case study, use cases and best practices.

One thing I learned is that the preserveLegacyExternalStorage flag doesn't have any effect on new installs on Android 11 according to the docs. This contradicts @seadowg's experience at #4084 which is weird. Either way, I've updated the list above to reflect that new installs of v1.29+ should just go straight to using scoped storage.

[seadowg]

This contradicts @seadowg's experience at #4084 which is weird

We've been testing with requestLegacyExternalStorage not preserveLegacyExternalStorage. The difference is obvious right? Wow.

I think the first (requestLegacyExternalStorage) is for maintaining access to legacy storage while targeting Android 10 (and is ignored in Android 11). The second (preserveLegacyExternalStorage) is for maintaining access to legacy storage when targeting Android 11. The catch for the latter is:

Note: If you set preserveLegacyExternalStorage to true, the legacy storage model remains in effect only until the user uninstalls your app. If the user installs or reinstalls your app on a device that runs Android 11, then your app cannot opt out the scoped storage model, regardless of the value of preserveLegacyExternalStorage.

So it only works for updates to apps and not fresh installs.

If that is all true it means we could continue to run forced migrations when the app targets API 30 (Android 11) on update if we still feel that would be useful. I don't think this changes you todo list but it does give us an extra step we could carry out when moving to API 30.

[lognaturel]

The difference is obvious right? Wow.

🙈🙈🙈 My brain did not notice there were two. Wow indeed.