Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Consolidate the logic of delete_device/delete_devices. (#12970)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
clokep committed Jun 7, 2022
1 parent c51f5b9 commit 9dc3293
Show file tree
Hide file tree
Showing 8 changed files with 12 additions and 48 deletions.
1 change: 1 addition & 0 deletions changelog.d/12970.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove the `delete_device` method and always call `delete_devices`.
33 changes: 2 additions & 31 deletions synapse/handlers/device.py
Original file line number Diff line number Diff line change
Expand Up @@ -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_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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -638,7 +609,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
)
Expand Down
2 changes: 1 addition & 1 deletion synapse/module_api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/admin/devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 3 additions & 1 deletion synapse/rest/client/devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions synapse/rest/client/logout.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, {}
Expand Down
10 changes: 0 additions & 10 deletions synapse/storage/databases/main/devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions tests/handlers/test_device.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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(
Expand Down

0 comments on commit 9dc3293

Please sign in to comment.