From ad971a444cb66e4ee3a97a86f11a6c677ab67951 Mon Sep 17 00:00:00 2001
From: Jaco Greeff <jaco@ethcore.io>
Date: Fri, 9 Dec 2016 00:00:56 +0100
Subject: [PATCH 1/8] Add autoRemove functionality for api.subscribe

---
 js/src/api/subscriptions/manager.js | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/js/src/api/subscriptions/manager.js b/js/src/api/subscriptions/manager.js
index bc9632592..300ede5f8 100644
--- a/js/src/api/subscriptions/manager.js
+++ b/js/src/api/subscriptions/manager.js
@@ -59,7 +59,7 @@ export default class Manager {
     return subscription;
   }
 
-  subscribe (subscriptionName, callback) {
+  subscribe (subscriptionName, callback, autoRemove = false) {
     return new Promise((resolve, reject) => {
       const subscription = this._validateType(subscriptionName);
 
@@ -75,6 +75,7 @@ export default class Manager {
       this.subscriptions[subscriptionId] = {
         name: subscriptionName,
         id: subscriptionId,
+        autoRemove,
         callback
       };
 
@@ -104,13 +105,16 @@ export default class Manager {
     const { callback } = this.subscriptions[subscriptionId];
 
     try {
-      callback(error, data);
+      return callback(error, data);
     } catch (error) {
       console.error(`Unable to update callback for subscriptionId ${subscriptionId}`, error);
     }
+
+    return true;
   }
 
   _updateSubscriptions = (subscriptionName, error, data) => {
+    const cleanup = [];
     const subscriptions = this.subscriptions
       .filter(subscription => subscription.name === subscriptionName);
 
@@ -118,8 +122,16 @@ export default class Manager {
 
     subscriptions
       .forEach((subscription) => {
-        this._sendData(subscription.id, error, data);
+        const result = this._sendData(subscription.id, error, data);
+
+        if (subscription.autoRemove && !result) {
+          cleanup.push(subscription.id);
+        }
       });
+
+    cleanup.forEach((subscriptionId) => {
+      delete this.subscriptions[subscriptionId];
+    });
   }
 }
 

From 4ce3142c630c8ca236240aa7642536fb1d2aa777 Mon Sep 17 00:00:00 2001
From: Jaco Greeff <jaco@ethcore.io>
Date: Fri, 9 Dec 2016 00:15:27 +0100
Subject: [PATCH 2/8] Add autoRemove functionality to api.contract.subscribe

---
 js/src/api/contract/contract.js | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/js/src/api/contract/contract.js b/js/src/api/contract/contract.js
index ed922a02c..f34cd8575 100644
--- a/js/src/api/contract/contract.js
+++ b/js/src/api/contract/contract.js
@@ -240,8 +240,8 @@ export default class Contract {
   }
 
   _bindEvent = (event) => {
-    event.subscribe = (options = {}, callback) => {
-      return this._subscribe(event, options, callback);
+    event.subscribe = (options = {}, callback, autoRemove = false) => {
+      return this._subscribe(event, options, callback, autoRemove);
     };
 
     event.unsubscribe = (subscriptionId) => {
@@ -307,16 +307,16 @@ export default class Contract {
     return this._api.eth.newFilter(options);
   }
 
-  subscribe (eventName = null, options = {}, callback) {
+  subscribe (eventName = null, options = {}, callback, autoRemove = false) {
     try {
       const event = this._findEvent(eventName);
-      return this._subscribe(event, options, callback);
+      return this._subscribe(event, options, callback, autoRemove);
     } catch (e) {
       return Promise.reject(e);
     }
   }
 
-  _subscribe (event = null, _options, callback) {
+  _subscribe (event = null, _options, callback, autoRemove = false) {
     const subscriptionId = nextSubscriptionId++;
     const { skipInitFetch } = _options;
     delete _options['skipInitFetch'];
@@ -326,6 +326,7 @@ export default class Contract {
       .then((filterId) => {
         this._subscriptions[subscriptionId] = {
           options: _options,
+          autoRemove,
           callback,
           filterId
         };
@@ -338,7 +339,11 @@ export default class Contract {
         return this._api.eth
           .getFilterLogs(filterId)
           .then((logs) => {
-            callback(null, this.parseEventLogs(logs));
+            const result = callback(null, this.parseEventLogs(logs));
+
+            if (autoRemove && !result) {
+              this.unsubscribe(subscriptionId);
+            }
 
             this._subscribeToChanges();
             return subscriptionId;
@@ -438,16 +443,22 @@ export default class Contract {
         })
       )
       .then((logsArray) => {
-        logsArray.forEach((logs, idx) => {
+        logsArray.forEach((logs, subscriptionId) => {
           if (!logs || !logs.length) {
             return;
           }
 
+          let result = false;
+
           try {
-            subscriptions[idx].callback(null, this.parseEventLogs(logs));
+            result = subscriptions[subscriptionId].callback(null, this.parseEventLogs(logs));
           } catch (error) {
             console.error('_sendSubscriptionChanges', error);
           }
+
+          if (subscriptions[subscriptionId].autoRemove && !result) {
+            this.unsubscribe(subscriptionId);
+          }
         });
       })
       .catch((error) => {

From 1ecda93de97b3974fc07812d9fcc4a530f65ff5e Mon Sep 17 00:00:00 2001
From: Jaco Greeff <jaco@ethcore.io>
Date: Fri, 9 Dec 2016 00:15:39 +0100
Subject: [PATCH 3/8] Align api.subscribe with contract

---
 js/src/api/subscriptions/manager.js | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/js/src/api/subscriptions/manager.js b/js/src/api/subscriptions/manager.js
index 300ede5f8..960961287 100644
--- a/js/src/api/subscriptions/manager.js
+++ b/js/src/api/subscriptions/manager.js
@@ -82,7 +82,11 @@ export default class Manager {
       if (!engine.isStarted) {
         engine.start();
       } else {
-        this._sendData(subscriptionId, error, data);
+        const result = this._sendData(subscriptionId, error, data);
+
+        if (autoRemove && !result) {
+          this.unsubscribe(subscriptionId);
+        }
       }
 
       resolve(subscriptionId);
@@ -102,19 +106,18 @@ export default class Manager {
   }
 
   _sendData (subscriptionId, error, data) {
-    const { callback } = this.subscriptions[subscriptionId];
+    let result = false;
 
     try {
-      return callback(error, data);
+      result = this.subscriptions[subscriptionId](error, data);
     } catch (error) {
       console.error(`Unable to update callback for subscriptionId ${subscriptionId}`, error);
     }
 
-    return true;
+    return result;
   }
 
   _updateSubscriptions = (subscriptionName, error, data) => {
-    const cleanup = [];
     const subscriptions = this.subscriptions
       .filter(subscription => subscription.name === subscriptionName);
 
@@ -125,13 +128,9 @@ export default class Manager {
         const result = this._sendData(subscription.id, error, data);
 
         if (subscription.autoRemove && !result) {
-          cleanup.push(subscription.id);
+          this.unsubscribe(subscription.id);
         }
       });
-
-    cleanup.forEach((subscriptionId) => {
-      delete this.subscriptions[subscriptionId];
-    });
   }
 }
 

From 3989e2642b9665b1b90a94b39eb2016a6bbb8550 Mon Sep 17 00:00:00 2001
From: Jaco Greeff <jaco@ethcore.io>
Date: Fri, 9 Dec 2016 00:25:50 +0100
Subject: [PATCH 4/8] Typo

---
 js/src/api/subscriptions/manager.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/js/src/api/subscriptions/manager.js b/js/src/api/subscriptions/manager.js
index 960961287..685aa243c 100644
--- a/js/src/api/subscriptions/manager.js
+++ b/js/src/api/subscriptions/manager.js
@@ -109,7 +109,7 @@ export default class Manager {
     let result = false;
 
     try {
-      result = this.subscriptions[subscriptionId](error, data);
+      result = this.subscriptions[subscriptionId].callback(error, data);
     } catch (error) {
       console.error(`Unable to update callback for subscriptionId ${subscriptionId}`, error);
     }

From 10c356de65bd269833ef12d6927a00d89ca3d9f8 Mon Sep 17 00:00:00 2001
From: Jaco Greeff <jaco@ethcore.io>
Date: Fri, 9 Dec 2016 00:52:21 +0100
Subject: [PATCH 5/8] Simplify, consolidate uses between subscription managers

---
 js/src/api/contract/contract.js     | 30 ++++++++++++++++-------------
 js/src/api/subscriptions/manager.js | 19 +++++++-----------
 2 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/js/src/api/contract/contract.js b/js/src/api/contract/contract.js
index f34cd8575..233115d68 100644
--- a/js/src/api/contract/contract.js
+++ b/js/src/api/contract/contract.js
@@ -316,6 +316,21 @@ export default class Contract {
     }
   }
 
+  _sendData (subscriptionId, error, logs) {
+    const { autoRemove, callback } = this._subscriptions[subscriptionId];
+    let result = false;
+
+    try {
+      result = callback(error, logs);
+    } catch (error) {
+      console.warn('_sendData', subscriptionId, error);
+    }
+
+    if (autoRemove && !result) {
+      this.unsubscribe(subscriptionId);
+    }
+  }
+
   _subscribe (event = null, _options, callback, autoRemove = false) {
     const subscriptionId = nextSubscriptionId++;
     const { skipInitFetch } = _options;
@@ -339,12 +354,7 @@ export default class Contract {
         return this._api.eth
           .getFilterLogs(filterId)
           .then((logs) => {
-            const result = callback(null, this.parseEventLogs(logs));
-
-            if (autoRemove && !result) {
-              this.unsubscribe(subscriptionId);
-            }
-
+            this._sendData(subscriptionId, null, this.parseEventLogs(logs));
             this._subscribeToChanges();
             return subscriptionId;
           });
@@ -448,17 +458,11 @@ export default class Contract {
             return;
           }
 
-          let result = false;
-
           try {
-            result = subscriptions[subscriptionId].callback(null, this.parseEventLogs(logs));
+            this.sendData(subscriptionId, null, this.parseEventLogs(logs));
           } catch (error) {
             console.error('_sendSubscriptionChanges', error);
           }
-
-          if (subscriptions[subscriptionId].autoRemove && !result) {
-            this.unsubscribe(subscriptionId);
-          }
         });
       })
       .catch((error) => {
diff --git a/js/src/api/subscriptions/manager.js b/js/src/api/subscriptions/manager.js
index 685aa243c..a2716684b 100644
--- a/js/src/api/subscriptions/manager.js
+++ b/js/src/api/subscriptions/manager.js
@@ -82,11 +82,7 @@ export default class Manager {
       if (!engine.isStarted) {
         engine.start();
       } else {
-        const result = this._sendData(subscriptionId, error, data);
-
-        if (autoRemove && !result) {
-          this.unsubscribe(subscriptionId);
-        }
+        this._sendData(subscriptionId, error, data);
       }
 
       resolve(subscriptionId);
@@ -106,15 +102,18 @@ export default class Manager {
   }
 
   _sendData (subscriptionId, error, data) {
+    const { autoRemove, callback } = this.subscriptions[subscriptionId];
     let result = false;
 
     try {
-      result = this.subscriptions[subscriptionId].callback(error, data);
+      result = callback(error, data);
     } catch (error) {
       console.error(`Unable to update callback for subscriptionId ${subscriptionId}`, error);
     }
 
-    return result;
+    if (autoRemove && !result) {
+      this.unsubscribe(subscriptionId);
+    }
   }
 
   _updateSubscriptions = (subscriptionName, error, data) => {
@@ -125,11 +124,7 @@ export default class Manager {
 
     subscriptions
       .forEach((subscription) => {
-        const result = this._sendData(subscription.id, error, data);
-
-        if (subscription.autoRemove && !result) {
-          this.unsubscribe(subscription.id);
-        }
+        this._sendData(subscription.id, error, data);
       });
   }
 }

From 13f962ae01c981ab4d4f7972af80b24dd951e246 Mon Sep 17 00:00:00 2001
From: Jaco Greeff <jaco@ethcore.io>
Date: Fri, 9 Dec 2016 00:56:12 +0100
Subject: [PATCH 6/8] Attempt to fix ci webpack

---
 js/src/api/contract/contract.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/js/src/api/contract/contract.js b/js/src/api/contract/contract.js
index 233115d68..2ef24c685 100644
--- a/js/src/api/contract/contract.js
+++ b/js/src/api/contract/contract.js
@@ -240,7 +240,7 @@ export default class Contract {
   }
 
   _bindEvent = (event) => {
-    event.subscribe = (options = {}, callback, autoRemove = false) => {
+    event.subscribe = (options = {}, callback, autoRemove) => {
       return this._subscribe(event, options, callback, autoRemove);
     };
 
@@ -307,7 +307,7 @@ export default class Contract {
     return this._api.eth.newFilter(options);
   }
 
-  subscribe (eventName = null, options = {}, callback, autoRemove = false) {
+  subscribe (eventName = null, options = {}, callback, autoRemove) {
     try {
       const event = this._findEvent(eventName);
       return this._subscribe(event, options, callback, autoRemove);

From 1b0945940bb4e2162577696167aeb0b4b9745f18 Mon Sep 17 00:00:00 2001
From: Jaco Greeff <jaco@ethcore.io>
Date: Fri, 9 Dec 2016 18:14:05 +0100
Subject: [PATCH 7/8] Test for boolean result before unsubscribe

---
 js/src/api/contract/contract.js     | 2 +-
 js/src/api/subscriptions/manager.js | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/js/src/api/contract/contract.js b/js/src/api/contract/contract.js
index 873dac38c..58dd62e7b 100644
--- a/js/src/api/contract/contract.js
+++ b/js/src/api/contract/contract.js
@@ -325,7 +325,7 @@ export default class Contract {
       console.warn('_sendData', subscriptionId, error);
     }
 
-    if (autoRemove && !result) {
+    if (autoRemove && !result && typeof result === 'boolean') {
       this.unsubscribe(subscriptionId);
     }
   }
diff --git a/js/src/api/subscriptions/manager.js b/js/src/api/subscriptions/manager.js
index a2716684b..f1afe685a 100644
--- a/js/src/api/subscriptions/manager.js
+++ b/js/src/api/subscriptions/manager.js
@@ -111,7 +111,7 @@ export default class Manager {
       console.error(`Unable to update callback for subscriptionId ${subscriptionId}`, error);
     }
 
-    if (autoRemove && !result) {
+    if (autoRemove && !result && typeof result === 'boolean') {
       this.unsubscribe(subscriptionId);
     }
   }

From 0cbef3050893f3b577cd995cbb7d0dc9304dea2a Mon Sep 17 00:00:00 2001
From: Jaco Greeff <jaco@ethcore.io>
Date: Fri, 9 Dec 2016 18:17:31 +0100
Subject: [PATCH 8/8] Unsubscribe on true

---
 js/src/api/contract/contract.js     | 4 ++--
 js/src/api/subscriptions/manager.js | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/js/src/api/contract/contract.js b/js/src/api/contract/contract.js
index 58dd62e7b..bfe7cabc4 100644
--- a/js/src/api/contract/contract.js
+++ b/js/src/api/contract/contract.js
@@ -317,7 +317,7 @@ export default class Contract {
 
   _sendData (subscriptionId, error, logs) {
     const { autoRemove, callback } = this._subscriptions[subscriptionId];
-    let result = false;
+    let result = true;
 
     try {
       result = callback(error, logs);
@@ -325,7 +325,7 @@ export default class Contract {
       console.warn('_sendData', subscriptionId, error);
     }
 
-    if (autoRemove && !result && typeof result === 'boolean') {
+    if (autoRemove && result && typeof result === 'boolean') {
       this.unsubscribe(subscriptionId);
     }
   }
diff --git a/js/src/api/subscriptions/manager.js b/js/src/api/subscriptions/manager.js
index f1afe685a..25e6e6129 100644
--- a/js/src/api/subscriptions/manager.js
+++ b/js/src/api/subscriptions/manager.js
@@ -103,7 +103,7 @@ export default class Manager {
 
   _sendData (subscriptionId, error, data) {
     const { autoRemove, callback } = this.subscriptions[subscriptionId];
-    let result = false;
+    let result = true;
 
     try {
       result = callback(error, data);
@@ -111,7 +111,7 @@ export default class Manager {
       console.error(`Unable to update callback for subscriptionId ${subscriptionId}`, error);
     }
 
-    if (autoRemove && !result && typeof result === 'boolean') {
+    if (autoRemove && result && typeof result === 'boolean') {
       this.unsubscribe(subscriptionId);
     }
   }