From 07771fa487e1d281fdcae35a47db87ab675cb6b3 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 19 May 2023 07:23:09 -0400 Subject: [PATCH] Remove experimental configuration flags & unstable values for faster 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. --- changelog.d/15625.misc | 1 + synapse/config/experimental.py | 12 ------- synapse/federation/federation_server.py | 2 -- synapse/federation/transport/client.py | 29 ++------------- .../federation/transport/server/federation.py | 12 +------ tests/federation/transport/test_client.py | 35 ++----------------- 6 files changed, 8 insertions(+), 83 deletions(-) create mode 100644 changelog.d/15625.misc diff --git a/changelog.d/15625.misc b/changelog.d/15625.misc new file mode 100644 index 000000000..7ea8cc943 --- /dev/null +++ b/changelog.d/15625.misc @@ -0,0 +1 @@ +Remove the unstable identifiers from faster joins ([MSC3706](https://github.com/matrix-org/matrix-spec-proposals/pull/3706). diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 6e453bd96..d769b7f66 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -84,18 +84,6 @@ class ExperimentalConfig(Config): "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) diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index c590d8f96..f4ca70a69 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -739,12 +739,10 @@ class FederationServer(FederationBase): "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 diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py index d2fa9976d..1cfc4446c 100644 --- a/synapse/federation/transport/client.py +++ b/synapse/federation/transport/client.py @@ -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( @@ -363,12 +362,8 @@ class TransportLayerClient: ) -> "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, @@ -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 @@ -964,14 +957,6 @@ class SendJoinParser(ByteParser[SendJoinResponse]): ] 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), @@ -980,14 +965,6 @@ class SendJoinParser(ByteParser[SendJoinResponse]): ) ) - 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( diff --git a/synapse/federation/transport/server/federation.py b/synapse/federation/transport/server/federation.py index 36b036250..3a744e25b 100644 --- a/synapse/federation/transport/server/federation.py +++ b/synapse/federation/transport/server/federation.py @@ -440,7 +440,6 @@ class FederationV2SendJoinServlet(BaseFederationServerServlet): 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, @@ -453,16 +452,7 @@ class FederationV2SendJoinServlet(BaseFederationServerServlet): # 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 diff --git a/tests/federation/transport/test_client.py b/tests/federation/transport/test_client.py index 3d61b1e8a..93e5c85a2 100644 --- a/tests/federation/transport/test_client.py +++ b/tests/federation/transport/test_client.py @@ -86,18 +86,7 @@ class SendJoinParserTestCase(TestCase): 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""" @@ -113,28 +102,10 @@ class SendJoinParserTestCase(TestCase): 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. @@ -143,7 +114,7 @@ class SendJoinParserTestCase(TestCase): 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.