V5: Option "subscribeToSelf" using volatile data (#268)

* use volatiles instead of request ids to track notifications from our own actions

* update dist build

Author: scottinet

dist/kuzzle.js

@@ -1,6 +1,6 @@
 {
   "name": "kuzzle-sdk",
-  "version": "5.0.10",
+  "version": "5.0.11",
   "description": "Official Javascript SDK for Kuzzle",
   "author": "The Kuzzle Team <[email protected]>",
   "repository": {
@@ -54,4 +54,4 @@
   "engines": {
     "node": ">= 6.9.1"
   }
-}
+}

dist/kuzzle.js.map

@@ -42,9 +42,8 @@ function Kuzzle (host, options, cb) {
 
   Object.defineProperties(this, {
     // 'private' properties
-    cleanHistoryTimer: {
-      value: null,
-      writable: true
+    id: {
+      value: uuidv4()
     },
     collections: {
       value: {},
@@ -72,10 +71,6 @@ function Kuzzle (host, options, cb) {
       value: false,
       writable: true
     },
-    requestHistory: {
-      value: {},
-      writable: true
-    },
     state: {
       value: 'initializing',
       writable: true
@@ -293,8 +288,6 @@ function Kuzzle (host, options, cb) {
     this.state = 'ready';
   }
 
-  this.cleanHistoryTimer = setInterval(function () { cleanHistory(self.requestHistory); }, 1000);
-
   if (this.bluebird) {
     return this.bluebird.promisifyAll(this, {
       suffix: 'Promise',
@@ -838,21 +831,6 @@ function cleanQueue () {
   }
 }
 
-
-/**
- * Clean history from requests made more than 10s ago
- */
-function cleanHistory (requestHistory) {
-  var
-    now = Date.now();
-
-  Object.keys(requestHistory).forEach(function (key) {
-    if (requestHistory[key] < now - 10000) {
-      delete requestHistory[key];
-    }
-  });
-}
-
 /**
  * Emit a request to Kuzzle
  *
@@ -886,9 +864,6 @@ function emitRequest (request, cb) {
   }
 
   this.network.send(request);
-
-  // Track requests made to allow Room.subscribeToSelf to work
-  self.requestHistory[request.requestId] = Date.now();
 }
 
 /**
@@ -1162,7 +1137,6 @@ Kuzzle.prototype.listIndexes = function (options, cb) {
 Kuzzle.prototype.disconnect = function () {
   var collection;
 
-  clearInterval(this.cleanHistoryTimer);
   this.state = 'disconnected';
   this.network.close();
   this.network = null;
@@ -1439,6 +1413,7 @@ Kuzzle.prototype.query = function (queryArgs, query, options, cb) {
   }
 
   object.volatile.sdkVersion = this.sdkVersion;
+  object.volatile.sdkInstanceId = this.id;
 
   if (self.state === 'connected' || (options && options.queuable === false)) {
     if (self.state === 'connected') {

package.json

@@ -321,12 +321,7 @@ function notificationCallback (data) {
     delete data.result;
   }
 
-  if (this.kuzzle.requestHistory[data.requestId]) {
-    if (this.subscribeToSelf) {
-      this.callback(null, data);
-    }
-    delete this.kuzzle.requestHistory[data.requestId];
-  } else {
+  if (this.subscribeToSelf || !data.volatile || data.volatile.sdkInstanceId !== this.kuzzle.id) {
     this.callback(null, data);
   }
 }

src/Kuzzle.js

@@ -416,23 +416,23 @@ describe('Room methods', function () {
         });
     });
 
-    it('should delete the result from history if emitted by this instance', function () {
-      room.subscribeToSelf = true;
-      kuzzle.requestHistory.bar = {};
-      notifCB.call(room, {type: 'document', result: {}, action: 'foo', requestId: 'bar'});
-
-      should(room.callback)
-        .be.calledOnce();
-      should(kuzzle.requestHistory).be.empty();
-    });
-
     it('should not forward the message if subscribeToSelf is false and the response comes from a query emitted by this instance', function () {
       room.subscribeToSelf = false;
-      kuzzle.requestHistory.bar = {};
-      notifCB.call(room, {type: 'document', result: {}, requestId: 'bar', action: 'foo'});
 
+      notifCB.call(room, {type: 'document', result: {}, action: 'foo', volatile: {sdkInstanceId: kuzzle.id}});
       should(room.callback).not.be.called();
-      should(kuzzle.requestHistory).be.empty();
+
+      room.callback.resetHistory();
+      notifCB.call(room, {type: 'document', result: {}, action: 'foo', volatile: {sdkInstanceId: 'foobar'}});
+      should(room.callback).be.calledOnce();
+
+      room.callback.resetHistory();
+      notifCB.call(room, {type: 'document', result: {}, action: 'foo', volatile: {barfoo: 'foobar'}});
+      should(room.callback).be.calledOnce();
+
+      room.callback.resetHistory();
+      notifCB.call(room, {type: 'document', result: {}, action: 'foo'});
+      should(room.callback).be.calledOnce();
     });
 
     it('should fire a "tokenExpired" event when receiving a TokenExpired notification', function () {

src/Room.js

@@ -18,13 +18,11 @@ describe('Query management', function () {
 
     it('should emit the request when asked to', function () {
       var
-        start = Date.now(),
         request = {requestId: 'bar'},
         spy = sinon.spy(kuzzle.network, 'send');
 
       emitRequest.call(kuzzle, request);
       should(spy).be.calledWithMatch(request);
-      should(kuzzle.requestHistory.bar).be.within(start, Date.now());
     });
 
     it('should trigger a tokenExpired event if the token has expired', function (done) {
@@ -123,7 +121,7 @@ describe('Query management', function () {
         collection: 'collection',
         controller: 'controller',
         index: 'index',
-        volatile: { sdkVersion: kuzzle.sdkVersion },
+        volatile: { sdkInstanceId: kuzzle.id, sdkVersion: kuzzle.sdkVersion },
         requestId: sinon.match(/^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i)
       });
     });
@@ -139,7 +137,7 @@ describe('Query management', function () {
         collection: 'collection',
         controller: 'controller',
         index: 'index',
-        volatile: { sdkVersion: kuzzle.sdkVersion },
+        volatile: { sdkInstanceId: kuzzle.id, sdkVersion: kuzzle.sdkVersion },
         requestId: sinon.match(/^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i)
       }, sinon.match(function(f) {return f === cb;}));
     });
@@ -378,51 +376,4 @@ describe('Query management', function () {
       should(kuzzle.offlineQueue[1].query.jwt).be.undefined();
     });
   });
-
-  describe('#cleanHistory', function () {
-    it('should be started by kuzzle constructor', function () {
-      var 
-        cleanStub = sinon.stub(),
-        clock = sinon.useFakeTimers(),
-        kuzzle;
-
-      // we need to re-import so that fake timers can take effect
-      Kuzzle = rewire('../../src/Kuzzle');
-      Kuzzle.__set__('cleanHistory', cleanStub);
-      kuzzle = new Kuzzle('foo', {connect: 'manual'});
-
-      clock.tick(1000);
-
-      should(kuzzle.cleanHistoryTimer).be.not.null();
-      should(cleanStub.calledOnce).be.true();
-
-      clock.restore();
-    });
-
-    it('should clean oldest entries every 1s', function () {
-      var
-        i,
-        clock = sinon.useFakeTimers(),
-        kuzzle;
-
-      Kuzzle = rewire('../../src/Kuzzle');
-      kuzzle = new Kuzzle('foo', {connect: 'manual'});
-
-      for (i = 100000; i >= 0; i -= 10000) {
-        kuzzle.requestHistory[i] = -i;
-      }
-
-      clock.tick(1000);
-
-      // should only contains i == 0 entry
-      should(Object.keys(kuzzle.requestHistory)).match(['0']);
-
-      kuzzle.requestHistory.foobar = -100000;
-
-      clock.tick(1000);
-      should(Object.keys(kuzzle.requestHistory)).match(['0']);
-
-      clock.restore();
-    });
-  });
 });