Merge pull request #610 from share/merge-inflight-fetch

⚡️ Deduplicate `fetch()` network requests

lib/client/doc.js

@@ -425,12 +425,17 @@ Doc.prototype._resubscribe = function() {
 
 // Request the current document snapshot or ops that bring us up to date
 Doc.prototype.fetch = function(callback) {
-  if (this.connection.canSend) {
-    var isDuplicate = this.connection.sendFetch(this);
-    pushActionCallback(this.inflightFetch, isDuplicate, callback);
-    return;
-  }
+  this._fetch({}, callback);
+};
+
+Doc.prototype._fetch = function(options, callback) {
   this.pendingFetch.push(callback);
+  var shouldSend = this.connection.canSend && (
+    options.force || !this.inflightFetch.length
+  );
+  if (!shouldSend) return;
+  this.inflightFetch.push(this.pendingFetch.shift());
+  this.connection.sendFetch(this);
 };
 
 // Fetch the initial document and keep receiving updates
@@ -490,18 +495,6 @@ Doc.prototype._flushSubscribe = function() {
   }
 };
 
-function pushActionCallback(inflight, isDuplicate, callback) {
-  if (isDuplicate) {
-    var lastCallback = inflight.pop();
-    inflight.push(function(err) {
-      lastCallback && lastCallback(err);
-      callback && callback(err);
-    });
-  } else {
-    inflight.push(callback);
-  }
-}
-
 function combineCallbacks(callbacks) {
   callbacks = callbacks.filter(util.truthy);
   if (!callbacks.length) return null;
@@ -1043,7 +1036,7 @@ Doc.prototype._hardRollback = function(err) {
 
   // Fetch the latest version from the server to get us back into a working state
   var doc = this;
-  this.fetch(function() {
+  this._fetch({force: true}, function() {
     // We want to check that no errors are swallowed, so we check that:
     // - there are callbacks to call, and
     // - that every single pending op called a callback

test/client/doc.js

@@ -5,6 +5,7 @@ var json0 = require('ot-json0').type;
 var richText = require('rich-text').type;
 var ShareDBError = require('../../lib/error');
 var errorHandler = require('../util').errorHandler;
+var sinon = require('sinon');
 
 describe('Doc', function() {
   beforeEach(function() {
@@ -84,6 +85,23 @@ describe('Doc', function() {
     });
   });
 
+  describe('fetch', function() {
+    it('only fetches once when calling in quick succession', function(done) {
+      var connection = this.connection;
+      var doc = connection.get('dogs', 'fido');
+      sinon.spy(connection, 'sendFetch');
+      var count = 0;
+      var finish = function() {
+        count++;
+        expect(connection.sendFetch).to.have.been.calledOnce;
+        if (count === 3) done();
+      };
+      doc.fetch(finish);
+      doc.fetch(finish);
+      doc.fetch(finish);
+    });
+  });
+
   describe('when connection closed', function() {
     beforeEach(function(done) {
       this.op1 = [{p: ['snacks'], oi: true}];

test/middleware.js

@@ -734,11 +734,11 @@ describe('middleware', function() {
         next();
       });
 
-      sinon.spy(doc, 'fetch');
+      sinon.spy(doc, '_fetch');
 
       doc.submitOp([{p: ['fetch'], oi: true}], function(error) {
         expect(error).to.be.ok;
-        expect(doc.fetch.calledOnce).to.be.true;
+        expect(doc._fetch.calledOnce).to.be.true;
         done();
       });
     });