diff --git a/docs/api/backend.md b/docs/api/backend.md index b1db01078..b3efa8b04 100644 --- a/docs/api/backend.md +++ b/docs/api/backend.md @@ -113,6 +113,19 @@ Optional {: .d-inline-block } +`doNotCommitNoOps` -- boolean + +Optional +{: .label .label-grey } + +> Default: `false` + +> If set to `true`, will avoid committing no-ops to the database. Clients that submit no-ops +> (or ops that are transformed to no-ops) will have their ops acknowledged as if they were +> committed, but the doc version will not increment. + +{: .d-inline-block } + `errorHandler` -- Function Optional diff --git a/docs/api/sharedb-error.md b/docs/api/sharedb-error.md index 4dc89fcd0..7b5e90587 100644 --- a/docs/api/sharedb-error.md +++ b/docs/api/sharedb-error.md @@ -100,3 +100,9 @@ Representation of an error, with a machine-parsable [code](#error-codes). ### `ERR_TYPE_CANNOT_BE_PROJECTED` > The document's type cannot be projected. [`json0`]({{ site.baseurl }}{% link types/json0.md %}) is currently the only type that supports projections. + +### `ERR_NO_OP` + +> The submitted op resulted in a no-op, possibly after transformation by a remote op. + +> This is normal behavior and the client should swallow this error without bumping doc version. \ No newline at end of file diff --git a/lib/backend.js b/lib/backend.js index 7a3cd41cc..dcc95aa33 100644 --- a/lib/backend.js +++ b/lib/backend.js @@ -43,6 +43,7 @@ function Backend(options) { 'new Backend({doNotForwardSendPresenceErrorsToClient: true})\n\n' ); } + this.doNotCommitNoOps = !!options.doNotCommitNoOps; // Map from event name to a list of middleware this.middleware = Object.create(null); 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/protocol.js b/lib/protocol.js index 208af5b6b..723478490 100644 --- a/lib/protocol.js +++ b/lib/protocol.js @@ -1,6 +1,6 @@ module.exports = { major: 1, - minor: 1, + minor: 2, checkAtLeast: checkAtLeast }; diff --git a/lib/submit-request.js b/lib/submit-request.js index ff379801c..e7e106c77 100644 --- a/lib/submit-request.js +++ b/lib/submit-request.js @@ -2,6 +2,7 @@ var ot = require('./ot'); var projections = require('./projections'); var ShareDBError = require('./error'); var types = require('./types'); +var protocol = require('./protocol'); var ERROR_CODE = ShareDBError.CODES; @@ -204,6 +205,19 @@ SubmitRequest.prototype.commit = function(callback) { }; } + var skipNoOp = backend.doNotCommitNoOps && + protocol.checkAtLeast(request.agent.protocol, '1.2') && + request.op.op && + request.op.op.length === 0 && + request._fixupOps.length === 0; + + if (skipNoOp) { + // 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 +375,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..cc829778b 100644 --- a/test/client/submit.js +++ b/test/client/submit.js @@ -1290,6 +1290,92 @@ module.exports = function() { }); }); + it('commits both of two identical ops submitted from different clients by default', function(done) { + var backend = this.backend; + backend.doNotCommitNoOps = false; + 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']}); + next(); + }, + function(next) { + backend.db.getOps('dogs', id, 0, null, {}, function(error, ops) { + if (error) return next(error); + // Expect: + // v0: create + // v1: update + // v2: duplicate update + expect(ops).to.have.length(3); + next(); + }); + } + ], done); + }); + + it('only commits one of two identical ops submitted from different clients', function(done) { + var backend = this.backend; + backend.doNotCommitNoOps = true; + 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']}); + 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; + backend.doNotCommitNoOps = true; + 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);