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

Commit

Permalink
Make get_device return None if the device doesn't exist rather than…
Browse files Browse the repository at this point in the history
… raising an exception. (#11565)

Co-authored-by: Sean Quah <[email protected]>
  • Loading branch information
reivilibre and squahtx authored Dec 13, 2021
1 parent aa8708e commit e5cdb9e
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 13 deletions.
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

0 comments on commit e5cdb9e

Please sign in to comment.