WIP: Applied feedback

Author: Yoric

docs/modules/spam_checker_callbacks.md

@@ -15,19 +15,19 @@ _Signature extended to support Allow and Code in Synapse v1.60.0_
 _Boolean and string return value types deprecated in Synapse v1.60.0_
 
 ```python
-async def check_event_for_spam(event: "synapse.module_api.EventBase") -> Union["synapse.module_api.Allow", "synapse.module_api.Codes", str, bool]
+async def check_event_for_spam(event: "synapse.module_api.EventBase") -> Union["synapse.module_api.ALLOW", "synapse.module_api.error.Codes", str, bool]
 ```
 
 Called when receiving an event from a client or via federation. The callback must return either:
   - `synapse.module_api.ALLOW`, to allow the operation. Other callbacks
     may still decide to reject it.
   - `synapse.api.Codes` to reject the operation with an error code. In case
-    of doubt, `synapse.api.Codes.FORBIDDEN` is a good error code.
+    of doubt, `synapse.api.error.Codes.FORBIDDEN` is a good error code.
   - (deprecated) a `str` to reject the operation and specify an error message. Note that clients
     typically will not localize the error message to the user's preferred locale.
   - (deprecated) on `False`, behave as `ALLOW`. Deprecated as confusing, as some
     callbacks in expect `True` to allow and others `True` to reject.
-  - (deprecated) on `True`, behave as `synapse.api.Codes.FORBIDDEN`. Deprecated as confusing, as
+  - (deprecated) on `True`, behave as `synapse.api.error.Codes.FORBIDDEN`. Deprecated as confusing, as
     some callbacks in expect `True` to allow and others `True` to reject.
 
 If multiple modules implement this callback, they will be considered in order. If a

synapse/events/spamcheck.py

@@ -275,9 +275,9 @@ async def check_event_for_spam(
             with Measure(
                 self.clock, "{}.{}".format(callback.__module__, callback.__qualname__)
             ):
-                res: Union[
-                    Decision, DEPRECATED_STR, DEPRECATED_BOOL
-                ] = await delay_cancellation(callback(event))
+                res: Union[Decision, str, bool] = await delay_cancellation(
+                    callback(event)
+                )
                 if res is False or res is Allow.ALLOW:
                     # This spam-checker accepts the event.
                     # Other spam-checkers may reject it, though.

synapse/module_api/__init__.py

@@ -36,7 +36,7 @@
 from twisted.web.resource import Resource
 
 from synapse import spam_checker_api
-from synapse.api.errors import Codes, SynapseError
+from synapse.api.errors import SynapseError
 from synapse.events import EventBase
 from synapse.events.presence_router import (
     GET_INTERESTED_USERS_CALLBACK,
@@ -151,7 +151,6 @@
     "run_in_background",
     "cached",
     "Allow",
-    "Codes",
     "UserID",
     "DatabasePool",
     "LoggingTransaction",

synapse/module_api/errors.py

@@ -15,6 +15,7 @@
 """Exception types which are exposed as part of the stable module API"""
 
 from synapse.api.errors import (
+    Codes,
     InvalidClientCredentialsError,
     RedirectException,
     SynapseError,
@@ -24,6 +25,7 @@
 from synapse.storage.push_rule import RuleNotFoundException
 
 __all__ = [
+    "Codes",
     "InvalidClientCredentialsError",
     "RedirectException",
     "SynapseError",

synapse/spam_checker_api/__init__.py

@@ -27,6 +27,10 @@ class RegistrationBehaviour(Enum):
     DENY = "deny"
 
 
+# We define the following singleton enum rather than a string to be able to
+# write `Union[Allow, ..., str]` in some of the callbacks for the spam-checker
+# API, where the `str` is required to maintain backwards compatibility with
+# previous versions of the API.
 class Allow(Enum):
     """
     Singleton to allow events to pass through in SpamChecker APIs.