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

Make get_device return None if the device doesn't exist rather than raising an exception. #11565

Merged
merged 4 commits into from
Dec 13, 2021
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/11565.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Make `get_device` return `None` if the device doesn't exist rather than raising an exception.
4 changes: 1 addition & 3 deletions synapse/handlers/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -997,9 +997,7 @@ async def create_access_token_for_user_id(
# really don't want is active access_tokens without a record of the
# device, so we double-check it here.
if device_id is not None:
try:
await self.store.get_device(user_id, device_id)
except StoreError:
if await self.store.get_device(user_id, device_id) is None:
await self.store.delete_access_token(access_token)
raise StoreError(400, "Login raced against device deletion")

Expand Down
10 changes: 6 additions & 4 deletions synapse/handlers/device.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,10 @@ async def get_device(self, user_id: str, device_id: str) -> JsonDict:
Raises:
errors.NotFoundError: if the device was not found
"""
try:
device = await self.store.get_device(user_id, device_id)
except errors.StoreError:
raise errors.NotFoundError
device = await self.store.get_device(user_id, device_id)
if device is None:
raise errors.NotFoundError()

ips = await self.store.get_last_client_ip_by_device(user_id, device_id)
_update_device_from_client_ips(device, ips)

Expand Down Expand Up @@ -602,6 +602,8 @@ async def rehydrate_device(
access_token, device_id
)
old_device = await self.store.get_device(user_id, old_device_id)
if old_device is None:
raise errors.NotFoundError()
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
Expand Down
2 changes: 2 additions & 0 deletions synapse/rest/admin/devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ async def on_GET(
device = await self.device_handler.get_device(
target_user.to_string(), device_id
)
if device is None:
raise NotFoundError("No device found")
return HTTPStatus.OK, device

async def on_DELETE(
Expand Down
6 changes: 4 additions & 2 deletions synapse/rest/client/devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,17 @@
from typing import TYPE_CHECKING, Tuple

from synapse.api import errors
from synapse.api.errors import NotFoundError
from synapse.http.server import HttpServer
from synapse.http.servlet import (
RestServlet,
assert_params_in_dict,
parse_json_object_from_request,
)
from synapse.http.site import SynapseRequest
from synapse.rest.client._base import client_patterns, interactive_auth_handler
from synapse.types import JsonDict

from ._base import client_patterns, interactive_auth_handler

if TYPE_CHECKING:
from synapse.server import HomeServer

Expand Down Expand Up @@ -116,6 +116,8 @@ async def on_GET(
device = await self.device_handler.get_device(
requester.user.to_string(), device_id
)
if device is None:
raise NotFoundError("No device found")
return 200, device

@interactive_auth_handler
Expand Down
10 changes: 6 additions & 4 deletions synapse/storage/databases/main/devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,23 +101,25 @@ def count_devices_by_users_txn(txn, user_ids):
"count_devices_by_users", count_devices_by_users_txn, user_ids
)

async def get_device(self, user_id: str, device_id: str) -> Dict[str, Any]:
async def get_device(
self, user_id: str, device_id: str
) -> Optional[Dict[str, Any]]:
"""Retrieve a device. Only returns devices that are not marked as
hidden.

Args:
user_id: The ID of the user which owns the device
device_id: The ID of the device to retrieve
Returns:
A dict containing the device information
Raises:
StoreError: if the device is not found
A dict containing the device information, or `None` if the device does not
exist.
"""
return await self.db_pool.simple_select_one(
table="devices",
keyvalues={"user_id": user_id, "device_id": device_id, "hidden": False},
retcols=("user_id", "device_id", "display_name"),
desc="get_device",
allow_none=True,
)

async def get_devices_by_user(self, user_id: str) -> Dict[str, Dict[str, str]]:
Expand Down