Make a Revision class for revision ids rather than using a string.

Only thing I don't like is that ulid is public for the comparison method?
This commit is contained in:
gnuxie 2022-12-08 14:22:07 +00:00
parent 0680ac71e5
commit b267b6199f
2 changed files with 51 additions and 21 deletions

View File

@ -21,7 +21,7 @@ import ManagementRoomOutput from "./ManagementRoomOutput";
import { MatrixSendClient } from "./MatrixEmitter"; import { MatrixSendClient } from "./MatrixEmitter";
import AccessControlUnit, { Access } from "./models/AccessControlUnit"; import AccessControlUnit, { Access } from "./models/AccessControlUnit";
import { RULE_ROOM, RULE_SERVER, RULE_USER } from "./models/ListRule"; import { RULE_ROOM, RULE_SERVER, RULE_USER } from "./models/ListRule";
import PolicyList, { ListRuleChange } from "./models/PolicyList"; import PolicyList, { ListRuleChange, Revision } from "./models/PolicyList";
import { RoomUpdateError } from "./models/RoomUpdateError"; import { RoomUpdateError } from "./models/RoomUpdateError";
import { ProtectionManager } from "./protections/ProtectionManager"; import { ProtectionManager } from "./protections/ProtectionManager";
import { EventRedactionQueue, RedactUserInRoom } from "./queues/EventRedactionQueue"; import { EventRedactionQueue, RedactUserInRoom } from "./queues/EventRedactionQueue";
@ -92,12 +92,12 @@ export class ProtectedRoomsSet {
* Intended to be `this.syncWithUpdatedPolicyList` so we can add it in `this.watchList` and remove it in `this.unwatchList`. * 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. * Otherwise we would risk being informed about lists we no longer watch.
*/ */
private readonly listUpdateListener: (list: PolicyList, changes: ListRuleChange[], revisionId: string) => void; private readonly listUpdateListener: (list: PolicyList, changes: ListRuleChange[], revision: Revision) => void;
/** /**
* The revision of a each watched list that we have applied to protected rooms. * The revision of a each watched list that we have applied to protected rooms.
*/ */
private readonly listRevisions = new Map<PolicyList, /** The last revision we used to sync protected rooms. */ string>(); private readonly listRevisions = new Map<PolicyList, /** The last revision we used to sync protected rooms. */ Revision>();
constructor( constructor(
private readonly client: MatrixSendClient, private readonly client: MatrixSendClient,
@ -217,7 +217,7 @@ export class ProtectedRoomsSet {
/** /**
* Synchronize all the protected rooms with all of the policies described in the watched policy lists. * Synchronize all the protected rooms with all of the policies described in the watched policy lists.
*/ */
public async syncRoomsWithPolicies() { private async syncRoomsWithPolicies() {
let hadErrors = false; let hadErrors = false;
const [aclErrors, banErrors] = await Promise.all([ const [aclErrors, banErrors] = await Promise.all([
this.applyServerAcls(this.policyLists, this.protectedRoomsByActivity()), this.applyServerAcls(this.policyLists, this.protectedRoomsByActivity()),
@ -245,11 +245,11 @@ export class ProtectedRoomsSet {
*/ */
public async syncLists() { public async syncLists() {
for (const list of this.policyLists) { for (const list of this.policyLists) {
const { revisionId } = await list.updateList(); const { revision } = await list.updateList();
const previousRevision = this.listRevisions.get(list); const previousRevision = this.listRevisions.get(list);
if (previousRevision === undefined || revisionId > previousRevision) { if (previousRevision === undefined || revision.supersedes(previousRevision)) {
this.listRevisions.set(list, revisionId); this.listRevisions.set(list, revision);
// we rely `this.listUpdateListener` to print the changes to the list. // we rely on `this.listUpdateListener` to print the changes to the list.
} }
} }
await this.syncRoomsWithPolicies(); await this.syncRoomsWithPolicies();
@ -277,11 +277,11 @@ export class ProtectedRoomsSet {
* @param policyList The `PolicyList` which we will check for changes and apply them to all protected rooms. * @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. * @returns When all of the protected rooms have been updated.
*/ */
private async syncWithUpdatedPolicyList(policyList: PolicyList, changes: ListRuleChange[], revisionId: string): Promise<void> { private async syncWithUpdatedPolicyList(policyList: PolicyList, changes: ListRuleChange[], revision: Revision): Promise<void> {
// avoid resyncing the rooms if we have already done so for the latest revision of this list. // avoid resyncing the rooms if we have already done so for the latest revision of this list.
const previousRevision = this.listRevisions.get(policyList); const previousRevision = this.listRevisions.get(policyList);
if (previousRevision === undefined || revisionId > previousRevision) { if (previousRevision === undefined || revision.supersedes(previousRevision)) {
this.listRevisions.set(policyList, revisionId); this.listRevisions.set(policyList, revision);
await this.syncRoomsWithPolicies(); await this.syncRoomsWithPolicies();
} }
// This can fail if the change is very large and it is much less important than applying bans, so do it last. // This can fail if the change is very large and it is much less important than applying bans, so do it last.

View File

@ -21,8 +21,6 @@ import { MatrixSendClient } from "../MatrixEmitter";
import AwaitLock from "await-lock"; import AwaitLock from "await-lock";
import { monotonicFactory } from "ulidx"; import { monotonicFactory } from "ulidx";
const ulid = monotonicFactory();
export const SHORTCODE_EVENT_TYPE = "org.matrix.mjolnir.shortcode"; export const SHORTCODE_EVENT_TYPE = "org.matrix.mjolnir.shortcode";
export enum ChangeType { export enum ChangeType {
@ -58,8 +56,8 @@ export interface ListRuleChange {
declare interface PolicyList { declare interface PolicyList {
// PolicyList.update is emitted when the PolicyList has pulled new rules from Matrix and informs listeners of any changes. // 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[], revisionId: string) => void): this on(event: 'PolicyList.update', listener: (list: PolicyList, changes: ListRuleChange[], revision: Revision) => void): this
emit(event: 'PolicyList.update', list: PolicyList, changes: ListRuleChange[], revisionId: string): boolean emit(event: 'PolicyList.update', list: PolicyList, changes: ListRuleChange[], revision: Revision): boolean
} }
/** /**
@ -101,12 +99,12 @@ class PolicyList extends EventEmitter {
private static readonly EVENT_RULE_ANNOTATION_KEY = 'org.matrix.mjolnir.annotation.rule'; private static readonly EVENT_RULE_ANNOTATION_KEY = 'org.matrix.mjolnir.annotation.rule';
/** /**
* A ULID for the current revision of the list state. * An ID that represents the current version of the list state.
* Each time we use `updateList` we create a new revision to represent the change of state. * Each time we use `updateList` we create a new revision to represent the change of state.
* Listeners can then use the revision id to work out whether they have already applied * Listeners can then use the revision to work out whether they have already applied
* the latest revision. * the latest revision.
*/ */
private revisionId = ulid(); private revisionId = new Revision();
/** /**
* A lock to protect `updateList` from a situation where one call to `getRoomState` can start and end before another. * A lock to protect `updateList` from a situation where one call to `getRoomState` can start and end before another.
@ -378,7 +376,7 @@ class PolicyList extends EventEmitter {
* @param state Room state to update the list with, provided by `updateList` * @param state Room state to update the list with, provided by `updateList`
* @returns Any changes that have been made to the PolicyList. * @returns Any changes that have been made to the PolicyList.
*/ */
private updateListWithState(state: any): { revisionId: string, changes: ListRuleChange[] } { private updateListWithState(state: any): { revision: Revision, changes: ListRuleChange[] } {
const changes: ListRuleChange[] = []; const changes: ListRuleChange[] = [];
for (const event of state) { for (const event of state) {
if (event['state_key'] === '' && event['type'] === SHORTCODE_EVENT_TYPE) { if (event['state_key'] === '' && event['type'] === SHORTCODE_EVENT_TYPE) {
@ -476,7 +474,7 @@ class PolicyList extends EventEmitter {
} }
} }
if (changes.length > 0) { if (changes.length > 0) {
this.revisionId = ulid(); this.revisionId = new Revision();
this.emit('PolicyList.update', this, changes, this.revisionId); this.emit('PolicyList.update', this, changes, this.revisionId);
} }
if (this.batchedEvents.keys.length !== 0) { if (this.batchedEvents.keys.length !== 0) {
@ -486,7 +484,7 @@ class PolicyList extends EventEmitter {
// we don't want Mjolnir to stop working properly. Though, I am not confident a burried warning is going to alert us. // we don't want Mjolnir to stop working properly. Though, I am not confident a burried warning is going to alert us.
LogService.warn("PolicyList", "The policy list is being informed about events that it cannot find in the room state, this is really bad and you should seek help."); LogService.warn("PolicyList", "The policy list is being informed about events that it cannot find in the room state, this is really bad and you should seek help.");
} }
return { revisionId: this.revisionId, changes }; return { revision: this.revisionId, changes };
} }
/** /**
@ -566,3 +564,35 @@ class UpdateBatcher {
this.checkBatch(eventId); this.checkBatch(eventId);
} }
} }
/**
* Represents a specific version of the state contained in `PolicyList`.
* These are unique and can be compared with `supersedes`.
* We use a ULID to work out whether a revision supersedes another.
*/
export class Revision {
/**
* Ensures that ULIDs are monotonic.
*/
private static makeULID = monotonicFactory();
/**
* Is only public for the comparison method,
* I feel like I'm missing something here and it is possible without
*/
public readonly ulid = Revision.makeULID();
constructor() {
// nothing to do.
}
/**
* Check whether this revision supersedes another revision.
* @param revision The revision we want to check this supersedes.
* @returns True if this Revision supersedes the other revision.
*/
public supersedes(revision: Revision): boolean {
return this.ulid > revision.ulid;
}
}