Add a return type to parse_string. (#10438)

And set the required attribute in a few places which will error if
a parameter is not provided.

Author: clokep

changelog.d/10438.misc

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

synapse/http/servlet.py

@@ -172,14 +172,50 @@ def parse_bytes_from_args(
     return default
 
 
+@overload
+def parse_string(
+    request: Request,
+    name: str,
+    default: str,
+    *,
+    allowed_values: Optional[Iterable[str]] = None,
+    encoding: str = "ascii",
+) -> str:
+    ...
+
+
+@overload
+def parse_string(
+    request: Request,
+    name: str,
+    *,
+    required: Literal[True],
+    allowed_values: Optional[Iterable[str]] = None,
+    encoding: str = "ascii",
+) -> str:
+    ...
+
+
+@overload
+def parse_string(
+    request: Request,
+    name: str,
+    *,
+    required: bool = False,
+    allowed_values: Optional[Iterable[str]] = None,
+    encoding: str = "ascii",
+) -> Optional[str]:
+    ...
+
+
 def parse_string(
     request: Request,
     name: str,
     default: Optional[str] = None,
     required: bool = False,
     allowed_values: Optional[Iterable[str]] = None,
     encoding: str = "ascii",
-):
+) -> Optional[str]:
     """
     Parse a string parameter from the request query string.
 

synapse/rest/admin/users.py

@@ -90,8 +90,8 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
                 errcode=Codes.INVALID_PARAM,
             )
 
-        user_id = parse_string(request, "user_id", default=None)
-        name = parse_string(request, "name", default=None)
+        user_id = parse_string(request, "user_id")
+        name = parse_string(request, "name")
         guests = parse_boolean(request, "guests", default=True)
         deactivated = parse_boolean(request, "deactivated", default=False)
 

synapse/rest/client/v1/room.py

@@ -413,7 +413,7 @@ async def on_POST(self, request, room_id):
         assert_params_in_dict(body, ["state_events_at_start", "events"])
 
         prev_events_from_query = parse_strings_from_args(request.args, "prev_event")
-        chunk_id_from_query = parse_string(request, "chunk_id", default=None)
+        chunk_id_from_query = parse_string(request, "chunk_id")
 
         if prev_events_from_query is None:
             raise SynapseError(
@@ -735,7 +735,7 @@ def __init__(self, hs):
         self.auth = hs.get_auth()
 
     async def on_GET(self, request):
-        server = parse_string(request, "server", default=None)
+        server = parse_string(request, "server")
 
         try:
             await self.auth.get_user_by_req(request, allow_guest=True)
@@ -755,7 +755,7 @@ async def on_GET(self, request):
                 raise e
 
         limit = parse_integer(request, "limit", 0)
-        since_token = parse_string(request, "since", None)
+        since_token = parse_string(request, "since")
 
         if limit == 0:
             # zero is a special value which corresponds to no limit.
@@ -789,7 +789,7 @@ async def on_GET(self, request):
     async def on_POST(self, request):
         await self.auth.get_user_by_req(request, allow_guest=True)
 
-        server = parse_string(request, "server", default=None)
+        server = parse_string(request, "server")
         content = parse_json_object_from_request(request)
 
         limit: Optional[int] = int(content.get("limit", 100))

synapse/rest/client/v2_alpha/keys.py

@@ -194,7 +194,7 @@ def __init__(self, hs):
     async def on_GET(self, request):
         requester = await self.auth.get_user_by_req(request, allow_guest=True)
 
-        from_token_string = parse_string(request, "from")
+        from_token_string = parse_string(request, "from", required=True)
         set_tag("from", from_token_string)
 
         # We want to enforce they do pass us one, but we ignore it and return

synapse/rest/client/v2_alpha/relations.py

@@ -158,19 +158,21 @@ async def on_GET(
         event = await self.event_handler.get_event(requester.user, room_id, parent_id)
 
         limit = parse_integer(request, "limit", default=5)
-        from_token = parse_string(request, "from")
-        to_token = parse_string(request, "to")
+        from_token_str = parse_string(request, "from")
+        to_token_str = parse_string(request, "to")
 
         if event.internal_metadata.is_redacted():
             # If the event is redacted, return an empty list of relations
             pagination_chunk = PaginationChunk(chunk=[])
         else:
             # Return the relations
-            if from_token:
-                from_token = RelationPaginationToken.from_string(from_token)
+            from_token = None
+            if from_token_str:
+                from_token = RelationPaginationToken.from_string(from_token_str)
 
-            if to_token:
-                to_token = RelationPaginationToken.from_string(to_token)
+            to_token = None
+            if to_token_str:
+                to_token = RelationPaginationToken.from_string(to_token_str)
 
             pagination_chunk = await self.store.get_relations_for_event(
                 event_id=parent_id,
@@ -256,19 +258,21 @@ async def on_GET(
             raise SynapseError(400, "Relation type must be 'annotation'")
 
         limit = parse_integer(request, "limit", default=5)
-        from_token = parse_string(request, "from")
-        to_token = parse_string(request, "to")
+        from_token_str = parse_string(request, "from")
+        to_token_str = parse_string(request, "to")
 
         if event.internal_metadata.is_redacted():
             # If the event is redacted, return an empty list of relations
             pagination_chunk = PaginationChunk(chunk=[])
         else:
             # Return the relations
-            if from_token:
-                from_token = AggregationPaginationToken.from_string(from_token)
+            from_token = None
+            if from_token_str:
+                from_token = AggregationPaginationToken.from_string(from_token_str)
 
-            if to_token:
-                to_token = AggregationPaginationToken.from_string(to_token)
+            to_token = None
+            if to_token_str:
+                to_token = AggregationPaginationToken.from_string(to_token_str)
 
             pagination_chunk = await self.store.get_aggregation_groups_for_event(
                 event_id=parent_id,
@@ -336,14 +340,16 @@ async def on_GET(self, request, room_id, parent_id, relation_type, event_type, k
             raise SynapseError(400, "Relation type must be 'annotation'")
 
         limit = parse_integer(request, "limit", default=5)
-        from_token = parse_string(request, "from")
-        to_token = parse_string(request, "to")
+        from_token_str = parse_string(request, "from")
+        to_token_str = parse_string(request, "to")
 
-        if from_token:
-            from_token = RelationPaginationToken.from_string(from_token)
+        from_token = None
+        if from_token_str:
+            from_token = RelationPaginationToken.from_string(from_token_str)
 
-        if to_token:
-            to_token = RelationPaginationToken.from_string(to_token)
+        to_token = None
+        if to_token_str:
+            to_token = RelationPaginationToken.from_string(to_token_str)
 
         result = await self.store.get_relations_for_event(
             event_id=parent_id,

synapse/rest/client/v2_alpha/sync.py

@@ -112,7 +112,7 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
             default="online",
             allowed_values=self.ALLOWED_PRESENCE,
         )
-        filter_id = parse_string(request, "filter", default=None)
+        filter_id = parse_string(request, "filter")
         full_state = parse_boolean(request, "full_state", default=False)
 
         logger.debug(

synapse/rest/consent/consent_resource.py

@@ -112,7 +112,7 @@ async def _async_render_GET(self, request):
             request (twisted.web.http.Request):
         """
         version = parse_string(request, "v", default=self._default_consent_version)
-        username = parse_string(request, "u", required=False, default="")
+        username = parse_string(request, "u", default="")
         userhmac = None
         has_consented = False
         public_version = username == ""

synapse/rest/media/v1/preview_url_resource.py

@@ -186,15 +186,11 @@ async def _async_render_OPTIONS(self, request: Request) -> None:
         respond_with_json(request, 200, {}, send_cors=True)
 
     async def _async_render_GET(self, request: SynapseRequest) -> None:
-        # This will always be set by the time Twisted calls us.
-        assert request.args is not None
-
         # XXX: if get_user_by_req fails, what should we do in an async render?
         requester = await self.auth.get_user_by_req(request)
-        url = parse_string(request, "url")
-        if b"ts" in request.args:
-            ts = parse_integer(request, "ts")
-        else:
+        url = parse_string(request, "url", required=True)
+        ts = parse_integer(request, "ts")
+        if ts is None:
             ts = self.clock.time_msec()
 
         # XXX: we could move this into _do_preview if we wanted.

synapse/storage/databases/main/__init__.py

@@ -249,7 +249,7 @@ async def get_users_paginate(
         name: Optional[str] = None,
         guests: bool = True,
         deactivated: bool = False,
-        order_by: UserSortOrder = UserSortOrder.USER_ID.value,
+        order_by: str = UserSortOrder.USER_ID.value,
         direction: str = "f",
     ) -> Tuple[List[JsonDict], int]:
         """Function to retrieve a paginated list of users from