Adress comments

5 files changed, 31 insertions, 27 deletions

diff --git a/docs/modules/third_party_rules_callbacks.md b/docs/modules/third_party_rules_callbacks.md
index 2ca23449e3..76428f5e43 100644
--- a/docs/modules/third_party_rules_callbacks.md
+++ b/docs/modules/third_party_rules_callbacks.md
@@ -265,23 +265,32 @@ server_.
 
 If multiple modules implement this callback, Synapse runs them all in order.
 
-### `on_threepid_unbind`
+### `unbind_threepid`
 
-_First introduced in Synapse v1.64.0_
+_First introduced in Synapse v1.74.0_
 
 ```python
-async def on_threepid_unbind(
+async def unbind_threepid(
     user_id: str, medium: str, address: str, identity_server: str
 ) -> Tuple[bool, bool]:
 ```
 
 Called before a threepid association is removed.
 
-It should return a tuple of 2 booleans reporting if a changed happened for the first, and if unbind
-needs to stop there for the second (True value). In this case no other module unbind will be
-called, and the default unbind made to the IS that was used on bind will also be skipped.
-In any case the mapping will be removed from the Synapse 3pid remote table, except if an Exception
-was raised at some point.
+The module is given the Matrix ID of the user to which an association is to be removed,
+as well as the medium (`email` or `msisdn`), address of the third-party identifier and
+the identity server where the threepid was successfully registered.
+
+A module can hence do its own custom unbinding, if for example it did also registered a custom
+binding logic with `on_threepid_bind`.
+
+It should return a tuple of 2 booleans:
+- first one should be `True` on a success calling the identity server, otherwise `False` if
+the identity server doesn't support unbinding (or no identity server found to contact).
+- second one should be `True` if unbind needs to stop there. In this case no other module
+unbind will be called, and the default unbind made to the IS that was used on bind will also be
+skipped. In any case the mapping will be removed from the Synapse 3pid remote table,
+except if an Exception was raised at some point.
 
 ## Example
 
diff --git a/synapse/events/third_party_rules.py b/synapse/events/third_party_rules.py
index e438f712fd..8030311697 100644
--- a/synapse/events/third_party_rules.py
+++ b/synapse/events/third_party_rules.py
@@ -45,9 +45,7 @@ CHECK_CAN_DEACTIVATE_USER_CALLBACK = Callable[[str, bool], Awaitable[bool]]
 ON_PROFILE_UPDATE_CALLBACK = Callable[[str, ProfileInfo, bool, bool], Awaitable]
 ON_USER_DEACTIVATION_STATUS_CHANGED_CALLBACK = Callable[[str, bool, bool], Awaitable]
 ON_THREEPID_BIND_CALLBACK = Callable[[str, str, str], Awaitable]
-ON_THREEPID_UNBIND_CALLBACK = Callable[
-    [str, str, str, str], Awaitable[Tuple[bool, bool]]
-]
+UNBIND_THREEPID_CALLBACK = Callable[[str, str, str, str], Awaitable[Tuple[bool, bool]]]
 
 
 def load_legacy_third_party_event_rules(hs: "HomeServer") -> None:
@@ -177,7 +175,7 @@ class ThirdPartyEventRules:
             ON_USER_DEACTIVATION_STATUS_CHANGED_CALLBACK
         ] = []
         self._on_threepid_bind_callbacks: List[ON_THREEPID_BIND_CALLBACK] = []
-        self._on_threepid_unbind_callbacks: List[ON_THREEPID_UNBIND_CALLBACK] = []
+        self._unbind_threepid_callbacks: List[UNBIND_THREEPID_CALLBACK] = []
 
     def register_third_party_rules_callbacks(
         self,
@@ -197,7 +195,7 @@ class ThirdPartyEventRules:
             ON_USER_DEACTIVATION_STATUS_CHANGED_CALLBACK
         ] = None,
         on_threepid_bind: Optional[ON_THREEPID_BIND_CALLBACK] = None,
-        on_threepid_unbind: Optional[ON_THREEPID_UNBIND_CALLBACK] = None,
+        unbind_threepid: Optional[UNBIND_THREEPID_CALLBACK] = None,
     ) -> None:
         """Register callbacks from modules for each hook."""
         if check_event_allowed is not None:
@@ -235,8 +233,8 @@ class ThirdPartyEventRules:
         if on_threepid_bind is not None:
             self._on_threepid_bind_callbacks.append(on_threepid_bind)
 
