From 7a4b36d3aad5edb3be0b9a0b507a2372f61ef660 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 3 Feb 2023 17:47:45 +0000 Subject: [PATCH 01/13] Fix type errors in tests/api --- mypy.ini | 1 - tests/api/test_auth.py | 10 ++++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/mypy.ini b/mypy.ini index 978d92940b20..906e11d75fe5 100644 --- a/mypy.ini +++ b/mypy.ini @@ -32,7 +32,6 @@ exclude = (?x) |synapse/storage/databases/main/cache.py |synapse/storage/schema/ - |tests/api/test_auth.py |tests/app/test_openid_listener.py |tests/appservice/test_scheduler.py |tests/federation/test_federation_catch_up.py diff --git a/tests/api/test_auth.py b/tests/api/test_auth.py index e0f363555b3c..dcf31e881650 100644 --- a/tests/api/test_auth.py +++ b/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 @@ -44,7 +44,9 @@ class AuthTestCase(unittest.HomeserverTestCase): def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer): 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) @@ -418,7 +420,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, @@ -446,7 +448,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, From 5808c00855f6e770e9466bdaab59571624677456 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 3 Feb 2023 17:50:32 +0000 Subject: [PATCH 02/13] Mark test methods as returning None. --- tests/api/test_auth.py | 52 +++++++++++++----------- tests/api/test_filtering.py | 74 +++++++++++++++++----------------- tests/api/test_ratelimiting.py | 18 ++++----- 3 files changed, 74 insertions(+), 70 deletions(-) diff --git a/tests/api/test_auth.py b/tests/api/test_auth.py index dcf31e881650..927406678ae7 100644 --- a/tests/api/test_auth.py +++ b/tests/api/test_auth.py @@ -63,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" ) @@ -76,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={}) @@ -88,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) @@ -100,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 ) @@ -114,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( @@ -133,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( @@ -155,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) @@ -168,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) @@ -181,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 @@ -202,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 @@ -219,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, @@ -251,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, @@ -281,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", @@ -300,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( @@ -320,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" @@ -338,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) @@ -359,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 @@ -383,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 @@ -402,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 @@ -430,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 @@ -461,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) @@ -478,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( @@ -488,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. """ @@ -505,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 diff --git a/tests/api/test_filtering.py b/tests/api/test_filtering.py index d5524d296e38..e43b42311276 100644 --- a/tests/api/test_filtering.py +++ b/tests/api/test_filtering.py @@ -45,7 +45,7 @@ def prepare(self, reactor, clock, hs): self.filtering = hs.get_filtering() self.datastore = hs.get_datastores().main - def test_errors_on_invalid_filters(self): + def test_errors_on_invalid_filters(self) -> None: # See USER_FILTER_SCHEMA for the filter schema. invalid_filters = [ # `account_data` must be a dictionary @@ -63,7 +63,7 @@ def test_errors_on_invalid_filters(self): with self.assertRaises(SynapseError): self.filtering.check_valid_filter(filter) - def test_ignores_unknown_filter_fields(self): + def test_ignores_unknown_filter_fields(self) -> None: # For forward compatibility, we must ignore unknown filter fields. # See USER_FILTER_SCHEMA for the filter schema. filters = [ @@ -76,7 +76,7 @@ def test_ignores_unknown_filter_fields(self): self.filtering.check_valid_filter(filter) # Must not raise. - def test_valid_filters(self): + def test_valid_filters(self) -> None: valid_filters = [ { "room": { @@ -132,22 +132,22 @@ def test_valid_filters(self): except jsonschema.ValidationError as e: self.fail(e) - def test_limits_are_applied(self): + def test_limits_are_applied(self) -> None: # TODO pass - def test_definition_types_works_with_literals(self): + def test_definition_types_works_with_literals(self) -> None: definition = {"types": ["m.room.message", "org.matrix.foo.bar"]} event = MockEvent(sender="@foo:bar", type="m.room.message", room_id="!foo:bar") self.assertTrue(Filter(self.hs, definition)._check(event)) - def test_definition_types_works_with_wildcards(self): + def test_definition_types_works_with_wildcards(self) -> None: definition = {"types": ["m.*", "org.matrix.foo.bar"]} event = MockEvent(sender="@foo:bar", type="m.room.message", room_id="!foo:bar") self.assertTrue(Filter(self.hs, definition)._check(event)) - def test_definition_types_works_with_unknowns(self): + def test_definition_types_works_with_unknowns(self) -> None: definition = {"types": ["m.room.message", "org.matrix.foo.bar"]} event = MockEvent( sender="@foo:bar", @@ -156,24 +156,24 @@ def test_definition_types_works_with_unknowns(self): ) self.assertFalse(Filter(self.hs, definition)._check(event)) - def test_definition_not_types_works_with_literals(self): + def test_definition_not_types_works_with_literals(self) -> None: definition = {"not_types": ["m.room.message", "org.matrix.foo.bar"]} event = MockEvent(sender="@foo:bar", type="m.room.message", room_id="!foo:bar") self.assertFalse(Filter(self.hs, definition)._check(event)) - def test_definition_not_types_works_with_wildcards(self): + def test_definition_not_types_works_with_wildcards(self) -> None: definition = {"not_types": ["m.room.message", "org.matrix.*"]} event = MockEvent( sender="@foo:bar", type="org.matrix.custom.event", room_id="!foo:bar" ) self.assertFalse(Filter(self.hs, definition)._check(event)) - def test_definition_not_types_works_with_unknowns(self): + def test_definition_not_types_works_with_unknowns(self) -> None: definition = {"not_types": ["m.*", "org.*"]} event = MockEvent(sender="@foo:bar", type="com.nom.nom.nom", room_id="!foo:bar") self.assertTrue(Filter(self.hs, definition)._check(event)) - def test_definition_not_types_takes_priority_over_types(self): + def test_definition_not_types_takes_priority_over_types(self) -> None: definition = { "not_types": ["m.*", "org.*"], "types": ["m.room.message", "m.room.topic"], @@ -181,35 +181,35 @@ def test_definition_not_types_takes_priority_over_types(self): event = MockEvent(sender="@foo:bar", type="m.room.topic", room_id="!foo:bar") self.assertFalse(Filter(self.hs, definition)._check(event)) - def test_definition_senders_works_with_literals(self): + def test_definition_senders_works_with_literals(self) -> None: definition = {"senders": ["@flibble:wibble"]} event = MockEvent( sender="@flibble:wibble", type="com.nom.nom.nom", room_id="!foo:bar" ) self.assertTrue(Filter(self.hs, definition)._check(event)) - def test_definition_senders_works_with_unknowns(self): + def test_definition_senders_works_with_unknowns(self) -> None: definition = {"senders": ["@flibble:wibble"]} event = MockEvent( sender="@challenger:appears", type="com.nom.nom.nom", room_id="!foo:bar" ) self.assertFalse(Filter(self.hs, definition)._check(event)) - def test_definition_not_senders_works_with_literals(self): + def test_definition_not_senders_works_with_literals(self) -> None: definition = {"not_senders": ["@flibble:wibble"]} event = MockEvent( sender="@flibble:wibble", type="com.nom.nom.nom", room_id="!foo:bar" ) self.assertFalse(Filter(self.hs, definition)._check(event)) - def test_definition_not_senders_works_with_unknowns(self): + def test_definition_not_senders_works_with_unknowns(self) -> None: definition = {"not_senders": ["@flibble:wibble"]} event = MockEvent( sender="@challenger:appears", type="com.nom.nom.nom", room_id="!foo:bar" ) self.assertTrue(Filter(self.hs, definition)._check(event)) - def test_definition_not_senders_takes_priority_over_senders(self): + def test_definition_not_senders_takes_priority_over_senders(self) -> None: definition = { "not_senders": ["@misspiggy:muppets"], "senders": ["@kermit:muppets", "@misspiggy:muppets"], @@ -219,14 +219,14 @@ def test_definition_not_senders_takes_priority_over_senders(self): ) self.assertFalse(Filter(self.hs, definition)._check(event)) - def test_definition_rooms_works_with_literals(self): + def test_definition_rooms_works_with_literals(self) -> None: definition = {"rooms": ["!secretbase:unknown"]} event = MockEvent( sender="@foo:bar", type="m.room.message", room_id="!secretbase:unknown" ) self.assertTrue(Filter(self.hs, definition)._check(event)) - def test_definition_rooms_works_with_unknowns(self): + def test_definition_rooms_works_with_unknowns(self) -> None: definition = {"rooms": ["!secretbase:unknown"]} event = MockEvent( sender="@foo:bar", @@ -235,7 +235,7 @@ def test_definition_rooms_works_with_unknowns(self): ) self.assertFalse(Filter(self.hs, definition)._check(event)) - def test_definition_not_rooms_works_with_literals(self): + def test_definition_not_rooms_works_with_literals(self) -> None: definition = {"not_rooms": ["!anothersecretbase:unknown"]} event = MockEvent( sender="@foo:bar", @@ -244,7 +244,7 @@ def test_definition_not_rooms_works_with_literals(self): ) self.assertFalse(Filter(self.hs, definition)._check(event)) - def test_definition_not_rooms_works_with_unknowns(self): + def test_definition_not_rooms_works_with_unknowns(self) -> None: definition = {"not_rooms": ["!secretbase:unknown"]} event = MockEvent( sender="@foo:bar", @@ -253,7 +253,7 @@ def test_definition_not_rooms_works_with_unknowns(self): ) self.assertTrue(Filter(self.hs, definition)._check(event)) - def test_definition_not_rooms_takes_priority_over_rooms(self): + def test_definition_not_rooms_takes_priority_over_rooms(self) -> None: definition = { "not_rooms": ["!secretbase:unknown"], "rooms": ["!secretbase:unknown"], @@ -263,7 +263,7 @@ def test_definition_not_rooms_takes_priority_over_rooms(self): ) self.assertFalse(Filter(self.hs, definition)._check(event)) - def test_definition_combined_event(self): + def test_definition_combined_event(self) -> None: definition = { "not_senders": ["@misspiggy:muppets"], "senders": ["@kermit:muppets"], @@ -279,7 +279,7 @@ def test_definition_combined_event(self): ) self.assertTrue(Filter(self.hs, definition)._check(event)) - def test_definition_combined_event_bad_sender(self): + def test_definition_combined_event_bad_sender(self) -> None: definition = { "not_senders": ["@misspiggy:muppets"], "senders": ["@kermit:muppets"], @@ -295,7 +295,7 @@ def test_definition_combined_event_bad_sender(self): ) self.assertFalse(Filter(self.hs, definition)._check(event)) - def test_definition_combined_event_bad_room(self): + def test_definition_combined_event_bad_room(self) -> None: definition = { "not_senders": ["@misspiggy:muppets"], "senders": ["@kermit:muppets"], @@ -311,7 +311,7 @@ def test_definition_combined_event_bad_room(self): ) self.assertFalse(Filter(self.hs, definition)._check(event)) - def test_definition_combined_event_bad_type(self): + def test_definition_combined_event_bad_type(self) -> None: definition = { "not_senders": ["@misspiggy:muppets"], "senders": ["@kermit:muppets"], @@ -327,7 +327,7 @@ def test_definition_combined_event_bad_type(self): ) self.assertFalse(Filter(self.hs, definition)._check(event)) - def test_filter_labels(self): + def test_filter_labels(self) -> None: definition = {"org.matrix.labels": ["#fun"]} event = MockEvent( sender="@foo:bar", @@ -356,7 +356,7 @@ def test_filter_labels(self): ) self.assertTrue(Filter(self.hs, definition)._check(event)) - def test_filter_not_labels(self): + def test_filter_not_labels(self) -> None: definition = {"org.matrix.not_labels": ["#fun"]} event = MockEvent( sender="@foo:bar", @@ -377,7 +377,7 @@ def test_filter_not_labels(self): self.assertTrue(Filter(self.hs, definition)._check(event)) @unittest.override_config({"experimental_features": {"msc3874_enabled": True}}) - def test_filter_rel_type(self): + def test_filter_rel_type(self) -> None: definition = {"org.matrix.msc3874.rel_types": ["m.thread"]} event = MockEvent( sender="@foo:bar", @@ -407,7 +407,7 @@ def test_filter_rel_type(self): self.assertTrue(Filter(self.hs, definition)._check(event)) @unittest.override_config({"experimental_features": {"msc3874_enabled": True}}) - def test_filter_not_rel_type(self): + def test_filter_not_rel_type(self) -> None: definition = {"org.matrix.msc3874.not_rel_types": ["m.thread"]} event = MockEvent( sender="@foo:bar", @@ -436,7 +436,7 @@ def test_filter_not_rel_type(self): self.assertTrue(Filter(self.hs, definition)._check(event)) - def test_filter_presence_match(self): + def test_filter_presence_match(self) -> None: user_filter_json = {"presence": {"types": ["m.*"]}} filter_id = self.get_success( self.datastore.add_user_filter( @@ -455,7 +455,7 @@ def test_filter_presence_match(self): results = self.get_success(user_filter.filter_presence(events=events)) self.assertEqual(events, results) - def test_filter_presence_no_match(self): + def test_filter_presence_no_match(self) -> None: user_filter_json = {"presence": {"types": ["m.*"]}} filter_id = self.get_success( @@ -479,7 +479,7 @@ def test_filter_presence_no_match(self): results = self.get_success(user_filter.filter_presence(events=events)) self.assertEqual([], results) - def test_filter_room_state_match(self): + def test_filter_room_state_match(self) -> None: user_filter_json = {"room": {"state": {"types": ["m.*"]}}} filter_id = self.get_success( self.datastore.add_user_filter( @@ -498,7 +498,7 @@ def test_filter_room_state_match(self): results = self.get_success(user_filter.filter_room_state(events=events)) self.assertEqual(events, results) - def test_filter_room_state_no_match(self): + def test_filter_room_state_no_match(self) -> None: user_filter_json = {"room": {"state": {"types": ["m.*"]}}} filter_id = self.get_success( self.datastore.add_user_filter( @@ -519,7 +519,7 @@ def test_filter_room_state_no_match(self): results = self.get_success(user_filter.filter_room_state(events)) self.assertEqual([], results) - def test_filter_rooms(self): + def test_filter_rooms(self) -> None: definition = { "rooms": ["!allowed:example.com", "!excluded:example.com"], "not_rooms": ["!excluded:example.com"], @@ -535,7 +535,7 @@ def test_filter_rooms(self): self.assertEqual(filtered_room_ids, ["!allowed:example.com"]) - def test_filter_relations(self): + def test_filter_relations(self) -> None: events = [ # An event without a relation. MockEvent( @@ -574,7 +574,7 @@ async def events_have_relations(*args, **kwargs): ) self.assertEqual(filtered_events, events[1:]) - def test_add_filter(self): + def test_add_filter(self) -> None: user_filter_json = {"room": {"state": {"types": ["m.*"]}}} filter_id = self.get_success( @@ -595,7 +595,7 @@ def test_add_filter(self): ), ) - def test_get_filter(self): + def test_get_filter(self) -> None: user_filter_json = {"room": {"state": {"types": ["m.*"]}}} filter_id = self.get_success( diff --git a/tests/api/test_ratelimiting.py b/tests/api/test_ratelimiting.py index b5fd08d43711..fa6c1c02ce95 100644 --- a/tests/api/test_ratelimiting.py +++ b/tests/api/test_ratelimiting.py @@ -6,7 +6,7 @@ class TestRatelimiter(unittest.HomeserverTestCase): - def test_allowed_via_can_do_action(self): + def test_allowed_via_can_do_action(self) -> None: limiter = Ratelimiter( store=self.hs.get_datastores().main, clock=self.clock, @@ -31,7 +31,7 @@ def test_allowed_via_can_do_action(self): self.assertTrue(allowed) self.assertEqual(20.0, time_allowed) - def test_allowed_appservice_ratelimited_via_can_requester_do_action(self): + def test_allowed_appservice_ratelimited_via_can_requester_do_action(self) -> None: appservice = ApplicationService( token="fake_token", id="foo", @@ -64,7 +64,7 @@ def test_allowed_appservice_ratelimited_via_can_requester_do_action(self): self.assertTrue(allowed) self.assertEqual(20.0, time_allowed) - def test_allowed_appservice_via_can_requester_do_action(self): + def test_allowed_appservice_via_can_requester_do_action(self) -> None: appservice = ApplicationService( token="fake_token", id="foo", @@ -97,7 +97,7 @@ def test_allowed_appservice_via_can_requester_do_action(self): self.assertTrue(allowed) self.assertEqual(-1, time_allowed) - def test_allowed_via_ratelimit(self): + def test_allowed_via_ratelimit(self) -> None: limiter = Ratelimiter( store=self.hs.get_datastores().main, clock=self.clock, @@ -120,7 +120,7 @@ def test_allowed_via_ratelimit(self): limiter.ratelimit(None, key="test_id", _time_now_s=10) ) - def test_allowed_via_can_do_action_and_overriding_parameters(self): + def test_allowed_via_can_do_action_and_overriding_parameters(self) -> None: """Test that we can override options of can_do_action that would otherwise fail an action """ @@ -169,7 +169,7 @@ def test_allowed_via_can_do_action_and_overriding_parameters(self): self.assertTrue(allowed) self.assertEqual(1.0, time_allowed) - def test_allowed_via_ratelimit_and_overriding_parameters(self): + def test_allowed_via_ratelimit_and_overriding_parameters(self) -> None: """Test that we can override options of the ratelimit method that would otherwise fail an action """ @@ -204,7 +204,7 @@ def test_allowed_via_ratelimit_and_overriding_parameters(self): limiter.ratelimit(None, key=("test_id",), _time_now_s=1, burst_count=10) ) - def test_pruning(self): + def test_pruning(self) -> None: limiter = Ratelimiter( store=self.hs.get_datastores().main, clock=self.clock, @@ -223,7 +223,7 @@ def test_pruning(self): self.assertNotIn("test_id_1", limiter.actions) - def test_db_user_override(self): + def test_db_user_override(self) -> None: """Test that users that have ratelimiting disabled in the DB aren't ratelimited. """ @@ -250,7 +250,7 @@ def test_db_user_override(self): for _ in range(20): self.get_success_or_raise(limiter.ratelimit(requester, _time_now_s=0)) - def test_multiple_actions(self): + def test_multiple_actions(self) -> None: limiter = Ratelimiter( store=self.hs.get_datastores().main, clock=self.clock, From 21915dde65508a2d8a73cf2b49020d65e9ff1b01 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 3 Feb 2023 18:01:55 +0000 Subject: [PATCH 03/13] Easy untyped defs in test_filtering --- tests/api/test_filtering.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/tests/api/test_filtering.py b/tests/api/test_filtering.py index e43b42311276..026aa4fbd98e 100644 --- a/tests/api/test_filtering.py +++ b/tests/api/test_filtering.py @@ -14,23 +14,28 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - +from typing import List, Union from unittest.mock import patch import jsonschema from frozendict import frozendict +from twisted.test.proto_helpers import MemoryReactor + from synapse.api.constants import EduTypes, EventContentFields from synapse.api.errors import SynapseError from synapse.api.filtering import Filter -from synapse.events import make_event_from_dict +from synapse.events import EventBase, make_event_from_dict +from synapse.server import HomeServer +from synapse.types import JsonDict +from synapse.util import Clock from tests import unittest user_localpart = "test_user" -def MockEvent(**kwargs): +def MockEvent(**kwargs: object) -> EventBase: if "event_id" not in kwargs: kwargs["event_id"] = "fake_event_id" if "type" not in kwargs: @@ -41,13 +46,13 @@ def MockEvent(**kwargs): class FilteringTestCase(unittest.HomeserverTestCase): - def prepare(self, reactor, clock, hs): + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.filtering = hs.get_filtering() self.datastore = hs.get_datastores().main def test_errors_on_invalid_filters(self) -> None: # See USER_FILTER_SCHEMA for the filter schema. - invalid_filters = [ + invalid_filters: List[JsonDict] = [ # `account_data` must be a dictionary {"account_data": "Hello World"}, # `event_fields` entries must not contain backslashes @@ -66,7 +71,7 @@ def test_errors_on_invalid_filters(self) -> None: def test_ignores_unknown_filter_fields(self) -> None: # For forward compatibility, we must ignore unknown filter fields. # See USER_FILTER_SCHEMA for the filter schema. - filters = [ + filters: List[JsonDict] = [ {"org.matrix.msc9999.future_option": True}, {"presence": {"org.matrix.msc9999.future_option": True}}, {"room": {"org.matrix.msc9999.future_option": True}}, @@ -77,7 +82,7 @@ def test_ignores_unknown_filter_fields(self) -> None: # Must not raise. def test_valid_filters(self) -> None: - valid_filters = [ + valid_filters: List[JsonDict] = [ { "room": { "timeline": {"limit": 20}, @@ -536,7 +541,7 @@ def test_filter_rooms(self) -> None: self.assertEqual(filtered_room_ids, ["!allowed:example.com"]) def test_filter_relations(self) -> None: - events = [ + events: List[Union[EventBase, JsonDict]] = [ # An event without a relation. MockEvent( event_id="$no_relation", @@ -561,7 +566,7 @@ def test_filter_relations(self) -> None: # Filter for a particular sender. definition = {"related_by_senders": ["@foo:bar"]} - async def events_have_relations(*args, **kwargs): + async def events_have_relations(*args: object, **kwargs: object) -> List[str]: return ["$with_relation"] with patch.object( From 4b4a863e0abdaeb68fb71eac050f4ceb05827283 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 3 Feb 2023 18:03:19 +0000 Subject: [PATCH 04/13] Easy untyped defs in test_auth --- tests/api/test_auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/api/test_auth.py b/tests/api/test_auth.py index 927406678ae7..6e36e73f0d75 100644 --- a/tests/api/test_auth.py +++ b/tests/api/test_auth.py @@ -41,7 +41,7 @@ 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() # type-ignore: datastores is None until hs.setup() is called---but it'll From e11e6f3f5909a767fb4f31a014fc6f1f65a81567 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 3 Feb 2023 18:06:09 +0000 Subject: [PATCH 05/13] FilterEvent: Use a Union instead of TypeVar --- synapse/api/filtering.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/synapse/api/filtering.py b/synapse/api/filtering.py index 2b5af264b43d..1b8553dfc945 100644 --- a/synapse/api/filtering.py +++ b/synapse/api/filtering.py @@ -26,7 +26,6 @@ Mapping, Optional, Set, - TypeVar, Union, ) @@ -202,7 +201,7 @@ def check_valid_filter(self, user_filter_json: JsonDict) -> None: # Filters work across events, presence EDUs, and account data. -FilterEvent = TypeVar("FilterEvent", EventBase, UserPresenceState, JsonDict) +FilterEvent = Union["FilterEvent", EventBase, UserPresenceState, JsonDict] class FilterCollection: From 11d90e6b3cfcda6b51aa19bc85b9c4c819288660 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 3 Feb 2023 18:07:19 +0000 Subject: [PATCH 06/13] Mark this dir as having typed defs :) --- mypy.ini | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mypy.ini b/mypy.ini index 906e11d75fe5..7f523666ece5 100644 --- a/mypy.ini +++ b/mypy.ini @@ -73,6 +73,9 @@ disallow_untyped_defs = False [mypy-tests.*] disallow_untyped_defs = False +[mypy-tests.api.*] +disallow_untyped_defs = True + [mypy-tests.config.*] disallow_untyped_defs = True From 1f0d8dc050c9c7fb9503c9799e8cdefe1ac7bb8d Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 3 Feb 2023 18:17:41 +0000 Subject: [PATCH 07/13] Changelog --- changelog.d/14983.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/14983.misc diff --git a/changelog.d/14983.misc b/changelog.d/14983.misc new file mode 100644 index 000000000000..93ceaeafc9b9 --- /dev/null +++ b/changelog.d/14983.misc @@ -0,0 +1 @@ +Improve type hints. From 7d753abab1f3cc252689c4d9cfc5aa1572674c43 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 3 Feb 2023 18:44:38 +0000 Subject: [PATCH 08/13] Revert "FilterEvent: Use a Union instead of TypeVar" This reverts commit e11e6f3f5909a767fb4f31a014fc6f1f65a81567. --- synapse/api/filtering.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/api/filtering.py b/synapse/api/filtering.py index 1b8553dfc945..2b5af264b43d 100644 --- a/synapse/api/filtering.py +++ b/synapse/api/filtering.py @@ -26,6 +26,7 @@ Mapping, Optional, Set, + TypeVar, Union, ) @@ -201,7 +202,7 @@ def check_valid_filter(self, user_filter_json: JsonDict) -> None: # Filters work across events, presence EDUs, and account data. -FilterEvent = Union["FilterEvent", EventBase, UserPresenceState, JsonDict] +FilterEvent = TypeVar("FilterEvent", EventBase, UserPresenceState, JsonDict) class FilterCollection: From e37218a4e4a5710196912469075e42564210aa85 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 3 Feb 2023 18:45:09 +0000 Subject: [PATCH 09/13] Use existing version of `MockEvent` --- tests/api/test_filtering.py | 13 ++----------- tests/events/test_utils.py | 2 ++ 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/tests/api/test_filtering.py b/tests/api/test_filtering.py index 026aa4fbd98e..ae5ae1cf397f 100644 --- a/tests/api/test_filtering.py +++ b/tests/api/test_filtering.py @@ -25,26 +25,17 @@ from synapse.api.constants import EduTypes, EventContentFields from synapse.api.errors import SynapseError from synapse.api.filtering import Filter -from synapse.events import EventBase, make_event_from_dict +from synapse.events import EventBase from synapse.server import HomeServer from synapse.types import JsonDict from synapse.util import Clock from tests import unittest +from tests.events.test_utils import MockEvent user_localpart = "test_user" -def MockEvent(**kwargs: object) -> EventBase: - if "event_id" not in kwargs: - kwargs["event_id"] = "fake_event_id" - if "type" not in kwargs: - kwargs["type"] = "fake_type" - if "content" not in kwargs: - kwargs["content"] = {} - return make_event_from_dict(kwargs) - - class FilteringTestCase(unittest.HomeserverTestCase): def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.filtering = hs.get_filtering() diff --git a/tests/events/test_utils.py b/tests/events/test_utils.py index ff7b349d756c..4174a237ec84 100644 --- a/tests/events/test_utils.py +++ b/tests/events/test_utils.py @@ -35,6 +35,8 @@ def MockEvent(**kwargs: Any) -> EventBase: kwargs["event_id"] = "fake_event_id" if "type" not in kwargs: kwargs["type"] = "fake_type" + if "content" not in kwargs: + kwargs["content"] = {} return make_event_from_dict(kwargs) From 316fab01500cf2720dbc73dad16858a2afd15d15 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 3 Feb 2023 19:02:17 +0000 Subject: [PATCH 10/13] Fixup tests for `filter_presence` --- tests/api/test_filtering.py | 43 ++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/tests/api/test_filtering.py b/tests/api/test_filtering.py index ae5ae1cf397f..1fcbdf52c19f 100644 --- a/tests/api/test_filtering.py +++ b/tests/api/test_filtering.py @@ -25,6 +25,7 @@ from synapse.api.constants import EduTypes, EventContentFields from synapse.api.errors import SynapseError from synapse.api.filtering import Filter +from synapse.api.presence import UserPresenceState from synapse.events import EventBase from synapse.server import HomeServer from synapse.types import JsonDict @@ -433,14 +434,24 @@ def test_filter_not_rel_type(self) -> None: self.assertTrue(Filter(self.hs, definition)._check(event)) def test_filter_presence_match(self) -> None: - user_filter_json = {"presence": {"types": ["m.*"]}} + """Check that filter_presence return events which matches the filter.""" + user_filter_json = {"presence": {"senders": ["@foo:bar"]}} filter_id = self.get_success( self.datastore.add_user_filter( user_localpart=user_localpart, user_filter=user_filter_json ) ) - event = MockEvent(sender="@foo:bar", type="m.profile") - events = [event] + presence_states = [ + UserPresenceState( + user_id="@foo:bar", + state="unavailable", + last_active_ts=0, + last_federation_update_ts=0, + last_user_sync_ts=0, + status_msg=None, + currently_active=False, + ), + ] user_filter = self.get_success( self.filtering.get_user_filter( @@ -448,23 +459,29 @@ def test_filter_presence_match(self) -> None: ) ) - results = self.get_success(user_filter.filter_presence(events=events)) - self.assertEqual(events, results) + results = self.get_success(user_filter.filter_presence(presence_states)) + self.assertEqual(presence_states, results) def test_filter_presence_no_match(self) -> None: - user_filter_json = {"presence": {"types": ["m.*"]}} + """Check that filter_presence does not return events rejected by the filter.""" + user_filter_json = {"presence": {"not_senders": ["@foo:bar"]}} filter_id = self.get_success( self.datastore.add_user_filter( user_localpart=user_localpart + "2", user_filter=user_filter_json ) ) - event = MockEvent( - event_id="$asdasd:localhost", - sender="@foo:bar", - type="custom.avatar.3d.crazy", - ) - events = [event] + presence_states = [ + UserPresenceState( + user_id="@foo:bar", + state="unavailable", + last_active_ts=0, + last_federation_update_ts=0, + last_user_sync_ts=0, + status_msg=None, + currently_active=False, + ), + ] user_filter = self.get_success( self.filtering.get_user_filter( @@ -472,7 +489,7 @@ def test_filter_presence_no_match(self) -> None: ) ) - results = self.get_success(user_filter.filter_presence(events=events)) + results = self.get_success(user_filter.filter_presence(presence_states)) self.assertEqual([], results) def test_filter_room_state_match(self) -> None: From 3e3ef67781aac4d0ba07c8c1758139836c19a34b Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 3 Feb 2023 19:02:48 +0000 Subject: [PATCH 11/13] Rename filter_presence argument --- synapse/api/filtering.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/api/filtering.py b/synapse/api/filtering.py index 2b5af264b43d..83c42fc25a22 100644 --- a/synapse/api/filtering.py +++ b/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) From c9a411611b4f2669a536edf0abe8d22ec31b4c7e Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 3 Feb 2023 19:07:44 +0000 Subject: [PATCH 12/13] Call `_check_event_relations` w/ homogeneous lists --- tests/api/test_filtering.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/api/test_filtering.py b/tests/api/test_filtering.py index 1fcbdf52c19f..6dbb8c2df0a7 100644 --- a/tests/api/test_filtering.py +++ b/tests/api/test_filtering.py @@ -549,7 +549,7 @@ def test_filter_rooms(self) -> None: self.assertEqual(filtered_room_ids, ["!allowed:example.com"]) def test_filter_relations(self) -> None: - events: List[Union[EventBase, JsonDict]] = [ + events = [ # An event without a relation. MockEvent( event_id="$no_relation", @@ -564,9 +564,8 @@ def test_filter_relations(self) -> None: type="org.matrix.custom.event", room_id="!foo:bar", ), - # Non-EventBase objects get passed through. - {}, ] + jsondicts: List[JsonDict] = [{}] # For the following tests we patch the datastore method (intead of injecting # events). This is a bit cheeky, but tests the logic of _check_event_relations. @@ -585,7 +584,15 @@ async def events_have_relations(*args: object, **kwargs: object) -> List[str]: Filter(self.hs, definition)._check_event_relations(events) ) ) + # Non-EventBase objects get passed through. + filtered_jsondicts = list( + self.get_success( + Filter(self.hs, definition)._check_event_relations(jsondicts) + ) + ) + self.assertEqual(filtered_events, events[1:]) + self.assertEqual(filtered_jsondicts, [{}]) def test_add_filter(self) -> None: user_filter_json = {"room": {"state": {"types": ["m.*"]}}} From 0d96301c36e9c62d063a19425a0e53ebcdc4f270 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 3 Feb 2023 19:10:13 +0000 Subject: [PATCH 13/13] Whoops, removed unused imports --- tests/api/test_filtering.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/api/test_filtering.py b/tests/api/test_filtering.py index 6dbb8c2df0a7..0f45615160ed 100644 --- a/tests/api/test_filtering.py +++ b/tests/api/test_filtering.py @@ -14,7 +14,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -from typing import List, Union +from typing import List from unittest.mock import patch import jsonschema @@ -26,7 +26,6 @@ from synapse.api.errors import SynapseError from synapse.api.filtering import Filter from synapse.api.presence import UserPresenceState -from synapse.events import EventBase from synapse.server import HomeServer from synapse.types import JsonDict from synapse.util import Clock