Annotate events with executed push rule (#3284)

* unit test paginating /notifications

* add push rule to event

* 1% more test coverage

spec/unit/matrix-client.spec.ts

@@ -51,6 +51,11 @@ import {
     Method,
     Room,
     EventTimelineSet,
+    PushRuleActionName,
+    TweakName,
+    RuleId,
+    IPushRule,
+    ConditionKind,
 } from "../../src";
 import { supportsMatrixCall } from "../../src/webrtc/call";
 import { makeBeaconEvent } from "../test-utils/beacon";
@@ -2751,7 +2756,7 @@ describe("MatrixClient", function () {
                 actions: ["notify"],
                 room_id: "__proto__",
                 event: testUtils.mkMessage({
-                    user: userId,
+                    user: "@villain:server.org",
                     room: "!roomId:server.org",
                     msg: "I am nefarious",
                 }),
@@ -2762,11 +2767,12 @@ describe("MatrixClient", function () {
 
             const goodNotification = {
                 actions: ["notify"],
-                room_id: "!roomId:server.org",
+                room_id: "!favouriteRoom:server.org",
-                event: testUtils.mkMessage({
+                event: new MatrixEvent({
-                    user: userId,
+                    sender: "@bob:server.org",
-                    room: "!roomId:server.org",
+                    room_id: "!roomId:server.org",
-                    msg: "I am nice",
+                    type: "m.call.invite",
+                    content: {},
                 }),
                 profile_tag: null,
                 read: true,
@@ -2774,12 +2780,12 @@ describe("MatrixClient", function () {
             };
 
             const highlightNotification = {
-                actions: ["notify", { set_tweak: "highlight" }],
+                actions: ["notify", { set_tweak: "highlight", value: true }],
                 room_id: "!roomId:server.org",
                 event: testUtils.mkMessage({
-                    user: userId,
+                    user: "@bob:server.org",
                     room: "!roomId:server.org",
-                    msg: "I am highlighted",
+                    msg: "I am highlighted banana",
                 }),
                 profile_tag: null,
                 read: true,
@@ -2795,6 +2801,41 @@ describe("MatrixClient", function () {
                 httpLookups = [response];
             };
 
+            const callRule: IPushRule = {
+                actions: [PushRuleActionName.Notify],
+                conditions: [
+                    {
+                        kind: ConditionKind.EventMatch,
+                        key: "type",
+                        pattern: "m.call.invite",
+                    },
+                ],
+                default: true,
+                enabled: true,
+                rule_id: ".m.rule.call",
+            };
+            const masterRule: IPushRule = {
+                actions: [PushRuleActionName.DontNotify],
+                conditions: [],
+                default: true,
+                enabled: false,
+                rule_id: RuleId.Master,
+            };
+            const bananaRule = {
+                actions: [PushRuleActionName.Notify, { set_tweak: TweakName.Highlight, value: true }],
+                pattern: "banana",
+                rule_id: "banana",
+                default: false,
+                enabled: true,
+            } as IPushRule;
+            const pushRules = {
+                global: {
+                    underride: [callRule],
+                    override: [masterRule],
+                    content: [bananaRule],
+                },
+            };
+
             beforeEach(() => {
                 makeClient();
 
@@ -2807,6 +2848,8 @@ describe("MatrixClient", function () {
                 client.setNotifTimelineSet(notifTimelineSet);
 
                 setNotifsResponse();
+
+                client.setPushRules(pushRules);
             });
 
             it("should throw when trying to paginate forwards", async () => {
@@ -2839,7 +2882,7 @@ describe("MatrixClient", function () {
                 expect(timelineEvents.length).toEqual(2);
             });
 
-            it("sets push actions on events and add to timeline", async () => {
+            it("sets push details on events and add to timeline", async () => {
                 setNotifsResponse([goodNotification, highlightNotification]);
 
                 const timelineSet = client.getNotifTimelineSet()!;
@@ -2853,9 +2896,15 @@ describe("MatrixClient", function () {
                         highlight: true,
                     },
                 });
+                expect(highlightEvent.getPushDetails().rule).toEqual({
+                    ...bananaRule,
+                    kind: "content",
+                });
                 expect(goodEvent.getPushActions()).toEqual({
                     notify: true,
-                    tweaks: {},
+                    tweaks: {
+                        highlight: false,
+                    },
                 });
             });
         });

spec/unit/models/event.spec.ts

@@ -17,6 +17,7 @@ limitations under the License.
 import { MatrixEvent, MatrixEventEvent } from "../../../src/models/event";
 import { emitPromise } from "../../test-utils/test-utils";
 import { Crypto, IEventDecryptionResult } from "../../../src/crypto";
+import { IAnnotatedPushRule, PushRuleActionName, TweakName } from "../../../src";
 
 describe("MatrixEvent", () => {
     it("should create copies of itself", () => {
@@ -216,4 +217,95 @@ describe("MatrixEvent", () => {
             expect(encryptedEvent.replyEventId).toBeUndefined();
         });
     });
+
+    describe("push details", () => {
+        const pushRule = {
+            actions: [PushRuleActionName.Notify, { set_tweak: TweakName.Highlight, value: true }],
+            pattern: "banana",
+            rule_id: "banana",
+            kind: "override",
+            default: false,
+            enabled: true,
+        } as IAnnotatedPushRule;
+        describe("setPushActions()", () => {
+            it("sets actions on event", () => {
+                const actions = { notify: false, tweaks: {} };
+                const event = new MatrixEvent({
+                    type: "com.example.test",
+                    content: {
+                        isTest: true,
+                    },
+                });
+                event.setPushActions(actions);
+
+                expect(event.getPushActions()).toBe(actions);
+            });
+
+            it("sets actions to undefined", () => {
+                const event = new MatrixEvent({
+                    type: "com.example.test",
+                    content: {
+                        isTest: true,
+                    },
+                });
+                event.setPushActions(null);
+
+                // undefined is set on state
+                expect(event.getPushDetails().actions).toBe(undefined);
+                // but pushActions getter returns null when falsy
+                expect(event.getPushActions()).toBe(null);
+            });
+
+            it("clears existing push rule", () => {
+                const prevActions = { notify: true, tweaks: { highlight: true } };
+                const actions = { notify: false, tweaks: {} };
+                const event = new MatrixEvent({
+                    type: "com.example.test",
+                    content: {
+                        isTest: true,
+                    },
+                });
+                event.setPushDetails(prevActions, pushRule);
+
+                event.setPushActions(actions);
+
+                // rule is not in event push cache
+                expect(event.getPushDetails()).toEqual({ actions });
+            });
+        });
+
+        describe("setPushDetails()", () => {
+            it("sets actions and rule on event", () => {
+                const actions = { notify: false, tweaks: {} };
+                const event = new MatrixEvent({
+                    type: "com.example.test",
+                    content: {
+                        isTest: true,
+                    },
+                });
+                event.setPushDetails(actions, pushRule);
+
+                expect(event.getPushDetails()).toEqual({
+                    actions,
+                    rule: pushRule,
+                });
+            });
+            it("clears existing push rule", () => {
+                const prevActions = { notify: true, tweaks: { highlight: true } };
+                const actions = { notify: false, tweaks: {} };
+                const event = new MatrixEvent({
+                    type: "com.example.test",
+                    content: {
+                        isTest: true,
+                    },
+                });
+                event.setPushDetails(prevActions, pushRule);
+
+                event.setPushActions(actions);
+
+                // rule is not in event push cache
+                expect(event.getPushDetails()).toEqual({ actions });
+            });
+        });
+    });
 });

spec/unit/pushprocessor.spec.ts

@@ -1,6 +1,7 @@
 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 { mockClientMethodsUser } from "../test-utils/client";
 
 describe("NotificationService", function () {
     const testUserId = "@ali:matrix.org";
@@ -45,9 +46,7 @@ describe("NotificationService", function () {
                 },
             };
         },
-        credentials: {
+        ...mockClientMethodsUser(testUserId),
-            userId: testUserId,
-        },
         supportsIntentionalMentions: () => true,
         pushRules: {
             device: {},

src/client.ts

@@ -30,6 +30,7 @@ import {
     MatrixEvent,
     MatrixEventEvent,
     MatrixEventHandlerMap,
+    PushDetails,
 } from "./models/event";
 import { StubStore } from "./store/stub";
 import { CallEvent, CallEventHandlerMap, createNewMatrixCall, MatrixCall, supportsMatrixCall } from "./webrtc/call";
@@ -5385,11 +5386,28 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
      */
     public getPushActionsForEvent(event: MatrixEvent, forceRecalculate = false): IActionsObject | null {
         if (!event.getPushActions() || forceRecalculate) {
-            event.setPushActions(this.pushProcessor.actionsForEvent(event));
+            const { actions, rule } = this.pushProcessor.actionsAndRuleForEvent(event);
+            event.setPushDetails(actions, rule);
         }
         return event.getPushActions();
     }
 
+    /**
+     * Obtain a dict of actions which should be performed for this event according
+     * to the push rules for this user.  Caches the dict on the event.
+     * @param event - The event to get push actions for.
+     * @param forceRecalculate - forces to recalculate actions for an event
+     * Useful when an event just got decrypted
+     * @returns A dict of actions to perform.
+     */
+    public getPushDetailsForEvent(event: MatrixEvent, forceRecalculate = false): PushDetails | null {
+        if (!event.getPushDetails() || forceRecalculate) {
+            const { actions, rule } = this.pushProcessor.actionsAndRuleForEvent(event);
+            event.setPushDetails(actions, rule);
+        }
+        return event.getPushDetails();
+    }
+
     /**
      * @param info - The kind of info to set (e.g. 'avatar_url')
      * @param data - The JSON object to set.
@@ -6064,7 +6082,11 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
                     for (let i = 0; i < res.notifications.length; i++) {
                         const notification = res.notifications[i];
                         const event = this.getEventMapper()(notification.event);
-                        event.setPushActions(PushProcessor.actionListToActionsObject(notification.actions));
+
+                        // @TODO(kerrya) reprocessing every notification is ugly
+                        // remove if we get server MSC3994 support
+                        this.getPushDetailsForEvent(event, true);
+
                         event.event.room_id = notification.room_id; // XXX: gutwrenching
                         matrixEvents[i] = event;
                     }

src/models/event.ts

@@ -37,6 +37,7 @@ import { EventStatus } from "./event-status";
 import { DecryptionError } from "../crypto/algorithms";
 import { CryptoBackend } from "../common-crypto/CryptoBackend";
 import { WITHHELD_MESSAGES } from "../crypto/OlmDevice";
+import { IAnnotatedPushRule } from "../@types/PushRules";
 
 export { EventStatus } from "./event-status";
 
@@ -121,6 +122,11 @@ export interface IMentions {
     room?: boolean;
 }
 
+export interface PushDetails {
+    rule?: IAnnotatedPushRule;
+    actions?: IActionsObject;
+}
+
 /**
  * When an event is a visibility change event, as per MSC3531,
  * the visibility change implied by the event.
@@ -220,7 +226,8 @@ export type MatrixEventHandlerMap = {
 } & Pick<ThreadEventHandlerMap, ThreadEvent.Update>;
 
 export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, MatrixEventHandlerMap> {
-    private pushActions: IActionsObject | null = null;
+    // applied push rule and action for this event
+    private pushDetails: PushDetails = {};
     private _replacingEvent: MatrixEvent | null = null;
     private _localRedactionEvent: MatrixEvent | null = null;
     private _isCancelled = false;
@@ -888,7 +895,7 @@ export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, Mat
             // highlighting when the user's name is mentioned rely on this happening. We also want
             // to set the push actions before emitting so that any notification listeners don't
             // pick up the wrong contents.
-            this.setPushActions(null);
+            this.setPushDetails();
 
             if (options.emit !== false) {
                 this.emit(MatrixEventEvent.Decrypted, this, err);
@@ -1241,16 +1248,42 @@ export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, Mat
      * @returns push actions
      */
     public getPushActions(): IActionsObject | null {
-        return this.pushActions;
+        return this.pushDetails.actions || null;
+    }
+
+    /**
+     * Get the push details, if known, for this event
+     *
+     * @returns push actions
+     */
+    public getPushDetails(): PushDetails {
+        return this.pushDetails;
     }
 
     /**
      * Set the push actions for this event.
+     * Clears rule from push details if present
+     * @deprecated use `setPushDetails`
      *
      * @param pushActions - push actions
      */
     public setPushActions(pushActions: IActionsObject | null): void {
-        this.pushActions = pushActions;
+        this.pushDetails = {
+            actions: pushActions || undefined,
+        };
+    }
+
+    /**
+     * Set the push details for this event.
+     *
+     * @param pushActions - push actions
+     * @param rule - the executed push rule
+     */
+    public setPushDetails(pushActions?: IActionsObject, rule?: IAnnotatedPushRule): void {
+        this.pushDetails = {
+            actions: pushActions,
+            rule,
+        };
     }
 
     /**

src/pushprocessor.ts

@@ -688,17 +688,24 @@ export class PushProcessor {
         if (!rulesets) {
             return null;
         }
-        if (ev.getSender() === this.client.credentials.userId) {
+
+        if (ev.getSender() === this.client.getSafeUserId()) {
             return null;
         }
 
         return this.matchingRuleFromKindSet(ev, rulesets.global);
     }
 
-    private pushActionsForEventAndRulesets(ev: MatrixEvent, rulesets?: IPushRules): IActionsObject {
+    private pushActionsForEventAndRulesets(
+        ev: MatrixEvent,
+        rulesets?: IPushRules,
+    ): {
+        actions?: IActionsObject;
+        rule?: IAnnotatedPushRule;
+    } {
         const rule = this.matchingRuleForEventWithRulesets(ev, rulesets);
         if (!rule) {
-            return {} as IActionsObject;
+            return {};
         }
 
         const actionObj = PushProcessor.actionListToActionsObject(rule.actions);
@@ -710,7 +717,7 @@ export class PushProcessor {
             actionObj.tweaks.highlight = rule.kind == PushRuleKind.ContentSpecific;
         }
 
-        return actionObj;
+        return { actions: actionObj, rule };
     }
 
     public ruleMatchesEvent(rule: Partial<IPushRule> & Pick<IPushRule, "conditions">, ev: MatrixEvent): boolean {
@@ -732,6 +739,14 @@ export class PushProcessor {
      * Get the user's push actions for the given event
      */
     public actionsForEvent(ev: MatrixEvent): IActionsObject {
+        const { actions } = this.pushActionsForEventAndRulesets(ev, this.client.pushRules);
+        return actions || ({} as IActionsObject);
+    }
+
+    public actionsAndRuleForEvent(ev: MatrixEvent): {
+        actions?: IActionsObject;
+        rule?: IAnnotatedPushRule;
+    } {
         return this.pushActionsForEventAndRulesets(ev, this.client.pushRules);
     }