Merge branch 'develop' into dmr/typing/tests.appservice

Author: David Robertson

changelog.d/14960.feature

@@ -0,0 +1 @@
+Experimental support to suppress notifications from message edits ([MSC3958](https://github.com/matrix-org/matrix-spec-proposals/pull/3958)).

changelog.d/14983.misc

@@ -0,0 +1 @@
+Improve type hints.

mypy.ini

@@ -72,6 +72,9 @@ disallow_untyped_defs = False
 [mypy-tests.*]
 disallow_untyped_defs = False
 
+[mypy-tests.api.*]
+disallow_untyped_defs = True
+
 [mypy-tests.app.*]
 disallow_untyped_defs = True
 

rust/benches/evaluator.rs

@@ -170,6 +170,7 @@ fn bench_eval_message(b: &mut Bencher) {
         false,
         false,
         false,
+        false,
     );
 
     b.iter(|| eval.run(&rules, Some("bob"), Some("person")));

rust/src/push/base_rules.rs

@@ -63,6 +63,23 @@ pub const BASE_PREPEND_OVERRIDE_RULES: &[PushRule] = &[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: Some(Cow::Borrowed("m.replace")),
+                pattern_type: None,
+            },
+        ))]),
+        actions: Cow::Borrowed(&[Action::DontNotify]),
+        default: true,
+        default_enabled: true,
+    },
     PushRule {
         rule_id: Cow::Borrowed("global/override/.m.rule.suppress_notices"),
         priority_class: 5,

rust/src/push/evaluator.rs

@@ -523,7 +523,7 @@ fn test_requires_room_version_supports_condition() {
     };
     let rules = PushRules::new(vec![custom_rule]);
     result = evaluator.run(
-        &FilteredPushRules::py_new(rules, BTreeMap::new(), true, false, true, false),
+        &FilteredPushRules::py_new(rules, BTreeMap::new(), true, false, true, false, false),
         None,
         None,
     );

rust/src/push/mod.rs

@@ -419,6 +419,7 @@ pub struct FilteredPushRules {
     msc3381_polls_enabled: bool,
     msc3664_enabled: bool,
     msc3952_intentional_mentions: bool,
+    msc3958_suppress_edits_enabled: bool,
 }
 
 #[pymethods]
@@ -431,6 +432,7 @@ impl FilteredPushRules {
         msc3381_polls_enabled: bool,
         msc3664_enabled: bool,
         msc3952_intentional_mentions: bool,
+        msc3958_suppress_edits_enabled: bool,
     ) -> Self {
         Self {
             push_rules,
@@ -439,6 +441,7 @@ impl FilteredPushRules {
             msc3381_polls_enabled,
             msc3664_enabled,
             msc3952_intentional_mentions,
+            msc3958_suppress_edits_enabled,
         }
     }
 
@@ -476,6 +479,11 @@ impl FilteredPushRules {
                 {
                     return false;
                 }
+                if !self.msc3958_suppress_edits_enabled
+                    && rule.rule_id == "global/override/.com.beeper.suppress_edits"
+                {
+                    return false;
+                }
 
                 true
             })

stubs/synapse/synapse_rust/push.pyi

@@ -47,6 +47,7 @@ class FilteredPushRules:
         msc3381_polls_enabled: bool,
         msc3664_enabled: bool,
         msc3952_intentional_mentions: bool,
+        msc3958_suppress_edits_enabled: bool,
     ): ...
     def rules(self) -> Collection[Tuple[PushRule, bool]]: ...
 

synapse/api/filtering.py

@@ -252,9 +252,9 @@ def unread_thread_notifications(self) -> bool:
         return self._room_timeline_filter.unread_thread_notifications
 
     async def filter_presence(
-        self, events: Iterable[UserPresenceState]
+        self, presence_states: Iterable[UserPresenceState]
     ) -> List[UserPresenceState]:
-        return await self._presence_filter.filter(events)
+        return await self._presence_filter.filter(presence_states)
 
     async def filter_account_data(self, events: Iterable[JsonDict]) -> List[JsonDict]:
         return await self._account_data.filter(events)

synapse/config/experimental.py

@@ -173,3 +173,8 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
         self.msc3952_intentional_mentions = experimental.get(
             "msc3952_intentional_mentions", False
         )
+
+        # MSC3959: Do not generate notifications for edits.
+        self.msc3958_supress_edit_notifs = experimental.get(
+            "msc3958_supress_edit_notifs", False
+        )

synapse/storage/databases/main/push_rule.py

