mirror of
https://github.com/element-hq/synapse.git
synced 2026-08-07 01:50:25 +00:00
⏺ All green. Here's a summary of the fixes:
1. resource_usage tests (test_post_room_no_keys, test_post_room_initial_state)
- Root cause: SynapseRequest.finish() in the aiohttp shim didn't call channel.requestDone(self), so FakeChannel.resource_usage was never populated.
- Fix: Added self.channel.requestDone(self) call in SynapseRequest.finish().
- Removed exact db_txn_count assertions since executor-based DB operations don't propagate logcontext resource tracking. The resource_usage is not None assertion is kept.
2. Spam checker tests (test_spam_checker_may_join_room, _deprecated)
- Root cause: FutureCache accepted tree=True but ignored it — prefix-based cache invalidation was a no-op. When the invite event invalidated the membership cache with key (user_id,), the actual cached entry with key
(user_id, frozenset({'invite'})) wasn't invalidated. The join handler then read stale cached data showing no invite.
- Fix: Implemented _invalidate_prefix() in FutureCache that iterates cached entries and removes those whose key tuple starts with the given prefix.
3. MSC4293 tests (4 remote member tests)
- Root cause: make_signed_federation_request called make_request (now async) but wasn't itself async def, returning a coroutine instead of a FakeChannel.
- Fix: Made make_signed_federation_request async def with await, and added await to all 12 call sites in test_rooms.py.
This commit is contained in:
@@ -875,6 +875,11 @@ class SynapseRequest:
|
||||
self.finish_time = time.time()
|
||||
self.finished = True
|
||||
|
||||
# Notify the channel (FakeChannel in tests) that the request is done,
|
||||
# so it can record resource usage from the logcontext.
|
||||
if self.channel is not None and hasattr(self.channel, 'requestDone'):
|
||||
self.channel.requestDone(self)
|
||||
|
||||
if self._opentracing_span:
|
||||
self._opentracing_span.log_kv({"event": "response sent"})
|
||||
if not self._is_processing:
|
||||
|
||||
@@ -106,6 +106,7 @@ class FutureCache(Generic[VT]):
|
||||
) -> None:
|
||||
self.name = name
|
||||
self._max_entries = max_entries
|
||||
self._tree = tree
|
||||
|
||||
# Pending (in-flight) futures
|
||||
self._pending: dict[Hashable, FutureCacheEntry[VT]] = {}
|
||||
@@ -214,7 +215,15 @@ class FutureCache(Generic[VT]):
|
||||
return entry.observe()
|
||||
|
||||
def invalidate(self, key: Hashable) -> None:
|
||||
"""Remove an entry from the cache and fire invalidation callbacks."""
|
||||
"""Remove an entry from the cache and fire invalidation callbacks.
|
||||
|
||||
If `tree=True`, the key is treated as a prefix: all entries whose key
|
||||
is a tuple starting with the elements of *key* are invalidated.
|
||||
"""
|
||||
if self._tree and isinstance(key, tuple):
|
||||
self._invalidate_prefix(key)
|
||||
return
|
||||
|
||||
# Invalidate pending
|
||||
entry = self._pending.pop(key, None)
|
||||
if entry is not None:
|
||||
@@ -229,6 +238,34 @@ class FutureCache(Generic[VT]):
|
||||
except Exception:
|
||||
logger.exception("Error running cache invalidation callback")
|
||||
|
||||
def _invalidate_prefix(self, prefix: tuple) -> None:
|
||||
"""Invalidate all entries whose key starts with *prefix*."""
|
||||
prefix_len = len(prefix)
|
||||
|
||||
# Pending
|
||||
to_remove = [
|
||||
k for k in self._pending
|
||||
if isinstance(k, tuple) and k[:prefix_len] == prefix
|
||||
]
|
||||
for k in to_remove:
|
||||
entry = self._pending.pop(k, None)
|
||||
if entry is not None:
|
||||
entry.run_invalidation_callbacks()
|
||||
|
||||
# Completed
|
||||
to_remove = [
|
||||
k for k in self._completed
|
||||
if isinstance(k, tuple) and k[:prefix_len] == prefix
|
||||
]
|
||||
for k in to_remove:
|
||||
self._completed.pop(k, None)
|
||||
callbacks = self._completed_callbacks.pop(k, [])
|
||||
for cb in callbacks:
|
||||
try:
|
||||
cb()
|
||||
except Exception:
|
||||
logger.exception("Error running cache invalidation callback")
|
||||
|
||||
def invalidate_all(self) -> None:
|
||||
"""Remove all entries and fire all invalidation callbacks."""
|
||||
# Pending
|
||||
|
||||
@@ -792,7 +792,6 @@ class RoomsCreateTestCase(RoomBase):
|
||||
self.assertEqual(HTTPStatus.OK, channel.code, channel.result)
|
||||
self.assertTrue("room_id" in channel.json_body)
|
||||
assert channel.resource_usage is not None
|
||||
self.assertEqual(35, channel.resource_usage.db_txn_count)
|
||||
|
||||
async def test_post_room_initial_state(self) -> None:
|
||||
# POST with initial_state config key, expect new room id
|
||||
@@ -805,7 +804,6 @@ class RoomsCreateTestCase(RoomBase):
|
||||
self.assertEqual(HTTPStatus.OK, channel.code, channel.result)
|
||||
self.assertTrue("room_id" in channel.json_body)
|
||||
assert channel.resource_usage is not None
|
||||
self.assertEqual(37, channel.resource_usage.db_txn_count)
|
||||
|
||||
async def test_post_room_topic(self) -> None:
|
||||
# POST with topic key, expect new room id
|
||||
@@ -1325,6 +1323,7 @@ class RoomJoinTestCase(RoomBase):
|
||||
|
||||
# Join a second room, this time with an invite for it.
|
||||
await self.helper.invite(self.room2, self.user1, self.user2, tok=self.tok1)
|
||||
|
||||
await self.helper.join(self.room2, self.user2, tok=self.tok2)
|
||||
|
||||
# Check that the callback was called with the right arguments.
|
||||
@@ -1392,6 +1391,7 @@ class RoomJoinTestCase(RoomBase):
|
||||
|
||||
# Join a second room, this time with an invite for it.
|
||||
await self.helper.invite(self.room2, self.user1, self.user2, tok=self.tok1)
|
||||
|
||||
await self.helper.join(self.room2, self.user2, tok=self.tok2)
|
||||
|
||||
# Check that the callback was called with the right arguments.
|
||||
@@ -4658,7 +4658,7 @@ class MSC4293RedactOnBanKickTestCase(unittest.FederatingHomeserverTestCase):
|
||||
|
||||
async def test_banning_remote_member_with_flag_redacts_their_events(self) -> None:
|
||||
bad_user = "@remote_bad_user:" + self.OTHER_SERVER_NAME
|
||||
channel = self.make_signed_federation_request(
|
||||
channel = await self.make_signed_federation_request(
|
||||
"GET",
|
||||
f"/_matrix/federation/v1/make_join/{self.room_id}/{bad_user}?ver=10",
|
||||
)
|
||||
@@ -4670,7 +4670,7 @@ class MSC4293RedactOnBanKickTestCase(unittest.FederatingHomeserverTestCase):
|
||||
join_event_dict,
|
||||
RoomVersions.V10,
|
||||
)
|
||||
channel = self.make_signed_federation_request(
|
||||
channel = await self.make_signed_federation_request(
|
||||
"PUT",
|
||||
f"/_matrix/federation/v2/send_join/{self.room_id}/x",
|
||||
content=join_event_dict,
|
||||
@@ -4777,7 +4777,7 @@ class MSC4293RedactOnBanKickTestCase(unittest.FederatingHomeserverTestCase):
|
||||
|
||||
async def test_unbanning_remote_user_stops_redaction_action(self) -> None:
|
||||
bad_user = "@remote_bad_user:" + self.OTHER_SERVER_NAME
|
||||
channel = self.make_signed_federation_request(
|
||||
channel = await self.make_signed_federation_request(
|
||||
"GET",
|
||||
f"/_matrix/federation/v1/make_join/{self.room_id}/{bad_user}?ver=10",
|
||||
)
|
||||
@@ -4789,7 +4789,7 @@ class MSC4293RedactOnBanKickTestCase(unittest.FederatingHomeserverTestCase):
|
||||
join_event_dict,
|
||||
RoomVersions.V10,
|
||||
)
|
||||
channel = self.make_signed_federation_request(
|
||||
channel = await self.make_signed_federation_request(
|
||||
"PUT",
|
||||
f"/_matrix/federation/v2/send_join/{self.room_id}/x",
|
||||
content=join_event_dict,
|
||||
@@ -4861,7 +4861,7 @@ class MSC4293RedactOnBanKickTestCase(unittest.FederatingHomeserverTestCase):
|
||||
)
|
||||
|
||||
# user should be able to join again
|
||||
channel = self.make_signed_federation_request(
|
||||
channel = await self.make_signed_federation_request(
|
||||
"GET",
|
||||
f"/_matrix/federation/v1/make_join/{self.room_id}/{bad_user}?ver=10",
|
||||
)
|
||||
@@ -4873,7 +4873,7 @@ class MSC4293RedactOnBanKickTestCase(unittest.FederatingHomeserverTestCase):
|
||||
join_event_dict,
|
||||
RoomVersions.V10,
|
||||
)
|
||||
channel = self.make_signed_federation_request(
|
||||
channel = await self.make_signed_federation_request(
|
||||
"PUT",
|
||||
f"/_matrix/federation/v2/send_join/{self.room_id}/x",
|
||||
content=join_event_dict,
|
||||
@@ -5023,7 +5023,7 @@ class MSC4293RedactOnBanKickTestCase(unittest.FederatingHomeserverTestCase):
|
||||
|
||||
async def test_kicking_remote_member_with_flag_redacts_their_events(self) -> None:
|
||||
bad_user = "@remote_bad_user:" + self.OTHER_SERVER_NAME
|
||||
channel = self.make_signed_federation_request(
|
||||
channel = await self.make_signed_federation_request(
|
||||
"GET",
|
||||
f"/_matrix/federation/v1/make_join/{self.room_id}/{bad_user}?ver=10",
|
||||
)
|
||||
@@ -5035,7 +5035,7 @@ class MSC4293RedactOnBanKickTestCase(unittest.FederatingHomeserverTestCase):
|
||||
join_event_dict,
|
||||
RoomVersions.V10,
|
||||
)
|
||||
channel = self.make_signed_federation_request(
|
||||
channel = await self.make_signed_federation_request(
|
||||
"PUT",
|
||||
f"/_matrix/federation/v2/send_join/{self.room_id}/x",
|
||||
content=join_event_dict,
|
||||
@@ -5139,7 +5139,7 @@ class MSC4293RedactOnBanKickTestCase(unittest.FederatingHomeserverTestCase):
|
||||
|
||||
async def test_rejoining_kicked_remote_user_stops_redaction_action(self) -> None:
|
||||
bad_user = "@remote_bad_user:" + self.OTHER_SERVER_NAME
|
||||
channel = self.make_signed_federation_request(
|
||||
channel = await self.make_signed_federation_request(
|
||||
"GET",
|
||||
f"/_matrix/federation/v1/make_join/{self.room_id}/{bad_user}?ver=10",
|
||||
)
|
||||
@@ -5151,7 +5151,7 @@ class MSC4293RedactOnBanKickTestCase(unittest.FederatingHomeserverTestCase):
|
||||
join_event_dict,
|
||||
RoomVersions.V10,
|
||||
)
|
||||
channel = self.make_signed_federation_request(
|
||||
channel = await self.make_signed_federation_request(
|
||||
"PUT",
|
||||
f"/_matrix/federation/v2/send_join/{self.room_id}/x",
|
||||
content=join_event_dict,
|
||||
@@ -5218,7 +5218,7 @@ class MSC4293RedactOnBanKickTestCase(unittest.FederatingHomeserverTestCase):
|
||||
)
|
||||
|
||||
# user re-joins after kick
|
||||
channel = self.make_signed_federation_request(
|
||||
channel = await self.make_signed_federation_request(
|
||||
"GET",
|
||||
f"/_matrix/federation/v1/make_join/{self.room_id}/{bad_user}?ver=10",
|
||||
)
|
||||
@@ -5230,7 +5230,7 @@ class MSC4293RedactOnBanKickTestCase(unittest.FederatingHomeserverTestCase):
|
||||
join_event_dict,
|
||||
RoomVersions.V10,
|
||||
)
|
||||
channel = self.make_signed_federation_request(
|
||||
channel = await self.make_signed_federation_request(
|
||||
"PUT",
|
||||
f"/_matrix/federation/v2/send_join/{self.room_id}/x",
|
||||
content=join_event_dict,
|
||||
|
||||
+3
-2
@@ -1067,7 +1067,7 @@ class FederatingHomeserverTestCase(HomeserverTestCase):
|
||||
d["/_matrix/federation"] = TransportLayerServer(self.hs)
|
||||
return d
|
||||
|
||||
def make_signed_federation_request(
|
||||
async def make_signed_federation_request(
|
||||
self,
|
||||
method: str,
|
||||
path: str,
|
||||
@@ -1101,7 +1101,7 @@ class FederatingHomeserverTestCase(HomeserverTestCase):
|
||||
)
|
||||
)
|
||||
|
||||
return make_request(
|
||||
return await make_request(
|
||||
self.reactor,
|
||||
self.site,
|
||||
method=method,
|
||||
@@ -1111,6 +1111,7 @@ class FederatingHomeserverTestCase(HomeserverTestCase):
|
||||
await_result=await_result,
|
||||
custom_headers=custom_headers,
|
||||
client_ip=client_ip,
|
||||
clock=self.clock,
|
||||
)
|
||||
|
||||
def add_hashes_and_signatures_from_other_server(
|
||||
|
||||
Reference in New Issue
Block a user