EventRedactionQueue documentation improvements from review

src/Mjolnir.ts

@@ -149,8 +149,9 @@ export class Mjolnir {
     }
 
     /**
-     * Retruns the handler to flag a user that is not listed on a watchlist
+     * Returns the handler to flag a user for redaction, removing any future messages that they send.
-     * 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<RoomUpdateError[]> {
         return await this.eventRedactionQueue.process(this.client, roomId);

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
+     * Carry out the redaction.
-     * when processing this redaction.
+     * Called by the EventRedactionQueue.
      * @param client A MatrixClient to use to carry out the redaction.
      */
-    redact(client: MatrixClient): Promise<any>
+    redact(client: MatrixClient): Promise<void>
     /**
      * 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<string, QueuedRedaction[]> = new Map<string, QueuedRedaction[]>();
 
     /**
@@ -76,9 +79,9 @@ export class EventRedactionQueue {
     }
 
     /**
-     * Adds a QueuedRedaction to the queue to be processed when process is called.
+     * Adds a `QueuedRedaction` to the queue. It will be processed when `process` is called.
-     * @param redaction A QueuedRedaction to await processing
+     * @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.
+     * @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;

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
+ * A queue of users who have been flagged for redaction typically by the flooding or image protection.
- * for redaction 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.
  */