Skip to content

Commit

Permalink
🐛 Avoid mutating milestone snapshots when building from ops
Browse files Browse the repository at this point in the history
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
  • Loading branch information
alecgibson committed May 14, 2024
1 parent a5c19b9 commit 8a52f10
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/backend.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
Expand Down
32 changes: 32 additions & 0 deletions test/client/snapshot-version-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -440,6 +441,37 @@ 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) {
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');
Expand Down

0 comments on commit 8a52f10

Please sign in to comment.