Repository: sanic-org/sanic-ext
Pull request #127 (2022-09-11 21:13:40)

Allow defining injection signal
Solves #19 

prryplatypus: Hey @ahopkins, I'm having an issue with these tests, and I'm guessing that resetting and finalizing the signal router doesn't mix well with the testing client.

The logs can be seen here:
![Logs](https://i.imgur.com/ccTWhpB.png)

As you can see from the code here only the first log is output, and the second one isn't while serving a request. Thoughts on this?
![Code](https://i.imgur.com/68tUEtL.png)
prryplatypus: Managed to work around it in https://github.com/sanic-org/sanic-ext/pull/127/commits/c43dea7af73fb96de9719d3c30c045a0e03c2dab, only we can't register signals dynamically
ahopkins: @prryplatypus Doesn't look like that will work. Global variables is not the solution.
ahopkins: What is the problem? Why can we not just register it as a tuple, then have a signal factory that takes the event as an input? 
prryplatypus: I don't think there's any global variables in the workaround!

The issue seems to be that the signals don't get properly registered if it is done from within another signal handler.

As you can see from the logs, the signals technically got registered, but when a request was served, it acted as if no injection signal handlers had been defined.

Therefore what the workaround does is define both signal handlers always and within them only look for injections if there is any defined for their signal they are handling.
prryplatypus: Per discussion with @ahopkins, we're going to reduce the scope of this PR for now and simply make it a config value that applies to all injections. This can change in the future when we make it easier to dynamically add signal handlers from within other signal handlers.

All changes up to now reverted in https://github.com/sanic-org/sanic-ext/pull/127/commits/1d96f4a3b8deaca85b5318758bb7cca24270f8ee. 

Patch:
diff --git a/sanic_ext/config.py b/sanic_ext/config.py
index 51ad001..8dece6e 100644
--- a/sanic_ext/config.py
+++ b/sanic_ext/config.py
@@ -5,6 +5,8 @@ from typing import Any, Dict, List, Optional, Sequence, Union
 
 from sanic import Sanic
 from sanic.config import Config as SanicConfig
+from sanic.exceptions import SanicException
+from sanic.signals import Event
 
 
 class Config(SanicConfig):
@@ -33,6 +35,7 @@ class Config(SanicConfig):
         http_auto_head: bool = True,
         http_auto_options: bool = True,
         http_auto_trace: bool = False,
+        injection_signal: Union[str, Event] = Event.HTTP_ROUTING_AFTER,
         oas: bool = True,
         oas_autodoc: bool = True,
         oas_ignore_head: bool = True,
@@ -79,6 +82,7 @@ class Config(SanicConfig):
         self.HTTP_AUTO_HEAD = http_auto_head
         self.HTTP_AUTO_OPTIONS = http_auto_options
         self.HTTP_AUTO_TRACE = http_auto_trace
+        self.INJECTION_SIGNAL = injection_signal
         self.OAS = oas
         self.OAS_AUTODOC = oas_autodoc
         self.OAS_IGNORE_HEAD = oas_ignore_head
@@ -108,6 +112,15 @@ class Config(SanicConfig):
                 self.TRACE_EXCLUDED_HEADERS.split(",")
             )
 
+        if isinstance(self.INJECTION_SIGNAL, str):
+            self.INJECTION_SIGNAL = Event(self.INJECTION_SIGNAL)
+
+        valid_signals = ("http.handler.before", "http.routing.after")
+        if self.INJECTION_SIGNAL.value not in valid_signals:
+            raise SanicException(
+                f"Injection signal may only be one of {valid_signals}"
+            )
+
         self.load({key.upper(): value for key, value in kwargs.items()})
 
     @classmethod
diff --git a/sanic_ext/extensions/injection/injector.py b/sanic_ext/extensions/injection/injector.py
index d9d35f0..68e2a71 100644
--- a/sanic_ext/extensions/injection/injector.py
+++ b/sanic_ext/extensions/injection/injector.py
@@ -6,6 +6,7 @@ from typing import Any, Callable, Dict, Optional, Tuple, Type, get_type_hints
 
 from sanic import Sanic
 from sanic.constants import HTTP_METHODS
+from sanic.signals import Event
 
 from sanic_ext.extensions.injection.constructor import gather_args
 
@@ -30,7 +31,9 @@ def add_injection(app: Sanic, injection_registry: InjectionRegistry) -> None:
                     router_types.add(return_type)
         injection_registry.finalize(router_types)
 
-    @app.signal("http.routing.after")
+    injection_signal: Event = app.ext.config.INJECTION_SIGNAL
+
+    @app.signal(injection_signal)
     async def inject_kwargs(request, route, kwargs, **_):
         nonlocal signature_registry
 


Test patch:
diff --git a/tests/extensions/injection/test_injection_config.py b/tests/extensions/injection/test_injection_config.py
new file mode 100644
index 0000000..303ea17
--- /dev/null
+++ b/tests/extensions/injection/test_injection_config.py
@@ -0,0 +1,40 @@
+from unittest.mock import Mock
+
+import pytest
+from sanic.exceptions import SanicException
+from sanic.signals import Event
+
+from sanic_ext.config import Config
+from sanic_ext.extensions.injection.injector import add_injection
+
+
+def test_default_config_signal():
+    config = Config()
+    assert config.INJECTION_SIGNAL == Event.HTTP_ROUTING_AFTER
+
+
+def test_not_allowed_signals_error():
+    with pytest.raises(SanicException):
+        Config(injection_signal=Event.HTTP_LIFECYCLE_REQUEST)
+
+
+def test_http_routing_after_succeeds():
+    Config(injection_signal=Event.HTTP_ROUTING_AFTER)
+
+
+@pytest.mark.skipif(
+    not hasattr(Event, "HTTP_HANDLER_BEFORE"),
+    reason="Sanic version does not support HTTP_HANDLER_BEFORE",
+)
+def test_http_handler_before_succeeds():
+    Config(injection_signal=Event.HTTP_HANDLER_BEFORE)
+
+
+def test_add_injection_uses_signal_config():
+    app = Mock()
+
+    app.signal = Mock(return_value=Mock())
+    app.ext.config.INJECTION_SIGNAL = "random_string"
+    add_injection(app, Mock())
+
+    app.signal.assert_called_once_with("random_string")