Implement MSC3758: a push rule condition to match event properties ex…

…actly (matrix-org#3179)

* Add some comments.

* Support MSC3758 to exactly match values in push rule conditions.

* Update to stable prefix.

spec/unit/pushprocessor.spec.ts

@@ -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";
@@ -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 },

src/@types/PushRules.ts

@@ -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",
@@ -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
 }
@@ -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

src/pushprocessor.ts

@@ -25,6 +25,7 @@ import {
     ICallStartedPrefixCondition,
     IContainsDisplayNameCondition,
     IEventMatchCondition,
+    IEventPropertyIsCondition,
     IPushRule,
     IPushRules,
     IRoomMemberCountCondition,
@@ -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:
@@ -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;
@@ -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;
         }
@@ -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,
@@ -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;