From f403d89dac006ffaa1ee8c4a30c13aa6500f3b0f Mon Sep 17 00:00:00 2001 From: "alex.oleshkevich" Date: Fri, 30 Jun 2023 11:55:06 +0200 Subject: [PATCH 01/10] remove intemediate code --- starlette/templating.py | 67 ------------------- tests/test_templates.py | 141 ---------------------------------------- 2 files changed, 208 deletions(-) diff --git a/starlette/templating.py b/starlette/templating.py index 4c409fc39..a96506608 100644 --- a/starlette/templating.py +++ b/starlette/templating.py @@ -140,7 +140,6 @@ def url_for(context: dict, __name: str, **path_params: typing.Any) -> URL: def get_template(self, name: str) -> "jinja2.Template": return self.env.get_template(name) - @typing.overload def TemplateResponse( self, request: Request, @@ -151,72 +150,6 @@ def TemplateResponse( media_type: typing.Optional[str] = None, background: typing.Optional[BackgroundTask] = None, ) -> _TemplateResponse: - ... - - @typing.overload - def TemplateResponse( - self, - name: str, - context: typing.Optional[dict] = None, - status_code: int = 200, - headers: typing.Optional[typing.Mapping[str, str]] = None, - media_type: typing.Optional[str] = None, - background: typing.Optional[BackgroundTask] = None, - ) -> _TemplateResponse: - # Deprecated usage - ... - - def TemplateResponse( - self, *args: typing.Any, **kwargs: typing.Any - ) -> _TemplateResponse: - if args: - if isinstance( - args[0], str - ): # the first argument is template name (old style) - warnings.warn( - "Argument 1 of TemplateResponse must be a Request instance.", - DeprecationWarning, - ) - - name = args[0] - context = args[1] if len(args) > 1 else kwargs.get("context", {}) - status_code = ( - args[2] if len(args) > 2 else kwargs.get("status_code", 200) - ) - headers = args[2] if len(args) > 2 else kwargs.get("headers") - media_type = args[3] if len(args) > 3 else kwargs.get("media_type") - background = args[4] if len(args) > 4 else kwargs.get("background") - - if "request" not in context: - raise ValueError('context must include a "request" key') - request = context["request"] - else: # the first argument is a request instance (new style) - request = args[0] - name = args[1] if len(args) > 1 else kwargs["name"] - context = args[2] if len(args) > 2 else kwargs.get("context") - status_code = ( - args[3] if len(args) > 3 else kwargs.get("status_code", 200) - ) - headers = args[4] if len(args) > 4 else kwargs.get("headers") - media_type = args[5] if len(args) > 5 else kwargs.get("media_type") - background = args[6] if len(args) > 6 else kwargs.get("background") - else: # all arguments are kwargs - if "request" not in kwargs: - warnings.warn( - "TemplateResponse requires `request` keyword argument.", - DeprecationWarning, - ) - if "request" not in kwargs.get("context", {}): - raise ValueError('context must include a "request" key') - - context = kwargs.get("context", {}) - request = kwargs.get("request", context.get("request")) - name = typing.cast(str, kwargs["name"]) - status_code = kwargs.get("status_code", 200) - headers = kwargs.get("headers") - media_type = kwargs.get("media_type") - background = kwargs.get("background") - context = context or {} context.setdefault("request", request) for context_processor in self.context_processors: diff --git a/tests/test_templates.py b/tests/test_templates.py index db0b2bb63..566718d18 100644 --- a/tests/test_templates.py +++ b/tests/test_templates.py @@ -1,12 +1,10 @@ import os from pathlib import Path -from unittest import mock import jinja2 import pytest from starlette.applications import Starlette -from starlette.background import BackgroundTask from starlette.middleware import Middleware from starlette.middleware.base import BaseHTTPMiddleware from starlette.routing import Route @@ -154,142 +152,3 @@ def test_templates_with_environment(tmpdir): def test_templates_with_environment_options_emit_warning(tmpdir): with pytest.warns(DeprecationWarning): Jinja2Templates(str(tmpdir), autoescape=True) - - -def test_templates_with_kwargs_only(tmpdir, test_client_factory): - # MAINTAINERS: remove after 1.0 - path = os.path.join(tmpdir, "index.html") - with open(path, "w") as file: - file.write("value: {{ a }}") - templates = Jinja2Templates(directory=str(tmpdir)) - - spy = mock.MagicMock() - - def page(request): - return templates.TemplateResponse( - request=request, - name="index.html", - context={"a": "b"}, - status_code=201, - headers={"x-key": "value"}, - media_type="text/plain", - background=BackgroundTask(func=spy), - ) - - app = Starlette(routes=[Route("/", page)]) - client = test_client_factory(app) - response = client.get("/") - - assert response.text == "value: b" # context was rendered - assert response.status_code == 201 - assert response.headers["x-key"] == "value" - assert response.headers["content-type"] == "text/plain; charset=utf-8" - spy.assert_called() - - -def test_templates_with_kwargs_only_requires_request_in_context(tmpdir): - # MAINTAINERS: remove after 1.0 - - templates = Jinja2Templates(directory=str(tmpdir)) - with pytest.warns( - DeprecationWarning, - match="TemplateResponse requires `request` keyword argument.", - ): - with pytest.raises(ValueError): - templates.TemplateResponse(name="index.html", context={"a": "b"}) - - -def test_templates_with_kwargs_only_warns_when_no_request_keyword( - tmpdir, test_client_factory -): - # MAINTAINERS: remove after 1.0 - - path = os.path.join(tmpdir, "index.html") - with open(path, "w") as file: - file.write("Hello") - - templates = Jinja2Templates(directory=str(tmpdir)) - - def page(request): - return templates.TemplateResponse( - name="index.html", context={"request": request} - ) - - app = Starlette(routes=[Route("/", page)]) - client = test_client_factory(app) - - with pytest.warns( - DeprecationWarning, - match="TemplateResponse requires `request` keyword argument.", - ): - client.get("/") - - -def test_templates_with_requires_request_in_context(tmpdir): - # MAINTAINERS: remove after 1.0 - templates = Jinja2Templates(directory=str(tmpdir)) - with pytest.warns(DeprecationWarning): - with pytest.raises(ValueError): - templates.TemplateResponse("index.html", context={}) - - -def test_templates_warns_when_first_argument_isnot_request(tmpdir, test_client_factory): - # MAINTAINERS: remove after 1.0 - path = os.path.join(tmpdir, "index.html") - with open(path, "w") as file: - file.write("value: {{ a }}") - templates = Jinja2Templates(directory=str(tmpdir)) - - spy = mock.MagicMock() - - def page(request): - return templates.TemplateResponse( - "index.html", - {"a": "b", "request": request}, - status_code=201, - headers={"x-key": "value"}, - media_type="text/plain", - background=BackgroundTask(func=spy), - ) - - app = Starlette(routes=[Route("/", page)]) - client = test_client_factory(app) - with pytest.warns(DeprecationWarning): - response = client.get("/") - - assert response.text == "value: b" # context was rendered - assert response.status_code == 201 - assert response.headers["x-key"] == "value" - assert response.headers["content-type"] == "text/plain; charset=utf-8" - spy.assert_called() - - -def test_templates_when_first_argument_is_request(tmpdir, test_client_factory): - # MAINTAINERS: remove after 1.0 - path = os.path.join(tmpdir, "index.html") - with open(path, "w") as file: - file.write("value: {{ a }}") - templates = Jinja2Templates(directory=str(tmpdir)) - - spy = mock.MagicMock() - - def page(request): - return templates.TemplateResponse( - request, - "index.html", - {"a": "b"}, - status_code=201, - headers={"x-key": "value"}, - media_type="text/plain", - background=BackgroundTask(func=spy), - ) - - app = Starlette(routes=[Route("/", page)]) - client = test_client_factory(app) - response = client.get("/") - - assert response.text == "value: b" # context was rendered - assert response.status_code == 201 - assert response.headers["x-key"] == "value" - assert response.headers["content-type"] == "text/plain; charset=utf-8" - spy.assert_called() From 67008c4acf1f15f6365360e510da709a17fdec8b Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 1 Jul 2023 23:33:14 +0200 Subject: [PATCH 02/10] Bump coverage from 7.2.5 to 7.2.7 (#2204) Bumps [coverage](https://github.com/nedbat/coveragepy) from 7.2.5 to 7.2.7. - [Release notes](https://github.com/nedbat/coveragepy/releases) - [Changelog](https://github.com/nedbat/coveragepy/blob/master/CHANGES.rst) - [Commits](https://github.com/nedbat/coveragepy/compare/7.2.5...7.2.7) --- updated-dependencies: - dependency-name: coverage dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 036256cda..5804608ee 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,7 +3,7 @@ # Testing black==23.3.0 -coverage==7.2.5 +coverage==7.2.7 importlib-metadata==6.6.0 mypy==1.3.0 ruff==0.0.263 From 872e3f71b9be6fc72bd3d144f212d49d54117462 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 1 Jul 2023 23:38:10 +0200 Subject: [PATCH 03/10] Bump mkdocs-material from 9.1.15 to 9.1.17 (#2200) Bumps [mkdocs-material](https://github.com/squidfunk/mkdocs-material) from 9.1.15 to 9.1.17. - [Release notes](https://github.com/squidfunk/mkdocs-material/releases) - [Changelog](https://github.com/squidfunk/mkdocs-material/blob/master/CHANGELOG) - [Commits](https://github.com/squidfunk/mkdocs-material/compare/9.1.15...9.1.17) --- updated-dependencies: - dependency-name: mkdocs-material dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 5804608ee..46f981dfb 100644 --- a/requirements.txt +++ b/requirements.txt @@ -16,7 +16,7 @@ trio==0.21.0 # Documentation mkdocs==1.4.3 -mkdocs-material==9.1.15 +mkdocs-material==9.1.17 mkautodoc==0.2.0 # Packaging From 602368cbb25aed5d6c012a704725b0bdb45f44af Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 1 Jul 2023 23:41:13 +0200 Subject: [PATCH 04/10] Bump ruff from 0.0.263 to 0.0.275 (#2201) Bumps [ruff](https://github.com/astral-sh/ruff) from 0.0.263 to 0.0.275. - [Release notes](https://github.com/astral-sh/ruff/releases) - [Changelog](https://github.com/astral-sh/ruff/blob/main/BREAKING_CHANGES.md) - [Commits](https://github.com/astral-sh/ruff/compare/v0.0.263...v0.0.275) --- updated-dependencies: - dependency-name: ruff dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 46f981dfb..791d87216 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,7 +6,7 @@ black==23.3.0 coverage==7.2.7 importlib-metadata==6.6.0 mypy==1.3.0 -ruff==0.0.263 +ruff==0.0.275 typing_extensions==4.5.0 types-contextvars==2.4.7.2 types-PyYAML==6.0.12.10 From 5092623ab30c7b62728a82cfa2b9a31cef266400 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 1 Jul 2023 23:48:16 +0200 Subject: [PATCH 05/10] Bump mypy from 1.3.0 to 1.4.1 (#2203) Bumps [mypy](https://github.com/python/mypy) from 1.3.0 to 1.4.1. - [Commits](https://github.com/python/mypy/compare/v1.3.0...v1.4.1) --- updated-dependencies: - dependency-name: mypy dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 791d87216..9a6a31487 100644 --- a/requirements.txt +++ b/requirements.txt @@ -5,7 +5,7 @@ black==23.3.0 coverage==7.2.7 importlib-metadata==6.6.0 -mypy==1.3.0 +mypy==1.4.1 ruff==0.0.275 typing_extensions==4.5.0 types-contextvars==2.4.7.2 From 91650852b31f35e0a7247035bef6ac3c759019b6 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 2 Jul 2023 00:35:45 +0200 Subject: [PATCH 06/10] Bump typing-extensions from 4.5.0 to 4.7.0 (#2202) Bumps [typing-extensions](https://github.com/python/typing_extensions) from 4.5.0 to 4.7.0. - [Release notes](https://github.com/python/typing_extensions/releases) - [Changelog](https://github.com/python/typing_extensions/blob/main/CHANGELOG.md) - [Commits](https://github.com/python/typing_extensions/compare/4.5.0...4.7.0) --- updated-dependencies: - dependency-name: typing-extensions dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 9a6a31487..88cd2a159 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,7 +7,7 @@ coverage==7.2.7 importlib-metadata==6.6.0 mypy==1.4.1 ruff==0.0.275 -typing_extensions==4.5.0 +typing_extensions==4.7.0 types-contextvars==2.4.7.2 types-PyYAML==6.0.12.10 types-dataclasses==0.6.6 From 53e1c50f592481f4b5af7883b3cf059571eba0cd Mon Sep 17 00:00:00 2001 From: Amin Alaee Date: Mon, 3 Jul 2023 09:13:59 +0200 Subject: [PATCH 07/10] Update pyproject.toml (#2205) --- pyproject.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 03c250a46..3ff3ca71b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -60,7 +60,6 @@ combine-as-imports = true [tool.mypy] disallow_untyped_defs = true ignore_missing_imports = true -no_implicit_optional = true show_error_codes = true [[tool.mypy.overrides]] From 0b3ea0986350f13c2841c70dee3e53992c37871e Mon Sep 17 00:00:00 2001 From: Stratos Gerakakis Date: Wed, 5 Jul 2023 22:55:54 +0200 Subject: [PATCH 08/10] Warn users when using `lifespan` together with `on_startup`/`on_shutdown` (#2193) Co-authored-by: Marcelo Trylesinski --- starlette/routing.py | 6 ++++++ tests/test_routing.py | 44 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/starlette/routing.py b/starlette/routing.py index 8e01c8562..132600be6 100644 --- a/starlette/routing.py +++ b/starlette/routing.py @@ -606,6 +606,12 @@ def __init__( "See more about it on https://www.starlette.io/lifespan/.", DeprecationWarning, ) + if lifespan: + warnings.warn( + "The `lifespan` parameter cannot be used with `on_startup` or " + "`on_shutdown`. Both `on_startup` and `on_shutdown` will be " + "ignored." + ) if lifespan is None: self.lifespan_context: Lifespan = _DefaultLifespan(self) diff --git a/tests/test_routing.py b/tests/test_routing.py index 129293224..04f425cad 100644 --- a/tests/test_routing.py +++ b/tests/test_routing.py @@ -648,6 +648,50 @@ async def run_shutdown(): assert shutdown_complete +def test_lifespan_with_on_events(test_client_factory: typing.Callable[..., TestClient]): + lifespan_called = False + startup_called = False + shutdown_called = False + + @contextlib.asynccontextmanager + async def lifespan(app: Starlette): + nonlocal lifespan_called + lifespan_called = True + yield + + # We do not expected, neither of run_startup nor run_shutdown to be called + # we thus mark them as #pragma: no cover, to fulfill test coverage + def run_startup(): # pragma: no cover + nonlocal startup_called + startup_called = True + + def run_shutdown(): # pragma: no cover + nonlocal shutdown_called + shutdown_called = True + + with pytest.warns( + UserWarning, + match=( + "The `lifespan` parameter cannot be used with `on_startup` or `on_shutdown`." # noqa: E501 + ), + ): + app = Router( + on_startup=[run_startup], on_shutdown=[run_shutdown], lifespan=lifespan + ) + + assert not lifespan_called + assert not startup_called + assert not shutdown_called + + # Triggers the lifespan events + with test_client_factory(app): + ... + + assert lifespan_called + assert not startup_called + assert not shutdown_called + + def test_lifespan_sync(test_client_factory): startup_complete = False shutdown_complete = False From d6007d7198c35c1a7ed81e678a81c3bca86bee5e Mon Sep 17 00:00:00 2001 From: Alex Oleshkevich Date: Thu, 6 Jul 2023 18:27:45 +0200 Subject: [PATCH 09/10] Add `follow_redirects` parameter to `TestClient` (#2207) --- starlette/testclient.py | 3 ++- tests/test_testclient.py | 25 ++++++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/starlette/testclient.py b/starlette/testclient.py index 7d4f3e396..099edc3ff 100644 --- a/starlette/testclient.py +++ b/starlette/testclient.py @@ -382,6 +382,7 @@ def __init__( backend_options: typing.Optional[typing.Dict[str, typing.Any]] = None, cookies: httpx._client.CookieTypes = None, headers: typing.Dict[str, str] = None, + follow_redirects: bool = True, ) -> None: self.async_backend = _AsyncBackend( backend=backend, backend_options=backend_options or {} @@ -409,7 +410,7 @@ def __init__( base_url=base_url, headers=headers, transport=transport, - follow_redirects=True, + follow_redirects=follow_redirects, cookies=cookies, ) diff --git a/tests/test_testclient.py b/tests/test_testclient.py index 1bf9692a6..0f36d4dcc 100644 --- a/tests/test_testclient.py +++ b/tests/test_testclient.py @@ -10,7 +10,7 @@ from starlette.applications import Starlette from starlette.middleware import Middleware -from starlette.responses import JSONResponse, Response +from starlette.responses import JSONResponse, RedirectResponse, Response from starlette.routing import Route from starlette.testclient import TestClient from starlette.websockets import WebSocket, WebSocketDisconnect @@ -319,3 +319,26 @@ async def app(scope, receive, send): response = client.get("/") cookie_set = len(response.cookies) == 1 assert cookie_set == ok + + +def test_forward_follow_redirects(test_client_factory): + async def app(scope, receive, send): + if "/ok" in scope["path"]: + response = Response("ok") + else: + response = RedirectResponse("/ok") + await response(scope, receive, send) + + client = test_client_factory(app, follow_redirects=True) + response = client.get("/") + assert response.status_code == 200 + + +def test_forward_nofollow_redirects(test_client_factory): + async def app(scope, receive, send): + response = RedirectResponse("/ok") + await response(scope, receive, send) + + client = test_client_factory(app, follow_redirects=False) + response = client.get("/") + assert response.status_code == 307 From 11a3f1227e69a29fc9dc56f348f982cb5372a1d1 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Thu, 13 Jul 2023 09:48:10 +0200 Subject: [PATCH 10/10] Stop `body_stream` in case `more_body=False` (#2194) --- starlette/middleware/base.py | 2 ++ tests/middleware/test_base.py | 65 +++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/starlette/middleware/base.py b/starlette/middleware/base.py index 2ff0e047b..170a805a7 100644 --- a/starlette/middleware/base.py +++ b/starlette/middleware/base.py @@ -170,6 +170,8 @@ async def body_stream() -> typing.AsyncGenerator[bytes, None]: body = message.get("body", b"") if body: yield body + if not message.get("more_body", False): + break if app_exc is not None: raise app_exc diff --git a/tests/middleware/test_base.py b/tests/middleware/test_base.py index f7dcf521c..cf4780cce 100644 --- a/tests/middleware/test_base.py +++ b/tests/middleware/test_base.py @@ -265,6 +265,71 @@ async def send(message): assert background_task_run.is_set() +@pytest.mark.anyio +async def test_do_not_block_on_background_tasks(): + request_body_sent = False + response_complete = anyio.Event() + events: List[Union[str, Message]] = [] + + async def sleep_and_set(): + events.append("Background task started") + await anyio.sleep(0.1) + events.append("Background task finished") + + async def endpoint_with_background_task(_): + return PlainTextResponse( + content="Hello", background=BackgroundTask(sleep_and_set) + ) + + async def passthrough( + request: Request, call_next: Callable[[Request], Awaitable[Response]] + ) -> Response: + return await call_next(request) + + app = Starlette( + middleware=[Middleware(BaseHTTPMiddleware, dispatch=passthrough)], + routes=[Route("/", endpoint_with_background_task)], + ) + + scope = { + "type": "http", + "version": "3", + "method": "GET", + "path": "/", + } + + async def receive() -> Message: + nonlocal request_body_sent + if not request_body_sent: + request_body_sent = True + return {"type": "http.request", "body": b"", "more_body": False} + await response_complete.wait() + return {"type": "http.disconnect"} + + async def send(message: Message): + if message["type"] == "http.response.body": + events.append(message) + if not message.get("more_body", False): + response_complete.set() + + async with anyio.create_task_group() as tg: + tg.start_soon(app, scope, receive, send) + tg.start_soon(app, scope, receive, send) + + # Without the fix, the background tasks would start and finish before the + # last http.response.body is sent. + assert events == [ + {"body": b"Hello", "more_body": True, "type": "http.response.body"}, + {"body": b"", "more_body": False, "type": "http.response.body"}, + {"body": b"Hello", "more_body": True, "type": "http.response.body"}, + {"body": b"", "more_body": False, "type": "http.response.body"}, + "Background task started", + "Background task started", + "Background task finished", + "Background task finished", + ] + + @pytest.mark.anyio async def test_run_context_manager_exit_even_if_client_disconnects(): # test for https://github.com/encode/starlette/issues/1678#issuecomment-1172916042