Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Allow to preserve previous snapshot version that will be available in commit middleware #667

Closed
wants to merge 1 commit into from
Closed

✨ Allow to preserve previous snapshot version that will be available in commit middleware #667

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions docs/middleware/actions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ copy:
`snapshot` -- [`Snapshot`]({{ site.baseurl }}{% link api/snapshot.md %})
> The snapshot

`preservePreapplySnapshot` -- boolean
> This flag can be set to true in any middleware action before `commit`. This will inform shareDb
to clone snapshot data and store it in the request object, so it is available in commit middleware as `preapplySnapshot`

`preapplySnapshot` -- [`Snapshot`]({{ site.baseurl }}{% link api/snapshot.md %})
> The snapshot captures the state prior to the application of operations. It is accessible exclusively during the `commit` action, and only when the `preservePreapplySnapshot` flag is set to `true`.

`extra` -- Object
> `extra.source` -- Object
>> The submitted source when [`doc.submitSource`]({{ site.baseurl }}{% link api/doc.md %}http://localhost:4000/api/doc#submitsource--boolean) is set to `true`
Expand Down
12 changes: 12 additions & 0 deletions lib/submit-request.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ var ot = require('./ot');
var projections = require('./projections');
var ShareDBError = require('./error');
var types = require('./types');
var util = require('./util');

var ERROR_CODE = ShareDBError.CODES;

Expand Down Expand Up @@ -42,6 +43,8 @@ function SubmitRequest(backend, agent, index, id, op, options) {
this.ops = [];
this.channels = null;
this._fixupOps = [];
this.preservePreapplySnapshot = false;
this.preapplySnapshot = null;
}
module.exports = SubmitRequest;

Expand Down Expand Up @@ -180,6 +183,15 @@ SubmitRequest.prototype.apply = function(callback) {
this.backend.trigger(this.backend.MIDDLEWARE_ACTIONS.apply, this.agent, this, function(err) {
if (err) return callback(err);

/**
* Middleware is able to opt in to store old snapshot
* which could be useful in cases where we want to compare the old snapshot
* (snapshot without the ops applied) and the new snapshot (with the ops applied)
*/
if (request.preservePreapplySnapshot) {
request.preapplySnapshot = util.clone(request.snapshot);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I see this written down, is this any more advantageous than consumers just doing this themselves in middleware directly?

With this approach:

backend.use('apply', (request, next) => {
  request.preservePreapplySnapshot = true;
});

Or we can just do this with no changes to ShareDB at all:

backend.use('apply', (request, next) => {
  request.preapplySnapshot = clone(request.snapshot);
});

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah actaully you are right, but there is an argument to be made that it should be part of sharedb itself for simplicity sake, but I am fine to just close this PR and just add some example of this use case in the docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We decided to just add the documentation about this use case

}

// Apply the submitted op to the snapshot
err = ot.apply(request.snapshot, request.op);
if (err) return callback(err);
Expand Down
165 changes: 165 additions & 0 deletions test/middleware.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -773,4 +773,169 @@ describe('middleware', function() {
});
});
});

describe('preservePreapplySnapshot', function() {
var connection;
var backend;
var doc;

beforeEach(function() {
backend = this.backend;
connection = backend.connect();
doc = connection.get('dogs', 'fido');
});

describe('preservePreapplySnapshot = true', function() {
beforeEach(function() {
backend.use('apply', function(request, next) {
request.preservePreapplySnapshot = true;
next();
});
});

describe('create', function() {
it('has version equal to 0 and data undefined', function(done) {
backend.use('commit', function(request, next) {
expect(request.preapplySnapshot).to.not.equal(request.snapshot);

expect(request.preapplySnapshot.v).to.equal(0);
expect(request.preapplySnapshot.type).to.be.null;
expect(request.preapplySnapshot.data).to.be.undefined;

expect(request.snapshot.v).to.equal(1);
expect(request.snapshot.type).to.be.ok;
expect(request.snapshot.data).to.be.deep.equal({name: 'fido'});

next();
});

doc.create({name: 'fido'}, done);
});
});

describe('op', function() {
it('has version and data before the apply', function(done) {
backend.use('commit', function(request, next) {
if (request.op.create) return next();
expect(request.preapplySnapshot).to.not.equal(request.snapshot);

expect(request.preapplySnapshot.v).to.equal(1);
expect(request.preapplySnapshot.type).to.be.ok;
expect(request.preapplySnapshot.data).to.be.deep.equal({name: 'fido'});

expect(request.snapshot.v).to.equal(2);
expect(request.preapplySnapshot.type).to.be.ok;
expect(request.snapshot.data).to.be.deep.equal({name: 'bfooar'});

next();
});

doc.create({name: 'fido'}, function(error) {
if (error) return done(error);

doc.submitOp([
{
p: ['name'],
oi: 'bar'
},
{
p: ['name', 1],
si: 'foo'
}
], done);
});
});

it('has version and data before the apply with $fixOps set', function(done) {
backend.use('apply', function(request, next) {
if (request.op.create) return next();
request.$fixup([{p: ['tricks', 1], li: 'stay'}]);
next();
});

backend.use('commit', function(request, next) {
if (request.op.create) return next();

expect(request.preapplySnapshot).to.not.equal(request.snapshot);

expect(request.preapplySnapshot.v).to.equal(1);
expect(request.preapplySnapshot.type).to.be.ok;
expect(request.preapplySnapshot.data).to.be.deep.equal({name: 'fido'});

expect(request.snapshot.v).to.equal(2);
expect(request.preapplySnapshot.type).to.be.ok;
expect(request.snapshot.data).to.be.deep.equal({
name: 'fido',
tricks: ['fetch', 'stay']
});

next();
});

doc.create({name: 'fido'}, function(error) {
if (error) return done(error);

doc.submitOp([{p: ['tricks'], oi: ['fetch']}], done);
});
});
});

describe('del', function() {
it('has version and data before the apply', function(done) {
backend.use('commit', function(request, next) {
if (request.op.create) return next();
expect(request.preapplySnapshot).to.not.equal(request.snapshot);

expect(request.preapplySnapshot.v).to.equal(1);
expect(request.preapplySnapshot.type).to.be.ok;
expect(request.preapplySnapshot.data).to.be.deep.equal({name: 'fido'});

expect(request.snapshot.v).to.equal(2);
expect(request.snapshot.type).to.be.null;
expect(request.snapshot.data).to.be.undefined;

next();
});

doc.create({name: 'fido'}, function(error) {
if (error) return done(error);

doc.del(done);
});
});
});
});

describe('preservePreapplySnapshot = false (default)', function() {
it('does\'t store the preapplySnapshot', function(done) {
backend.use('commit', function(request, next) {
expect(request.preapplySnapshot).to.be.null;
next();
});

doc.create({name: 'fido'}, function(error) {
if (error) return done(error);

process.nextTick(function() {
doc.submitOp([
{
p: ['name'],
oi: 'bar'
},
{
p: ['name', 1],
si: 'foo'
}
], function(error) {
if (error) return done(error);

process.nextTick(function() {
doc.del(done);
});
});
});
});
});
});
});
});
Loading