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

Remove remaining usage of cursor_to_dict. #16564

Merged
merged 12 commits into from
Oct 31, 2023
26 changes: 13 additions & 13 deletions synapse/handlers/room_summary.py
Original file line number Diff line number Diff line change
Expand Up @@ -703,24 +703,24 @@ async def _build_room_entry(self, room_id: str, for_federation: bool) -> JsonDic
# there should always be an entry
assert stats is not None, "unable to retrieve stats for %s" % (room_id,)

entry = {
"room_id": stats["room_id"],
"name": stats["name"],
"topic": stats["topic"],
"canonical_alias": stats["canonical_alias"],
"num_joined_members": stats["joined_members"],
"avatar_url": stats["avatar"],
"join_rule": stats["join_rules"],
entry: JsonDict = {
"room_id": stats.room_id,
"name": stats.name,
"topic": stats.topic,
"canonical_alias": stats.canonical_alias,
"num_joined_members": stats.joined_members,
"avatar_url": stats.avatar,
"join_rule": stats.join_rules,
"world_readable": (
stats["history_visibility"] == HistoryVisibility.WORLD_READABLE
stats.history_visibility == HistoryVisibility.WORLD_READABLE
),
"guest_can_join": stats["guest_access"] == "can_join",
"room_type": stats["room_type"],
"guest_can_join": stats.guest_access == "can_join",
"room_type": stats.room_type,
}

if self._msc3266_enabled:
entry["im.nheko.summary.version"] = stats["version"]
entry["im.nheko.summary.encryption"] = stats["encryption"]
entry["im.nheko.summary.version"] = stats.version
entry["im.nheko.summary.encryption"] = stats.encryption

# Federation requests need to provide additional information so the
# requested server is able to filter the response appropriately.
Expand Down
11 changes: 8 additions & 3 deletions synapse/rest/admin/rooms.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
from typing import TYPE_CHECKING, List, Optional, Tuple, cast
from urllib import parse as urlparse

import attr

from synapse.api.constants import Direction, EventTypes, JoinRules, Membership
from synapse.api.errors import AuthError, Codes, NotFoundError, SynapseError
from synapse.api.filtering import Filter
Expand Down Expand Up @@ -306,10 +308,13 @@ async def on_GET(
raise NotFoundError("Room not found")

members = await self.store.get_users_in_room(room_id)
ret["joined_local_devices"] = await self.store.count_devices_by_users(members)
ret["forgotten"] = await self.store.is_locally_forgotten_room(room_id)
result = attr.asdict(ret)
result["joined_local_devices"] = await self.store.count_devices_by_users(
members
)
result["forgotten"] = await self.store.is_locally_forgotten_room(room_id)
clokep marked this conversation as resolved.
Show resolved Hide resolved

return HTTPStatus.OK, ret
return HTTPStatus.OK, result

async def on_DELETE(
self, request: SynapseRequest, room_id: str
Expand Down
54 changes: 44 additions & 10 deletions synapse/storage/databases/main/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,27 @@ class RatelimitOverride:
burst_count: int


@attr.s(slots=True, frozen=True, auto_attribs=True)
class RoomStats:
room_id: str
name: Optional[str]
canonical_alias: Optional[str]
joined_members: str
joined_local_members: int
version: str
creator: str
encryption: Optional[str]
federatable: bool
public: bool
join_rules: Optional[str]
guest_access: Optional[str]
history_visibility: Optional[str]
state_events: int
avatar: Optional[str]
topic: Optional[str]
room_type: Optional[str]


class RoomSortOrder(Enum):
"""
Enum to define the sorting method used when returning rooms with get_rooms_paginate
Expand Down Expand Up @@ -204,7 +225,7 @@ async def get_room(self, room_id: str) -> Optional[Dict[str, Any]]:
allow_none=True,
)

async def get_room_with_stats(self, room_id: str) -> Optional[Dict[str, Any]]:
async def get_room_with_stats(self, room_id: str) -> Optional[RoomStats]:
"""Retrieve room with statistics.

Args:
Expand All @@ -215,7 +236,7 @@ async def get_room_with_stats(self, room_id: str) -> Optional[Dict[str, Any]]:

def get_room_with_stats_txn(
txn: LoggingTransaction, room_id: str
) -> Optional[Dict[str, Any]]:
) -> Optional[RoomStats]:
sql = """
SELECT room_id, state.name, state.canonical_alias, curr.joined_members,
curr.local_users_in_room AS joined_local_members, rooms.room_version AS version,
Expand All @@ -229,15 +250,28 @@ def get_room_with_stats_txn(
WHERE room_id = ?
"""
txn.execute(sql, [room_id])
# Catch error if sql returns empty result to return "None" instead of an error
try:
res = self.db_pool.cursor_to_dict(txn)[0]
except IndexError:
row = txn.fetchone()
if not row:
return None

res["federatable"] = bool(res["federatable"])
res["public"] = bool(res["public"])
return res
return RoomStats(
room_id=row[0],
name=row[1],
canonical_alias=row[2],
joined_members=row[3],
joined_local_members=row[4],
version=row[5],
creator=row[6],
clokep marked this conversation as resolved.
Show resolved Hide resolved
encryption=row[7],
federatable=bool(row[8]),
public=bool(row[9]),
join_rules=row[10],
guest_access=row[11],
history_visibility=row[12],
state_events=row[13],
avatar=row[14],
topic=row[15],
room_type=row[16],
)

return await self.db_pool.runInteraction(
"get_room_with_stats", get_room_with_stats_txn, room_id
Expand Down
22 changes: 11 additions & 11 deletions tests/handlers/test_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -342,10 +342,10 @@ def test_auto_create_auto_join_rooms_federated(self) -> None:
# Ensure the room is properly not federated.
room = self.get_success(self.store.get_room_with_stats(room_id["room_id"]))
assert room is not None
self.assertFalse(room["federatable"])
self.assertFalse(room["public"])
self.assertEqual(room["join_rules"], "public")
self.assertIsNone(room["guest_access"])
self.assertFalse(room.federatable)
self.assertFalse(room.public)
self.assertEqual(room.join_rules, "public")
self.assertIsNone(room.guest_access)

# The user should be in the room.
rooms = self.get_success(self.store.get_rooms_for_user(user_id))
Expand All @@ -372,7 +372,7 @@ def test_auto_join_mxid_localpart(self) -> None:
# Ensure the room is properly a public room.
room = self.get_success(self.store.get_room_with_stats(room_id["room_id"]))
assert room is not None
self.assertEqual(room["join_rules"], "public")
self.assertEqual(room.join_rules, "public")

# Both users should be in the room.
rooms = self.get_success(self.store.get_rooms_for_user(inviter))
Expand Down Expand Up @@ -411,9 +411,9 @@ def test_auto_create_auto_join_room_preset(self) -> None:
# Ensure the room is properly a private room.
room = self.get_success(self.store.get_room_with_stats(room_id["room_id"]))
assert room is not None
self.assertFalse(room["public"])
self.assertEqual(room["join_rules"], "invite")
self.assertEqual(room["guest_access"], "can_join")
self.assertFalse(room.public)
self.assertEqual(room.join_rules, "invite")
self.assertEqual(room.guest_access, "can_join")

# Both users should be in the room.
rooms = self.get_success(self.store.get_rooms_for_user(inviter))
Expand Down Expand Up @@ -455,9 +455,9 @@ def test_auto_create_auto_join_room_preset_guest(self) -> None:
# Ensure the room is properly a private room.
room = self.get_success(self.store.get_room_with_stats(room_id["room_id"]))
assert room is not None
self.assertFalse(room["public"])
self.assertEqual(room["join_rules"], "invite")
self.assertEqual(room["guest_access"], "can_join")
self.assertFalse(room.public)
self.assertEqual(room.join_rules, "invite")
self.assertEqual(room.guest_access, "can_join")

# Both users should be in the room.
rooms = self.get_success(self.store.get_rooms_for_user(inviter))
Expand Down
11 changes: 3 additions & 8 deletions tests/storage/test_room.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,9 @@ def test_get_room_unknown_room(self) -> None:
def test_get_room_with_stats(self) -> None:
res = self.get_success(self.store.get_room_with_stats(self.room.to_string()))
assert res is not None
self.assertLessEqual(
{
"room_id": self.room.to_string(),
"creator": self.u_creator.to_string(),
"public": True,
}.items(),
res.items(),
)
self.assertEqual(res.room_id, self.room.to_string())
self.assertEqual(res.creator, self.u_creator.to_string())
self.assertTrue(res.public)

def test_get_room_with_stats_unknown_room(self) -> None:
self.assertIsNone(
Expand Down