@@ -90,6 +90,7 @@ def _load_rules(
         msc3664_enabled=experimental_config.msc3664_enabled,
         msc3381_polls_enabled=experimental_config.msc3381_polls_enabled,
         msc3952_intentional_mentions=experimental_config.msc3952_intentional_mentions,
+        msc3958_suppress_edits_enabled=experimental_config.msc3958_supress_edit_notifs,
     )
 
     return filtered_rules

tests/api/test_auth.py

@@ -31,7 +31,7 @@
 from synapse.appservice import ApplicationService
 from synapse.server import HomeServer
 from synapse.storage.databases.main.registration import TokenLookupResult
-from synapse.types import Requester
+from synapse.types import Requester, UserID
 from synapse.util import Clock
 
 from tests import unittest
@@ -41,10 +41,12 @@
 
 
 class AuthTestCase(unittest.HomeserverTestCase):
-    def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer):
+    def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
         self.store = Mock()
 
-        hs.datastores.main = self.store
+        # type-ignore: datastores is None until hs.setup() is called---but it'll
+        # have been called by the HomeserverTestCase machinery.
+        hs.datastores.main = self.store  # type: ignore[union-attr]
         hs.get_auth_handler().store = self.store
         self.auth = Auth(hs)
 
@@ -61,7 +63,7 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer):
         self.store.insert_client_ip = simple_async_mock(None)
         self.store.is_support_user = simple_async_mock(False)
 
-    def test_get_user_by_req_user_valid_token(self):
+    def test_get_user_by_req_user_valid_token(self) -> None:
         user_info = TokenLookupResult(
             user_id=self.test_user, token_id=5, device_id="device"
         )
@@ -74,7 +76,7 @@ def test_get_user_by_req_user_valid_token(self):
         requester = self.get_success(self.auth.get_user_by_req(request))
         self.assertEqual(requester.user.to_string(), self.test_user)
 
-    def test_get_user_by_req_user_bad_token(self):
+    def test_get_user_by_req_user_bad_token(self) -> None:
         self.store.get_user_by_access_token = simple_async_mock(None)
 
         request = Mock(args={})
@@ -86,7 +88,7 @@ def test_get_user_by_req_user_bad_token(self):
         self.assertEqual(f.code, 401)
         self.assertEqual(f.errcode, "M_UNKNOWN_TOKEN")
 
-    def test_get_user_by_req_user_missing_token(self):
+    def test_get_user_by_req_user_missing_token(self) -> None:
         user_info = TokenLookupResult(user_id=self.test_user, token_id=5)
         self.store.get_user_by_access_token = simple_async_mock(user_info)
 
@@ -98,7 +100,7 @@ def test_get_user_by_req_user_missing_token(self):
         self.assertEqual(f.code, 401)
         self.assertEqual(f.errcode, "M_MISSING_TOKEN")
 
-    def test_get_user_by_req_appservice_valid_token(self):
+    def test_get_user_by_req_appservice_valid_token(self) -> None:
         app_service = Mock(
             token="foobar", url="a_url", sender=self.test_user, ip_range_whitelist=None
         )
@@ -112,7 +114,7 @@ def test_get_user_by_req_appservice_valid_token(self):
         requester = self.get_success(self.auth.get_user_by_req(request))
         self.assertEqual(requester.user.to_string(), self.test_user)
 
