From b164777d9c47625aa062c085aa5af29c766c679b Mon Sep 17 00:00:00 2001 From: Daniel Troeder Date: Fri, 31 Dec 2021 12:07:34 +0100 Subject: [PATCH 1/2] remove NBSP in comments --- starlette/routing.py | 4 ++-- starlette/testclient.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/starlette/routing.py b/starlette/routing.py index 0388304c9..c5bc94a77 100644 --- a/starlette/routing.py +++ b/starlette/routing.py @@ -670,7 +670,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: partial_scope = child_scope if partial is not None: - #  Handle partial matches. These are cases where an endpoint is + # Handle partial matches. These are cases where an endpoint is # able to handle the request, but is not a preferred option. # We use this in particular to deal with "405 Method Not Allowed". scope.update(partial_scope) @@ -698,7 +698,7 @@ def __eq__(self, other: typing.Any) -> bool: return isinstance(other, Router) and self.routes == other.routes # The following usages are now discouraged in favour of configuration - #  during Router.__init__(...) + # during Router.__init__(...) def mount( self, path: str, app: ASGIApp, name: str = None ) -> None: # pragma: nocover diff --git a/starlette/testclient.py b/starlette/testclient.py index 7567d18fd..8f5170b07 100644 --- a/starlette/testclient.py +++ b/starlette/testclient.py @@ -428,7 +428,7 @@ def __init__( asgi_app = app else: app = typing.cast(ASGI2App, app) - asgi_app = _WrapASGI2(app) #  type: ignore + asgi_app = _WrapASGI2(app) # type: ignore adapter = _ASGIAdapter( asgi_app, portal_factory=self._portal_factory, From 158ab3fc98d501dc349e16e5fec80ffb247d89e8 Mon Sep 17 00:00:00 2001 From: Daniel Troeder Date: Sat, 1 Jan 2022 14:11:14 +0100 Subject: [PATCH 2/2] allow 'name' in key word arguments of url_for() and url_path_for() (fix encode/starlette#608) --- starlette/applications.py | 4 ++-- starlette/requests.py | 6 ++++-- starlette/routing.py | 30 +++++++++++++++++++++--------- starlette/templating.py | 4 ++-- tests/test_requests.py | 21 +++++++++++++++++++++ tests/test_routing.py | 30 +++++++++++++++++++++++++++++- 6 files changed, 79 insertions(+), 16 deletions(-) diff --git a/starlette/applications.py b/starlette/applications.py index d83f70d2f..4502b174b 100644 --- a/starlette/applications.py +++ b/starlette/applications.py @@ -111,8 +111,8 @@ def debug(self, value: bool) -> None: self._debug = value self.middleware_stack = self.build_middleware_stack() - def url_path_for(self, name: str, **path_params: typing.Any) -> URLPath: - return self.router.url_path_for(name, **path_params) + def url_path_for(self, *args: str, **path_params: typing.Any) -> URLPath: + return self.router.url_path_for(*args, **path_params) async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: scope["app"] = self diff --git a/starlette/requests.py b/starlette/requests.py index e3c91e284..a0552ae02 100644 --- a/starlette/requests.py +++ b/starlette/requests.py @@ -172,9 +172,11 @@ def state(self) -> State: self._state = State(self.scope["state"]) return self._state - def url_for(self, name: str, **path_params: typing.Any) -> str: + def url_for(self, *args: str, **path_params: typing.Any) -> str: + if len(args) != 1: + raise TypeError("url_for() takes exactly one positional argument") router: Router = self.scope["router"] - url_path = router.url_path_for(name, **path_params) + url_path = router.url_path_for(*args, **path_params) return url_path.make_absolute_url(base_url=self.base_url) diff --git a/starlette/routing.py b/starlette/routing.py index c5bc94a77..ffd2bbd00 100644 --- a/starlette/routing.py +++ b/starlette/routing.py @@ -27,7 +27,7 @@ class NoMatchFound(Exception): """ - Raised by `.url_for(name, **path_params)` and `.url_path_for(name, **path_params)` + Raised by `.url_for(*args, **path_params)` and `.url_path_for(*args, **path_params)` if no matching route exists. """ @@ -160,7 +160,7 @@ class BaseRoute: def matches(self, scope: Scope) -> typing.Tuple[Match, Scope]: raise NotImplementedError() # pragma: no cover - def url_path_for(self, name: str, **path_params: typing.Any) -> URLPath: + def url_path_for(self, *args: str, **path_params: typing.Any) -> URLPath: raise NotImplementedError() # pragma: no cover async def handle(self, scope: Scope, receive: Receive, send: Send) -> None: @@ -239,7 +239,10 @@ def matches(self, scope: Scope) -> typing.Tuple[Match, Scope]: return Match.FULL, child_scope return Match.NONE, {} - def url_path_for(self, name: str, **path_params: typing.Any) -> URLPath: + def url_path_for(self, *args: str, **path_params: typing.Any) -> URLPath: + if len(args) != 1: + raise TypeError("url_path_for() takes exactly one positional argument") + name = args[0] seen_params = set(path_params.keys()) expected_params = set(self.param_convertors.keys()) @@ -308,7 +311,10 @@ def matches(self, scope: Scope) -> typing.Tuple[Match, Scope]: return Match.FULL, child_scope return Match.NONE, {} - def url_path_for(self, name: str, **path_params: typing.Any) -> URLPath: + def url_path_for(self, *args: str, **path_params: typing.Any) -> URLPath: + if len(args) != 1: + raise TypeError("url_path_for() takes exactly one positional argument") + name = args[0] seen_params = set(path_params.keys()) expected_params = set(self.param_convertors.keys()) @@ -381,7 +387,10 @@ def matches(self, scope: Scope) -> typing.Tuple[Match, Scope]: return Match.FULL, child_scope return Match.NONE, {} - def url_path_for(self, name: str, **path_params: typing.Any) -> URLPath: + def url_path_for(self, *args: str, **path_params: typing.Any) -> URLPath: + if len(args) != 1: + raise TypeError("url_path_for() takes exactly one positional argument") + name = args[0] if self.name is not None and name == self.name and "path" in path_params: # 'name' matches "". path_params["path"] = path_params["path"].lstrip("/") @@ -451,7 +460,10 @@ def matches(self, scope: Scope) -> typing.Tuple[Match, Scope]: return Match.FULL, child_scope return Match.NONE, {} - def url_path_for(self, name: str, **path_params: typing.Any) -> URLPath: + def url_path_for(self, *args: str, **path_params: typing.Any) -> URLPath: + if len(args) != 1: + raise TypeError("url_path_for() takes exactly one positional argument") + name = args[0] if self.name is not None and name == self.name and "path" in path_params: # 'name' matches "". path = path_params.pop("path") @@ -591,13 +603,13 @@ async def not_found(self, scope: Scope, receive: Receive, send: Send) -> None: response = PlainTextResponse("Not Found", status_code=404) await response(scope, receive, send) - def url_path_for(self, name: str, **path_params: typing.Any) -> URLPath: + def url_path_for(self, *args: str, **path_params: typing.Any) -> URLPath: for route in self.routes: try: - return route.url_path_for(name, **path_params) + return route.url_path_for(*args, **path_params) except NoMatchFound: pass - raise NoMatchFound(name, path_params) + raise NoMatchFound(args[0], path_params) async def startup(self) -> None: """ diff --git a/starlette/templating.py b/starlette/templating.py index 7a1e21827..f4e0612c7 100644 --- a/starlette/templating.py +++ b/starlette/templating.py @@ -65,9 +65,9 @@ def _create_env( self, directory: typing.Union[str, PathLike], **env_options: typing.Any ) -> "jinja2.Environment": @pass_context - def url_for(context: dict, name: str, **path_params: typing.Any) -> str: + def url_for(context: dict, *args: str, **path_params: typing.Any) -> str: request = context["request"] - return request.url_for(name, **path_params) + return request.url_for(*args, **path_params) loader = jinja2.FileSystemLoader(directory) env_options.setdefault("loader", loader) diff --git a/tests/test_requests.py b/tests/test_requests.py index 799e61f80..d935de099 100644 --- a/tests/test_requests.py +++ b/tests/test_requests.py @@ -3,6 +3,7 @@ import anyio import pytest +from starlette.applications import Starlette from starlette.datastructures import Address from starlette.requests import ClientDisconnect, Request, State from starlette.responses import JSONResponse, PlainTextResponse, Response @@ -415,6 +416,26 @@ async def app(scope, receive, send): assert result["cookies"] == expected +def test_request_url_for_allows_name_arg(test_client_factory): + app = Starlette() + + @app.route("/users/{name}") + async def func_users(request): + raise NotImplementedError() # pragma: no cover + + @app.route("/test") + async def func_url_for_test(request: Request): + with pytest.raises(TypeError, match="takes exactly one positional argument"): + request.url_for("func_users", "abcde", "fghij") + url = request.url_for("func_users", name="abcde") + return Response(str(url), media_type="text/plain") + + client = test_client_factory(app) + response = client.get("/test") + assert response.status_code == 200 + assert response.text == "http://testserver/users/abcde" + + def test_chunked_encoding(test_client_factory): async def app(scope, receive, send): request = Request(scope, receive) diff --git a/tests/test_routing.py b/tests/test_routing.py index 7077c5616..3e8d0e415 100644 --- a/tests/test_routing.py +++ b/tests/test_routing.py @@ -22,6 +22,11 @@ def user(request): return Response(content, media_type="text/plain") +def user2(request): + content = "User 2 " + request.path_params["name"] + return Response(content, media_type="text/plain") + + def user_me(request): content = "User fixed me" return Response(content, media_type="text/plain") @@ -108,6 +113,7 @@ async def websocket_params(session: WebSocket): Route("/", endpoint=users), Route("/me", endpoint=user_me), Route("/{username}", endpoint=user), + Route("/n/{name}", endpoint=user2), Route("/nomatch", endpoint=user_no_match), ], ), @@ -179,6 +185,10 @@ def test_router(client): assert response.status_code == 200 assert response.text == "User tomchristie" + response = client.get("/users/n/tomchristie") + assert response.status_code == 200 + assert response.text == "User 2 tomchristie" + response = client.get("/users/me") assert response.status_code == 200 assert response.text == "User fixed me" @@ -241,7 +251,12 @@ def test_route_converters(client): def test_url_path_for(): assert app.url_path_for("homepage") == "/" - assert app.url_path_for("user", username="tomchristie") == "/users/tomchristie" + assert app.url_path_for("user", username="tomchristie1") == "/users/tomchristie1" + assert app.url_path_for("user2", name="tomchristie2") == "/users/n/tomchristie2" + with pytest.raises(NoMatchFound): + assert app.url_path_for("user", name="tomchristie1") + with pytest.raises(NoMatchFound): + assert app.url_path_for("user2", username="tomchristie2") assert app.url_path_for("websocket_endpoint") == "/ws" with pytest.raises( NoMatchFound, match='No route exists for name "broken" and params "".' @@ -255,6 +270,19 @@ def test_url_path_for(): app.url_path_for("user", username="tom/christie") with pytest.raises(AssertionError): app.url_path_for("user", username="") + with pytest.raises(TypeError, match="takes exactly one positional argument"): + assert app.url_path_for("user", "args2", name="tomchristie1") + with pytest.raises(TypeError, match="takes exactly one positional argument"): + assert app.url_path_for(name="tomchristie1") + ws_route = WebSocketRoute("/ws", endpoint=websocket_endpoint) + with pytest.raises(TypeError, match="takes exactly one positional argument"): + ws_route.url_path_for("foo", "bar") + mount = Mount("/users", ok, name="users") + with pytest.raises(TypeError, match="takes exactly one positional argument"): + mount.url_path_for("foo", "bar") + host = Host("{subdomain}.example.org", app=subdomain_app, name="subdomains") + with pytest.raises(TypeError, match="takes exactly one positional argument"): + host.url_path_for("foo", "bar") def test_url_for():