From 4b636fcb2d39eed2e41ea46a4aeafb3db1fa2b5c Mon Sep 17 00:00:00 2001
From: Alec Gibson <12036746+alecgibson@users.noreply.github.com>
Date: Tue, 27 Aug 2024 17:47:18 +0100
Subject: [PATCH] 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
---
 lib/agent.js                         | 17 ++++-------------
 test/client/presence/doc-presence.js | 20 --------------------
 2 files changed, 4 insertions(+), 33 deletions(-)

diff --git a/lib/agent.js b/lib/agent.js
index 4b11e1e54..a3ee100d6 100644
--- a/lib/agent.js
+++ b/lib/agent.js
@@ -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) {
@@ -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
@@ -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);
 };
 
diff --git a/test/client/presence/doc-presence.js b/test/client/presence/doc-presence.js
index b30865194..45772b9aa 100644
--- a/test/client/presence/doc-presence.js
+++ b/test/client/presence/doc-presence.js
@@ -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