diff --git a/changelog.d/10982.bugfix b/changelog.d/10982.bugfix
new file mode 100644
index 0000000000000000000000000000000000000000..5c9e15eeaa42466758d35b66774b77728f7f14d8
--- /dev/null
+++ b/changelog.d/10982.bugfix
@@ -0,0 +1 @@
+Fix a long-standing bug where the remainder of a batch of user directory changes would be silently dropped if the server left a room early in the batch.
\ No newline at end of file
diff --git a/synapse/handlers/user_directory.py b/synapse/handlers/user_directory.py
index 18d8c8744e7559b22ce85c1771c1fd1879f53143..97f60b58068a7f4d8f74988ef7db8276ce8fc157 100644
--- a/synapse/handlers/user_directory.py
+++ b/synapse/handlers/user_directory.py
@@ -220,7 +220,7 @@ class UserDirectoryHandler(StateDeltasHandler):
for user_id in user_ids:
await self._handle_remove_user(room_id, user_id)
- return
+ continue
else:
logger.debug("Server is still in room: %r", room_id)
diff --git a/tests/handlers/test_stats.py b/tests/handlers/test_stats.py
index 24b7ef6efcf8e4b9e729e8581bcac62342e89b5b..56207f4db6f60a02d31e63206b43059c7da6c6cd 100644
--- a/tests/handlers/test_stats.py
+++ b/tests/handlers/test_stats.py
@@ -103,12 +103,7 @@ class StatsRoomTests(unittest.HomeserverTestCase):
# Do the initial population of the stats via the background update
self._add_background_updates()
- while not self.get_success(
- self.store.db_pool.updates.has_completed_background_updates()
- ):
- self.get_success(
- self.store.db_pool.updates.do_next_background_update(100), by=0.1
- )
+ self.wait_for_background_updates()
def test_initial_room(self):
"""
@@ -140,12 +135,7 @@ class StatsRoomTests(unittest.HomeserverTestCase):
# Do the initial population of the user directory via the background update
self._add_background_updates()
- while not self.get_success(
- self.store.db_pool.updates.has_completed_background_updates()
- ):
- self.get_success(
- self.store.db_pool.updates.do_next_background_update(100), by=0.1
- )
+ self.wait_for_background_updates()
r = self.get_success(self.get_all_room_state())
@@ -568,12 +558,7 @@ class StatsRoomTests(unittest.HomeserverTestCase):
)
)
- while not self.get_success(
- self.store.db_pool.updates.has_completed_background_updates()
- ):
- self.get_success(
- self.store.db_pool.updates.do_next_background_update(100), by=0.1
- )
+ self.wait_for_background_updates()
r1stats_complete = self._get_current_stats("room", r1)
u1stats_complete = self._get_current_stats("user", u1)
diff --git a/tests/handlers/test_user_directory.py b/tests/handlers/test_user_directory.py
index b3c3af113b28cc7b906b508ef517fbd29542e7d4..03fd5a3e2c52d4e9398b00c4258a242e29dc6285 100644
--- a/tests/handlers/test_user_directory.py
+++ b/tests/handlers/test_user_directory.py
@@ -363,6 +363,45 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase):
self.assertEqual(len(s["results"]), 1)
self.assertEqual(s["results"][0]["user_id"], user)
+ def test_process_join_after_server_leaves_room(self) -> None:
+ alice = self.register_user("alice", "pass")
+ alice_token = self.login(alice, "pass")
+ bob = self.register_user("bob", "pass")
+ bob_token = self.login(bob, "pass")
+
+ # Alice makes two rooms. Bob joins one of them.
+ room1 = self.helper.create_room_as(alice, tok=alice_token)
+ room2 = self.helper.create_room_as(alice, tok=alice_token)
+ print("room1=", room1)
+ print("room2=", room2)
+ self.helper.join(room1, bob, tok=bob_token)
+
+ # The user sharing tables should have been updated.
+ public1 = self.get_success(self.user_dir_helper.get_users_in_public_rooms())
+ self.assertEqual(set(public1), {(alice, room1), (alice, room2), (bob, room1)})
+
+ # Alice leaves room1. The user sharing tables should be updated.
+ self.helper.leave(room1, alice, tok=alice_token)
+ public2 = self.get_success(self.user_dir_helper.get_users_in_public_rooms())
+ self.assertEqual(set(public2), {(alice, room2), (bob, room1)})
+
+ # Pause the processing of new events.
+ dir_handler = self.hs.get_user_directory_handler()
+ dir_handler.update_user_directory = False
+
+ # Bob leaves one room and joins the other.
+ self.helper.leave(room1, bob, tok=bob_token)
+ self.helper.join(room2, bob, tok=bob_token)
+
+ # Process the leave and join in one go.
+ dir_handler.update_user_directory = True
+ dir_handler.notify_new_event()
+ self.wait_for_background_updates()
+
+ # The user sharing tables should have been updated.
+ public3 = self.get_success(self.user_dir_helper.get_users_in_public_rooms())
+ self.assertEqual(set(public3), {(alice, room2), (bob, room2)})
+
def test_private_room(self) -> None:
"""
A user can be searched for only by people that are either in a public
diff --git a/tests/storage/databases/main/test_room.py b/tests/storage/databases/main/test_room.py
index ffee7071534231655c7d0d00a4b3ca521712e333..7496974da3a85b834fd40d1faa732895842fe03d 100644
--- a/tests/storage/databases/main/test_room.py
+++ b/tests/storage/databases/main/test_room.py
@@ -79,12 +79,7 @@ class RoomBackgroundUpdateStoreTestCase(HomeserverTestCase):
self.store.db_pool.updates._all_done = False
# Now let's actually drive the updates to completion
- while not self.get_success(
- self.store.db_pool.updates.has_completed_background_updates()
- ):
- self.get_success(
- self.store.db_pool.updates.do_next_background_update(100), by=0.1
- )
+ self.wait_for_background_updates()
# Make sure the background update filled in the room creator
room_creator_after = self.get_success(
diff --git a/tests/storage/test_cleanup_extrems.py b/tests/storage/test_cleanup_extrems.py
index 7cc5e621ba9a7304965699fa51eb225f69250ca5..a59c28f896816b9b17dcb164535383cc8d7d8411 100644
--- a/tests/storage/test_cleanup_extrems.py
+++ b/tests/storage/test_cleanup_extrems.py
@@ -66,12 +66,7 @@ class CleanupExtremBackgroundUpdateStoreTestCase(HomeserverTestCase):
# Ugh, have to reset this flag
self.store.db_pool.updates._all_done = False
- while not self.get_success(
- self.store.db_pool.updates.has_completed_background_updates()
- ):
- self.get_success(
- self.store.db_pool.updates.do_next_background_update(100), by=0.1
- )
+ self.wait_for_background_updates()
def test_soft_failed_extremities_handled_correctly(self):
"""Test that extremities are correctly calculated in the presence of
diff --git a/tests/storage/test_client_ips.py b/tests/storage/test_client_ips.py
index 3cc8038f1e6522dfc114f9ca64826391363ebd0c..dada4f98c9344c5a47065635b19dae9aee161da4 100644
--- a/tests/storage/test_client_ips.py
+++ b/tests/storage/test_client_ips.py
@@ -242,12 +242,7 @@ class ClientIpStoreTestCase(unittest.HomeserverTestCase):
def test_devices_last_seen_bg_update(self):
# First make sure we have completed all updates.
- while not self.get_success(
- self.store.db_pool.updates.has_completed_background_updates()
- ):
- self.get_success(
- self.store.db_pool.updates.do_next_background_update(100), by=0.1
- )
+ self.wait_for_background_updates()
user_id = "@user:id"
device_id = "MY_DEVICE"
@@ -311,12 +306,7 @@ class ClientIpStoreTestCase(unittest.HomeserverTestCase):
self.store.db_pool.updates._all_done = False
# Now let's actually drive the updates to completion
- while not self.get_success(
- self.store.db_pool.updates.has_completed_background_updates()
- ):
- self.get_success(
- self.store.db_pool.updates.do_next_background_update(100), by=0.1
- )
+ self.wait_for_background_updates()
# We should now get the correct result again
result = self.get_success(
@@ -337,12 +327,7 @@ class ClientIpStoreTestCase(unittest.HomeserverTestCase):
def test_old_user_ips_pruned(self):
# First make sure we have completed all updates.
- while not self.get_success(
- self.store.db_pool.updates.has_completed_background_updates()
- ):
- self.get_success(
- self.store.db_pool.updates.do_next_background_update(100), by=0.1
- )
+ self.wait_for_background_updates()
user_id = "@user:id"
device_id = "MY_DEVICE"
diff --git a/tests/storage/test_event_chain.py b/tests/storage/test_event_chain.py
index 93136f0717939e667482b422ef0eb618cdc2acd3..b31c5eb5ecc6ec75fdf4a2354c82ab0f6b35383b 100644
--- a/tests/storage/test_event_chain.py
+++ b/tests/storage/test_event_chain.py
@@ -578,12 +578,7 @@ class EventChainBackgroundUpdateTestCase(HomeserverTestCase):
# Ugh, have to reset this flag
self.store.db_pool.updates._all_done = False
- while not self.get_success(
- self.store.db_pool.updates.has_completed_background_updates()
- ):
- self.get_success(
- self.store.db_pool.updates.do_next_background_update(100), by=0.1
- )
+ self.wait_for_background_updates()
# Test that the `has_auth_chain_index` has been set
self.assertTrue(self.get_success(self.store.has_auth_chain_index(room_id)))
@@ -619,12 +614,7 @@ class EventChainBackgroundUpdateTestCase(HomeserverTestCase):
# Ugh, have to reset this flag
self.store.db_pool.updates._all_done = False
- while not self.get_success(
- self.store.db_pool.updates.has_completed_background_updates()
- ):
- self.get_success(
- self.store.db_pool.updates.do_next_background_update(100), by=0.1
- )
+ self.wait_for_background_updates()
# Test that the `has_auth_chain_index` has been set
self.assertTrue(self.get_success(self.store.has_auth_chain_index(room_id1)))
diff --git a/tests/storage/test_roommember.py b/tests/storage/test_roommember.py
index c72dc40510a420594bed20a776fb1413ee89c7a5..2873e22ccf8cf601dda2b0432a5aa14ad1d08b72 100644
--- a/tests/storage/test_roommember.py
+++ b/tests/storage/test_roommember.py
@@ -169,12 +169,7 @@ class CurrentStateMembershipUpdateTestCase(unittest.HomeserverTestCase):
def test_can_rerun_update(self):
# First make sure we have completed all updates.
- while not self.get_success(
- self.store.db_pool.updates.has_completed_background_updates()
- ):
- self.get_success(
- self.store.db_pool.updates.do_next_background_update(100), by=0.1
- )
+ self.wait_for_background_updates()
# Now let's create a room, which will insert a membership
user = UserID("alice", "test")
@@ -197,9 +192,4 @@ class CurrentStateMembershipUpdateTestCase(unittest.HomeserverTestCase):
self.store.db_pool.updates._all_done = False
# Now let's actually drive the updates to completion
- while not self.get_success(
- self.store.db_pool.updates.has_completed_background_updates()
- ):
- self.get_success(
- self.store.db_pool.updates.do_next_background_update(100), by=0.1
- )
+ self.wait_for_background_updates()
diff --git a/tests/storage/test_user_directory.py b/tests/storage/test_user_directory.py
index fddfb8db2875500edeb8bf1bc4571a73d9447bc9..9f483ad681c6ee8a2b99c2e9afb9fa9bba6fb342 100644
--- a/tests/storage/test_user_directory.py
+++ b/tests/storage/test_user_directory.py
@@ -212,12 +212,7 @@ class UserDirectoryInitialPopulationTestcase(HomeserverTestCase):
)
)
- while not self.get_success(
- self.store.db_pool.updates.has_completed_background_updates()
- ):
- self.get_success(
- self.store.db_pool.updates.do_next_background_update(100), by=0.1
- )
+ self.wait_for_background_updates()
def test_initial(self) -> None:
"""
diff --git a/tests/unittest.py b/tests/unittest.py
index ae393ee53eee8a682ad8ead02a3c70e420eabdee..81c1a9e9d2d29f4592de2a012242bb39c997d659 100644
--- a/tests/unittest.py
+++ b/tests/unittest.py
@@ -317,6 +317,15 @@ class HomeserverTestCase(TestCase):
self.reactor.advance(0.01)
time.sleep(0.01)
+ def wait_for_background_updates(self) -> None:
+ """Block until all background database updates have completed."""
+ while not self.get_success(
+ self.store.db_pool.updates.has_completed_background_updates()
+ ):
+ self.get_success(
+ self.store.db_pool.updates.do_next_background_update(100), by=0.1
+ )
+
def make_homeserver(self, reactor, clock):
"""
Make and return a homeserver.