My proposed 0.10.9 changes (#489)

* fs.js: forgot one more error refactoring * Fix path argument in iOS excludeFromBackupKey (#473) * Fix link to fs.readStream() and to fs.writeStream() and insert link to new function fs.hash() * Fix the documentation part of wkh237#467 "Example code for writeStream ignores that stream.write() returns a promise?" * More fixes for issue wkh237#460 "Error normalization" IMPORTANT: I wrote the iOS code BLIND (not even syntax highlighting) - this needs to be tested. - Two or three methods that used callbacks to return results were changed to RN promises - All methods using promises now use a Unix like code string for the first parameter, e.g. "ENOENT" for "File does not exist" (http://www.alorelang.org/doc/errno.html). The React Native bridge code itself uses this schema: it inserts "EUNSPECIFIED" when the "code" it gets back from Android/iOS code is undefined (null, nil). The RN bridge assigns the code (or "EUNSPECIFIED") to the "code" property of the error object it returns to Javascript, following the node.js example (the "code" property is not part of "standard" Javascript Error objects) - Important errors like "No such file" are reported (instead of a general error), using the code property. - I added a few extra error checks that the IDE suggested, mostly for Android (for which I have an IDE), if it seemed important I tried to do the same for teh iOS equivalent function - I followed IDE suggestions on some of the Java code, like making fields private - RNFetchBlobFS.java removeSession(): Added reporting of all failures to delete - IS THIS DESIRABLE (or do we not care)? - readStream: The same schema is used for the emitted events when they are error events - iOS: added an import for the crypto-digest headers - they are needed for the hash() function submitted in an earlier commit - Fixed a link in the README.md - unfortunately the anchor-links change whenever even one character of the linked headline in the Wiki page changes * Fix one issue raised in wkh237#477 by using code from https://stackoverflow.com/a/40874952/544779 * fix some access rights, remove unused items * update gradle version setting in build.gradle * Revert gradle settings to previous values :-( * add a missing closing ")" * Removed the part of an obsolete callback function parameter that I had left in when I converted mkdir to promises (low-level code) * let mkdir resolve with "undefined" instead of "null" (my mistake) * mkdir: normalize iOS and Android error if something already exists (file OR folder); return "true" (boolean) on success (failure is rejected promise) - it is not possibel to return "undefined" from a React Native promise from Java * fix a long/int issue * my mistake - according to https://facebook.github.io/react-native/docs/native-modules-android.html#argument-types "long" is not possible as argument type of an exported RN native module function * Adde "utf8" as default encoding for fs.readFile - fixes #450 and #484 * follow my IDEA IDE's recommendations - SparseArray instead of HashMap, and make some fields private * polyfill/File.js: add a parameter===undefined? check (this happened silently in the test suite) * make var static again * Normalized errors for fs.ls() * forgot one parameter * more parameter checks * forgot to resolve the promise * Forgot ; * add more error parameter checks * change readStream()/writeStream() default encoding to utf8 to match the tests in react-native-fetch-blob-dev * default encoding is set in fs.js (now), no need to do it twice * ReadStream error events: Set a default error code "EUNSPECIFIED" if no code is returned (should not happen, actually) * writeFile() Android and iOS: improve errors; ReadStream: Add "ENOENT" (no such file) error event to Android version and add the thus far missing "code" parameter to iOS version * oops - one "}" too many - removed * add EISDIR error to readFile()s error vocabulary (iOS and Android) * "or directory" is misplaced in a "no such file" error message for readFile() * Android: two reject() calls did not have a code, iOS: slice() did not have code EISDIR, and "could not resolve URI" now is EINVAL everywhere * writeStream: return ENOENT, EISDIR and EUNSPECIFIED according to the normalized schema (#460); Open a new question about behavior on ENOENT (#491) * "+ +" was one plus sign too many * this if has a whole block (that ois why I prefer a style where {} are mandatory even for single-statement blocks) * I renamed this variable * 1) #491 "writeStream() does not create file if it doesn't exist?" 2) I had gone overboard with the "@[..]" in the ios code, making some error strings arrays 3) fix typos: rename all ENODIR => ENOTDIR * Java: getParentFolder() may return null - prevent a NullPointerException by adding one more check * Relating to #298 -- looping through an array is not supposed to be done with for...in * Fix IOS syntax errors in #489 * #489 Fix typo and missing return statement * fix error code
joltup · Aug 31, 2017 · 7fa5761 · 7fa5761
1 parent a78acc7
commit 7fa5761
Show file tree

Hide file tree

Showing 21 changed files with 867 additions and 596 deletions.
diff --git a/README.md b/README.md
@@ -590,10 +590,11 @@ File Access APIs
 - [dirs](https://github.com/wkh237/react-native-fetch-blob/wiki/File-System-Access-API#dirs)
 - [createFile](https://github.com/wkh237/react-native-fetch-blob/wiki/File-System-Access-API#createfilepath-data-encodingpromise)
 - [writeFile (0.6.0)](https://github.com/wkh237/react-native-fetch-blob/wiki/File-System-Access-API#writefilepathstring-contentstring--array-encodingstring-appendbooleanpromise)
-- [appendFile (0.6.0) ](https://github.com/wkh237/react-native-fetch-blob/wiki/File-System-Access-API#appendfilepathstring-contentstring--array-encodingstringpromise)
+- [appendFile (0.6.0) ](https://github.com/wkh237/react-native-fetch-blob/wiki/File-System-Access-API#appendfilepathstring-contentstring--arraynumber-encodingstring-promisenumber)
 - [readFile (0.6.0)](https://github.com/wkh237/react-native-fetch-blob/wiki/File-System-Access-API#readfilepath-encodingpromise)
-- [readStream](https://github.com/wkh237/react-native-fetch-blob/wiki/File-System-Access-API#readstreampath-encoding-buffersizepromise)
-- [writeStream](https://github.com/wkh237/react-native-fetch-blob/wiki/File-System-Access-API#writestreampathstring-encodingstring-appendbooleanpromise)
+- [readStream](https://github.com/wkh237/react-native-fetch-blob/wiki/File-System-Access-API#readstreampath-encoding-buffersize-interval-promisernfbreadstream)
+- [hash (0.10.9)](https://github.com/wkh237/react-native-fetch-blob/wiki/File-System-Access-API#hashpath-algorithm-promise)
+- [writeStream](https://github.com/wkh237/react-native-fetch-blob/wiki/File-System-Access-API#writestreampathstring-encodingstringpromise)
 - [hash](https://github.com/wkh237/react-native-fetch-blob/wiki/File-System-Access-API#hashpath-algorithmpromise)
 - [unlink](https://github.com/wkh237/react-native-fetch-blob/wiki/File-System-Access-API#unlinkpathstringpromise)
 - [mkdir](https://github.com/wkh237/react-native-fetch-blob/wiki/File-System-Access-API#mkdirpathstringpromise)
@@ -644,6 +645,45 @@ RNFetchBlob.fs.readStream(
 
 When using `writeStream`, the stream object becomes writable, and you can then perform operations like `write` and `close`.
 
+Since version 0.10.9 `write()` resolves with the `RNFetchBlob` instance so you can promise-chain write calls:
+
+```js
+RNFetchBlob.fs.writeStream(
+    PATH_TO_FILE,
+    // encoding, should be one of `base64`, `utf8`, `ascii`
+    'utf8',
+    // should data append to existing content ?
+    true
+)
+.then(ofstream => ofstream.write('foo'))
+.then(ofstream => ofstream.write('bar'))
+.then(ofstream => ofstream.write('foobar'))
+.then(ofstream => ofstream.close())
+.catch(console.error)
+```
+
+or 
+
+```js
+RNFetchBlob.fs.writeStream(
+    PATH_TO_FILE,
+    // encoding, should be one of `base64`, `utf8`, `ascii`
+    'utf8',
+    // should data append to existing content ?
+    true
+)
+.then(stream => Promise.all([
+    stream.write('foo'),
+    stream.write('bar'),
+    stream.write('foobar')
+]))
+// Use array destructuring to get the stream object from the first item of the array we get from Promise.all()
+.then(([stream]) => stream.close())
+.catch(console.error)
+```
+
+You should **NOT** do something like this:
+
 ```js
 RNFetchBlob.fs.writeStream(
     PATH_TO_FILE,
@@ -652,13 +692,18 @@ RNFetchBlob.fs.writeStream(
     // should data append to existing content ?
     true)
 .then((ofstream) => {
+    // BAD IDEA - Don't do this, those writes are unchecked:
     ofstream.write('foo')
     ofstream.write('bar')
     ofstream.close()
 })
-
+.catch(console.error)  // Cannot catch any write() errors!
 ```
 
+The problem with the above code is that the promises from the `ofstream.write()` calls are detached and "Lost".
+That means the entire promise chain A) resolves without waiting for the writes to finish and B) any errors caused by them are lost.
+That code may _seem_ to work if there are no errors, but those writes are of the type "fire and forget": You start them and then turn away and never know if they really succeeded.
+
 ### Cache File Management
 
 When using `fileCache` or `path` options along with `fetch` API, response data will automatically store into the file system. The files will **NOT** removed unless you `unlink` it. There're several ways to remove the files

diff --git a/android/build.gradle b/android/build.gradle
@@ -34,5 +34,6 @@ android {
 
 dependencies {
     compile 'com.facebook.react:react-native:+'
+    //compile 'com.squareup.okhttp3:okhttp:+'
     //{RNFetchBlob_PRE_0.28_DEPDENDENCY}
 }
diff --git a/android/gradle/wrapper/gradle-wrapper.properties b/android/gradle/wrapper/gradle-wrapper.properties
@@ -1,4 +1,4 @@
-#Wed May 18 12:33:41 CST 2016
+#Sat Aug 12 07:48:35 CEST 2017
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
 zipStoreBase=GRADLE_USER_HOME

diff --git a/android/src/main/java/com/RNFetchBlob/RNFetchBlob.java b/android/src/main/java/com/RNFetchBlob/RNFetchBlob.java
@@ -4,6 +4,7 @@
 import android.app.DownloadManager;
 import android.content.Intent;
 import android.net.Uri;
+import android.util.SparseArray;
 
 import com.facebook.react.bridge.ActivityEventListener;
 import com.facebook.react.bridge.Callback;
@@ -34,26 +35,23 @@
 
 public class RNFetchBlob extends ReactContextBaseJavaModule {
 
-    // Cookies
-    private final ForwardingCookieHandler mCookieHandler;
-    private final CookieJarContainer mCookieJarContainer;
     private final OkHttpClient mClient;
 
     static ReactApplicationContext RCTContext;
-    static LinkedBlockingQueue<Runnable> taskQueue = new LinkedBlockingQueue<>();
-    static ThreadPoolExecutor threadPool = new ThreadPoolExecutor(5, 10, 5000, TimeUnit.MILLISECONDS, taskQueue);
+    private static LinkedBlockingQueue<Runnable> taskQueue = new LinkedBlockingQueue<>();
+    private static ThreadPoolExecutor threadPool = new ThreadPoolExecutor(5, 10, 5000, TimeUnit.MILLISECONDS, taskQueue);
     static LinkedBlockingQueue<Runnable> fsTaskQueue = new LinkedBlockingQueue<>();
-    static ThreadPoolExecutor fsThreadPool = new ThreadPoolExecutor(2, 10, 5000, TimeUnit.MILLISECONDS, taskQueue);
-    static public boolean ActionViewVisible = false;
-    static HashMap<Integer, Promise> promiseTable = new HashMap<>();
+    private static ThreadPoolExecutor fsThreadPool = new ThreadPoolExecutor(2, 10, 5000, TimeUnit.MILLISECONDS, taskQueue);
+    private static boolean ActionViewVisible = false;
+    private static SparseArray<Promise> promiseTable = new SparseArray<>();
 
     public RNFetchBlob(ReactApplicationContext reactContext) {
 
         super(reactContext);
 
         mClient = OkHttpClientProvider.getOkHttpClient();
-        mCookieHandler = new ForwardingCookieHandler(reactContext);
-        mCookieJarContainer = (CookieJarContainer) mClient.cookieJar();
+        ForwardingCookieHandler mCookieHandler = new ForwardingCookieHandler(reactContext);
+        CookieJarContainer mCookieJarContainer = (CookieJarContainer) mClient.cookieJar();
         mCookieJarContainer.setCookieJar(new JavaNetCookieJar(mCookieHandler));
 
         RCTContext = reactContext;
@@ -85,11 +83,21 @@ public Map<String, Object> getConstants() {
     }
 
     @ReactMethod
-    public void createFile(final String path, final String content, final String encode, final Callback callback) {
+    public void createFile(final String path, final String content, final String encode, final Promise promise) {
         threadPool.execute(new Runnable() {
             @Override
             public void run() {
-                RNFetchBlobFS.createFile(path, content, encode, callback);
+                RNFetchBlobFS.createFile(path, content, encode, promise);
+            }
+        });
+    }
+
+    @ReactMethod
+    public void createFileASCII(final String path, final ReadableArray dataArray, final Promise promise) {
+        threadPool.execute(new Runnable() {
+            @Override
+            public void run() {
+                RNFetchBlobFS.createFileASCII(path, dataArray, promise);
             }
         });
 
@@ -124,21 +132,10 @@ public void onHostDestroy() {
             };
             RCTContext.addLifecycleEventListener(listener);
         } catch(Exception ex) {
-            promise.reject(ex.getLocalizedMessage());
+            promise.reject("EUNSPECIFIED", ex.getLocalizedMessage());
         }
     }
 
-    @ReactMethod
-    public void createFileASCII(final String path, final ReadableArray dataArray, final Callback callback) {
-        threadPool.execute(new Runnable() {
-            @Override
-            public void run() {
-                RNFetchBlobFS.createFileASCII(path, dataArray, callback);
-            }
-        });
-
-    }
-
     @ReactMethod
     public void writeArrayChunk(final String streamId, final ReadableArray dataArray, final Callback callback) {
         RNFetchBlobFS.writeArrayChunk(streamId, dataArray, callback);
@@ -150,8 +147,8 @@ public void unlink(String path, Callback callback) {
     }
 
     @ReactMethod
-    public void mkdir(String path, Callback callback) {
-        RNFetchBlobFS.mkdir(path, callback);
+    public void mkdir(String path, Promise promise) {
+        RNFetchBlobFS.mkdir(path, promise);
     }
 
     @ReactMethod
@@ -176,8 +173,8 @@ public void mv(String path, String dest, Callback callback) {
     }
 
     @ReactMethod
-    public void ls(String path, Callback callback) {
-        RNFetchBlobFS.ls(path, callback);
+    public void ls(String path, Promise promise) {
+        RNFetchBlobFS.ls(path, promise);
     }
 
     @ReactMethod
@@ -355,10 +352,10 @@ public void getContentIntent(String mime, Promise promise) {
 
     @ReactMethod
     public void addCompleteDownload (ReadableMap config, Promise promise) {
-        DownloadManager dm = (DownloadManager) RNFetchBlob.RCTContext.getSystemService(RNFetchBlob.RCTContext.DOWNLOAD_SERVICE);
+        DownloadManager dm = (DownloadManager) RCTContext.getSystemService(RCTContext.DOWNLOAD_SERVICE);
         String path = RNFetchBlobFS.normalizePath(config.getString("path"));
         if(path == null) {
-            promise.reject("RNFetchblob.addCompleteDownload can not resolve URI:" + config.getString("path"), "RNFetchblob.addCompleteDownload can not resolve URI:" + path);
+            promise.reject("EINVAL", "RNFetchblob.addCompleteDownload can not resolve URI:" + config.getString("path"));
             return;
         }
         try {
@@ -375,7 +372,7 @@ public void addCompleteDownload (ReadableMap config, Promise promise) {
             promise.resolve(null);
         }
         catch(Exception ex) {
-            promise.reject("RNFetchblob.addCompleteDownload failed", ex.getStackTrace().toString());
+            promise.reject("EUNSPECIFIED", ex.getLocalizedMessage());
         }
 
     }

diff --git a/android/src/main/java/com/RNFetchBlob/RNFetchBlobBody.java b/android/src/main/java/com/RNFetchBlob/RNFetchBlobBody.java
@@ -1,5 +1,6 @@
 package com.RNFetchBlob;
 
+import android.support.annotation.NonNull;
 import android.util.Base64;
 
 import com.facebook.react.bridge.Arguments;
@@ -21,21 +22,20 @@
 import okhttp3.RequestBody;
 import okio.BufferedSink;
 
-public class RNFetchBlobBody extends RequestBody{
+class RNFetchBlobBody extends RequestBody{
 
-    InputStream requestStream;
-    long contentLength = 0;
-    ReadableArray form;
-    String mTaskId;
-    String rawBody;
-    RNFetchBlobReq.RequestType requestType;
-    MediaType mime;
-    File bodyCache;
+    private InputStream requestStream;
+    private long contentLength = 0;
+    private ReadableArray form;
+    private String mTaskId;
+    private String rawBody;
+    private RNFetchBlobReq.RequestType requestType;
+    private MediaType mime;
+    private File bodyCache;
     int reported = 0;
-    Boolean chunkedEncoding = false;
+    private Boolean chunkedEncoding = false;
 
-
-    public RNFetchBlobBody(String taskId) {
+    RNFetchBlobBody(String taskId) {
         this.mTaskId = taskId;
     }
 
@@ -49,7 +49,7 @@ RNFetchBlobBody setMIME(MediaType mime) {
         return this;
     }
 
-    RNFetchBlobBody setRequestType( RNFetchBlobReq.RequestType type) {
+    RNFetchBlobBody setRequestType(RNFetchBlobReq.RequestType type) {
         this.requestType = type;
         return this;
     }
@@ -114,7 +114,7 @@ public MediaType contentType() {
     }
 
     @Override
-    public void writeTo(BufferedSink sink) {
+    public void writeTo(@NonNull BufferedSink sink) {
         try {
             pipeStreamToSink(requestStream, sink);
         } catch(Exception ex) {
@@ -186,8 +186,7 @@ private File createMultipartBodyCache() throws IOException {
         ArrayList<FormField> fields = countFormDataLength();
         ReactApplicationContext ctx = RNFetchBlob.RCTContext;
 
-        for(int i = 0;i < fields.size(); i++) {
-            FormField field = fields.get(i);
+        for(FormField field : fields) {
             String data = field.data;
             String name = field.name;
             // skip invalid fields
@@ -258,17 +257,14 @@ private File createMultipartBodyCache() throws IOException {
      * @param sink      The request body buffer sink
      * @throws IOException
      */
-    private void pipeStreamToSink(InputStream stream, BufferedSink sink) throws Exception {
-
-        byte [] chunk = new byte[10240];
+    private void pipeStreamToSink(InputStream stream, BufferedSink sink) throws IOException {
+        byte[] chunk = new byte[10240];
         int totalWritten = 0;
         int read;
         while((read = stream.read(chunk, 0, 10240)) > 0) {
-            if(read > 0) {
-                sink.write(chunk, 0, read);
-                totalWritten += read;
-                emitUploadProgress(totalWritten);
-            }
+            sink.write(chunk, 0, read);
+            totalWritten += read;
+            emitUploadProgress(totalWritten);
         }
         stream.close();
     }
@@ -291,7 +287,7 @@ private void pipeStreamToFileStream(InputStream is, FileOutputStream os) throws
 
     /**
      * Compute approximate content length for form data
-     * @return
+     * @return ArrayList<FormField>
      */
     private ArrayList<FormField> countFormDataLength() {
         long total = 0;
@@ -300,11 +296,11 @@ private ArrayList<FormField> countFormDataLength() {
         for(int i = 0;i < form.size(); i++) {
             FormField field = new FormField(form.getMap(i));
             list.add(field);
-            String data = field.data;
-            if(data == null) {
+            if(field.data == null) {
                 RNFetchBlobUtils.emitWarningEvent("RNFetchBlob multipart request builder has found a field without `data` property, the field `"+ field.name +"` will be removed implicitly.");
             }
             else if (field.filename != null) {
+                String data = field.data;
                 // upload from storage
                 if (data.startsWith(RNFetchBlobConst.FILE_PREFIX)) {
                     String orgPath = data.substring(RNFetchBlobConst.FILE_PREFIX.length());
@@ -333,7 +329,7 @@ else if (field.filename != null) {
             }
             // data field
             else {
-                total += field.data != null ? field.data.getBytes().length : 0;
+                total += field.data.getBytes().length;
             }
         }
         contentLength = total;
@@ -346,11 +342,11 @@ else if (field.filename != null) {
      */
     private class FormField {
         public String name;
-        public String filename;
-        public String mime;
+        String filename;
+        String mime;
         public String data;
 
-        public FormField(ReadableMap rawData) {
+        FormField(ReadableMap rawData) {
             if(rawData.hasKey("name"))
                 name = rawData.getString("name");
             if(rawData.hasKey("filename"))
@@ -368,7 +364,7 @@ public FormField(ReadableMap rawData) {
 
     /**
      * Emit progress event
-     * @param written
+     * @param written  Integer
      */
     private void emitUploadProgress(int written) {
         RNFetchBlobProgressConfig config = RNFetchBlobReq.getReportUploadProgress(mTaskId);

diff --git a/android/src/main/java/com/RNFetchBlob/RNFetchBlobConfig.java b/android/src/main/java/com/RNFetchBlob/RNFetchBlobConfig.java
@@ -3,10 +3,7 @@
 import com.facebook.react.bridge.ReadableArray;
 import com.facebook.react.bridge.ReadableMap;
 
-import java.util.HashMap;
-
-
-public class RNFetchBlobConfig {
+class RNFetchBlobConfig {
 
     public Boolean fileCache;
     public String path;