From 6e2b45d78e3968de239db2283a82f0a4b996c389 Mon Sep 17 00:00:00 2001 From: MattC Date: Wed, 3 Aug 2022 14:12:58 +1000 Subject: [PATCH 01/10] Add - remote join capability to the module API "update_room_membership" method, in a backwards compatible manner. --- synapse/module_api/__init__.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 6d8bf5408364..d44267ad4814 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -117,6 +117,7 @@ JsonMapping, Requester, RoomAlias, + RoomID, StateMap, UserID, UserInfo, @@ -929,6 +930,7 @@ async def update_room_membership( room_id: str, new_membership: str, content: Optional[JsonDict] = None, + is_remote_join: bool = False, ) -> EventBase: """Updates the membership of a user to the given value. @@ -946,6 +948,9 @@ async def update_room_membership( https://spec.matrix.org/unstable/client-server-api/#mroommember for the list of allowed values. content: Additional values to include in the resulting event's content. + is_remote_join: Must be True if room_id refers to a remote room and + new_membership is "join" (i.e. a remote join is needed), otherwise must + be False. Returns: The newly created membership event. @@ -999,11 +1004,16 @@ async def update_room_membership( if "displayname" not in content: content["displayname"] = profile["displayname"] + remote_room_hosts = None + if is_remote_join: + remote_room_hosts = [RoomID.from_string(room_id).domain] + event_id, _ = await self._hs.get_room_member_handler().update_membership( requester=requester, target=target_user_id, room_id=room_id, action=new_membership, + remote_room_hosts=remote_room_hosts, content=content, ) From e6ada9d377917159c16ba6681afd1829c6d12149 Mon Sep 17 00:00:00 2001 From: MattC Date: Wed, 3 Aug 2022 14:14:57 +1000 Subject: [PATCH 02/10] Document - Add changelog entry for 13441. --- changelog.d/13441.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/13441.feature diff --git a/changelog.d/13441.feature b/changelog.d/13441.feature new file mode 100644 index 000000000000..18f9b3e76ef7 --- /dev/null +++ b/changelog.d/13441.feature @@ -0,0 +1 @@ +Add remote join capability to the module API "update_room_membership" method (in a backwards compatible manner). From 216dd623508b558f98f08b2686c4037590636bc4 Mon Sep 17 00:00:00 2001 From: MattC Date: Thu, 4 Aug 2022 11:36:25 +1000 Subject: [PATCH 03/10] Alter - approach to feed through "remote_room_hosts" parameter. --- synapse/module_api/__init__.py | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index d44267ad4814..683076c44fd8 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -117,7 +117,6 @@ JsonMapping, Requester, RoomAlias, - RoomID, StateMap, UserID, UserInfo, @@ -930,7 +929,7 @@ async def update_room_membership( room_id: str, new_membership: str, content: Optional[JsonDict] = None, - is_remote_join: bool = False, + remote_room_hosts: Optional[List[str]] = None, ) -> EventBase: """Updates the membership of a user to the given value. @@ -948,9 +947,7 @@ async def update_room_membership( https://spec.matrix.org/unstable/client-server-api/#mroommember for the list of allowed values. content: Additional values to include in the resulting event's content. - is_remote_join: Must be True if room_id refers to a remote room and - new_membership is "join" (i.e. a remote join is needed), otherwise must - be False. + remote_room_hosts: Remote servers to send the update to. Returns: The newly created membership event. @@ -1004,17 +1001,13 @@ async def update_room_membership( if "displayname" not in content: content["displayname"] = profile["displayname"] - remote_room_hosts = None - if is_remote_join: - remote_room_hosts = [RoomID.from_string(room_id).domain] - event_id, _ = await self._hs.get_room_member_handler().update_membership( requester=requester, target=target_user_id, room_id=room_id, action=new_membership, - remote_room_hosts=remote_room_hosts, content=content, + remote_room_hosts=remote_room_hosts, ) # Try to retrieve the resulting event. From 4c0380b8601d0a63f0613ae7ab5ef0f6649a97f8 Mon Sep 17 00:00:00 2001 From: MattC Date: Thu, 4 Aug 2022 12:22:43 +1000 Subject: [PATCH 04/10] Add - unit test coverage. --- tests/module_api/test_api.py | 40 +++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/tests/module_api/test_api.py b/tests/module_api/test_api.py index 169e29b59001..e1249743a360 100644 --- a/tests/module_api/test_api.py +++ b/tests/module_api/test_api.py @@ -11,11 +11,12 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -from unittest.mock import Mock +from unittest.mock import MagicMock, Mock, patch from twisted.internet import defer from synapse.api.constants import EduTypes, EventTypes +from synapse.api.errors import NotFoundError from synapse.events import EventBase from synapse.federation.units import Transaction from synapse.handlers.presence import UserPresenceState @@ -26,7 +27,7 @@ from tests.events.test_presence_router import send_presence_update, sync_presence from tests.replication._base import BaseMultiWorkerStreamTestCase -from tests.test_utils import simple_async_mock +from tests.test_utils import make_awaitable, simple_async_mock from tests.test_utils.event_injection import inject_member_event from tests.unittest import HomeserverTestCase, override_config from tests.utils import USE_POSTGRES_FOR_TESTS @@ -409,7 +410,8 @@ def test_send_local_online_presence_to_federation(self): self.assertTrue(found_update) - def test_update_membership(self): + @patch("synapse.handlers.room_member.RoomMemberMasterHandler._remote_join") + def test_update_membership(self, mocked_remote_join): """Tests that the module API can update the membership of a user in a room.""" peter = self.register_user("peter", "hackme") lesley = self.register_user("lesley", "hackme") @@ -532,6 +534,38 @@ def test_update_membership(self): self.assertEqual(res["displayname"], "simone") self.assertIsNone(res["avatar_url"]) + # Check that no remote join attempts have occurred thus far. + self.assertFalse(mocked_remote_join.called) + + # Necessary to fake a remote join. + fake_stream_id = 1 + if isinstance(mocked_remote_join, MagicMock): + # AyncMock is not supported in this Python version. + mocked_remote_join.return_value = make_awaitable( + ("fake-event-id", fake_stream_id) + ) + else: + mocked_remote_join.return_value = "fake-event-id", fake_stream_id + fake_remote_host = f"{self.module_api.server_name}-remote" + + # Given that the join is to be faked, we expect the relevant join event not to + # be persisted and the module API method to raise that. + self.get_failure( + defer.ensureDeferred( + self.module_api.update_room_membership( + sender=peter, + target=peter, + room_id=f"!nonexistent:{fake_remote_host}", + new_membership="join", + remote_room_hosts=[fake_remote_host], + ) + ), + NotFoundError, + ) + + # Check that a remote join was attempted. + self.assertEqual(mocked_remote_join.call_count, 1) + def test_get_room_state(self): """Tests that a module can retrieve the state of a room through the module API.""" user_id = self.register_user("peter", "hackme") From f178b456614b28ffa2b097e6adc70a8076b57715 Mon Sep 17 00:00:00 2001 From: MattC Date: Fri, 5 Aug 2022 09:59:15 +1000 Subject: [PATCH 05/10] Drop - redundant assert in the "update_room_membership" module API method. EventsWorkerStore.get_event will raise a "NotFoundError" before the assert. --- synapse/module_api/__init__.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 683076c44fd8..e387338d3695 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -1013,10 +1013,6 @@ async def update_room_membership( # Try to retrieve the resulting event. event = await self._hs.get_datastores().main.get_event(event_id) - # update_membership is supposed to always return after the event has been - # successfully persisted. - assert event is not None - return event async def create_and_send_event_into_room(self, event_dict: JsonDict) -> EventBase: From b1903358934d707854d6d1a1f1a04c67dc155bf8 Mon Sep 17 00:00:00 2001 From: MattC Date: Fri, 5 Aug 2022 10:01:06 +1000 Subject: [PATCH 06/10] Document - Add clarity to the "remote_room_hosts" parameter docstring. --- synapse/module_api/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index e387338d3695..77c0a024921e 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -947,7 +947,7 @@ async def update_room_membership( https://spec.matrix.org/unstable/client-server-api/#mroommember for the list of allowed values. content: Additional values to include in the resulting event's content. - remote_room_hosts: Remote servers to send the update to. + remote_room_hosts: Remote servers to use for remote joins/knocks/etc. Returns: The newly created membership event. From 30bc9e53847292da07e1b853c664fe79653be77a Mon Sep 17 00:00:00 2001 From: MattC Date: Fri, 5 Aug 2022 10:41:14 +1000 Subject: [PATCH 07/10] Refactor - remote join unit tests into separate method. --- tests/module_api/test_api.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/module_api/test_api.py b/tests/module_api/test_api.py index e1249743a360..8ce966726eb3 100644 --- a/tests/module_api/test_api.py +++ b/tests/module_api/test_api.py @@ -410,8 +410,7 @@ def test_send_local_online_presence_to_federation(self): self.assertTrue(found_update) - @patch("synapse.handlers.room_member.RoomMemberMasterHandler._remote_join") - def test_update_membership(self, mocked_remote_join): + def test_update_membership(self): """Tests that the module API can update the membership of a user in a room.""" peter = self.register_user("peter", "hackme") lesley = self.register_user("lesley", "hackme") @@ -534,9 +533,8 @@ def test_update_membership(self, mocked_remote_join): self.assertEqual(res["displayname"], "simone") self.assertIsNone(res["avatar_url"]) - # Check that no remote join attempts have occurred thus far. - self.assertFalse(mocked_remote_join.called) - + @patch("synapse.handlers.room_member.RoomMemberMasterHandler._remote_join") + def test_update_room_membership_remote_join(self, mocked_remote_join): # Necessary to fake a remote join. fake_stream_id = 1 if isinstance(mocked_remote_join, MagicMock): @@ -553,8 +551,8 @@ def test_update_membership(self, mocked_remote_join): self.get_failure( defer.ensureDeferred( self.module_api.update_room_membership( - sender=peter, - target=peter, + sender=f"@user:{self.module_api.server_name}", + target=f"@user:{self.module_api.server_name}", room_id=f"!nonexistent:{fake_remote_host}", new_membership="join", remote_room_hosts=[fake_remote_host], From a5721fa130c22e9fa3cc729104626564def97e93 Mon Sep 17 00:00:00 2001 From: MattC Date: Fri, 5 Aug 2022 11:24:36 +1000 Subject: [PATCH 08/10] Drop - redundant monkey patch from unit tests. Tests already have appropriate set-up phases to handle clean-up. --- tests/module_api/test_api.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/tests/module_api/test_api.py b/tests/module_api/test_api.py index 8ce966726eb3..514a5c28685d 100644 --- a/tests/module_api/test_api.py +++ b/tests/module_api/test_api.py @@ -11,7 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import Mock from twisted.internet import defer @@ -27,7 +27,7 @@ from tests.events.test_presence_router import send_presence_update, sync_presence from tests.replication._base import BaseMultiWorkerStreamTestCase -from tests.test_utils import make_awaitable, simple_async_mock +from tests.test_utils import simple_async_mock from tests.test_utils.event_injection import inject_member_event from tests.unittest import HomeserverTestCase, override_config from tests.utils import USE_POSTGRES_FOR_TESTS @@ -533,17 +533,14 @@ def test_update_membership(self): self.assertEqual(res["displayname"], "simone") self.assertIsNone(res["avatar_url"]) - @patch("synapse.handlers.room_member.RoomMemberMasterHandler._remote_join") - def test_update_room_membership_remote_join(self, mocked_remote_join): + def test_update_room_membership_remote_join(self): + """Test that the module API can join a remote room.""" # Necessary to fake a remote join. fake_stream_id = 1 - if isinstance(mocked_remote_join, MagicMock): - # AyncMock is not supported in this Python version. - mocked_remote_join.return_value = make_awaitable( - ("fake-event-id", fake_stream_id) - ) - else: - mocked_remote_join.return_value = "fake-event-id", fake_stream_id + mocked_remote_join = simple_async_mock( + return_value=("fake-event-id", fake_stream_id) + ) + self.hs.get_room_member_handler()._remote_join = mocked_remote_join fake_remote_host = f"{self.module_api.server_name}-remote" # Given that the join is to be faked, we expect the relevant join event not to From c32cb1e1966e58ad56d2b72976e081276b08ec00 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 5 Aug 2022 11:09:21 +0200 Subject: [PATCH 09/10] Keep track of the update in docstring --- synapse/module_api/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 77c0a024921e..3edb9e243192 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -934,6 +934,7 @@ async def update_room_membership( """Updates the membership of a user to the given value. Added in Synapse v1.46.0. + Changed in Synapse v1.65.0: Added the 'remote_room_hosts' parameter. Args: sender: The user performing the membership change. Must be a user local to From a8669f6abc635afa09423b718df466397d50d78c Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 5 Aug 2022 11:11:27 +0200 Subject: [PATCH 10/10] Update changelog.d/13441.feature --- changelog.d/13441.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/13441.feature b/changelog.d/13441.feature index 18f9b3e76ef7..3a4ae8bf0135 100644 --- a/changelog.d/13441.feature +++ b/changelog.d/13441.feature @@ -1 +1 @@ -Add remote join capability to the module API "update_room_membership" method (in a backwards compatible manner). +Add remote join capability to the module API's `update_room_membership` method (in a backwards compatible manner).