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.

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

Conversation

dawidreedsy
Copy link
Contributor

Occasionally, it is necessary to reference the previous version of a snapshot to execute auxiliary operations within the commit middleware, for example:

Let's imagine we have this doc that describes user:

{
  id: '123',
  emailConfirmed: false;
}

Now we would want to send notification to the admin whenever emailConfirmed changed from false to true

At the moment there is no simple way to check in the middlewares if something is changing, apart form trying to apply the ops manually in the apply middleware. Something like

backend.use('apply', function(request, next) {
  const changedSnapshot = applyOps(clone(request.snapshot), request.op);
  if(
    request.snapshot.data.emailConfirmed === false &&
    request.snapshot.data.emailConfirmed === true
  ) {
    notifyAdmin();
  }
})

This approach has few drawbacks:

  1. We need to apply ops twice once in this middleware and once in the sharedb itself.
  2. It is quite difficult for app to make considerations and mimick the flow of $fixOps

Solution

Let's make it possible to opt-in for storing the pre-apply snapshot version in request by setting a preservePreapplySnapshot flag to true in any middleware that happens before apply. This will allow us to:

  1. Have the old and new version of snapshot available in the commit middleware
  2. We do not have much memory overhead as this is opt-in, so if user never needs it the snapshot would never be cloned.
  3. We only need to apply ops once
  4. The apply mechanism is using shareDb power house.

…in commit middleware

Occasionally, it is necessary to reference the previous version of a snapshot to execute auxiliary operations within the commit middleware, for example:

Let's imagine we have this doc that describes user:
```typescript
{
  id: '123',
  emailConfirmed: false;
}
```
Now we would want to send notification to the admin whenever `emailConfirmed` changed from `false` to `true`

At the moment there is no simple way to check in the middlewares if something is changing, apart form trying to apply the ops manually in the `apply` middleware. Something like

```typescript
backend.use('apply', function(request, next) {
  const changedSnapshot = applyOps(clone(request.snapshot), request.op);
  if(
    request.snapshot.data.emailConfirmed === false &&
    request.snapshot.data.emailConfirmed === true
  ) {
    notifyAdmin();
  }
})
```

This approach has few drawbacks:
1. We need to apply ops twice once in this middleware and once in the sharedb itself.
2. It is quite difficult for app to make considerations and mimick the flow of `$fixOps`

Solution
---
Let's make it possible to opt-in for storing the pre-apply snapshot version in request by setting a `preservePreapplySnapshot` flag to `true` in any middleware that happens before apply. This will allow us to:
1. Have the old and new version of snapshot available in the `commit` middleware
2. We do not have much memory overhead as this is opt-in, so if user never needs it the snapshot would never be cloned.
3. We only need to apply ops once
4. The apply mechanism is using shareDb power house.
@dawidreedsy dawidreedsy marked this pull request as draft June 26, 2024 10:11
@dawidreedsy dawidreedsy marked this pull request as ready for review June 26, 2024 10:12
@coveralls
Copy link

Coverage Status

coverage: 97.448% (+0.003%) from 97.445%
when pulling 6490158 on add-preapply-snapshot
into 10ba551 on master.

* (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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants