Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement MSC3758: a push rule condition to match event properties exactly #3179

Merged
merged 4 commits into from
Mar 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 74 additions & 1 deletion spec/unit/pushprocessor.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as utils from "../test-utils/test-utils";
import { IActionsObject, PushProcessor } from "../../src/pushprocessor";
import { ConditionKind, EventType, IContent, MatrixClient, MatrixEvent } from "../../src";
import { ConditionKind, EventType, IContent, MatrixClient, MatrixEvent, PushRuleActionName } from "../../src";

describe("NotificationService", function () {
const testUserId = "@ali:matrix.org";
Expand Down Expand Up @@ -507,6 +507,79 @@ describe("NotificationService", function () {
});
});

describe("Test exact event matching", () => {
it.each([
// Simple string matching.
{ value: "bar", eventValue: "bar", expected: true },
// Matches are case-sensitive.
{ value: "bar", eventValue: "BAR", expected: false },
// Matches must match the full string.
{ value: "bar", eventValue: "barbar", expected: false },
// Values should not be type-coerced.
{ value: "bar", eventValue: true, expected: false },
{ value: "bar", eventValue: 1, expected: false },
{ value: "bar", eventValue: false, expected: false },
// Boolean matching.
{ value: true, eventValue: true, expected: true },
{ value: false, eventValue: false, expected: true },
// Types should not be coerced.
{ value: true, eventValue: "true", expected: false },
{ value: true, eventValue: 1, expected: false },
{ value: false, eventValue: null, expected: false },
// Null matching.
{ value: null, eventValue: null, expected: true },
// Types should not be coerced
{ value: null, eventValue: false, expected: false },
{ value: null, eventValue: 0, expected: false },
{ value: null, eventValue: "", expected: false },
{ value: null, eventValue: undefined, expected: false },
// Compound values should never be matched.
{ value: "bar", eventValue: ["bar"], expected: false },
{ value: "bar", eventValue: { bar: true }, expected: false },
{ value: true, eventValue: [true], expected: false },
{ value: true, eventValue: { true: true }, expected: false },
{ value: null, eventValue: [], expected: false },
{ value: null, eventValue: {}, expected: false },
])("test $value against $eventValue", ({ value, eventValue, expected }) => {
matrixClient.pushRules! = {
global: {
override: [
{
actions: [PushRuleActionName.Notify],
conditions: [
{
kind: ConditionKind.EventPropertyIs,
key: "content.foo",
value: value,
},
],
default: true,
enabled: true,
rule_id: ".m.rule.test",
},
],
},
};

testEvent = utils.mkEvent({
type: "m.room.message",
room: testRoomId,
user: "@alfred:localhost",
event: true,
content: {
foo: eventValue,
},
});

const actions = pushProcessor.actionsForEvent(testEvent);
if (expected) {
expect(actions?.notify).toBeTruthy();
} else {
expect(actions?.notify).toBeFalsy();
}
});
});

it.each([
// The properly escaped key works.
{ key: "content.m\\.test.foo", pattern: "bar", expected: true },
Expand Down
9 changes: 9 additions & 0 deletions src/@types/PushRules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export function isDmMemberCountCondition(condition: AnyMemberCountCondition): bo

export enum ConditionKind {
EventMatch = "event_match",
EventPropertyIs = "event_property_is",
ContainsDisplayName = "contains_display_name",
RoomMemberCount = "room_member_count",
SenderNotificationPermission = "sender_notification_permission",
Expand All @@ -77,9 +78,16 @@ export interface IPushRuleCondition<N extends ConditionKind | string> {
export interface IEventMatchCondition extends IPushRuleCondition<ConditionKind.EventMatch> {
key: string;
pattern?: string;
// Note that value property is an optimization for patterns which do not do
// any globbing and when the key is not "content.body".
value?: string;
}

export interface IEventPropertyIsCondition extends IPushRuleCondition<ConditionKind.EventPropertyIs> {
key: string;
value: string | boolean | null | number;
}

export interface IContainsDisplayNameCondition extends IPushRuleCondition<ConditionKind.ContainsDisplayName> {
// no additional fields
}
Expand All @@ -105,6 +113,7 @@ export interface ICallStartedPrefixCondition extends IPushRuleCondition<Conditio
// IPushRuleCondition<Exclude<string, ConditionKind>> unfortunately does not resolve this at the time of writing.
export type PushRuleCondition =
| IEventMatchCondition
| IEventPropertyIsCondition
| IContainsDisplayNameCondition
| IRoomMemberCountCondition
| ISenderNotificationPermissionCondition
Expand Down
36 changes: 33 additions & 3 deletions src/pushprocessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
ICallStartedPrefixCondition,
IContainsDisplayNameCondition,
IEventMatchCondition,
IEventPropertyIsCondition,
IPushRule,
IPushRules,
IRoomMemberCountCondition,
Expand Down Expand Up @@ -337,6 +338,8 @@ export class PushProcessor {
switch (cond.kind) {
case ConditionKind.EventMatch:
return this.eventFulfillsEventMatchCondition(cond, ev);
case ConditionKind.EventPropertyIs:
return this.eventFulfillsEventPropertyIsCondition(cond, ev);
case ConditionKind.ContainsDisplayName:
return this.eventFulfillsDisplayNameCondition(cond, ev);
case ConditionKind.RoomMemberCount:
Expand Down Expand Up @@ -435,6 +438,13 @@ export class PushProcessor {
return content.body.search(pat) > -1;
}

/**
* Check whether the given event matches the push rule condition by fetching
* the property from the event and comparing against the condition's glob-based
* pattern.
* @param cond - The push rule condition to check for a match.
* @param ev - The event to check for a match.
*/
private eventFulfillsEventMatchCondition(cond: IEventMatchCondition, ev: MatrixEvent): boolean {
if (!cond.key) {
return false;
Expand All @@ -445,6 +455,9 @@ export class PushProcessor {
return false;
}

// XXX This does not match in a case-insensitive manner.
//
// See https://spec.matrix.org/v1.5/client-server-api/#conditions-1
if (cond.value) {
return cond.value === val;
}
Expand All @@ -461,6 +474,20 @@ export class PushProcessor {
return !!val.match(regex);
}

/**
* Check whether the given event matches the push rule condition by fetching
* the property from the event and comparing exactly against the condition's
* value.
* @param cond - The push rule condition to check for a match.
* @param ev - The event to check for a match.
*/
private eventFulfillsEventPropertyIsCondition(cond: IEventPropertyIsCondition, ev: MatrixEvent): boolean {
if (!cond.key || cond.value === undefined) {
return false;
}
return cond.value === this.valueForDottedKey(cond.key, ev);
}

private eventFulfillsCallStartedCondition(
_cond: ICallStartedCondition | ICallStartedPrefixCondition,
ev: MatrixEvent,
Expand Down Expand Up @@ -578,10 +605,13 @@ export class PushProcessor {
}

for (; currentIndex < parts.length; ++currentIndex) {
const thisPart = parts[currentIndex];
if (isNullOrUndefined(val[thisPart])) {
return null;
// The previous iteration resulted in null or undefined, bail (and
// avoid the type error of attempting to retrieve a property).
if (isNullOrUndefined(val)) {
return undefined;
}

const thisPart = parts[currentIndex];
val = val[thisPart];
}
return val;
Expand Down