Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Remove the experimental implementation of MSC3772. #14094

Merged
merged 7 commits into from
Oct 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/14094.removal
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove the experimental implementation of [MSC3772](https://github.com/matrix-org/matrix-spec-proposals/pull/3772).
13 changes: 0 additions & 13 deletions rust/src/push/base_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,19 +257,6 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[
default: true,
default_enabled: true,
},
PushRule {
rule_id: Cow::Borrowed("global/underride/.org.matrix.msc3772.thread_reply"),
priority_class: 1,
conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::RelationMatch {
rel_type: Cow::Borrowed("m.thread"),
event_type_pattern: None,
sender: None,
sender_type: Some(Cow::Borrowed("user_id")),
})]),
actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_FALSE_ACTION]),
default: true,
default_enabled: true,
},
PushRule {
rule_id: Cow::Borrowed("global/underride/.m.rule.message"),
priority_class: 1,
Expand Down
105 changes: 3 additions & 102 deletions rust/src/push/evaluator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use std::{
borrow::Cow,
collections::{BTreeMap, BTreeSet},
};
use std::collections::BTreeMap;

use anyhow::{Context, Error};
use lazy_static::lazy_static;
Expand Down Expand Up @@ -49,13 +46,6 @@ pub struct PushRuleEvaluator {
/// The `notifications` section of the current power levels in the room.
notification_power_levels: BTreeMap<String, i64>,

/// The relations related to the event as a mapping from relation type to
/// set of sender/event type 2-tuples.
relations: BTreeMap<String, BTreeSet<(String, String)>>,

/// Is running "relation" conditions enabled?
relation_match_enabled: bool,

/// The power level of the sender of the event, or None if event is an
/// outlier.
sender_power_level: Option<i64>,
Expand All @@ -70,8 +60,6 @@ impl PushRuleEvaluator {
room_member_count: u64,
sender_power_level: Option<i64>,
notification_power_levels: BTreeMap<String, i64>,
relations: BTreeMap<String, BTreeSet<(String, String)>>,
relation_match_enabled: bool,
) -> Result<Self, Error> {
let body = flattened_keys
.get("content.body")
Expand All @@ -83,8 +71,6 @@ impl PushRuleEvaluator {
body,
room_member_count,
notification_power_levels,
relations,
relation_match_enabled,
sender_power_level,
})
}
Expand Down Expand Up @@ -203,89 +189,11 @@ impl PushRuleEvaluator {
false
}
}
KnownCondition::RelationMatch {
rel_type,
event_type_pattern,
sender,
sender_type,
} => {
self.match_relations(rel_type, sender, sender_type, user_id, event_type_pattern)?
}
};

Ok(result)
}

/// Evaluates a relation condition.
fn match_relations(
&self,
rel_type: &str,
sender: &Option<Cow<str>>,
sender_type: &Option<Cow<str>>,
user_id: Option<&str>,
event_type_pattern: &Option<Cow<str>>,
) -> Result<bool, Error> {
// First check if relation matching is enabled...
if !self.relation_match_enabled {
return Ok(false);
}

// ... and if there are any relations to match against.
let relations = if let Some(relations) = self.relations.get(rel_type) {
relations
} else {
return Ok(false);
};

// Extract the sender pattern from the condition
let sender_pattern = if let Some(sender) = sender {
Some(sender.as_ref())
} else if let Some(sender_type) = sender_type {
if sender_type == "user_id" {
if let Some(user_id) = user_id {
Some(user_id)
} else {
return Ok(false);
}
} else {
warn!("Unrecognized sender_type: {sender_type}");
return Ok(false);
}
} else {
None
};

let mut sender_compiled_pattern = if let Some(pattern) = sender_pattern {
Some(get_glob_matcher(pattern, GlobMatchType::Whole)?)
} else {
None
};

let mut type_compiled_pattern = if let Some(pattern) = event_type_pattern {
Some(get_glob_matcher(pattern, GlobMatchType::Whole)?)
} else {
None
};

for (relation_sender, event_type) in relations {
if let Some(pattern) = &mut sender_compiled_pattern {
if !pattern.is_match(relation_sender)? {
continue;
}
}

if let Some(pattern) = &mut type_compiled_pattern {
if !pattern.is_match(event_type)? {
continue;
}
}

return Ok(true);
}

Ok(false)
}

