diff --git a/lib/submit-request.js b/lib/submit-request.js index b1e1b066d..5c33fb9b1 100644 --- a/lib/submit-request.js +++ b/lib/submit-request.js @@ -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()); @@ -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( @@ -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'); diff --git a/test/client/submit.js b/test/client/submit.js index 88cef7864..c74058499 100644 --- a/test/client/submit.js +++ b/test/client/submit.js @@ -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); @@ -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) {