chat: Plumb message object end to end

This will make it possible for future commits to add hooks that allow
plugins to augment chat messages with arbitrary metadata.

src/node/db/API.js

@@ -20,6 +20,7 @@
  */
 
 const Changeset = require('../../static/js/Changeset');
+const ChatMessage = require('../../static/js/ChatMessage');
 const CustomError = require('../utils/customError');
 const padManager = require('./PadManager');
 const padMessageHandler = require('../handler/PadMessageHandler');
@@ -364,7 +365,7 @@ exports.appendChatMessage = async (padID, text, authorID, time) => {
   // @TODO - missing getPadSafe() call ?
 
   // save chat message to database and send message to all connected clients
-  await padMessageHandler.sendChatMessageToPadClients(time, authorID, text, padID);
+  await padMessageHandler.sendChatMessageToPadClients(new ChatMessage(text, authorID, time), padID);
 };
 
 /* ***************

src/node/db/Pad.js

@@ -5,6 +5,7 @@
 
 
 const Changeset = require('../../static/js/Changeset');
+const ChatMessage = require('../../static/js/ChatMessage');
 const AttributePool = require('../../static/js/AttributePool');
 const db = require('./DB');
 const settings = require('../utils/Settings');
@@ -274,31 +275,44 @@ Pad.prototype.appendText = async function (newText) {
   await this.appendRevision(changeset);
 };
 
-Pad.prototype.appendChatMessage = async function (text, userId, time) {
+/**
+ * Adds a chat message to the pad, including saving it to the database.
+ *
+ * @param {(ChatMessage|string)} msgOrText - Either a chat message object (recommended) or a string
+ *     containing the raw text of the user's chat message (deprecated).
+ * @param {?string} [userId] - The user's author ID. Deprecated; use `msgOrText.userId` instead.
+ * @param {?number} [time] - Message timestamp (milliseconds since epoch). Deprecated; use
+ *     `msgOrText.time` instead.
+ */
+Pad.prototype.appendChatMessage = async function (msgOrText, userId = null, time = null) {
+  const msg =
+      msgOrText instanceof ChatMessage ? msgOrText : new ChatMessage(msgOrText, userId, time);
   this.chatHead++;
-  // save the chat entry in the database
   await Promise.all([
-    db.set(`pad:${this.id}:chat:${this.chatHead}`, {text, userId, time}),
+    // Don't save the display name in the database because the user can change it at any time. The
+    // `userName` property will be populated with the current value when the message is read from
+    // the database.
+    db.set(`pad:${this.id}:chat:${this.chatHead}`, {...msg, userName: undefined}),
     this.saveToDatabase(),
   ]);
 };
 
+/**
+ * @param {number} entryNum - ID of the desired chat message.
+ * @returns {?ChatMessage}
+ */
 Pad.prototype.getChatMessage = async function (entryNum) {
-  // get the chat entry
   const entry = await db.get(`pad:${this.id}:chat:${entryNum}`);
-
+  if (entry == null) return null;
-  // get the authorName if the entry exists
+  const message = ChatMessage.fromObject(entry);
-  if (entry != null) {
+  message.userName = await authorManager.getAuthorName(message.userId);
-    entry.userName = await authorManager.getAuthorName(entry.userId);
+  return message;
-  }
-
-  return entry;
 };
 
 /**
  * @param {number} start - ID of the first desired chat message.
  * @param {number} end - ID of the last desired chat message.
- * @returns {object[]} Any existing messages with IDs between `start` (inclusive) and `end`
+ * @returns {ChatMessage[]} Any existing messages with IDs between `start` (inclusive) and `end`
  *     (inclusive), in order. Note: `start` and `end` form a closed interval, not a half-open
  *     interval as is typical in code.
  */

src/node/handler/PadMessageHandler.js

@@ -21,6 +21,7 @@
 
 const padManager = require('../db/PadManager');
 const Changeset = require('../../static/js/Changeset');
