Dima/matrix-js-sdk
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #4135 from matrix-org/t3chguy/fix/27173

Browse Source 
Fix merging of default push rules
This commit is contained in:
David Baker
2024-03-28 16:23:58 +00:00
committed by GitHub
parent 706002cdcb 467b49a0dc
commit 78a225795b
2 changed files with 352 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
spec/unit/pushprocessor.spec.ts
+320 -38
View File
@@ -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",
]);
});
});
src/pushprocessor.ts
+32 -19
View File
@@ -25,8 +25,8 @@ import {
ICallStartedPrefixCondition,
IContainsDisplayNameCondition,
IEventMatchCondition,
IEventPropertyIsCondition,
IEventPropertyContainsCondition,
IEventPropertyIsCondition,
IPushRule,
IPushRules,
IRoomMemberCountCondition,
@@ -115,8 +115,14 @@ const DEFAULT_OVERRIDE_RULES: Record<string, IPushRule> = {
},
};
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<string | typeof UserDefinedRules>;
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<string, IPushRule> = {
},
};
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<string, IPushRule>,
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;
// Split the incomingRules into defaults and custom
const incomingDefaultRules = incomingRules.filter((rule) => rule.default);
const incomingCustomRules = incomingRules.filter((rule) => !rule.default);
function insertDefaultPushRule(ruleId: string): void {
if (ruleId in defaultRules) {
logger.warn(`Adding default global push rule ${ruleId}`);
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,