From 1ed082f3d4fdfa7a1f96208c561cb72bb7e1f797 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 28 Mar 2024 11:58:52 +0000 Subject: [PATCH] Fix merging of default push rules Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- spec/unit/pushprocessor.spec.ts | 138 ++++++++++++++++++++++++++++++-- src/pushprocessor.ts | 43 +++++++--- 2 files changed, 163 insertions(+), 18 deletions(-) diff --git a/spec/unit/pushprocessor.spec.ts b/spec/unit/pushprocessor.spec.ts index 90d218036..136ecc56c 100644 --- a/spec/unit/pushprocessor.spec.ts +++ b/spec/unit/pushprocessor.spec.ts @@ -1,6 +1,16 @@ import * as utils from "../test-utils/test-utils"; import { IActionsObject, PushProcessor } from "../../src/pushprocessor"; -import { ConditionKind, EventType, IContent, MatrixClient, MatrixEvent, PushRuleActionName, RuleId } from "../../src"; +import { + ConditionKind, + EventType, + IContent, + IPushRule, + MatrixClient, + MatrixEvent, + PushRuleActionName, + RuleId, + TweakName, +} from "../../src"; import { mockClientMethodsUser } from "../test-utils/client"; describe("NotificationService", function () { @@ -12,21 +22,21 @@ describe("NotificationService", function () { let pushProcessor: PushProcessor; - const msc3914RoomCallRule = { + const msc3914RoomCallRule: IPushRule = { rule_id: ".org.matrix.msc3914.rule.room.call", default: true, enabled: true, conditions: [ { - kind: "event_match", + kind: ConditionKind.EventMatch, key: "type", pattern: "org.matrix.msc3401.call", }, { - kind: "call_started", + kind: ConditionKind.CallStarted, }, ], - actions: ["notify", { set_tweak: "sound", value: "default" }], + actions: [PushRuleActionName.Notify, { set_tweak: TweakName.Sound, value: "default" }], }; let matrixClient: MatrixClient; @@ -188,6 +198,108 @@ describe("NotificationService", function () { }); it("should add default rules in the correct order", () => { + matrixClient.pushRules = PushProcessor.rewriteDefaultRules({ + device: {}, + global: { + content: [], + override: [ + { + rule_id: ".m.rule.master", + default: true, + enabled: false, + conditions: [], + actions: [], + }, + { + actions: [ + PushRuleActionName.Notify, + { + set_tweak: TweakName.Sound, + value: "default", + }, + { + set_tweak: TweakName.Highlight, + }, + ], + enabled: true, + pattern: "coffee", + rule_id: "coffee", + default: false, + }, + { + actions: [ + PushRuleActionName.Notify, + { + set_tweak: TweakName.Sound, + value: "default", + }, + { + set_tweak: TweakName.Highlight, + }, + ], + conditions: [ + { + kind: ConditionKind.ContainsDisplayName, + }, + ], + enabled: true, + default: true, + rule_id: ".m.rule.contains_display_name", + }, + { + actions: [ + PushRuleActionName.Notify, + { + set_tweak: TweakName.Sound, + value: "default", + }, + ], + conditions: [ + { + is: "2", + kind: ConditionKind.RoomMemberCount, + }, + ], + enabled: true, + rule_id: ".m.rule.room_one_to_one", + default: true, + }, + ], + room: [], + sender: [], + underride: [ + { + actions: [ + PushRuleActionName.Notify, + { + set_tweak: TweakName.Highlight, + value: false, + }, + ], + conditions: [], + enabled: true, + rule_id: "user-defined", + default: false, + }, + msc3914RoomCallRule, + { + actions: [ + PushRuleActionName.Notify, + { + set_tweak: TweakName.Highlight, + value: false, + }, + ], + conditions: [], + enabled: true, + rule_id: ".m.rule.fallback", + default: true, + }, + ], + }, + }); + pushProcessor = new PushProcessor(matrixClient); + // By the time we get here, we expect the PushProcessor to have merged the new .m.rule.is_room_mention rule into the existing list of rules. // Check that has happened, and that it is in the right place. const containsDisplayNameRuleIdx = matrixClient.pushRules?.global.override?.findIndex( @@ -205,6 +317,22 @@ describe("NotificationService", function () { expect(containsDisplayNameRuleIdx).toBeLessThan(isRoomMentionRuleIdx!); expect(isRoomMentionRuleIdx).toBeLessThan(mReactionRuleIdx!); + + expect(matrixClient.pushRules?.global.override?.map((r) => r.rule_id)).toEqual([ + ".m.rule.master", + "coffee", + ".m.rule.contains_display_name", + ".m.rule.room_one_to_one", + ".m.rule.is_room_mention", + ".m.rule.reaction", + ".org.matrix.msc3786.rule.room.server_acl", + ]); + expect(matrixClient.pushRules?.global.underride?.map((r) => r.rule_id)).toEqual([ + "user-defined", + ".org.matrix.msc3914.rule.room.call", + // Assert that unknown default rules are maintained + ".m.rule.fallback", + ]); }); // User IDs diff --git a/src/pushprocessor.ts b/src/pushprocessor.ts index 162c36dc9..b13cfc0ca 100644 --- a/src/pushprocessor.ts +++ b/src/pushprocessor.ts @@ -25,8 +25,8 @@ import { ICallStartedPrefixCondition, IContainsDisplayNameCondition, IEventMatchCondition, - IEventPropertyIsCondition, IEventPropertyContainsCondition, + IEventPropertyIsCondition, IPushRule, IPushRules, IRoomMemberCountCondition, @@ -49,6 +49,10 @@ const RULEKINDS_IN_ORDER = [ PushRuleKind.Underride, ]; +const UserDefinedRules = Symbol("UserDefinedRules"); + +type OrderedRules = Array; + // The default override rules to apply to the push rules that arrive from the server. // We do this for two reasons: // 1. Synapse is unlikely to send us the push rule in an incremental sync - see @@ -115,8 +119,9 @@ const DEFAULT_OVERRIDE_RULES: Record = { }, }; -const EXPECTED_DEFAULT_OVERRIDE_RULE_IDS = [ +const EXPECTED_DEFAULT_OVERRIDE_RULE_IDS: OrderedRules = [ RuleId.Master, + UserDefinedRules, RuleId.SuppressNotices, RuleId.InviteToSelf, RuleId.MemberEvent, @@ -151,7 +156,8 @@ const DEFAULT_UNDERRIDE_RULES: Record = { }, }; -const EXPECTED_DEFAULT_UNDERRIDE_RULE_IDS = [ +const EXPECTED_DEFAULT_UNDERRIDE_RULE_IDS: OrderedRules = [ + UserDefinedRules, RuleId.IncomingCall, RuleId.EncryptedDM, RuleId.DM, @@ -162,6 +168,7 @@ const EXPECTED_DEFAULT_UNDERRIDE_RULE_IDS = [ /** * Make sure that each of the rules listed in `defaultRuleIds` is listed in the given set of push rules. * + * @param kind - the kind of push rule set being merged. * @param incomingRules - the existing set of known push rules for the user. * @param defaultRules - a lookup table for the default definitions of push rules. * @param defaultRuleIds - the IDs of the expected default push rules, in order. @@ -169,17 +176,23 @@ const EXPECTED_DEFAULT_UNDERRIDE_RULE_IDS = [ * @returns A copy of `incomingRules`, with any missing default rules inserted in the right place. */ function mergeRulesWithDefaults( + kind: PushRuleKind, incomingRules: IPushRule[], defaultRules: Record, - defaultRuleIds: string[], + defaultRuleIds: OrderedRules, ): IPushRule[] { - // Calculate the index after the last default rule in `incomingRules` - // to allow us to split the incomingRules into defaults and custom - let firstCustomRuleIndex = incomingRules.findIndex((r) => !r.default); - if (firstCustomRuleIndex < 0) firstCustomRuleIndex = incomingRules.length; + // Find the indices of the edges of the user-defined rules in the incoming rules + const incomingRulesEnabled = incomingRules.map((rule) => rule.enabled); + const userDefinedRulesRange: [number, number] = [ + incomingRulesEnabled.indexOf(false), + incomingRulesEnabled.lastIndexOf(false), + ]; - function insertDefaultPushRule(ruleId: string): void { - if (ruleId in defaultRules) { + function insertDefaultPushRule(ruleId: OrderedRules[number]): void { + if (ruleId === UserDefinedRules) { + // Re-insert any user-defined rules that were in `incomingRules` + newRules.push(...incomingRules.slice(...userDefinedRulesRange)); + } else if (ruleId in defaultRules) { logger.warn(`Adding default global push rule ${ruleId}`); newRules.push(defaultRules[ruleId]); } else { @@ -189,7 +202,11 @@ function mergeRulesWithDefaults( let nextExpectedRuleIdIndex = 0; const newRules: IPushRule[] = []; - for (const rule of incomingRules.slice(0, firstCustomRuleIndex)) { + // Process the default rules by merging them with defaults + for (const rule of [ + ...incomingRules.slice(0, userDefinedRulesRange[0]), + ...incomingRules.slice(userDefinedRulesRange[1]), + ]) { const ruleIndex = defaultRuleIds.indexOf(rule.rule_id); if (ruleIndex === -1) { // an unrecognised rule; copy it over @@ -212,8 +229,6 @@ function mergeRulesWithDefaults( insertDefaultPushRule(ruleId); } - // Finally any non-default rules that were in `incomingRules` - newRules.push(...incomingRules.slice(firstCustomRuleIndex)); return newRules; } @@ -281,12 +296,14 @@ export class PushProcessor { // Merge the client-level defaults with the ones from the server newRules.global.override = mergeRulesWithDefaults( + PushRuleKind.Override, newRules.global.override, DEFAULT_OVERRIDE_RULES, EXPECTED_DEFAULT_OVERRIDE_RULE_IDS, ); newRules.global.underride = mergeRulesWithDefaults( + PushRuleKind.Underride, newRules.global.underride, DEFAULT_UNDERRIDE_RULES, EXPECTED_DEFAULT_UNDERRIDE_RULE_IDS,