Skip to content

Commit

Permalink
⚡️ Deduplicate fetch() network requests
Browse files Browse the repository at this point in the history
At the moment, if a client calls `doc.fetch()` with a fetch request
already in progress, the client will send a second request, which
probably isn't needed (or wanted).

This change prevents duplicate requests from being sent when there's
already an inflight fetch request, and adds a `force` option, which can
be used to force a duplicate request to be sent. This is currently used
internally for `_hardRollback()`, which forcibly unsets the
`doc.version`, and therefore requires a separate `fetch()` call, since
fetch requests are [handled differently][1] depending on the presence
of `version`.

[1]: https://github.com/share/sharedb/blob/21750e320f6865ee14620d6bf9610cb7ebc5f18e/lib/agent.js#L607
  • Loading branch information
alecgibson committed May 9, 2023
1 parent 21750e3 commit 8a1dbe5
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 19 deletions.
17 changes: 16 additions & 1 deletion docs/api/doc.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,26 @@ A `Doc` instance can be obtained with [`connection.getDoc()`]({{ site.baseurl }}
Populate [`data`](#data--object) with a snapshot of the document from the server

```js
doc.fetch([callback])
doc.fetch([options [,callback]])
```

{: .d-inline-block }

`options` -- Object

Optional
{: .label .label-grey }

> Default: `{}`
> `options.force` -- boolean (optional)
> > Default: `false`
> > If there's an inflight fetch in progress, calling `fetch()` again will not send a duplicate request. To force another network call to be made, set `force: true`.
{: .d-inline-block }

`callback` -- Function

Optional
Expand Down
30 changes: 12 additions & 18 deletions lib/client/doc.js
Original file line number Diff line number Diff line change
Expand Up @@ -424,13 +424,19 @@ 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;
Doc.prototype.fetch = function(options, callback) {
options = options || {};
if (typeof options === 'function') {
callback = options;
options = {};
}
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 +496,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 +1037,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

0 comments on commit 8a1dbe5

Please sign in to comment.