feat(web)!: throttle in the storage of the application's injector, reset through Throttle

README.md

@@ -79,8 +79,13 @@ urlpatterns = winter.django.create_urls(app)
   `get_previous_page_url` and `get_next_page_url` is gone.
 - The request pipeline takes the injector from the application. `winter.application.set_injector` is needed only
   by `winter.sqlalchemy`, `winter.ddd` and `winter.messaging`.
-- `winter.django.create_urls` installs the Django throttling identity on its first call: replace it with
-  `winter.web.set_throttling_identity_resolver` after that.
+- Throttling counts in the storage that the injector of the application binds,
+  e.g. `binder.bind(ThrottlingStorage, to=RedisThrottlingStorage(configuration))`; an application with `@throttling`
+  and no storage fails on startup. The Django adapter throttles by the Django user, by the client address otherwise.
+  `set_throttling_storage` and `set_throttling_identity_resolver` are gone.
+- `winter.web.throttling.reset(request, scope)` is gone: an API method with `@throttling` declares a
+  `throttle: winter.web.Throttle` argument, and `throttle.reset()` forgets what the scope has counted for the sender:
+  for the identity that the request was counted for, even if the method has logged the user in since.
 - OpenAPI: `generate_openapi(app, title=..., version=...)` returns the spec and checks `@raises` annotations
   of the application (`allow_missing_raises_annotation=True` skips the check).
   `winter.openapi.serve_spec(app, spec, url='openapi.json')` and

docs/architecture.md

@@ -84,17 +84,17 @@ at least two of them.
 
 - **Framework behaviour is still process-global.** A `winter.web.WebApplication` holds the code of
   a web application, its injector and its interceptors. The adapters derive its exception handlers
-  from its definitions and handle each of its requests in a child of its injector. The other
-  behavioural registries of `winter.web` and `winter.openapi` (native response types, throttling,
-  OpenAPI inspectors) are module singletons, and
+  from its definitions, handle each of its requests in a child of its injector and throttle in the
+  storage that its injector binds. The other behavioural registries of `winter.web` and
+  `winter.openapi` (native response types, OpenAPI inspectors) are module singletons, and
   `winter.sqlalchemy`, `winter.ddd` and `winter.messaging` take the injector from the service
   locator `winter.application.get_injector`.
 - **Models are initialised once per process.** The Django adapter's `_initialize_process` and
-  `winter.openapi.initialization.initialize` fill those module singletons with the Django entries and
-  the standard inspectors on first use. In the
+  `winter.openapi.initialization.initialize` fill those module singletons with the Django response type
+  and the standard inspectors on first use. In the
   target, a `WebApplication` is given these registries like its interceptors, and they start with the
   standard entries. Building from an application, an adapter adds
-  its own entries (the native response type, the throttling identity) to that application only. The
+  its own entries (the native response type) to that application only. The
   `initialize` functions then disappear.
 - **`winter.wsgi`, `winter.asgi` and `winter.rabbitmq` do not exist yet.** HTTP is served through
   `winter.django` only, and `winter.messaging` has no transport adapter.
@@ -110,7 +110,8 @@ at least two of them.
 - **Isolation test** `tests/test_application_scope.py` builds two applications in one process
   and asserts that the handler of a `@problem` exception of one does not handle that exception in
   the other, that the arguments of an API method come from the injector of the application that
-  serves it, and that an interceptor of one does not run in the other. Each registry that moves into
+  serves it, that an interceptor of one does not run in the other, and that each counts throttled
+  requests in the storage of its own injector. Each registry that moves into
   the application adds its case there.
 - **This document** describes intent and is updated when the intent changes, not when the code
   moves. Deviations are listed above while they exist.

tests/api/api_with_throttling.py

@@ -1,8 +1,7 @@
 
 import winter.web
 from winter.web import route_post, route_get
-from winter.web import Request
-from winter.web.throttling import reset
+from winter.web import Throttle
 
 
 @route_get('with-throttling/')
@@ -33,7 +32,7 @@ def method_for_request_without_user(self) -> int:
 
     @route_get('with-reset/{?is_reset}')
     @winter.web.throttling('5/s', 'reset_scope')
-    def simple_method_with_reset(self, request: Request, is_reset: bool) -> int:
+    def simple_method_with_reset(self, throttle: Throttle, is_reset: bool) -> int:
         if is_reset:
-            reset(request, 'reset_scope')
+            throttle.reset()
         return 1

