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

fix(android): get external files directory for Android 10+ #317

Merged
merged 6 commits into from
Oct 22, 2021

Conversation

erisu
Copy link
Member

@erisu erisu commented Oct 6, 2021

Platforms affected

Android

Motivation and Context

To resolve mounted storage for Android 11.
Android 11 (API 30) has deprecated the method Environment.getExternalStorageDirectory().

Description

This PR has replaced the Environment.getExternalStorageDirectory() usage with context.getExternalFilesDir(null).

Per Android documentation, getExternalFilesDir is one of the suggested alternative solutions.

PR also includes a little bit of refactoring to remove code duplication that related to the calling of getExternalStorageDirectory, now getExternalFilesDir.

Testing

  • In-Progress...

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change
  • Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
  • If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
  • I've updated the documentation if necessary

@erisu erisu added this to the 5.0.4 milestone Oct 6, 2021
@erisu erisu requested a review from breautek October 6, 2021 08:07
@erisu erisu changed the title fix(android): getting external files directory for Android 10+ fix(android): get external files directory for Android 10+ Oct 6, 2021
Copy link

@breautek breautek left a comment

Choose a reason for hiding this comment

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

lgtm -- later tonight I'll try to give it an actual test

Copy link

@retoko retoko left a comment

Choose a reason for hiding this comment

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

From google docs:

Starting in November 2021, app updates will be required to target API level 30 or above

What needs to release this fix? any help?

@erisu erisu marked this pull request as ready for review October 14, 2021 12:53
@erisu
Copy link
Member Author

erisu commented Oct 21, 2021

Last call for reviews. This PR will be merged in tomorrow (JST).

@erisu erisu requested a review from breautek October 21, 2021 12:40
@erisu erisu merged commit 4093f7e into apache:master Oct 22, 2021
@erisu erisu deleted the fix/get-external-dir branch October 22, 2021 02:50
klaw-PLS added a commit to klaw-PLS/cordova-plugin-media that referenced this pull request Jan 17, 2022
As stated in issue apache#325, it looks like PR apache#317 missed a spot in the corrections it was making.  This commit adds that fix to the missed location.
@klaw-PLS klaw-PLS mentioned this pull request Jan 18, 2022
5 tasks
erisu pushed a commit that referenced this pull request Apr 25, 2022
As stated in issue #325, it looks like PR #317 missed a spot in the corrections it was making.  This commit adds that fix to the missed location.
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