From bbeeb97f753e158e9aadd53aff78b076d756917c Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Mon, 13 Mar 2017 17:53:23 +0000 Subject: [PATCH] Implement _simple_delete_many_txn, use it to delete devices (But this doesn't implement the same for deleting access tokens or e2e keys. Also respond to code review. --- synapse/handlers/device.py | 34 ++++++++++++++++++++ synapse/rest/client/v2_alpha/devices.py | 20 ++++++------ synapse/storage/_base.py | 41 +++++++++++++++++++++++++ synapse/storage/devices.py | 17 ++++++++++ 4 files changed, 101 insertions(+), 11 deletions(-) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index e859b3165f..efaa0c8d6e 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -169,6 +169,40 @@ class DeviceHandler(BaseHandler): yield self.notify_device_update(user_id, [device_id]) + @defer.inlineCallbacks + def delete_devices(self, user_id, device_ids): + """ Delete several devices + + Args: + user_id (str): + device_ids (str): The list of device IDs to delete + + Returns: + defer.Deferred: + """ + + try: + yield self.store.delete_devices(user_id, device_ids) + except errors.StoreError, e: + if e.code == 404: + # no match + pass + else: + raise + + # Delete access tokens and e2e keys for each device. Not optimised as it is not + # considered as part of a critical path. + for device_id in device_ids: + yield self.store.user_delete_access_tokens( + user_id, device_id=device_id, + delete_refresh_tokens=True, + ) + yield self.store.delete_e2e_keys_by_device( + user_id=user_id, device_id=device_id + ) + + yield self.notify_device_update(user_id, device_ids) + @defer.inlineCallbacks def update_device(self, user_id, device_id, content): """ Update the given device diff --git a/synapse/rest/client/v2_alpha/devices.py b/synapse/rest/client/v2_alpha/devices.py index fd9516a601..b57ba95d24 100644 --- a/synapse/rest/client/v2_alpha/devices.py +++ b/synapse/rest/client/v2_alpha/devices.py @@ -47,13 +47,13 @@ class DevicesRestServlet(servlet.RestServlet): class DeleteDevicesRestServlet(servlet.RestServlet): + """ + API for bulk deletion of devices. Accepts a JSON object with a devices + key which lists the device_ids to delete. Requires user interactive auth. + """ PATTERNS = client_v2_patterns("/delete_devices", releases=[], v2_alpha=False) def __init__(self, hs): - """ - Args: - hs (synapse.server.HomeServer): server - """ super(DeleteDevicesRestServlet, self).__init__() self.hs = hs self.auth = hs.get_auth() @@ -64,14 +64,13 @@ class DeleteDevicesRestServlet(servlet.RestServlet): def on_POST(self, request): try: body = servlet.parse_json_object_from_request(request) - except errors.SynapseError as e: if e.errcode == errors.Codes.NOT_JSON: # deal with older clients which didn't pass a J*DELETESON dict # the same as those that pass an empty dict body = {} else: - raise + raise e if 'devices' not in body: raise errors.SynapseError( @@ -86,11 +85,10 @@ class DeleteDevicesRestServlet(servlet.RestServlet): defer.returnValue((401, result)) requester = yield self.auth.get_user_by_req(request) - for d_id in body['devices']: - yield self.device_handler.delete_device( - requester.user.to_string(), - d_id, - ) + yield self.device_handler.delete_devices( + requester.user.to_string(), + body['devices'], + ) defer.returnValue((200, {})) diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py index a7a8ec9b7b..13b106bba1 100644 --- a/synapse/storage/_base.py +++ b/synapse/storage/_base.py @@ -840,6 +840,47 @@ class SQLBaseStore(object): return txn.execute(sql, keyvalues.values()) + def _simple_delete_many(self, table, column, iterable, keyvalues, desc): + return self.runInteraction( + desc, self._simple_delete_many_txn, table, column, iterable, keyvalues + ) + + @staticmethod + def _simple_delete_many_txn(txn, table, column, iterable, keyvalues): + """Executes a DELETE query on the named table. + + Filters rows by if value of `column` is in `iterable`. + + Args: + txn : Transaction object + table : string giving the table name + column : column name to test for inclusion against `iterable` + iterable : list + keyvalues : dict of column names and values to select the rows with + """ + if not iterable: + return + + sql = "DELETE FROM %s" % table + + clauses = [] + values = [] + clauses.append( + "%s IN (%s)" % (column, ",".join("?" for _ in iterable)) + ) + values.extend(iterable) + + for key, value in keyvalues.items(): + clauses.append("%s = ?" % (key,)) + values.append(value) + + if clauses: + sql = "%s WHERE %s" % ( + sql, + " AND ".join(clauses), + ) + return txn.execute(sql, values) + def _get_cache_dict(self, db_conn, table, entity_column, stream_column, max_value, limit=100000): # Fetch a mapping of room_id -> max stream position for "recent" rooms. diff --git a/synapse/storage/devices.py b/synapse/storage/devices.py index bd56ba2515..563071b7a9 100644 --- a/synapse/storage/devices.py +++ b/synapse/storage/devices.py @@ -108,6 +108,23 @@ class DeviceStore(SQLBaseStore): desc="delete_device", ) + def delete_devices(self, user_id, device_ids): + """Deletes several devices. + + Args: + user_id (str): The ID of the user which owns the devices + device_ids (list): The IDs of the devices to delete + Returns: + defer.Deferred + """ + return self._simple_delete_many( + table="devices", + column="device_id", + iterable=device_ids, + keyvalues={"user_id": user_id}, + desc="delete_devices", + ) + def update_device(self, user_id, device_id, new_display_name=None): """Update a device.