fix: Multiplayer changes attributed to incorrect user (#4282)

* fix: Multiplayer changes attributed to the wrong user, performance improvements

* fix: Actually use _last_ editor

server/collaboration/LoggerExtension.ts

@@ -8,17 +8,17 @@ import Logger from "@server/logging/Logger";
 
 export default class LoggerExtension implements Extension {
   async onLoadDocument(data: onLoadDocumentPayload) {
-    Logger.info("hocuspocus", `Loaded document "${data.documentName}"`, {
+    Logger.info("multiplayer", `Loaded document "${data.documentName}"`, {
       userId: data.context.user?.id,
     });
   }
 
   async onConnect(data: onConnectPayload) {
-    Logger.info("hocuspocus", `New connection to "${data.documentName}"`);
+    Logger.info("multiplayer", `New connection to "${data.documentName}"`);
   }
 
   async onDisconnect(data: onDisconnectPayload) {
-    Logger.info("hocuspocus", `Closed connection to "${data.documentName}"`, {
+    Logger.info("multiplayer", `Closed connection to "${data.documentName}"`, {
       userId: data.context.user?.id,
     });
   }

server/collaboration/PersistenceExtension.ts

@@ -1,6 +1,7 @@
 import {
   onStoreDocumentPayload,
   onLoadDocumentPayload,
+  onChangePayload,
   Extension,
 } from "@hocuspocus/server";
 import * as Y from "yjs";
@@ -15,6 +16,12 @@ import markdownToYDoc from "./utils/markdownToYDoc";
   spanName: "persistence",
 })
 export default class PersistenceExtension implements Extension {
+  /**
+   * Map of documentId -> userIds that have modified the document since it
+   * was last persisted to the database. The map is cleared on every save.
+   */
+  documentCollaboratorIds = new Map<string, Set<string>>();
+
   async onLoadDocument({ documentName, ...data }: onLoadDocumentPayload) {
     const [, documentId] = documentName.split(".");
     const fieldName = "default";
@@ -62,19 +69,41 @@ export default class PersistenceExtension implements Extension {
     });
   }
 
+  async onChange({ context, documentName }: onChangePayload) {
+    Logger.debug(
+      "multiplayer",
+      `${context.user?.name} changed ${documentName}`
+    );
+
+    const state = this.documentCollaboratorIds.get(documentName) ?? new Set();
+    state.add(context.user?.id);
+    this.documentCollaboratorIds.set(documentName, state);
+  }
+
   async onStoreDocument({
     document,
     context,
     documentName,
   }: onStoreDocumentPayload) {
     const [, documentId] = documentName.split(".");
-    Logger.info("database", `Persisting ${documentId}`);
+
+    // Find the collaborators that have modified the document since it was last
+    // persisted and clear the map.
+    const documentCollaboratorIds = this.documentCollaboratorIds.get(
+      documentName
+    );
+    const collaboratorIds = documentCollaboratorIds
+      ? Array.from(documentCollaboratorIds.values())
+      : [context.user?.id];
+    this.documentCollaboratorIds.delete(documentName);
 
     try {
       await documentCollaborativeUpdater({
         documentId,
         ydoc: document,
-        userId: context.user?.id,
+        // TODO: Right now we're attributing all changes to the last editor,
+        // It would be nice in the future to have multiple editors per revision.
+        userId: collaboratorIds.pop(),
       });
     } catch (err) {
       Logger.error("Unable to persist document", err, {

server/commands/documentCollaborativeUpdater.ts

@@ -4,6 +4,7 @@ import { Node } from "prosemirror-model";
 import * as Y from "yjs";
 import { sequelize } from "@server/database/sequelize";
 import { schema, serializer } from "@server/editor";
+import Logger from "@server/logging/Logger";
 import { Document, Event } from "@server/models";
 
 export default async function documentCollaborativeUpdater({
@@ -18,7 +19,10 @@ export default async function documentCollaborativeUpdater({
   return sequelize.transaction(async (transaction) => {
     const document = await Document.unscoped()
       .scope("withState")
-      .findByPk(documentId, {
+      .findOne({
+        where: {
+          id: documentId,
+        },
         transaction,
         lock: {
           of: Document,
@@ -38,6 +42,11 @@ export default async function documentCollaborativeUpdater({
       return;
     }
 
+    Logger.info(
+      "multiplayer",
+      `Persisting ${documentId}, attributed to ${userId}`
+    );
+
     // extract collaborators from doc user data
     const pud = new Y.PermanentUserData(ydoc);
     const pudIds = Array.from(pud.clients.values());

server/commands/documentUpdater.ts

@@ -112,7 +112,7 @@ export default async function documentUpdater({
   }
 
   if (document.title !== previousTitle) {
-    Event.schedule({
+    await Event.schedule({
       name: "documents.title_change",
       documentId: document.id,
       collectionId: document.collectionId,

server/logging/Logger.ts

@@ -11,7 +11,7 @@ const isProduction = env.ENVIRONMENT === "production";
 
 type LogCategory =
   | "lifecycle"
-  | "hocuspocus"
+  | "multiplayer"
   | "http"
   | "commands"
   | "worker"

server/models/Event.ts

@@ -103,7 +103,7 @@ class Event extends IdModel {
    */
   static schedule(event: Partial<Event>) {
     const now = new Date();
-    globalEventQueue.add(
+    return globalEventQueue.add(
       this.build({
         createdAt: now,
         ...event,