tests/apps.py

@@ -1,15 +1,9 @@
 from django.apps import AppConfig
 
-import winter.web
-from winter.web import InMemoryThrottlingStorage
-
 
 class TestAppConfig(AppConfig):
     name = 'tests'
 
     def ready(self):
         # Importing the URLs creates the application and its URL patterns
         from .urls import urlpatterns   # noqa: F401
-
-        # The Redis storage is covered in tests/redis; the sample application throttles in memory.
-        winter.web.set_throttling_storage(InMemoryThrottlingStorage())

tests/conftest.py

@@ -8,6 +8,8 @@
 from sqlalchemy.engine import Engine
 
 from winter.application import set_injector
+from winter.web import InMemoryThrottlingStorage
+from winter.web import ThrottlingStorage
 
 
 def pytest_configure():
@@ -44,6 +46,8 @@ def pytest_configure():
 class Configuration(Module):
     def configure(self, binder):
         binder.bind(Engine, to=CallableProvider(make_engine), scope=singleton)
+        # The Redis storage is covered in tests/redis; the sample application throttles in memory
+        binder.bind(ThrottlingStorage, to=InMemoryThrottlingStorage, scope=singleton)
 
 
 def make_engine():

tests/test_application_scope.py

@@ -2,7 +2,8 @@
 Isolation test for docs/architecture.md, principle 2: framework behaviour is never process-global.
 Two applications in one process must not share behaviour: the handler of a @problem exception of one application
 does not handle that exception in another, the arguments of an API method come from the injector of the
