Add a "add" method on Ruleset

- Add wrapper types for each kind of push rule
- Add a trait to add a push rule to a rule set

ruma-common/src/push.rs

@@ -1,6 +1,38 @@
 //! Common types for the [push notifications module][push]
 //!
 //! [push]: https://matrix.org/docs/spec/client_server/r0.6.1#id89
+//!
+//! ## Understanding the types of this module
+//!
+//! Push rules are grouped in `RuleSet`s, and are grouped in five kinds (for
+//! more details about the different kind of rules, see the `Ruleset` documentation,
+//! or the specification). These five kinds are:
+//!
+//! - content rules
+//! - override rules
+//! - underride rules
+//! - room rules
+//! - sender rules
+//!
+//! Each of these kind of rule has a corresponding type, that is
+//! just a wrapper arround another type:
+//!
+//! - `PushRule` for room and sender rules
+//! - `ConditionalPushRule` for override and underride rules: push rules that may depend on a
+//!   condition
+//! - `PatternedPushRules` for content rules, that can filter events based on a pattern to trigger
+//!   the rule or not
+//!
+//! Having these wrapper types allows to tell at the type level what kind of rule you are
+//! handling, and makes sure the `Ruleset::add` method adds your rule to the correct field
+//! of `Ruleset`, and that rules that are not of the same kind are never mixed even if they share
+//! the same representation.
+//!
+//! It is still possible to write code that is generic over a representation by manipulating
+//! `PushRule`, `ConditonalPushRule` or `PatternedPushRule` directly, instead of the wrappers.
+//!
+//! There is also the `AnyPushRule` type that is the most generic form of push rule, with all
+//! the possible fields.
 
 use std::collections::BTreeSet;
 
@@ -18,34 +50,6 @@ pub use self::{
     condition::{ComparisonOperator, PushCondition, RoomMemberCountIs},
 };
 
-macro_rules! ord_by_rule_id {
-    ($t:ty) => {
-        impl Ord for $t {
-            fn cmp(&self, other: &Self) -> core::cmp::Ordering {
-                self.rule_id.cmp(&other.rule_id)
-            }
-        }
-
-        impl PartialOrd for $t {
-            fn partial_cmp(&self, other: &Self) -> Option<core::cmp::Ordering> {
-                Some(self.cmp(other))
-            }
-        }
-
-        impl PartialEq for $t {
-            fn eq(&self, other: &Self) -> bool {
-                self.rule_id == other.rule_id
-            }
-        }
-
-        impl Eq for $t {}
-    };
-}
-
-ord_by_rule_id!(PushRule);
-ord_by_rule_id!(ConditionalPushRule);
-ord_by_rule_id!(PatternedPushRule);
-
 /// A push ruleset scopes a set of rules according to some criteria.
 ///
 /// For example, some rules may only be applied for messages from a particular sender, a particular
