🐛 Avoid mutating milestone snapshots when building from ops

At the moment, we directly pass our milestone snapshot to
`ot.applyOps()`, which [mutates the snapshot][1].

In most Production-grade implementations of the `MilestoneDb`, this is
fine, since mutating the in-memory `Snapshot` will have no effect on the
underlying database representation.

However, this does break the `MemoryMilestoneDb` (eg in tests).

In order to fix this, and generally be defensive (eg if db adapters
decide to implement an in-memory cache), this change clones the snapshot
before calling `ot.applyOps()` on it.

[1]: https://github.com/share/sharedb/blob/a5c19b986d65c93abbc9ed11f7a25c0dbaf0788c/lib/ot.js#L172

Author: alecgibson

lib/backend.js

@@ -909,7 +909,7 @@ Backend.prototype._fetchSnapshotByTimestamp = function(collection, id, timestamp
 };
 
 Backend.prototype._buildSnapshotFromOps = function(id, startingSnapshot, ops, callback) {
-  var snapshot = startingSnapshot || new Snapshot(id, 0, null, undefined, null);
+  var snapshot = util.clone(startingSnapshot) || new Snapshot(id, 0, null, undefined, null);
   var error = ot.applyOps(snapshot, ops, {_normalizeLegacyJson0Ops: true});
   callback(error, snapshot);
 };

test/client/snapshot-version-request.js

@@ -6,6 +6,7 @@ var sinon = require('sinon');
 var async = require('async');
 var json0v2 = require('ot-json0-v2').type;
 var types = require('../../lib/types');
+var clone = require('../../lib/util').clone;
 
 describe('SnapshotVersionRequest', function() {
   var backend;
@@ -440,6 +441,39 @@ describe('SnapshotVersionRequest', function() {
       ], done);
     });
 
+    it('does not mutate the milestone snapshot', function(done) {
+      var connection = backendWithMilestones.connect();
+      var doc = connection.get('books', 'mocking-bird');
+      var milestoneDb = backendWithMilestones.milestoneDb;
+      var milestone;
+
+      async.waterfall([
+        doc.create.bind(doc, {title: 'To Kill a Mocking Bird'}),
+        doc.submitOp.bind(doc, {p: ['author'], oi: 'Harper Lea'}),
+        doc.submitOp.bind(doc, {p: ['author'], od: 'Harper Lea', oi: 'Harper Lee'}),
+        milestoneDb.getMilestoneSnapshot.bind(milestoneDb, 'books', 'mocking-bird', 3),
+        function(snapshot, next) {
+          milestone = clone(snapshot);
+          next();
+        },
+        function(next) {
+          // Internally, this calls ot.applyOps(), which mutates the snapshot it's passed.
+          // This call shouldn't cause the in-memory milestone snapshot to be mutated.
+          connection.fetchSnapshot('books', 'mocking-bird', 3, next);
+        },
+        function(snapshot, next) {
+          expect(snapshot.v).to.equal(3);
+          expect(snapshot.data).to.eql({title: 'To Kill a Mocking Bird', author: 'Harper Lee'});
+          next();
+        },
+        milestoneDb.getMilestoneSnapshot.bind(milestoneDb, 'books', 'mocking-bird', 3),
+        function(snapshot, next) {
+          expect(snapshot).to.eql(milestone);
+          next();
+        }
+      ], done);
+    });
+
     describe('with version null', function() {
       it('fetches a latest version', function(done) {
         var doc = backendWithMilestones.connect().get('books', 'mocking-bird');