URL encoding in query function and refactor

1. query now accepts other things as part of str.format
2. refactor all use of the query function in the project
3. function documentation could be much better
4. needs documentation on how to use query function

synadm/api.py

@@ -74,15 +74,17 @@ def __init__(self, log, user, token, base_url, path, timeout, debug,
         self.verify = verify
 
     def query(self, method, urlpart, params=None, data=None, token=None,
-              base_url_override=None, verify=None):
+              base_url_override=None, verify=None, *args, **kwargs):
         """Generic wrapper around requests methods.
 
-        Handles requests methods, logging and exceptions.
+        Handles requests methods, logging and exceptions, and URL encoding.
 
         Args:
             urlpart (string): The path to the API endpoint, excluding
                 self.base_url and self.path (the part after
-                proto://fqdn:port/path).
+                proto://fqdn:port/path). It will be passed to Python's
+                str.format, so the string should not be already formatted
+                (as f-strings or with str.format) as to sanitize the URL.
             params (dict, optional): URL parameters (?param1&paarm2).  Defaults
                 to None.
             data (dict, optional): Request body used in POST, PUT, DELETE
@@ -91,6 +93,10 @@ def query(self, method, urlpart, params=None, data=None, token=None,
                 on initialization can be overwritten using this argument.
             verify(bool): Mandatory SSL verification is turned on by default
                 and can be turned off using this method.
+            *args: Arguments that will be URL encoded and passed to Python's
+                str.format.
+            **kwargs: Keyword arguments that will be URL encoded (only the
+                values) and passed to Python's str.format.
 
         Returns:
             string or None: Usually a JSON string containing
@@ -99,6 +105,14 @@ def query(self, method, urlpart, params=None, data=None, token=None,
                 JSON strings. On exceptions the error type and description are
                 logged and None is returned.
         """
+        args = list(args)
+        kwargs = dict(kwargs)
+        for i in range(len(args)):
+            args[i] = urllib.parse.quote(args[i], safe="")
+        for i in kwargs.keys():
+            kwargs[i] = urllib.parse.quote(kwargs[i], safe="")
+        urlpart = urlpart.format(*args, **kwargs)
+
         if base_url_override:
             self.log.debug("base_url override!")
             url = f"{base_url_override}/{self.path}/{urlpart}"
@@ -309,7 +323,8 @@ def room_get_id(self, room_alias):
                 response as it might contain Synapse's error message.
         """
         room_directory = self.query(
-            "get", f"client/r0/directory/room/{urllib.parse.quote(room_alias)}"
+            "get", "client/r0/directory/room/{room_alias}",
+            room_alias=room_alias
         )
         if "room_id" in room_directory:
             return room_directory["room_id"]
@@ -327,7 +342,8 @@ def room_get_aliases(self, room_id):
                 error message or None on exceptions.
         """
         return self.query(
-            "get", f"client/r0/rooms/{urllib.parse.quote(room_id)}/aliases"
+            "get", "client/r0/rooms/{room_id}/aliases",
+            room_id=room_id
         )
 
     def raw_request(self, endpoint, method, data, token=None):
@@ -466,7 +482,8 @@ def user_membership(self, user_id, return_aliases, matrix_api):
                 an exception occured. See Synapse admin API docs for details.
         """
 
-        rooms = self.query("get", f"v1/users/{user_id}/joined_rooms")
+        rooms = self.query("get", "v1/users/{user_id}/joined_rooms",
+                           user_id=user_id)
         # Translate room ID's into aliases if requested.
         if return_aliases and rooms is not None and "joined_rooms" in rooms:
             for i, room_id in enumerate(rooms["joined_rooms"]):
@@ -487,9 +504,9 @@ def user_deactivate(self, user_id, gdpr_erase):
             string: JSON string containing the admin API's response or None if
                 an exception occured. See Synapse admin API docs for details.
         """
-        return self.query("post", f"v1/deactivate/{user_id}", data={
+        return self.query("post", "v1/deactivate/{user_id}", data={
             "erase": gdpr_erase
-        })
+        }, user_id=user_id)
 
     def user_password(self, user_id, password, no_logout):
         """Set the user password, and log the user out if requested
@@ -508,7 +525,8 @@ def user_password(self, user_id, password, no_logout):
         data = {"new_password": password}
         if no_logout:
             data.update({"logout_devices": False})
-        return self.query("post", f"v1/reset_password/{user_id}", data=data)
+        return self.query("post", "v1/reset_password/{user_id}", data=data,
+                          user_id=user_id)
 
     def user_details(self, user_id):
         """Get information about a given user