@@ -54,24 +58,24 @@ ord_by_rule_id!(PatternedPushRule);
 #[cfg_attr(not(feature = "unstable-exhaustive-types"), non_exhaustive)]
 pub struct Ruleset {
     /// These rules configure behavior for (unencrypted) messages that match certain patterns.
-    pub content: BTreeSet<PatternedPushRule>,
+    pub content: BTreeSet<ContentPushRule>,
 
     /// These user-configured rules are given the highest priority.
     ///
     /// This field is named `override_` instead of `override` because the latter is a reserved
     /// keyword in Rust.
     #[serde(rename = "override")]
-    pub override_: BTreeSet<ConditionalPushRule>,
+    pub override_: BTreeSet<OverridePushRule>,
 
     /// These rules change the behavior of all messages for a given room.
-    pub room: BTreeSet<PushRule>,
+    pub room: BTreeSet<RoomPushRule>,
 
     /// These rules configure notification behavior for messages from a specific Matrix user ID.
-    pub sender: BTreeSet<PushRule>,
+    pub sender: BTreeSet<SenderPushRule>,
 
     /// These rules are identical to override rules, but have a lower priority than `content`,
     /// `room` and `sender` rules.
-    pub underride: BTreeSet<ConditionalPushRule>,
+    pub underride: BTreeSet<UnderridePushRule>,
 }
 
 impl Ruleset {
@@ -79,6 +83,15 @@ impl Ruleset {
     pub fn new() -> Self {
         Default::default()
     }
+
+    /// Adds a rule to the rule set.
+    ///
+    /// Returns `true` if the new rule was correctly added, and `false`
+    /// if a rule with the same `rule_id` is already present for this kind
+    /// of rule.
+    pub fn add<R: RulesetMember>(&mut self, rule: R) -> bool {
+        rule.add_to(self)
+    }
 }
 
 impl IntoIterator for Ruleset {
@@ -86,15 +99,76 @@ impl IntoIterator for Ruleset {
     type IntoIter = std::vec::IntoIter<Self::Item>;
 
     fn into_iter(self) -> Self::IntoIter {
-        self.content.into_iter().map(Into::into)
+        self.content
-            .chain(self.override_.into_iter().map(Into::into))
+            .into_iter()
-            .chain(self.room.into_iter().map(Into::into))
+            .map(|x| x.0.into())
-            .chain(self.sender.into_iter().map(Into::into))
+            .chain(self.override_.into_iter().map(|x| x.0.into()))
-            .chain(self.underride.into_iter().map(Into::into))
+            .chain(self.room.into_iter().map(|x| x.0.into()))
-            .collect::<Vec<_>>().into_iter()
+            .chain(self.sender.into_iter().map(|x| x.0.into()))
+            .chain(self.underride.into_iter().map(|x| x.0.into()))
+            .collect::<Vec<_>>()
+            .into_iter()
     }
 }
 
+/// A trait for types that can be added in a Ruleset
+pub trait RulesetMember {
+    /// Adds a value in the correct field of a Ruleset.
+    fn add_to(self, ruleset: &mut Ruleset) -> bool;
+}
+
+/// Creates a new wrapper type around a PushRule type
+/// to make it possible to tell what kind of rule it is
+/// even if the inner type is the same.
+///
+/// For instance, override and underride rules are both
+/// represented as `ConditionalPushRule`s, so it is impossible
+/// to tell if a rule is an override or an underride rule when
+/// all you have is a `ConditionalPushRule`. With these wrapper types
+/// it becomes possible.
+macro_rules! rulekind {
+    ($name:ident, $inner:ty, $field:ident) => {
+        #[derive(Clone, Debug, Serialize, Deserialize)]
+        #[doc = "Wrapper type to disambiguate the kind of the wrapped rule"]
+        pub struct $name(pub $inner);
+
+        impl RulesetMember for $name {
+            fn add_to(self, ruleset: &mut Ruleset) -> bool {
+                ruleset.$field.insert(self)
+            }
+        }
+
+        // The following trait are needed to be able to make
+        // a BTreeSet of the new type
+
+        impl Ord for $name {
+            fn cmp(&self, other: &Self) -> core::cmp::Ordering {
+                self.0.rule_id.cmp(&other.0.rule_id)
+            }
+        }
+
+        impl PartialOrd for $name {
+            fn partial_cmp(&self, other: &Self) -> Option<core::cmp::Ordering> {
+                Some(self.cmp(other))
+            }
+        }
+
+        impl PartialEq for $name {
+            fn eq(&self, other: &Self) -> bool {
+                self.0.rule_id == other.0.rule_id
+            }
+        }
+
+        impl Eq for $name {}
+    };
+}
+
+rulekind!(OverridePushRule, ConditionalPushRule, override_);
+rulekind!(UnderridePushRule, ConditionalPushRule, underride);
+rulekind!(RoomPushRule, PushRule, room);
+rulekind!(SenderPushRule, PushRule, sender);
+rulekind!(ContentPushRule, PatternedPushRule, content);
+
 /// A push rule is a single rule that states under what conditions an event should be passed onto a
 /// push gateway and how the notification should be presented.
 ///

ruma-common/src/push/predefined.rs

@@ -2,8 +2,8 @@
 ///!
 ///! [predefined push rules]: https://matrix.org/docs/spec/client_server/r0.6.1#predefined-rules
 use super::{
-    Action::*, ConditionalPushRule, PatternedPushRule, PushCondition::*, RoomMemberCountIs,
+    Action::*, ConditionalPushRule, ContentPushRule, OverridePushRule, PatternedPushRule,
-    Ruleset, Tweak,
+    PushCondition::*, RoomMemberCountIs, Ruleset, Tweak, UnderridePushRule,
 };
 
 use ruma_identifiers::UserId;
@@ -34,22 +34,22 @@ impl Ruleset {
     ///   user's ID (for instance those to send notifications when they are mentioned).
     pub fn server_default(user_id: &UserId) -> Self {
         Self {
-            content: set![PatternedPushRule::contains_user_name(user_id)],
+            content: set![ContentPushRule::contains_user_name(user_id)],
             override_: set![
-                ConditionalPushRule::master(),
+                OverridePushRule::master(),
-                ConditionalPushRule::suppress_notices(),
+                OverridePushRule::suppress_notices(),
-                ConditionalPushRule::invite_for_me(user_id),
+                OverridePushRule::invite_for_me(user_id),
-                ConditionalPushRule::member_event(),
+                OverridePushRule::member_event(),
-                ConditionalPushRule::contains_display_name(),
+                OverridePushRule::contains_display_name(),
-                ConditionalPushRule::tombstone(),
+                OverridePushRule::tombstone(),
-                ConditionalPushRule::roomnotif(),
+                OverridePushRule::roomnotif(),
             ],
             underride: set![
-                ConditionalPushRule::call(),
+                UnderridePushRule::call(),
-                ConditionalPushRule::encrypted_room_one_to_one(),
+                UnderridePushRule::encrypted_room_one_to_one(),
-                ConditionalPushRule::room_one_to_one(),
+                UnderridePushRule::room_one_to_one(),
-                ConditionalPushRule::message(),
+                UnderridePushRule::message(),
-                ConditionalPushRule::encrypted(),
+                UnderridePushRule::encrypted(),
             ],
             ..Default::default()
         }
@@ -57,22 +57,22 @@ impl Ruleset {
 }
 
 /// Default override push rules
-impl ConditionalPushRule {
+impl OverridePushRule {
     /// Matches all events, this can be enabled to turn off all push notifications other than those
     /// generated by override rules set by the user.
     pub fn master() -> Self {
-        Self {
+        OverridePushRule(ConditionalPushRule {
             actions: vec![DontNotify],
             default: true,
             enabled: false,
             rule_id: ".m.rule.master".into(),
             conditions: vec![],
-        }
+        })
     }
 
     /// Matches messages with a `msgtype` of `notice`.
     pub fn suppress_notices() -> Self {
-        Self {
+        OverridePushRule(ConditionalPushRule {
             actions: vec![DontNotify],
             default: true,
             enabled: true,
@@ -81,12 +81,12 @@ impl ConditionalPushRule {
                 key: "content.msgtype".into(),
                 pattern: "m.notice".into(),
             }],
-        }
+        })
     }
 
     /// Matches any invites to a new room for this user.
     pub fn invite_for_me(user_id: &UserId) -> Self {
-        Self {
+        OverridePushRule(ConditionalPushRule {
             actions: vec![
                 Notify,
                 SetTweak(Tweak::Sound("default".into())),
@@ -100,24 +100,24 @@ impl ConditionalPushRule {
                 EventMatch { key: "content.membership".into(), pattern: "invite".into() },
                 EventMatch { key: "state_key".into(), pattern: user_id.to_string() },
             ],
-        }
+        })
     }
 
     /// Matches any `m.room.member_event`.
     pub fn member_event() -> Self {
-        Self {
+        OverridePushRule(ConditionalPushRule {
             actions: vec![DontNotify],
             default: true,
             enabled: true,
             rule_id: ".m.rule.member_event".into(),
             conditions: vec![EventMatch { key: "type".into(), pattern: "m.room.member".into() }],
-        }
+        })
     }
 
     /// Matches any message whose content is unencrypted and contains the user's current display
     /// name in the room in which it was sent.
     pub fn contains_display_name() -> Self {
-        Self {
+        OverridePushRule(ConditionalPushRule {
             actions: vec![
                 Notify,
                 SetTweak(Tweak::Sound("default".into())),
@@ -127,14 +127,14 @@ impl ConditionalPushRule {
             enabled: true,
             rule_id: ".m.rule.contains_display_name".into(),
             conditions: vec![ContainsDisplayName],
-        }
+        })
     }
 
     /// Matches any state event whose type is `m.room.tombstone`. This
     /// is intended to notify users of a room when it is upgraded,
     /// similar to what an `@room` notification would accomplish.
     pub fn tombstone() -> Self {
-        Self {
+        OverridePushRule(ConditionalPushRule {
             actions: vec![Notify, SetTweak(Tweak::Highlight(true))],
             default: true,
             enabled: false,
@@ -143,13 +143,13 @@ impl ConditionalPushRule {
                 EventMatch { key: "type".into(), pattern: "m.room.tombstone".into() },
                 EventMatch { key: "state_key".into(), pattern: "".into() },
             ],
-        }
+        })
     }
 
     /// Matches any message whose content is unencrypted and contains the text `@room`, signifying
     /// the whole room should be notified of the event.
     pub fn roomnotif() -> Self {
-        Self {
+        OverridePushRule(ConditionalPushRule {
             actions: vec![Notify, SetTweak(Tweak::Highlight(true))],
             default: true,
             enabled: true,
@@ -158,16 +158,16 @@ impl ConditionalPushRule {
                 EventMatch { key: "content.body".into(), pattern: "@room".into() },
                 SenderNotificationPermission { key: "room".into() },
             ],
-        }
+        })
     }
 }
 
 /// Default content push rules
-impl PatternedPushRule {
+impl ContentPushRule {
     /// Matches any message whose content is unencrypted and contains the local part of the user's
     /// Matrix ID, separated by word boundaries.
     pub fn contains_user_name(user_id: &UserId) -> Self {
-        Self {
+        ContentPushRule(PatternedPushRule {
             rule_id: ".m.rules.contains_user_name".into(),
             enabled: true,
             default: true,
@@ -177,15 +177,15 @@ impl PatternedPushRule {
                 SetTweak(Tweak::Sound("default".into())),
                 SetTweak(Tweak::Highlight(true)),
             ],
-        }
+        })
     }
 }
 
 /// Default underrides push rules
-impl ConditionalPushRule {
+impl UnderridePushRule {
     /// Matches any incoming VOIP call.
     pub fn call() -> Self {
-        Self {
+        UnderridePushRule(ConditionalPushRule {
             rule_id: ".m.rules.call".into(),
             default: true,
             enabled: true,
@@ -195,7 +195,7 @@ impl ConditionalPushRule {
                 SetTweak(Tweak::Sound("ring".into())),
                 SetTweak(Tweak::Highlight(false)),
             ],
-        }
+        })
     }
 
     /// Matches any encrypted event sent in a room with exactly two members. Unlike other push
@@ -203,7 +203,7 @@ impl ConditionalPushRule {
     /// encrypted. This causes the rule to be an "all or nothing" match where it either matches all
     /// events that are encrypted (in 1:1 rooms) or none.
     pub fn encrypted_room_one_to_one() -> Self {
-        Self {
+        UnderridePushRule(ConditionalPushRule {
             rule_id: ".m.rules.encrypted_room_one_to_one".into(),
             default: true,
             enabled: true,
@@ -216,12 +216,12 @@ impl ConditionalPushRule {
                 SetTweak(Tweak::Sound("default".into())),
                 SetTweak(Tweak::Highlight(false)),
             ],
-        }
+        })
     }
 
     /// Matches any message sent in a room with exactly two members.
     pub fn room_one_to_one() -> Self {
-        Self {
+        UnderridePushRule(ConditionalPushRule {
             rule_id: ".m.rules.room_one_to_one".into(),
             default: true,
             enabled: true,
@@ -234,18 +234,18 @@ impl ConditionalPushRule {
                 SetTweak(Tweak::Sound("default".into())),
                 SetTweak(Tweak::Highlight(false)),
             ],
-        }
+        })
     }
 
     /// Matches all chat messages.
     pub fn message() -> Self {
-        Self {
+        UnderridePushRule(ConditionalPushRule {
             rule_id: ".m.rules.message".into(),
             default: true,
             enabled: true,
             conditions: vec![EventMatch { key: "type".into(), pattern: "m.room.message".into() }],
             actions: vec![Notify, SetTweak(Tweak::Highlight(false))],
-        }
+        })
     }
 
     /// Matches all encrypted events. Unlike other push rules, this rule cannot be matched against
@@ -253,12 +253,12 @@ impl ConditionalPushRule {
     /// "all or nothing" match where it either matches all events that are encrypted (in group
     /// rooms) or none.
     pub fn encrypted() -> Self {
-        Self {
+        UnderridePushRule(ConditionalPushRule {
             rule_id: ".m.rules.encrypted".into(),
             default: true,
             enabled: true,
             conditions: vec![EventMatch { key: "type".into(), pattern: "m.room.encrypted".into() }],
             actions: vec![Notify, SetTweak(Tweak::Highlight(false))],
-        }
+        })
     }
 }