diff --git a/changelog.d/15051.misc b/changelog.d/15051.misc
new file mode 100644
index 0000000000..fabfe77d35
--- /dev/null
+++ b/changelog.d/15051.misc
@@ -0,0 +1 @@
+Update [MSC3952](https://github.com/matrix-org/matrix-spec-proposals/pull/3952) support based on changes to the MSC.
diff --git a/rust/benches/evaluator.rs b/rust/benches/evaluator.rs
index 9a871f5693..7c987d4948 100644
--- a/rust/benches/evaluator.rs
+++ b/rust/benches/evaluator.rs
@@ -44,7 +44,6 @@ fn bench_match_exact(b: &mut Bencher) {
let eval = PushRuleEvaluator::py_new(
flattened_keys,
false,
- BTreeSet::new(),
10,
Some(0),
Default::default(),
@@ -92,7 +91,6 @@ fn bench_match_word(b: &mut Bencher) {
let eval = PushRuleEvaluator::py_new(
flattened_keys,
false,
- BTreeSet::new(),
10,
Some(0),
Default::default(),
@@ -140,7 +138,6 @@ fn bench_match_word_miss(b: &mut Bencher) {
let eval = PushRuleEvaluator::py_new(
flattened_keys,
false,
- BTreeSet::new(),
10,
Some(0),
Default::default(),
@@ -188,7 +185,6 @@ fn bench_eval_message(b: &mut Bencher) {
let eval = PushRuleEvaluator::py_new(
flattened_keys,
false,
- BTreeSet::new(),
10,
Some(0),
Default::default(),
diff --git a/rust/src/push/base_rules.rs b/rust/src/push/base_rules.rs
index 62de51d915..3d72a4a4c3 100644
--- a/rust/src/push/base_rules.rs
+++ b/rust/src/push/base_rules.rs
@@ -21,13 +21,13 @@ use lazy_static::lazy_static;
use serde_json::Value;
use super::KnownCondition;
-use crate::push::PushRule;
use crate::push::RelatedEventMatchTypeCondition;
use crate::push::SetTweak;
use crate::push::TweakValue;
use crate::push::{Action, ExactEventMatchCondition, SimpleJsonValue};
use crate::push::{Condition, EventMatchTypeCondition};
use crate::push::{EventMatchCondition, EventMatchPatternType};
+use crate::push::{ExactEventMatchTypeCondition, PushRule};
const HIGHLIGHT_ACTION: Action = Action::SetTweak(SetTweak {
set_tweak: Cow::Borrowed("highlight"),
@@ -144,7 +144,12 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[
PushRule {
rule_id: Cow::Borrowed(".org.matrix.msc3952.is_user_mention"),
priority_class: 5,
- conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::IsUserMention)]),
+ conditions: Cow::Borrowed(&[Condition::Known(
+ KnownCondition::ExactEventPropertyContainsType(ExactEventMatchTypeCondition {
+ key: Cow::Borrowed("content.org.matrix.msc3952.mentions.user_ids"),
+ value_type: Cow::Borrowed(&EventMatchPatternType::UserId),
+ }),
+ )]),
actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION, SOUND_ACTION]),
default: true,
default_enabled: true,
diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs
index a65c645caf..55846627cc 100644
--- a/rust/src/push/evaluator.rs
+++ b/rust/src/push/evaluator.rs
@@ -13,7 +13,7 @@
// limitations under the License.
use std::borrow::Cow;
-use std::collections::{BTreeMap, BTreeSet};
+use std::collections::BTreeMap;
use crate::push::{EventMatchPatternType, JsonValue};
use anyhow::{Context, Error};
@@ -72,8 +72,6 @@ pub struct PushRuleEvaluator {
/// True if the event has a mentions property and MSC3952 support is enabled.
has_mentions: bool,
- /// The user mentions that were part of the message.
- user_mentions: BTreeSet<String>,
/// The number of users in the room.
room_member_count: u64,
@@ -114,7 +112,6 @@ impl PushRuleEvaluator {
pub fn py_new(
flattened_keys: BTreeMap<String, JsonValue>,
has_mentions: bool,
- user_mentions: BTreeSet<String>,
room_member_count: u64,
sender_power_level: Option<i64>,
notification_power_levels: BTreeMap<String, i64>,
@@ -134,7 +131,6 @@ impl PushRuleEvaluator {
flattened_keys,
body,
has_mentions,
- user_mentions,
room_member_count,
notification_power_levels,
sender_power_level,
@@ -310,15 +306,30 @@ impl PushRuleEvaluator {
Some(Cow::Borrowed(pattern)),
)?
}
- KnownCondition::ExactEventPropertyContains(exact_event_match) => {
- self.match_exact_event_property_contains(exact_event_match)?
- }
- KnownCondition::IsUserMention => {
- if let Some(uid) = user_id {
- self.user_mentions.contains(uid)
+ KnownCondition::ExactEventPropertyContains(exact_event_match) => self
+ .match_exact_event_property_contains(
+ exact_event_match.key.clone(),
+ exact_event_match.value.clone(),
+ )?,
+ KnownCondition::ExactEventPropertyContainsType(exact_event_match) => {
+ // The `pattern_type` can either be "user_id" or "user_localpart",
+ // either way if we don't have a `user_id` then the condition can't
+ // match.
+ let user_id = if let Some(user_id) = user_id {
+ user_id
} else {
- false
- }
+ return Ok(false);
+ };
+
+ let pattern = match &*exact_event_match.value_type {
+ EventMatchPatternType::UserId => user_id,
+ EventMatchPatternType::UserLocalpart => get_localpart_from_id(user_id)?,
+ };
+
+ self.match_exact_event_property_contains(
+ exact_event_match.key.clone(),
+ Cow::Borrowed(&SimpleJsonValue::Str(pattern.to_string())),
+ )?
}
KnownCondition::ContainsDisplayName => {
if let Some(dn) = display_name {
@@ -456,24 +467,21 @@ impl PushRuleEvaluator {
/// Evaluates a `exact_event_property_contains` condition. (MSC3758)
fn match_exact_event_property_contains(
&self,
- exact_event_match: &ExactEventMatchCondition,
+ key: Cow<str>,
+ value: Cow<SimpleJsonValue>,
) -> Result<bool, Error> {
// First check if the feature is enabled.
if !self.msc3966_exact_event_property_contains {
return Ok(false);
}
- let value = &exact_event_match.value;
-
- let haystack = if let Some(JsonValue::Array(haystack)) =
- self.flattened_keys.get(&*exact_event_match.key)
- {
+ let haystack = if let Some(JsonValue::Array(haystack)) = self.flattened_keys.get(&*key) {
haystack
} else {
return Ok(false);
};
- Ok(haystack.contains(&**value))
+ Ok(haystack.contains(&value))
}
/// Match the member count against an 'is' condition
@@ -510,7 +518,6 @@ fn push_rule_evaluator() {
let evaluator = PushRuleEvaluator::py_new(
flattened_keys,
false,
- BTreeSet::new(),
10,
Some(0),
BTreeMap::new(),
@@ -542,7 +549,6 @@ fn test_requires_room_version_supports_condition() {
let evaluator = PushRuleEvaluator::py_new(
flattened_keys,
false,
- BTreeSet::new(),
10,
Some(0),
BTreeMap::new(),
diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs
index 97feb6efc9..6391d2ed47 100644
--- a/rust/src/push/mod.rs
+++ b/rust/src/push/mod.rs
@@ -340,8 +340,12 @@ pub enum KnownCondition {
RelatedEventMatchType(RelatedEventMatchTypeCondition),
#[serde(rename = "org.matrix.msc3966.exact_event_property_contains")]
ExactEventPropertyContains(ExactEventMatchCondition),
- #[serde(rename = "org.matrix.msc3952.is_user_mention")]
- IsUserMention,
+ // Identical to exact_event_property_contains but gives predefined patterns. Cannot be added by users.
+ #[serde(
+ skip_deserializing,
+ rename = "org.matrix.msc3966.exact_event_property_contains"
+ )]
+ ExactEventPropertyContainsType(ExactEventMatchTypeCondition),
ContainsDisplayName,
RoomMemberCount {
#[serde(skip_serializing_if = "Option::is_none")]
@@ -398,6 +402,15 @@ pub struct ExactEventMatchCondition {
pub value: Cow<'static, SimpleJsonValue>,
}
+/// The body of a [`Condition::ExactEventMatch`] that uses user_id or user_localpart as a pattern.
+#[derive(Serialize, Debug, Clone)]
+pub struct ExactEventMatchTypeCondition {
+ pub key: Cow<'static, str>,
+ // During serialization, the pattern_type property gets replaced with a
+ // pattern property of the correct value in synapse.push.clientformat.format_push_rules_for_user.
+ pub value_type: Cow<'static, EventMatchPatternType>,
+}
+
/// The body of a [`Condition::RelatedEventMatch`]
#[derive(Serialize, Deserialize, Debug, Clone)]
pub struct RelatedEventMatchCondition {
@@ -740,17 +753,6 @@ fn test_deserialize_unstable_msc3758_condition() {
}
#[test]
-fn test_deserialize_unstable_msc3952_user_condition() {
- let json = r#"{"kind":"org.matrix.msc3952.is_user_mention"}"#;
-
- let condition: Condition = serde_json::from_str(json).unwrap();
- assert!(matches!(
- condition,
- Condition::Known(KnownCondition::IsUserMention)
- ));
-}
-
-#[test]
fn test_deserialize_custom_condition() {
let json = r#"{"kind":"custom_tag"}"#;
diff --git a/stubs/synapse/synapse_rust/push.pyi b/stubs/synapse/synapse_rust/push.pyi
index a8f0ed2435..c17796ffbd 100644
--- a/stubs/synapse/synapse_rust/push.pyi
+++ b/stubs/synapse/synapse_rust/push.pyi
@@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-from typing import Any, Collection, Dict, Mapping, Optional, Sequence, Set, Tuple, Union
+from typing import Any, Collection, Dict, Mapping, Optional, Sequence, Tuple, Union
from synapse.types import JsonDict, JsonValue
@@ -58,7 +58,6 @@ class PushRuleEvaluator:
self,
flattened_keys: Mapping[str, JsonValue],
has_mentions: bool,
- user_mentions: Set[str],
room_member_count: int,
sender_power_level: Optional[int],
notification_power_levels: Mapping[str, int],
diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py
index 7c81f055b6..fc64f2bda1 100644
--- a/synapse/config/experimental.py
+++ b/synapse/config/experimental.py
@@ -179,10 +179,16 @@ class ExperimentalConfig(Config):
"msc3873_escape_event_match_key", False
)
- # MSC3952: Intentional mentions, this depends on MSC3758.
+ # MSC3966: exact_event_property_contains push rule condition.
+ self.msc3966_exact_event_property_contains = experimental.get(
+ "msc3966_exact_event_property_contains", False
+ )
+
+ # MSC3952: Intentional mentions, this depends on MSC3758 and MSC3966.
self.msc3952_intentional_mentions = (
experimental.get("msc3952_intentional_mentions", False)
and self.msc3758_exact_event_match
+ and self.msc3966_exact_event_property_contains
)
# MSC3959: Do not generate notifications for edits.
diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py
index 3c4a152d6b..abcf687f05 100644
--- a/synapse/push/bulk_push_rule_evaluator.py
+++ b/synapse/push/bulk_push_rule_evaluator.py
@@ -23,7 +23,6 @@ from typing import (
Mapping,
Optional,
Sequence,
- Set,
Tuple,
Union,
)
@@ -396,18 +395,10 @@ class BulkPushRuleEvaluator:
del notification_levels[key]
# Pull out any user and room mentions.
- mentions = event.content.get(EventContentFields.MSC3952_MENTIONS)
- has_mentions = self._intentional_mentions_enabled and isinstance(mentions, dict)
- user_mentions: Set[str] = set()
- if has_mentions:
- # mypy seems to have lost the type even though it must be a dict here.
- assert isinstance(mentions, dict)
- # Remove out any non-string items and convert to a set.
- user_mentions_raw = mentions.get("user_ids")
- if isinstance(user_mentions_raw, list):
- user_mentions = set(
- filter(lambda item: isinstance(item, str), user_mentions_raw)
- )
+ has_mentions = (
+ self._intentional_mentions_enabled
+ and EventContentFields.MSC3952_MENTIONS in event.content
+ )
evaluator = PushRuleEvaluator(
_flatten_dict(
@@ -415,7 +406,6 @@ class BulkPushRuleEvaluator:
msc3873_escape_event_match_key=self.hs.config.experimental.msc3873_escape_event_match_key,
),
has_mentions,
- user_mentions,
room_member_count,
sender_power_level,
notification_levels,
diff --git a/synapse/push/clientformat.py b/synapse/push/clientformat.py
index bb76c169c6..222afbdcc8 100644
--- a/synapse/push/clientformat.py
+++ b/synapse/push/clientformat.py
@@ -41,11 +41,12 @@ def format_push_rules_for_user(
rulearray.append(template_rule)
- pattern_type = template_rule.pop("pattern_type", None)
- if pattern_type == "user_id":
- template_rule["pattern"] = user.to_string()
- elif pattern_type == "user_localpart":
- template_rule["pattern"] = user.localpart
+ for type_key in ("pattern", "value"):
+ type_value = template_rule.pop(f"{type_key}_type", None)
+ if type_value == "user_id":
+ template_rule[type_key] = user.to_string()
+ elif type_value == "user_localpart":
+ template_rule[type_key] = user.localpart
template_rule["enabled"] = enabled
diff --git a/tests/push/test_bulk_push_rule_evaluator.py b/tests/push/test_bulk_push_rule_evaluator.py
index 1458076a90..73fecfd4ad 100644
--- a/tests/push/test_bulk_push_rule_evaluator.py
+++ b/tests/push/test_bulk_push_rule_evaluator.py
@@ -233,6 +233,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
"experimental_features": {
"msc3758_exact_event_match": True,
"msc3952_intentional_mentions": True,
+ "msc3966_exact_event_property_contains": True,
}
}
)
@@ -336,6 +337,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
"experimental_features": {
"msc3758_exact_event_match": True,
"msc3952_intentional_mentions": True,
+ "msc3966_exact_event_property_contains": True,
}
}
)
diff --git a/tests/push/test_push_rule_evaluator.py b/tests/push/test_push_rule_evaluator.py
index 1d30e3c3e4..d4a4bc4d93 100644
--- a/tests/push/test_push_rule_evaluator.py
+++ b/tests/push/test_push_rule_evaluator.py
@@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-from typing import Any, Dict, List, Optional, Set, Union, cast
+from typing import Any, Dict, List, Optional, Union, cast
import frozendict
@@ -147,8 +147,6 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
self,
content: JsonMapping,
*,
- has_mentions: bool = False,
- user_mentions: Optional[Set[str]] = None,
related_events: Optional[JsonDict] = None,
) -> PushRuleEvaluator:
event = FrozenEvent(
@@ -167,8 +165,7 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
power_levels: Dict[str, Union[int, Dict[str, int]]] = {}
return PushRuleEvaluator(
_flatten_dict(event),
- has_mentions,
- user_mentions or set(),
+ False,
room_member_count,
sender_power_level,
cast(Dict[str, int], power_levels.get("notifications", {})),
@@ -204,32 +201,6 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
# A display name with spaces should work fine.
self.assertTrue(evaluator.matches(condition, "@user:test", "foo bar"))
- def test_user_mentions(self) -> None:
- """Check for user mentions."""
- condition = {"kind": "org.matrix.msc3952.is_user_mention"}
-
- # No mentions shouldn't match.
- evaluator = self._get_evaluator({}, has_mentions=True)
- self.assertFalse(evaluator.matches(condition, "@user:test", None))
-
- # An empty set shouldn't match
- evaluator = self._get_evaluator({}, has_mentions=True, user_mentions=set())
- self.assertFalse(evaluator.matches(condition, "@user:test", None))
-
- # The Matrix ID appearing anywhere in the mentions list should match
- evaluator = self._get_evaluator(
- {}, has_mentions=True, user_mentions={"@user:test"}
- )
- self.assertTrue(evaluator.matches(condition, "@user:test", None))
-
- evaluator = self._get_evaluator(
- {}, has_mentions=True, user_mentions={"@another:test", "@user:test"}
- )
- self.assertTrue(evaluator.matches(condition, "@user:test", None))
-
- # Note that invalid data is tested at tests.push.test_bulk_push_rule_evaluator.TestBulkPushRuleEvaluator.test_mentions
- # since the BulkPushRuleEvaluator is what handles data sanitisation.
-
def _assert_matches(
self, condition: JsonDict, content: JsonMapping, msg: Optional[str] = None
) -> None:
|
