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

Consolidate the logic of delete_device/delete_devices #12970

Merged
merged 3 commits into from
Jun 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative to this would be to have it pass through to delete_devices, but that doesn't really seem necessary?

Note that delete_devices does not have @trace on it.

"""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