diff --git a/lib/agent.js b/lib/agent.js index a3ee100d6..ef84c164f 100644 --- a/lib/agent.js +++ b/lib/agent.js @@ -768,6 +768,7 @@ Agent.prototype._submit = function(collection, id, op, callback) { // part of normal operation, since inflight ops need to be resent after // disconnect. In this case, ack the op so the client can proceed if (err.code === ERROR_CODE.ERR_OP_ALREADY_SUBMITTED) return callback(null, ack); + if (err.code === ERROR_CODE.ERR_NO_OP) return callback(err, ack); return callback(err); } diff --git a/lib/client/doc.js b/lib/client/doc.js index dd702eace..acb457374 100644 --- a/lib/client/doc.js +++ b/lib/client/doc.js @@ -328,6 +328,15 @@ Doc.prototype._handleSubscribe = function(error, snapshot) { Doc.prototype._handleOp = function(err, message) { if (err) { + if (err.code === ERROR_CODE.ERR_NO_OP && message.seq === this.inflightOp.seq) { + // Our op was a no-op, either because we submitted a no-op, or - more + // likely - because our op was transformed into a no-op by the server + // because of a similar remote op. In this case, the server has avoided + // committing the op to the database, and we should just clear the in-flight + // op and call the callbacks. However, let's first catch ourselves up to + // the remote, so that we're in a nice consistent state + return this.fetch(this._clearInflightOp.bind(this)); + } if (this.inflightOp) { return this._rollback(err); } diff --git a/lib/error.js b/lib/error.js index 0715faf38..cde95bdbe 100644 --- a/lib/error.js +++ b/lib/error.js @@ -35,6 +35,7 @@ ShareDBError.CODES = { ERR_MAX_SUBMIT_RETRIES_EXCEEDED: 'ERR_MAX_SUBMIT_RETRIES_EXCEEDED', ERR_MESSAGE_BADLY_FORMED: 'ERR_MESSAGE_BADLY_FORMED', ERR_MILESTONE_ARGUMENT_INVALID: 'ERR_MILESTONE_ARGUMENT_INVALID', + ERR_NO_OP: 'ERR_NO_OP', ERR_OP_ALREADY_SUBMITTED: 'ERR_OP_ALREADY_SUBMITTED', ERR_OP_NOT_ALLOWED_IN_PROJECTION: 'ERR_OP_NOT_ALLOWED_IN_PROJECTION', ERR_OP_SUBMIT_REJECTED: 'ERR_OP_SUBMIT_REJECTED', diff --git a/lib/submit-request.js b/lib/submit-request.js index ff379801c..efde23f38 100644 --- a/lib/submit-request.js +++ b/lib/submit-request.js @@ -204,6 +204,13 @@ SubmitRequest.prototype.commit = function(callback) { }; } + if (request.op.op && request.op.op.length === 0 && request._fixupOps.length === 0) { + // The op is a no-op, either because it was submitted as such, or - more + // likely - because it was transformed into one. Let's avoid committing it + // and tell the client. + return callback(request.noOpError()); + } + // Try committing the operation and snapshot to the database atomically backend.db.commit( request.collection, @@ -361,3 +368,9 @@ SubmitRequest.prototype.maxRetriesError = function() { 'Op submit failed. Exceeded max submit retries of ' + this.maxRetries ); }; +SubmitRequest.prototype.noOpError = function() { + return new ShareDBError( + ERROR_CODE.ERR_NO_OP, + 'Op is a no-op. Skipping commit.' + ); +}; diff --git a/test/client/submit.js b/test/client/submit.js index 8fa098930..3ba32eb05 100644 --- a/test/client/submit.js +++ b/test/client/submit.js @@ -1290,6 +1290,59 @@ module.exports = function() { }); }); + it('only commits one of two identical ops submitted from different clients', function(done) { + var backend = this.backend; + var doc1 = backend.connect().get('dogs', id); + var doc2 = backend.connect().get('dogs', id); + var op = [{p: ['tricks', 0], ld: 'fetch'}]; + + async.series([ + doc1.create.bind(doc1, {tricks: ['fetch', 'sit']}), + doc2.fetch.bind(doc2), + async.parallel.bind(async.parallel, [ + doc1.submitOp.bind(doc1, op), + doc2.submitOp.bind(doc2, op) + ]), + function(next) { + expect(doc1.data).to.eql({tricks: ['sit']}); + expect(doc2.data).to.eql({tricks: ['sit']}); + expect(doc1.version).to.equal(2); + expect(doc2.version).to.equal(2); + next(); + }, + function(next) { + backend.db.getOps('dogs', id, 0, null, {}, function(error, ops) { + if (error) return next(error); + // Expect: + // v0: create + // v1: update + // no duplicate update + expect(ops).to.have.length(2); + next(); + }); + } + ], done); + }); + + it('fixes up even if an op is fixed up to become a no-op', function(done) { + var backend = this.backend; + var doc = backend.connect().get('dogs', id); + + backend.use('apply', function(req, next) { + req.$fixup([{p: ['fixme'], od: true}]); + next(); + }); + + async.series([ + doc.create.bind(doc, {}), + doc.submitOp.bind(doc, [{p: ['fixme'], oi: true}]), + function(next) { + expect(doc.data).to.eql({}); + next(); + } + ], done); + }); + describe('type.deserialize', function() { it('can create a new doc', function(done) { var doc = this.backend.connect().get('dogs', id);