diff --git a/README.md b/README.md index 696e8d23..8b1dd1c3 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,17 @@ urlpatterns = winter.django.create_urls(app) The handler of the nearest class in the exception's MRO handles it; two handlers of one exception class in an application fail on startup. A handler of the application replaces the built-in handler of the same exception class: a handler of `Exception` replaces the default 500 response. +- Arguments come from the request or from the injector of the application, see + [Values from the injector](#values-from-the-injector). Custom argument resolvers are gone: `ArgumentResolver`, + `ArgumentsResolver`, `arguments_resolver`, `RequestBodyArgumentResolver`, `ResponseHeaderArgumentResolver`, + `winter.web.pagination.PagePositionArgumentResolver` and `winter.django.HttpRequestArgumentResolver`. A resolver + of a type becomes a provider of that type marked `@winter.web.request_scope`. An argument of an API method, an + interceptor or an exception handler that is neither mapped from the request nor bound in the injector fails on + startup instead of on the first request. +- An argument named `response_headers` of an exception handler no longer receives the headers of the response: + declare a `ResponseHeader[T]` with `@winter.web.response_header`, as in an API method. +- 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. - OpenAPI: `generate_openapi(app, title=..., version=...)` returns the spec and checks `@raises` annotations @@ -328,7 +339,8 @@ class HelloWorldInterceptor(Interceptor): The only way now to register an interceptor is to add it to InterceptorRegistry singleton. -Don't forget to import this adding during app initialization. +Don't forget to import this adding during app initialization, before the URL patterns are created: they check +the arguments of the registered interceptors on startup. ```python from winter.web import interceptor_registry from .interceptors import HelloWorldInterceptor @@ -339,6 +351,42 @@ class SomeInitializationClassInMyApplication: ``` +## Values from the injector +An argument that is not mapped from the request (path and query parameters, `@request_body`, `@request_header`, +`@response_header`, `PagePosition`) comes from the injector of the application, which must bind its type: +otherwise the application fails on startup. The dependencies of the class itself go to `__init__`, where the +injector creates them without bindings. A provider marked `@winter.web.request_scope` is called once per request, +and its value exists only while Winter handles a request. Every request binds `winter.web.Request` and, under +Django, `django.http.HttpRequest`. +```python +import injector +import winter.web +from django.http import HttpRequest +from winter.application import Application + + +class AccountModule(injector.Module): + @injector.provider + @winter.web.request_scope + def account(self, request: HttpRequest, accounts: AccountQueries) -> Account: + return accounts.get(request.user.pk) + + +class ProfileAPI: + @injector.inject + def __init__(self, profiles: ProfileQueries): + self._profiles = profiles + + @winter.web.route_get('profile/') + def get(self, account: Account) -> ProfileDTO: + return self._profiles.get(account) + + +app = Application(modules=[profiles], injector=injector.Injector([AccountModule()])) +``` + +Interceptors and exception handlers of a request get the same request-scoped values as its API method. + ## Undefined JSON fields By default, if a JSON request contains a field that is not defined in a dataclass, an exception will be thrown. diff --git a/docs/architecture.md b/docs/architecture.md index 63d97af4..ee845531 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -68,8 +68,9 @@ at least two of them. subpackage can later ship as its own distribution from its own repository without any import path changing. 2. **Framework behaviour is never process-global.** Every behavioural registry (exception - handlers, interceptors, argument resolvers, throttling configuration, OpenAPI inspectors) - belongs to an `Application` instance, so two applications can live in one process. + handlers, interceptors, output processors, throttling configuration, OpenAPI inspectors) + belongs to an `Application` instance, and the values that its methods receive come from its + injector, so two applications can live in one process. Declarative metadata about types (component annotations, JSON encoders for a type) may be global, like the classes it describes. 3. **Glue between models, or between adapters, is application code.** Mapping a domain @@ -81,18 +82,19 @@ at least two of them. ## Current deviations - **Framework behaviour is still process-global.** An `Application` holds the code of an - application and its injector, and the adapters derive its exception handlers from its definitions. - The other behavioural registries of `winter.web` and `winter.openapi` (interceptors, argument - resolvers, output processors, response header serializers, native response types, throttling, - OpenAPI inspectors) are module singletons, and the application installs its injector as the - service locator `winter.application.get_injector`. + application and its injector. 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` (interceptors, output processors, response header serializers, + native response types, throttling, 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.** `winter.web.initialization.initialize`, the Django adapter's `_initialize_process` and `winter.openapi.initialization.initialize` fill those module - singletons with the standard resolvers and inspectors on first use. In the target, each - `Application` owns these registries in an injector of its own, a child of the injector it is given, - and they start with the standard entries. Building from an application, an adapter adds its own - entries (the Django request resolver, the native response type, the throttling identity) to that - application only. The `initialize` functions then disappear. + singletons with the standard output processors, serializers and inspectors on first use. In the + target, each `Application` owns these registries in an injector of its own, a child of the injector + it is given, 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 + `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. - **All dependencies are mandatory.** Django, SQLAlchemy, Redis and pydantic are installed with @@ -106,6 +108,7 @@ at least two of them. There are no `ignore_imports`, and none may be added. - **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. Each registry that moves into `Application` adds its case there. + the other, and that the arguments of an API method come from the injector of the application + that serves it. Each registry that moves into `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. diff --git a/tests/api/api_with_exceptions.py b/tests/api/api_with_exceptions.py index 57f38083..d7257097 100644 --- a/tests/api/api_with_exceptions.py +++ b/tests/api/api_with_exceptions.py @@ -10,10 +10,6 @@ def __init__(self, message: str): self.message = message -class WithUnknownArgumentException(Exception): - pass - - class ExceptionWithoutHandler(Exception): pass @@ -29,15 +25,6 @@ class ExceptionHandlers: def handle_custom(self, exception: CustomException) -> CustomExceptionDTO: return CustomExceptionDTO(exception.message) - @winter.web.exception_handler - @response_status(400) - def handle_with_unknown_argument( - self, - exception: WithUnknownArgumentException, - unknown_argument: int, - ) -> str: # pragma: no cover - pass - class ChildCustomException(CustomException): pass @@ -69,9 +56,3 @@ def not_declared_but_thrown(self) -> str: @raises(ExceptionWithoutHandler) def declared_but_no_handler(self) -> str: raise ExceptionWithoutHandler() - - @raises(WithUnknownArgumentException) - @raises(CustomException) # second "raises" just for 100% test coverage - @route_get('with_unknown_argument_exception/') - def with_unknown_argument_handler(self) -> str: - raise WithUnknownArgumentException() diff --git a/tests/api/api_with_response_headers.py b/tests/api/api_with_response_headers.py index e842a139..f5a2c1c1 100644 --- a/tests/api/api_with_response_headers.py +++ b/tests/api/api_with_response_headers.py @@ -47,7 +47,3 @@ def two_headers(self, header1: ResponseHeader[str], header2: ResponseHeader[str] header1.set('header1') header2.set('header2') return 'OK' - - @winter.web.route_get('header-without-annotation/') - def header_without_annotation(self, header: ResponseHeader[str]) -> str: # pragma: no cover - pass diff --git a/tests/application/test_application.py b/tests/application/test_application.py index 1c04064f..b3201a05 100644 --- a/tests/application/test_application.py +++ b/tests/application/test_application.py @@ -7,15 +7,6 @@ from winter.application import Application from winter.application import get_injector -from winter.application import set_injector - - -@pytest.fixture(autouse=True) -def restore_injector(): - # Application installs its injector globally; the rest of the suite runs with the one from conftest - injector = get_injector() - yield - set_injector(injector) class Defined: @@ -103,13 +94,13 @@ def test_add(): assert app.definitions == (Defined, defined) -def test_new_injector_is_installed(): +def test_new_injector_is_not_installed_globally(): # Act app = Application(modules=[]) # Assert assert isinstance(app.injector, Injector) - assert get_injector() is app.injector + assert get_injector() is not app.injector def test_given_injector_is_shared(): diff --git a/tests/apps.py b/tests/apps.py index 6d95693f..cc2247ac 100644 --- a/tests/apps.py +++ b/tests/apps.py @@ -10,10 +10,11 @@ class TestAppConfig(AppConfig): name = 'tests' def ready(self): + # Before the URL patterns, which check the arguments of the interceptors on startup + interceptor_registry.add_interceptor(HelloWorldInterceptor()) + # Importing the URLs creates the application and its URL patterns from .urls import urlpatterns # noqa: F401 - interceptor_registry.add_interceptor(HelloWorldInterceptor()) - # The Redis storage is covered in tests/redis; the sample application throttles in memory. winter.web.set_throttling_storage(InMemoryThrottlingStorage()) diff --git a/tests/test_application_scope.py b/tests/test_application_scope.py index 5ebabf7b..e01e0138 100644 --- a/tests/test_application_scope.py +++ b/tests/test_application_scope.py @@ -1,11 +1,13 @@ """ 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. +does not handle that exception in another, and the arguments of an API method come from the injector of the +application that serves it. """ from http import HTTPStatus from django.test import RequestFactory +from injector import Injector import winter.django import winter.web @@ -37,3 +39,33 @@ def test_problem_handler_of_one_application_does_not_handle_exceptions_in_anothe # Assert assert response.status_code == HTTPStatus.INTERNAL_SERVER_ERROR + + +class _Name: + def __init__(self, value: str): + self.value = value + + +class _NameAPI: + @winter.web.route_get('name/') + def get(self, name: _Name) -> str: + return name.value + + +def _application_with_name(value: str) -> Application: + app = Application(modules=[], injector=Injector([lambda binder: binder.bind(_Name, to=_Name(value))])) + app.add(_NameAPI) + return app + + +def test_argument_of_api_method_comes_from_injector_of_its_application(): + [url_1] = winter.django.create_urls(_application_with_name('one')) + [url_2] = winter.django.create_urls(_application_with_name('two')) + + # Act + response_1 = url_1.callback(RequestFactory().get('/name/')) + response_2 = url_2.callback(RequestFactory().get('/name/')) + + # Assert + assert response_1.content == b'"one"' + assert response_2.content == b'"two"' diff --git a/tests/web/test_argument_resolver.py b/tests/web/test_argument_resolver.py index 21c306b6..26883991 100644 --- a/tests/web/test_argument_resolver.py +++ b/tests/web/test_argument_resolver.py @@ -1,82 +1,110 @@ import pytest +from injector import Injector from mock import Mock -from winter.web import ArgumentsResolver from winter.components import ComponentMethod -from winter.web.argument_resolver import ArgumentNotSupported +from winter.web import Request +from winter.web.argument_resolver import ArgumentsResolver +from winter.web.request_scope import create_request_injector + + +class _Dependency: + pass + + +def _resolver(is_supported: bool, value=None): + resolver = Mock() + resolver.is_supported.return_value = is_supported + resolver.resolve_argument.return_value = value + return resolver + + +def _resolve(arguments_resolver: ArgumentsResolver, func, app_injector=None, context=None): + request_injector = create_request_injector(app_injector or Injector(), Mock(spec=Request), {}) + return arguments_resolver.resolve_arguments(ComponentMethod(func), Mock(), {}, request_injector, context) def test_resolve_arguments_returns_empty_dict_for_empty_arguments(): def func(): # pragma: no cover pass - expected_resolved_arguments = {} - method = ComponentMethod(func) - arguments_resolver = ArgumentsResolver() - # Act - resolved_arguments = arguments_resolver.resolve_arguments(method, request=Mock(), response_headers={}) + resolved_arguments = _resolve(ArgumentsResolver([]), func) # Assert - assert resolved_arguments == expected_resolved_arguments + assert resolved_arguments == {} -def test_resolve_arguments_resolves_argument_with_the_first_resolver(): +def test_first_resolver_that_supports_argument_resolves_it(): def func(a: int): # pragma: no cover pass - expected_resolved_value = 1 - expected_resolved_arguments = { - 'a': expected_resolved_value, - } - method = ComponentMethod(func) - arguments_resolver = ArgumentsResolver() - resolver = Mock() - resolver.is_supported.return_value = True - resolver.resolve_argument.return_value = expected_resolved_value - arguments_resolver.add_argument_resolver(resolver) + arguments_resolver = ArgumentsResolver([_resolver(False), _resolver(True, 1), _resolver(True, 2)]) # Act - resolved_arguments = arguments_resolver.resolve_arguments(method, request=Mock(), response_headers={}) + resolved_arguments = _resolve(arguments_resolver, func) # Assert - assert resolved_arguments == expected_resolved_arguments + assert resolved_arguments == {'a': 1} -def test_resolve_arguments_resolves_argument_with_the_second_resolver(): +def test_context_value_is_taken_before_resolvers(): def func(a: int): # pragma: no cover pass - expected_resolved_value = 1 - expected_resolved_arguments = { - 'a': expected_resolved_value, - } - method = ComponentMethod(func) - arguments_resolver = ArgumentsResolver() - resolver1 = Mock() - resolver1.is_supported.return_value = False - arguments_resolver.add_argument_resolver(resolver1) - resolver2 = Mock() - resolver2.is_supported.return_value = True - resolver2.resolve_argument.return_value = expected_resolved_value - arguments_resolver.add_argument_resolver(resolver2) + # Act + resolved_arguments = _resolve(ArgumentsResolver([_resolver(True, 1)]), func, context={'a': 2}) + + # Assert + assert resolved_arguments == {'a': 2} + + +def test_argument_that_no_resolver_supports_comes_from_injector(): + def func(dependency: _Dependency): # pragma: no cover + pass + + dependency = _Dependency() + app_injector = Injector([lambda binder: binder.bind(_Dependency, to=dependency)]) # Act - resolved_arguments = arguments_resolver.resolve_arguments(method, request=Mock(), response_headers={}) + resolved_arguments = _resolve(ArgumentsResolver([_resolver(False)]), func, app_injector) # Assert - assert resolved_arguments == expected_resolved_arguments + assert resolved_arguments == {'dependency': dependency} -def test_resolve_arguments_fails(): - def func(a: int): # pragma: no cover +def test_argument_that_injector_only_can_create_is_not_resolved(): + def func(dependency: _Dependency): # pragma: no cover pass - arg_name = 'a' - method = ComponentMethod(func) - arguments_resolver = ArgumentsResolver() + # Act + with pytest.raises(TypeError) as exception_info: + _resolve(ArgumentsResolver([]), func) # Assert - with pytest.raises(ArgumentNotSupported, match=f'Unable to resolve argument {arg_name}: int'): - # Act - arguments_resolver.resolve_arguments(method, request=Mock(), response_headers={}) + assert str(exception_info.value) == ( + 'test_argument_that_injector_only_can_create_is_not_resolved..func: argument dependency (_Dependency) ' + 'is not mapped from the request and has no binding in the injector' + ) + + +@pytest.mark.parametrize('request_types, context_names', [ + ([_Dependency], ()), + ((), ['dependency']), +]) +def test_check_accepts_request_types_and_context_names(request_types, context_names): + def func(dependency: _Dependency): # pragma: no cover + pass + + # Act + ArgumentsResolver([]).check_arguments(ComponentMethod(func), Injector(), request_types, context_names) + + +def test_check_accepts_binding_of_parent_injector(): + def func(dependency: _Dependency): # pragma: no cover + pass + + parent_injector = Injector([lambda binder: binder.bind(_Dependency)]) + + # Act + ArgumentsResolver([]).check_arguments(ComponentMethod(func), parent_injector.create_child_injector(), ()) diff --git a/tests/web/test_exception_handlers.py b/tests/web/test_exception_handlers.py index ceca9bd2..663f2074 100644 --- a/tests/web/test_exception_handlers.py +++ b/tests/web/test_exception_handlers.py @@ -109,6 +109,42 @@ def test_handler_annotated_with_union_handles_each_exception(exception, expected assert response.content == expected_content +class _FailingHandlers: + @winter.web.exception_handler + def handle_other(self, exception: _OtherError): + raise _BaseError() + + @winter.web.exception_handler + def handle_base(self, exception: _BaseError) -> str: # pragma: no cover + return 'base' + + +def test_exception_of_failing_handler_goes_to_handler_of_exception(): + # Act + response = _get(_raising(_OtherError()), _FailingHandlers) + + # Assert + assert response.status_code == HTTPStatus.INTERNAL_SERVER_ERROR + assert response.content == b'Server Error (500)' + + +class _RetryHandler: + @winter.web.exception_handler + @winter.web.response_header('Retry-After', 'retry_after') + @winter.web.response_status(HTTPStatus.SERVICE_UNAVAILABLE) + def handle(self, exception: _OtherError, retry_after: winter.web.ResponseHeader[int]): + retry_after.set(30) + + +def test_handler_sets_response_header(): + # Act + response = _get(_raising(_OtherError()), _RetryHandler) + + # Assert + assert response.status_code == HTTPStatus.SERVICE_UNAVAILABLE + assert response['Retry-After'] == '30' + + def test_two_handlers_of_same_exception_in_application_fail_startup(): class _OtherHandlers: @winter.web.exception_handler diff --git a/tests/web/test_exception_handling.py b/tests/web/test_exception_handling.py index e97df853..7b0b26cd 100644 --- a/tests/web/test_exception_handling.py +++ b/tests/web/test_exception_handling.py @@ -2,7 +2,6 @@ import pytest -from winter.web.argument_resolver import ArgumentNotSupported from tests.api.api_with_exceptions import CustomException from tests.api.api_with_exceptions import ExceptionWithoutHandler @@ -29,7 +28,6 @@ def test_api_with_exceptions(api_client, url_path, expected_status, expected_bod @pytest.mark.parametrize( 'url_path', ( '/with_exceptions/declared_but_no_handler/', - '/with_exceptions/with_unknown_argument_exception/', ), ) def test_api_with_exceptions_throws_500(api_client, url_path): diff --git a/tests/web/test_request_scope.py b/tests/web/test_request_scope.py new file mode 100644 index 00000000..65f4ffe1 --- /dev/null +++ b/tests/web/test_request_scope.py @@ -0,0 +1,185 @@ +import json +from dataclasses import dataclass +from http import HTTPStatus + +import injector +import pytest +from django.http import HttpRequest +from django.test import RequestFactory +from mock import patch + +import winter.django +import winter.web +from winter.application import Application +from winter.web import interceptor_registry + + +class _Visit: + def __init__(self, number: int, path: str): + self.number = number + self.path = path + + +class _VisitModule(injector.Module): + def __init__(self): + self._visits = 0 + + @injector.provider + @winter.web.request_scope + def visit(self, request: HttpRequest) -> _Visit: + self._visits += 1 + return _Visit(self._visits, request.path) + + +class _VisitAPI: + @winter.web.route_get('visit/') + def get(self, visit: _Visit, same_visit: _Visit, request: winter.web.Request) -> dict: + return {'number': visit.number, 'path': visit.path, 'same': visit is same_visit, 'method': request.method} + + +def _create_urls(*definitions, app_injector=None): + app = Application(modules=[], injector=app_injector or injector.Injector()) + for definition in definitions: + app.add(definition) + return winter.django.create_urls(app) + + +def _get(url, path: str): + return url.callback(RequestFactory().get(path)) + + +def test_request_scoped_value_is_created_once_per_request(): + [url] = _create_urls(_VisitAPI, app_injector=injector.Injector([_VisitModule()])) + + # Act + first_response = _get(url, '/visit/') + second_response = _get(url, '/visit/') + + # Assert + assert json.loads(first_response.content) == {'number': 1, 'path': '/visit/', 'same': True, 'method': 'GET'} + assert json.loads(second_response.content) == {'number': 2, 'path': '/visit/', 'same': True, 'method': 'GET'} + + +class _VisitInterceptor(winter.web.Interceptor): + def __init__(self): + self.visits = [] + + def pre_handle(self, visit: _Visit): + self.visits.append(visit) + + +class _VisitNumberAPI: + @winter.web.route_get('visit/') + def get(self, visit: _Visit) -> int: + return visit.number + + +def test_interceptor_and_api_method_get_same_request_scoped_value(): + interceptor = _VisitInterceptor() + with patch.object(interceptor_registry, '_interceptors', [interceptor]): + [url] = _create_urls(_VisitNumberAPI, app_injector=injector.Injector([_VisitModule()])) + + # Act + response = _get(url, '/visit/') + + # Assert + assert response.content == b'1' + assert [visit.number for visit in interceptor.visits] == [1] + + +def test_request_scoped_value_does_not_exist_outside_request(): + app_injector = injector.Injector([_VisitModule()]) + + # Act + with pytest.raises(LookupError) as exception_info: + app_injector.get(_Visit) + + # Assert + assert str(exception_info.value) == '_Visit is request-scoped: it exists only while Winter handles a request' + + +@dataclass +class _NewTodo: + title: str = '' + + +class _TodoAPI: + @winter.web.route_post('todos/') + def create(self, todo: _NewTodo): # pragma: no cover + pass + + +def test_argument_neither_mapped_from_request_nor_bound_fails_startup(): + # Act + with pytest.raises(TypeError) as exception_info: + _create_urls(_TodoAPI) + + # Assert + assert str(exception_info.value) == ( + '_TodoAPI.create: argument todo (_NewTodo) is not mapped from the request and has no binding in the injector' + ) + + +class _ResponseHeaderAPI: + @winter.web.route_get('header/') + def get(self, header: winter.web.ResponseHeader[str]): # pragma: no cover + pass + + +def test_response_header_without_annotation_fails_startup(): + # Act + with pytest.raises(TypeError) as exception_info: + _create_urls(_ResponseHeaderAPI) + + # Assert + assert str(exception_info.value) == ( + '_ResponseHeaderAPI.get: argument header (ResponseHeader[str]) is not mapped from the request ' + 'and has no binding in the injector' + ) + + +class _ExceptionHandlers: + @winter.web.exception_handler + def handle(self, exception: LookupError, todo: _NewTodo): # pragma: no cover + pass + + +def test_exception_handler_argument_neither_mapped_nor_bound_fails_startup(): + # Act + with pytest.raises(TypeError) as exception_info: + _create_urls(_ExceptionHandlers) + + # Assert + assert str(exception_info.value) == ( + '_ExceptionHandlers.handle: argument todo (_NewTodo) is not mapped from the request ' + 'and has no binding in the injector' + ) + + +class _TodoInterceptor(winter.web.Interceptor): + def pre_handle(self, todo: _NewTodo): # pragma: no cover + pass + + +def test_interceptor_argument_neither_mapped_nor_bound_fails_startup(): + with patch.object(interceptor_registry, '_interceptors', [_TodoInterceptor()]): + # Act + with pytest.raises(TypeError) as exception_info: + _create_urls() + + # Assert + assert str(exception_info.value) == ( + '_TodoInterceptor.pre_handle: argument todo (_NewTodo) is not mapped from the request ' + 'and has no binding in the injector' + ) + + +def test_interceptor_added_after_startup_does_not_get_unbound_argument(): + [url] = _create_urls(_VisitNumberAPI, app_injector=injector.Injector([_VisitModule()])) + + with patch.object(interceptor_registry, '_interceptors', [_TodoInterceptor()]): + # Act + response = _get(url, '/visit/') + + # Assert + assert response.status_code == HTTPStatus.INTERNAL_SERVER_ERROR diff --git a/tests/web/test_response_header.py b/tests/web/test_response_header.py index e483c87d..48a21a73 100644 --- a/tests/web/test_response_header.py +++ b/tests/web/test_response_header.py @@ -77,11 +77,3 @@ def test_two_response_headers(api_client): assert response.json() == 'OK' assert response.headers['x-header1'] == 'header1' assert response.headers['x-header2'] == 'header2' - - -def test_header_without_annotation(api_client): - # Act - response = api_client.get('/with-response-headers/header-without-annotation/') - - # Assert - assert response.status_code == 500 # It's better to be replaced with something more human readable diff --git a/winter/application/application.py b/winter/application/application.py index deda81cd..0a20c011 100644 --- a/winter/application/application.py +++ b/winter/application/application.py @@ -10,7 +10,6 @@ from injector import Injector -from .injection import set_injector from .module_discovery import import_recursively Definition = type | types.FunctionType @@ -41,8 +40,6 @@ def __init__( for module in _import_recursively(module_name): for definition in _defined_in(module): self.add(definition) - # The request pipeline still takes the injector from winter.application.get_injector - set_injector(self.injector) @property def definitions(self) -> tuple[Definition, ...]: diff --git a/winter/django/__init__.py b/winter/django/__init__.py index 37b30338..ef195470 100644 --- a/winter/django/__init__.py +++ b/winter/django/__init__.py @@ -1,5 +1,4 @@ from . import json_encoders # noqa: F401 registers the Django lazy string encoder -from .http_request_argument_resolver import HttpRequestArgumentResolver from .output_template import output_template from .request import DjangoRequest from .reverse import reverse diff --git a/winter/django/http_request_argument_resolver.py b/winter/django/http_request_argument_resolver.py deleted file mode 100644 index 98e0fa82..00000000 --- a/winter/django/http_request_argument_resolver.py +++ /dev/null @@ -1,25 +0,0 @@ -import inspect -from typing import MutableMapping - -import django.http - -from winter.components import ComponentMethodArgument -from winter.web import Request -from winter.web.argument_resolver import ArgumentResolver -from .request import DjangoRequest - - -class HttpRequestArgumentResolver(ArgumentResolver): - """Resolves arguments annotated with Django's HttpRequest to the underlying Django request.""" - - def is_supported(self, argument: ComponentMethodArgument) -> bool: - return inspect.isclass(argument.type_) and issubclass(argument.type_, django.http.HttpRequest) - - def resolve_argument( - self, - argument: ComponentMethodArgument, - request: Request, - response_headers: MutableMapping[str, str], - ): - assert isinstance(request, DjangoRequest), 'HttpRequestArgumentResolver works with the Django adapter only' - return request.django_request diff --git a/winter/django/view.py b/winter/django/view.py index d6df0ce5..e5c946d4 100644 --- a/winter/django/view.py +++ b/winter/django/view.py @@ -7,20 +7,20 @@ from django.http.response import HttpResponseBase from django.urls import URLPattern from django.urls import re_path +from injector import Injector import winter.web.initialization from winter.application import Application from winter.web import Response -from winter.web import arguments_resolver 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 +from winter.web.request_handler import check_arguments from winter.web.routing import Route from winter.web.urls import rewrite_uritemplate_with_regexps -from .http_request_argument_resolver import HttpRequestArgumentResolver from .request import DjangoRequest from .throttling import django_user_identity @@ -32,7 +32,11 @@ def create_urls(app: Application) -> List[URLPattern]: """URL patterns of the routes of an application, relative to where they are included in the urlpatterns.""" winter.web.initialization.initialize() _initialize_process() - return _create_urls_from_routes(find_routes(app), find_exception_handlers(app)) + routes = find_routes(app) + exception_handlers = find_exception_handlers(app) + # Each request binds the Django request, see _create_dispatch_function + check_arguments(routes, exception_handlers, app.injector, request_types=[django.http.HttpRequest]) + return _create_urls_from_routes(routes, exception_handlers, app.injector) _process_initialized = False @@ -42,13 +46,16 @@ def _initialize_process(): """Process-global until the registries move into Application: docs/architecture.md, Current deviations.""" global _process_initialized if not _process_initialized: - arguments_resolver.add_argument_resolver(HttpRequestArgumentResolver()) register_native_response_type(HttpResponseBase) set_throttling_identity_resolver(django_user_identity) _process_initialized = True -def _create_urls_from_routes(routes: List[Route], exception_handlers: ExceptionHandlers) -> List[URLPattern]: +def _create_urls_from_routes( + routes: List[Route], + exception_handlers: ExceptionHandlers, + injector: Injector, +) -> List[URLPattern]: django_urls = [] grouped_routes = defaultdict(list) @@ -56,7 +63,7 @@ def _create_urls_from_routes(routes: List[Route], exception_handlers: ExceptionH grouped_routes[route.url_path].append(route) for url_path, routes in grouped_routes.items(): - django_view = _create_django_view_from_routes(routes, exception_handlers).as_view() + django_view = _create_django_view_from_routes(routes, exception_handlers, injector).as_view() winter_url_path = f'^{url_path}$' methods = [route.method for route in routes] django_url_path = rewrite_uritemplate_with_regexps(winter_url_path, methods) @@ -68,7 +75,11 @@ def _create_urls_from_routes(routes: List[Route], exception_handlers: ExceptionH return django_urls -def _create_django_view_from_routes(routes: List[Route], exception_handlers: ExceptionHandlers) -> 'View': +def _create_django_view_from_routes( + routes: List[Route], + exception_handlers: ExceptionHandlers, + injector: Injector, +) -> 'View': from django.views.generic import View class WinterView(View): @@ -77,7 +88,7 @@ def as_view(cls, **initkwargs): return super().as_view(**initkwargs) for route in routes: - dispatch = _create_dispatch_function(route, exception_handlers) + dispatch = _create_dispatch_function(route, exception_handlers, injector) dispatch.route = route dispatch_method_name = route.http_method.lower() setattr(WinterView, dispatch_method_name, dispatch) @@ -85,10 +96,12 @@ def as_view(cls, **initkwargs): return WinterView() -def _create_dispatch_function(route: Route, exception_handlers: ExceptionHandlers): +def _create_dispatch_function(route: Route, exception_handlers: ExceptionHandlers, injector: Injector): @wraps(route.method.func) def dispatch(winter_view, request: django.http.HttpRequest, **path_parameters): - response = handle_request(route, DjangoRequest(request, path_parameters), exception_handlers) + winter_request = DjangoRequest(request, path_parameters) + request_values = {django.http.HttpRequest: request} + response = handle_request(route, winter_request, exception_handlers, injector, request_values) return to_django_response(response) return dispatch diff --git a/winter/openapi/initialization.py b/winter/openapi/initialization.py index 10b612ef..b87879e4 100644 --- a/winter/openapi/initialization.py +++ b/winter/openapi/initialization.py @@ -2,7 +2,7 @@ from winter.web.exceptions import RequestDataDecodeException from winter.web.exceptions import ThrottleException from winter.web.exceptions import UnsupportedMediaTypeException -from winter.web.pagination import PagePositionArgumentResolver +from winter.web.pagination.page_position_argument_resolver import PagePositionArgumentResolver from winter.web.pagination.limits import MaximumLimitValueExceeded from .annotations import register_global_exception from .inspection.inspection import register_type_inspector diff --git a/winter/web/__init__.py b/winter/web/__init__.py index 9f7ab1d1..4a7ce5f8 100644 --- a/winter/web/__init__.py +++ b/winter/web/__init__.py @@ -1,8 +1,5 @@ from . import pagination -from .argument_resolver import ArgumentResolver -from .argument_resolver import ArgumentsResolver -from .argument_resolver import arguments_resolver from .exceptions import exception_handler from .exceptions import problem from .exceptions.raises import raises @@ -16,15 +13,14 @@ from .query_parameters import query_parameters from .request_body_annotation import request_body from .request import Request -from .request_body_resolver import RequestBodyArgumentResolver from .request_handler import handle_request from .request_handler import register_native_response_type +from .request_scope import request_scope from .request_header_annotation import request_header from .response import Response from .response_entity import ResponseEntity from .response_header_annotation import ResponseHeader from .response_header_annotation import response_header -from .response_header_resolver import ResponseHeaderArgumentResolver from .response_header_serializer import response_headers_serializer from .response_status_annotation import response_status from .routing import route diff --git a/winter/web/argument_resolver.py b/winter/web/argument_resolver.py index 397fda2b..dc9f35c2 100644 --- a/winter/web/argument_resolver.py +++ b/winter/web/argument_resolver.py @@ -1,17 +1,20 @@ import abc from abc import abstractmethod from typing import Any +from typing import Collection from typing import Dict -from typing import List from typing import Mapping from typing import MutableMapping from typing import Optional +from typing import Sequence +from injector import Injector from winter.web.request import Request from winter.components import ComponentMethod from winter.components import ComponentMethodArgument from winter.typing import get_type_name +from .request_scope import is_bound class ArgumentNotSupported(Exception): @@ -22,7 +25,7 @@ def __init__(self, argument: ComponentMethodArgument): class ArgumentResolver(abc.ABC): - """ArgumentResolver interface is used to map http request contents to API method arguments.""" + """Maps a part of the request, such as a query parameter or the body, to the arguments that declare it.""" @abstractmethod def is_supported(self, argument: ComponentMethodArgument) -> bool: # pragma: no cover @@ -39,51 +42,68 @@ def resolve_argument( class ArgumentsResolver: + """ + Resolves the arguments of a method: an argument mapped from the request by one of the resolvers is taken + from the request, any other one from the injector of the request, which must bind its type. + """ - def __init__(self): - super().__init__() - self._argument_resolvers: List[ArgumentResolver] = [] - self._cache = {} + def __init__(self, argument_resolvers: Sequence[ArgumentResolver]): + self._argument_resolvers = tuple(argument_resolvers) + self._cache: Dict[ComponentMethodArgument, Optional[ArgumentResolver]] = {} - def add_argument_resolver(self, argument_resolver: ArgumentResolver): - self._argument_resolvers.append(argument_resolver) + def check_arguments( + self, + method: ComponentMethod, + app_injector: Injector, + request_types: Collection[type], + context_names: Collection[str] = (), + ): + """Fails on startup for an argument that a request would not resolve.""" + for argument in method.arguments: + if argument.name in context_names or self._get_argument_resolver(argument) is not None: + continue + if argument.type_ not in request_types and not is_bound(app_injector, argument.type_): + raise _unbound_argument(argument) def resolve_arguments( self, method: ComponentMethod, request: Request, response_headers: MutableMapping[str, str], + request_injector: Injector, context: Optional[Mapping[str, Any]] = None, ) -> Dict[str, Any]: - resolved_arguments = {} if context is None: context = {} - + resolved_arguments = {} for argument in method.arguments: if argument.name in context: resolved_arguments[argument.name] = context[argument.name] + continue + argument_resolver = self._get_argument_resolver(argument) + if argument_resolver is not None: + value = argument_resolver.resolve_argument(argument, request, response_headers) + elif is_bound(request_injector, argument.type_): + value = request_injector.get(argument.type_) else: - resolved_arguments[argument.name] = self._resolve_argument(argument, request, response_headers) - + raise _unbound_argument(argument) + resolved_arguments[argument.name] = value return resolved_arguments - def _resolve_argument( - self, - argument: ComponentMethodArgument, - request: Request, - response_headers: MutableMapping[str, str], - ) -> Any: - argument_resolver = self._get_argument_resolver(argument) - return argument_resolver.resolve_argument(argument, request, response_headers) - - def _get_argument_resolver(self, argument: ComponentMethodArgument) -> 'ArgumentResolver': - if argument in self._cache: - return self._cache[argument] - for argument_resolver in self._argument_resolvers: - if argument_resolver.is_supported(argument): - self._cache[argument] = argument_resolver - return argument_resolver - raise ArgumentNotSupported(argument) - - -arguments_resolver = ArgumentsResolver() + def _get_argument_resolver(self, argument: ComponentMethodArgument) -> Optional[ArgumentResolver]: + if argument not in self._cache: + self._cache[argument] = next( + (resolver for resolver in self._argument_resolvers if resolver.is_supported(argument)), + None, + ) + return self._cache[argument] + + +def _unbound_argument(argument: ComponentMethodArgument) -> TypeError: + # Whatever the injector could create is not enough: a forgotten @request_body would become an empty object + method = argument.method + method_name = method.full_name if method.component is not None else method.func.__qualname__ + return TypeError( + f'{method_name}: argument {argument.name} ({get_type_name(argument.type_)}) ' + f'is not mapped from the request and has no binding in the injector', + ) diff --git a/winter/web/exceptions/handlers.py b/winter/web/exceptions/handlers.py index 95eebafc..c2f3eb7d 100644 --- a/winter/web/exceptions/handlers.py +++ b/winter/web/exceptions/handlers.py @@ -2,6 +2,7 @@ from dataclasses import dataclass from typing import Dict from typing import Iterable +from typing import Iterator from typing import Tuple from typing import Type @@ -35,6 +36,10 @@ def __init__(self, handlers: Dict[Type[Exception], ComponentMethod]): def get(self, exception_cls: Type[Exception]) -> ComponentMethod: return next(self._handlers[cls] for cls in exception_cls.__mro__ if cls in self._handlers) + def __iter__(self) -> Iterator[ComponentMethod]: + """Each handler once, also the one of several exceptions.""" + return iter(dict.fromkeys(self._handlers.values())) + def find_handlers(classes: Iterable[type]) -> Dict[Type[Exception], ComponentMethod]: """The handlers marked with @exception_handler in the classes, by the exception they handle.""" diff --git a/winter/web/initialization.py b/winter/web/initialization.py index cc14f377..236c951b 100644 --- a/winter/web/initialization.py +++ b/winter/web/initialization.py @@ -1,14 +1,5 @@ -from .argument_resolver import arguments_resolver from .output_processor import register_output_processor_resolver -from .pagination.page_position_argument_resolver import PagePositionArgumentResolver from .pagination.page_processor_resolver import PageOutputProcessorResolver -from .path_parameters_argument_resolver import PathParametersArgumentResolver -from .query_parameters.query_parameter_argument_resolver import QueryParameterArgumentResolver -from .query_parameters.query_parameters_argument_resolver import QueryParametersArgumentResolver -from .request_argument_resolver import RequestArgumentResolver -from .request_body_resolver import RequestBodyArgumentResolver -from .request_header_resolver import RequestHeaderArgumentResolver -from .response_header_resolver import ResponseHeaderArgumentResolver from .response_header_serializer import response_headers_serializer from .response_header_serializers import DateTimeResponseHeaderSerializer from .response_header_serializers import LastModifiedResponseHeaderSerializer @@ -18,7 +9,8 @@ def initialize(): """ - Registers the standard resolvers of winter.web on the first call. Adapters call it before building their routes. + Registers the standard output processors and response header serializers of winter.web on the first call. + Adapters call it before building their routes. Process-global until the registries move into Application: docs/architecture.md, Current deviations. """ global _process_initialized @@ -31,11 +23,3 @@ def _initialize_process(): register_output_processor_resolver(PageOutputProcessorResolver()) response_headers_serializer.add_serializer(DateTimeResponseHeaderSerializer()) response_headers_serializer.add_serializer(LastModifiedResponseHeaderSerializer()) - arguments_resolver.add_argument_resolver(QueryParameterArgumentResolver()) - arguments_resolver.add_argument_resolver(QueryParametersArgumentResolver()) - arguments_resolver.add_argument_resolver(PathParametersArgumentResolver()) - arguments_resolver.add_argument_resolver(RequestBodyArgumentResolver()) - arguments_resolver.add_argument_resolver(ResponseHeaderArgumentResolver()) - arguments_resolver.add_argument_resolver(PagePositionArgumentResolver()) - arguments_resolver.add_argument_resolver(RequestHeaderArgumentResolver()) - arguments_resolver.add_argument_resolver(RequestArgumentResolver()) diff --git a/winter/web/pagination/__init__.py b/winter/web/pagination/__init__.py index 4809d346..ae67e8e4 100644 --- a/winter/web/pagination/__init__.py +++ b/winter/web/pagination/__init__.py @@ -1,3 +1,2 @@ from .limits import limits from .order_by import order_by -from .page_position_argument_resolver import PagePositionArgumentResolver diff --git a/winter/web/request_argument_resolver.py b/winter/web/request_argument_resolver.py deleted file mode 100644 index 7a95311e..00000000 --- a/winter/web/request_argument_resolver.py +++ /dev/null @@ -1,20 +0,0 @@ -import inspect -from typing import MutableMapping - -from winter.components import ComponentMethodArgument -from .argument_resolver import ArgumentResolver -from .request import Request - - -class RequestArgumentResolver(ArgumentResolver): - - def is_supported(self, argument: ComponentMethodArgument) -> bool: - return inspect.isclass(argument.type_) and issubclass(argument.type_, Request) - - def resolve_argument( - self, - argument: ComponentMethodArgument, - request: Request, - response_headers: MutableMapping[str, str], - ): - return request diff --git a/winter/web/request_handler.py b/winter/web/request_handler.py index b8b8a52f..9d38192c 100644 --- a/winter/web/request_handler.py +++ b/winter/web/request_handler.py @@ -1,32 +1,84 @@ import json from typing import Any +from typing import Collection +from typing import Iterable +from typing import Mapping + +from injector import Injector -from winter.application import get_injector from winter.components import ComponentMethod from winter.json import JSONEncoder -from .argument_resolver import arguments_resolver +from .argument_resolver import ArgumentsResolver from .default_response_status import get_response_status from .exceptions import ExceptionHandlers from .exceptions import ThrottleException from .exceptions.handlers import exception_argument_name from .interceptor import interceptor_registry from .output_processor import get_output_processor +from .pagination.page_position_argument_resolver import PagePositionArgumentResolver +from .path_parameters_argument_resolver import PathParametersArgumentResolver +from .query_parameters.query_parameter_argument_resolver import QueryParameterArgumentResolver +from .query_parameters.query_parameters_argument_resolver import QueryParametersArgumentResolver from .request import Request +from .request_body_resolver import RequestBodyArgumentResolver +from .request_header_resolver import RequestHeaderArgumentResolver +from .request_scope import create_request_injector from .response import Response from .response_entity import ResponseEntity +from .response_header_resolver import ResponseHeaderArgumentResolver from .response_header_serializer import response_headers_serializer from .routing import Route from .throttling import create_throttle_class - -def handle_request(route: Route, request: Request, exception_handlers: ExceptionHandlers) -> Response: +# The arguments mapped from the request; any other argument comes from the injector of the request +_arguments_resolver = ArgumentsResolver([ + QueryParameterArgumentResolver(), + QueryParametersArgumentResolver(), + PathParametersArgumentResolver(), + RequestBodyArgumentResolver(), + ResponseHeaderArgumentResolver(), + PagePositionArgumentResolver(), + RequestHeaderArgumentResolver(), +]) + + +def check_arguments( + routes: Iterable[Route], + exception_handlers: ExceptionHandlers, + app_injector: Injector, + request_types: Collection[type], +): + """ + 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. + """ + request_types = {Request, *request_types} + for route in routes: + _arguments_resolver.check_arguments(route.method, app_injector, request_types) + for interceptor in interceptor_registry: + pre_handle = ComponentMethod.get_or_create(interceptor.__class__.pre_handle) + _arguments_resolver.check_arguments(pre_handle, app_injector, request_types, {'method'}) + for method in exception_handlers: + _arguments_resolver.check_arguments(method, app_injector, request_types, {exception_argument_name(method)}) + + +def handle_request( + route: Route, + request: Request, + exception_handlers: ExceptionHandlers, + app_injector: Injector, + request_values: Mapping[type, object], +) -> Response: """ Run the whole handling pipeline for a matched route: throttling, interceptors, argument resolution, the API method, the application's exception handlers, output processing 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. """ method = route.method response_headers: dict[str, Any] = {} + injector = create_request_injector(app_injector, request, request_values) throttle = create_throttle_class(route) try: @@ -35,20 +87,21 @@ def handle_request(route: Route, request: Request, exception_handlers: Exception for interceptor in interceptor_registry: pre_handle = ComponentMethod.get_or_create(interceptor.__class__.pre_handle) - arguments = arguments_resolver.resolve_arguments(pre_handle, request, response_headers, {'method': method}) + context = {'method': method} + arguments = _arguments_resolver.resolve_arguments(pre_handle, request, response_headers, injector, context) pre_handle(interceptor, **arguments) - arguments = arguments_resolver.resolve_arguments(method, request, response_headers) - api_instance = get_injector().get(method.component.component_cls) + arguments = _arguments_resolver.resolve_arguments(method, request, response_headers, injector) + api_instance = injector.get(method.component.component_cls) result = method(api_instance, **arguments) except Exception as exception: method = exception_handlers.get(type(exception)) try: - result = _handle_exception(method, exception, request, response_headers) + result = _handle_exception(method, exception, request, response_headers, injector) except Exception as handler_exception: # A failing handler is not retried: its exception goes to the handler of Exception method = exception_handlers.get(Exception) - result = _handle_exception(method, handler_exception, request, response_headers) + result = _handle_exception(method, handler_exception, request, response_headers, injector) return _to_response(request, result, method, response_headers) @@ -58,10 +111,11 @@ def _handle_exception( exception: Exception, request: Request, response_headers: dict[str, Any], + injector: Injector, ) -> Any: - context = {exception_argument_name(method): exception, 'response_headers': response_headers} - arguments = arguments_resolver.resolve_arguments(method, request, response_headers, context) - handler = get_injector().get(method.component.component_cls) + context = {exception_argument_name(method): exception} + arguments = _arguments_resolver.resolve_arguments(method, request, response_headers, injector, context) + handler = injector.get(method.component.component_cls) return method(handler, **arguments) diff --git a/winter/web/request_scope.py b/winter/web/request_scope.py new file mode 100644 index 00000000..47b53aff --- /dev/null +++ b/winter/web/request_scope.py @@ -0,0 +1,62 @@ +from typing import Any +from typing import Mapping + +import injector + +from .request import Request + + +class _RequestValues(dict): + """The values of the request-scoped providers of one request. Bound only in the injector of that request.""" + + +class RequestScope(injector.Scope): + """ + One value per request. The value lives in the injector of the request and goes away with it, so a + request-scoped type exists only while Winter handles a request. + """ + + def get(self, key: type, provider: injector.Provider) -> injector.Provider: + return _RequestScopedProvider(key, provider) + + +request_scope = injector.ScopeDecorator(RequestScope) + + +class _RequestScopedProvider(injector.Provider): + def __init__(self, key: type, provider: injector.Provider): + self._key = key + self._provider = provider + + def get(self, asking_injector: injector.Injector) -> Any: + # The scope belongs to the injector that binds the provider; the values belong to the one asked + if not is_bound(asking_injector, _RequestValues): + raise LookupError(f'{self._key.__name__} is request-scoped: it exists only while Winter handles a request') + values = asking_injector.get(_RequestValues) + if self._key not in values: + values[self._key] = self._provider.get(asking_injector) + return values[self._key] + + +def create_request_injector( + app_injector: injector.Injector, + request: Request, + request_values: Mapping[type, object], +) -> injector.Injector: + """The injector of one request: a child of the application's injector that binds the request.""" + + def configure(binder: injector.Binder): + for type_, value in {Request: request, **request_values, _RequestValues: _RequestValues()}.items(): + binder.bind(type_, to=injector.InstanceProvider(value)) + + return app_injector.create_child_injector([configure]) + + +def is_bound(injector_: injector.Injector, type_: type) -> bool: + """Whether the injector or one of its parents binds the type explicitly, not only can create it.""" + binder = injector_.binder + while binder is not None: + if binder.has_explicit_binding_for(type_): + return True + binder = binder.parent + return False