tests(threads): add an exhaustive test to check for all notification mode combinations

Author: bnjbvr

crates/matrix-sdk/src/notification_settings/mod.rs

@@ -571,6 +571,12 @@ impl NotificationSettings {
         }
         Ok(())
     }
+
+    /// Returns the inner ruleset currently known by this
+    /// [`NotificationSettings`] instance.
+    pub async fn ruleset(&self) -> Ruleset {
+        self.rules.read().await.ruleset.clone()
+    }
 }
 
 // The http mocking library is not supported for wasm32

crates/matrix-sdk/src/room/mod.rs

@@ -127,7 +127,7 @@ use ruma::{
         StaticStateEventContent, SyncStateEvent,
     },
     int,
-    push::{Action, PushConditionRoomCtx, Ruleset},
+    push::{Action, AnyPushRuleRef, PushConditionRoomCtx, Ruleset},
     serde::Raw,
     time::Instant,
     EventId, Int, MatrixToUri, MatrixUri, MxcUri, OwnedEventId, OwnedRoomId, OwnedServerName,
@@ -226,6 +226,49 @@ impl PushContext {
     pub async fn for_event<T>(&self, event: &Raw<T>) -> Vec<Action> {
         self.push_rules.get_actions(event, &self.push_condition_room_ctx).await.to_owned()
     }
+
+    /// Compute the push rules for a given event, with extra logging to help
+    /// debugging.
+    #[doc(hidden)]
+    #[instrument(skip_all)]
+    pub async fn traced_for_event<T>(&self, event: &Raw<T>) -> Vec<Action> {
+        let rules = self
+            .push_rules
+            .iter()
+            .filter_map(|r| {
+                if !r.enabled() {
+                    return None;
+                }
+
+                let simplified_action = if r.actions().is_empty() { "inhibit" } else { "notify" };
+
+                let conditions = match r {
+                    AnyPushRuleRef::Override(r) => {
+                        format!("{:?}", r.conditions)
+                    }
+                    AnyPushRuleRef::Content(r) => format!("content-body-match:{}", r.pattern),
+                    AnyPushRuleRef::Room(r) => format!("room-match:{}", r.rule_id),
+                    AnyPushRuleRef::Sender(r) => format!("sender-match:{}", r.rule_id),
+                    AnyPushRuleRef::Underride(r) => format!("{:?}", r.conditions),
+                    _ => format!("<unknown push rule kind>"),
+                };
+
+                Some(format!("- {}: {conditions} => {simplified_action}", r.rule_id(),))
+            })
+            .collect::<Vec<_>>()
+            .join("\n");
+        trace!("rules:\n\n{rules}\n\n");
+
+        let found = self.push_rules.get_match(event, &self.push_condition_room_ctx).await;
+
+        if let Some(found) = found {
+            trace!("rule {} matched", found.rule_id());
+            found.actions().to_owned()
+        } else {
+            trace!("no match");
+            Vec::new()
+        }
+    }
 }
 
 macro_rules! make_media_type {

crates/matrix-sdk/src/test_utils/mocks/mod.rs

@@ -1385,13 +1385,68 @@ impl MatrixMockServer {
         self.mock_endpoint(mock, EnablePushRuleEndpoint).expect_default_access_token()
     }
 
+    /// Create a prebuilt mock for the endpoint used to set push rules actions.
+    pub fn mock_set_push_rules_actions(
+        &self,
+        kind: RuleKind,
+        rule_id: PushRuleIdSpec<'_>,
+    ) -> MockEndpoint<'_, SetPushRulesActionsEndpoint> {
+        let rule_id = rule_id.to_path();
+        let mock = Mock::given(method("PUT")).and(path_regex(format!(
+            "^/_matrix/client/v3/pushrules/global/{kind}/{rule_id}/actions",
+        )));
+        self.mock_endpoint(mock, SetPushRulesActionsEndpoint).expect_default_access_token()
+    }
+
+    /// Create a prebuilt mock for the endpoint used to set push rules.
+    pub fn mock_set_push_rules(
+        &self,
+        kind: RuleKind,
+        rule_id: PushRuleIdSpec<'_>,
+    ) -> MockEndpoint<'_, SetPushRulesEndpoint> {
+        let rule_id = rule_id.to_path();
+        let mock = Mock::given(method("PUT"))
+            .and(path_regex(format!("^/_matrix/client/v3/pushrules/global/{kind}/{rule_id}$",)));
+        self.mock_endpoint(mock, SetPushRulesEndpoint).expect_default_access_token()
+    }
+
+    /// Create a prebuilt mock for the endpoint used to delete push rules.
+    pub fn mock_delete_push_rules(
+        &self,
+        kind: RuleKind,
+        rule_id: PushRuleIdSpec<'_>,
+    ) -> MockEndpoint<'_, DeletePushRulesEndpoint> {
+        let rule_id = rule_id.to_path();
+        let mock = Mock::given(method("DELETE"))
+            .and(path_regex(format!("^/_matrix/client/v3/pushrules/global/{kind}/{rule_id}$",)));
+        self.mock_endpoint(mock, DeletePushRulesEndpoint).expect_default_access_token()
+    }
+
     /// Create a prebuilt mock for the federation version endpoint.
     pub fn mock_federation_version(&self) -> MockEndpoint<'_, FederationVersionEndpoint> {
         let mock = Mock::given(method("GET")).and(path("/_matrix/federation/v1/version"));
         self.mock_endpoint(mock, FederationVersionEndpoint)
     }
 }
 
