From 18956829d78e838eb15bbef43840600ed541f1c4 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 6 Jun 2022 11:31:40 -0400 Subject: [PATCH 1/3] Remove the delete_device method from the datastore. --- synapse/handlers/device.py | 4 ++-- synapse/storage/databases/main/devices.py | 10 ---------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index a0cbeedc3001..53f8abdecf80 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -407,7 +407,7 @@ async def delete_device(self, user_id: str, device_id: str) -> None: """ try: - await self.store.delete_device(user_id, device_id) + await self.store.delete_devices(user_id, [device_id]) except errors.StoreError as e: if e.code == 404: # no match @@ -638,7 +638,7 @@ async def rehydrate_device( await self.store.update_device(user_id, device_id, old_device["display_name"]) # can't call self.delete_device because that will clobber the # access token so call the storage layer directly - await self.store.delete_device(user_id, old_device_id) + await self.store.delete_devices(user_id, [old_device_id]) await self.store.delete_e2e_keys_by_device( user_id=user_id, device_id=old_device_id ) diff --git a/synapse/storage/databases/main/devices.py b/synapse/storage/databases/main/devices.py index d900064c071a..71e7863dd82b 100644 --- a/synapse/storage/databases/main/devices.py +++ b/synapse/storage/databases/main/devices.py @@ -1433,16 +1433,6 @@ async def store_device( ) raise StoreError(500, "Problem storing device.") - async def delete_device(self, user_id: str, device_id: str) -> None: - """Delete a device and its device_inbox. - - Args: - user_id: The ID of the user which owns the device - device_id: The ID of the device to delete - """ - - await self.delete_devices(user_id, [device_id]) - async def delete_devices(self, user_id: str, device_ids: List[str]) -> None: """Deletes several devices. From 31ffcd1942299850e1a12184784f9654c06591fd Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 6 Jun 2022 11:32:17 -0400 Subject: [PATCH 2/3] Remove the delete_device method from the handler. --- synapse/handlers/device.py | 31 +------------------------------ synapse/module_api/__init__.py | 2 +- synapse/rest/admin/devices.py | 2 +- synapse/rest/client/devices.py | 4 +++- synapse/rest/client/logout.py | 4 ++-- tests/handlers/test_device.py | 4 ++-- 6 files changed, 10 insertions(+), 37 deletions(-) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 53f8abdecf80..b79c5517033b 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -397,35 +397,6 @@ async def _delete_stale_devices(self) -> None: for user_id, user_devices in devices.items(): await self.delete_devices(user_id, user_devices) - @trace - async def delete_device(self, user_id: str, device_id: str) -> None: - """Delete the given device - - Args: - user_id: The user to delete the device from. - device_id: The device to delete. - """ - - try: - await self.store.delete_devices(user_id, [device_id]) - except errors.StoreError as e: - if e.code == 404: - # no match - set_tag("error", True) - log_kv( - {"reason": "User doesn't have device id.", "device_id": device_id} - ) - else: - raise - - await self._auth_handler.delete_access_tokens_for_user( - user_id, device_id=device_id - ) - - await self.store.delete_e2e_keys_by_device(user_id=user_id, device_id=device_id) - - await self.notify_device_update(user_id, [device_id]) - @trace async def delete_all_devices_for_user( self, user_id: str, except_device_id: Optional[str] = None @@ -591,7 +562,7 @@ async def store_dehydrated_device( user_id, device_id, device_data ) if old_device_id is not None: - await self.delete_device(user_id, old_device_id) + await self.delete_devices(user_id, [old_device_id]) return device_id async def get_dehydrated_device( diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index a8ad575fcd96..30b2aeffdd88 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -799,7 +799,7 @@ def invalidate_access_token( if device_id: # delete the device, which will also delete its access tokens yield defer.ensureDeferred( - self._hs.get_device_handler().delete_device(user_id, device_id) + self._hs.get_device_handler().delete_devices(user_id, [device_id]) ) else: # no associated device. Just delete the access token. diff --git a/synapse/rest/admin/devices.py b/synapse/rest/admin/devices.py index cef46ba0ddf2..d9348801026b 100644 --- a/synapse/rest/admin/devices.py +++ b/synapse/rest/admin/devices.py @@ -80,7 +80,7 @@ async def on_DELETE( if u is None: raise NotFoundError("Unknown user") - await self.device_handler.delete_device(target_user.to_string(), device_id) + await self.device_handler.delete_devices(target_user.to_string(), [device_id]) return HTTPStatus.OK, {} async def on_PUT( diff --git a/synapse/rest/client/devices.py b/synapse/rest/client/devices.py index ad6fd6492baa..6fab102437f7 100644 --- a/synapse/rest/client/devices.py +++ b/synapse/rest/client/devices.py @@ -147,7 +147,9 @@ async def on_DELETE( can_skip_ui_auth=True, ) - await self.device_handler.delete_device(requester.user.to_string(), device_id) + await self.device_handler.delete_devices( + requester.user.to_string(), [device_id] + ) return 200, {} async def on_PUT( diff --git a/synapse/rest/client/logout.py b/synapse/rest/client/logout.py index 193a6951b91b..23dfa4518fc5 100644 --- a/synapse/rest/client/logout.py +++ b/synapse/rest/client/logout.py @@ -45,8 +45,8 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: access_token = self.auth.get_access_token_from_request(request) await self._auth_handler.delete_access_token(access_token) else: - await self._device_handler.delete_device( - requester.user.to_string(), requester.device_id + await self._device_handler.delete_devices( + requester.user.to_string(), [requester.device_id] ) return 200, {} diff --git a/tests/handlers/test_device.py b/tests/handlers/test_device.py index 01ea7d2a4281..b8b465d35b8f 100644 --- a/tests/handlers/test_device.py +++ b/tests/handlers/test_device.py @@ -154,7 +154,7 @@ def test_delete_device(self) -> None: self._record_users() # delete the device - self.get_success(self.handler.delete_device(user1, "abc")) + self.get_success(self.handler.delete_devices(user1, ["abc"])) # check the device was deleted self.get_failure(self.handler.get_device(user1, "abc"), NotFoundError) @@ -179,7 +179,7 @@ def test_delete_device_and_device_inbox(self) -> None: ) # delete the device - self.get_success(self.handler.delete_device(user1, "abc")) + self.get_success(self.handler.delete_devices(user1, ["abc"])) # check that the device_inbox was deleted res = self.get_success( From efa6b8245cd69ac8f7ecc27ac6d0988547d2f7cc Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 6 Jun 2022 11:33:30 -0400 Subject: [PATCH 3/3] Newsfragment --- changelog.d/12970.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/12970.misc diff --git a/changelog.d/12970.misc b/changelog.d/12970.misc new file mode 100644 index 000000000000..8f874aa07b3e --- /dev/null +++ b/changelog.d/12970.misc @@ -0,0 +1 @@ +Remove the `delete_device` method and always call `delete_devices`.