Remove the experimental implementation of MSC3772. (#14094)

MSC3772 has been abandoned.
This commit is contained in:
Patrick Cloke 2022-10-12 06:26:39 -04:00 committed by GitHub
parent 3f057e4c54
commit 09be8ab5f9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 22 additions and 365 deletions

View File

@ -0,0 +1 @@
Remove the experimental implementation of [MSC3772](https://github.com/matrix-org/matrix-spec-proposals/pull/3772).

View File

@ -257,19 +257,6 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[
default: true, default: true,
default_enabled: 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 { PushRule {
rule_id: Cow::Borrowed("global/underride/.m.rule.message"), rule_id: Cow::Borrowed("global/underride/.m.rule.message"),
priority_class: 1, priority_class: 1,

View File

@ -12,10 +12,7 @@
// See the License for the specific language governing permissions and // See the License for the specific language governing permissions and
// limitations under the License. // limitations under the License.
use std::{ use std::collections::BTreeMap;
borrow::Cow,
collections::{BTreeMap, BTreeSet},
};
use anyhow::{Context, Error}; use anyhow::{Context, Error};
use lazy_static::lazy_static; use lazy_static::lazy_static;
@ -49,13 +46,6 @@ pub struct PushRuleEvaluator {
/// The `notifications` section of the current power levels in the room. /// The `notifications` section of the current power levels in the room.
notification_power_levels: BTreeMap<String, i64>, 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 /// The power level of the sender of the event, or None if event is an
/// outlier. /// outlier.
sender_power_level: Option<i64>, sender_power_level: Option<i64>,
@ -70,8 +60,6 @@ impl PushRuleEvaluator {
room_member_count: u64, room_member_count: u64,
sender_power_level: Option<i64>, sender_power_level: Option<i64>,
notification_power_levels: BTreeMap<String, i64>, notification_power_levels: BTreeMap<String, i64>,
relations: BTreeMap<String, BTreeSet<(String, String)>>,
relation_match_enabled: bool,
) -> Result<Self, Error> { ) -> Result<Self, Error> {
let body = flattened_keys let body = flattened_keys
.get("content.body") .get("content.body")
@ -83,8 +71,6 @@ impl PushRuleEvaluator {
body, body,
room_member_count, room_member_count,
notification_power_levels, notification_power_levels,
relations,
relation_match_enabled,
sender_power_level, sender_power_level,
}) })
} }
@ -203,89 +189,11 @@ impl PushRuleEvaluator {
false 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) 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. /// Evaluates a `event_match` condition.
fn match_event_match( fn match_event_match(
&self, &self,
@ -359,15 +267,8 @@ impl PushRuleEvaluator {
fn push_rule_evaluator() { fn push_rule_evaluator() {
let mut flattened_keys = BTreeMap::new(); let mut flattened_keys = BTreeMap::new();
flattened_keys.insert("content.body".to_string(), "foo bar bob hello".to_string()); flattened_keys.insert("content.body".to_string(), "foo bar bob hello".to_string());
let evaluator = PushRuleEvaluator::py_new( let evaluator =
flattened_keys, PushRuleEvaluator::py_new(flattened_keys, 10, Some(0), BTreeMap::new()).unwrap();
10,
Some(0),
BTreeMap::new(),
BTreeMap::new(),
true,
)
.unwrap();
let result = evaluator.run(&FilteredPushRules::default(), None, Some("bob")); let result = evaluator.run(&FilteredPushRules::default(), None, Some("bob"));
assert_eq!(result.len(), 3); assert_eq!(result.len(), 3);

View File

@ -275,16 +275,6 @@ pub enum KnownCondition {
SenderNotificationPermission { SenderNotificationPermission {
key: Cow<'static, str>, 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 { impl IntoPy<PyObject> for Condition {
@ -401,21 +391,15 @@ impl PushRules {
pub struct FilteredPushRules { pub struct FilteredPushRules {
push_rules: PushRules, push_rules: PushRules,
enabled_map: BTreeMap<String, bool>, enabled_map: BTreeMap<String, bool>,
msc3772_enabled: bool,
} }
#[pymethods] #[pymethods]
impl FilteredPushRules { impl FilteredPushRules {
#[new] #[new]
pub fn py_new( pub fn py_new(push_rules: PushRules, enabled_map: BTreeMap<String, bool>) -> Self {
push_rules: PushRules,
enabled_map: BTreeMap<String, bool>,
msc3772_enabled: bool,
) -> Self {
Self { Self {
push_rules, push_rules,
enabled_map, enabled_map,
msc3772_enabled,
} }
} }
@ -430,25 +414,13 @@ impl FilteredPushRules {
/// Iterates over all the rules and their enabled state, including base /// Iterates over all the rules and their enabled state, including base
/// rules, in the order they should be executed in. /// rules, in the order they should be executed in.
fn iter(&self) -> impl Iterator<Item = (&PushRule, bool)> { fn iter(&self) -> impl Iterator<Item = (&PushRule, bool)> {
self.push_rules self.push_rules.iter().map(|r| {
.iter() let enabled = *self
.filter(|rule| { .enabled_map
// Ignore disabled experimental push rules .get(&*r.rule_id)
if !self.msc3772_enabled .unwrap_or(&r.default_enabled);
&& rule.rule_id == "global/underride/.org.matrix.msc3772.thread_reply" (r, enabled)
{ })
return false;
}
true
})
.map(|r| {
let enabled = *self
.enabled_map
.get(&*r.rule_id)
.unwrap_or(&r.default_enabled);
(r, enabled)
})
} }
} }

View File

@ -25,9 +25,7 @@ class PushRules:
def rules(self) -> Collection[PushRule]: ... def rules(self) -> Collection[PushRule]: ...
class FilteredPushRules: class FilteredPushRules:
def __init__( def __init__(self, push_rules: PushRules, enabled_map: Dict[str, bool]): ...
self, push_rules: PushRules, enabled_map: Dict[str, bool], msc3772_enabled: bool
): ...
def rules(self) -> Collection[Tuple[PushRule, bool]]: ... def rules(self) -> Collection[Tuple[PushRule, bool]]: ...
def get_base_rule_ids() -> Collection[str]: ... def get_base_rule_ids() -> Collection[str]: ...
@ -39,8 +37,6 @@ class PushRuleEvaluator:
room_member_count: int, room_member_count: int,
sender_power_level: Optional[int], sender_power_level: Optional[int],
notification_power_levels: Mapping[str, int], notification_power_levels: Mapping[str, int],
relations: Mapping[str, Set[Tuple[str, str]]],
relation_match_enabled: bool,
): ... ): ...
def run( def run(
self, self,

View File

@ -95,8 +95,6 @@ class ExperimentalConfig(Config):
# MSC2815 (allow room moderators to view redacted event content) # MSC2815 (allow room moderators to view redacted event content)
self.msc2815_enabled: bool = experimental.get("msc2815_enabled", False) 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 # MSC3773: Thread notifications
self.msc3773_enabled: bool = experimental.get("msc3773_enabled", False) self.msc3773_enabled: bool = experimental.get("msc3773_enabled", False)

View File

@ -13,18 +13,15 @@
# See the License for the specific language governing permissions and # See the License for the specific language governing permissions and
# limitations under the License. # limitations under the License.
import itertools
import logging import logging
from typing import ( from typing import (
TYPE_CHECKING, TYPE_CHECKING,
Any, Any,
Collection, Collection,
Dict, Dict,
Iterable,
List, List,
Mapping, Mapping,
Optional, Optional,
Set,
Tuple, Tuple,
Union, Union,
) )
@ -38,7 +35,7 @@ from synapse.events.snapshot import EventContext
from synapse.state import POWER_KEY from synapse.state import POWER_KEY
from synapse.storage.databases.main.roommember import EventIdMembership from synapse.storage.databases.main.roommember import EventIdMembership
from synapse.storage.state import StateFilter 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.caches import register_cache
from synapse.util.metrics import measure_func from synapse.util.metrics import measure_func
from synapse.visibility import filter_event_for_clients_with_state from synapse.visibility import filter_event_for_clients_with_state
@ -117,9 +114,6 @@ class BulkPushRuleEvaluator:
resizable=False, resizable=False,
) )
# Whether to support MSC3772 is supported.
self._relations_match_enabled = self.hs.config.experimental.msc3772_enabled
async def _get_rules_for_event( async def _get_rules_for_event(
self, self,
event: EventBase, event: EventBase,
@ -200,51 +194,6 @@ class BulkPushRuleEvaluator:
return pl_event.content if pl_event else {}, 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") @measure_func("action_for_event_by_user")
async def action_for_event_by_user( async def action_for_event_by_user(
self, event: EventBase, context: EventContext self, event: EventBase, context: EventContext
@ -276,16 +225,11 @@ class BulkPushRuleEvaluator:
sender_power_level, sender_power_level,
) = await self._get_power_levels_and_sender_level(event, context) ) = await self._get_power_levels_and_sender_level(event, context)
# Find the event's thread ID.
relation = relation_from_event(event) relation = relation_from_event(event)
# If the event does not have a relation, then cannot have any mutual # If the event does not have a relation, then it cannot have a thread ID.
# relations or thread ID.
relations = {}
thread_id = MAIN_TIMELINE thread_id = MAIN_TIMELINE
if relation: 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. # Recursively attempt to find the thread this event relates to.
if relation.rel_type == RelationTypes.THREAD: if relation.rel_type == RelationTypes.THREAD:
thread_id = relation.parent_id thread_id = relation.parent_id
@ -306,8 +250,6 @@ class BulkPushRuleEvaluator:
room_member_count, room_member_count,
sender_power_level, sender_power_level,
notification_levels, notification_levels,
relations,
self._relations_match_enabled,
) )
users = rules_by_user.keys() users = rules_by_user.keys()

View File

@ -259,9 +259,6 @@ class CacheInvalidationWorkerStore(SQLBaseStore):
self._attempt_to_invalidate_cache("get_applicable_edit", (relates_to,)) self._attempt_to_invalidate_cache("get_applicable_edit", (relates_to,))
self._attempt_to_invalidate_cache("get_thread_summary", (relates_to,)) self._attempt_to_invalidate_cache("get_thread_summary", (relates_to,))
self._attempt_to_invalidate_cache("get_thread_participated", (relates_to,)) self._attempt_to_invalidate_cache("get_thread_participated", (relates_to,))
self._attempt_to_invalidate_cache(
"get_mutual_event_relations_for_rel_type", (relates_to,)
)
async def invalidate_cache_and_stream( async def invalidate_cache_and_stream(
self, cache_name: str, keys: Tuple[Any, ...] self, cache_name: str, keys: Tuple[Any, ...]

View File

@ -2024,11 +2024,6 @@ class PersistEventsStore:
self.store._invalidate_cache_and_stream( self.store._invalidate_cache_and_stream(
txn, self.store.get_thread_participated, (redacted_relates_to,) txn, self.store.get_thread_participated, (redacted_relates_to,)
) )
self.store._invalidate_cache_and_stream(
txn,
self.store.get_mutual_event_relations_for_rel_type,
(redacted_relates_to,),
)
self.db_pool.simple_delete_txn( self.db_pool.simple_delete_txn(
txn, table="event_relations", keyvalues={"event_id": redacted_event_id} txn, table="event_relations", keyvalues={"event_id": redacted_event_id}

View File

@ -29,7 +29,6 @@ from typing import (
) )
from synapse.api.errors import StoreError from synapse.api.errors import StoreError
from synapse.config.homeserver import ExperimentalConfig
from synapse.replication.slave.storage._slaved_id_tracker import SlavedIdTracker from synapse.replication.slave.storage._slaved_id_tracker import SlavedIdTracker
from synapse.storage._base import SQLBaseStore from synapse.storage._base import SQLBaseStore
from synapse.storage.database import ( from synapse.storage.database import (
@ -63,9 +62,7 @@ logger = logging.getLogger(__name__)
def _load_rules( def _load_rules(
rawrules: List[JsonDict], rawrules: List[JsonDict], enabled_map: Dict[str, bool]
enabled_map: Dict[str, bool],
experimental_config: ExperimentalConfig,
) -> FilteredPushRules: ) -> FilteredPushRules:
"""Take the DB rows returned from the DB and convert them into a full """Take the DB rows returned from the DB and convert them into a full
`FilteredPushRules` object. `FilteredPushRules` object.
@ -83,9 +80,7 @@ def _load_rules(
push_rules = PushRules(ruleslist) push_rules = PushRules(ruleslist)
filtered_rules = FilteredPushRules( filtered_rules = FilteredPushRules(push_rules, enabled_map)
push_rules, enabled_map, msc3772_enabled=experimental_config.msc3772_enabled
)
return filtered_rules return filtered_rules
@ -165,7 +160,7 @@ class PushRulesWorkerStore(
enabled_map = await self.get_push_rules_enabled_for_user(user_id) enabled_map = await self.get_push_rules_enabled_for_user(user_id)
return _load_rules(rows, enabled_map, self.hs.config.experimental) return _load_rules(rows, enabled_map)
async def get_push_rules_enabled_for_user(self, user_id: str) -> Dict[str, bool]: async def get_push_rules_enabled_for_user(self, user_id: str) -> Dict[str, bool]:
results = await self.db_pool.simple_select_list( results = await self.db_pool.simple_select_list(
@ -224,9 +219,7 @@ class PushRulesWorkerStore(
results: Dict[str, FilteredPushRules] = {} results: Dict[str, FilteredPushRules] = {}
for user_id, rules in raw_rules.items(): for user_id, rules in raw_rules.items():
results[user_id] = _load_rules( results[user_id] = _load_rules(rules, enabled_map_by_user.get(user_id, {}))
rules, enabled_map_by_user.get(user_id, {}), self.hs.config.experimental
)
return results return results

View File

@ -776,59 +776,6 @@ class RelationsWorkerStore(SQLBaseStore):
"get_if_user_has_annotated_event", _get_if_user_has_annotated_event "get_if_user_has_annotated_event", _get_if_user_has_annotated_event
) )
@cached(iterable=True)
async def get_mutual_event_relations_for_rel_type(
self, event_id: str, relation_type: str
) -> Set[Tuple[str, str]]:
raise NotImplementedError()
@cachedList(
cached_method_name="get_mutual_event_relations_for_rel_type",
list_name="relation_types",
)
async def get_mutual_event_relations(
self, event_id: str, relation_types: Collection[str]
) -> 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:
event_id: The event ID which is targeted by relations.
relation_types: The relation types to check for mutual relations.
Returns:
A dictionary of relation type to:
A set of tuples of:
The sender
The event type
"""
rel_type_sql, rel_type_args = make_in_list_sql_clause(
self.database_engine, "relation_type", relation_types
)
sql = f"""
SELECT DISTINCT relation_type, sender, type FROM event_relations
INNER JOIN events USING (event_id)
WHERE relates_to_id = ? AND {rel_type_sql}
"""
def _get_event_relations(
txn: LoggingTransaction,
) -> Dict[str, Set[Tuple[str, str]]]:
txn.execute(sql, [event_id] + rel_type_args)
result: Dict[str, Set[Tuple[str, str]]] = {
rel_type: set() for rel_type in relation_types
}
for rel_type, sender, type in txn.fetchall():
result[rel_type].add((sender, type))
return result
return await self.db_pool.runInteraction(
"get_event_relations", _get_event_relations
)
@cached() @cached()
async def get_thread_id(self, event_id: str) -> Optional[str]: async def get_thread_id(self, event_id: str) -> Optional[str]:
""" """

View File

@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and # See the License for the specific language governing permissions and
# limitations under the License. # limitations under the License.
from typing import Dict, Optional, Set, Tuple, Union from typing import Dict, Optional, Union
import frozendict import frozendict
@ -38,12 +38,7 @@ from tests.test_utils.event_injection import create_event, inject_member_event
class PushRuleEvaluatorTestCase(unittest.TestCase): class PushRuleEvaluatorTestCase(unittest.TestCase):
def _get_evaluator( def _get_evaluator(self, content: JsonDict) -> PushRuleEvaluator:
self,
content: JsonDict,
relations: Optional[Dict[str, Set[Tuple[str, str]]]] = None,
relations_match_enabled: bool = False,
) -> PushRuleEvaluator:
event = FrozenEvent( event = FrozenEvent(
{ {
"event_id": "$event_id", "event_id": "$event_id",
@ -63,8 +58,6 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
room_member_count, room_member_count,
sender_power_level, sender_power_level,
power_levels.get("notifications", {}), power_levels.get("notifications", {}),
relations or {},
relations_match_enabled,
) )
def test_display_name(self) -> None: def test_display_name(self) -> None:
@ -299,71 +292,6 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
{"sound": "default", "highlight": True}, {"sound": "default", "highlight": True},
) )
def test_relation_match(self) -> None:
"""Test the relation_match push rule kind."""
# Check if the experimental feature is disabled.
evaluator = self._get_evaluator(
{}, {"m.annotation": {("@user:test", "m.reaction")}}
)
# A push rule evaluator with the experimental rule enabled.
evaluator = self._get_evaluator(
{}, {"m.annotation": {("@user:test", "m.reaction")}}, True
)
# Check just relation type.
condition = {
"kind": "org.matrix.msc3772.relation_match",
"rel_type": "m.annotation",
}
self.assertTrue(evaluator.matches(condition, "@user:test", "foo"))
# Check relation type and sender.
condition = {
"kind": "org.matrix.msc3772.relation_match",
"rel_type": "m.annotation",
"sender": "@user:test",
}
self.assertTrue(evaluator.matches(condition, "@user:test", "foo"))
condition = {
"kind": "org.matrix.msc3772.relation_match",
"rel_type": "m.annotation",
"sender": "@other:test",
}
self.assertFalse(evaluator.matches(condition, "@user:test", "foo"))
# Check relation type and event type.
condition = {
"kind": "org.matrix.msc3772.relation_match",
"rel_type": "m.annotation",
"type": "m.reaction",
}
self.assertTrue(evaluator.matches(condition, "@user:test", "foo"))
# Check just sender, this fails since rel_type is required.
condition = {
"kind": "org.matrix.msc3772.relation_match",
"sender": "@user:test",
}
self.assertFalse(evaluator.matches(condition, "@user:test", "foo"))
# Check sender glob.
condition = {
"kind": "org.matrix.msc3772.relation_match",
"rel_type": "m.annotation",
"sender": "@*:test",
}
self.assertTrue(evaluator.matches(condition, "@user:test", "foo"))
# Check event type glob.
condition = {
"kind": "org.matrix.msc3772.relation_match",
"rel_type": "m.annotation",
"event_type": "*.reaction",
}
self.assertTrue(evaluator.matches(condition, "@user:test", "foo"))
class TestBulkPushRuleEvaluator(unittest.HomeserverTestCase): class TestBulkPushRuleEvaluator(unittest.HomeserverTestCase):
"""Tests for the bulk push rule evaluator""" """Tests for the bulk push rule evaluator"""