-application that serves it, and an interceptor of one application does not run in another.
+application that serves it, an interceptor of one application does not run in another, and each application counts
+throttled requests in the storage of its own injector.
 """
 from http import HTTPStatus
 
@@ -96,3 +97,32 @@ def test_interceptor_of_one_application_does_not_run_in_another():
     # Assert
     assert response_1.status_code == HTTPStatus.INTERNAL_SERVER_ERROR
     assert response_2.content == b'"ok"'
+
+
+class _ThrottledAPI:
+    @winter.web.route_get('throttled/')
+    @winter.web.throttling('1/m')
+    def get(self) -> str:
+        return 'ok'
+
+
+def _application_with_throttling() -> winter.web.WebApplication:
+    storage = winter.web.InMemoryThrottlingStorage()
+    app = winter.web.WebApplication(
+        modules=[],
+        injector=Injector([lambda binder: binder.bind(winter.web.ThrottlingStorage, to=storage)]),
+    )
+    app.add(_ThrottledAPI)
+    return app
+
+
+def test_throttled_requests_of_one_application_are_not_counted_in_another():
+    [url_1] = winter.django.create_urls(_application_with_throttling())
+    [url_2] = winter.django.create_urls(_application_with_throttling())
+    url_1.callback(RequestFactory().get('/throttled/'))
+
+    # Act
+    response = url_2.callback(RequestFactory().get('/throttled/'))
+
+    # Assert
+    assert response.status_code == HTTPStatus.OK

tests/web/test_throttling.py

@@ -3,12 +3,13 @@
 
 import freezegun
 import pytest
-from mock import patch
 
+import winter.django
+import winter.web
 from winter.web import ThrottlingMisconfigurationException
 from winter.web.throttling import InMemoryThrottlingStorage
-from winter.web.throttling import get_throttling_storage
-from winter.web.throttling import storage
+from winter.web.throttling import Throttle
+from winter.web.throttling.throttling import Throttling
 
 expected_error_response = {
     'status': 429,
@@ -71,12 +72,60 @@ def test_get_throttling_with_conditional_reset(api_client):
             assert response.status_code == HTTPStatus.OK, i
 
 
-@patch.object(storage, '_throttling_storage', None)
-def test_get_throttling_storage_without_configuration():
-    with pytest.raises(ThrottlingMisconfigurationException) as exc_info:
-        get_throttling_storage()
+class _ThrottledAPI:
+    @winter.web.route_get('throttled/')
+    @winter.web.throttling('5/s')
+    def get(self):  # pragma: no cover
+        pass
 
-    assert 'Throttling storage must be set' in str(exc_info.value)
+
+def test_throttling_without_storage_in_injector_fails_startup():
+    app = winter.web.WebApplication(modules=[])
+    app.add(_ThrottledAPI)
+
+    # Act
+    with pytest.raises(ThrottlingMisconfigurationException) as exception_info:
+        winter.django.create_urls(app)
+
+    # Assert
+    assert str(exception_info.value) == (
+        '_ThrottledAPI.get: @throttling needs a ThrottlingStorage bound in the injector'
+    )
+
+
+class _NotThrottledAPI:
+    @winter.web.route_get('not-throttled/')
+    def get(self, throttle: Throttle):  # pragma: no cover
+        pass
+
+
+def test_throttle_argument_without_throttling_fails_startup():
+    app = winter.web.WebApplication(modules=[])
+    app.add(_NotThrottledAPI)
+
+    # Act
+    with pytest.raises(TypeError) as exception_info:
+        winter.django.create_urls(app)
+
+    # Assert
+    assert str(exception_info.value) == (
+        '_NotThrottledAPI.get: argument throttle (Throttle) '
+        'is not mapped from the request and has no binding in the injector'
+    )
+
+
+def test_throttle_resets_identity_it_counted_the_request_for():
+    storage = InMemoryThrottlingStorage()
+    identities = iter(['10.0.0.1', '42'])
+    throttling = Throttling(num_requests=1, duration=60, scope='login')
+    throttle = Throttle(throttling, storage, None, lambda request: next(identities))
+    throttle.allow_request()
+
+    # Act
+    throttle.reset()
+
+    # Assert
+    assert storage.is_request_allowed('throttle_login_10.0.0.1', duration=60, num_requests=1)
 
 
 def test_in_memory_storage_sliding_window():

tests/web/test_throttling_identity.py

@@ -1,7 +1,5 @@
 from unittest import mock
 
-from winter.web.throttling import get_throttling_identity
-from winter.web.throttling import identity
 from winter.web.throttling import remote_address_identity
 
 
@@ -20,8 +18,3 @@ def test_remote_address_identity_prefers_x_forwarded_for():
     request = _request(headers={'X-Forwarded-For': '203.0.113.5, 10.0.0.1'})
 
     assert remote_address_identity(request) == '203.0.113.5,10.0.0.1'
-
-
-def test_installed_resolver_decides_the_identity():
-    with mock.patch.object(identity, '_identity_resolver', lambda request: 'api-key'):
-        assert get_throttling_identity(_request()) == 'api-key'

winter/django/view.py

@@ -12,7 +12,6 @@
 from winter.web import WebApplication
 from winter.web import handle_request
 from winter.web import register_native_response_type
-from winter.web import set_throttling_identity_resolver
 from winter.web.autodiscovery import find_routes
 from winter.web.exception_handlers import find_exception_handlers
 from winter.web.exceptions import ExceptionHandlers
@@ -40,11 +39,10 @@ def create_urls(app: WebApplication) -> List[URLPattern]:
 
 
 def _initialize_process():
-    """Process-global until these registries belong to an application: docs/architecture.md, Current deviations."""
+    """Process-global until native response types belong to an application: docs/architecture.md, Current deviations."""
     global _process_initialized
     if not _process_initialized:
         register_native_response_type(HttpResponseBase)
-        set_throttling_identity_resolver(django_user_identity)
         _process_initialized = True
 
 
@@ -98,7 +96,7 @@ def _create_dispatch_function(app: WebApplication, route: Route, exception_handl
     def dispatch(winter_view, request: django.http.HttpRequest, **path_parameters):
         winter_request = DjangoRequest(request, path_parameters)
         request_values = {django.http.HttpRequest: request}
-        response = handle_request(app, route, winter_request, exception_handlers, request_values)
+        response = handle_request(app, route, winter_request, exception_handlers, request_values, django_user_identity)
         return to_django_response(response)
 
     return dispatch

winter/web/__init__.py

@@ -26,10 +26,9 @@
 from .routing import route_post
 from .routing import route_put
 from .throttling import InMemoryThrottlingStorage
+from .throttling import Throttle
 from .throttling import ThrottlingMisconfigurationException
 from .throttling import ThrottlingStorage
-from .throttling import set_throttling_identity_resolver
-from .throttling import set_throttling_storage
 from .throttling import throttling
 from .urls import register_url_regexp
 from .web_application import WebApplication

winter/web/request_handler.py

@@ -21,11 +21,17 @@
 from .request_body_resolver import RequestBodyArgumentResolver
 from .request_header_resolver import RequestHeaderArgumentResolver
 from .request_scope import create_request_injector
+from .request_scope import is_bound
 from .response import Response
 from .response_entity import ResponseEntity
 from .response_header_resolver import ResponseHeaderArgumentResolver
 from .routing import Route
-from .throttling import create_throttle_class
+from .throttling import IdentityResolver
+from .throttling import Throttle
+from .throttling import ThrottlingMisconfigurationException
+from .throttling import ThrottlingStorage
+from .throttling import create_throttle
+from .throttling import get_throttling
 from .web_application import WebApplication
 
 # The arguments mapped from the request; any other argument comes from the injector of the request
@@ -48,11 +54,20 @@ def check_arguments(
 ):
     """
     Fails on startup for an argument of an API method, an interceptor or an exception handler that is not mapped
