diff --git a/README.md b/README.md index 8b1dd1c3..be3f76f5 100644 --- a/README.md +++ b/README.md @@ -22,17 +22,17 @@ Package structure and layer rules: [docs/architecture.md](docs/architecture.md). ## Upgrading to Winter 34 -An application is composed explicitly in a `winter.application.Application`, which imports every module of -the given packages on construction (by default, those of the directory of the module that creates it), and -adapters build from it. `find_package_routes`, `create_django_urls_from_routes`, `WinterApplication` and the -`setup()` functions of `winter.web`, `winter.django` and `winter.openapi` are gone. +A web application is composed explicitly in a `winter.web.WebApplication`, which imports every module of the +given packages and their subpackages on construction, and adapters build from it. `find_package_routes`, +`create_django_urls_from_routes`, `WinterApplication` and the `setup()` functions of `winter.web`, `winter.django` +and `winter.openapi` are gone. ```python import service_a import winter.django -from winter.application import Application +import winter.web -app = Application(modules=[service_a], injector=injector) # several applications may share one injector +app = winter.web.WebApplication(modules=[service_a], injector=injector) # applications may share one injector urlpatterns = winter.django.create_urls(app) ``` @@ -64,6 +64,9 @@ urlpatterns = winter.django.create_urls(app) 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. +- Interceptors belong to applications: `WebApplication(..., interceptors=[AuthorizationInterceptor, AuditInterceptor])` + lists their classes in the order they run. The injector of each request creates them, so their dependencies go + to `__init__`. `interceptor_registry` and `InterceptorRegistry` are 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 @@ -125,9 +128,9 @@ To use it with Django, in `urls.py`: ```python import some_package import winter.django -from winter.application import Application +import winter.web -app = Application(modules=[some_package]) +app = winter.web.WebApplication(modules=[some_package]) urlpatterns = [ *winter.django.create_urls(app), ] @@ -316,7 +319,8 @@ class TodoProblemExistsExampleAPI: ## Interceptors You can define interceptors to pre-handle a web request before it gets to an endpoint code. -The pre_handle method arguments will be injected the same way as it's done in methods with winter.web.route annotation. +The pre_handle method arguments are resolved the same way as those of an API method, and `method` receives the API +method of the request. It's not supported to return any response from interceptors. However, the exceptions thrown from within an interceptor will be handled automatically. ```python @@ -336,19 +340,10 @@ 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, before the URL patterns are created: they check -the arguments of the registered interceptors on startup. +An application lists the classes of its interceptors in the order they run. The injector of each request creates +them, so their dependencies go to `__init__`: ```python -from winter.web import interceptor_registry -from .interceptors import HelloWorldInterceptor - -class SomeInitializationClassInMyApplication: - def initialize_my_app(self): - interceptor_registry.add_interceptor(HelloWorldInterceptor()) - +app = winter.web.WebApplication(modules=[some_package], interceptors=[HelloWorldInterceptor]) ``` ## Values from the injector @@ -362,7 +357,6 @@ Django, `django.http.HttpRequest`. import injector import winter.web from django.http import HttpRequest -from winter.application import Application class AccountModule(injector.Module): @@ -382,7 +376,7 @@ class ProfileAPI: return self._profiles.get(account) -app = Application(modules=[profiles], injector=injector.Injector([AccountModule()])) +app = winter.web.WebApplication(modules=[profiles], injector=injector.Injector([AccountModule()])) ``` Interceptors and exception handlers of a request get the same request-scoped values as its API method. diff --git a/docs/architecture.md b/docs/architecture.md index ee845531..5313599a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -57,7 +57,7 @@ independent of each other. Dashed packages are planned and do not exist yet. |---|---|---| | Adapters | Bind the models to external technologies and standards. Independent of each other. | `django`, `openapi`, `sqlalchemy`, `redis`, `wsgi` (planned), `asgi` (planned), `rabbitmq` (planned) | | Models | Framework-agnostic descriptions of what an application does. Independent of each other. | `web` (HTTP), `ddd` (domain: aggregates, repositories, domain events), `messaging` (events) | -| Foundations | Shared vocabulary and mechanisms. Unaware of the layers above. May use each other. | `components` (declarative metadata), `application` (scope, injector, discovery), `json`, `pagination`, `typing` (what the stdlib `typing` lacks) | +| Foundations | Shared vocabulary and mechanisms. Unaware of the layers above. May use each other. | `components` (declarative metadata), `application` (discovery of definitions, injector), `json`, `pagination`, `typing` (what the stdlib `typing` lacks) | A package belongs to Foundations when it knows nothing about models or adapters and is used by at least two of them. @@ -69,8 +69,9 @@ at least two of them. import path changing. 2. **Framework behaviour is never process-global.** Every behavioural registry (exception 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. + belongs to the application object an adapter builds from, such as `winter.web.WebApplication`, + 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,18 @@ at least two of them. ## Current deviations -- **Framework behaviour is still process-global.** An `Application` holds the code of an - 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 +- **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` (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 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 + 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 `initialize` functions then disappear. - **`winter.wsgi`, `winter.asgi` and `winter.rabbitmq` do not exist yet.** HTTP is served through @@ -108,7 +109,8 @@ 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, 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. + 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 + 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. diff --git a/tests/application/caller_directory/app.py b/tests/application/caller_directory/app.py deleted file mode 100644 index fdda5311..00000000 --- a/tests/application/caller_directory/app.py +++ /dev/null @@ -1,13 +0,0 @@ -from winter.application import Application - -print(f'executing {__name__}') - - -class DefinedBeforeApplication: - pass - - -app = Application() - -if __name__ == '__main__': - print(*(definition.__qualname__ for definition in app.definitions)) diff --git a/tests/application/caller_directory/__init__.py b/tests/application/scanned/__init__.py similarity index 100% rename from tests/application/caller_directory/__init__.py rename to tests/application/scanned/__init__.py diff --git a/tests/application/caller_directory/api.py b/tests/application/scanned/api.py similarity index 100% rename from tests/application/caller_directory/api.py rename to tests/application/scanned/api.py diff --git a/tests/application/caller_directory/not_a_package/broken.py b/tests/application/scanned/not_a_package/broken.py similarity index 100% rename from tests/application/caller_directory/not_a_package/broken.py rename to tests/application/scanned/not_a_package/broken.py diff --git a/tests/application/caller_directory/subpackage/__init__.py b/tests/application/scanned/subpackage/__init__.py similarity index 100% rename from tests/application/caller_directory/subpackage/__init__.py rename to tests/application/scanned/subpackage/__init__.py diff --git a/tests/application/caller_directory/subpackage/deep.py b/tests/application/scanned/subpackage/deep.py similarity index 100% rename from tests/application/caller_directory/subpackage/deep.py rename to tests/application/scanned/subpackage/deep.py diff --git a/tests/application/test_application.py b/tests/application/test_application.py deleted file mode 100644 index b3201a05..00000000 --- a/tests/application/test_application.py +++ /dev/null @@ -1,115 +0,0 @@ -import subprocess -import sys -from pathlib import Path - -import pytest -from injector import Injector - -from winter.application import Application -from winter.application import get_injector - - -class Defined: - pass - - -def defined(): # pragma: no cover - pass - - -def test_definitions_are_classes_and_functions_defined_in_scanned_modules(): - import tests.application.caller_directory.api - from tests.application.caller_directory.api import API - from tests.application.caller_directory.api import function - - # Act - app = Application(modules=[tests.application.caller_directory.api]) - - # Assert - assert app.definitions == (API, function) - - -def test_package_is_scanned_with_its_submodules(): - import tests.application.caller_directory.subpackage - from tests.application.caller_directory.subpackage.deep import Deep - - # Act - app = Application(modules=[tests.application.caller_directory.subpackage]) - - # Assert - assert app.definitions == (Deep,) - - -def test_default_modules_are_those_of_caller_directory(): - from tests.application.caller_directory.api import API - from tests.application.caller_directory.api import function - from tests.application.caller_directory.subpackage.deep import Deep - - # Act - from tests.application.caller_directory.app import DefinedBeforeApplication - from tests.application.caller_directory.app import app - - # Assert - assert app.definitions == (API, function, DefinedBeforeApplication, Deep) - - -def test_caller_run_as_main_is_not_executed_twice(): - repository = Path(__file__).parents[2] - - # Act - result = subprocess.run( - [sys.executable, '-m', 'tests.application.caller_directory.app'], - cwd=repository, - capture_output=True, - text=True, - check=True, - ) - - # Assert - assert result.stdout.splitlines() == ['executing __main__', 'API function DefinedBeforeApplication Deep'] - - -def test_unimportable_module_fails_construction(): - import tests.application.unimportable - - # Act - with pytest.raises(RuntimeError, match='Unimportable') as exception_info: - Application(modules=[tests.application.unimportable]) - - # Assert - assert exception_info.value.__notes__ == [ - 'Application imports every module of tests.application.unimportable, so each of them must be importable', - ] - - -def test_add(): - app = Application(modules=[]) - - # Act - app.add(Defined) - app.add(defined) - app.add(Defined) - - # Assert - assert app.definitions == (Defined, defined) - - -def test_new_injector_is_not_installed_globally(): - # Act - app = Application(modules=[]) - - # Assert - assert isinstance(app.injector, Injector) - assert get_injector() is not app.injector - - -def test_given_injector_is_shared(): - injector = Injector() - - # Act - app_1 = Application(modules=[], injector=injector) - app_2 = Application(modules=[], injector=injector) - - # Assert - assert app_1.injector is injector - assert app_2.injector is injector diff --git a/tests/application/test_definitions.py b/tests/application/test_definitions.py new file mode 100644 index 00000000..a86a4e6b --- /dev/null +++ b/tests/application/test_definitions.py @@ -0,0 +1,67 @@ +import pytest + +from winter.application import find_definitions + + +def test_definitions_are_classes_and_functions_defined_in_modules(): + import tests.application.scanned.api + from tests.application.scanned.api import API + from tests.application.scanned.api import function + + # Act + definitions = find_definitions([tests.application.scanned.api]) + + # Assert + assert definitions == (API, function) + + +def test_package_is_scanned_with_its_submodules(): + import tests.application.scanned.subpackage + from tests.application.scanned.subpackage.deep import Deep + + # Act + definitions = find_definitions([tests.application.scanned.subpackage]) + + # Assert + assert definitions == (Deep,) + + +def test_directory_without_init_is_not_scanned(): + import tests.application.scanned + from tests.application.scanned.api import API + from tests.application.scanned.api import function + from tests.application.scanned.subpackage.deep import Deep + + # Act + definitions = find_definitions([tests.application.scanned]) + + # Assert + assert definitions == (API, function, Deep) + + +def test_definition_of_module_given_twice_is_found_once(): + import tests.application.scanned + import tests.application.scanned.subpackage + from tests.application.scanned.api import API + from tests.application.scanned.api import function + from tests.application.scanned.subpackage.deep import Deep + + # Act + definitions = find_definitions([tests.application.scanned, tests.application.scanned.subpackage]) + + # Assert + assert definitions == (API, function, Deep) + + +def test_unimportable_module_fails(): + import tests.application.unimportable + + # Act + with pytest.raises(RuntimeError, match='Unimportable') as exception_info: + find_definitions([tests.application.unimportable]) + + # Assert + assert exception_info.value.__notes__ == [ + 'Finding the definitions imports every module of tests.application.unimportable, ' + 'so each of them must be importable', + ] diff --git a/tests/apps.py b/tests/apps.py index cc2247ac..9f44a987 100644 --- a/tests/apps.py +++ b/tests/apps.py @@ -1,18 +1,13 @@ from django.apps import AppConfig -from tests.web.interceptors import HelloWorldInterceptor import winter.web from winter.web import InMemoryThrottlingStorage -from winter.web import interceptor_registry 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 diff --git a/tests/openapi/test_exception_spec.py b/tests/openapi/test_exception_spec.py index 0068e53e..5d271ec2 100644 --- a/tests/openapi/test_exception_spec.py +++ b/tests/openapi/test_exception_spec.py @@ -5,7 +5,6 @@ import winter.openapi import winter.web -from winter.application import Application from winter.application import get_injector from winter.web.routing import get_route from winter.openapi.generator import generate_openapi_for_routes @@ -106,7 +105,7 @@ class _TestAPI: def get_resource(self): # pragma: no cover pass - app = Application(modules=[], injector=get_injector()) + app = winter.web.WebApplication(modules=[], injector=get_injector()) app.add(_Handlers) app.add(_TestAPI) diff --git a/tests/openapi/test_serving.py b/tests/openapi/test_serving.py index 63c0e14c..732654bf 100644 --- a/tests/openapi/test_serving.py +++ b/tests/openapi/test_serving.py @@ -7,7 +7,6 @@ import winter.django import winter.openapi import winter.web -from winter.application import Application from winter.application import get_injector @@ -26,14 +25,14 @@ def get_orders(self) -> list[int]: # pragma: no cover return [] -def _application(*definitions) -> Application: - app = Application(modules=[], injector=get_injector()) +def _application(*definitions) -> winter.web.WebApplication: + app = winter.web.WebApplication(modules=[], injector=get_injector()) for definition in definitions: app.add(definition) return app -def _get(app: Application, route_name: str, path: str): +def _get(app: winter.web.WebApplication, route_name: str, path: str): [url] = [url for url in winter.django.create_urls(app) if url.name == route_name] return url.callback(RequestFactory().get(path)) diff --git a/tests/openapi/test_validators.py b/tests/openapi/test_validators.py index e56b0174..22501e5b 100644 --- a/tests/openapi/test_validators.py +++ b/tests/openapi/test_validators.py @@ -4,7 +4,6 @@ import winter.openapi import winter.web -from winter.application import Application from winter.application import get_injector from winter.web import problem from winter.web.routing import get_route @@ -16,7 +15,7 @@ def test_validate_missing_raises_annotations_with_missed_raises_and_not_global_e @problem(HTTPStatus.BAD_REQUEST) class MissingException(Exception): pass - app = Application(modules=[], injector=get_injector()) + app = winter.web.WebApplication(modules=[], injector=get_injector()) app.add(MissingException) expected_error_message = f'You are missing declaration for next exceptions: {MissingException.__name__}' @@ -60,7 +59,7 @@ def endpoint(self): # pragma: no cover def not_a_component(): # pragma: no cover pass - app = Application(modules=[], injector=get_injector()) + app = winter.web.WebApplication(modules=[], injector=get_injector()) for definition in [DeclaredException, GlobalException, _TestAPI, not_a_component]: app.add(definition) diff --git a/tests/test_application_scope.py b/tests/test_application_scope.py index e01e0138..1b5759e0 100644 --- a/tests/test_application_scope.py +++ b/tests/test_application_scope.py @@ -1,8 +1,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, and the arguments of an API method come from the injector of the -application that serves it. +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. """ from http import HTTPStatus @@ -11,7 +11,6 @@ import winter.django import winter.web -from winter.application import Application from winter.application import get_injector @@ -27,9 +26,9 @@ def raise_a(self): def test_problem_handler_of_one_application_does_not_handle_exceptions_in_another(): - app_1 = Application(modules=[], injector=get_injector()) + app_1 = winter.web.WebApplication(modules=[], injector=get_injector()) app_1.add(_ErrorA) - app_2 = Application(modules=[], injector=get_injector()) + app_2 = winter.web.WebApplication(modules=[], injector=get_injector()) app_2.add(_API) winter.django.create_urls(app_1) [url] = winter.django.create_urls(app_2) @@ -52,8 +51,8 @@ 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))])) +def _application_with_name(value: str) -> winter.web.WebApplication: + app = winter.web.WebApplication(modules=[], injector=Injector([lambda binder: binder.bind(_Name, to=_Name(value))])) app.add(_NameAPI) return app @@ -69,3 +68,31 @@ def test_argument_of_api_method_comes_from_injector_of_its_application(): # Assert assert response_1.content == b'"one"' assert response_2.content == b'"two"' + + +class _RejectingInterceptor(winter.web.Interceptor): + def pre_handle(self): + raise _ErrorA() + + +class _OKAPI: + @winter.web.route_get('ok/') + def get(self) -> str: + return 'ok' + + +def test_interceptor_of_one_application_does_not_run_in_another(): + app_1 = winter.web.WebApplication(modules=[], interceptors=[_RejectingInterceptor]) + app_1.add(_OKAPI) + app_2 = winter.web.WebApplication(modules=[]) + app_2.add(_OKAPI) + [url_1] = winter.django.create_urls(app_1) + [url_2] = winter.django.create_urls(app_2) + + # Act + response_1 = url_1.callback(RequestFactory().get('/ok/')) + response_2 = url_2.callback(RequestFactory().get('/ok/')) + + # Assert + assert response_1.status_code == HTTPStatus.INTERNAL_SERVER_ERROR + assert response_2.content == b'"ok"' diff --git a/tests/urls.py b/tests/urls.py index 09d16674..5c0d83c2 100644 --- a/tests/urls.py +++ b/tests/urls.py @@ -3,11 +3,11 @@ import tests.api import winter.django import winter.web -from winter.application import Application from winter.application import get_injector from winter.ddd import NotFoundException +from tests.web.interceptors import HelloWorldInterceptor -app = Application(modules=[tests.api], injector=get_injector()) +app = winter.web.WebApplication(modules=[tests.api], injector=get_injector(), interceptors=[HelloWorldInterceptor]) # Mapping a domain exception to an HTTP status is application wiring, not framework behaviour app.add(winter.web.problem(status=HTTPStatus.NOT_FOUND)(NotFoundException)) urlpatterns = winter.django.create_urls(app) diff --git a/tests/web/test_autodiscovery.py b/tests/web/test_autodiscovery.py index 041c7251..f422146f 100644 --- a/tests/web/test_autodiscovery.py +++ b/tests/web/test_autodiscovery.py @@ -1,4 +1,4 @@ -from winter.application import Application +import winter.web from winter.application import get_injector from winter.web.autodiscovery import find_routes @@ -6,7 +6,7 @@ def test_find_routes(): import tests.api.package - app = Application(modules=[tests.api.package], injector=get_injector()) + app = winter.web.WebApplication(modules=[tests.api.package], injector=get_injector()) # Act routes = find_routes(app) diff --git a/tests/web/test_exception_handlers.py b/tests/web/test_exception_handlers.py index 663f2074..aa3f152a 100644 --- a/tests/web/test_exception_handlers.py +++ b/tests/web/test_exception_handlers.py @@ -5,7 +5,6 @@ import winter.django import winter.web -from winter.application import Application from winter.application import get_injector from winter.web.exceptions import RedirectException @@ -36,7 +35,7 @@ def raise_(self): def _get(*definitions): - app = Application(modules=[], injector=get_injector()) + app = winter.web.WebApplication(modules=[], injector=get_injector()) for definition in definitions: app.add(definition) [url] = winter.django.create_urls(app) diff --git a/tests/web/test_interceptors.py b/tests/web/test_interceptors.py index d5fd19bd..8f4948fd 100644 --- a/tests/web/test_interceptors.py +++ b/tests/web/test_interceptors.py @@ -1,4 +1,9 @@ +import injector import pytest +from django.test import RequestFactory + +import winter.django +import winter.web @pytest.mark.parametrize( @@ -13,3 +18,47 @@ def test_interceptor_headers(api_client, hello_world_query, hello_world_header): response = api_client.get(url) assert response.headers.get('x-method') == 'SimpleAPI.get' assert response.headers.get('x-hello-world') == hello_world_header + + +class _Calls(list): + pass + + +class _RecordingInterceptor(winter.web.Interceptor): + @injector.inject + def __init__(self, calls: _Calls): + self._calls = calls + + def pre_handle(self): + self._calls.append(type(self).__name__) + + +class _InterceptorA(_RecordingInterceptor): + pass + + +class _InterceptorB(_RecordingInterceptor): + pass + + +class _CallsAPI: + @winter.web.route_get('calls/') + def get(self, calls: _Calls): + calls.append('API') + + +def test_interceptors_run_in_given_order_before_api_method(): + calls = _Calls() + app = winter.web.WebApplication( + modules=[], + injector=injector.Injector([lambda binder: binder.bind(_Calls, to=calls)]), + interceptors=[_InterceptorB, _InterceptorA], + ) + app.add(_CallsAPI) + [url] = winter.django.create_urls(app) + + # Act + url.callback(RequestFactory().get('/calls/')) + + # Assert + assert calls == ['_InterceptorB', '_InterceptorA', 'API'] diff --git a/tests/web/test_request_scope.py b/tests/web/test_request_scope.py index 65f4ffe1..a89922d4 100644 --- a/tests/web/test_request_scope.py +++ b/tests/web/test_request_scope.py @@ -1,17 +1,13 @@ 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: @@ -37,8 +33,8 @@ def get(self, visit: _Visit, same_visit: _Visit, request: winter.web.Request) -> 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()) +def _create_urls(*definitions, app_injector=None, interceptors=()): + app = winter.web.WebApplication(modules=[], injector=app_injector or injector.Injector(), interceptors=interceptors) for definition in definitions: app.add(definition) return winter.django.create_urls(app) @@ -60,12 +56,17 @@ def test_request_scoped_value_is_created_once_per_request(): assert json.loads(second_response.content) == {'number': 2, 'path': '/visit/', 'same': True, 'method': 'GET'} +class _SeenVisits(list): + pass + + class _VisitInterceptor(winter.web.Interceptor): - def __init__(self): - self.visits = [] + @injector.inject + def __init__(self, seen_visits: _SeenVisits): + self._seen_visits = seen_visits def pre_handle(self, visit: _Visit): - self.visits.append(visit) + self._seen_visits.append(visit) class _VisitNumberAPI: @@ -75,16 +76,16 @@ def get(self, visit: _Visit) -> int: 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()])) + seen_visits = _SeenVisits() + app_injector = injector.Injector([_VisitModule(), lambda binder: binder.bind(_SeenVisits, to=seen_visits)]) + [url] = _create_urls(_VisitNumberAPI, app_injector=app_injector, interceptors=[_VisitInterceptor]) - # Act - response = _get(url, '/visit/') + # Act + response = _get(url, '/visit/') # Assert assert response.content == b'1' - assert [visit.number for visit in interceptor.visits] == [1] + assert [visit.number for visit in seen_visits] == [1] def test_request_scoped_value_does_not_exist_outside_request(): @@ -162,24 +163,12 @@ def pre_handle(self, todo: _NewTodo): # pragma: no cover 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() + # Act + with pytest.raises(TypeError) as exception_info: + _create_urls(interceptors=[_TodoInterceptor]) # 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_web_application.py b/tests/web/test_web_application.py new file mode 100644 index 00000000..8a193c02 --- /dev/null +++ b/tests/web/test_web_application.py @@ -0,0 +1,76 @@ +import pytest +from injector import Injector + +import winter.web +from winter.application import get_injector + + +class _Defined: + pass + + +def _defined(): # pragma: no cover + pass + + +def test_definitions_are_those_of_modules(): + import tests.application.scanned.api + from tests.application.scanned.api import API + from tests.application.scanned.api import function + + # Act + app = winter.web.WebApplication(modules=[tests.application.scanned.api]) + + # Assert + assert app.definitions == (API, function) + + +def test_add(): + app = winter.web.WebApplication(modules=[]) + + # Act + app.add(_Defined) + app.add(_defined) + app.add(_Defined) + + # Assert + assert app.definitions == (_Defined, _defined) + + +def test_new_injector_is_not_installed_globally(): + # Act + app = winter.web.WebApplication(modules=[]) + + # Assert + assert isinstance(app.injector, Injector) + assert get_injector() is not app.injector + + +def test_given_injector_is_shared(): + injector = Injector() + + # Act + app_1 = winter.web.WebApplication(modules=[], injector=injector) + app_2 = winter.web.WebApplication(modules=[], injector=injector) + + # Assert + assert app_1.injector is injector + assert app_2.injector is injector + + +class _Interceptor(winter.web.Interceptor): + def pre_handle(self): # pragma: no cover + pass + + +@pytest.mark.parametrize('interceptor, expected_message', [ + (_Interceptor(), 'Interceptors are classes created by the injector: _Interceptor()'), + (_Defined, '_Defined is not an Interceptor'), +]) +def test_interceptor_must_be_interceptor_class(interceptor, expected_message): + # Act + with pytest.raises(TypeError) as exception_info: + winter.web.WebApplication(modules=[], interceptors=[interceptor]) + + # Assert + assert str(exception_info.value) == expected_message diff --git a/winter/application/__init__.py b/winter/application/__init__.py index e15d1804..c004ca43 100644 --- a/winter/application/__init__.py +++ b/winter/application/__init__.py @@ -1,3 +1,4 @@ -from .application import Application +from .definitions import Definition +from .definitions import find_definitions from .injection import get_injector from .injection import set_injector diff --git a/winter/application/application.py b/winter/application/application.py deleted file mode 100644 index 0a20c011..00000000 --- a/winter/application/application.py +++ /dev/null @@ -1,76 +0,0 @@ -import importlib -import inspect -import os -import pkgutil -import sys -import types -from collections.abc import Sequence -from typing import Any -from typing import Literal - -from injector import Injector - -from .module_discovery import import_recursively - -Definition = type | types.FunctionType - - -class Application: - """ - The code of an application and the injector it runs with, in one object that adapters build from. - - The modules are scanned on construction: each one is imported together with its submodules, and the - classes and functions defined in them become the application's definitions. A directory without - `__init__.py` is not a package and is not scanned. By default, the modules are those of the directory - of the module that creates the Application. - """ - - def __init__( - self, - modules: Sequence[types.ModuleType] | Literal['caller_directory'] = 'caller_directory', - injector: Injector | Literal['new'] = 'new', - ): - self.injector = Injector() if injector == 'new' else injector - if modules == 'caller_directory': - module_names = _caller_directory(sys._getframe(1).f_globals) - else: - module_names = [module.__name__ for module in modules] - self._definitions: dict[Definition, None] = {} - for module_name in module_names: - for module in _import_recursively(module_name): - for definition in _defined_in(module): - self.add(definition) - - @property - def definitions(self) -> tuple[Definition, ...]: - """Classes and functions of the application, in the order they were scanned or added.""" - return tuple(self._definitions) - - def add(self, definition: Definition) -> None: - """Adds a class or a function that is not defined in the scanned modules, such as a generated one.""" - self._definitions.setdefault(definition, None) - - -def _caller_directory(caller_globals: dict[str, Any]) -> list[str]: - path = caller_globals['__file__'] - package = caller_globals['__package__'] - prefix = f'{package}.' if package else '' - caller_name = prefix + inspect.getmodulename(path) - module_names = [prefix + module.name for module in pkgutil.iter_modules([os.path.dirname(path)])] - # Run with `python -m`, the caller is __main__: importing it by its own name would execute it a second time - return [caller_globals['__name__'] if name == caller_name else name for name in module_names] - - -def _import_recursively(module_name: str) -> list[types.ModuleType]: - try: - module = importlib.import_module(module_name) - return [module, *import_recursively(module_name)] if hasattr(module, '__path__') else [module] - except Exception as error: - error.add_note(f'Application imports every module of {module_name}, so each of them must be importable') - raise - - -def _defined_in(module: types.ModuleType): - for value in vars(module).values(): - if (inspect.isclass(value) or inspect.isfunction(value)) and value.__module__ == module.__name__: - yield value diff --git a/winter/application/definitions.py b/winter/application/definitions.py new file mode 100644 index 00000000..e8b725bd --- /dev/null +++ b/winter/application/definitions.py @@ -0,0 +1,38 @@ +import importlib +import inspect +import types +from collections.abc import Sequence + +from .module_discovery import import_recursively + +Definition = type | types.FunctionType + + +def find_definitions(modules: Sequence[types.ModuleType]) -> tuple[Definition, ...]: + """ + The classes and functions defined in the modules, in the order they are scanned. Each module is imported together + with its submodules. A directory without `__init__.py` is not a package and is not scanned. + """ + definitions: dict[Definition, None] = {} + for module in modules: + for scanned_module in _import_recursively(module.__name__): + for definition in _defined_in(scanned_module): + definitions.setdefault(definition, None) + return tuple(definitions) + + +def _import_recursively(module_name: str) -> list[types.ModuleType]: + try: + module = importlib.import_module(module_name) + return [module, *import_recursively(module_name)] if hasattr(module, '__path__') else [module] + except Exception as error: + error.add_note( + f'Finding the definitions imports every module of {module_name}, so each of them must be importable', + ) + raise + + +def _defined_in(module: types.ModuleType): + for value in vars(module).values(): + if (inspect.isclass(value) or inspect.isfunction(value)) and value.__module__ == module.__name__: + yield value diff --git a/winter/django/view.py b/winter/django/view.py index e5c946d4..f80aeea0 100644 --- a/winter/django/view.py +++ b/winter/django/view.py @@ -7,11 +7,10 @@ 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 WebApplication from winter.web import handle_request from winter.web import register_native_response_type from winter.web import set_throttling_identity_resolver @@ -28,22 +27,22 @@ from django.views.generic import View -def create_urls(app: Application) -> List[URLPattern]: +def create_urls(app: WebApplication) -> 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() 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) + check_arguments(app, routes, exception_handlers, request_types=[django.http.HttpRequest]) + return _create_urls_from_routes(app, routes, exception_handlers) _process_initialized = False def _initialize_process(): - """Process-global until the registries move into Application: docs/architecture.md, Current deviations.""" + """Process-global until these registries belong to an application: docs/architecture.md, Current deviations.""" global _process_initialized if not _process_initialized: register_native_response_type(HttpResponseBase) @@ -52,9 +51,9 @@ def _initialize_process(): def _create_urls_from_routes( + app: WebApplication, routes: List[Route], exception_handlers: ExceptionHandlers, - injector: Injector, ) -> List[URLPattern]: django_urls = [] grouped_routes = defaultdict(list) @@ -63,7 +62,7 @@ def _create_urls_from_routes( 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, injector).as_view() + django_view = _create_django_view_from_routes(app, routes, exception_handlers).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) @@ -76,9 +75,9 @@ def _create_urls_from_routes( def _create_django_view_from_routes( + app: WebApplication, routes: List[Route], exception_handlers: ExceptionHandlers, - injector: Injector, ) -> 'View': from django.views.generic import View @@ -88,7 +87,7 @@ def as_view(cls, **initkwargs): return super().as_view(**initkwargs) for route in routes: - dispatch = _create_dispatch_function(route, exception_handlers, injector) + dispatch = _create_dispatch_function(app, route, exception_handlers) dispatch.route = route dispatch_method_name = route.http_method.lower() setattr(WinterView, dispatch_method_name, dispatch) @@ -96,12 +95,12 @@ def as_view(cls, **initkwargs): return WinterView() -def _create_dispatch_function(route: Route, exception_handlers: ExceptionHandlers, injector: Injector): +def _create_dispatch_function(app: WebApplication, route: Route, exception_handlers: ExceptionHandlers): @wraps(route.method.func) 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(route, winter_request, exception_handlers, injector, request_values) + response = handle_request(app, route, winter_request, exception_handlers, request_values) return to_django_response(response) return dispatch diff --git a/winter/openapi/generator.py b/winter/openapi/generator.py index dde31fb6..52f1093f 100644 --- a/winter/openapi/generator.py +++ b/winter/openapi/generator.py @@ -29,9 +29,9 @@ from openapi_pydantic.v3.v3_0 import Server from openapi_pydantic.v3.v3_0 import Tag -from winter.application import Application from winter.components import ComponentMethod from winter.web import MediaType +from winter.web import WebApplication from winter.web.default_response_status import get_response_status from winter.web.exception_handlers import built_in_exception_handlers from winter.web.exception_handlers import find_exception_handlers @@ -141,7 +141,7 @@ def build_schema(self, type_info: TypeInfo, *, output: bool) -> Schema: def generate_openapi( - app: Application, + app: WebApplication, *, title: str, version: str, diff --git a/winter/openapi/initialization.py b/winter/openapi/initialization.py index b87879e4..0557ecb5 100644 --- a/winter/openapi/initialization.py +++ b/winter/openapi/initialization.py @@ -18,7 +18,7 @@ def initialize(): """ Registers the standard inspectors and global exceptions once per process, before the first spec is generated. - Process-global until the registries move into Application: docs/architecture.md, Current deviations. + Process-global until these registries belong to an application: docs/architecture.md, Current deviations. """ global _initialized if _initialized: diff --git a/winter/openapi/serving.py b/winter/openapi/serving.py index 695d7e8a..c93b2350 100644 --- a/winter/openapi/serving.py +++ b/winter/openapi/serving.py @@ -5,11 +5,11 @@ from typing import Dict import winter.web -from winter.application import Application +from winter.web import WebApplication from .swagger_ui import get_swagger_ui_html -def serve_spec(app: Application, spec: Dict[str, Any], *, url: str): +def serve_spec(app: WebApplication, spec: Dict[str, Any], *, url: str): """ Adds a route that serves the spec to the application. `url` is relative to the application root, like the url of any route. The servers of the spec are prefixed with the path the application is mounted at, so the spec stays @@ -31,7 +31,7 @@ def get(self, request: winter.web.Request) -> winter.web.Response: app.add(OpenAPISpec) -def serve_swagger_ui(app: Application, *, spec_url: str, url: str): +def serve_swagger_ui(app: WebApplication, *, spec_url: str, url: str): """ Adds a route with Swagger UI to the application, for the spec served at `spec_url`. Both urls are relative to the application root. diff --git a/winter/openapi/validators.py b/winter/openapi/validators.py index 8c168aed..6a22817b 100644 --- a/winter/openapi/validators.py +++ b/winter/openapi/validators.py @@ -3,11 +3,11 @@ from typing import Type -from winter.application import Application +from winter.web import WebApplication from winter.openapi.annotations import GlobalExceptionAnnotation -def validate_missing_raises_annotations(app: Application): +def validate_missing_raises_annotations(app: WebApplication): """Every @problem exception of the application is declared with @raises by one of its API methods or is global.""" from winter.web.exceptions.problem_annotation import ProblemAnnotation from winter.web.exceptions.raises import ExceptionAnnotation diff --git a/winter/web/__init__.py b/winter/web/__init__.py index 4a7ce5f8..3af32435 100644 --- a/winter/web/__init__.py +++ b/winter/web/__init__.py @@ -4,8 +4,6 @@ from .exceptions import problem from .exceptions.raises import raises from .interceptor import Interceptor -from .interceptor import InterceptorRegistry -from .interceptor import interceptor_registry from .media_type import InvalidMediaTypeException from .media_type import MediaType from .output_processor import register_output_processor_resolver @@ -36,3 +34,4 @@ from .throttling import set_throttling_storage from .throttling import throttling from .urls import register_url_regexp +from .web_application import WebApplication diff --git a/winter/web/autodiscovery.py b/winter/web/autodiscovery.py index df161545..e287af66 100644 --- a/winter/web/autodiscovery.py +++ b/winter/web/autodiscovery.py @@ -1,12 +1,12 @@ from typing import List -from winter.application import Application from winter.components import Component from .routing import Route from .routing import get_route +from .web_application import WebApplication -def find_routes(app: Application) -> List[Route]: +def find_routes(app: WebApplication) -> List[Route]: """Routes of the API classes among the definitions of an application.""" components = Component.get_all() return [ diff --git a/winter/web/exception_handlers.py b/winter/web/exception_handlers.py index ee0f2268..abe791ba 100644 --- a/winter/web/exception_handlers.py +++ b/winter/web/exception_handlers.py @@ -3,7 +3,6 @@ from typing import Iterable from typing import Type -from winter.application import Application from winter.components import Component from .exceptions import RedirectException from .exceptions import RequestDataDecodeException @@ -21,6 +20,7 @@ from .response_header_annotation import ResponseHeader from .response_header_annotation import response_header from .response_status_annotation import response_status +from .web_application import WebApplication class BuiltInExceptionHandlers: @@ -47,7 +47,7 @@ def handle_redirect(self, exception: RedirectException, location_header: Respons _problem_handler_generator = ProblemExceptionHandlerGenerator(ProblemExceptionMapper()) -def find_exception_handlers(app: Application) -> ExceptionHandlers: +def find_exception_handlers(app: WebApplication) -> ExceptionHandlers: """ The exception handlers of an application: the @exception_handler methods and the handlers of the @problem exceptions among its definitions, over the built-in ones. A handler of the application replaces the built-in diff --git a/winter/web/initialization.py b/winter/web/initialization.py index 236c951b..82e4c142 100644 --- a/winter/web/initialization.py +++ b/winter/web/initialization.py @@ -11,7 +11,7 @@ def initialize(): """ 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. + Process-global until these registries belong to an application: docs/architecture.md, Current deviations. """ global _process_initialized if not _process_initialized: diff --git a/winter/web/interceptor.py b/winter/web/interceptor.py index aa448624..eace1047 100644 --- a/winter/web/interceptor.py +++ b/winter/web/interceptor.py @@ -3,20 +3,11 @@ class Interceptor(ABC): + """ + Runs before the API method in each request of the web applications that list it. The arguments of pre_handle are + resolved like those of an API method, and `method` receives the API method of the request. + """ + @abstractmethod def pre_handle(self, **kwargs): pass - - -class InterceptorRegistry: - def __init__(self): - self._interceptors = [] - - def add_interceptor(self, interceptor: Interceptor): - self._interceptors.append(interceptor) - - def __iter__(self): - return iter(self._interceptors) - - -interceptor_registry = InterceptorRegistry() diff --git a/winter/web/request_handler.py b/winter/web/request_handler.py index 9d38192c..c06d7ba0 100644 --- a/winter/web/request_handler.py +++ b/winter/web/request_handler.py @@ -13,7 +13,6 @@ 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 @@ -29,6 +28,7 @@ from .response_header_serializer import response_headers_serializer from .routing import Route from .throttling import create_throttle_class +from .web_application import WebApplication # The arguments mapped from the request; any other argument comes from the injector of the request _arguments_resolver = ArgumentsResolver([ @@ -43,9 +43,9 @@ def check_arguments( + app: WebApplication, routes: Iterable[Route], exception_handlers: ExceptionHandlers, - app_injector: Injector, request_types: Collection[type], ): """ @@ -54,42 +54,42 @@ def check_arguments( """ 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'}) + _arguments_resolver.check_arguments(route.method, app.injector, request_types) + 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'}) for method in exception_handlers: - _arguments_resolver.check_arguments(method, app_injector, request_types, {exception_argument_name(method)}) + _arguments_resolver.check_arguments(method, app.injector, request_types, {exception_argument_name(method)}) def handle_request( + app: WebApplication, 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. + Run the whole handling pipeline for a matched route of the application: throttling, its interceptors, argument + resolution, the API method, its 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) + injector = create_request_injector(app.injector, request, request_values) throttle = create_throttle_class(route) try: if throttle and not throttle.allow_request(request): raise ThrottleException() - for interceptor in interceptor_registry: - pre_handle = ComponentMethod.get_or_create(interceptor.__class__.pre_handle) + for interceptor_cls in app.interceptors: + pre_handle = ComponentMethod.get_or_create(interceptor_cls.pre_handle) context = {'method': method} arguments = _arguments_resolver.resolve_arguments(pre_handle, request, response_headers, injector, context) - pre_handle(interceptor, **arguments) + pre_handle(injector.get(interceptor_cls), **arguments) arguments = _arguments_resolver.resolve_arguments(method, request, response_headers, injector) api_instance = injector.get(method.component.component_cls) diff --git a/winter/web/web_application.py b/winter/web/web_application.py new file mode 100644 index 00000000..16d1e646 --- /dev/null +++ b/winter/web/web_application.py @@ -0,0 +1,46 @@ +import inspect +from collections.abc import Sequence +from types import ModuleType +from typing import Literal + +from injector import Injector + +from winter.application import Definition +from winter.application import find_definitions +from .interceptor import Interceptor + + +class WebApplication: + """ + The code of a web application, the injector it runs with and the interceptors of its requests, in one object that + adapters build from. The classes and functions defined in the modules are its definitions: the API classes, the + exception handlers and the @problem exceptions among them belong to it. + """ + + def __init__( + self, + modules: Sequence[ModuleType], + injector: Injector | Literal['new'] = 'new', + interceptors: Sequence[type[Interceptor]] = (), + ): + """The interceptors run in the given order and are created by the injector of each request.""" + for interceptor in interceptors: + if not inspect.isclass(interceptor): + raise TypeError(f'Interceptors are classes created by the injector: {type(interceptor).__qualname__}()') + if not issubclass(interceptor, Interceptor): + raise TypeError(f'{interceptor.__qualname__} is not an Interceptor') + self.injector = Injector() if injector == 'new' else injector + self.interceptors = tuple(interceptors) + self._definitions = dict.fromkeys(find_definitions(modules)) + + @property + def definitions(self) -> tuple[Definition, ...]: + """Classes and functions of the application, in the order they were scanned or added.""" + return tuple(self._definitions) + + def add(self, definition: Definition) -> None: + """ + Adds a class or a function that is not defined in the modules, such as a generated one. Adapters take the + definitions when they build from the application, so one added afterwards is not served. + """ + self._definitions.setdefault(definition, None)