Use proper promise idiom in Provider

jabbany · Oct 26, 2016 · 7eb76b2 · 7eb76b2
1 parent 6874e37
commit 7eb76b2
Show file tree

Hide file tree

Showing 9 changed files with 172 additions and 13 deletions.
diff --git a/Gruntfile.coffee b/Gruntfile.coffee
@@ -22,7 +22,7 @@ module.exports = (grunt) ->
 
   SRC_MODULES = 
     'filter': ['src/filter/SimpleFilter.js']
-    'provider': ['src/CommentProvider.js']
+    'provider': ['src/CommentProvider.js', 'src/Promises.js']
     'format-bilibili': ['src/parsers/BilibiliFormat.js']
     'format-acfun': ['src/parsers/AcfunFormat.js']
     'format-common': ['src/parsers/CommonDanmakuFormat.js']

diff --git a/dist/CommentCoreLibrary.js b/dist/CommentCoreLibrary.js
@@ -3,7 +3,7 @@
  * @license MIT
  * @author Jim Chen
  */
-var BinArray = (function( ) {
+var BinArray = (function () {
 
     var BinArray = {};
 
@@ -1266,7 +1266,7 @@ var CommentProvider = (function () {
         var promises = [];
         // TODO: This race logic needs to be rethought to provide redundancy
         for (var type in this._staticSources) {
-            promises.push(Promise.race(this._staticSources[type])
+            promises.push(Promises.any(this._staticSources[type])
                 .then(function (data) {
                     return this.applyParsersList(data, type);
                 }.bind(this)));
@@ -1275,7 +1275,7 @@ var CommentProvider = (function () {
             // No static loaders
             return Promise.resolve([]);
         }
-        return Promise.race(promises).then(function (commentList) {
+        return Promises.any(promises).then(function (commentList) {
             for (var i = 0; i < this._targets.length; i++) {
                 this._targets[i].load(commentList);
             }
@@ -1345,6 +1345,61 @@ var CommentProvider = (function () {
     return CommentProvider;
 })();
 
+/**
+ * Promises extra functionality
+ * @license MIT
+ * @author Jim Chen
+ */
+var Promises = (function( ) {
+
+    var Promises = {};
+
+    /**
+     * Resolves as soon as any promise resolves in the order of the input array
+     * 
+     * @param arr - array of promises
+     * @return promise that resolves if any one promise resolves and rejects
+     *         if otherwise
+     **/
+    Promises.any = function (promises) {
+        if (!Array.isArray(promises)) {
+            // Is raw object or promise, resolve it directly
+            return Promise.resolve(promises);
+        }
+        if (promises.length === 0) {
+            // No promises to resolve so we think it failed
+            return Promise.reject();
+        }
+        return new Promise(function (resolve, reject) {
+            var hasResolved = false;
+            var hasCompleted = 0;
+            var errors = [];
+            for (var i = 0; i < promises.length; i++) {
+                promises[i].then(function (value) {
+                    hasCompleted++;
+                    if (!hasResolved) {
+                        hasResolved = true;
+                        resolve(value);
+                    }
+                }).catch((function (i) {
+                    return function (e) {
+                        hasCompleted++;
+                        errors[i] = e;
+                        if (hasCompleted === promises.length) {
+                            // All promises have completed and we are in rejecting case
+                            if (!hasResolved) {
+                                reject(errors);
+                            }
+                        }
+                    }
+                })(i));
+            }
+        });
+    };
+
+    return Promises;
+})();
+
 /** 
  * Bilibili Format Parser
  * Takes in an XMLDoc/LooseXMLDoc and parses that into a Generic Comment List

diff --git a/dist/CommentCoreLibrary.min.js b/dist/CommentCoreLibrary.min.js
diff --git a/spec/CommentProvider_spec.coffee b/spec/CommentProvider_spec.coffee
@@ -183,7 +183,7 @@ describe 'CommentProvider', ->
       it 'fails if no sources are available', (done) ->
         provider.addStaticSource (Promise.reject 'Error'), CommentProvider.SOURCE_TEXT
         provider.load().catch (e) ->
-          expect(e).toBe 'Error'
+          expect(e).toBeTruthy()
           done()
 
     describe '.start', ->

diff --git a/spec/Promises_spec.coffee b/spec/Promises_spec.coffee
@@ -0,0 +1,50 @@
+'use strict'
+describe 'Promises', ->
+  describe 'any', ->
+    it 'resolves single object', (done) ->
+      p = Promises.any 'Foo'
+      p.then (value) ->
+        expect(value).toBe 'Foo'
+        done()
+
+    it 'rejects single promise', (done) ->
+      p = Promises.any Promise.reject 'Bar'
+      p.catch (value) ->
+        expect(value).toBe 'Bar'
+        done()
+
+    it 'resolves empty', (done) ->
+      p = Promises.any []
+      p.catch (e) ->
+        expect(e).toBeFalsy()
+        done()
+
+    it 'falls back', (done) ->
+      p = Promises.any [
+        Promise.reject 'A'
+        Promise.reject 'B'
+        Promise.resolve 'C'
+      ]
+      p.then (value) ->
+        expect(value).toBe 'C'
+        done()
+
+    it 'resolves on first', (done) ->
+      p = Promises.any [
+        Promise.reject 'A'
+        Promise.resolve 'B'
+        Promise.resolve 'C'
+      ]
+      p.then (value) ->
+        expect(value).toBe 'B'
+        done()
+
+    it 'rejects if none resolve', (done) ->
+      p = Promises.any [
+        Promise.reject 'A'
+        Promise.reject 'B'
+        Promise.reject 'C'
+      ]
+      p.catch (errors) ->
+        expect(errors).toEqual ['A', 'B', 'C']
+        done()
diff --git a/spec/core/CommentSpaceAllocator_spec.coffee b/spec/core/CommentSpaceAllocator_spec.coffee
@@ -52,8 +52,8 @@ describe 'CommentSpaceAllocators', ->
         c1.cindex = -1
         expect(typeof anchorCSA.remove(c1)).toBe 'undefined'
 
-      it 'removal for cindex < 0', ->
-        c1.cindex = -1
+      it 'removal for cindex not found', ->
+        c1.cindex = 0
         expect(typeof anchorCSA.remove(c1)).toBe 'undefined'
       # TODO: We need more extensive test cases
 
@@ -75,7 +75,7 @@ describe 'CommentSpaceAllocators', ->
         s1.cindex = -1
         expect(typeof scrollCSA.remove(s1)).toBe 'undefined'
 
-      it 'removal for cindex < 0', ->
-        s1.cindex = -1
+      it 'removal for cindex not found', ->
+        s1.cindex = 0
         expect(typeof scrollCSA.remove(s1)).toBe 'undefined'
       # TODO: We need more extensive test cases
diff --git a/src/Array.js b/src/Array.js
@@ -3,7 +3,7 @@
  * @license MIT
  * @author Jim Chen
  */
-var BinArray = (function( ) {
+var BinArray = (function () {
 
     var BinArray = {};
 

diff --git a/src/CommentProvider.js b/src/CommentProvider.js
@@ -268,7 +268,7 @@ var CommentProvider = (function () {
         var promises = [];
         // TODO: This race logic needs to be rethought to provide redundancy
         for (var type in this._staticSources) {
-            promises.push(Promise.race(this._staticSources[type])
+            promises.push(Promises.any(this._staticSources[type])
                 .then(function (data) {
                     return this.applyParsersList(data, type);
                 }.bind(this)));
@@ -277,7 +277,7 @@ var CommentProvider = (function () {
             // No static loaders
             return Promise.resolve([]);
         }
-        return Promise.race(promises).then(function (commentList) {
+        return Promises.any(promises).then(function (commentList) {
             for (var i = 0; i < this._targets.length; i++) {
                 this._targets[i].load(commentList);
             }

diff --git a/src/Promises.js b/src/Promises.js
@@ -0,0 +1,54 @@
+/**
+ * Promises extra functionality
+ * @license MIT
+ * @author Jim Chen
+ */
+var Promises = (function( ) {
+
+    var Promises = {};
+
+    /**
+     * Resolves as soon as any promise resolves in the order of the input array
+     * 
+     * @param arr - array of promises
+     * @return promise that resolves if any one promise resolves and rejects
+     *         if otherwise
+     **/
+    Promises.any = function (promises) {
+        if (!Array.isArray(promises)) {
+            // Is raw object or promise, resolve it directly
+            return Promise.resolve(promises);
+        }
+        if (promises.length === 0) {
+            // No promises to resolve so we think it failed
+            return Promise.reject();
+        }
+        return new Promise(function (resolve, reject) {
+            var hasResolved = false;
+            var hasCompleted = 0;
+            var errors = [];
+            for (var i = 0; i < promises.length; i++) {
+                promises[i].then(function (value) {
+                    hasCompleted++;
+                    if (!hasResolved) {
+                        hasResolved = true;
+                        resolve(value);
+                    }
+                }).catch((function (i) {
+                    return function (e) {
+                        hasCompleted++;
+                        errors[i] = e;
+                        if (hasCompleted === promises.length) {
+                            // All promises have completed and we are in rejecting case
+                            if (!hasResolved) {
+                                reject(errors);
+                            }
+                        }
+                    }
+                })(i));
+            }
+        });
+    };
+
+    return Promises;
+})();