joltup / rn-fetch-blob

A project committed to making file access and data transfer easier, efficient for React Native developers.
MIT License
2.81k stars 770 forks source link

Added support for Android API 29 and API 30 #646

Closed chaits98 closed 2 years ago

chaits98 commented 3 years ago
edeckers commented 3 years ago

Thank you! This PR is really useful as Google Play requires app updates to target at least targetSdk 29 from November 2nd (https://support.google.com/googleplay/android-developer/answer/113469#targetsdk). New apps are required to do so since August 3rd. I applied this PR to my project and it works just fine for me on a Pixel 3 device running Android 10; looking forward to a merge to master!

mrarronz commented 3 years ago

I have encountered a crash issue after merging this PR to my project. The crash title is

java.lang.IllegalStateException: cannot make a new request because the previous response is still open: please call response.close()

This crash occurs when I download multiple items one by one. Hi @chaits98, have you ever seen this issue? Could you please provide some help with this? Thanks

chaits98 commented 3 years ago

@mrarronz I will fix this issue over the weekend and update the pr for the same. Can you please share a working example where you encountered this issue.

mrarronz commented 3 years ago

@chaits98 Thank you very much! Sorry that I can't provide a working example due to security, my project downloads 3 items at once, I have a download page shows multiple download items, when user click "download all", all the items are added to a download queue, 3 items are allowed to download at the same time.

After merging this PR, when I click "download all", the app crashes. And it works again after relaunch the app and continue downloading. I guess this crash is related to the okhttp library interceptor crash.

Thank you again for your help!

chaits98 commented 3 years ago

@mrarronz I will work on this then. As far as network interceptors are concerned, I faced issues in downloading and uploading files when a network interceptor was attached. [Chucker: https://github.com/ChuckerTeam/chucker].

Please disable the network inspector and try uploading or downloading files.

This happens because network inspector forces the application to read from stream twice, first for network inspector and second time to write the file to storage, and the stream is closed after the first time it is read.

Check function "pipeStreamToSink" in "RNFetchBlobBody.java" file. [stream.close() is called] private void pipeStreamToSink(InputStream stream, BufferedSink sink) throws IOException { byte[] chunk = new byte[10240]; long totalWritten = 0; int read; while((read = stream.read(chunk, 0, 10240)) > 0) { sink.write(chunk, 0, read); totalWritten += read; emitUploadProgress(totalWritten); } stream.close(); }

Also, please check your implementation of network inspector or provide a code snippet so that I can help you better

mrarronz commented 3 years ago

@chaits98 My project is totally a react native project, I have no implementation of network inspector for Android. Looking forward to your fixing on this, much appreciate your help!

cristianoccazinsp commented 3 years ago

Title says SDK 29, but changes are for SDK 30. Typo? As far as I know, SDK 29 works fine right now. Either way, looking forward this change since we may see SDK 30 as a requirement soon.

chaits98 commented 3 years ago

@cristianoccazinsp The changes are for Android SDK 29 only Environment.getExternalStorageDirectory() was deprecated with SDK 29. Please go through the following link for the same. https://developer.android.com/reference/android/os/Environment#getExternalStorageDirectory()

cristianoccazinsp commented 3 years ago

Got it, but in the files you've changed, you have bumped the SDK to 30. RN still uses SDK 29, any reason to upgrade to 30?

chaits98 commented 3 years ago

Oh, my bad. I did that for my use case, you can change it back to 29 but the same changes work for SDK 30 as well.

cristianoccazinsp commented 3 years ago

Sadly, this repo may not be maintained anymore (https://github.com/joltup/rn-fetch-blob/issues/666), so until a new maintainer appears we may not see PRs merged anytime soon.

What doesn't work exactly with SDK 29? I have been using SDK 29 and this library for a few months already without issues, but I wonder what isn't working and we haven't noticed or are not using.

chaits98 commented 3 years ago

Environment.getExternalStorageDirectory() was deprecated and it cause problems for me when I tried to download and upload files using this library. This broke my code and in order to fix that I decided to update the code regarding the same.

I'm sure that people will pitch in to support this library, let's see how this pans out over time.

mrarronz commented 3 years ago

@chaits98 on Android 9 and lower versions Environment.getExternalStorageDirectory() works, think about that a user downloads some contents on the app, after we updating the app the download folder changes to Environment.getExternalFilesDir(string), then the user may not see the downloaded contents, so how to deal with that?

chaits98 commented 3 years ago

@mrarronz I was unable to reproduce your previous issue.

For your query regarding seeing downloaded content: If you want to save this to the shared Downloads directory, I suggest you go through this article: https://medium.com/@alessandromautone/public-file-storage-on-android-10-622c49a874a9 So basically you will have to copy the downloaded file to the shared Downloads folder.

If you want to open the downloaded file from within your application, you can add the following flag to your intent intent.setFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION);

RonRadtke commented 3 years ago

Hey, Thanks for your work here. I forked the repo (https://github.com/RonRadtke/react-native-blob-util and integrated the PR. Hopefully it will help some ppl

riorafe commented 3 years ago

This pull request works perfectly on android API 29 and API 30. Previously, I cannot download any single file using android download manager.

Hope this PR will be merged to master soon

cristianoccazinsp commented 3 years ago

Just migrate to https://github.com/RonRadtke/react-native-blob-util since this fork is pretty much dead.

keremoge commented 2 years ago

Thanks for the PR, It works like a charm, that PR should be merged 👍

RonRadtke commented 2 years ago

@keremoge This repo is unmaintained. Please migrate to https://github.com/RonRadtke/react-native-blob-util. There all these changes and some more are implemented.

noway commented 2 years ago

After merging this PR, when I click "download all", the app crashes. And it works again after relaunch the app and continue downloading. I guess this crash is related to the okhttp library interceptor crash.

@mrarronz Have you managed to fix interceptor crash? I experience it too.

mrarronz commented 2 years ago

@noway See the screenshot below, this is how I fix the crash.

intercepter crash
noway commented 2 years ago

Thanks

chaits98 commented 2 years ago

Hello anyone and everyone who is using this fork.

I haven't had the time to update everyone here that this is compatible with Android API 30 (Android 11).

@mrarronz @noway I will add the code for your fix as well so that you or anyone else using this fork does not face the crash.

noway commented 2 years ago

awesome thanks @chaits98 I'm using Ron Radtke's fork atm and this is his fix. https://github.com/RonRadtke/react-native-blob-util/commit/6806822d024c4684c14d11b371e8ae5e1f6f0f3b

I'm not 100% confident it fixes the issue, but i think it happens less?

chaits98 commented 2 years ago

Sorry for the delay everyone. I have updated my fork with fix mentioned by @noway

Fix https://github.com/RonRadtke/react-native-blob-util/commit/6806822d024c4684c14d11b371e8ae5e1f6f0f3b

Also my fork works well with Android API 30 (Android 11). I will take some time out and check my fork's usage with API 31 and check if any changes are required according to Android 12's documentation.