Skip to content

Commit eff464d

Browse files
alecgibsonalecgibson
authored
Merge pull request #659 from share/src-seq-create-meta
🗃 Add `_create` metadata to snapshots to avoid `getCommittedOpVersion()`
2 parents f4effa3 + 5e5f1b4 commit eff464d

File tree

2 files changed

+125
-70
lines changed

2 files changed

+125
-70
lines changed

‎lib/submit-request.js‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ SubmitRequest.prototype.submit = function(callback) {
109109
// case, we should return a non-fatal 'Op already submitted' error. We
110110
// must get the past ops and check their src and seq values to
111111
// differentiate.
112-
backend.db.getCommittedOpVersion(collection, id, snapshot, op, null, function(err, version) {
112+
request._fetchCreateOpVersion(function(error, version) {
113113
if (err) return callback(err);
114114
if (version == null) {
115115
callback(request.alreadyCreatedError());
@@ -194,6 +194,15 @@ SubmitRequest.prototype.commit = function(callback) {
194194
backend.trigger(backend.MIDDLEWARE_ACTIONS.commit, this.agent, this, function(err) {
195195
if (err) return callback(err);
196196
if (request._fixupOps.length) request.op.m.fixup = request._fixupOps;
197+
if (request.op.create) {
198+
// When we create the snapshot, we store a pointer to the op that created
199+
// it. This allows us to return OP_ALREADY_SUBMITTED errors when appropriate.
200+
request.snapshot.m._create = {
201+
src: request.op.src,
202+
seq: request.op.seq,
203+
v: request.op.v
204+
};
205+
}
197206

198207
// Try committing the operation and snapshot to the database atomically
199208
backend.db.commit(
@@ -290,6 +299,20 @@ SubmitRequest.prototype._shouldSaveMilestoneSnapshot = function(snapshot) {
290299
return this.saveMilestoneSnapshot;
291300
};
292301

302+
SubmitRequest.prototype._fetchCreateOpVersion = function(callback) {
303+
var create = this.snapshot.m._create;
304+
if (create) {
305+
var version = (create.src === this.op.src && create.seq === this.op.seq) ? create.v : null;
306+
return callback(null, version);
307+
}
308+
309+
// We can only reach here if the snapshot is missing the create metadata.
310+
// This can happen if a client tries to re-create or resubmit a create op to
311+
// a "legacy" snapshot that existed before we started adding the meta (should
312+
// be uncommon) or when using a driver that doesn't support metadata (eg Postgres).
313+
this.backend.db.getCommittedOpVersion(this.collection, this.id, this.snapshot, this.op, null, callback);
314+
};
315+
293316
// Non-fatal client errors:
294317
SubmitRequest.prototype.alreadySubmittedError = function() {
295318
return new ShareDBError(ERROR_CODE.ERR_OP_ALREADY_SUBMITTED, 'Op already submitted');

‎test/client/submit.js‎

Lines changed: 101 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ var deserializedType = require('./deserialized-type');
66
var numberType = require('./number-type');
77
var errorHandler = require('../util').errorHandler;
88
var richText = require('rich-text');
9+
var MemoryDB = require('../../lib/db/memory');
910
types.register(deserializedType.type);
1011
types.register(deserializedType.type2);
1112
types.register(numberType.type);
@@ -206,93 +207,124 @@ module.exports = function() {
206207
});
207208
});
208209

209-
it('can create a new doc then fetch', function(done) {
210-
var doc = this.backend.connect().get('dogs', 'fido');
211-
doc.create({age: 3}, function(err) {
212-
if (err) return done(err);
213-
doc.fetch(function(err) {
214-
if (err) return done(err);
215-
expect(doc.data).eql({age: 3});
216-
expect(doc.version).eql(1);
217-
done();
218-
});
210+
211+
describe('create', function() {
212+
describe('metadata enabled', function() {
213+
runCreateTests();
219214
});
220-
});
221215

222-
it('calling create on the same doc twice fails', function(done) {
223-
var doc = this.backend.connect().get('dogs', 'fido');
224-
doc.create({age: 3}, function(err) {
225-
if (err) return done(err);
226-
doc.create({age: 4}, function(err) {
227-
expect(err).instanceOf(Error);
228-
expect(doc.version).equal(1);
229-
expect(doc.data).eql({age: 3});
230-
done();
216+
describe('no snapshot metadata available', function() {
217+
beforeEach(function() {
218+
var getSnapshot = MemoryDB.prototype.getSnapshot;
219+
sinon.stub(MemoryDB.prototype, 'getSnapshot')
220+
.callsFake(function() {
221+
var args = Array.from(arguments);
222+
var callback = args.pop();
223+
args.push(function(error, snapshot) {
224+
if (snapshot) delete snapshot.m;
225+
callback(error, snapshot);
226+
});
227+
getSnapshot.apply(this, args);
228+
});
231229
});
232-
});
233-
});
234230

235-
it('trying to create an already created doc without fetching fails and fetches', function(done) {
236-
var doc = this.backend.connect().get('dogs', 'fido');
237-
var doc2 = this.backend.connect().get('dogs', 'fido');
238-
doc.create({age: 3}, function(err) {
239-
if (err) return done(err);
240-
doc2.create({age: 4}, function(err) {
241-
expect(err).instanceOf(Error);
242-
expect(doc2.version).equal(1);
243-
expect(doc2.data).eql({age: 3});
244-
done();
231+
afterEach(function() {
232+
sinon.restore();
245233
});
246-
});
247-
});
248234

249-
it('does not fail when resubmitting a create op', function(done) {
250-
var backend = this.backend;
251-
var connection = backend.connect();
252-
var submitted = false;
253-
backend.use('submit', function(request, next) {
254-
if (!submitted) {
255-
submitted = true;
256-
connection.close();
257-
backend.connect(connection);
258-
}
259-
next();
235+
runCreateTests();
260236
});
261237

262-
var doc = connection.get('dogs', 'fido');
263-
doc.create({age: 3}, function(error) {
264-
expect(doc.version).to.equal(1);
265-
done(error);
266-
});
267-
});
238+
function runCreateTests() {
239+
it('can create a new doc then fetch', function(done) {
240+
var doc = this.backend.connect().get('dogs', 'fido');
241+
doc.create({age: 3}, function(err) {
242+
if (err) return done(err);
243+
doc.fetch(function(err) {
244+
if (err) return done(err);
245+
expect(doc.data).eql({age: 3});
246+
expect(doc.version).eql(1);
247+
done();
248+
});
249+
});
250+
});
268251

269-
it('does not fail when resubmitting a create op on a doc that was deleted', function(done) {
270-
var backend = this.backend;
271-
var connection1 = backend.connect();
272-
var connection2 = backend.connect();
273-
var doc1 = connection1.get('dogs', 'fido');
274-
var doc2 = connection2.get('dogs', 'fido');
275-
276-
async.series([
277-
doc1.create.bind(doc1, {age: 3}),
278-
doc1.del.bind(doc1),
279-
function(next) {
252+
it('calling create on the same doc twice fails', function(done) {
253+
var doc = this.backend.connect().get('dogs', 'fido');
254+
doc.create({age: 3}, function(err) {
255+
if (err) return done(err);
256+
doc.create({age: 4}, function(err) {
257+
expect(err).instanceOf(Error);
258+
expect(doc.version).equal(1);
259+
expect(doc.data).eql({age: 3});
260+
done();
261+
});
262+
});
263+
});
264+
265+
it('trying to create an already created doc without fetching fails and fetches', function(done) {
266+
var doc = this.backend.connect().get('dogs', 'fido');
267+
var doc2 = this.backend.connect().get('dogs', 'fido');
268+
doc.create({age: 3}, function(err) {
269+
if (err) return done(err);
270+
doc2.create({age: 4}, function(err) {
271+
expect(err).instanceOf(Error);
272+
expect(doc2.version).equal(1);
273+
expect(doc2.data).eql({age: 3});
274+
done();
275+
});
276+
});
277+
});
278+
279+
it('does not fail when resubmitting a create op', function(done) {
280+
var backend = this.backend;
281+
var connection = backend.connect();
280282
var submitted = false;
281283
backend.use('submit', function(request, next) {
282284
if (!submitted) {
283285
submitted = true;
284-
connection2.close();
285-
backend.connect(connection2);
286+
connection.close();
287+
backend.connect(connection);
286288
}
287289
next();
288290
});
289291

290-
doc2.create({name: 'Fido'}, function(error) {
291-
expect(doc2.version).to.equal(3);
292-
next(error);
292+
var doc = connection.get('dogs', 'fido');
293+
doc.create({age: 3}, function(error) {
294+
expect(doc.version).to.equal(1);
295+
done(error);
293296
});
294-
}
295-
], done);
297+
});
298+
299+
it('does not fail when resubmitting a create op on a doc that was deleted', function(done) {
300+
var backend = this.backend;
301+
var connection1 = backend.connect();
302+
var connection2 = backend.connect();
303+
var doc1 = connection1.get('dogs', 'fido');
304+
var doc2 = connection2.get('dogs', 'fido');
305+
306+
async.series([
307+
doc1.create.bind(doc1, {age: 3}),
308+
doc1.del.bind(doc1),
309+
function(next) {
310+
var submitted = false;
311+
backend.use('submit', function(request, next) {
312+
if (!submitted) {
313+
submitted = true;
314+
connection2.close();
315+
backend.connect(connection2);
316+
}
317+
next();
318+
});
319+
320+
doc2.create({name: 'Fido'}, function(error) {
321+
expect(doc2.version).to.equal(3);
322+
next(error);
323+
});
324+
}
325+
], done);
326+
});
327+
}
296328
});
297329

298330
it('server fetches and transforms by already committed op', function(done) {

0 commit comments

Comments
 (0)