diff --git a/src/Mjolnir.ts b/src/Mjolnir.ts index ae20d171..50034d50 100644 --- a/src/Mjolnir.ts +++ b/src/Mjolnir.ts @@ -436,7 +436,6 @@ export class Mjolnir { private async addPolicyList(roomId: string, roomRef: string): Promise { const list = new PolicyList(roomId, roomRef, this.client); this.ruleServer?.watch(list); - list.on('PolicyList.batch', (...args) => this.protectedRoomsTracker.syncWithPolicyList(...args)); await list.updateList(); this.policyLists.push(list); this.protectedRoomsTracker.watchList(list); diff --git a/src/ProtectedRoomsSet.ts b/src/ProtectedRoomsSet.ts index f937500b..92fb235b 100644 --- a/src/ProtectedRoomsSet.ts +++ b/src/ProtectedRoomsSet.ts @@ -88,6 +88,12 @@ export class ProtectedRoomsSet { */ private readonly accessControlUnit = new AccessControlUnit([]); + /** + * Intended to be `this.syncWithUpdatedPolicyList` so we can add it in `this.watchList` and remove it in `this.unwatchList`. + * Otherwise we would risk being informed about lists we no longer watch. + */ + private readonly listUpdateListener: (list: PolicyList, changes: ListRuleChange[]) => void; + constructor( private readonly client: MatrixSendClient, private readonly clientUserId: string, @@ -108,6 +114,7 @@ export class ProtectedRoomsSet { // Setup room activity watcher this.protectedRoomActivityTracker = new ProtectedRoomActivityTracker(); + this.listUpdateListener = this.syncWithUpdatedPolicyList.bind(this); } /** @@ -143,12 +150,14 @@ export class ProtectedRoomsSet { if (!this.policyLists.includes(policyList)) { this.policyLists.push(policyList); this.accessControlUnit.watchList(policyList); + policyList.on('PolicyList.update', this.listUpdateListener); } } public unwatchList(policyList: PolicyList): void { this.policyLists = this.policyLists.filter(list => list.roomId !== policyList.roomId); this.accessControlUnit.unwatchList(policyList); + policyList.off('PolicyList.update', this.listUpdateListener) } /** @@ -248,15 +257,13 @@ export class ProtectedRoomsSet { } /** - * Pulls any changes to the rules that are in a policy room and updates all protected rooms - * with those changes. Does not fail if there are errors updating the room, these are reported to the management room. + * Updates all protected rooms with those any changes that have been made to a policy list. + * Does not fail if there are errors updating the room, these are reported to the management room. + * Do not use directly as a listener, use `this.listUpdateListener`. * @param policyList The `PolicyList` which we will check for changes and apply them to all protected rooms. * @returns When all of the protected rooms have been updated. */ - public async syncWithPolicyList(policyList: PolicyList): Promise { - // this bit can move away into a listener. - const changes = await policyList.updateList(); - + private async syncWithUpdatedPolicyList(policyList: PolicyList, changes: ListRuleChange[]): Promise { let hadErrors = false; const [aclErrors, banErrors] = await Promise.all([ this.applyServerAcls(this.policyLists, this.protectedRoomsByActivity()), diff --git a/src/models/PolicyList.ts b/src/models/PolicyList.ts index 4034ec4e..278d8671 100644 --- a/src/models/PolicyList.ts +++ b/src/models/PolicyList.ts @@ -56,9 +56,6 @@ declare interface PolicyList { // PolicyList.update is emitted when the PolicyList has pulled new rules from Matrix and informs listeners of any changes. on(event: 'PolicyList.update', listener: (list: PolicyList, changes: ListRuleChange[]) => void): this emit(event: 'PolicyList.update', list: PolicyList, changes: ListRuleChange[]): boolean - // PolicyList.batch is emitted when the PolicyList has created a batch from the events provided by `updateForEvent`. - on(event: 'PolicyList.batch', listener: (list: PolicyList) => void): this - emit(event: 'PolicyList.batch', list: PolicyList): boolean } /** @@ -476,7 +473,8 @@ export default PolicyList; /** * Helper class that emits a batch event on a `PolicyList` when it has made a batch - * out of the events given to `addToBatch`. + * out of the Matrix events given to `addToBatch` via `updateForEvent`. + * The `UpdateBatcher` will then call `list.update()` on the associated `PolicyList` once it has finished batching events. */ class UpdateBatcher { // Whether we are waiting for more events to form a batch. @@ -510,7 +508,8 @@ class UpdateBatcher { await new Promise(resolve => setTimeout(resolve, this.waitPeriodMS)); } while ((Date.now() - start) < this.maxWaitMS && this.latestEventId !== eventId) this.reset(); - this.banList.emit('PolicyList.batch', this.banList); + // batching finished, update the associated list. + await this.banList.updateList(); } /**