Improve type hints relating to query parameters

The regression fixed by #12410 was not spotted because we annotated args as a Dict[str, Any] but then added args["limit"] as an int. This is fine because int is a subtype of Any.

synapse/synapse/federation/transport/client.py

Lines 506 to 519 in 1a90c1e

    
           args: Dict[str, Any] = { 
        
               "include_all_networks": "true" if include_all_networks else "false" 
        
           } 
        
           if third_party_instance_id: 
        
               args["third_party_instance_id"] = (third_party_instance_id,) 
        
           if limit: 
        
               args["limit"] = [limit] 
        
           if since_token: 
        
               args["since"] = [since_token] 
        
           try: 
        
               response = await self.client.get_json( 
        
                   destination=remote_server, path=path, args=args, ignore_backoff=True 
        
               )

We pass args to get_json, which expects args: Optional[QueryArgs], which is:

synapse/synapse/http/matrixfederationclient.py

Line 102 in bc9dff1

QueryArgs = Dict[str, Union[str, List[str]]]

This isn't found by mypy because args: Dict[str, Any] means that we do no typechecking of the values in args.

All functions which accept args: QueryArgs forward args as the query attribute of MatrixFederationRequest. That attribute is typed more weakly as Dict[Any, Any].

synapse/synapse/http/matrixfederationclient.py

Lines 147 to 149 in bc9dff1

    
               query: Optional[dict] = None 
        
               """Query arguments. 
        
               """

The query attribute is seems to only ever be passed to encode_query_args. That expects a type similar to QueryArgs, and converts its argument list in a form that can be passed to urlencode.

synapse/synapse/http/client.py

Lines 914 to 935 in a121507

    
           def encode_query_args(args: Optional[Mapping[str, Union[str, List[str]]]]) -> bytes: 
        
               """ 
        
               Encodes a map of query arguments to bytes which can be appended to a URL. 
        
               Args: 
        
                   args: The query arguments, a mapping of string to string or list of strings. 
        
               Returns: 
        
                   The query arguments encoded as bytes. 
        
               """ 
        
               if args is None: 
        
                   return b"" 
        
               encoded_args = {} 
        
               for k, vs in args.items(): 
        
                   if isinstance(vs, str): 
        
                       vs = [vs] 
        
                   encoded_args[k] = [v.encode("utf8") for v in vs] 
        
               query_str = urllib.parse.urlencode(encoded_args, True) 
        
               return query_str.encode("utf8")

We also have a QueryParams type elsewhere, which again is only forwarded to or passed to urlencode.

synapse/synapse/http/client.py

Lines 100 to 102 in a121507

    
           # the type of the query params, to be passed into `urlencode` 
        
           QueryParamValue = Union[str, bytes, Iterable[Union[str, bytes]]] 
        
           QueryParams = Union[Mapping[str, QueryParamValue], Mapping[bytes, QueryParamValue]]

I would like to see:

A stronger type annotation for MatrixFederationRequest.params
Some unification of QueryArgs and QueryParams
Some kind of audit of how we pass query parameters to MatrixFederationRequest
- at the very least something that would have caught the regression fixed by Fix fetching public rooms over federation #12410.

For completeness, typeshed's annotation of urlencode is here: https://github.com/python/typeshed/blob/754f421990e9b7f7f9a38481ad0fbd46e05b3d38/stdlib/urllib/parse.pyi#L148-L155

Navigation Menu

Repository navigation

Improve type hints relating to query parameters #12411

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	args: Dict[str, Any] = {
	"include_all_networks": "true" if include_all_networks else "false"
	}
	if third_party_instance_id:
	args["third_party_instance_id"] = (third_party_instance_id,)
	if limit:
	args["limit"] = [limit]
	if since_token:
	args["since"] = [since_token]

	try:
	response = await self.client.get_json(
	destination=remote_server, path=path, args=args, ignore_backoff=True
	)

	def encode_query_args(args: Optional[Mapping[str, Union[str, List[str]]]]) -> bytes:
	"""
	Encodes a map of query arguments to bytes which can be appended to a URL.

	Args:
	args: The query arguments, a mapping of string to string or list of strings.

	Returns:
	The query arguments encoded as bytes.
	"""
	if args is None:
	return b""

	encoded_args = {}
	for k, vs in args.items():
	if isinstance(vs, str):
	vs = [vs]
	encoded_args[k] = [v.encode("utf8") for v in vs]

	query_str = urllib.parse.urlencode(encoded_args, True)

	return query_str.encode("utf8")

	# the type of the query params, to be passed into `urlencode`
	QueryParamValue = Union[str, bytes, Iterable[Union[str, bytes]]]
	QueryParams = Union[Mapping[str, QueryParamValue], Mapping[bytes, QueryParamValue]]

Improve type hints relating to query parameters #12411

Description

Activity

DMRobertson commented on Apr 7, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions