Skip to content

Commit 4b636fc

Browse files
committed
Review markups
- 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
1 parent 87ef11d commit 4b636fc

File tree

2 files changed

+4
-33
lines changed

2 files changed

+4
-33
lines changed

lib/agent.js

+4-13
Original file line numberDiff line numberDiff line change
@@ -830,16 +830,7 @@ Agent.prototype._broadcastPresence = function(presence, callback) {
830830
var latestDocVersion = util.dig(agent.latestDocVersions, presence.c, presence.d);
831831
var presenceIsUpToDate = presence.v === latestDocVersion;
832832
if (!presenceIsUpToDate) {
833-
// null presence can't be transformed, so skip the database call and just
834-
// set the version to the latest known Doc version
835-
if (presence.p === null) {
836-
transformer = function(agent, presence, callback) {
837-
presence.v = latestDocVersion;
838-
callback(null, presence);
839-
};
840-
} else {
841-
transformer = backend.transformPresenceToLatestVersion.bind(backend);
842-
}
833+
transformer = backend.transformPresenceToLatestVersion.bind(backend);
843834
}
844835

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

869-
util.digOrCreate(latestDocVersions, collection, id, function() {
870-
return snapshot.v;
871-
});
860+
var versions = latestDocVersions[collection] || (latestDocVersions[collection] = Object.create(null));
861+
versions[id] = snapshot.v;
872862

873863
agent._subscribeMapToStream(agent.latestDocVersionStreams, collection, id, stream, function(op) {
874864
// op.v behind snapshot.v by 1
@@ -882,6 +872,7 @@ Agent.prototype._subscribeDocVersion = function(collection, id, callback) {
882872
Agent.prototype._unsubscribeDocVersion = function(collection, id, callback) {
883873
var stream = util.dig(this.latestDocVersionStreams, collection, id);
884874
if (stream) stream.destroy();
875+
util.digAndRemove(this.latestDocVersions, collection, id);
885876
util.nextTick(callback);
886877
};
887878

test/client/presence/doc-presence.js

-20
Original file line numberDiff line numberDiff line change
@@ -315,26 +315,6 @@ describe('DocPresence', function() {
315315
], done);
316316
});
317317

318-
it('does not call getOps() for old presence when it is null', function(done) {
319-
var localPresence1 = presence1.create('presence-1');
320-
321-
async.series([
322-
doc1.unsubscribe.bind(doc1),
323-
doc2.submitOp.bind(doc2, {index: 5, value: 'ern'}),
324-
function(next) {
325-
expect(doc1.version).to.eql(1);
326-
expect(doc2.version).to.eql(2);
327-
328-
sinon.spy(Backend.prototype, 'getOps');
329-
localPresence1.submit(null, function(error) {
330-
if (error) return next(error);
331-
expect(Backend.prototype.getOps).not.to.have.been.called;
332-
next();
333-
});
334-
}
335-
], done);
336-
});
337-
338318
// This test case attempts to force us into a tight race condition corner case:
339319
// 1. doc1 sends presence, as well as submits an op
340320
// 2. doc2 receives the op first, followed by the presence, which is now out-of-date

0 commit comments

Comments
 (0)