Remove cached ops without using setTimeout

See share#219

Author: gkubisa

lib/client/doc.js

@@ -83,7 +83,7 @@ function Doc(connection, collection, id) {
   this.requestReplyPresence = true;
   // A list of ops sent by the server. These are needed for transforming presence data,
   // if we get that presence data for an older version of the document.
-  // The ops are cached for 1 minute by default, which should be lots, considering that the presence
+  // The ops are cached for at least 1 minute by default, which should be lots, considering that the presence
   // data is supposed to be synced in real-time.
   this.cachedOps = [];
   this.cachedOpsTimeout = 60000;
@@ -362,6 +362,7 @@ Doc.prototype._handleOp = function(err, message) {
 
   var serverOp = {
     src: message.src,
+    time: Date.now(),
     create: !!message.create,
     op: message.op,
     del: !!message.del
@@ -914,6 +915,7 @@ Doc.prototype._opAcknowledged = function(message) {
   this.version++;
   this._cacheOp({
     src: this.inflightOp.src,
+    time: Date.now(),
     create: !!this.inflightOp.create,
     op: this.inflightOp.op,
     del: !!this.inflightOp.del
@@ -1283,8 +1285,18 @@ Doc.prototype._emitPresence = function(srcList, submitted) {
 };
 
 Doc.prototype._cacheOp = function(op) {
+  // Remove the old ops.
+  var oldOpTime = Date.now() - this.cachedOpsTimeout;
+  var i;
+  for (i = 0; i < this.cachedOps.length; i++) {
+    if (this.cachedOps[i].time >= oldOpTime) {
+      break;
+    }
+  }
+  if (i > 0) {
+    this.cachedOps.splice(0, i);
+  }
+
+  // Cache the new op.
   this.cachedOps.push(op);
-  setTimeout(function() {
-    if (this.cachedOps[0] === op) this.cachedOps.shift();
-  }.bind(this), this.cachedOpsTimeout);
 };

test/client/presence.js

@@ -473,48 +473,44 @@ types.register(presenceType.type3);
       ], allDone);
     });
 
-    it('removes cached ops', function(allDone) {
-      var op = { index: 1, value: 'b' };
-      this.doc.cachedOpsTimeout = 0;
+    it('expires cached ops', function(allDone) {
+      var op1 = { index: 1, value: 'b' };
+      var op2 = { index: 2, value: 'b' };
+      var op3 = { index: 3, value: 'b' };
+      this.doc.cachedOpsTimeout = 60;
       async.series([
+        // Cache 2 ops.
         this.doc.create.bind(this.doc, [ 'a' ], typeName),
-        this.doc.submitOp.bind(this.doc, op),
-        this.doc.del.bind(this.doc),
+        this.doc.submitOp.bind(this.doc, op1),
         function(done) {
-          expect(this.doc.cachedOps.length).to.equal(3);
+          expect(this.doc.cachedOps.length).to.equal(2);
           expect(this.doc.cachedOps[0].create).to.equal(true);
-          expect(this.doc.cachedOps[1].op).to.equal(op);
-          expect(this.doc.cachedOps[2].del).to.equal(true);
+          expect(this.doc.cachedOps[1].op).to.equal(op1);
           done();
         }.bind(this),
-        setTimeout,
-        function(done) {
-          expect(this.doc.cachedOps.length).to.equal(0);
-          done();
-        }.bind(this)
-      ], allDone);
-    });
 
-    it('removes correct cached ops', function(allDone) {
-      var op = { index: 1, value: 'b' };
-      this.doc.cachedOpsTimeout = 0;
-      async.series([
-        this.doc.create.bind(this.doc, [ 'a' ], typeName),
-        this.doc.submitOp.bind(this.doc, op),
-        this.doc.del.bind(this.doc),
+        // Cache another op before the first 2 expire.
+        function (callback) {
+          setTimeout(callback, 30);
+        },
+        this.doc.submitOp.bind(this.doc, op2),
         function(done) {
           expect(this.doc.cachedOps.length).to.equal(3);
           expect(this.doc.cachedOps[0].create).to.equal(true);
-          expect(this.doc.cachedOps[1].op).to.equal(op);
-          expect(this.doc.cachedOps[2].del).to.equal(true);
-          this.doc.cachedOps.shift();
-          this.doc.cachedOps.push({ op: true });
+          expect(this.doc.cachedOps[1].op).to.equal(op1);
+          expect(this.doc.cachedOps[2].op).to.equal(op2);
           done();
         }.bind(this),
-        setTimeout,
+
+        // Cache another op after the first 2 expire.
+        function (callback) {
+          setTimeout(callback, 31);
+        },
+        this.doc.submitOp.bind(this.doc, op3),
         function(done) {
-          expect(this.doc.cachedOps.length).to.equal(1);
-          expect(this.doc.cachedOps[0].op).to.equal(true);
+          expect(this.doc.cachedOps.length).to.equal(2);
+          expect(this.doc.cachedOps[0].op).to.equal(op2);
+          expect(this.doc.cachedOps[1].op).to.equal(op3);
           done();
         }.bind(this)
       ], allDone);