diff --git a/spec/unit/pushprocessor.spec.ts b/spec/unit/pushprocessor.spec.ts index 90d2180361b..ed7261e727d 100644 --- a/spec/unit/pushprocessor.spec.ts +++ b/spec/unit/pushprocessor.spec.ts @@ -1,8 +1,35 @@ 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"; +const msc3914RoomCallRule: IPushRule = { + rule_id: ".org.matrix.msc3914.rule.room.call", + default: true, + enabled: true, + conditions: [ + { + kind: ConditionKind.EventMatch, + key: "type", + pattern: "org.matrix.msc3401.call", + }, + { + kind: ConditionKind.CallStarted, + }, + ], + actions: [PushRuleActionName.Notify, { set_tweak: TweakName.Sound, value: "default" }], +}; + describe("NotificationService", function () { const testUserId = "@ali:matrix.org"; const testDisplayName = "Alice M"; @@ -12,23 +39,6 @@ describe("NotificationService", function () { let pushProcessor: PushProcessor; - const msc3914RoomCallRule = { - rule_id: ".org.matrix.msc3914.rule.room.call", - default: true, - enabled: true, - conditions: [ - { - kind: "event_match", - key: "type", - pattern: "org.matrix.msc3401.call", - }, - { - kind: "call_started", - }, - ], - actions: ["notify", { set_tweak: "sound", value: "default" }], - }; - let matrixClient: MatrixClient; beforeEach(function () { @@ -187,26 +197,6 @@ describe("NotificationService", function () { pushProcessor = new PushProcessor(matrixClient); }); - it("should add default rules in the correct order", () => { - // 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( - (rule) => rule.rule_id === RuleId.ContainsDisplayName, - ); - expect(containsDisplayNameRuleIdx).toBeGreaterThan(-1); - const isRoomMentionRuleIdx = matrixClient.pushRules?.global.override?.findIndex( - (rule) => rule.rule_id === RuleId.IsRoomMention, - ); - expect(isRoomMentionRuleIdx).toBeGreaterThan(-1); - const mReactionRuleIdx = matrixClient.pushRules?.global.override?.findIndex( - (rule) => rule.rule_id === ".m.rule.reaction", - ); - expect(mReactionRuleIdx).toBeGreaterThan(-1); - - expect(containsDisplayNameRuleIdx).toBeLessThan(isRoomMentionRuleIdx!); - expect(isRoomMentionRuleIdx).toBeLessThan(mReactionRuleIdx!); - }); - // User IDs it("should bing on a user ID.", function () { @@ -722,3 +712,295 @@ describe("Test PushProcessor.partsForDottedKey", function () { expect(PushProcessor.partsForDottedKey(path)).toStrictEqual(expected); }); }); + +describe("rewriteDefaultRules", () => { + it("should add default rules in the correct order", () => { + const pushRules = PushProcessor.rewriteDefaultRules({ + device: {}, + global: { + content: [], + override: [ + // Include user-defined push rules inbetween .m.rule.master and other default rules to assert they are maintained in-order. + { + 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, + }, + ], + }, + }); + + // 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 = pushRules.global.override?.findIndex( + (rule) => rule.rule_id === RuleId.ContainsDisplayName, + ); + expect(containsDisplayNameRuleIdx).toBeGreaterThan(-1); + const isRoomMentionRuleIdx = pushRules.global.override?.findIndex( + (rule) => rule.rule_id === RuleId.IsRoomMention, + ); + expect(isRoomMentionRuleIdx).toBeGreaterThan(-1); + const mReactionRuleIdx = pushRules.global.override?.findIndex((rule) => rule.rule_id === ".m.rule.reaction"); + expect(mReactionRuleIdx).toBeGreaterThan(-1); + + expect(containsDisplayNameRuleIdx).toBeLessThan(isRoomMentionRuleIdx!); + expect(isRoomMentionRuleIdx).toBeLessThan(mReactionRuleIdx!); + + expect(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(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", + ]); + }); + + it("should add missing msc3914 rule in correct place", () => { + const pushRules = PushProcessor.rewriteDefaultRules({ + device: {}, + global: { + // Sample push rules from a Synapse user. + // Note that rules 2 and 3 are backwards, this will trigger a warning in the console. + underride: [ + { + conditions: [ + { + kind: "event_match", + key: "type", + pattern: "m.call.invite", + }, + ], + actions: [ + "notify", + { + set_tweak: "sound", + value: "ring", + }, + { + set_tweak: "highlight", + value: false, + }, + ], + rule_id: ".m.rule.call", + default: true, + enabled: true, + }, + { + conditions: [ + { + kind: "event_match", + key: "type", + pattern: "m.room.message", + }, + { + kind: "room_member_count", + is: "2", + }, + ], + actions: [ + "notify", + { + set_tweak: "sound", + value: "TEST1", + }, + { + set_tweak: "highlight", + value: false, + }, + ], + rule_id: ".m.rule.room_one_to_one", + default: true, + enabled: true, + }, + { + conditions: [ + { + kind: "event_match", + key: "type", + pattern: "m.room.encrypted", + }, + { + kind: "room_member_count", + is: "2", + }, + ], + actions: [ + "notify", + { + set_tweak: "sound", + value: "TEST2", + }, + { + set_tweak: "highlight", + value: false, + }, + ], + rule_id: ".m.rule.encrypted_room_one_to_one", + default: true, + enabled: true, + }, + { + conditions: [ + { + kind: "event_match", + key: "type", + pattern: "m.room.message", + }, + ], + actions: ["dont_notify"], + rule_id: ".m.rule.message", + default: true, + enabled: true, + }, + { + conditions: [ + { + kind: "event_match", + key: "type", + pattern: "m.room.encrypted", + }, + ], + actions: ["dont_notify"], + rule_id: ".m.rule.encrypted", + default: true, + enabled: true, + }, + { + conditions: [ + { + kind: "event_match", + key: "type", + pattern: "im.vector.modular.widgets", + }, + { + kind: "event_match", + key: "content.type", + pattern: "jitsi", + }, + { + kind: "event_match", + key: "state_key", + pattern: "*", + }, + ], + actions: [ + "notify", + { + set_tweak: "highlight", + value: false, + }, + ], + rule_id: ".im.vector.jitsi", + default: true, + enabled: true, + }, + ] as IPushRule[], + }, + }); + + expect(pushRules.global.underride?.map((r) => r.rule_id)).toEqual([ + ".m.rule.call", + ".org.matrix.msc3914.rule.room.call", + ".m.rule.room_one_to_one", + ".m.rule.encrypted_room_one_to_one", + ".m.rule.message", + ".m.rule.encrypted", + ".im.vector.jitsi", + ]); + }); +}); diff --git a/src/pushprocessor.ts b/src/pushprocessor.ts index 162c36dc9bd..734a72751c3 100644 --- a/src/pushprocessor.ts +++ b/src/pushprocessor.ts @@ -25,8 +25,8 @@ import { ICallStartedPrefixCondition, IContainsDisplayNameCondition, IEventMatchCondition, - IEventPropertyIsCondition, IEventPropertyContainsCondition, + IEventPropertyIsCondition, IPushRule, IPushRules, IRoomMemberCountCondition, @@ -115,8 +115,14 @@ const DEFAULT_OVERRIDE_RULES: Record = { }, }; -const EXPECTED_DEFAULT_OVERRIDE_RULE_IDS = [ +// A special rule id for `EXPECTED_DEFAULT_OVERRIDE_RULE_IDS` and friends which denotes where user-defined rules live in the order. +const UserDefinedRules = Symbol("UserDefinedRules"); + +type OrderedRules = Array; + +const EXPECTED_DEFAULT_OVERRIDE_RULE_IDS: OrderedRules = [ RuleId.Master, + UserDefinedRules, RuleId.SuppressNotices, RuleId.InviteToSelf, RuleId.MemberEvent, @@ -151,8 +157,10 @@ const DEFAULT_UNDERRIDE_RULES: Record = { }, }; -const EXPECTED_DEFAULT_UNDERRIDE_RULE_IDS = [ +const EXPECTED_DEFAULT_UNDERRIDE_RULE_IDS: OrderedRules = [ + UserDefinedRules, RuleId.IncomingCall, + ".org.matrix.msc3914.rule.room.call", RuleId.EncryptedDM, RuleId.DM, RuleId.Message, @@ -162,35 +170,40 @@ 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. + * @param orderedRuleIds - the IDs of the expected push rules, in order. * * @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[], + orderedRuleIds: 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; - - function insertDefaultPushRule(ruleId: string): void { - if (ruleId in defaultRules) { - logger.warn(`Adding default global push rule ${ruleId}`); + // Split the incomingRules into defaults and custom + const incomingDefaultRules = incomingRules.filter((rule) => rule.default); + const incomingCustomRules = incomingRules.filter((rule) => !rule.default); + + function insertDefaultPushRule(ruleId: OrderedRules[number]): void { + if (ruleId === UserDefinedRules) { + // Re-insert any user-defined rules that were in `incomingRules` + newRules.push(...incomingCustomRules); + } else if (ruleId in defaultRules) { + logger.warn(`Adding default global ${kind} push rule ${ruleId}`); newRules.push(defaultRules[ruleId]); } else { - logger.warn(`Missing default global push rule ${ruleId}`); + logger.warn(`Missing default global ${kind} push rule ${ruleId}`); } } let nextExpectedRuleIdIndex = 0; const newRules: IPushRule[] = []; - for (const rule of incomingRules.slice(0, firstCustomRuleIndex)) { - const ruleIndex = defaultRuleIds.indexOf(rule.rule_id); + // Merge our expected rules (including the incoming custom rules) into the incoming default rules. + for (const rule of incomingDefaultRules) { + const ruleIndex = orderedRuleIds.indexOf(rule.rule_id); if (ruleIndex === -1) { // an unrecognised rule; copy it over newRules.push(rule); @@ -198,7 +211,7 @@ function mergeRulesWithDefaults( } while (ruleIndex > nextExpectedRuleIdIndex) { // insert new rules - const defaultRuleId = defaultRuleIds[nextExpectedRuleIdIndex]; + const defaultRuleId = orderedRuleIds[nextExpectedRuleIdIndex]; insertDefaultPushRule(defaultRuleId); nextExpectedRuleIdIndex += 1; } @@ -208,12 +221,10 @@ function mergeRulesWithDefaults( } // Now copy over any remaining default rules - for (const ruleId of defaultRuleIds.slice(nextExpectedRuleIdIndex)) { + for (const ruleId of orderedRuleIds.slice(nextExpectedRuleIdIndex)) { insertDefaultPushRule(ruleId); } - // Finally any non-default rules that were in `incomingRules` - newRules.push(...incomingRules.slice(firstCustomRuleIndex)); return newRules; } @@ -281,12 +292,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,