/// Evaluates a `event_match` condition.
fn match_event_match(
&self,
Expand Down Expand Up @@ -359,15 +267,8 @@ impl PushRuleEvaluator {
fn push_rule_evaluator() {
let mut flattened_keys = BTreeMap::new();
flattened_keys.insert("content.body".to_string(), "foo bar bob hello".to_string());
let evaluator = PushRuleEvaluator::py_new(
flattened_keys,
10,
Some(0),
BTreeMap::new(),
BTreeMap::new(),
true,
)
.unwrap();
let evaluator =
PushRuleEvaluator::py_new(flattened_keys, 10, Some(0), BTreeMap::new()).unwrap();

let result = evaluator.run(&FilteredPushRules::default(), None, Some("bob"));
assert_eq!(result.len(), 3);
Expand Down
44 changes: 8 additions & 36 deletions rust/src/push/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,16 +275,6 @@ pub enum KnownCondition {
SenderNotificationPermission {
key: Cow<'static, str>,
},
#[serde(rename = "org.matrix.msc3772.relation_match")]
RelationMatch {
rel_type: Cow<'static, str>,
#[serde(skip_serializing_if = "Option::is_none", rename = "type")]
event_type_pattern: Option<Cow<'static, str>>,
#[serde(skip_serializing_if = "Option::is_none")]
sender: Option<Cow<'static, str>>,
#[serde(skip_serializing_if = "Option::is_none")]
sender_type: Option<Cow<'static, str>>,
},
}

impl IntoPy<PyObject> for Condition {
Expand Down Expand Up @@ -401,21 +391,15 @@ impl PushRules {
pub struct FilteredPushRules {
push_rules: PushRules,
enabled_map: BTreeMap<String, bool>,
msc3772_enabled: bool,
}

#[pymethods]
impl FilteredPushRules {
#[new]
pub fn py_new(
push_rules: PushRules,
enabled_map: BTreeMap<String, bool>,
msc3772_enabled: bool,
) -> Self {
pub fn py_new(push_rules: PushRules, enabled_map: BTreeMap<String, bool>) -> Self {
Self {
push_rules,
enabled_map,
msc3772_enabled,
}
}

Expand All @@ -430,25 +414,13 @@ impl FilteredPushRules {
/// Iterates over all the rules and their enabled state, including base
/// rules, in the order they should be executed in.
fn iter(&self) -> impl Iterator<Item = (&PushRule, bool)> {
self.push_rules
.iter()
.filter(|rule| {
// Ignore disabled experimental push rules
if !self.msc3772_enabled
&& rule.rule_id == "global/underride/.org.matrix.msc3772.thread_reply"
{
return false;
}

true
})
.map(|r| {
let enabled = *self
.enabled_map
.get(&*r.rule_id)
.unwrap_or(&r.default_enabled);
(r, enabled)
})
self.push_rules.iter().map(|r| {
let enabled = *self
.enabled_map
.get(&*r.rule_id)
.unwrap_or(&r.default_enabled);
(r, enabled)
})
}
}

Expand Down
6 changes: 1 addition & 5 deletions stubs/synapse/synapse_rust/push.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,7 @@ class PushRules:
def rules(self) -> Collection[PushRule]: ...

class FilteredPushRules:
def __init__(
self, push_rules: PushRules, enabled_map: Dict[str, bool], msc3772_enabled: bool
): ...
def __init__(self, push_rules: PushRules, enabled_map: Dict[str, bool]): ...
def rules(self) -> Collection[Tuple[PushRule, bool]]: ...

def get_base_rule_ids() -> Collection[str]: ...
Expand All @@ -39,8 +37,6 @@ class PushRuleEvaluator:
room_member_count: int,
sender_power_level: Optional[int],
notification_power_levels: Mapping[str, int],
relations: Mapping[str, Set[Tuple[str, str]]],
relation_match_enabled: bool,
): ...
def run(
self,
Expand Down
2 changes: 0 additions & 2 deletions synapse/config/experimental.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,6 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
# MSC2815 (allow room moderators to view redacted event content)
self.msc2815_enabled: bool = experimental.get("msc2815_enabled", False)

# MSC3772: A push rule for mutual relations.
self.msc3772_enabled: bool = experimental.get("msc3772_enabled", False)
# MSC3773: Thread notifications
self.msc3773_enabled: bool = experimental.get("msc3773_enabled", False)

Expand Down
64 changes: 3 additions & 61 deletions synapse/push/bulk_push_rule_evaluator.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,15 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import itertools
import logging
from typing import (
TYPE_CHECKING,
Any,
Collection,
Dict,
Iterable,
List,
Mapping,
Optional,
Set,
Tuple,
Union,
)
Expand All @@ -38,7 +35,7 @@
from synapse.state import POWER_KEY
from synapse.storage.databases.main.roommember import EventIdMembership
from synapse.storage.state import StateFilter
from synapse.synapse_rust.push import FilteredPushRules, PushRule, PushRuleEvaluator
from synapse.synapse_rust.push import FilteredPushRules, PushRuleEvaluator
from synapse.util.caches import register_cache
from synapse.util.metrics import measure_func
from synapse.visibility import filter_event_for_clients_with_state
Expand Down Expand Up @@ -117,9 +114,6 @@ def __init__(self, hs: "HomeServer"):
resizable=False,
)

# Whether to support MSC3772 is supported.
self._relations_match_enabled = self.hs.config.experimental.msc3772_enabled

async def _get_rules_for_event(
self,
event: EventBase,
Expand Down Expand Up @@ -200,51 +194,6 @@ async def _get_power_levels_and_sender_level(

return pl_event.content if pl_event else {}, sender_level

async def _get_mutual_relations(
self, parent_id: str, rules: Iterable[Tuple[PushRule, bool]]
) -> Dict[str, Set[Tuple[str, str]]]:
"""
Fetch event metadata for events which related to the same event as the given event.

If the given event has no relation information, returns an empty dictionary.

Args:
parent_id: The event ID which is targeted by relations.
rules: The push rules which will be processed for this event.

Returns:
A dictionary of relation type to:
A set of tuples of:
The sender
The event type
"""

# If the experimental feature is not enabled, skip fetching relations.
if not self._relations_match_enabled:
return {}

# Pre-filter to figure out which relation types are interesting.
rel_types = set()
for rule, enabled in rules:
if not enabled:
continue

for condition in rule.conditions:
if condition["kind"] != "org.matrix.msc3772.relation_match":
continue

# rel_type is required.
rel_type = condition.get("rel_type")
if rel_type:
rel_types.add(rel_type)

# If no valid rules were found, no mutual relations.
if not rel_types:
return {}

# If any valid rules were found, fetch the mutual relations.
return await self.store.get_mutual_event_relations(parent_id, rel_types)

@measure_func("action_for_event_by_user")
async def action_for_event_by_user(
self, event: EventBase, context: EventContext
Expand Down Expand Up @@ -276,16 +225,11 @@ async def action_for_event_by_user(
sender_power_level,
) = await self._get_power_levels_and_sender_level(event, context)

# Find the event's thread ID.
relation = relation_from_event(event)
# If the event does not have a relation, then cannot have any mutual
# relations or thread ID.
relations = {}
# If the event does not have a relation, then it cannot have a thread ID.
thread_id = MAIN_TIMELINE
if relation:
relations = await self._get_mutual_relations(
relation.parent_id,
itertools.chain(*(r.rules() for r in rules_by_user.values())),
)
# Recursively attempt to find the thread this event relates to.
if relation.rel_type == RelationTypes.THREAD:
thread_id = relation.parent_id
Expand All @@ -306,8 +250,6 @@ async def action_for_event_by_user(
room_member_count,
sender_power_level,
notification_levels,
relations,
self._relations_match_enabled,
)

users = rules_by_user.keys()
Expand Down
Loading