From 5acc38c8b58b45635586838315ada3579fa241a7 Mon Sep 17 00:00:00 2001 From: gnuxie Date: Thu, 16 Sep 2021 11:46:44 +0100 Subject: [PATCH] EventRedactionQueue documentation improvements from review --- src/Mjolnir.ts | 16 +++++++++----- src/queues/EventRedactionQueue.ts | 28 +++++++++++++++--------- src/queues/UnlistedUserRedactionQueue.ts | 5 ++--- 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/src/Mjolnir.ts b/src/Mjolnir.ts index ff9b292..8ce085d 100644 --- a/src/Mjolnir.ts +++ b/src/Mjolnir.ts @@ -149,8 +149,9 @@ export class Mjolnir { } /** - * Retruns the handler to flag a user that is not listed on a watchlist - * for redaction, removing any future messages that they send. + * Returns the handler to flag a user for redaction, removing any future messages that they send. + * Typically this is used by the flooding or image protection on users that have not been banned from a list yet. + * It cannot used to redact any previous messages the user has sent, in that cas you should use the `EventRedactionQueue`. */ public get unlistedUserRedactionHandler(): UnlistedUserRedactionQueue { return this.unlistedUserRedactionQueue; @@ -499,7 +500,7 @@ export class Mjolnir { } /** - * Sync all the rooms with all the watched lists, banning and applying any changes ACLS. + * Sync all the rooms with all the watched lists, banning and applying any changed ACLS. * @param verbose Whether to report any errors to the management room. */ public async syncLists(verbose = true) { @@ -610,7 +611,10 @@ export class Mjolnir { } return; } else if (event['type'] === "m.room.member") { - // Only apply bans and then redactions in the room we're looking at. + // The reason we have to apply bans on each member change is because + // we cannot eagerly ban users (that is to ban them when they have never been a member) + // as they can be force joined to a room they might not have known existed. + // Only apply bans and then redactions in the room we are currently looking at. const banErrors = await applyUserBans(this.banLists, [roomId], this); const redactionErrors = await this.processRedactionQueue(roomId); await this.printActionResult(banErrors); @@ -690,8 +694,10 @@ export class Mjolnir { /** * Process all queued redactions, this is usually called at the end of the sync process, * after all users have been banned and ACLs applied. + * If a redaction cannot be processed, the redaction is skipped and removed from the queue. + * We then carry on processing the next redactions. * @param roomId Limit processing to one room only, otherwise process redactions for all rooms. - * @returns An array of descriptive errors if any were encountered that can be reported to a management room. + * @returns The list of errors encountered, for reporting to the management room. */ public async processRedactionQueue(roomId?: string): Promise { return await this.eventRedactionQueue.process(this.client, roomId); diff --git a/src/queues/EventRedactionQueue.ts b/src/queues/EventRedactionQueue.ts index 96cf869..f71b918 100644 --- a/src/queues/EventRedactionQueue.ts +++ b/src/queues/EventRedactionQueue.ts @@ -23,11 +23,11 @@ export interface QueuedRedaction { /** The room which the redaction will take place in. */ readonly roomId: string; /** - * Carries out the redaction task and is called by the EventRedactionQueue - * when processing this redaction. + * Carry out the redaction. + * Called by the EventRedactionQueue. * @param client A MatrixClient to use to carry out the redaction. */ - redact(client: MatrixClient): Promise + redact(client: MatrixClient): Promise /** * Used to test whether the redaction is the equivalent to another redaction. * @param redaction Another QueuedRedaction to test if this redaction is an equivalent to. @@ -64,6 +64,9 @@ export class RedactUserInRoom implements QueuedRedaction { * This is a queue for events so that other protections can happen first (e.g. applying room bans to every room). */ export class EventRedactionQueue { + /** + * This map is indexed by roomId and its values are a list of redactions waiting to be processed for that room. + */ private toRedact: Map = new Map(); /** @@ -76,9 +79,9 @@ export class EventRedactionQueue { } /** - * Adds a QueuedRedaction to the queue to be processed when process is called. - * @param redaction A QueuedRedaction to await processing - * @returns A boolean that is true if the redaction was added to the queue and false if it was duplicated. + * Adds a `QueuedRedaction` to the queue. It will be processed when `process` is called. + * @param redaction A `QueuedRedaction` to await processing + * @returns `true` if the redaction was added to the queue, `false` if it is a duplicate of a redaction already present in the queue. */ public add(redaction: QueuedRedaction): boolean { if (this.has(redaction)) { @@ -89,12 +92,17 @@ export class EventRedactionQueue { entry.push(redaction); } else { this.toRedact.set(redaction.roomId, [redaction]); - }return true; + } + return true; } } /** - * Process the redaction queue, carrying out the action of each QueuedRedaction in sequence. + * Process the redaction queue, carrying out the action of each `QueuedRedaction` in sequence. + * If a redaction cannot be processed, the redaction is skipped and removed from the queue. + * We then carry on processing the next redactions. + * The reason we skip is at the moment is that we would have to think about all of the situations + * where we would not want failures to try again (e.g. messages were already redacted) and handle them explicitly. * @param client The matrix client to use for processing redactions. * @param limitToRoomId If the roomId is provided, only redactions for that room will be processed. * @returns A description of any errors encountered by each QueuedRedaction that was processed. @@ -125,13 +133,13 @@ export class EventRedactionQueue { // There might not actually be any queued redactions for this room. let queuedRedactions = this.toRedact.get(limitToRoomId); if (queuedRedactions) { - await redact(queuedRedactions); this.toRedact.delete(limitToRoomId); + await redact(queuedRedactions); } } else { for (const [roomId, redactions] of this.toRedact) { - await redact(redactions); this.toRedact.delete(roomId); + await redact(redactions); } } return errors; diff --git a/src/queues/UnlistedUserRedactionQueue.ts b/src/queues/UnlistedUserRedactionQueue.ts index 4130e9d..1d3cbd6 100644 --- a/src/queues/UnlistedUserRedactionQueue.ts +++ b/src/queues/UnlistedUserRedactionQueue.ts @@ -18,10 +18,9 @@ import { logMessage } from "../LogProxy"; import config from "../config"; /** - * This class is a queue of users who have been flagged - * for redaction by the flooding or image protection. + * A queue of users who have been flagged for redaction typically by the flooding or image protection. * Specifically any new events sent by a queued user will be redacted. - * This does not handle previously sent events, for that see the EventRedactionQueue. + * This does not handle previously sent events, for that see the `EventRedactionQueue`. * These users are not listed as banned in any watch list and so may continue * to view a room until a moderator can investigate. */