Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

⚡️ Deduplicate fetch() network requests #610

Merged
merged 1 commit into from
May 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 11 additions & 18 deletions lib/client/doc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions test/client/doc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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}];
Expand Down
4 changes: 2 additions & 2 deletions test/middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
Expand Down