Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add a return type to parse_string. #10438

Merged
merged 3 commits into from
Jul 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/10438.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve servlet type hints.
38 changes: 37 additions & 1 deletion synapse/http/servlet.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not Literal[False]?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it doesn't matter if it is provided or not, the default is 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.

Expand Down
4 changes: 2 additions & 2 deletions synapse/rest/admin/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
8 changes: 4 additions & 4 deletions synapse/rest/client/v1/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -726,7 +726,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)
Expand All @@ -746,7 +746,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.
Expand Down Expand Up @@ -780,7 +780,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))
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/client/v2_alpha/keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be an actual bugfix.

set_tag("from", from_token_string)

# We want to enforce they do pass us one, but we ignore it and return
Expand Down
42 changes: 24 additions & 18 deletions synapse/rest/client/v2_alpha/relations.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/client/v2_alpha/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/consent/consent_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 == ""
Expand Down
10 changes: 3 additions & 7 deletions synapse/rest/media/v1/preview_url_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be an actual bugfix.

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.
Expand Down
2 changes: 1 addition & 1 deletion synapse/storage/databases/main/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion synapse/storage/databases/main/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ async def get_rooms_paginate(
self,
start: int,
limit: int,
order_by: RoomSortOrder,
order_by: str,
reverse_order: bool,
search_term: Optional[str],
) -> Tuple[List[Dict[str, Any]], int]:
Expand Down
2 changes: 1 addition & 1 deletion synapse/storage/databases/main/stats.py
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,7 @@ async def get_users_media_usage_paginate(
limit: int,
from_ts: Optional[int] = None,
until_ts: Optional[int] = None,
order_by: Optional[UserSortOrder] = UserSortOrder.USER_ID.value,
order_by: Optional[str] = UserSortOrder.USER_ID.value,
direction: Optional[str] = "f",
search_term: Optional[str] = None,
) -> Tuple[List[JsonDict], Dict[str, int]]:
Expand Down
16 changes: 9 additions & 7 deletions synapse/streams/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,20 +47,22 @@ async def from_request(
) -> "PaginationConfig":
direction = parse_string(request, "dir", default="f", allowed_values=["f", "b"])

from_tok = parse_string(request, "from")
to_tok = parse_string(request, "to")
from_tok_str = parse_string(request, "from")
to_tok_str = parse_string(request, "to")

try:
if from_tok == "END":
from_tok = None
if from_tok_str == "END":
from_tok = None # For backwards compat.
elif from_tok:
from_tok = await StreamToken.from_string(store, from_tok)
elif from_tok_str:
from_tok = await StreamToken.from_string(store, from_tok_str)
except Exception:
raise SynapseError(400, "'from' parameter is invalid")

try:
if to_tok:
to_tok = await StreamToken.from_string(store, to_tok)
to_tok = None
if to_tok_str:
to_tok = await StreamToken.from_string(store, to_tok_str)
except Exception:
raise SynapseError(400, "'to' parameter is invalid")

Expand Down