fix(sdk): Allow legacy push rules to be missing when changing NotificationSettings

They are being removed from the spec with MSC4210, so we can't error if they are not found.

Author: zecakeh

crates/matrix-sdk/src/error.rs

@@ -710,6 +710,13 @@ pub enum NotificationSettingsError {
     UnableToSavePushRules,
 }
 
+impl NotificationSettingsError {
+    /// Whether this error is the [`RuleNotFound`](Self::RuleNotFound) variant.
+    pub fn is_rule_not_found(&self) -> bool {
+        matches!(self, Self::RuleNotFound(_))
+    }
+}
+
 impl From<InsertPushRuleError> for NotificationSettingsError {
     fn from(_: InsertPushRuleError) -> Self {
         Self::UnableToAddPushRule

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

@@ -585,8 +585,8 @@ mod tests {
     };
     use ruma::{
         push::{
-            Action, AnyPushRuleRef, NewPatternedPushRule, NewPushRule, PredefinedOverrideRuleId,
-            PredefinedUnderrideRuleId, RuleKind,
+            Action, AnyPushRuleRef, NewPatternedPushRule, NewPushRule, PredefinedContentRuleId,
+            PredefinedOverrideRuleId, PredefinedUnderrideRuleId, RuleKind,
         },
         OwnedRoomId, RoomId,
     };
@@ -604,7 +604,7 @@ mod tests {
         notification_settings::{
             IsEncrypted, IsOneToOne, NotificationSettings, RoomNotificationMode,
         },
-        test_utils::logged_in_client,
+        test_utils::{logged_in_client, mocks::MatrixMockServer},
         Client,
     };
 
@@ -1682,4 +1682,81 @@ mod tests {
 
         assert_matches!(result, Err(NotificationSettingsError::InvalidParameter(_)));
     }
+
+    #[async_test]
+    #[allow(deprecated)]
+    async fn test_enable_mention_ignore_missing_legacy_push_rules() {
+        let server = MatrixMockServer::new().await;
+        let client = server.client_builder().build().await;
+        let mut ruleset = get_server_default_ruleset();
+
+        // Make sure that the legacy mention push rules are missing.
+        if let Some(idx) = ruleset
+            .override_
+            .iter()
+            .position(|rule| rule.rule_id == PredefinedOverrideRuleId::ContainsDisplayName.as_ref())
+        {
+            ruleset.override_.shift_remove_index(idx);
+        }
+
+        if let Some(idx) = ruleset
+            .override_
+            .iter()
+            .position(|rule| rule.rule_id == PredefinedOverrideRuleId::RoomNotif.as_ref())
+        {
+            ruleset.override_.shift_remove_index(idx);
+        }
+
+        if let Some(idx) = ruleset
+            .content
+            .iter()
+            .position(|rule| rule.rule_id == PredefinedContentRuleId::ContainsUserName.as_ref())
+        {
+            ruleset.content.shift_remove_index(idx);
+        }
+
+        assert_matches!(
+            ruleset.iter().find(|rule| {
+                rule.rule_id() == PredefinedOverrideRuleId::ContainsDisplayName.as_ref()
+                    || rule.rule_id() == PredefinedOverrideRuleId::RoomNotif.as_ref()
+                    || rule.rule_id() == PredefinedContentRuleId::ContainsUserName.as_ref()
+            }),
+            None,
+            "ruleset must not have legacy mention push rules"
+        );
+
+        let settings = NotificationSettings::new(client, ruleset);
+
+        server
+            .mock_enable_push_rule(RuleKind::Override, PredefinedOverrideRuleId::IsUserMention)
+            .ok()
+            .mock_once()
+            .named("is_user_mention")
+            .mount()
+            .await;
+        settings
+            .set_push_rule_enabled(
+                RuleKind::Override,
+                PredefinedOverrideRuleId::IsUserMention,
+                false,
+            )
+            .await
+            .unwrap();
+
+        server
+            .mock_enable_push_rule(RuleKind::Override, PredefinedOverrideRuleId::IsRoomMention)
+            .ok()
+            .mock_once()
+            .named("is_room_mention")
+            .mount()
+            .await;
+        settings
+            .set_push_rule_enabled(
+                RuleKind::Override,
+                PredefinedOverrideRuleId::IsRoomMention,
+                false,
+            )
+            .await
+            .unwrap();
+    }
 }

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

@@ -133,22 +133,32 @@ impl RuleCommands {
         )?;
 
         // For compatibility purpose, we still need to add commands for
-        // `ContainsUserName` and `ContainsDisplayName` (deprecated rules).
+        // `ContainsUserName` and `ContainsDisplayName` (removed rules).
         #[allow(deprecated)]
         {
             // `ContainsUserName`
-            self.set_enabled_internal(
+            if let Err(err) = self.set_enabled_internal(
                 RuleKind::Content,
                 PredefinedContentRuleId::ContainsUserName.as_str(),
                 enabled,
-            )?;
+            ) {
+                // This rule has been removed from the spec, so it's fine if it wasn't found.
+                if !err.is_rule_not_found() {
+                    return Err(err);
+                }
+            }
 
             // `ContainsDisplayName`
-            self.set_enabled_internal(
+            if let Err(err) = self.set_enabled_internal(
                 RuleKind::Override,
                 PredefinedOverrideRuleId::ContainsDisplayName.as_str(),
                 enabled,
-            )?;
+            ) {
+                // This rule has been removed from the spec, so it's fine if it wasn't found.
+                if !err.is_rule_not_found() {
+                    return Err(err);
+                }
+            }
         }
 
         Ok(())
@@ -164,14 +174,19 @@ impl RuleCommands {
             enabled,
         )?;
 
-        // For compatibility purpose, we still need to set `RoomNotif` (deprecated
+        // For compatibility purpose, we still need to set `RoomNotif` (removed
         // rule).
         #[allow(deprecated)]
-        self.set_enabled_internal(
+        if let Err(err) = self.set_enabled_internal(
             RuleKind::Override,
             PredefinedOverrideRuleId::RoomNotif.as_str(),
             enabled,
-        )?;
+        ) {
+            // This rule has been removed from the spec, so it's fine if it wasn't found.
+            if !err.is_rule_not_found() {
+                return Err(err);
+            }
+        }
 
         Ok(())
     }

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