+const ChatMessage = require('../../static/js/ChatMessage');
 const AttributePool = require('../../static/js/AttributePool');
 const AttributeManager = require('../../static/js/AttributeManager');
 const authorManager = require('../db/AuthorManager');
@@ -340,37 +341,37 @@ exports.handleCustomMessage = (padID, msgString) => {
  * @param message the message from the client
  */
 const handleChatMessage = async (socket, message) => {
-  const time = Date.now();
+  const chatMessage = ChatMessage.fromObject(message.data.message);
-  const text = message.data.text;
   const {padId, author: authorId} = sessioninfos[socket.id];
-  await exports.sendChatMessageToPadClients(time, authorId, text, padId);
+  // Don't trust the user-supplied values.
+  chatMessage.time = Date.now();
+  chatMessage.userId = authorId;
+  await exports.sendChatMessageToPadClients(chatMessage, padId);
 };
 
 /**
- * Sends a chat message to all clients of this pad
+ * Adds a new chat message to a pad and sends it to connected clients.
- * @param time the timestamp of the chat message
+ *
- * @param userId the author id of the chat message
+ * @param {(ChatMessage|number)} mt - Either a chat message object (recommended) or the timestamp of
- * @param text the text of the chat message
+ *     the chat message in ms since epoch (deprecated).
- * @param padId the padId to send the chat message to
+ * @param {string} puId - If `mt` is a chat message object, this is the destination pad ID.
+ *     Otherwise, this is the user's author ID (deprecated).
+ * @param {string} [text] - The text of the chat message. Deprecated; use `mt.text` instead.
+ * @param {string} [padId] - The destination pad ID. Deprecated; pass a chat message
+ *     object as the first argument and the destination pad ID as the second argument instead.
  */
-exports.sendChatMessageToPadClients = async (time, userId, text, padId) => {
+exports.sendChatMessageToPadClients = async (mt, puId, text = null, padId = null) => {
-  // get the pad
+  const message = mt instanceof ChatMessage ? mt : new ChatMessage(text, puId, mt);
+  padId = mt instanceof ChatMessage ? puId : padId;
   const pad = await padManager.getPad(padId);
-
+  // pad.appendChatMessage() ignores the userName property so we don't need to wait for
-  // get the author
+  // authorManager.getAuthorName() to resolve before saving the message to the database.
-  const userName = await authorManager.getAuthorName(userId);
+  const promise = pad.appendChatMessage(message);
-
+  message.userName = await authorManager.getAuthorName(message.userId);
-  // save the chat message
+  socketio.sockets.in(padId).json.send({
-  const promise = pad.appendChatMessage(text, userId, time);
-
-  const msg = {
     type: 'COLLABROOM',
-    data: {type: 'CHAT_MESSAGE', userId, userName, time, text},
+    data: {type: 'CHAT_MESSAGE', message},
-  };
+  });
-
-  // broadcast the chat message to everyone on the pad
-  socketio.sockets.in(padId).json.send(msg);
-
   await promise;
 };
 

src/node/utils/tar.json

@@ -19,6 +19,7 @@
   , "pad_impexp.js"
   , "pad_savedrevs.js"
   , "pad_connectionstatus.js"
+  , "ChatMessage.js"
   , "chat.js"
   , "vendors/gritter.js"
   , "$js-cookie/dist/js.cookie.js"

src/static/js/ChatMessage.js

@@ -0,0 +1,50 @@
+'use strict';
+
+/**
+ * Represents a chat message stored in the database and transmitted among users. Plugins can extend
+ * the object with additional properties.
+ *
+ * Supports serialization to JSON.
+ */
+class ChatMessage {
+  static fromObject(obj) {
+    return Object.assign(new ChatMessage(), obj);
+  }
+
+  /**
+   * @param {?string} [text] - Initial value of the `text` property.
+   * @param {?string} [userId] - Initial value of the `userId` property.
+   * @param {?number} [time] - Initial value of the `time` property.
+   */
+  constructor(text = null, userId = null, time = null) {
+    /**
+     * The raw text of the user's chat message (before any rendering or processing).
+     *
+     * @type {?string}
+     */
+    this.text = text;
+
+    /**
+     * The user's author ID.
+     *
+     * @type {?string}
+     */
+    this.userId = userId;
+
+    /**
+     * The message's timestamp, as milliseconds since epoch.
+     *
+     * @type {?number}
+     */
+    this.time = time;
+
+    /**
+     * The user's display name.
+     *
+     * @type {?string}
+     */
+    this.userName = null;
+  }
+}
+
+module.exports = ChatMessage;

src/static/js/chat.js

@@ -15,6 +15,7 @@
  * limitations under the License.
  */
 
+const ChatMessage = require('./ChatMessage');
 const padutils = require('./pad_utils').padutils;
 const padcookie = require('./pad_cookie').padcookie;
 const Tinycon = require('tinycon/tinycon');
@@ -102,10 +103,11 @@ exports.chat = (() => {
     send() {
       const text = $('#chatinput').val();
       if (text.replace(/\s+/, '').length === 0) return;
-      this._pad.collabClient.sendMessage({type: 'CHAT_MESSAGE', text});
+      this._pad.collabClient.sendMessage({type: 'CHAT_MESSAGE', message: new ChatMessage(text)});
       $('#chatinput').val('');
     },
     async addMessage(msg, increment, isHistoryAdd) {
+      msg = ChatMessage.fromObject(msg);
       // correct the time
       msg.time += this._pad.clientTimeOffset;
 

src/static/js/collab_client.js

@@ -272,7 +272,7 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad)
     } else if (msg.type === 'CLIENT_MESSAGE') {
       callbacks.onClientMessage(msg.payload);
     } else if (msg.type === 'CHAT_MESSAGE') {
-      chat.addMessage(msg, true, false);
+      chat.addMessage(msg.message, true, false);
     } else if (msg.type === 'CHAT_MESSAGES') {
       for (let i = msg.messages.length - 1; i >= 0; i--) {
         chat.addMessage(msg.messages[i], true, true);

src/tests/frontend/helper/methods.js

@@ -12,7 +12,7 @@ helper.spyOnSocketIO = () => {
     } else if (msg.data.type === 'USER_NEWINFO') {
       helper.userInfos.push(msg);
     } else if (msg.data.type === 'CHAT_MESSAGE') {
-      helper.chatMessages.push(msg.data);
+      helper.chatMessages.push(msg.data.message);
     } else if (msg.data.type === 'CHAT_MESSAGES') {
       helper.chatMessages.push(...msg.data.messages);
     }

src/tests/frontend/specs/chat_hooks.js

@@ -1,11 +1,13 @@
 'use strict';
 
 describe('chat hooks', function () {
+  let ChatMessage;
   let hooks;
   const hooksBackup = {};
 
   const loadPad = async (opts = {}) => {
     await helper.aNewPad(opts);
+    ChatMessage = helper.padChrome$.window.require('ep_etherpad-lite/static/js/ChatMessage');
     ({hooks} = helper.padChrome$.window.require('ep_etherpad-lite/static/js/pluginfw/plugin_defs'));
     for (const [name, defs] of Object.entries(hooks)) {
       hooksBackup[name] = defs;
@@ -61,10 +63,10 @@ describe('chat hooks', function () {
       });
     }
 
-    it('message is an object', async function () {
+    it('message is a ChatMessage object', async function () {
       await Promise.all([
         checkHook('chatNewMessage', ({message}) => {
-          expect(message).to.be.an('object');
+          expect(message).to.be.a(ChatMessage);
         }),
         helper.sendChatMessage(`${this.test.title}{enter}`),
       ]);