-        if on_threepid_unbind is not None:
-            self._on_threepid_unbind_callbacks.append(on_threepid_unbind)
+        if unbind_threepid is not None:
+            self._unbind_threepid_callbacks.append(unbind_threepid)
 
     async def check_event_allowed(
         self, event: EventBase, context: EventContext
@@ -532,7 +530,7 @@ class ThirdPartyEventRules:
                     "Failed to run module API callback %s: %s", callback, e
                 )
 
-    async def on_threepid_unbind(
+    async def unbind_threepid(
         self, user_id: str, medium: str, address: str, identity_server: str
     ) -> Tuple[bool, bool]:
         """Called before a threepid association is removed.
@@ -555,7 +553,7 @@ class ThirdPartyEventRules:
         """
 
         global_changed = False
-        for callback in self._on_threepid_unbind_callbacks:
+        for callback in self._unbind_threepid_callbacks:
             try:
                 (changed, stop) = await callback(
                     user_id, medium, address, identity_server
diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py
index b98c7b3b40..3257b8d0db 100644
--- a/synapse/handlers/identity.py
+++ b/synapse/handlers/identity.py
@@ -278,10 +278,7 @@ class IdentityHandler:
         medium = threepid["medium"]
         address = threepid["address"]
 
-        (
-            changed,
-            stop,
-        ) = await self.hs.get_third_party_event_rules().on_threepid_unbind(
+        (changed, stop,) = await self.hs.get_third_party_event_rules().unbind_threepid(
             mxid, medium, address, id_server
         )
 
diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py
index 6da9e1d8b2..dc34afcbff 100644
--- a/synapse/module_api/__init__.py
+++ b/synapse/module_api/__init__.py
@@ -67,7 +67,7 @@ from synapse.events.third_party_rules import (
     ON_NEW_EVENT_CALLBACK,
     ON_PROFILE_UPDATE_CALLBACK,
     ON_THREEPID_BIND_CALLBACK,
-    ON_THREEPID_UNBIND_CALLBACK,
+    UNBIND_THREEPID_CALLBACK,
     ON_USER_DEACTIVATION_STATUS_CHANGED_CALLBACK,
 )
 from synapse.handlers.account_data import ON_ACCOUNT_DATA_UPDATED_CALLBACK
@@ -320,7 +320,7 @@ class ModuleApi:
             ON_USER_DEACTIVATION_STATUS_CHANGED_CALLBACK
         ] = None,
         on_threepid_bind: Optional[ON_THREEPID_BIND_CALLBACK] = None,
-        on_threepid_unbind: Optional[ON_THREEPID_UNBIND_CALLBACK] = None,
+        unbind_threepid: Optional[UNBIND_THREEPID_CALLBACK] = None,
     ) -> None:
         """Registers callbacks for third party event rules capabilities.
 
@@ -337,7 +337,7 @@ class ModuleApi:
             on_profile_update=on_profile_update,
             on_user_deactivation_status_changed=on_user_deactivation_status_changed,
             on_threepid_bind=on_threepid_bind,
-            on_threepid_unbind=on_threepid_unbind,
+            unbind_threepid=unbind_threepid,
         )
 
     def register_presence_router_callbacks(
diff --git a/tests/rest/client/test_third_party_rules.py b/tests/rest/client/test_third_party_rules.py
index 2e1b4753dc..f6e9b4ea20 100644
--- a/tests/rest/client/test_third_party_rules.py
+++ b/tests/rest/client/test_third_party_rules.py
@@ -933,8 +933,8 @@ class ThirdPartyRulesTestCase(unittest.FederatingHomeserverTestCase):
         # Check that the mock was called with the right parameters
         self.assertEqual(args, (user_id, "email", "foo@example.com"))
 
-    def test_on_threepid_unbind(self) -> None:
-        """Tests that the on_threepid_unbind module callback is called correctly before
+    def test_unbind_threepid(self) -> None:
+        """Tests that the unbind_threepid module callback is called correctly before
         removing a 3PID mapping.
         """
 
@@ -972,7 +972,7 @@ class ThirdPartyRulesTestCase(unittest.FederatingHomeserverTestCase):
         # call identityserver.org
         threepid_unbind_mock = Mock(return_value=make_awaitable((True, True)))
         third_party_rules = self.hs.get_third_party_event_rules()
-        third_party_rules._on_threepid_unbind_callbacks.append(threepid_unbind_mock)
+        third_party_rules._unbind_threepid_callbacks.append(threepid_unbind_mock)
 
         # Deactivate the account, this should remove the 3pid mapping
         # and call the module handler.