From bf0691f9bb4a43b39db7c96d451e7d9218a5dc43 Mon Sep 17 00:00:00 2001 From: Cadence Ember Date: Tue, 10 Oct 2023 14:03:53 +1300 Subject: [PATCH] Refactor reaction removals and add tests --- d2m/actions/remove-reaction.js | 95 +++++--------- d2m/converters/remove-reaction.js | 88 +++++++++++++ d2m/converters/remove-reaction.test.js | 172 +++++++++++++++++++++++++ d2m/discord-packets.js | 10 +- d2m/event-dispatcher.js | 49 +++---- test/test.js | 6 +- 6 files changed, 318 insertions(+), 102 deletions(-) create mode 100644 d2m/converters/remove-reaction.js create mode 100644 d2m/converters/remove-reaction.test.js diff --git a/d2m/actions/remove-reaction.js b/d2m/actions/remove-reaction.js index eec93a4..3047c32 100644 --- a/d2m/actions/remove-reaction.js +++ b/d2m/actions/remove-reaction.js @@ -1,7 +1,7 @@ // @ts-check const Ty = require("../../types") -const assert = require("assert").strict +const DiscordTypes = require("discord-api-types/v10") const passthrough = require("../../passthrough") const {discord, sync, db, select} = passthrough @@ -9,15 +9,15 @@ const {discord, sync, db, select} = passthrough const api = sync.require("../../matrix/api") /** @type {import("../converters/emoji-to-key")} */ const emojiToKey = sync.require("../converters/emoji-to-key") -/** @type {import("../../m2d/converters/utils")} */ -const utils = sync.require("../../m2d/converters/utils") /** @type {import("../../m2d/converters/emoji")} */ const emoji = sync.require("../../m2d/converters/emoji") +/** @type {import("../converters/remove-reaction")} */ +const converter = sync.require("../converters/remove-reaction") /** - * @param {import("discord-api-types/v10").GatewayMessageReactionRemoveDispatchData} data + * @param {DiscordTypes.GatewayMessageReactionRemoveDispatchData | DiscordTypes.GatewayMessageReactionRemoveEmojiDispatchData | DiscordTypes.GatewayMessageReactionRemoveAllDispatchData} data */ -async function removeReaction(data) { +async function removeSomeReactions(data) { const roomID = select("channel_room", "room_id", {channel_id: data.channel_id}).pluck().get() if (!roomID) return const eventIDForMessage = select("event_message", "event_id", {message_id: data.message_id, part: 0}).pluck().get() @@ -25,76 +25,49 @@ async function removeReaction(data) { /** @type {Ty.Pagination>} */ const relations = await api.getRelations(roomID, eventIDForMessage, "m.annotation") - const key = await emojiToKey.emojiToKey(data.emoji) - const wantToRemoveMatrixReaction = data.user_id === discord.application.id - for (const event of relations.chunk) { - if (event.content["m.relates_to"].key === key) { - const lookingAtMatrixReaction = !utils.eventSenderIsFromDiscord(event.sender) - if (lookingAtMatrixReaction && wantToRemoveMatrixReaction) { - // We are removing a Matrix user's reaction, so we need to redact from the correct user ID (not @_ooye_matrix_bridge). - // Even though the bridge bot only reacted once on Discord-side, multiple Matrix users may have - // reacted on Matrix-side. Semantically, we want to remove the reaction from EVERY Matrix user. - await api.redactEvent(roomID, event.event_id) - // Clean up the database - const hash = utils.getEventIDHash(event.event_id) - db.prepare("DELETE FROM reaction WHERE hashed_event_id = ?").run(hash) - } - if (!lookingAtMatrixReaction && !wantToRemoveMatrixReaction) { - // We are removing a Discord user's reaction, so we just make the sim user remove it. - const mxid = select("sim", "mxid", {user_id: data.user_id}).pluck().get() - if (mxid === event.sender) { - await api.redactEvent(roomID, event.event_id, mxid) - } - } - } + // Run the proper strategy and any strategy-specific database changes + const removals = await + ( "user_id" in data ? removeReaction(data, relations) + : "emoji" in data ? removeEmojiReaction(data, relations) + : removeAllReactions(data, relations)) + + // Redact the events and delete individual stored events in the database + for (const removal of removals) { + await api.redactEvent(roomID, removal.eventID, removal.mxid) + if (removal.hash) db.prepare("DELETE FROM reaction WHERE hashed_event_id = ?").run(removal.hash) } } /** - * @param {import("discord-api-types/v10").GatewayMessageReactionRemoveEmojiDispatchData} data + * @param {DiscordTypes.GatewayMessageReactionRemoveDispatchData} data + * @param {Ty.Pagination>} relations */ -async function removeEmojiReaction(data) { - const roomID = select("channel_room", "room_id", {channel_id: data.channel_id}).pluck().get() - if (!roomID) return - const eventIDForMessage = select("event_message", "event_id", {message_id: data.message_id, part: 0}).pluck().get() - if (!eventIDForMessage) return - - /** @type {Ty.Pagination>} */ - const relations = await api.getRelations(roomID, eventIDForMessage, "m.annotation") +async function removeReaction(data, relations) { const key = await emojiToKey.emojiToKey(data.emoji) + return converter.removeReaction(data, relations, key) +} - for (const event of relations.chunk) { - if (event.content["m.relates_to"].key === key) { - const mxid = utils.eventSenderIsFromDiscord(event.sender) ? event.sender : undefined - await api.redactEvent(roomID, event.event_id, mxid) - } - } - +/** + * @param {DiscordTypes.GatewayMessageReactionRemoveEmojiDispatchData} data + * @param {Ty.Pagination>} relations + */ +async function removeEmojiReaction(data, relations) { + const key = await emojiToKey.emojiToKey(data.emoji) const discordPreferredEncoding = emoji.encodeEmoji(key, undefined) db.prepare("DELETE FROM reaction WHERE message_id = ? AND encoded_emoji = ?").run(data.message_id, discordPreferredEncoding) + + return converter.removeEmojiReaction(data, relations, key) } /** - * @param {import("discord-api-types/v10").GatewayMessageReactionRemoveAllDispatchData} data + * @param {DiscordTypes.GatewayMessageReactionRemoveAllDispatchData} data + * @param {Ty.Pagination>} relations */ -async function removeAllReactions(data) { - const roomID = select("channel_room", "room_id", {channel_id: data.channel_id}).pluck().get() - if (!roomID) return - const eventIDForMessage = select("event_message", "event_id", {message_id: data.message_id, part: 0}).pluck().get() - if (!eventIDForMessage) return - - /** @type {Ty.Pagination>} */ - const relations = await api.getRelations(roomID, eventIDForMessage, "m.annotation") - - for (const event of relations.chunk) { - const mxid = utils.eventSenderIsFromDiscord(event.sender) ? event.sender : undefined - await api.redactEvent(roomID, event.event_id, mxid) - } - +async function removeAllReactions(data, relations) { db.prepare("DELETE FROM reaction WHERE message_id = ?").run(data.message_id) + + return converter.removeAllReactions(data, relations) } -module.exports.removeReaction = removeReaction -module.exports.removeEmojiReaction = removeEmojiReaction -module.exports.removeAllReactions = removeAllReactions +module.exports.removeSomeReactions = removeSomeReactions diff --git a/d2m/converters/remove-reaction.js b/d2m/converters/remove-reaction.js new file mode 100644 index 0000000..4fed269 --- /dev/null +++ b/d2m/converters/remove-reaction.js @@ -0,0 +1,88 @@ +// @ts-check + +const Ty = require("../../types") +const DiscordTypes = require("discord-api-types/v10") + +const passthrough = require("../../passthrough") +const {discord, sync, select} = passthrough +/** @type {import("../../m2d/converters/utils")} */ +const utils = sync.require("../../m2d/converters/utils") + +/** + * @typedef ReactionRemoveRequest + * @prop {string} eventID + * @prop {string | null} mxid + * @prop {BigInt} [hash] + */ + +/** + * @param {DiscordTypes.GatewayMessageReactionRemoveDispatchData} data + * @param {Ty.Pagination>} relations + * @param {string} key + */ +function removeReaction(data, relations, key) { + /** @type {ReactionRemoveRequest[]} */ + const removals = [] + + const wantToRemoveMatrixReaction = data.user_id === discord.application.id + for (const event of relations.chunk) { + const eventID = event.event_id + if (event.content["m.relates_to"].key === key) { + const lookingAtMatrixReaction = !utils.eventSenderIsFromDiscord(event.sender) + if (lookingAtMatrixReaction && wantToRemoveMatrixReaction) { + // We are removing a Matrix user's reaction, so we need to redact from the correct user ID (not @_ooye_matrix_bridge). + // Even though the bridge bot only reacted once on Discord-side, multiple Matrix users may have + // reacted on Matrix-side. Semantically, we want to remove the reaction from EVERY Matrix user. + // Also need to clean up the database. + const hash = utils.getEventIDHash(event.event_id) + removals.push({eventID, mxid: null, hash}) + } + if (!lookingAtMatrixReaction && !wantToRemoveMatrixReaction) { + // We are removing a Discord user's reaction, so we just make the sim user remove it. + const mxid = select("sim", "mxid", {user_id: data.user_id}).pluck().get() + if (mxid === event.sender) { + removals.push({eventID, mxid}) + } + } + } + } + + return removals +} + +/** + * @param {DiscordTypes.GatewayMessageReactionRemoveEmojiDispatchData} data + * @param {Ty.Pagination>} relations + * @param {string} key + */ +function removeEmojiReaction(data, relations, key) { + /** @type {ReactionRemoveRequest[]} */ + const removals = [] + + for (const event of relations.chunk) { + const eventID = event.event_id + if (event.content["m.relates_to"].key === key) { + const mxid = utils.eventSenderIsFromDiscord(event.sender) ? event.sender : null + removals.push({eventID, mxid}) + } + } + + return removals +} + +/** + * @param {DiscordTypes.GatewayMessageReactionRemoveAllDispatchData} data + * @param {Ty.Pagination>} relations + * @returns {ReactionRemoveRequest[]} + */ +function removeAllReactions(data, relations) { + return relations.chunk.map(event => { + const eventID = event.event_id + const mxid = utils.eventSenderIsFromDiscord(event.sender) ? event.sender : null + return {eventID, mxid} + }) +} + +module.exports.removeReaction = removeReaction +module.exports.removeEmojiReaction = removeEmojiReaction +module.exports.removeAllReactions = removeAllReactions diff --git a/d2m/converters/remove-reaction.test.js b/d2m/converters/remove-reaction.test.js new file mode 100644 index 0000000..a63721f --- /dev/null +++ b/d2m/converters/remove-reaction.test.js @@ -0,0 +1,172 @@ +// @ts-check + +const {test} = require("supertape") +const removeReaction = require("./remove-reaction") + +const BRIDGE_ID = "684280192553844747" + +function fakeSpecificReactionRemoval(userID, emoji, emojiID) { + return { + channel_id: "THE_CHANNEL", + message_id: "THE_MESSAGE", + user_id: userID, + emoji: {id: emojiID, name: emoji} + } +} + +function fakeEmojiReactionRemoval(emoji, emojiID) { + return { + channel_id: "THE_CHANNEL", + message_id: "THE_MESSAGE", + emoji: {id: emojiID, name: emoji} + } +} + +function fakeAllReactionRemoval() { + return { + channel_id: "THE_CHANNEL", + message_id: "THE_MESSAGE" + } +} + +function fakeChunk(chunk) { + return { + chunk: chunk.map(({sender, key}, i) => ({ + content: { + "m.relates_to": { + rel_type: "m.annotation", + event_id: "$message", + key + } + }, + event_id: `$reaction_${i}`, + sender, + type: "m.reaction", + origin_server_ts: 0, + room_id: "!THE_ROOM", + unsigned: null + })) + } +} + +test("remove reaction: a specific discord user's reaction is removed", t => { + const removals = removeReaction.removeReaction( + fakeSpecificReactionRemoval("820865262526005258", "🐈", null), + fakeChunk([{key: "🐈", sender: "@_ooye_crunch_god:cadence.moe"}]), + "🐈" + ) + t.deepEqual(removals, [{ + eventID: "$reaction_0", + mxid: "@_ooye_crunch_god:cadence.moe" + }]) +}) + +test("remove reaction: a specific matrix user's reaction is removed", t => { + const removals = removeReaction.removeReaction( + fakeSpecificReactionRemoval(BRIDGE_ID, "🐈", null), + fakeChunk([{key: "🐈", sender: "@cadence:cadence.moe"}]), + "🐈" + ) + t.deepEqual(removals, [{ + eventID: "$reaction_0", + mxid: null, + hash: 2842343637291700751n + }]) +}) + +test("remove reaction: a specific discord user's reaction is removed when there are multiple reactions", t => { + const removals = removeReaction.removeReaction( + fakeSpecificReactionRemoval("820865262526005258", "🐈", null), + fakeChunk([ + {key: "🐈‍⬛", sender: "@_ooye_crunch_god:cadence.moe"}, + {key: "🐈", sender: "@_ooye_crunch_god:cadence.moe"}, + {key: "🐈", sender: "@_ooye_extremity:cadence.moe"}, + {key: "🐈", sender: "@cadence:cadence.moe"}, + {key: "🐈", sender: "@zoe:cadence.moe"} + ]), + "🐈" + ) + t.deepEqual(removals, [{ + eventID: "$reaction_1", + mxid: "@_ooye_crunch_god:cadence.moe" + }]) +}) + +test("remove reaction: a specific reaction leads to all matrix users' reaction of the emoji being removed", t => { + const removals = removeReaction.removeReaction( + fakeSpecificReactionRemoval(BRIDGE_ID, "🐈", null), + fakeChunk([ + {key: "🐈", sender: "@_ooye_crunch_god:cadence.moe"}, + {key: "🐈", sender: "@cadence:cadence.moe"}, + {key: "🐈‍⬛", sender: "@zoe:cadence.moe"}, + {key: "🐈", sender: "@zoe:cadence.moe"}, + {key: "🐈", sender: "@_ooye_extremity:cadence.moe"} + ]), + "🐈" + ) + t.deepEqual(removals, [{ + eventID: "$reaction_1", + mxid: null, + hash: -8635141960139030904n + }, { + eventID: "$reaction_3", + mxid: null, + hash: 326222869084879263n + }]) +}) + +test("remove reaction: an emoji removes all instances of the emoij from both sides", t => { + const removals = removeReaction.removeEmojiReaction( + fakeEmojiReactionRemoval("🐈", null), + fakeChunk([ + {key: "🐈", sender: "@_ooye_crunch_god:cadence.moe"}, + {key: "🐈", sender: "@cadence:cadence.moe"}, + {key: "🐈‍⬛", sender: "@zoe:cadence.moe"}, + {key: "🐈", sender: "@zoe:cadence.moe"}, + {key: "🐈", sender: "@_ooye_extremity:cadence.moe"} + ]), + "🐈" + ) + t.deepEqual(removals, [{ + eventID: "$reaction_0", + mxid: "@_ooye_crunch_god:cadence.moe" + }, { + eventID: "$reaction_1", + mxid: null + }, { + eventID: "$reaction_3", + mxid: null + }, { + eventID: "$reaction_4", + mxid: "@_ooye_extremity:cadence.moe" + }]) +}) + +test("remove reaction: remove all removes all from both sides", t => { + const removals = removeReaction.removeAllReactions( + fakeAllReactionRemoval(), + fakeChunk([ + {key: "🐈", sender: "@_ooye_crunch_god:cadence.moe"}, + {key: "🐈", sender: "@cadence:cadence.moe"}, + {key: "🐈‍⬛", sender: "@zoe:cadence.moe"}, + {key: "🐈", sender: "@zoe:cadence.moe"}, + {key: "🐈", sender: "@_ooye_extremity:cadence.moe"} + ]) + ) + t.deepEqual(removals, [{ + eventID: "$reaction_0", + mxid: "@_ooye_crunch_god:cadence.moe" + }, { + eventID: "$reaction_1", + mxid: null + }, { + eventID: "$reaction_2", + mxid: null + }, { + eventID: "$reaction_3", + mxid: null + }, { + eventID: "$reaction_4", + mxid: "@_ooye_extremity:cadence.moe" + }]) +}) diff --git a/d2m/discord-packets.js b/d2m/discord-packets.js index dfba8b0..83c31cd 100644 --- a/d2m/discord-packets.js +++ b/d2m/discord-packets.js @@ -155,14 +155,8 @@ const utils = { } else if (message.t === "MESSAGE_REACTION_ADD") { await eventDispatcher.onReactionAdd(client, message.d) - } else if (message.t === "MESSAGE_REACTION_REMOVE") { - await eventDispatcher.onReactionRemove(client, message.d) - - } else if (message.t === "MESSAGE_REACTION_REMOVE_EMOJI") { - await eventDispatcher.onReactionEmojiRemove(client, message.d) - - } else if (message.t === "MESSAGE_REACTION_REMOVE_ALL") { - await eventDispatcher.onRemoveAllReactions(client, message.d) + } else if (message.t === "MESSAGE_REACTION_REMOVE" || message.t === "MESSAGE_REACTION_REMOVE_EMOJI" || message.t === "MESSAGE_REACTION_REMOVE_ALL") { + await eventDispatcher.onSomeReactionsRemoved(client, message.d) } } catch (e) { // Let OOYE try to handle errors too diff --git a/d2m/event-dispatcher.js b/d2m/event-dispatcher.js index 82e3b64..0f9f1e6 100644 --- a/d2m/event-dispatcher.js +++ b/d2m/event-dispatcher.js @@ -1,4 +1,5 @@ const assert = require("assert").strict +const DiscordTypes = require("discord-api-types/v10") const util = require("util") const {sync, db, select, from} = require("../passthrough") @@ -80,7 +81,7 @@ module.exports = { * If more messages were missed, only the latest missed message will be posted. TODO: Consider bridging more, or post a warning when skipping history? * This can ONLY detect new messages, not any other kind of event. Any missed edits, deletes, reactions, etc will not be bridged. * @param {import("./discord-client")} client - * @param {import("discord-api-types/v10").GatewayGuildCreateDispatchData} guild + * @param {DiscordTypes.GatewayGuildCreateDispatchData} guild */ async checkMissedMessages(client, guild) { if (guild.unavailable) return @@ -126,7 +127,7 @@ module.exports = { * Announces to the parent room that the thread room has been created. * See notes.md, "Ignore MESSAGE_UPDATE and bridge THREAD_CREATE as the announcement" * @param {import("./discord-client")} client - * @param {import("discord-api-types/v10").APIThreadChannel} thread + * @param {DiscordTypes.APIThreadChannel} thread */ async onThreadCreate(client, thread) { const parentRoomID = select("channel_room", "room_id", {channel_id: thread.parent_id}).pluck().get() @@ -137,7 +138,7 @@ module.exports = { /** * @param {import("./discord-client")} client - * @param {import("discord-api-types/v10").GatewayGuildUpdateDispatchData} guild + * @param {DiscordTypes.GatewayGuildUpdateDispatchData} guild */ async onGuildUpdate(client, guild) { const spaceID = select("guild_space", "space_id", {guild_id: guild.id}).pluck().get() @@ -147,7 +148,7 @@ module.exports = { /** * @param {import("./discord-client")} client - * @param {import("discord-api-types/v10").GatewayChannelUpdateDispatchData} channelOrThread + * @param {DiscordTypes.GatewayChannelUpdateDispatchData} channelOrThread * @param {boolean} isThread */ async onChannelOrThreadUpdate(client, channelOrThread, isThread) { @@ -158,7 +159,7 @@ module.exports = { /** * @param {import("./discord-client")} client - * @param {import("discord-api-types/v10").GatewayMessageCreateDispatchData} message + * @param {DiscordTypes.GatewayMessageCreateDispatchData} message */ async onMessageCreate(client, message) { if (message.author.username === "Deleted User") return // Nothing we can do for deleted users. @@ -169,7 +170,7 @@ module.exports = { return } } - /** @type {import("discord-api-types/v10").APIGuildChannel} */ + /** @type {DiscordTypes.APIGuildChannel} */ const channel = client.channels.get(message.channel_id) if (!channel.guild_id) return // Nothing we can do in direct messages. const guild = client.guilds.get(channel.guild_id) @@ -180,7 +181,7 @@ module.exports = { /** * @param {import("./discord-client")} client - * @param {import("discord-api-types/v10").GatewayMessageUpdateDispatchData} data + * @param {DiscordTypes.GatewayMessageUpdateDispatchData} data */ async onMessageUpdate(client, data) { if (data.webhook_id) { @@ -193,9 +194,9 @@ module.exports = { // Based on looking at data they've sent me over the gateway, this is the best way to check for meaningful changes. // If the message content is a string then it includes all interesting fields and is meaningful. if (typeof data.content === "string") { - /** @type {import("discord-api-types/v10").GatewayMessageCreateDispatchData} */ + /** @type {DiscordTypes.GatewayMessageCreateDispatchData} */ const message = data - /** @type {import("discord-api-types/v10").APIGuildChannel} */ + /** @type {DiscordTypes.APIGuildChannel} */ const channel = client.channels.get(message.channel_id) if (!channel.guild_id) return // Nothing we can do in direct messages. const guild = client.guilds.get(channel.guild_id) @@ -205,7 +206,7 @@ module.exports = { /** * @param {import("./discord-client")} client - * @param {import("discord-api-types/v10").GatewayMessageReactionAddDispatchData} data + * @param {DiscordTypes.GatewayMessageReactionAddDispatchData} data */ async onReactionAdd(client, data) { if (data.user_id === client.user.id) return // m2d reactions are added by the discord bot user - do not reflect them back to matrix. @@ -215,31 +216,15 @@ module.exports = { /** * @param {import("./discord-client")} client - * @param {import("discord-api-types/v10").GatewayMessageReactionRemoveDispatchData} data + * @param {DiscordTypes.GatewayMessageReactionRemoveDispatchData | DiscordTypes.GatewayMessageReactionRemoveEmojiDispatchData | DiscordTypes.GatewayMessageReactionRemoveAllDispatchData} data */ - async onReactionRemove(client, data) { - await removeReaction.removeReaction(data) + async onSomeReactionsRemoved(client, data) { + await removeReaction.removeSomeReactions(data) }, /** * @param {import("./discord-client")} client - * @param {import("discord-api-types/v10").GatewayMessageReactionRemoveEmojiDispatchData} data - */ - async onReactionEmojiRemove(client, data) { - await removeReaction.removeEmojiReaction(data) - }, - - /** - * @param {import("./discord-client")} client - * @param {import("discord-api-types/v10").GatewayMessageReactionRemoveAllDispatchData} data - */ - async onRemoveAllReactions(client, data) { - await removeReaction.removeAllReactions(data) - }, - - /** - * @param {import("./discord-client")} client - * @param {import("discord-api-types/v10").GatewayMessageDeleteDispatchData} data + * @param {DiscordTypes.GatewayMessageDeleteDispatchData} data */ async onMessageDelete(client, data) { await deleteMessage.deleteMessage(data) @@ -247,7 +232,7 @@ module.exports = { /** * @param {import("./discord-client")} client - * @param {import("discord-api-types/v10").GatewayTypingStartDispatchData} data + * @param {DiscordTypes.GatewayTypingStartDispatchData} data */ async onTypingStart(client, data) { const roomID = select("channel_room", "room_id", {channel_id: data.channel_id}).pluck().get() @@ -262,7 +247,7 @@ module.exports = { /** * @param {import("./discord-client")} client - * @param {import("discord-api-types/v10").GatewayGuildEmojisUpdateDispatchData | import("discord-api-types/v10").GatewayGuildStickersUpdateDispatchData} data + * @param {DiscordTypes.GatewayGuildEmojisUpdateDispatchData | DiscordTypes.GatewayGuildStickersUpdateDispatchData} data */ async onExpressionsUpdate(client, data) { await createSpace.syncSpaceExpressions(data) diff --git a/test/test.js b/test/test.js index 98ecf1c..5cc851e 100644 --- a/test/test.js +++ b/test/test.js @@ -20,7 +20,10 @@ const sync = new HeatSync({watchFS: false}) const discord = { guilds: new Map([ [data.guild.general.id, data.guild.general] - ]) + ]), + application: { + id: "684280192553844747" + } } Object.assign(passthrough, { discord, config, sync, db }) @@ -49,6 +52,7 @@ file._actuallyUploadDiscordFileToMxc = function(url, res) { throw new Error(`Not require("../d2m/converters/message-to-event.test") require("../d2m/converters/message-to-event.embeds.test") require("../d2m/converters/edit-to-changes.test") + require("../d2m/converters/remove-reaction.test") require("../d2m/converters/thread-to-announcement.test") require("../d2m/converters/user-to-mxid.test") require("../d2m/converters/emoji-to-key.test")