Skip to content
This repository has been archived by the owner on Mar 16, 2019. It is now read-only.

fixed getExternalFilesDir null issue #557

Closed
wants to merge 5 commits into from
Closed

fixed getExternalFilesDir null issue #557

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9ab4ebb
bump to 0.10.8
bcpclone Aug 3, 2017
1336555
Update PULL_REQUEST_TEMPLATE
wkh237 Aug 3, 2017
2aea0b5
Fix path argument in iOS excludeFromBackupKey (#473)
grylance Aug 9, 2017
821eeb0
Fix README (#501)
moschan Aug 24, 2017
fda87ba
fixed getExternalFilesDir null issue
phodal Oct 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/PULL_REQUEST_TEMPLATE
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Thank you for making a pull request ! Just a gentle reminder :)

1. If the PR is offering a feature please make the request to our "Feature Branch" 0.11.0
2. Bug fix request to "Bug Fix Branch" 0.10.8
2. Bug fix request to "Bug Fix Branch" 0.10.9
3. Correct README.md can directly to master
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ RNFetchBlob
console.log('The file saved to ', res.path())
// Beware that when using a file path as Image source on Android,
// you must prepend "file://"" before the file path
imageView = <Image source={{ uri : Platform.OS === 'android' ? 'file://' + res.path() : '' + res.path() }}/>
imageView = <Image source={{ uri : Platform.OS === 'android' ? 'file://' + res.path() : '' + res.path() }}/>
})
```

Expand Down
7 changes: 6 additions & 1 deletion android/src/main/java/com/RNFetchBlob/RNFetchBlobFS.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,12 @@ static public Map<String, Object> getSystemfolders(ReactApplicationContext ctx)
state = Environment.getExternalStorageState();
if (state.equals(Environment.MEDIA_MOUNTED)) {
res.put("SDCardDir", Environment.getExternalStorageDirectory().getAbsolutePath());
res.put("SDCardApplicationDir", ctx.getExternalFilesDir(null).getParentFile().getAbsolutePath());
File externalFilesDir = ctx.getExternalFilesDir(null);
if(externalFilesDir != null) {
res.put("SDCardApplicationDir", externalFilesDir.getParentFile().getAbsolutePath());
} else {
res.put("SDCardApplicationDir", Environment.getExternalStorageDirectory().getAbsolutePath());
Copy link

Choose a reason for hiding this comment

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

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 ?

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link

Choose a reason for hiding this comment

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

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?

Copy link

@chrusart chrusart Dec 5, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

And one concern is that getSDCardDir() and getSDCardApplicationDir() methods doesn't fit to RNFetchBlob class, but the maintainer must put two cents to it.

Copy link

Choose a reason for hiding this comment

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

I do agree with @chrusart change. @phodal Please make the suggested change.

Copy link
Author

Choose a reason for hiding this comment

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

@sivakumar-cf In my option , i think @chrusart can create another PR, and I will closed this one.

Copy link

Choose a reason for hiding this comment

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

I will make a PR, wip :)

Copy link

Choose a reason for hiding this comment

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

PR #619 created.

}
}
res.put("MainBundleDir", ctx.getApplicationInfo().dataDir);
return res;
Expand Down
2 changes: 1 addition & 1 deletion ios.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ function openDocument(path:string, scheme:string) {
* @param {string} url URL of the resource, only file URL is supported
* @return {Promise}
*/
function excludeFromBackupKey(url:string) {
function excludeFromBackupKey(path:string) {
return RNFetchBlob.excludeFromBackupKey('file://' + path);
}

Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "react-native-fetch-blob",
"version": "0.10.7",
"version": "0.10.8",
"description": "A module provides upload, download, and files access API. Supports file stream read/write for process large files.",
"main": "index.js",
"scripts": {
Expand Down