Skip to content

Commit

Permalink
Review markups
Browse files Browse the repository at this point in the history
- Clean up `latestDocVersions` when unsubscribing
- Fix not setting version if the object already exists
- Removes `null` presence check, which won't work if the object has been
  destroyed
  • Loading branch information
alecgibson committed Aug 27, 2024
1 parent 87ef11d commit 4b636fc
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 33 deletions.
17 changes: 4 additions & 13 deletions lib/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -830,16 +830,7 @@ Agent.prototype._broadcastPresence = function(presence, callback) {
var latestDocVersion = util.dig(agent.latestDocVersions, presence.c, presence.d);
var presenceIsUpToDate = presence.v === latestDocVersion;
if (!presenceIsUpToDate) {
// null presence can't be transformed, so skip the database call and just
// set the version to the latest known Doc version
if (presence.p === null) {
transformer = function(agent, presence, callback) {
presence.v = latestDocVersion;
callback(null, presence);
};
} else {
transformer = backend.transformPresenceToLatestVersion.bind(backend);
}
transformer = backend.transformPresenceToLatestVersion.bind(backend);
}

transformer(agent, presence, function(error, presence) {
Expand All @@ -866,9 +857,8 @@ Agent.prototype._subscribeDocVersion = function(collection, id, callback) {
this.backend.subscribe(this, collection, id, null, function(error, stream, snapshot) {
if (error) return callback(error);

util.digOrCreate(latestDocVersions, collection, id, function() {
return snapshot.v;
});
var versions = latestDocVersions[collection] || (latestDocVersions[collection] = Object.create(null));
versions[id] = snapshot.v;

agent._subscribeMapToStream(agent.latestDocVersionStreams, collection, id, stream, function(op) {
// op.v behind snapshot.v by 1
Expand All @@ -882,6 +872,7 @@ Agent.prototype._subscribeDocVersion = function(collection, id, callback) {
Agent.prototype._unsubscribeDocVersion = function(collection, id, callback) {
var stream = util.dig(this.latestDocVersionStreams, collection, id);
if (stream) stream.destroy();
util.digAndRemove(this.latestDocVersions, collection, id);
util.nextTick(callback);
};

Expand Down
20 changes: 0 additions & 20 deletions test/client/presence/doc-presence.js
Original file line number Diff line number Diff line change
Expand Up @@ -315,26 +315,6 @@ describe('DocPresence', function() {
], done);
});

it('does not call getOps() for old presence when it is null', function(done) {
var localPresence1 = presence1.create('presence-1');

async.series([
doc1.unsubscribe.bind(doc1),
doc2.submitOp.bind(doc2, {index: 5, value: 'ern'}),
function(next) {
expect(doc1.version).to.eql(1);
expect(doc2.version).to.eql(2);

sinon.spy(Backend.prototype, 'getOps');
localPresence1.submit(null, function(error) {
if (error) return next(error);
expect(Backend.prototype.getOps).not.to.have.been.called;
next();
});
}
], done);
});

// This test case attempts to force us into a tight race condition corner case:
// 1. doc1 sends presence, as well as submits an op
// 2. doc2 receives the op first, followed by the presence, which is now out-of-date
Expand Down

0 comments on commit 4b636fc

Please sign in to comment.