ueberdosis / hocuspocus

The CRDT Yjs WebSocket backend for conflict-free real-time collaboration in your app.
https://tiptap.dev/docs/hocuspocus/introduction
MIT License
1.31k stars 126 forks source link

Provide access to the client connection instance in all lifecycle hooks #125

Open jamesopti opened 3 years ago

jamesopti commented 3 years ago

The problem I am facing I'm unable to set a client to read only in methods other than onConnect

The solution I would like I'd like to have access to the client connection instance in all of the lifecycle hooks.

Alternatives I have considered I've implemented a workaround that involves passing a getter to the entire Hocuspocus instance to my extension in order to iterate through the connections:

const document = this.getInstance().documents.get(documentName)
document.connections.forEach(({ connection }) => {
  const thisClientsVersion = connection.context.SCHEMA_VERSION
  if (!thisClientsVersion || thisClientsVersion < requiredVersion) {
    this.logger.warn({
      label: `${MODULE_NAME}.setOutdatedClientsToReadOnly`,
      message: `(${connection.socketId}) Client schema is out of date (v${thisClientsVersion}). Setting it to read only`,
    })
    connection.readOnly = true
  }
})

Additional context I need to be able to set some connections to read only when certain events happen.

hanspagel commented 3 years ago

In which hook you'd like to set the connection to readOnly? I’d assume in the onChange hook?

Would you mind to elaborate on the use case? I think once someone is able to write to the document, it seems frustrating to ignore changes later (after the users was idle or something).

And would you make it writeable again at some point? That would mean we’d need to trigger another hook for blocked changes.

jamesopti commented 3 years ago

Ah, I should have explained!

We've implemented a schema versioning system. Anytime new node types are added or existing ones modified, we bump a schema version in the code. Any client that connects to Hocuspocus passes its schema version via a query param.

If a user on schema version 1 is connected and editing the doc, then a user with schema version 2 connects, the user on schema version 1 must be put in read only mode and asked to reload, as their client may not be able to understand the content of the newer schema.

Another simpler use case is that a client's access level changes. If the user keeps a long lived connection to a document, their permissions could change and either require that their connection is set to read only or closed.

hanspagel commented 3 years ago

Ah, got it! That’s a great use case.

Out of interest: How does the client get notified that the schema is outdated? The schema version is stored in the Y.js document?

hanspagel commented 3 years ago

What if you’d have access to other connections in the onConnect hook? So you could write something like that:

async onConnect({ requestParameters, connection }) {
  // All connections to this document …
  connections
    // … with an older version …
    .filter(connection => requestParameters.version > connection.requestParameters.version)
    // … should be closed …
    .forEach(connection => {
      // … with a proper code and message.
      connection.close(Outdated.code, Outdated.reason)
    })
}
hanspagel commented 3 years ago

@jamesopti What do you think? Would that help? 🙃

jamesopti commented 3 years ago

Out of interest: How does the client get notified that the schema is outdated? The schema version is stored in the Y.js document?

Yup, exactly. We set a property on the doc like this:

async onCreateDocument({ document, documentName }) {
  document.getMap(this.configuration.metaDataKey).set('READY', false)
  await this.load(documentName)
  // Apply updates
  ...
  document.getMap(this.configuration.metaDataKey).set('READY', true)
}

@jamesopti What do you think? Would that help? 🙃

So this would partially solve our problem (the piece where the document is already loaded into memory).

But we also need to handle the case where the document has not been loaded, for example:

Perhaps onCreateDocument also needs to know the connection which triggered it to fire?

tommoor commented 3 years ago

I think it's worth considering how this versioning usecase could be a first class citizen of Hocuspocus. The other implementation of yjs that I've taken to production had to have exactly the same considerations, because the way that y-prosemirror is written if a node isn't understood it's just filtered out (deleted).

https://github.com/yjs/y-prosemirror/blob/master/src/plugins/sync-plugin.js#L448-L455

This means you absolutely cannot have old client versions connected or you'll lose content from newer clients as soon as they load it 🙈