Back out implementation of MSC2314 (#12474)

MSC2314 has now been closed, so we're backing out its implementation, which
originally happened in #6176.

Unfortunately it's not a direct revert, as that PR mixed in a bunch of
unrelated changes to tests etc.

changelog.d/12474.misc

@@ -0,0 +1 @@
+Back out experimental implementation of [MSC2314](https://github.com/matrix-org/matrix-spec-proposals/pull/2314).

synapse/federation/federation_server.py

@@ -515,7 +515,7 @@ class FederationServer(FederationBase):
         )
 
     async def on_room_state_request(
-        self, origin: str, room_id: str, event_id: Optional[str]
+        self, origin: str, room_id: str, event_id: str
     ) -> Tuple[int, JsonDict]:
         origin_host, _ = parse_server_name(origin)
         await self.check_server_matches_acl(origin_host, room_id)
@@ -530,18 +530,13 @@ class FederationServer(FederationBase):
         # - but that's non-trivial to get right, and anyway somewhat defeats
         # the point of the linearizer.
         async with self._server_linearizer.queue((origin, room_id)):
-            resp: JsonDict = dict(
+            resp = await self._state_resp_cache.wrap(
-                await self._state_resp_cache.wrap(
+                (room_id, event_id),
-                    (room_id, event_id),
+                self._on_context_state_request_compute,
-                    self._on_context_state_request_compute,
+                room_id,
-                    room_id,
+                event_id,
-                    event_id,
-                )
             )
 
-        room_version = await self.store.get_room_version_id(room_id)
-        resp["room_version"] = room_version
-
         return 200, resp
 
     async def on_state_ids_request(
@@ -574,14 +569,11 @@ class FederationServer(FederationBase):
         return {"pdu_ids": state_ids, "auth_chain_ids": list(auth_chain_ids)}
 
     async def _on_context_state_request_compute(
-        self, room_id: str, event_id: Optional[str]
+        self, room_id: str, event_id: str
     ) -> Dict[str, list]:
         pdus: Collection[EventBase]
-        if event_id:
+        event_ids = await self.handler.get_state_ids_for_pdu(room_id, event_id)
-            event_ids = await self.handler.get_state_ids_for_pdu(room_id, event_id)
+        pdus = await self.store.get_events_as_list(event_ids)
-            pdus = await self.store.get_events_as_list(event_ids)
-        else:
-            pdus = (await self.state.get_current_state(room_id)).values()
 
         auth_chain = await self.store.get_auth_chain(
             room_id, [pdu.event_id for pdu in pdus]

synapse/federation/transport/server/federation.py

@@ -160,7 +160,7 @@ class FederationStateV1Servlet(BaseFederationServerServlet):
         return await self.handler.on_room_state_request(
             origin,
             room_id,
-            parse_string_from_args(query, "event_id", None, required=False),
+            parse_string_from_args(query, "event_id", None, required=True),
         )
 
 

sytest-blacklist

@@ -21,10 +21,6 @@ Newly created users see their own presence in /initialSync (SYT-34)
 # Blacklisted due to https://github.com/matrix-org/synapse/issues/1396
 Should reject keys claiming to belong to a different user
 
-# Blacklisted due to https://github.com/matrix-org/matrix-doc/pull/2314 removing
-# this requirement from the spec
-Inbound federation of state requires event_id as a mandatory paramater
-
 # Blacklisted until MSC2753 is implemented
 Local users can peek into world_readable rooms by room ID
 We can't peek into rooms with shared history_visibility

tests/federation/test_federation_server.py

@@ -104,58 +104,21 @@ class ServerACLsTestCase(unittest.TestCase):
 
 
 class StateQueryTests(unittest.FederatingHomeserverTestCase):
-
     servlets = [
         admin.register_servlets,
         room.register_servlets,
         login.register_servlets,
     ]
 
-    def test_without_event_id(self):
-        """
-        Querying v1/state/<room_id> without an event ID will return the current
-        known state.
-        """
-        u1 = self.register_user("u1", "pass")
-        u1_token = self.login("u1", "pass")
-
-        room_1 = self.helper.create_room_as(u1, tok=u1_token)
-        self.inject_room_member(room_1, "@user:other.example.com", "join")
-
-        channel = self.make_signed_federation_request(
-            "GET", "/_matrix/federation/v1/state/%s" % (room_1,)
-        )
-        self.assertEqual(200, channel.code, channel.result)
-
-        self.assertEqual(
-            channel.json_body["room_version"],
-            self.hs.config.server.default_room_version.identifier,
-        )
-
-        members = set(
-            map(
-                lambda x: x["state_key"],
-                filter(
-                    lambda x: x["type"] == "m.room.member", channel.json_body["pdus"]
-                ),
-            )
-        )
-
-        self.assertEqual(members, {"@user:other.example.com", u1})
-        self.assertEqual(len(channel.json_body["pdus"]), 6)
-
     def test_needs_to_be_in_room(self):
-        """
+        """/v1/state/<room_id> requires the server to be in the room"""
-        Querying v1/state/<room_id> requires the server
-        be in the room to provide data.
-        """
         u1 = self.register_user("u1", "pass")
         u1_token = self.login("u1", "pass")
 
         room_1 = self.helper.create_room_as(u1, tok=u1_token)
 
         channel = self.make_signed_federation_request(
-            "GET", "/_matrix/federation/v1/state/%s" % (room_1,)
+            "GET", "/_matrix/federation/v1/state/%s?event_id=xyz" % (room_1,)
         )
         self.assertEqual(403, channel.code, channel.result)
         self.assertEqual(channel.json_body["errcode"], "M_FORBIDDEN")