diff --git a/src/node/db/API.js b/src/node/db/API.js index f119f584..d4886755 100644 --- a/src/node/db/API.js +++ b/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); }; /* *************** diff --git a/src/node/db/Pad.js b/src/node/db/Pad.js index adf98272..36585426 100644 --- a/src/node/db/Pad.js +++ b/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}`); - - // get the authorName if the entry exists - if (entry != null) { - entry.userName = await authorManager.getAuthorName(entry.userId); - } - - return entry; + if (entry == null) return null; + const message = ChatMessage.fromObject(entry); + message.userName = await authorManager.getAuthorName(message.userId); + return message; }; /** * @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. */ diff --git a/src/node/handler/PadMessageHandler.js b/src/node/handler/PadMessageHandler.js index e38d7716..36682abe 100644 --- a/src/node/handler/PadMessageHandler.js +++ b/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 text = message.data.text; + const chatMessage = ChatMessage.fromObject(message.data.message); 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 - * @param time the timestamp of the chat message - * @param userId the author id of the chat message - * @param text the text of the chat message - * @param padId the padId to send the chat message to + * Adds a new chat message to a pad and sends it to connected clients. + * + * @param {(ChatMessage|number)} mt - Either a chat message object (recommended) or the timestamp of + * the chat message in ms since epoch (deprecated). + * @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) => { - // get the pad +exports.sendChatMessageToPadClients = async (mt, puId, text = null, padId = null) => { + const message = mt instanceof ChatMessage ? mt : new ChatMessage(text, puId, mt); + padId = mt instanceof ChatMessage ? puId : padId; const pad = await padManager.getPad(padId); - - // get the author - const userName = await authorManager.getAuthorName(userId); - - // save the chat message - const promise = pad.appendChatMessage(text, userId, time); - - const msg = { + // pad.appendChatMessage() ignores the userName property so we don't need to wait for + // authorManager.getAuthorName() to resolve before saving the message to the database. + const promise = pad.appendChatMessage(message); + message.userName = await authorManager.getAuthorName(message.userId); + socketio.sockets.in(padId).json.send({ type: 'COLLABROOM', - data: {type: 'CHAT_MESSAGE', userId, userName, time, text}, - }; - - // broadcast the chat message to everyone on the pad - socketio.sockets.in(padId).json.send(msg); - + data: {type: 'CHAT_MESSAGE', message}, + }); await promise; }; diff --git a/src/node/utils/tar.json b/src/node/utils/tar.json index e6caa3e6..896913ff 100644 --- a/src/node/utils/tar.json +++ b/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" diff --git a/src/static/js/ChatMessage.js b/src/static/js/ChatMessage.js new file mode 100644 index 00000000..f6e10c2b --- /dev/null +++ b/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; diff --git a/src/static/js/chat.js b/src/static/js/chat.js index ef150065..62f35549 100755 --- a/src/static/js/chat.js +++ b/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; diff --git a/src/static/js/collab_client.js b/src/static/js/collab_client.js index e507d6b0..837fc3ba 100644 --- a/src/static/js/collab_client.js +++ b/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); diff --git a/src/tests/frontend/helper/methods.js b/src/tests/frontend/helper/methods.js index c6b9c6a1..b828cd60 100644 --- a/src/tests/frontend/helper/methods.js +++ b/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); } diff --git a/src/tests/frontend/specs/chat_hooks.js b/src/tests/frontend/specs/chat_hooks.js index eb305e40..a307ad5b 100644 --- a/src/tests/frontend/specs/chat_hooks.js +++ b/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}`), ]);