+/// A specification for a push rule ID.
+pub enum PushRuleIdSpec<'a> {
+    /// A precise rule ID.
+    Some(&'a str),
+    /// Any rule ID should match.
+    Any,
+}
+
+impl<'a> PushRuleIdSpec<'a> {
+    /// Convert this [`PushRuleIdSpec`] to a path.
+    pub fn to_path(&self) -> &str {
+        match self {
+            PushRuleIdSpec::Some(id) => id,
+            PushRuleIdSpec::Any => "[^/]*",
+        }
+    }
+}
+
 /// Parameter to [`MatrixMockServer::sync_room`].
 pub enum AnyRoomBuilder {
     /// A room we've been invited to.
@@ -4027,6 +4082,39 @@ impl<'a> MockEndpoint<'a, EnablePushRuleEndpoint> {
     }
 }
 
+/// A prebuilt mock for `PUT
+/// /_matrix/client/v3/pushrules/global/{kind}/{ruleId}/actions`.
+pub struct SetPushRulesActionsEndpoint;
+
+impl<'a> MockEndpoint<'a, SetPushRulesActionsEndpoint> {
+    /// Returns a successful empty JSON response.
+    pub fn ok(self) -> MatrixMock<'a> {
+        self.ok_empty_json()
+    }
+}
+
+/// A prebuilt mock for `PUT
+/// /_matrix/client/v3/pushrules/global/{kind}/{ruleId}`.
+pub struct SetPushRulesEndpoint;
+
+impl<'a> MockEndpoint<'a, SetPushRulesEndpoint> {
+    /// Returns a successful empty JSON response.
+    pub fn ok(self) -> MatrixMock<'a> {
+        self.ok_empty_json()
+    }
+}
+
+/// A prebuilt mock for `DELETE
+/// /_matrix/client/v3/pushrules/global/{kind}/{ruleId}`.
+pub struct DeletePushRulesEndpoint;
+
+impl<'a> MockEndpoint<'a, DeletePushRulesEndpoint> {
+    /// Returns a successful empty JSON response.
+    pub fn ok(self) -> MatrixMock<'a> {
+        self.ok_empty_json()
+    }
+}
+
 /// A prebuilt mock for the federation version endpoint.
 pub struct FederationVersionEndpoint;
 

crates/matrix-sdk/tests/integration/room/thread.rs

@@ -1,7 +1,11 @@
 use assert_matches2::assert_matches;
