From ce5150ddda66154183039cbeb015c40570f42ca3 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Wed, 2 Jun 2021 08:29:45 +0200 Subject: [PATCH 1/3] Add missing type hints to the admin API servlets --- synapse/rest/admin/__init__.py | 43 +++++++++++++++++----------------- synapse/rest/admin/_base.py | 3 ++- synapse/rest/admin/groups.py | 12 ++++++++-- synapse/rest/admin/media.py | 21 +++++++++-------- synapse/rest/admin/users.py | 6 +---- 5 files changed, 46 insertions(+), 39 deletions(-) diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index 9cb9a9f6aa93..1730cddd2d34 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -17,11 +17,13 @@ import logging import platform +from typing import TYPE_CHECKING, Tuple import synapse from synapse.api.errors import Codes, NotFoundError, SynapseError -from synapse.http.server import JsonResource +from synapse.http.server import HttpServer, JsonResource from synapse.http.servlet import RestServlet, parse_json_object_from_request +from synapse.http.site import SynapseRequest from synapse.rest.admin._base import admin_patterns, assert_requester_is_admin from synapse.rest.admin.devices import ( DeleteDevicesRestServlet, @@ -66,22 +68,25 @@ UserTokenRestServlet, WhoisRestServlet, ) -from synapse.types import RoomStreamToken +from synapse.types import JsonDict, RoomStreamToken from synapse.util.versionstring import get_version_string +if TYPE_CHECKING: + from synapse.server import HomeServer + logger = logging.getLogger(__name__) class VersionServlet(RestServlet): PATTERNS = admin_patterns("/server_version$") - def __init__(self, hs): + def __init__(self, hs: "HomeServer") -> None: self.res = { "server_version": get_version_string(synapse), "python_version": platform.python_version(), } - def on_GET(self, request): + def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: return 200, self.res @@ -90,17 +95,14 @@ class PurgeHistoryRestServlet(RestServlet): "/purge_history/(?P[^/]*)(/(?P[^/]+))?" ) - def __init__(self, hs): - """ - - Args: - hs (synapse.server.HomeServer) - """ + def __init__(self, hs: "HomeServer") -> None: self.pagination_handler = hs.get_pagination_handler() self.store = hs.get_datastore() self.auth = hs.get_auth() - async def on_POST(self, request, room_id, event_id): + async def on_POST( + self, request: SynapseRequest, room_id: str, event_id: str + ) -> Tuple[int, JsonDict]: await assert_requester_is_admin(self.auth, request) body = parse_json_object_from_request(request, allow_empty_body=True) @@ -173,16 +175,13 @@ async def on_POST(self, request, room_id, event_id): class PurgeHistoryStatusRestServlet(RestServlet): PATTERNS = admin_patterns("/purge_history_status/(?P[^/]+)") - def __init__(self, hs): - """ - - Args: - hs (synapse.server.HomeServer) - """ + def __init__(self, hs: "HomeServer") -> None: self.pagination_handler = hs.get_pagination_handler() self.auth = hs.get_auth() - async def on_GET(self, request, purge_id): + async def on_GET( + self, request: SynapseRequest, purge_id: str + ) -> Tuple[int, JsonDict]: await assert_requester_is_admin(self.auth, request) purge_status = self.pagination_handler.get_purge_status(purge_id) @@ -203,12 +202,12 @@ async def on_GET(self, request, purge_id): class AdminRestResource(JsonResource): """The REST resource which gets mounted at /_synapse/admin""" - def __init__(self, hs): + def __init__(self, hs: "HomeServer") -> None: JsonResource.__init__(self, hs, canonical_json=False) register_servlets(hs, self) -def register_servlets(hs, http_server): +def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: """ Register all the admin servlets. """ @@ -242,7 +241,9 @@ def register_servlets(hs, http_server): RateLimitRestServlet(hs).register(http_server) -def register_servlets_for_client_rest_resource(hs, http_server): +def register_servlets_for_client_rest_resource( + hs: "HomeServer", http_server: HttpServer +) -> None: """Register only the servlets which need to be exposed on /_matrix/client/xxx""" WhoisRestServlet(hs).register(http_server) PurgeHistoryStatusRestServlet(hs).register(http_server) diff --git a/synapse/rest/admin/_base.py b/synapse/rest/admin/_base.py index f203f6fdc6e5..b482819ea366 100644 --- a/synapse/rest/admin/_base.py +++ b/synapse/rest/admin/_base.py @@ -13,6 +13,7 @@ # limitations under the License. import re +from typing import Pattern from synapse.api.auth import Auth from synapse.api.errors import AuthError @@ -20,7 +21,7 @@ from synapse.types import UserID -def admin_patterns(path_regex: str, version: str = "v1"): +def admin_patterns(path_regex: str, version: str = "v1") -> Pattern[str]: """Returns the list of patterns for an admin endpoint Args: diff --git a/synapse/rest/admin/groups.py b/synapse/rest/admin/groups.py index 3b3ffde0b65c..c9b9a77ee9c8 100644 --- a/synapse/rest/admin/groups.py +++ b/synapse/rest/admin/groups.py @@ -12,10 +12,16 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging +from typing import TYPE_CHECKING, Tuple from synapse.api.errors import SynapseError from synapse.http.servlet import RestServlet +from synapse.http.site import SynapseRequest from synapse.rest.admin._base import admin_patterns, assert_user_is_admin +from synapse.types import JsonDict + +if TYPE_CHECKING: + from synapse.server import HomeServer logger = logging.getLogger(__name__) @@ -25,12 +31,14 @@ class DeleteGroupAdminRestServlet(RestServlet): PATTERNS = admin_patterns("/delete_group/(?P[^/]*)") - def __init__(self, hs): + def __init__(self, hs: "HomeServer") -> None: self.group_server = hs.get_groups_server_handler() self.is_mine_id = hs.is_mine_id self.auth = hs.get_auth() - async def on_POST(self, request, group_id): + async def on_POST( + self, request: SynapseRequest, group_id: str + ) -> Tuple[int, JsonDict]: requester = await self.auth.get_user_by_req(request) await assert_user_is_admin(self.auth, requester.user) diff --git a/synapse/rest/admin/media.py b/synapse/rest/admin/media.py index 2c71af4279bf..18c92a2a6e8d 100644 --- a/synapse/rest/admin/media.py +++ b/synapse/rest/admin/media.py @@ -17,6 +17,7 @@ from typing import TYPE_CHECKING, Tuple from synapse.api.errors import AuthError, Codes, NotFoundError, SynapseError +from synapse.http.server import HttpServer from synapse.http.servlet import RestServlet, parse_boolean, parse_integer from synapse.http.site import SynapseRequest from synapse.rest.admin._base import ( @@ -44,7 +45,7 @@ class QuarantineMediaInRoom(RestServlet): admin_patterns("/quarantine_media/(?P[^/]+)") ) - def __init__(self, hs: "HomeServer"): + def __init__(self, hs: "HomeServer") -> None: self.store = hs.get_datastore() self.auth = hs.get_auth() @@ -71,7 +72,7 @@ class QuarantineMediaByUser(RestServlet): PATTERNS = admin_patterns("/user/(?P[^/]+)/media/quarantine") - def __init__(self, hs: "HomeServer"): + def __init__(self, hs: "HomeServer") -> None: self.store = hs.get_datastore() self.auth = hs.get_auth() @@ -100,7 +101,7 @@ class QuarantineMediaByID(RestServlet): "/media/quarantine/(?P[^/]+)/(?P[^/]+)" ) - def __init__(self, hs: "HomeServer"): + def __init__(self, hs: "HomeServer") -> None: self.store = hs.get_datastore() self.auth = hs.get_auth() @@ -125,7 +126,7 @@ class ProtectMediaByID(RestServlet): PATTERNS = admin_patterns("/media/protect/(?P[^/]+)") - def __init__(self, hs: "HomeServer"): + def __init__(self, hs: "HomeServer") -> None: self.store = hs.get_datastore() self.auth = hs.get_auth() @@ -148,7 +149,7 @@ class UnprotectMediaByID(RestServlet): PATTERNS = admin_patterns("/media/unprotect/(?P[^/]+)") - def __init__(self, hs: "HomeServer"): + def __init__(self, hs: "HomeServer") -> None: self.store = hs.get_datastore() self.auth = hs.get_auth() @@ -171,7 +172,7 @@ class ListMediaInRoom(RestServlet): PATTERNS = admin_patterns("/room/(?P[^/]+)/media") - def __init__(self, hs: "HomeServer"): + def __init__(self, hs: "HomeServer") -> None: self.store = hs.get_datastore() self.auth = hs.get_auth() @@ -191,7 +192,7 @@ async def on_GET( class PurgeMediaCacheRestServlet(RestServlet): PATTERNS = admin_patterns("/purge_media_cache") - def __init__(self, hs: "HomeServer"): + def __init__(self, hs: "HomeServer") -> None: self.media_repository = hs.get_media_repository() self.auth = hs.get_auth() @@ -211,7 +212,7 @@ class DeleteMediaByID(RestServlet): PATTERNS = admin_patterns("/media/(?P[^/]+)/(?P[^/]+)") - def __init__(self, hs: "HomeServer"): + def __init__(self, hs: "HomeServer") -> None: self.store = hs.get_datastore() self.auth = hs.get_auth() self.server_name = hs.hostname @@ -241,7 +242,7 @@ class DeleteMediaByDateSize(RestServlet): PATTERNS = admin_patterns("/media/(?P[^/]+)/delete") - def __init__(self, hs: "HomeServer"): + def __init__(self, hs: "HomeServer") -> None: self.store = hs.get_datastore() self.auth = hs.get_auth() self.server_name = hs.hostname @@ -283,7 +284,7 @@ async def on_POST( return 200, {"deleted_media": deleted_media, "total": total} -def register_servlets_for_media_repo(hs: "HomeServer", http_server): +def register_servlets_for_media_repo(hs: "HomeServer", http_server: HttpServer) -> None: """ Media repo specific APIs. """ diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 8c9d21d3ea1b..f1ef39ff01a4 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -553,11 +553,7 @@ async def on_POST( class AccountValidityRenewServlet(RestServlet): PATTERNS = admin_patterns("/account_validity/validity$") - def __init__(self, hs): - """ - Args: - hs (synapse.server.HomeServer): server - """ + def __init__(self, hs: "HomeServer") -> None: self.hs = hs self.account_activity_handler = hs.get_account_validity_handler() self.auth = hs.get_auth() From 9dd518d64f5cb2805a691cc304396214198ea3f8 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Wed, 2 Jun 2021 11:25:19 +0200 Subject: [PATCH 2/3] add changelog and mypy --- changelog.d/10105.misc | 1 + synapse/rest/admin/__init__.py | 6 ++++-- synapse/rest/admin/_base.py | 4 ++-- synapse/rest/admin/media.py | 9 ++++----- synapse/rest/admin/users.py | 9 ++++----- 5 files changed, 15 insertions(+), 14 deletions(-) create mode 100644 changelog.d/10105.misc diff --git a/changelog.d/10105.misc b/changelog.d/10105.misc new file mode 100644 index 000000000000..244a893d3e42 --- /dev/null +++ b/changelog.d/10105.misc @@ -0,0 +1 @@ +Add missing type hints to the admin API servlets. \ No newline at end of file diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index 1730cddd2d34..df75b273630e 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -17,7 +17,7 @@ import logging import platform -from typing import TYPE_CHECKING, Tuple +from typing import TYPE_CHECKING, Optional, Tuple import synapse from synapse.api.errors import Codes, NotFoundError, SynapseError @@ -101,7 +101,7 @@ def __init__(self, hs: "HomeServer") -> None: self.auth = hs.get_auth() async def on_POST( - self, request: SynapseRequest, room_id: str, event_id: str + self, request: SynapseRequest, room_id: str, event_id: Optional[str] ) -> Tuple[int, JsonDict]: await assert_requester_is_admin(self.auth, request) @@ -121,6 +121,8 @@ async def on_POST( if event.room_id != room_id: raise SynapseError(400, "Event is for wrong room.") + # RoomStreamToken expects [int] not Optional[int] + assert event.internal_metadata.stream_ordering is not None room_token = RoomStreamToken( event.depth, event.internal_metadata.stream_ordering ) diff --git a/synapse/rest/admin/_base.py b/synapse/rest/admin/_base.py index b482819ea366..d9a2f6ca157f 100644 --- a/synapse/rest/admin/_base.py +++ b/synapse/rest/admin/_base.py @@ -13,7 +13,7 @@ # limitations under the License. import re -from typing import Pattern +from typing import Iterable, Pattern from synapse.api.auth import Auth from synapse.api.errors import AuthError @@ -21,7 +21,7 @@ from synapse.types import UserID -def admin_patterns(path_regex: str, version: str = "v1") -> Pattern[str]: +def admin_patterns(path_regex: str, version: str = "v1") -> Iterable[Pattern]: """Returns the list of patterns for an admin endpoint Args: diff --git a/synapse/rest/admin/media.py b/synapse/rest/admin/media.py index 18c92a2a6e8d..ce46216b9f43 100644 --- a/synapse/rest/admin/media.py +++ b/synapse/rest/admin/media.py @@ -38,12 +38,11 @@ class QuarantineMediaInRoom(RestServlet): this server. """ - PATTERNS = ( - admin_patterns("/room/(?P[^/]+)/media/quarantine") - + + PATTERNS = [ + *admin_patterns("/room/(?P[^/]+)/media/quarantine"), # This path kept around for legacy reasons - admin_patterns("/quarantine_media/(?P[^/]+)") - ) + *admin_patterns("/quarantine_media/(?P[^/]+)"), + ] def __init__(self, hs: "HomeServer") -> None: self.store = hs.get_datastore() diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index f1ef39ff01a4..85e18bfda447 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -478,13 +478,12 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: class WhoisRestServlet(RestServlet): path_regex = "/whois/(?P[^/]*)$" - PATTERNS = ( - admin_patterns(path_regex) - + + PATTERNS = [ + *admin_patterns(path_regex), # URL for spec reason # https://matrix.org/docs/spec/client_server/r0.6.1#get-matrix-client-r0-admin-whois-userid - client_patterns("/admin" + path_regex, v1=True) - ) + *client_patterns("/admin" + path_regex, v1=True), + ] def __init__(self, hs: "HomeServer"): self.hs = hs From a7e600b44649ab1b95a8c8d8acb2aa10678a737b Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Wed, 2 Jun 2021 11:37:13 +0200 Subject: [PATCH 3/3] remove return type hint from `__init__` --- synapse/rest/admin/__init__.py | 8 ++++---- synapse/rest/admin/groups.py | 2 +- synapse/rest/admin/media.py | 18 +++++++++--------- synapse/rest/admin/users.py | 2 +- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index df75b273630e..abf749b001ab 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -80,7 +80,7 @@ class VersionServlet(RestServlet): PATTERNS = admin_patterns("/server_version$") - def __init__(self, hs: "HomeServer") -> None: + def __init__(self, hs: "HomeServer"): self.res = { "server_version": get_version_string(synapse), "python_version": platform.python_version(), @@ -95,7 +95,7 @@ class PurgeHistoryRestServlet(RestServlet): "/purge_history/(?P[^/]*)(/(?P[^/]+))?" ) - def __init__(self, hs: "HomeServer") -> None: + def __init__(self, hs: "HomeServer"): self.pagination_handler = hs.get_pagination_handler() self.store = hs.get_datastore() self.auth = hs.get_auth() @@ -177,7 +177,7 @@ async def on_POST( class PurgeHistoryStatusRestServlet(RestServlet): PATTERNS = admin_patterns("/purge_history_status/(?P[^/]+)") - def __init__(self, hs: "HomeServer") -> None: + def __init__(self, hs: "HomeServer"): self.pagination_handler = hs.get_pagination_handler() self.auth = hs.get_auth() @@ -204,7 +204,7 @@ async def on_GET( class AdminRestResource(JsonResource): """The REST resource which gets mounted at /_synapse/admin""" - def __init__(self, hs: "HomeServer") -> None: + def __init__(self, hs: "HomeServer"): JsonResource.__init__(self, hs, canonical_json=False) register_servlets(hs, self) diff --git a/synapse/rest/admin/groups.py b/synapse/rest/admin/groups.py index c9b9a77ee9c8..68a3ba3cb7ac 100644 --- a/synapse/rest/admin/groups.py +++ b/synapse/rest/admin/groups.py @@ -31,7 +31,7 @@ class DeleteGroupAdminRestServlet(RestServlet): PATTERNS = admin_patterns("/delete_group/(?P[^/]*)") - def __init__(self, hs: "HomeServer") -> None: + def __init__(self, hs: "HomeServer"): self.group_server = hs.get_groups_server_handler() self.is_mine_id = hs.is_mine_id self.auth = hs.get_auth() diff --git a/synapse/rest/admin/media.py b/synapse/rest/admin/media.py index ce46216b9f43..625c2707dad2 100644 --- a/synapse/rest/admin/media.py +++ b/synapse/rest/admin/media.py @@ -44,7 +44,7 @@ class QuarantineMediaInRoom(RestServlet): *admin_patterns("/quarantine_media/(?P[^/]+)"), ] - def __init__(self, hs: "HomeServer") -> None: + def __init__(self, hs: "HomeServer"): self.store = hs.get_datastore() self.auth = hs.get_auth() @@ -71,7 +71,7 @@ class QuarantineMediaByUser(RestServlet): PATTERNS = admin_patterns("/user/(?P[^/]+)/media/quarantine") - def __init__(self, hs: "HomeServer") -> None: + def __init__(self, hs: "HomeServer"): self.store = hs.get_datastore() self.auth = hs.get_auth() @@ -100,7 +100,7 @@ class QuarantineMediaByID(RestServlet): "/media/quarantine/(?P[^/]+)/(?P[^/]+)" ) - def __init__(self, hs: "HomeServer") -> None: + def __init__(self, hs: "HomeServer"): self.store = hs.get_datastore() self.auth = hs.get_auth() @@ -125,7 +125,7 @@ class ProtectMediaByID(RestServlet): PATTERNS = admin_patterns("/media/protect/(?P[^/]+)") - def __init__(self, hs: "HomeServer") -> None: + def __init__(self, hs: "HomeServer"): self.store = hs.get_datastore() self.auth = hs.get_auth() @@ -148,7 +148,7 @@ class UnprotectMediaByID(RestServlet): PATTERNS = admin_patterns("/media/unprotect/(?P[^/]+)") - def __init__(self, hs: "HomeServer") -> None: + def __init__(self, hs: "HomeServer"): self.store = hs.get_datastore() self.auth = hs.get_auth() @@ -171,7 +171,7 @@ class ListMediaInRoom(RestServlet): PATTERNS = admin_patterns("/room/(?P[^/]+)/media") - def __init__(self, hs: "HomeServer") -> None: + def __init__(self, hs: "HomeServer"): self.store = hs.get_datastore() self.auth = hs.get_auth() @@ -191,7 +191,7 @@ async def on_GET( class PurgeMediaCacheRestServlet(RestServlet): PATTERNS = admin_patterns("/purge_media_cache") - def __init__(self, hs: "HomeServer") -> None: + def __init__(self, hs: "HomeServer"): self.media_repository = hs.get_media_repository() self.auth = hs.get_auth() @@ -211,7 +211,7 @@ class DeleteMediaByID(RestServlet): PATTERNS = admin_patterns("/media/(?P[^/]+)/(?P[^/]+)") - def __init__(self, hs: "HomeServer") -> None: + def __init__(self, hs: "HomeServer"): self.store = hs.get_datastore() self.auth = hs.get_auth() self.server_name = hs.hostname @@ -241,7 +241,7 @@ class DeleteMediaByDateSize(RestServlet): PATTERNS = admin_patterns("/media/(?P[^/]+)/delete") - def __init__(self, hs: "HomeServer") -> None: + def __init__(self, hs: "HomeServer"): self.store = hs.get_datastore() self.auth = hs.get_auth() self.server_name = hs.hostname diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 85e18bfda447..7d75564758bf 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -552,7 +552,7 @@ async def on_POST( class AccountValidityRenewServlet(RestServlet): PATTERNS = admin_patterns("/account_validity/validity$") - def __init__(self, hs: "HomeServer") -> None: + def __init__(self, hs: "HomeServer"): self.hs = hs self.account_activity_handler = hs.get_account_validity_handler() self.auth = hs.get_auth()