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

Ignore rooms with unknown room versions in the spaces summary. #10727

Merged
merged 3 commits into from
Sep 1, 2021
Merged
Show file tree
Hide file tree
Changes from 2 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/10727.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Do not include rooms with unknown room versions in the spaces summary results.
14 changes: 12 additions & 2 deletions synapse/handlers/room_summary.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,14 @@
Membership,
RoomTypes,
)
from synapse.api.errors import AuthError, Codes, NotFoundError, StoreError, SynapseError
from synapse.api.errors import (
AuthError,
Codes,
NotFoundError,
StoreError,
SynapseError,
UnsupportedRoomVersionError,
)
from synapse.events import EventBase
from synapse.events.utils import format_event_for_client_v2
from synapse.types import JsonDict
Expand Down Expand Up @@ -814,7 +821,10 @@ async def _is_local_room_accessible(
logger.info("room %s is unknown, omitting from summary", room_id)
return False

room_version = await self._store.get_room_version(room_id)
try:
room_version = await self._store.get_room_version(room_id)
except UnsupportedRoomVersionError:
return False
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a bit of a comment as to why this is OK, ie "this stops us breaking the summary in the case where we drop room version support", or something?


# Include the room if it has join rules of public or knock.
join_rules_event_id = state_ids.get((EventTypes.JoinRules, ""))
Expand Down
25 changes: 25 additions & 0 deletions tests/handlers/test_room_summary.py
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,31 @@ def test_max_depth(self):
]
self._assert_hierarchy(result, expected)

def test_unknown_room_version(self):
"""
If an room with an unknown room version is encountered it should not cause
the entire summary to skip.
"""
# Poke the database and update the room version to an unknown one.
self.get_success(
self.hs.get_datastores().main.db_pool.simple_update(
"rooms",
keyvalues={"room_id": self.room},
updatevalues={"room_version": "unknown-room-version"},
desc="updated-room-version",
)
)

result = self.get_success(self.handler.get_space_summary(self.user, self.space))
# The result should have only the space, along with a link from space -> room.
expected = [(self.space, [self.room])]
self._assert_rooms(result, expected)

result = self.get_success(
self.handler.get_room_hierarchy(self.user, self.space)
)
self._assert_hierarchy(result, expected)

def test_fed_complex(self):
"""
Return data over federation and ensure that it is handled properly.
Expand Down