-    def test_get_user_by_req_appservice_valid_token_good_ip(self):
+    def test_get_user_by_req_appservice_valid_token_good_ip(self) -> None:
         from netaddr import IPSet
 
         app_service = Mock(
@@ -131,7 +133,7 @@ def test_get_user_by_req_appservice_valid_token_good_ip(self):
         requester = self.get_success(self.auth.get_user_by_req(request))
         self.assertEqual(requester.user.to_string(), self.test_user)
 
-    def test_get_user_by_req_appservice_valid_token_bad_ip(self):
+    def test_get_user_by_req_appservice_valid_token_bad_ip(self) -> None:
         from netaddr import IPSet
 
         app_service = Mock(
@@ -153,7 +155,7 @@ def test_get_user_by_req_appservice_valid_token_bad_ip(self):
         self.assertEqual(f.code, 401)
         self.assertEqual(f.errcode, "M_UNKNOWN_TOKEN")
 
-    def test_get_user_by_req_appservice_bad_token(self):
+    def test_get_user_by_req_appservice_bad_token(self) -> None:
         self.store.get_app_service_by_token = Mock(return_value=None)
         self.store.get_user_by_access_token = simple_async_mock(None)
 
@@ -166,7 +168,7 @@ def test_get_user_by_req_appservice_bad_token(self):
         self.assertEqual(f.code, 401)
         self.assertEqual(f.errcode, "M_UNKNOWN_TOKEN")
 
-    def test_get_user_by_req_appservice_missing_token(self):
+    def test_get_user_by_req_appservice_missing_token(self) -> None:
         app_service = Mock(token="foobar", url="a_url", sender=self.test_user)
         self.store.get_app_service_by_token = Mock(return_value=app_service)
         self.store.get_user_by_access_token = simple_async_mock(None)
@@ -179,7 +181,7 @@ def test_get_user_by_req_appservice_missing_token(self):
         self.assertEqual(f.code, 401)
         self.assertEqual(f.errcode, "M_MISSING_TOKEN")
 
-    def test_get_user_by_req_appservice_valid_token_valid_user_id(self):
+    def test_get_user_by_req_appservice_valid_token_valid_user_id(self) -> None:
         masquerading_user_id = b"@doppelganger:matrix.org"
         app_service = Mock(
             token="foobar", url="a_url", sender=self.test_user, ip_range_whitelist=None
@@ -200,7 +202,7 @@ def test_get_user_by_req_appservice_valid_token_valid_user_id(self):
             requester.user.to_string(), masquerading_user_id.decode("utf8")
         )
 
-    def test_get_user_by_req_appservice_valid_token_bad_user_id(self):
+    def test_get_user_by_req_appservice_valid_token_bad_user_id(self) -> None:
         masquerading_user_id = b"@doppelganger:matrix.org"
         app_service = Mock(
             token="foobar", url="a_url", sender=self.test_user, ip_range_whitelist=None
@@ -217,7 +219,7 @@ def test_get_user_by_req_appservice_valid_token_bad_user_id(self):
         self.get_failure(self.auth.get_user_by_req(request), AuthError)
 
     @override_config({"experimental_features": {"msc3202_device_masquerading": True}})
-    def test_get_user_by_req_appservice_valid_token_valid_device_id(self):
+    def test_get_user_by_req_appservice_valid_token_valid_device_id(self) -> None:
         """
         Tests that when an application service passes the device_id URL parameter
         with the ID of a valid device for the user in question,
@@ -249,7 +251,7 @@ def test_get_user_by_req_appservice_valid_token_valid_device_id(self):
         self.assertEqual(requester.device_id, masquerading_device_id.decode("utf8"))
 
     @override_config({"experimental_features": {"msc3202_device_masquerading": True}})
-    def test_get_user_by_req_appservice_valid_token_invalid_device_id(self):
+    def test_get_user_by_req_appservice_valid_token_invalid_device_id(self) -> None:
         """
         Tests that when an application service passes the device_id URL parameter
         with an ID that is not a valid device ID for the user in question,
@@ -279,7 +281,7 @@ def test_get_user_by_req_appservice_valid_token_invalid_device_id(self):
         self.assertEqual(failure.value.code, 400)
         self.assertEqual(failure.value.errcode, Codes.EXCLUSIVE)
 
-    def test_get_user_by_req__puppeted_token__not_tracking_puppeted_mau(self):
+    def test_get_user_by_req__puppeted_token__not_tracking_puppeted_mau(self) -> None:
         self.store.get_user_by_access_token = simple_async_mock(
             TokenLookupResult(
                 user_id="@baldrick:matrix.org",
@@ -298,7 +300,7 @@ def test_get_user_by_req__puppeted_token__not_tracking_puppeted_mau(self):
         self.get_success(self.auth.get_user_by_req(request))
         self.store.insert_client_ip.assert_called_once()
 
-    def test_get_user_by_req__puppeted_token__tracking_puppeted_mau(self):
+    def test_get_user_by_req__puppeted_token__tracking_puppeted_mau(self) -> None:
         self.auth._track_puppeted_user_ips = True
         self.store.get_user_by_access_token = simple_async_mock(
             TokenLookupResult(
@@ -318,7 +320,7 @@ def test_get_user_by_req__puppeted_token__tracking_puppeted_mau(self):
         self.get_success(self.auth.get_user_by_req(request))
         self.assertEqual(self.store.insert_client_ip.call_count, 2)
 
-    def test_get_user_from_macaroon(self):
+    def test_get_user_from_macaroon(self) -> None:
         self.store.get_user_by_access_token = simple_async_mock(None)
 
         user_id = "@baldrick:matrix.org"
@@ -336,7 +338,7 @@ def test_get_user_from_macaroon(self):
             self.auth.get_user_by_access_token(serialized), InvalidClientTokenError
         )
 
-    def test_get_guest_user_from_macaroon(self):
+    def test_get_guest_user_from_macaroon(self) -> None:
         self.store.get_user_by_id = simple_async_mock({"is_guest": True})
         self.store.get_user_by_access_token = simple_async_mock(None)
 
@@ -357,7 +359,7 @@ def test_get_guest_user_from_macaroon(self):
         self.assertTrue(user_info.is_guest)
         self.store.get_user_by_id.assert_called_with(user_id)
 
-    def test_blocking_mau(self):
+    def test_blocking_mau(self) -> None:
         self.auth_blocking._limit_usage_by_mau = False
         self.auth_blocking._max_mau_value = 50
         lots_of_users = 100
@@ -381,7 +383,7 @@ def test_blocking_mau(self):
         self.store.get_monthly_active_count = simple_async_mock(small_number_of_users)
         self.get_success(self.auth_blocking.check_auth_blocking())
 
-    def test_blocking_mau__depending_on_user_type(self):
+    def test_blocking_mau__depending_on_user_type(self) -> None:
         self.auth_blocking._max_mau_value = 50
         self.auth_blocking._limit_usage_by_mau = True
 
@@ -400,7 +402,9 @@ def test_blocking_mau__depending_on_user_type(self):
         # Real users not allowed
         self.get_failure(self.auth_blocking.check_auth_blocking(), ResourceLimitError)
 
-    def test_blocking_mau__appservice_requester_allowed_when_not_tracking_ips(self):
+    def test_blocking_mau__appservice_requester_allowed_when_not_tracking_ips(
+        self,
+    ) -> None:
         self.auth_blocking._max_mau_value = 50
         self.auth_blocking._limit_usage_by_mau = True
         self.auth_blocking._track_appservice_user_ips = False
@@ -418,7 +422,7 @@ def test_blocking_mau__appservice_requester_allowed_when_not_tracking_ips(self):
             sender="@appservice:sender",
         )
         requester = Requester(
-            user="@appservice:server",
+            user=UserID.from_string("@appservice:server"),
             access_token_id=None,
             device_id="FOOBAR",
             is_guest=False,
@@ -428,7 +432,9 @@ def test_blocking_mau__appservice_requester_allowed_when_not_tracking_ips(self):
         )
         self.get_success(self.auth_blocking.check_auth_blocking(requester=requester))
 
-    def test_blocking_mau__appservice_requester_disallowed_when_tracking_ips(self):
+    def test_blocking_mau__appservice_requester_disallowed_when_tracking_ips(
+        self,
+    ) -> None:
         self.auth_blocking._max_mau_value = 50
         self.auth_blocking._limit_usage_by_mau = True
         self.auth_blocking._track_appservice_user_ips = True
@@ -446,7 +452,7 @@ def test_blocking_mau__appservice_requester_disallowed_when_tracking_ips(self):
             sender="@appservice:sender",
         )
         requester = Requester(
-            user="@appservice:server",
+            user=UserID.from_string("@appservice:server"),
             access_token_id=None,
             device_id="FOOBAR",
             is_guest=False,
@@ -459,7 +465,7 @@ def test_blocking_mau__appservice_requester_disallowed_when_tracking_ips(self):
             ResourceLimitError,
         )
 
-    def test_reserved_threepid(self):
+    def test_reserved_threepid(self) -> None:
         self.auth_blocking._limit_usage_by_mau = True
         self.auth_blocking._max_mau_value = 1
         self.store.get_monthly_active_count = simple_async_mock(2)
@@ -476,7 +482,7 @@ def test_reserved_threepid(self):
 
         self.get_success(self.auth_blocking.check_auth_blocking(threepid=threepid))
 
-    def test_hs_disabled(self):
+    def test_hs_disabled(self) -> None:
         self.auth_blocking._hs_disabled = True
         self.auth_blocking._hs_disabled_message = "Reason for being disabled"
         e = self.get_failure(
@@ -486,7 +492,7 @@ def test_hs_disabled(self):
         self.assertEqual(e.value.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)
         self.assertEqual(e.value.code, 403)
 
-    def test_hs_disabled_no_server_notices_user(self):
+    def test_hs_disabled_no_server_notices_user(self) -> None:
         """Check that 'hs_disabled_message' works correctly when there is no
         server_notices user.
         """
@@ -503,7 +509,7 @@ def test_hs_disabled_no_server_notices_user(self):
         self.assertEqual(e.value.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)
         self.assertEqual(e.value.code, 403)
 
-    def test_server_notices_mxid_special_cased(self):
+    def test_server_notices_mxid_special_cased(self) -> None:
         self.auth_blocking._hs_disabled = True
         user = "@user:server"
         self.auth_blocking._server_notices_mxid = user