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

Remove experimental configuration flags & unstable values for faster joins #15625

Merged
merged 3 commits into from
May 19, 2023
Merged
Show file tree
Hide file tree
Changes from all 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/15625.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove the unstable identifiers from faster joins ([MSC3706](https://github.com/matrix-org/matrix-spec-proposals/pull/3706).
12 changes: 0 additions & 12 deletions synapse/config/experimental.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,18 +84,6 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
"msc3984_appservice_key_query", False
)

# MSC3706 (server-side support for partial state in /send_join responses)
# Synapse will always serve partial state responses to requests using the stable
# query parameter `omit_members`. If this flag is set, Synapse will also serve
# partial state responses to requests using the unstable query parameter
# `org.matrix.msc3706.partial_state`.
self.msc3706_enabled: bool = experimental.get("msc3706_enabled", False)

# experimental support for faster joins over federation
# (MSC2775, MSC3706, MSC3895)
# requires a target server that can provide a partial join response (MSC3706)
self.faster_joins_enabled: bool = experimental.get("faster_joins", True)

# MSC3720 (Account status endpoint)
self.msc3720_enabled: bool = experimental.get("msc3720_enabled", False)

Expand Down
2 changes: 0 additions & 2 deletions synapse/federation/federation_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -739,12 +739,10 @@ async def on_send_join_request(
"event": event_json,
"state": [p.get_pdu_json(time_now) for p in state_events],
"auth_chain": [p.get_pdu_json(time_now) for p in auth_chain_events],
"org.matrix.msc3706.partial_state": caller_supports_partial_state,
"members_omitted": caller_supports_partial_state,
}

if servers_in_room is not None:
resp["org.matrix.msc3706.servers_in_room"] = list(servers_in_room)
resp["servers_in_room"] = list(servers_in_room)

return resp
Expand Down
29 changes: 3 additions & 26 deletions synapse/federation/transport/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ class TransportLayerClient:

def __init__(self, hs: "HomeServer"):
self.client = hs.get_federation_http_client()
self._faster_joins_enabled = hs.config.experimental.faster_joins_enabled
self._is_mine_server_name = hs.is_mine_server_name

async def get_room_state_ids(
Expand Down Expand Up @@ -363,12 +362,8 @@ async def send_join_v2(
) -> "SendJoinResponse":
path = _create_v2_path("/send_join/%s/%s", room_id, event_id)
query_params: Dict[str, str] = {}
if self._faster_joins_enabled:
# lazy-load state on join
query_params["org.matrix.msc3706.partial_state"] = (
"true" if omit_members else "false"
)
query_params["omit_members"] = "true" if omit_members else "false"
# lazy-load state on join
query_params["omit_members"] = "true" if omit_members else "false"

return await self.client.put_json(
destination=destination,
Expand Down Expand Up @@ -902,9 +897,7 @@ def _members_omitted_parser(response: SendJoinResponse) -> Generator[None, Any,
while True:
val = yield
if not isinstance(val, bool):
raise TypeError(
"members_omitted (formerly org.matrix.msc370c.partial_state) must be a boolean"
)
raise TypeError("members_omitted must be a boolean")
response.members_omitted = val


Expand Down Expand Up @@ -964,14 +957,6 @@ def __init__(self, room_version: RoomVersion, v1_api: bool):
]

if not v1_api:
self._coros.append(
ijson.items_coro(
_members_omitted_parser(self._response),
"org.matrix.msc3706.partial_state",
use_float="True",
)
)
# The stable field name comes last, so it "wins" if the fields disagree
self._coros.append(
ijson.items_coro(
_members_omitted_parser(self._response),
Expand All @@ -980,14 +965,6 @@ def __init__(self, room_version: RoomVersion, v1_api: bool):
)
)

self._coros.append(
ijson.items_coro(
_servers_in_room_parser(self._response),
"org.matrix.msc3706.servers_in_room",
use_float="True",
)
)

# Again, stable field name comes last
self._coros.append(
ijson.items_coro(
Expand Down
12 changes: 1 addition & 11 deletions synapse/federation/transport/server/federation.py
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,6 @@ def __init__(
server_name: str,
):
super().__init__(hs, authenticator, ratelimiter, server_name)
self._read_msc3706_query_param = hs.config.experimental.msc3706_enabled

async def on_PUT(
self,
Expand All @@ -453,16 +452,7 @@ async def on_PUT(
# TODO(paul): assert that event_id parsed from path actually
# match those given in content

partial_state = False
# The stable query parameter wins, if it disagrees with the unstable
# parameter for some reason.
stable_param = parse_boolean_from_args(query, "omit_members", default=None)
if stable_param is not None:
partial_state = stable_param
elif self._read_msc3706_query_param:
partial_state = parse_boolean_from_args(
query, "org.matrix.msc3706.partial_state", default=False
)
partial_state = parse_boolean_from_args(query, "omit_members", default=False)

result = await self.handler.on_send_join_request(
origin, content, room_id, caller_supports_partial_state=partial_state
Expand Down
35 changes: 3 additions & 32 deletions tests/federation/transport/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,18 +86,7 @@ def parse(response: JsonDict) -> bool:
return parsed_response.members_omitted

self.assertTrue(parse({"members_omitted": True}))
self.assertTrue(parse({"org.matrix.msc3706.partial_state": True}))

self.assertFalse(parse({"members_omitted": False}))
self.assertFalse(parse({"org.matrix.msc3706.partial_state": False}))

# If there's a conflict, the stable field wins.
self.assertTrue(
parse({"members_omitted": True, "org.matrix.msc3706.partial_state": False})
)
self.assertFalse(
parse({"members_omitted": False, "org.matrix.msc3706.partial_state": True})
)

def test_servers_in_room(self) -> None:
"""Check that the servers_in_room field is correctly parsed"""
Expand All @@ -113,28 +102,10 @@ def parse(response: JsonDict) -> Optional[List[str]]:
parsed_response = parser.finish()
return parsed_response.servers_in_room

self.assertEqual(
parse({"org.matrix.msc3706.servers_in_room": ["hs1", "hs2"]}),
["hs1", "hs2"],
)
self.assertEqual(parse({"servers_in_room": ["example.com"]}), ["example.com"])

# If both are provided, the stable identifier should win
self.assertEqual(
parse(
{
"org.matrix.msc3706.servers_in_room": ["old"],
"servers_in_room": ["new"],
}
),
["new"],
)

# And lastly, we should be able to tell if neither field was present.
self.assertEqual(
parse({}),
None,
)
# We should be able to tell the field is not present.
self.assertEqual(parse({}), None)

def test_errors_closing_coroutines(self) -> None:
"""Check we close all coroutines, even if closing the first raises an Exception.
Expand All @@ -143,7 +114,7 @@ def test_errors_closing_coroutines(self) -> None:
assertions about its attributes or type.
"""
parser = SendJoinParser(RoomVersions.V1, False)
response = {"org.matrix.msc3706.servers_in_room": ["hs1", "hs2"]}
response = {"servers_in_room": ["hs1", "hs2"]}
serialisation = json.dumps(response).encode()

# Mock the coroutines managed by this parser.
Expand Down