summary refs log tree commit diff
diff options
context:
space:
mode:
authorPatrick Cloke <clokep@users.noreply.github.com>2022-06-07 07:43:35 -0400
committerGitHub <noreply@github.com>2022-06-07 07:43:35 -0400
commit9dc3293e0b3a5cbf6fcc4a0cef7386b531190882 (patch)
tree97a293afac5f1b5e7649de833f2b6be5112b6e00
parentPrevent breaking old sqlite's when media retention is enabled (#12977) (diff)
downloadsynapse-9dc3293e0b3a5cbf6fcc4a0cef7386b531190882.tar.xz
Consolidate the logic of delete_device/delete_devices. (#12970)
By always using delete_devices and sometimes passing a list
with a single device ID.

Previously these methods had gotten out of sync with each
other and it seems there's little benefit to the single-device
variant.
-rw-r--r--changelog.d/12970.misc1
-rw-r--r--synapse/handlers/device.py33
-rw-r--r--synapse/module_api/__init__.py2
-rw-r--r--synapse/rest/admin/devices.py2
-rw-r--r--synapse/rest/client/devices.py4
-rw-r--r--synapse/rest/client/logout.py4
-rw-r--r--synapse/storage/databases/main/devices.py10
-rw-r--r--tests/handlers/test_device.py4
8 files changed, 12 insertions, 48 deletions
diff --git a/changelog.d/12970.misc b/changelog.d/12970.misc
new file mode 100644
index 0000000000..8f874aa07b
--- /dev/null
+++ b/changelog.d/12970.misc
@@ -0,0 +1 @@
+Remove the `delete_device` method and always call `delete_devices`.
diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py
index a0cbeedc30..b79c551703 100644
--- a/synapse/handlers/device.py
+++ b/synapse/handlers/device.py
@@ -398,35 +398,6 @@ class DeviceHandler(DeviceWorkerHandler):
             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_device(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
     ) -> None:
@@ -591,7 +562,7 @@ class DeviceHandler(DeviceWorkerHandler):
             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(
@@ -638,7 +609,7 @@ class DeviceHandler(DeviceWorkerHandler):
         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/module_api/__init__.py b/synapse/module_api/__init__.py
index a8ad575fcd..30b2aeffdd 100644
--- a/synapse/module_api/__init__.py
+++ b/synapse/module_api/__init__.py
@@ -799,7 +799,7 @@ class ModuleApi:
         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 cef46ba0dd..d934880102 100644
--- a/synapse/rest/admin/devices.py
+++ b/synapse/rest/admin/devices.py
@@ -80,7 +80,7 @@ class DeviceRestServlet(RestServlet):
         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 ad6fd6492b..6fab102437 100644
--- a/synapse/rest/client/devices.py
+++ b/synapse/rest/client/devices.py
@@ -147,7 +147,9 @@ class DeviceRestServlet(RestServlet):
             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 193a6951b9..23dfa4518f 100644
--- a/synapse/rest/client/logout.py
+++ b/synapse/rest/client/logout.py
@@ -45,8 +45,8 @@ class LogoutRestServlet(RestServlet):
             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/synapse/storage/databases/main/devices.py b/synapse/storage/databases/main/devices.py
index d900064c07..71e7863dd8 100644
--- a/synapse/storage/databases/main/devices.py
+++ b/synapse/storage/databases/main/devices.py
@@ -1433,16 +1433,6 @@ class DeviceStore(DeviceWorkerStore, DeviceBackgroundUpdateStore):
             )
             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.
 
diff --git a/tests/handlers/test_device.py b/tests/handlers/test_device.py
index 01ea7d2a42..b8b465d35b 100644
--- a/tests/handlers/test_device.py
+++ b/tests/handlers/test_device.py
@@ -154,7 +154,7 @@ class DeviceTestCase(unittest.HomeserverTestCase):
         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 @@ class DeviceTestCase(unittest.HomeserverTestCase):
         )
 
         # 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(