You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by GitBox <gi...@apache.org> on 2021/10/06 08:07:39 UTC
[GitHub] [cordova-plugin-media] erisu opened a new pull request #317: fix(android): getting external files directory for Android 10+
erisu opened a new pull request #317:
URL: https://github.com/apache/cordova-plugin-media/pull/317
### 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
- [x] 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](https://help.github.com/articles/closing-issues-using-keywords/))
- [ ] I've updated the documentation if necessary
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-media] erisu commented on pull request #317: fix(android): get external files directory for Android 10+
Posted by GitBox <gi...@apache.org>.
erisu commented on pull request #317:
URL: https://github.com/apache/cordova-plugin-media/pull/317#issuecomment-948574889
Last call for reviews.This PR will be merged in tomorrow (JST).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-media] retoko commented on a change in pull request #317: fix(android): get external files directory for Android 10+
Posted by GitBox <gi...@apache.org>.
retoko commented on a change in pull request #317:
URL: https://github.com/apache/cordova-plugin-media/pull/317#discussion_r728891176
##########
File path: src/android/AudioPlayer.java
##########
@@ -101,19 +103,29 @@ Licensed to the Apache Software Foundation (ASF) under one
*/
public AudioPlayer(AudioHandler handler, String id, String file) {
this.handler = handler;
+ context = handler.getApplicationContext();
this.id = id;
this.audioFile = file;
this.tempFiles = new LinkedList<String>();
+
}
- private String generateTempFile() {
- String tempFileName = null;
- if (Environment.getExternalStorageState().equals(Environment.MEDIA_MOUNTED)) {
- tempFileName = Environment.getExternalStorageDirectory().getAbsolutePath() + "/tmprecording-" + System.currentTimeMillis() + ".3gp";
- } else {
- tempFileName = "/data/data/" + handler.cordova.getActivity().getPackageName() + "/cache/tmprecording-" + System.currentTimeMillis() + ".3gp";
- }
- return tempFileName;
+ /**
+ * Creates an audio file path from the provided fileName or creates a new temporary file path.
+ *
+ * @param fileName the audio file name, if null a temporary 3gp file name is provided
+ * @return String
+ */
+ private String createAudioFilePath(String fileName) {
+ File dir = Environment.getExternalStorageState().equals(Environment.MEDIA_MOUNTED)
+ ? context.getExternalFilesDir(null)
+ : context.getCacheDir();
+
+ fileName = fileName.isEmpty()
Review comment:
Thanks, I tested this on androids 9,10,11 and it works!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-media] erisu merged pull request #317: fix(android): get external files directory for Android 10+
Posted by GitBox <gi...@apache.org>.
erisu merged pull request #317:
URL: https://github.com/apache/cordova-plugin-media/pull/317
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-media] retoko commented on a change in pull request #317: fix(android): get external files directory for Android 10+
Posted by GitBox <gi...@apache.org>.
retoko commented on a change in pull request #317:
URL: https://github.com/apache/cordova-plugin-media/pull/317#discussion_r728828485
##########
File path: src/android/AudioPlayer.java
##########
@@ -101,19 +103,29 @@ Licensed to the Apache Software Foundation (ASF) under one
*/
public AudioPlayer(AudioHandler handler, String id, String file) {
this.handler = handler;
+ context = handler.getApplicationContext();
this.id = id;
this.audioFile = file;
this.tempFiles = new LinkedList<String>();
+
}
- private String generateTempFile() {
- String tempFileName = null;
- if (Environment.getExternalStorageState().equals(Environment.MEDIA_MOUNTED)) {
- tempFileName = Environment.getExternalStorageDirectory().getAbsolutePath() + "/tmprecording-" + System.currentTimeMillis() + ".3gp";
- } else {
- tempFileName = "/data/data/" + handler.cordova.getActivity().getPackageName() + "/cache/tmprecording-" + System.currentTimeMillis() + ".3gp";
- }
- return tempFileName;
+ /**
+ * Creates an audio file path from the provided fileName or creates a new temporary file path.
+ *
+ * @param fileName the audio file name, if null a temporary 3gp file name is provided
+ * @return String
+ */
+ private String createAudioFilePath(String fileName) {
+ File dir = Environment.getExternalStorageState().equals(Environment.MEDIA_MOUNTED)
+ ? context.getExternalFilesDir(null)
+ : context.getCacheDir();
+
+ fileName = fileName.isEmpty()
Review comment:
hi! I try it, and get null pointer exception here
`isEmpty` work only with strings, not null
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-media] erisu edited a comment on pull request #317: fix(android): get external files directory for Android 10+
Posted by GitBox <gi...@apache.org>.
erisu edited a comment on pull request #317:
URL: https://github.com/apache/cordova-plugin-media/pull/317#issuecomment-948574889
Last call for reviews. This PR will be merged in tomorrow (JST).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-media] erisu commented on a change in pull request #317: fix(android): get external files directory for Android 10+
Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #317:
URL: https://github.com/apache/cordova-plugin-media/pull/317#discussion_r728872196
##########
File path: src/android/AudioPlayer.java
##########
@@ -101,19 +103,29 @@ Licensed to the Apache Software Foundation (ASF) under one
*/
public AudioPlayer(AudioHandler handler, String id, String file) {
this.handler = handler;
+ context = handler.getApplicationContext();
this.id = id;
this.audioFile = file;
this.tempFiles = new LinkedList<String>();
+
}
- private String generateTempFile() {
- String tempFileName = null;
- if (Environment.getExternalStorageState().equals(Environment.MEDIA_MOUNTED)) {
- tempFileName = Environment.getExternalStorageDirectory().getAbsolutePath() + "/tmprecording-" + System.currentTimeMillis() + ".3gp";
- } else {
- tempFileName = "/data/data/" + handler.cordova.getActivity().getPackageName() + "/cache/tmprecording-" + System.currentTimeMillis() + ".3gp";
- }
- return tempFileName;
+ /**
+ * Creates an audio file path from the provided fileName or creates a new temporary file path.
+ *
+ * @param fileName the audio file name, if null a temporary 3gp file name is provided
+ * @return String
+ */
+ private String createAudioFilePath(String fileName) {
+ File dir = Environment.getExternalStorageState().equals(Environment.MEDIA_MOUNTED)
+ ? context.getExternalFilesDir(null)
+ : context.getCacheDir();
+
+ fileName = fileName.isEmpty()
Review comment:
@retoko I pushed an update, try again please.
https://github.com/apache/cordova-plugin-media/pull/317/commits/644c4616c26c67e97b0a89c83e46aa5584ab44ca
The conditional statement will now check for `null` values first before `isEmpty`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org