-    from the request and whose type the injector does not bind. Adapters bind the request types in each request.
+    from the request and whose type the injector does not bind, and for an API method with @throttling when the
+    injector binds no ThrottlingStorage. Adapters bind the request types in each request; an API method with
+    @throttling also receives its Throttle.
     """
     request_types = {Request, *request_types}
     for route in routes:
-        _arguments_resolver.check_arguments(route.method, app.injector, request_types)
+        if get_throttling(route.method) is None:
+            _arguments_resolver.check_arguments(route.method, app.injector, request_types)
+            continue
+        if not is_bound(app.injector, ThrottlingStorage):
+            raise ThrottlingMisconfigurationException(
+                f'{route.method.full_name}: @throttling needs a ThrottlingStorage bound in the injector',
+            )
+        _arguments_resolver.check_arguments(route.method, app.injector, {*request_types, Throttle})
     for interceptor in app.interceptors:
         pre_handle = ComponentMethod.get_or_create(interceptor.pre_handle)
         _arguments_resolver.check_arguments(pre_handle, app.injector, request_types, {'method'})
@@ -66,21 +81,26 @@ def handle_request(
     request: Request,
     exception_handlers: ExceptionHandlers,
     request_values: Mapping[type, object],
+    throttling_identity: IdentityResolver,
 ) -> Response:
     """
     Run the whole handling pipeline for a matched route of the application: throttling, its interceptors, argument
     resolution, the API method, its exception handlers and JSON encoding.
     Framework-agnostic: adapters build the Request from their own request object and send the returned Response.
     The request is handled in an injector of its own, a child of the application's one, that binds the Request
     and the request values of the adapter, such as its own request object. Request-scoped values live in it.
+    Throttling counts the request in the storage of the application's injector for the identity that the adapter
+    resolves, since the adapter knows the users of its framework.
     """
     method = route.method
     response_headers: dict[str, str] = {}
+    throttle = create_throttle(method, request, app.injector, throttling_identity)
+    if throttle is not None:
+        request_values = {**request_values, Throttle: throttle}
     injector = create_request_injector(app.injector, request, request_values)
 
-    throttle = create_throttle_class(route)
     try:
-        if throttle and not throttle.allow_request(request):
+        if throttle is not None and not throttle.allow_request():
             raise ThrottleException()
 
         for interceptor_cls in app.interceptors:

winter/web/throttling/__init__.py

@@ -1,12 +1,9 @@
 from .exceptions import ThrottlingMisconfigurationException
 from .identity import IdentityResolver
-from .identity import get_throttling_identity
 from .identity import remote_address_identity
-from .identity import set_throttling_identity_resolver
 from .storage import InMemoryThrottlingStorage
 from .storage import ThrottlingStorage
-from .storage import get_throttling_storage
-from .storage import set_throttling_storage
-from .throttling import create_throttle_class
-from .throttling import reset
+from .throttling import Throttle
+from .throttling import create_throttle
+from .throttling import get_throttling
 from .throttling import throttling

winter/web/throttling/identity.py

@@ -10,16 +10,3 @@ def remote_address_identity(request: Request) -> str:
     """The client address: X-Forwarded-For when a proxy sets it, the socket address otherwise."""
     forwarded_for = request.headers.get('X-Forwarded-For')
     return ''.join(forwarded_for.split()) if forwarded_for else request.remote_address
-
-
-_identity_resolver: IdentityResolver = remote_address_identity
-
-
-def set_throttling_identity_resolver(resolver: IdentityResolver):
-    """Adapters and applications install the resolver that knows their notion of a user."""
-    global _identity_resolver
-    _identity_resolver = resolver
-
-
-def get_throttling_identity(request: Request) -> str:
-    return _identity_resolver(request)