-use matrix_sdk::{room::ThreadSubscription, test_utils::mocks::MatrixMockServer};
+use matrix_sdk::{
+    notification_settings::RoomNotificationMode,
+    room::ThreadSubscription,
+    test_utils::mocks::{MatrixMockServer, PushRuleIdSpec},
+};
 use matrix_sdk_test::{async_test, event_factory::EventFactory, JoinedRoomBuilder, ALICE};
-use ruma::{event_id, owned_event_id, room_id};
+use ruma::{event_id, owned_event_id, push::RuleKind, room_id};
 
 #[async_test]
 async fn test_subscribe_thread() {
@@ -238,3 +242,187 @@ async fn test_thread_push_rule_is_triggered_for_subscribed_threads() {
     let actions = push_context.for_event(&event).await;
     assert!(actions.is_empty());
 }
+
+#[async_test]
+async fn test_thread_push_rules_and_notification_modes() {
+    // This test checks that, given a combination of a global notification mode, and
+    // a room notification mode, we do get notifications for thread events according
+    // to the subscriptions.
+
+    let server = MatrixMockServer::new().await;
+    let client = server
+        .client_builder()
+        .on_builder(|builder| {
+            builder.with_threading_support(matrix_sdk::ThreadingSupport::Enabled {
+                with_subscriptions: true,
+            })
+        })
+        .build()
+        .await;
+
+    let room_id = room_id!("!test:example.org");
+    let f = EventFactory::new().room(room_id).sender(*ALICE);
+    // Make it so that the client has a member event for the current user.
+    let room = server
+        .sync_room(
+            &client,
+            JoinedRoomBuilder::new(room_id).add_state_event(f.member(client.user_id().unwrap())),
+        )
+        .await;
+
+    // Sanity check: we can get a push context.
+    room.push_context()
+        .await
+        .expect("getting a push context works")
+        .expect("the push context should exist");
+
+    // Mock push rules endpoints, allowing any modification to any rule.
+    server.mock_set_push_rules_actions(RuleKind::Underride, PushRuleIdSpec::Any).ok().mount().await;
+    server.mock_set_push_rules_actions(RuleKind::Room, PushRuleIdSpec::Any).ok().mount().await;
+    server.mock_set_push_rules(RuleKind::Underride, PushRuleIdSpec::Any).ok().mount().await;
+    server.mock_set_push_rules(RuleKind::Room, PushRuleIdSpec::Any).ok().mount().await;
+    server.mock_set_push_rules(RuleKind::Override, PushRuleIdSpec::Any).ok().mount().await;
+    server.mock_delete_push_rules(RuleKind::Room, PushRuleIdSpec::Any).ok().mount().await;
+    server.mock_delete_push_rules(RuleKind::Override, PushRuleIdSpec::Any).ok().mount().await;
+
+    // Given a thread I'm subscribed to,
+    let thread_root_id = owned_event_id!("$root");
+
+    server
+        .mock_get_thread_subscription()
+        .match_room_id(room_id.to_owned())
+        .match_thread_id(thread_root_id.clone())
+        .ok(true)
+        .mount()
+        .await;
+
+    // Given an event in the thread I'm subscribed to,
+    let event =
+        f.text_msg("hello to you too!").in_thread(&thread_root_id, &thread_root_id).into_raw_sync();
+
+    // If global mode = AllMessages,
+    let settings = client.notification_settings().await;
+
+    let is_encrypted = false;
+    let is_one_to_one = false;
+    settings
+        .set_default_room_notification_mode(
+            is_encrypted.into(),
+            is_one_to_one.into(),
+            RoomNotificationMode::AllMessages,
+        )
+        .await
+        .unwrap();
+
+    // If room mode = AllMessages,
+    settings.set_room_notification_mode(room_id, RoomNotificationMode::AllMessages).await.unwrap();
+    // Ack the push rules change via sync, for it to be applied in the push context.
+    let ruleset = settings.ruleset().await;
+    server
+        .mock_sync()
+        .ok_and_run(&client, |builder| {
+            builder.add_global_account_data(f.push_rules(ruleset));
+        })
+        .await;
+
+    // The thread event will trigger a notification.
+    let actions = room.push_context().await.unwrap().unwrap().traced_for_event(&event).await;
+    assert!(actions.iter().any(|action| action.should_notify()));
+
+    // If room mode = mentions and keywords only,
+    settings
+        .set_room_notification_mode(room_id, RoomNotificationMode::MentionsAndKeywordsOnly)
+        .await
+        .unwrap();
+    let ruleset = settings.ruleset().await;
+    server
+        .mock_sync()
+        .ok_and_run(&client, |builder| {
+            builder.add_global_account_data(f.push_rules(ruleset));
+        })
+        .await;
+
+    // The thread event will trigger a notification.
+    let actions = room.push_context().await.unwrap().unwrap().traced_for_event(&event).await;
+    // TODO: unexpected! this should trigger a thread mention
+    //assert!(actions.iter().any(|action| action.should_notify()));
+    assert!(!actions.iter().any(|action| action.should_notify()));
+
+    // If room mode = mute,
+    settings.set_room_notification_mode(room_id, RoomNotificationMode::Mute).await.unwrap();
+    let ruleset = settings.ruleset().await;
+    server
+        .mock_sync()
+        .ok_and_run(&client, |builder| {
+            builder.add_global_account_data(f.push_rules(ruleset));
+        })
+        .await;
+
+    // The thread event will trigger a notification.
+    let actions = room.push_context().await.unwrap().unwrap().traced_for_event(&event).await;
+    // TODO: unexpected! this should trigger a thread mention
+    //assert!(actions.iter().any(|action| action.should_notify()));
+    assert!(!actions.iter().any(|action| action.should_notify()));
+
+    // Now, if global mode = mentions only,
+    settings
+        .set_default_room_notification_mode(
+            is_encrypted.into(),
+            is_one_to_one.into(),
+            RoomNotificationMode::MentionsAndKeywordsOnly,
+        )
+        .await
+        .unwrap();
+
+    // If room mode = AllMessages,
+    settings.set_room_notification_mode(room_id, RoomNotificationMode::AllMessages).await.unwrap();
+    // Ack the push rules change via sync, for it to be applied in the push context.
+    let ruleset = settings.ruleset().await;
+    server
+        .mock_sync()
+        .ok_and_run(&client, |builder| {
+            builder.add_global_account_data(f.push_rules(ruleset));
+        })
+        .await;
+
+    // The thread event will trigger a notification.
+    let actions = room.push_context().await.unwrap().unwrap().traced_for_event(&event).await;
+    assert!(actions.iter().any(|action| action.should_notify()));
+
+    // If room mode = Mentions only,
+    settings
+        .set_room_notification_mode(room_id, RoomNotificationMode::MentionsAndKeywordsOnly)
+        .await
+        .unwrap();
+    // Ack the push rules change via sync, for it to be applied in the push context.
+    let ruleset = settings.ruleset().await;
+    server
+        .mock_sync()
+        .ok_and_run(&client, |builder| {
+            builder.add_global_account_data(f.push_rules(ruleset));
+        })
+        .await;
+
+    // The thread event will trigger a notification.
+    let actions = room.push_context().await.unwrap().unwrap().traced_for_event(&event).await;
+    // TODO: unexpected! this should trigger a thread mention
+    //assert!(actions.iter().any(|action| action.should_notify()));
+    assert!(!actions.iter().any(|action| action.should_notify()));
+
+    // If room mode = mute,
+    settings.set_room_notification_mode(room_id, RoomNotificationMode::Mute).await.unwrap();
+    // Ack the push rules change via sync, for it to be applied in the push context.
+    let ruleset = settings.ruleset().await;
+    server
+        .mock_sync()
+        .ok_and_run(&client, |builder| {
+            builder.add_global_account_data(f.push_rules(ruleset));
+        })
+        .await;
+
+    // The thread event will trigger a notification.
+    let actions = room.push_context().await.unwrap().unwrap().traced_for_event(&event).await;
+    // TODO: unexpected! this should trigger a thread mention
+    //assert!(actions.iter().any(|action| action.should_notify()));
+    assert!(!actions.iter().any(|action| action.should_notify()));
+}