@@ -40,6 +40,7 @@ use ruma::{
         RoomAccountDataEventType, StateEventType,
     },
     media::Method,
+    push::RuleKind,
     serde::Raw,
     time::Duration,
     DeviceId, EventId, MilliSecondsSinceUnixEpoch, MxcUri, OwnedDeviceId, OwnedEventId,
@@ -1361,6 +1362,19 @@ impl MatrixMockServer {
         self.mock_endpoint(mock, DeleteThreadSubscriptionEndpoint::default())
             .expect_default_access_token()
     }
+
+    /// Create a prebuilt mock for the endpoint used to enable a push rule.
+    pub fn mock_enable_push_rule(
+        &self,
+        kind: RuleKind,
+        rule_id: impl AsRef<str>,
+    ) -> MockEndpoint<'_, EnablePushRuleEndpoint> {
+        let rule_id = rule_id.as_ref();
+        let mock = Mock::given(method("PUT")).and(path_regex(format!(
+            "^/_matrix/client/v3/pushrules/global/{kind}/{rule_id}/enabled",
+        )));
+        self.mock_endpoint(mock, EnablePushRuleEndpoint).expect_default_access_token()
+    }
 }
 
 /// Parameter to [`MatrixMockServer::sync_room`].
@@ -1625,6 +1639,12 @@ impl<'a, T> MockEndpoint<'a, T> {
         self.respond_with(ResponseTemplate::new(200).set_body_json(json!({ "event_id": event_id })))
     }
 
+    /// Internal helper to return a 200 OK response with an empty JSON object in
+    /// the body.
+    fn ok_empty_json(self) -> MatrixMock<'a> {
+        self.respond_with(ResponseTemplate::new(200).set_body_json(json!({})))
+    }
+
     /// Returns an endpoint that emulates a permanent failure error (e.g. event
     /// is too large).
     ///
@@ -3897,3 +3917,14 @@ impl<'a> MockEndpoint<'a, DeleteThreadSubscriptionEndpoint> {
         self
     }
 }
+
+/// A prebuilt mock for `PUT
+/// /_matrix/client/v3/pushrules/global/{kind}/{ruleId}/enabled`.
+pub struct EnablePushRuleEndpoint;
+
+impl<'a> MockEndpoint<'a, EnablePushRuleEndpoint> {
+    /// Returns a successful empty JSON response.
+    pub fn ok(self) -> MatrixMock<'a> {
+        self.ok_empty_json()
+    }
+}