[GitHub] [cordova-plugin-file-transfer] breautek commented on a change in pull request #310: [Android] Enable using MediaStore API on Android Q

[GitBox <gi...@apache.org>]

breautek commented on a change in pull request #310:
URL: https://github.com/apache/cordova-plugin-file-transfer/pull/310#discussion_r705368872



##########
File path: src/android/FileTransfer.java
##########
@@ -792,7 +796,24 @@ public void run() {
                             // write bytes to file
                             byte[] buffer = new byte[MAX_BUFFER_SIZE];
                             int bytesRead = 0;
-                            outputStream = resourceApi.openOutputStream(targetUri);
+                            /* 
+                                This ensures that for Android Q and above that the new MediaStore API being used.
+
+                                Instead of using the provided target directory for saving from the web app, it will automatically
+                                save to the Downloads folder.
+                            */
+                            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {

Review comment:
       Thank you for your effort in preparing this PR.
   
   I just wanted to point out that this doesn't really honour the existing [download](https://github.com/apache/cordova-plugin-file-transfer#download) API, as it ignores the target download path and just throws the downloaded file in the Downloads container as also stated in your code comment. I think is a bad assumption, and I believe this is an unnecessary limitation. I'll explain further.
   
   We can still use file paths. The difference in API 29+/Scoped Storage is not all external file paths are writable (e.g. the external root directory is no longer a writable path).
   
   We don't need to use the `MediaStore` APIs directly, we can use MediaStore via using [Direct File Paths](https://developer.android.com/training/data-storage/shared/media#direct-file-paths). While MediaStore APIs is the recommended approach by Google, but it only really works if you can make these kind of assumptions. From a Cordova standpoint, I don't think we can make these kind of assumptions. I've done some testing with this already against the `cordova-plugin-file` and it appears to work.
   
   Interested to hear your thoughts on this.




-- 
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: commits-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org