Update MSC3958 support to interact with intentional mentions. (#15992)

* Updates the rule ID.
* Use `event_property_is` instead of `event_match`.

This updates the implementation of MSC3958 to match the latest
text from the MSC.
This commit is contained in:
Patrick Cloke 2023-08-02 08:41:32 -04:00 committed by GitHub
parent ca5d5de79b
commit 01a45869f0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 64 additions and 42 deletions

1
changelog.d/15992.misc Normal file
View file

@ -0,0 +1 @@
Update support for [MSC3958](https://github.com/matrix-org/matrix-spec-proposals/pull/3958) to match the latest revision of the MSC.

View file

@ -13,6 +13,9 @@
// limitations under the License. // limitations under the License.
#![feature(test)] #![feature(test)]
use std::borrow::Cow;
use synapse::push::{ use synapse::push::{
evaluator::PushRuleEvaluator, Condition, EventMatchCondition, FilteredPushRules, JsonValue, evaluator::PushRuleEvaluator, Condition, EventMatchCondition, FilteredPushRules, JsonValue,
PushRules, SimpleJsonValue, PushRules, SimpleJsonValue,
@ -26,15 +29,15 @@ fn bench_match_exact(b: &mut Bencher) {
let flattened_keys = [ let flattened_keys = [
( (
"type".to_string(), "type".to_string(),
JsonValue::Value(SimpleJsonValue::Str("m.text".to_string())), JsonValue::Value(SimpleJsonValue::Str(Cow::Borrowed("m.text"))),
), ),
( (
"room_id".to_string(), "room_id".to_string(),
JsonValue::Value(SimpleJsonValue::Str("!room:server".to_string())), JsonValue::Value(SimpleJsonValue::Str(Cow::Borrowed("!room:server"))),
), ),
( (
"content.body".to_string(), "content.body".to_string(),
JsonValue::Value(SimpleJsonValue::Str("test message".to_string())), JsonValue::Value(SimpleJsonValue::Str(Cow::Borrowed("test message"))),
), ),
] ]
.into_iter() .into_iter()
@ -71,15 +74,15 @@ fn bench_match_word(b: &mut Bencher) {
let flattened_keys = [ let flattened_keys = [
( (
"type".to_string(), "type".to_string(),
JsonValue::Value(SimpleJsonValue::Str("m.text".to_string())), JsonValue::Value(SimpleJsonValue::Str(Cow::Borrowed("m.text"))),
), ),
( (
"room_id".to_string(), "room_id".to_string(),
JsonValue::Value(SimpleJsonValue::Str("!room:server".to_string())), JsonValue::Value(SimpleJsonValue::Str(Cow::Borrowed("!room:server"))),
), ),
( (
"content.body".to_string(), "content.body".to_string(),
JsonValue::Value(SimpleJsonValue::Str("test message".to_string())), JsonValue::Value(SimpleJsonValue::Str(Cow::Borrowed("test message"))),
), ),
] ]
.into_iter() .into_iter()
@ -116,15 +119,15 @@ fn bench_match_word_miss(b: &mut Bencher) {
let flattened_keys = [ let flattened_keys = [
( (
"type".to_string(), "type".to_string(),
JsonValue::Value(SimpleJsonValue::Str("m.text".to_string())), JsonValue::Value(SimpleJsonValue::Str(Cow::Borrowed("m.text"))),
), ),
( (
"room_id".to_string(), "room_id".to_string(),
JsonValue::Value(SimpleJsonValue::Str("!room:server".to_string())), JsonValue::Value(SimpleJsonValue::Str(Cow::Borrowed("!room:server"))),
), ),
( (
"content.body".to_string(), "content.body".to_string(),
JsonValue::Value(SimpleJsonValue::Str("test message".to_string())), JsonValue::Value(SimpleJsonValue::Str(Cow::Borrowed("test message"))),
), ),
] ]
.into_iter() .into_iter()
@ -161,15 +164,15 @@ fn bench_eval_message(b: &mut Bencher) {
let flattened_keys = [ let flattened_keys = [
( (
"type".to_string(), "type".to_string(),
JsonValue::Value(SimpleJsonValue::Str("m.text".to_string())), JsonValue::Value(SimpleJsonValue::Str(Cow::Borrowed("m.text"))),
), ),
( (
"room_id".to_string(), "room_id".to_string(),
JsonValue::Value(SimpleJsonValue::Str("!room:server".to_string())), JsonValue::Value(SimpleJsonValue::Str(Cow::Borrowed("!room:server"))),
), ),
( (
"content.body".to_string(), "content.body".to_string(),
JsonValue::Value(SimpleJsonValue::Str("test message".to_string())), JsonValue::Value(SimpleJsonValue::Str(Cow::Borrowed("test message"))),
), ),
] ]
.into_iter() .into_iter()

View file

@ -63,22 +63,6 @@ pub const BASE_PREPEND_OVERRIDE_RULES: &[PushRule] = &[PushRule {
}]; }];
pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[
// We don't want to notify on edits. Not only can this be confusing in real
// time (2 notifications, one message) but it's especially confusing
// if a bridge needs to edit a previously backfilled message.
PushRule {
rule_id: Cow::Borrowed("global/override/.com.beeper.suppress_edits"),
priority_class: 5,
conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventMatch(
EventMatchCondition {
key: Cow::Borrowed("content.m\\.relates_to.rel_type"),
pattern: Cow::Borrowed("m.replace"),
},
))]),
actions: Cow::Borrowed(&[]),
default: true,
default_enabled: true,
},
PushRule { PushRule {
rule_id: Cow::Borrowed("global/override/.m.rule.suppress_notices"), rule_id: Cow::Borrowed("global/override/.m.rule.suppress_notices"),
priority_class: 5, priority_class: 5,
@ -146,7 +130,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[
priority_class: 5, priority_class: 5,
conditions: Cow::Borrowed(&[Condition::Known( conditions: Cow::Borrowed(&[Condition::Known(
KnownCondition::ExactEventPropertyContainsType(EventPropertyIsTypeCondition { KnownCondition::ExactEventPropertyContainsType(EventPropertyIsTypeCondition {
key: Cow::Borrowed("content.m\\.mentions.user_ids"), key: Cow::Borrowed(r"content.m\.mentions.user_ids"),
value_type: Cow::Borrowed(&EventMatchPatternType::UserId), value_type: Cow::Borrowed(&EventMatchPatternType::UserId),
}), }),
)]), )]),
@ -167,8 +151,8 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[
priority_class: 5, priority_class: 5,
conditions: Cow::Borrowed(&[ conditions: Cow::Borrowed(&[
Condition::Known(KnownCondition::EventPropertyIs(EventPropertyIsCondition { Condition::Known(KnownCondition::EventPropertyIs(EventPropertyIsCondition {
key: Cow::Borrowed("content.m\\.mentions.room"), key: Cow::Borrowed(r"content.m\.mentions.room"),
value: Cow::Borrowed(&SimpleJsonValue::Bool(true)), value: Cow::Owned(SimpleJsonValue::Bool(true)),
})), })),
Condition::Known(KnownCondition::SenderNotificationPermission { Condition::Known(KnownCondition::SenderNotificationPermission {
key: Cow::Borrowed("room"), key: Cow::Borrowed("room"),
@ -241,6 +225,21 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[
default: true, default: true,
default_enabled: true, default_enabled: true,
}, },
// We don't want to notify on edits *unless* the edit directly mentions a
// user, which is handled above.
PushRule {
rule_id: Cow::Borrowed("global/override/.org.matrix.msc3958.suppress_edits"),
priority_class: 5,
conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventPropertyIs(
EventPropertyIsCondition {
key: Cow::Borrowed(r"content.m\.relates_to.rel_type"),
value: Cow::Owned(SimpleJsonValue::Str(Cow::Borrowed("m.replace"))),
},
))]),
actions: Cow::Borrowed(&[]),
default: true,
default_enabled: true,
},
PushRule { PushRule {
rule_id: Cow::Borrowed("global/override/.org.matrix.msc3930.rule.poll_response"), rule_id: Cow::Borrowed("global/override/.org.matrix.msc3930.rule.poll_response"),
priority_class: 5, priority_class: 5,

View file

@ -117,7 +117,7 @@ impl PushRuleEvaluator {
msc3931_enabled: bool, msc3931_enabled: bool,
) -> Result<Self, Error> { ) -> Result<Self, Error> {
let body = match flattened_keys.get("content.body") { let body = match flattened_keys.get("content.body") {
Some(JsonValue::Value(SimpleJsonValue::Str(s))) => s.clone(), Some(JsonValue::Value(SimpleJsonValue::Str(s))) => s.clone().into_owned(),
_ => String::new(), _ => String::new(),
}; };
@ -313,13 +313,15 @@ impl PushRuleEvaluator {
}; };
let pattern = match &*exact_event_match.value_type { let pattern = match &*exact_event_match.value_type {
EventMatchPatternType::UserId => user_id, EventMatchPatternType::UserId => user_id.to_owned(),
EventMatchPatternType::UserLocalpart => get_localpart_from_id(user_id)?, EventMatchPatternType::UserLocalpart => {
get_localpart_from_id(user_id)?.to_owned()
}
}; };
self.match_event_property_contains( self.match_event_property_contains(
exact_event_match.key.clone(), exact_event_match.key.clone(),
Cow::Borrowed(&SimpleJsonValue::Str(pattern.to_string())), Cow::Borrowed(&SimpleJsonValue::Str(Cow::Owned(pattern))),
)? )?
} }
KnownCondition::ContainsDisplayName => { KnownCondition::ContainsDisplayName => {
@ -494,7 +496,7 @@ fn push_rule_evaluator() {
let mut flattened_keys = BTreeMap::new(); let mut flattened_keys = BTreeMap::new();
flattened_keys.insert( flattened_keys.insert(
"content.body".to_string(), "content.body".to_string(),
JsonValue::Value(SimpleJsonValue::Str("foo bar bob hello".to_string())), JsonValue::Value(SimpleJsonValue::Str(Cow::Borrowed("foo bar bob hello"))),
); );
let evaluator = PushRuleEvaluator::py_new( let evaluator = PushRuleEvaluator::py_new(
flattened_keys, flattened_keys,
@ -522,7 +524,7 @@ fn test_requires_room_version_supports_condition() {
let mut flattened_keys = BTreeMap::new(); let mut flattened_keys = BTreeMap::new();
flattened_keys.insert( flattened_keys.insert(
"content.body".to_string(), "content.body".to_string(),
JsonValue::Value(SimpleJsonValue::Str("foo bar bob hello".to_string())), JsonValue::Value(SimpleJsonValue::Str(Cow::Borrowed("foo bar bob hello"))),
); );
let flags = vec![RoomVersionFeatures::ExtensibleEvents.as_str().to_string()]; let flags = vec![RoomVersionFeatures::ExtensibleEvents.as_str().to_string()];
let evaluator = PushRuleEvaluator::py_new( let evaluator = PushRuleEvaluator::py_new(

View file

@ -256,7 +256,7 @@ impl<'de> Deserialize<'de> for Action {
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)] #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
#[serde(untagged)] #[serde(untagged)]
pub enum SimpleJsonValue { pub enum SimpleJsonValue {
Str(String), Str(Cow<'static, str>),
Int(i64), Int(i64),
Bool(bool), Bool(bool),
Null, Null,
@ -265,7 +265,7 @@ pub enum SimpleJsonValue {
impl<'source> FromPyObject<'source> for SimpleJsonValue { impl<'source> FromPyObject<'source> for SimpleJsonValue {
fn extract(ob: &'source PyAny) -> PyResult<Self> { fn extract(ob: &'source PyAny) -> PyResult<Self> {
if let Ok(s) = <PyString as pyo3::PyTryFrom>::try_from(ob) { if let Ok(s) = <PyString as pyo3::PyTryFrom>::try_from(ob) {
Ok(SimpleJsonValue::Str(s.to_string())) Ok(SimpleJsonValue::Str(Cow::Owned(s.to_string())))
// A bool *is* an int, ensure we try bool first. // A bool *is* an int, ensure we try bool first.
} else if let Ok(b) = <PyBool as pyo3::PyTryFrom>::try_from(ob) { } else if let Ok(b) = <PyBool as pyo3::PyTryFrom>::try_from(ob) {
Ok(SimpleJsonValue::Bool(b.extract()?)) Ok(SimpleJsonValue::Bool(b.extract()?))
@ -585,7 +585,7 @@ impl FilteredPushRules {
} }
if !self.msc3958_suppress_edits_enabled if !self.msc3958_suppress_edits_enabled
&& rule.rule_id == "global/override/.com.beeper.suppress_edits" && rule.rule_id == "global/override/.org.matrix.msc3958.suppress_edits"
{ {
return false; return false;
} }

View file

@ -409,12 +409,12 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
) )
) )
# Room mentions from those without power should not notify. # The edit should not cause a notification.
self.assertFalse( self.assertFalse(
self._create_and_process( self._create_and_process(
bulk_evaluator, bulk_evaluator,
{ {
"body": self.alice, "body": "Test message",
"m.relates_to": { "m.relates_to": {
"rel_type": RelationTypes.REPLACE, "rel_type": RelationTypes.REPLACE,
"event_id": event.event_id, "event_id": event.event_id,
@ -422,3 +422,20 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
}, },
) )
) )
# An edit which is a mention will cause a notification.
self.assertTrue(
self._create_and_process(
bulk_evaluator,
{
"body": "Test message",
"m.relates_to": {
"rel_type": RelationTypes.REPLACE,
"event_id": event.event_id,
},
"m.mentions": {
"user_ids": [self.alice],
},
},
)
)