Avoid sharing room hierarchy responses between users (#11355)

Different users may be allowed to see different rooms within a space,
so sharing responses between users is inadvisable.
This commit is contained in:
Sean Quah 2021-11-16 15:40:47 +00:00 committed by GitHub
parent dfa536490e
commit 88375beeaa
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 65 additions and 2 deletions

1
changelog.d/11355.bugfix Normal file
View File

@ -0,0 +1 @@
Fix a bug introduced in 1.41.0 where space hierarchy responses would be incorrectly reused if multiple users were to make the same request at the same time.

View File

@ -97,7 +97,7 @@ class RoomSummaryHandler:
# If a user tries to fetch the same page multiple times in quick succession, # If a user tries to fetch the same page multiple times in quick succession,
# only process the first attempt and return its result to subsequent requests. # only process the first attempt and return its result to subsequent requests.
self._pagination_response_cache: ResponseCache[ self._pagination_response_cache: ResponseCache[
Tuple[str, bool, Optional[int], Optional[int], Optional[str]] Tuple[str, str, bool, Optional[int], Optional[int], Optional[str]]
] = ResponseCache( ] = ResponseCache(
hs.get_clock(), hs.get_clock(),
"get_room_hierarchy", "get_room_hierarchy",
@ -282,7 +282,14 @@ class RoomSummaryHandler:
# This is due to the pagination process mutating internal state, attempting # This is due to the pagination process mutating internal state, attempting
# to process multiple requests for the same page will result in errors. # to process multiple requests for the same page will result in errors.
return await self._pagination_response_cache.wrap( return await self._pagination_response_cache.wrap(
(requested_room_id, suggested_only, max_depth, limit, from_token), (
requester,
requested_room_id,
suggested_only,
max_depth,
limit,
from_token,
),
self._get_room_hierarchy, self._get_room_hierarchy,
requester, requester,
requested_room_id, requested_room_id,

View File

@ -14,6 +14,8 @@
from typing import Any, Iterable, List, Optional, Tuple from typing import Any, Iterable, List, Optional, Tuple
from unittest import mock from unittest import mock
from twisted.internet.defer import ensureDeferred
from synapse.api.constants import ( from synapse.api.constants import (
EventContentFields, EventContentFields,
EventTypes, EventTypes,
@ -316,6 +318,59 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase):
AuthError, AuthError,
) )
def test_room_hierarchy_cache(self) -> None:
"""In-flight room hierarchy requests are deduplicated."""
# Run two `get_room_hierarchy` calls up until they block.
deferred1 = ensureDeferred(
self.handler.get_room_hierarchy(self.user, self.space)
)
deferred2 = ensureDeferred(
self.handler.get_room_hierarchy(self.user, self.space)
)
# Complete the two calls.
result1 = self.get_success(deferred1)
result2 = self.get_success(deferred2)
# Both `get_room_hierarchy` calls should return the same result.
expected = [(self.space, [self.room]), (self.room, ())]
self._assert_hierarchy(result1, expected)
self._assert_hierarchy(result2, expected)
self.assertIs(result1, result2)
# A subsequent `get_room_hierarchy` call should not reuse the result.
result3 = self.get_success(
self.handler.get_room_hierarchy(self.user, self.space)
)
self._assert_hierarchy(result3, expected)
self.assertIsNot(result1, result3)
def test_room_hierarchy_cache_sharing(self) -> None:
"""Room hierarchy responses for different users are not shared."""
user2 = self.register_user("user2", "pass")
# Make the room within the space invite-only.
self.helper.send_state(
self.room,
event_type=EventTypes.JoinRules,
body={"join_rule": JoinRules.INVITE},
tok=self.token,
)
# Run two `get_room_hierarchy` calls for different users up until they block.
deferred1 = ensureDeferred(
self.handler.get_room_hierarchy(self.user, self.space)
)
deferred2 = ensureDeferred(self.handler.get_room_hierarchy(user2, self.space))
# Complete the two calls.
result1 = self.get_success(deferred1)
result2 = self.get_success(deferred2)
# The `get_room_hierarchy` calls should return different results.
self._assert_hierarchy(result1, [(self.space, [self.room]), (self.room, ())])
self._assert_hierarchy(result2, [(self.space, [self.room])])
def _create_room_with_join_rule( def _create_room_with_join_rule(
self, join_rule: str, room_version: Optional[str] = None, **extra_content self, join_rule: str, room_version: Optional[str] = None, **extra_content
) -> str: ) -> str: