Remove cache for get_shared_rooms_for_users (#9416)

This PR remove the cache for the `get_shared_rooms_for_users` storage method (the db method driving the experimental "what rooms do I share with this user?" feature: [MSC2666](https://github.com/matrix-org/matrix-doc/pull/2666)). Currently subsequent requests to the endpoint will return the same result, even if your shared rooms with that user have changed.

The cache was added in https://github.com/matrix-org/synapse/pull/7785, but we forgot to ensure it was invalidated appropriately.

Upon attempting to invalidate it, I found that the cache had to be entirely invalidated whenever a user (remote or local) joined or left a room. This didn't make for a very useful cache, especially for a function that may or may not be called very often. Thus, I've opted to remove it instead of invalidating it.
This commit is contained in:
Andrew Morgan 2021-02-22 16:52:45 +00:00 committed by GitHub
parent e22b71810e
commit 0a363f9ca4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 43 additions and 37 deletions

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

@ -0,0 +1 @@
Fix a bug that caused multiple calls to the experimental `shared_rooms` endpoint to return stale results.

View File

@ -497,8 +497,7 @@ class UserDirectoryBackgroundUpdateStore(StateDeltasStore):
async def add_users_in_public_rooms( async def add_users_in_public_rooms(
self, room_id: str, user_ids: Iterable[str] self, room_id: str, user_ids: Iterable[str]
) -> None: ) -> None:
"""Insert entries into the users_who_share_private_rooms table. The first """Insert entries into the users_in_public_rooms table.
user should be a local user.
Args: Args:
room_id room_id
@ -670,7 +669,6 @@ class UserDirectoryStore(UserDirectoryBackgroundUpdateStore):
users.update(rows) users.update(rows)
return list(users) return list(users)
@cached()
async def get_shared_rooms_for_users( async def get_shared_rooms_for_users(
self, user_id: str, other_user_id: str self, user_id: str, other_user_id: str
) -> Set[str]: ) -> Set[str]:

View File

@ -54,61 +54,62 @@ class UserSharedRoomsTest(unittest.HomeserverTestCase):
A room should show up in the shared list of rooms between two users A room should show up in the shared list of rooms between two users
if it is public. if it is public.
""" """
u1 = self.register_user("user1", "pass") self._check_shared_rooms_with(room_one_is_public=True, room_two_is_public=True)
u1_token = self.login(u1, "pass")
u2 = self.register_user("user2", "pass")
u2_token = self.login(u2, "pass")
room = self.helper.create_room_as(u1, is_public=True, tok=u1_token)
self.helper.invite(room, src=u1, targ=u2, tok=u1_token)
self.helper.join(room, user=u2, tok=u2_token)
channel = self._get_shared_rooms(u1_token, u2)
self.assertEquals(200, channel.code, channel.result)
self.assertEquals(len(channel.json_body["joined"]), 1)
self.assertEquals(channel.json_body["joined"][0], room)
def test_shared_room_list_private(self): def test_shared_room_list_private(self):
""" """
A room should show up in the shared list of rooms between two users A room should show up in the shared list of rooms between two users
if it is private. if it is private.
""" """
u1 = self.register_user("user1", "pass") self._check_shared_rooms_with(
u1_token = self.login(u1, "pass") room_one_is_public=False, room_two_is_public=False
u2 = self.register_user("user2", "pass") )
u2_token = self.login(u2, "pass")
room = self.helper.create_room_as(u1, is_public=False, tok=u1_token)
self.helper.invite(room, src=u1, targ=u2, tok=u1_token)
self.helper.join(room, user=u2, tok=u2_token)
channel = self._get_shared_rooms(u1_token, u2)
self.assertEquals(200, channel.code, channel.result)
self.assertEquals(len(channel.json_body["joined"]), 1)
self.assertEquals(channel.json_body["joined"][0], room)
def test_shared_room_list_mixed(self): def test_shared_room_list_mixed(self):
""" """
The shared room list between two users should contain both public and private The shared room list between two users should contain both public and private
rooms. rooms.
""" """
self._check_shared_rooms_with(room_one_is_public=True, room_two_is_public=False)
def _check_shared_rooms_with(
self, room_one_is_public: bool, room_two_is_public: bool
):
"""Checks that shared public or private rooms between two users appear in
their shared room lists
"""
u1 = self.register_user("user1", "pass") u1 = self.register_user("user1", "pass")
u1_token = self.login(u1, "pass") u1_token = self.login(u1, "pass")
u2 = self.register_user("user2", "pass") u2 = self.register_user("user2", "pass")
u2_token = self.login(u2, "pass") u2_token = self.login(u2, "pass")
room_public = self.helper.create_room_as(u1, is_public=True, tok=u1_token) # Create a room. user1 invites user2, who joins
room_private = self.helper.create_room_as(u2, is_public=False, tok=u2_token) room_id_one = self.helper.create_room_as(
self.helper.invite(room_public, src=u1, targ=u2, tok=u1_token) u1, is_public=room_one_is_public, tok=u1_token
self.helper.invite(room_private, src=u2, targ=u1, tok=u2_token) )
self.helper.join(room_public, user=u2, tok=u2_token) self.helper.invite(room_id_one, src=u1, targ=u2, tok=u1_token)
self.helper.join(room_private, user=u1, tok=u1_token) self.helper.join(room_id_one, user=u2, tok=u2_token)
# Check shared rooms from user1's perspective.
# We should see the one room in common
channel = self._get_shared_rooms(u1_token, u2)
self.assertEquals(200, channel.code, channel.result)
self.assertEquals(len(channel.json_body["joined"]), 1)
self.assertEquals(channel.json_body["joined"][0], room_id_one)
# Create another room and invite user2 to it
room_id_two = self.helper.create_room_as(
u1, is_public=room_two_is_public, tok=u1_token
)
self.helper.invite(room_id_two, src=u1, targ=u2, tok=u1_token)
self.helper.join(room_id_two, user=u2, tok=u2_token)
# Check shared rooms again. We should now see both rooms.
channel = self._get_shared_rooms(u1_token, u2) channel = self._get_shared_rooms(u1_token, u2)
self.assertEquals(200, channel.code, channel.result) self.assertEquals(200, channel.code, channel.result)
self.assertEquals(len(channel.json_body["joined"]), 2) self.assertEquals(len(channel.json_body["joined"]), 2)
self.assertTrue(room_public in channel.json_body["joined"]) for room_id_id in channel.json_body["joined"]:
self.assertTrue(room_private in channel.json_body["joined"]) self.assertIn(room_id_id, [room_id_one, room_id_two])
def test_shared_room_list_after_leave(self): def test_shared_room_list_after_leave(self):
""" """
@ -132,6 +133,12 @@ class UserSharedRoomsTest(unittest.HomeserverTestCase):
self.helper.leave(room, user=u1, tok=u1_token) self.helper.leave(room, user=u1, tok=u1_token)
# Check user1's view of shared rooms with user2
channel = self._get_shared_rooms(u1_token, u2)
self.assertEquals(200, channel.code, channel.result)
self.assertEquals(len(channel.json_body["joined"]), 0)
# Check user2's view of shared rooms with user1
channel = self._get_shared_rooms(u2_token, u1) channel = self._get_shared_rooms(u2_token, u1)
self.assertEquals(200, channel.code, channel.result) self.assertEquals(200, channel.code, channel.result)
self.assertEquals(len(channel.json_body["joined"]), 0) self.assertEquals(len(channel.json_body["joined"]), 0)