@@ -524,7 +542,7 @@ def user_details(self, user_id):
                 an exception occured. See Synapse admin API docs for details.
 
         """
-        return self.query("get", f"v2/users/{user_id}")
+        return self.query("get", "v2/users/{user_id}", user_id=user_id)
 
     def user_login(self, user_id, expire_days, expire, _expire_ts):
         """Get an access token that can be used to authenticate as that user.
@@ -573,7 +591,8 @@ def user_login(self, user_id, expire_days, expire, _expire_ts):
         else:
             self.log.info("Token will never expire.")
 
-        return self.query("post", f"v1/users/{user_id}/login", data=data)
+        return self.query("post", "v1/users/{user_id}/login", data=data,
+                          user_id=user_id)
 
     def user_modify(self, user_id, password, display_name, threepid,
                     avatar_url, admin, deactivation, user_type):
@@ -601,12 +620,13 @@ def user_modify(self, user_id, password, display_name, threepid,
         if user_type:
             data.update({"user_type": None if user_type == 'null' else
                          user_type})
-        return self.query("put", f"v2/users/{user_id}", data=data)
+        return self.query("put", "v2/users/{user_id}", data=data,
+                          user_id=user_id)
 
     def user_whois(self, user_id):
         """ Return information about the active sessions for a specific user
         """
-        return self.query("get", f"v1/whois/{user_id}")
+        return self.query("get", "v1/whois/{user_id}", user_id=user_id)
 
     def user_devices(self, user_id):
         """ Return information about all devices for a specific user.
@@ -618,7 +638,8 @@ def user_devices(self, user_id):
             string: JSON string containing the admin API's response or None if
                 an exception occured. See Synapse admin API docs for details.
         """
-        return self.query("get", f"v2/users/{user_id}/devices")
+        return self.query("get", "v2/users/{user_id}/devices",
+                          user_id=user_id)
 
     def user_devices_get_todelete(self, devices_data, min_days, min_surviving,
                                   device_id, readable_seen):
@@ -705,28 +726,31 @@ def user_devices_delete(self, user_id, devices):
 
         devices is a list of device IDs
         """
-        return self.query("post", f"v2/users/{user_id}/delete_devices",
-                          data={"devices": devices})
+        return self.query("post", "v2/users/{user_id}/delete_devices",
+                          data={"devices": devices}, user_id=user_id)
 
     def user_auth_provider_search(self, provider, external_id):
         """ Finds a user based on their ID (external id) in auth provider
         represented by auth provider id (provider).
         """
         return self.query("get",
-                          f"v1/auth_providers/{provider}/users/{external_id}")
+                          "v1/auth_providers/{provider}/users/{external_id}",
+                          provider=provider, external_id=external_id)
 
     def user_3pid_search(self, medium, address):
         """ Finds a user based on their Third Party ID by specifying what kind
         of 3PID it is as medium.
         """
-        return self.query("get", f"v1/threepid/{medium}/users/{address}")
+        return self.query("get", "v1/threepid/{medium}/users/{address}",
+                          address=address)
 
     def room_join(self, room_id_or_alias, user_id):
         """ Allow an administrator to join an user account with a given user_id
         to a room with a given room_id_or_alias
         """
         data = {"user_id": user_id}
-        return self.query("post", f"v1/join/{room_id_or_alias}", data=data)
+        return self.query("post", "v1/join/{room_id_or_alias}", data=data,
+                          room_id_or_alias=room_id_or_alias)
 
     def room_list(self, _from, limit, name, order_by, reverse):
         """ List and search rooms
@@ -742,12 +766,12 @@ def room_list(self, _from, limit, name, order_by, reverse):
     def room_details(self, room_id):
         """ Get details about a room
         """
-        return self.query("get", f"v1/rooms/{room_id}")
+        return self.query("get", "v1/rooms/{room_id}", room_id=room_id)
 
     def room_members(self, room_id):
         """ Get a list of room members
         """
-        return self.query("get", f"v1/rooms/{room_id}/members")
+        return self.query("get", "v1/rooms/{room_id}/members", room_id=room_id)
 
     def room_state(self, room_id):
         """ Get a list of all state events in a room.
@@ -759,7 +783,7 @@ def room_state(self, room_id):
             string: JSON string containing the admin API's response or None if
                 an exception occured. See Synapse admin API docs for details.
         """
-        return self.query("get", f"v1/rooms/{room_id}/state")
+        return self.query("get", "v1/rooms/{room_id}/state", room_id=room_id)
 
     def room_power_levels(self, from_, limit, name, order_by, reverse,
                           room_id=None, all_details=True,
@@ -826,7 +850,8 @@ def room_delete(self, room_id, new_room_user_id, room_name, message,
             data.update({"room_name": room_name})
         if message:
             data.update({"message": message})
-        return self.query("delete", f"v1/rooms/{room_id}", data=data)
+        return self.query("delete", "v1/rooms/{room_id}", data=data,
+                          room_id=room_id)
 
     def block_room(self, room_id, block):
         """ Block or unblock a room.
@@ -843,7 +868,8 @@ def block_room(self, room_id, block):
         data = {
             "block": block
         }
-        return self.query("put", f"v1/rooms/{room_id}/block", data=data)
+        return self.query("put", "v1/rooms/{room_id}/block", data=data,
+                          room_id=room_id)
 
     def room_block_status(self, room_id):
         """ Returns if the room is blocked or not, and who blocked it.
@@ -856,7 +882,7 @@ def room_block_status(self, room_id):
                 an exception occured. See Synapse admin API docs for details.
         """
         # TODO prevent usage on versions before 1.48
-        return self.query("get", f"v1/rooms/{room_id}/block")
+        return self.query("get", "v1/rooms/{room_id}/block", room_id=room_id)
 
     def room_make_admin(self, room_id, user_id):
         """ Grant a user room admin permission. If the user is not in the room,
@@ -865,51 +891,55 @@ def room_make_admin(self, room_id, user_id):
         data = {}
         if user_id:
             data.update({"user_id": user_id})
-        return self.query("post", f"v1/rooms/{room_id}/make_room_admin",
-                          data=data)
+        return self.query("post", "v1/rooms/{room_id}/make_room_admin",
+                          data=data, room_id=room_id)
 
     def room_media_list(self, room_id):
         """ Get a list of known media in an (unencrypted) room.
         """
-        return self.query("get", f"v1/room/{room_id}/media")
+        return self.query("get", "v1/room/{room_id}/media", room_id=room_id)
 
     def media_quarantine(self, server_name, media_id):
         """ Quarantine a single piece of local or remote media
         """
         return self.query(
-            "post", f"v1/media/quarantine/{server_name}/{media_id}", data={}
+            "post", "v1/media/quarantine/{server_name}/{media_id}", data={},
+            server_name=server_name, media_id=media_id
         )
 
     def media_unquarantine(self, server_name, media_id):
         """ Removes a single piece of local or remote media from quarantine.
         """
         return self.query(
-            "post", f"v1/media/unquarantine/{server_name}/{media_id}", data={}
+            "post", "v1/media/unquarantine/{server_name}/{media_id}", data={},
+            server_name=server_name, media_id=media_id
         )
 
     def room_media_quarantine(self, room_id):
         """ Quarantine all local and remote media in a room
         """
         return self.query(
-            "post", f"v1/room/{room_id}/media/quarantine", data={}
+            "post", "v1/room/{room_id}/media/quarantine", data={},
+            room_id=room_id
         )
 
     def user_media_quarantine(self, user_id):
         """ Quarantine all local and remote media of a user
         """
         return self.query(
-            "post", f"v1/user/{user_id}/media/quarantine", data={}
+            "post", "v1/user/{user_id}/media/quarantine", data={},
+            user_id=user_id
         )
 
     def user_media(self, user_id, _from, limit, order_by, reverse, readable):
         """ Get a user's uploaded media
         """
-        result = self.query("get", f"v1/users/{user_id}/media", params={
+        result = self.query("get", "v1/users/{user_id}/media", params={
             "from": _from,
             "limit": limit,
             "order_by": order_by,
             "dir": "b" if reverse else None
-        })
+        }, user_id=user_id)
         if (readable and result is not None and "media" in result):
             for i, media in enumerate(result["media"]):
                 created = media["created_ts"]
@@ -928,7 +958,8 @@ def media_delete(self, server_name, media_id):
         """ Delete a specific (local) media_id
         """
         return self.query(
-            "delete", f"v1/media/{server_name}/{media_id}", data={}
+            "delete", "v1/media/{server_name}/{media_id}", data={},
+            server_name=server_name, media_id=media_id
         )
 
     def media_delete_by_date_or_size(self, server_name, before_days, before,
@@ -971,7 +1002,8 @@ def media_delete_by_date_or_size(self, server_name, before_days, before,
                 "keep_profiles": "false"
             })
         return self.query(
-            "post", f"v1/media/{server_name}/delete", data={}, params=params
+            "post", "v1/media/{server_name}/delete", data={}, params=params,
+            server_name=server_name
         )
 
     def media_protect(self, media_id):
@@ -980,7 +1012,7 @@ def media_protect(self, media_id):
         from being quarantined
         """
         return self.query(
-            "post", f"v1/media/protect/{media_id}", data={}
+            "post", "v1/media/protect/{media_id}", data={}, media_id=media_id
         )
 
     def purge_media_cache(self, before_days, before, _before_ts):
@@ -1016,7 +1048,8 @@ def version(self):
     def group_delete(self, group_id):
         """ Delete a local group (community)
         """
-        return self.query("post", f"v1/delete_group/{group_id}")
+        return self.query("post", "v1/delete_group/{group_id}",
+                          group_id=group_id)
 
     def purge_history(self, room_id, before_event_id, before_days, before,
                       _before_ts, delete_local):
@@ -1056,14 +1089,16 @@ def purge_history(self, room_id, before_event_id, before_days, before,
                 "delete_local_events": True,
             })
 
-        return self.query("post", f"v1/purge_history/{room_id}", data=data)
+        return self.query("post", "v1/purge_history/{room_id}", data=data,
+                          room_id=room_id)
 
     def purge_history_status(self, purge_id):
         """ Get status of a recent history purge
 
         The status will be one of active, complete, or failed.
         """
-        return self.query("get", f"v1/purge_history_status/{purge_id}")
+        return self.query("get", "v1/purge_history_status/{purge_id}",
+                          purge_id=purge_id)
 
     def regtok_list(self, valid, readable_expiry):
         """ List registration tokens
@@ -1113,7 +1148,8 @@ def regtok_details(self, token, readable_expiry):
                 an exception occured. See Synapse admin API docs for details.
 
         """
-        result = self.query("get", f"v1/registration_tokens/{token}")
+        result = self.query("get", "v1/registration_tokens/{token}",
+                            token=token)
 
         # Change expiry_time to a human readable format if requested
         if (
@@ -1208,7 +1244,8 @@ def regtok_update(self, token, uses_allowed, expiry_ts, expire_at):
             self.log.debug(f"Received --expire-at: {expire_at}")
             data["expiry_time"] = self._timestamp_from_datetime(expire_at)
 
-        return self.query("put", f"v1/registration_tokens/{token}", data=data)
+        return self.query("put", "v1/registration_tokens/{token}", data=data,
+                          token=token)
 
     def regtok_delete(self, token):
         """ Delete a registration token
@@ -1221,7 +1258,8 @@ def regtok_delete(self, token):
                 an exception occured. See Synapse admin API docs for details.
 
         """
-        return self.query("delete", f"v1/registration_tokens/{token}")
+        return self.query("delete", "v1/registration_tokens/{token}",
+                          token=token)
 
     def user_shadow_ban(self, user_id, unban):
         """ Shadow-ban or unban a user.
@@ -1234,7 +1272,8 @@ def user_shadow_ban(self, user_id, unban):
             method = "delete"
         else:
             method = "post"
-        return self.query(method, f"v1/users/{user_id}/shadow_ban")
+        return self.query(method, "v1/users/{user_id}/shadow_ban",
+                          user_id=user_id)
 
     def notice_send(self, receivers, content_plain, content_html, paginate,
                     regex):