Skip to content

Commit

Permalink
Merge pull request #659 from share/src-seq-create-meta
Browse files Browse the repository at this point in the history 
🗃 Add `_create` metadata to snapshots to avoid `getCommittedOpVersion()`
  • Loading branch information
@alecgibson
alecgibson authored May 28, 2024
2 parents f4effa3 + 5e5f1b4 commit eff464d
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 70 deletions.
25 changes: 24 additions & 1 deletion lib/submit-request.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ SubmitRequest.prototype.submit = function(callback) {
// case, we should return a non-fatal 'Op already submitted' error. We
// must get the past ops and check their src and seq values to
// differentiate.
backend.db.getCommittedOpVersion(collection, id, snapshot, op, null, function(err, version) {
request._fetchCreateOpVersion(function(error, version) {
if (err) return callback(err);
if (version == null) {
callback(request.alreadyCreatedError());
Expand Down Expand Up @@ -194,6 +194,15 @@ SubmitRequest.prototype.commit = function(callback) {
backend.trigger(backend.MIDDLEWARE_ACTIONS.commit, this.agent, this, function(err) {
if (err) return callback(err);
if (request._fixupOps.length) request.op.m.fixup = request._fixupOps;
if (request.op.create) {
// When we create the snapshot, we store a pointer to the op that created
// it. This allows us to return OP_ALREADY_SUBMITTED errors when appropriate.
request.snapshot.m._create = {
src: request.op.src,
seq: request.op.seq,
v: request.op.v
};
}

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

SubmitRequest.prototype._fetchCreateOpVersion = function(callback) {
var create = this.snapshot.m._create;
if (create) {
var version = (create.src === this.op.src && create.seq === this.op.seq) ? create.v : null;
return callback(null, version);
}

// We can only reach here if the snapshot is missing the create metadata.
// This can happen if a client tries to re-create or resubmit a create op to
// a "legacy" snapshot that existed before we started adding the meta (should
// be uncommon) or when using a driver that doesn't support metadata (eg Postgres).
this.backend.db.getCommittedOpVersion(this.collection, this.id, this.snapshot, this.op, null, callback);
};

// Non-fatal client errors:
SubmitRequest.prototype.alreadySubmittedError = function() {
return new ShareDBError(ERROR_CODE.ERR_OP_ALREADY_SUBMITTED, 'Op already submitted');
Expand Down
170 changes: 101 additions & 69 deletions test/client/submit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ var deserializedType = require('./deserialized-type');
var numberType = require('./number-type');
var errorHandler = require('../util').errorHandler;
var richText = require('rich-text');
var MemoryDB = require('../../lib/db/memory');
types.register(deserializedType.type);
types.register(deserializedType.type2);
types.register(numberType.type);
Expand Down Expand Up @@ -206,93 +207,124 @@ module.exports = function() {
});
});

it('can create a new doc then fetch', function(done) {
var doc = this.backend.connect().get('dogs', 'fido');
doc.create({age: 3}, function(err) {
if (err) return done(err);
doc.fetch(function(err) {
if (err) return done(err);
expect(doc.data).eql({age: 3});
expect(doc.version).eql(1);
done();
});

describe('create', function() {
describe('metadata enabled', function() {
runCreateTests();
});
});

it('calling create on the same doc twice fails', function(done) {
var doc = this.backend.connect().get('dogs', 'fido');
doc.create({age: 3}, function(err) {
if (err) return done(err);
doc.create({age: 4}, function(err) {
expect(err).instanceOf(Error);
expect(doc.version).equal(1);
expect(doc.data).eql({age: 3});
done();
describe('no snapshot metadata available', function() {
beforeEach(function() {
var getSnapshot = MemoryDB.prototype.getSnapshot;
sinon.stub(MemoryDB.prototype, 'getSnapshot')
.callsFake(function() {
var args = Array.from(arguments);
var callback = args.pop();
args.push(function(error, snapshot) {
if (snapshot) delete snapshot.m;
callback(error, snapshot);
});
getSnapshot.apply(this, args);
});
});
});
});

it('trying to create an already created doc without fetching fails and fetches', function(done) {
var doc = this.backend.connect().get('dogs', 'fido');
var doc2 = this.backend.connect().get('dogs', 'fido');
doc.create({age: 3}, function(err) {
if (err) return done(err);
doc2.create({age: 4}, function(err) {
expect(err).instanceOf(Error);
expect(doc2.version).equal(1);
expect(doc2.data).eql({age: 3});
done();
afterEach(function() {
sinon.restore();
});
});
});

it('does not fail when resubmitting a create op', function(done) {
var backend = this.backend;
var connection = backend.connect();
var submitted = false;
backend.use('submit', function(request, next) {
if (!submitted) {
submitted = true;
connection.close();
backend.connect(connection);
}
next();
runCreateTests();
});

var doc = connection.get('dogs', 'fido');
doc.create({age: 3}, function(error) {
expect(doc.version).to.equal(1);
done(error);
});
});
function runCreateTests() {
it('can create a new doc then fetch', function(done) {
var doc = this.backend.connect().get('dogs', 'fido');
doc.create({age: 3}, function(err) {
if (err) return done(err);
doc.fetch(function(err) {
if (err) return done(err);
expect(doc.data).eql({age: 3});
expect(doc.version).eql(1);
done();
});
});
});

it('does not fail when resubmitting a create op on a doc that was deleted', function(done) {
var backend = this.backend;
var connection1 = backend.connect();
var connection2 = backend.connect();
var doc1 = connection1.get('dogs', 'fido');
var doc2 = connection2.get('dogs', 'fido');

async.series([
doc1.create.bind(doc1, {age: 3}),
doc1.del.bind(doc1),
function(next) {
it('calling create on the same doc twice fails', function(done) {
var doc = this.backend.connect().get('dogs', 'fido');
doc.create({age: 3}, function(err) {
if (err) return done(err);
doc.create({age: 4}, function(err) {
expect(err).instanceOf(Error);
expect(doc.version).equal(1);
expect(doc.data).eql({age: 3});
done();
});
});
});

it('trying to create an already created doc without fetching fails and fetches', function(done) {
var doc = this.backend.connect().get('dogs', 'fido');
var doc2 = this.backend.connect().get('dogs', 'fido');
doc.create({age: 3}, function(err) {
if (err) return done(err);
doc2.create({age: 4}, function(err) {
expect(err).instanceOf(Error);
expect(doc2.version).equal(1);
expect(doc2.data).eql({age: 3});
done();
});
});
});

it('does not fail when resubmitting a create op', function(done) {
var backend = this.backend;
var connection = backend.connect();
var submitted = false;
backend.use('submit', function(request, next) {
if (!submitted) {
submitted = true;
connection2.close();
backend.connect(connection2);
connection.close();
backend.connect(connection);
}
next();
});

doc2.create({name: 'Fido'}, function(error) {
expect(doc2.version).to.equal(3);
next(error);
var doc = connection.get('dogs', 'fido');
doc.create({age: 3}, function(error) {
expect(doc.version).to.equal(1);
done(error);
});
}
], done);
});

it('does not fail when resubmitting a create op on a doc that was deleted', function(done) {
var backend = this.backend;
var connection1 = backend.connect();
var connection2 = backend.connect();
var doc1 = connection1.get('dogs', 'fido');
var doc2 = connection2.get('dogs', 'fido');

async.series([
doc1.create.bind(doc1, {age: 3}),
doc1.del.bind(doc1),
function(next) {
var submitted = false;
backend.use('submit', function(request, next) {
if (!submitted) {
submitted = true;
connection2.close();
backend.connect(connection2);
}
next();
});

doc2.create({name: 'Fido'}, function(error) {
expect(doc2.version).to.equal(3);
next(error);
});
}
], done);
});
}
});

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

0 comments on commit eff464d

Please sign in to comment.