From e1e1854041d9941fa5423efc42d6b230a8588d4b Mon Sep 17 00:00:00 2001 From: Dragory <2606411+Dragory@users.noreply.github.com> Date: Wed, 23 Dec 2020 05:28:21 +0200 Subject: [PATCH] Fix race conditions and duplicate stars in starboard --- backend/src/data/GuildStarboardReactions.ts | 14 +++++++++- ...8692857722-FixStarboardReactionsIndices.ts | 26 +++++++++++++++++++ .../events/StarboardReactionAddEvt.ts | 8 +++--- .../events/StarboardReactionRemoveEvts.ts | 4 +++ .../util/updateStarboardMessageStarCount.ts | 20 +++++++++++--- 5 files changed, 63 insertions(+), 9 deletions(-) create mode 100644 backend/src/migrations/1608692857722-FixStarboardReactionsIndices.ts diff --git a/backend/src/data/GuildStarboardReactions.ts b/backend/src/data/GuildStarboardReactions.ts index 3edc3d2c..5094491e 100644 --- a/backend/src/data/GuildStarboardReactions.ts +++ b/backend/src/data/GuildStarboardReactions.ts @@ -19,10 +19,22 @@ export class GuildStarboardReactions extends BaseGuildRepository { } async createStarboardReaction(messageId: string, reactorId: string) { + const existingReaction = await this.allStarboardReactions.findOne({ + where: { + guild_id: this.guildId, + message_id: messageId, + reactor_id: reactorId, + }, + }); + + if (existingReaction) { + return; + } + await this.allStarboardReactions.insert({ + guild_id: this.guildId, message_id: messageId, reactor_id: reactorId, - guild_id: this.guildId, }); } diff --git a/backend/src/migrations/1608692857722-FixStarboardReactionsIndices.ts b/backend/src/migrations/1608692857722-FixStarboardReactionsIndices.ts new file mode 100644 index 00000000..dcde8a09 --- /dev/null +++ b/backend/src/migrations/1608692857722-FixStarboardReactionsIndices.ts @@ -0,0 +1,26 @@ +import { MigrationInterface, QueryRunner, TableIndex, TableUnique } from "typeorm"; + +export class FixStarboardReactionsIndices1608692857722 implements MigrationInterface { + public async up(queryRunner: QueryRunner): Promise { + // Remove previously-added duplicate stars + await queryRunner.query(` + DELETE r1.* FROM starboard_reactions AS r1 + INNER JOIN starboard_reactions AS r2 + ON r2.guild_id = r1.guild_id AND r2.message_id = r1.message_id AND r2.reactor_id = r1.reactor_id AND r2.id < r1.id + `); + + await queryRunner.dropIndex("starboard_reactions", "IDX_dd871a4ef459dd294aa368e736"); + await queryRunner.createIndex( + "starboard_reactions", + new TableIndex({ + isUnique: true, + columnNames: ["guild_id", "message_id", "reactor_id"], + }), + ); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.dropIndex("starboard_reactions", "IDX_d08ee47552c92ec8afd1a5bd1b"); + await queryRunner.createIndex("starboard_reactions", new TableIndex({ columnNames: ["reactor_id", "message_id"] })); + } +} diff --git a/backend/src/plugins/Starboard/events/StarboardReactionAddEvt.ts b/backend/src/plugins/Starboard/events/StarboardReactionAddEvt.ts index 0de8af2f..b6142655 100644 --- a/backend/src/plugins/Starboard/events/StarboardReactionAddEvt.ts +++ b/backend/src/plugins/Starboard/events/StarboardReactionAddEvt.ts @@ -36,6 +36,8 @@ export const StarboardReactionAddEvt = starboardEvt({ categoryId: (msg.channel as TextChannel).parentID, }); + const boardLock = await pluginData.locks.acquire(`starboards`); + const applicableStarboards = Object.values(config.boards) .filter(board => board.enabled) // Can't star messages in the starboard channel itself @@ -59,8 +61,6 @@ export const StarboardReactionAddEvt = starboardEvt({ }); for (const starboard of applicableStarboards) { - const boardLock = await pluginData.locks.acquire(`starboards-channel-${starboard.channel_id}`); - // Save reaction into the database await pluginData.state.starboardReactions.createStarboardReaction(msg.id, userId).catch(noop); @@ -92,8 +92,8 @@ export const StarboardReactionAddEvt = starboardEvt({ // Otherwise, if the star count exceeds the required star count, save the message to the starboard await saveMessageToStarboard(pluginData, msg, starboard); } - - boardLock.unlock(); } + + boardLock.unlock(); }, }); diff --git a/backend/src/plugins/Starboard/events/StarboardReactionRemoveEvts.ts b/backend/src/plugins/Starboard/events/StarboardReactionRemoveEvts.ts index a42e5510..07f85670 100644 --- a/backend/src/plugins/Starboard/events/StarboardReactionRemoveEvts.ts +++ b/backend/src/plugins/Starboard/events/StarboardReactionRemoveEvts.ts @@ -4,7 +4,9 @@ export const StarboardReactionRemoveEvt = starboardEvt({ event: "messageReactionRemove", async listener(meta) { + const boardLock = await meta.pluginData.locks.acquire(`starboards`); await meta.pluginData.state.starboardReactions.deleteStarboardReaction(meta.args.message.id, meta.args.member.id); + boardLock.unlock(); }, }); @@ -12,6 +14,8 @@ export const StarboardReactionRemoveAllEvt = starboardEvt({ event: "messageReactionRemoveAll", async listener(meta) { + const boardLock = await meta.pluginData.locks.acquire(`starboards`); await meta.pluginData.state.starboardReactions.deleteAllStarboardReactionsForMessageId(meta.args.message.id); + boardLock.unlock(); }, }); diff --git a/backend/src/plugins/Starboard/util/updateStarboardMessageStarCount.ts b/backend/src/plugins/Starboard/util/updateStarboardMessageStarCount.ts index 61ce2bd5..2dc70247 100644 --- a/backend/src/plugins/Starboard/util/updateStarboardMessageStarCount.ts +++ b/backend/src/plugins/Starboard/util/updateStarboardMessageStarCount.ts @@ -2,6 +2,10 @@ import { Client, GuildTextableChannel, Message } from "eris"; import { noop } from "../../../utils"; import { createStarboardPseudoFooterForMessage } from "./createStarboardPseudoFooterForMessage"; import { TStarboardOpts } from "../types"; +import Timeout = NodeJS.Timeout; + +const DEBOUNCE_DELAY = 1000; +const debouncedUpdates: Record = {}; export async function updateStarboardMessageStarCount( starboard: TStarboardOpts, @@ -10,8 +14,16 @@ export async function updateStarboardMessageStarCount( starEmoji: string, starCount: number, ) { - const embed = starboardMessage.embeds[0]!; - embed.fields!.shift(); // Remove pseudo footer - embed.fields!.push(createStarboardPseudoFooterForMessage(starboard, originalMessage, starEmoji, starCount)); // Create new pseudo footer - await starboardMessage.edit({ embed }); + const key = `${originalMessage.id}-${starboardMessage.id}`; + if (debouncedUpdates[key]) { + clearTimeout(debouncedUpdates[key]); + } + + debouncedUpdates[key] = setTimeout(() => { + delete debouncedUpdates[key]; + const embed = starboardMessage.embeds[0]!; + embed.fields!.shift(); // Remove pseudo footer + embed.fields!.push(createStarboardPseudoFooterForMessage(starboard, originalMessage, starEmoji, starCount)); // Create new pseudo footer + starboardMessage.edit({ embed }); + }, DEBOUNCE_DELAY); }