fixed getExternalFilesDir null issue #557
Conversation
|
+1 |
|
Can you please merge this? |
| if(externalFilesDir != null) { | ||
| res.put("SDCardApplicationDir", externalFilesDir.getParentFile().getAbsolutePath()); | ||
| } else { | ||
| res.put("SDCardApplicationDir", Environment.getExternalStorageDirectory().getAbsolutePath()); |
There was a problem hiding this comment.
getExternalStorageDirectory() is the root directory of sd card and files stored there will not be deleted when application is uninstalled. Not sure if this is what we want to do, put some application files in a root of sdcard.
The other thing is that getSystemfolders() is used by getConstants() which are pre-defined keys exposed to JavaScript, and I'm sure they are not updated during runtime, only taken once on module load (they're constants right :) ). So external storage state can change and getExternalStorageDirectory() or getExternalFilesDir() should be invoked when needed, something like:
(not tested)
RNFetchBlobFS.java
static public void getSDCardDir(Callback callback) {
if (Environment.getExternalStorageState().equals(Environment.MEDIA_MOUNTED)) {
callback.invoke(Environment.getExternalStorageDirectory().getAbsolutePath());
} else {
callback.invoke(null, "Media storage not mounted");
}
}
static public void getSDCardApplicationDir(ReactApplicationContext ctx, Callback callback) {
if (Environment.getExternalStorageState().equals(Environment.MEDIA_MOUNTED)) {
try {
callback.invoke(ctx.getExternalFilesDir(null).getParentFile().getAbsolutePath());
} catch (Exception e) {
callback.invoke(null, e.getLocalizedMessage());
}
} else {
callback.invoke(null, "Media storage not mounted");
}
}
RNFetchBlob.java
@ReactMethod
public void getSDCardDir(Callback callback) {
RNFetchBlobFS.getSDCardDir(callback);
}
@ReactMethod
public void getSDCardApplicationDir(Callback callback) {
RNFetchBlobFS.getSDCardApplicationDir(this.getReactApplicationContext(), callback);
}
What do you think @phodal ?
There was a problem hiding this comment.
Your change is make sense.
Actually, I don't have to many experience on Android development. This PR just want to mention people that here was a problem. And, I search google for this PR, it fixed my issue.
There was a problem hiding this comment.
I agree, it will fix the NullPointerException. Do you plan to update the PR then or want me to test my stuff on Android and make new PR?
There was a problem hiding this comment.
And one concern is that getSDCardDir() and getSDCardApplicationDir() methods doesn't fit to RNFetchBlob class, but the maintainer must put two cents to it.
There was a problem hiding this comment.
@sivakumar-cf In my option , i think @chrusart can create another PR, and I will closed this one.
|
@phodal @cbjs @sivakumar-cf , please check my comment, we also need just |
this issue has happened in a cloud tests' machines, here was log in phodal/play-app#1
when I search getExternalFilesDir null in Google, it seems it's common problem. I don't very sure the path is correctly ? but just want to confirm it.
It seems in a error branch..