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

Commit

Permalink
Remove experimental configuration flags & unstable values for faster …
Browse files Browse the repository at this point in the history
…joins (#15625)

Synapse will no longer send (or respond to) the unstable flags
for faster joins. These were only available behind a configuration
flag and handled in parallel with the stable flags.
  • Loading branch information
clokep authored May 19, 2023
1 parent d0de452 commit 07771fa
Show file tree
Hide file tree
Showing 6 changed files with 8 additions and 83 deletions.
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

0 comments on commit 07771fa

Please sign in to comment.