From 513d91717731d20ed55056240fea55445d79c7bc Mon Sep 17 00:00:00 2001 From: Thomas De Smedt Date: Fri, 14 Jan 2022 20:04:11 +0100 Subject: [PATCH 1/3] test: add (failing) test for subapp url lookup --- tests/test_routing.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/test_routing.py b/tests/test_routing.py index 231c581fb..23d1b134f 100644 --- a/tests/test_routing.py +++ b/tests/test_routing.py @@ -517,6 +517,40 @@ def test_url_for_with_root_path(test_client_factory): } + +def echo_urls_for(*keys): + def echo_urls(request): + return JSONResponse( + { + k: request.url_for(k) + for k in keys + } + ) + return echo_urls + +def test_url_with_sub_app(test_client_factory): + sub_app = Starlette( + routes=[Route("/", echo_urls_for('subapp_index'), name="subapp_index", methods=["GET"]) + ]) + app = Starlette( + routes=[ + Route("/", echo_urls_for('index', 'submount:subapp_index'), name="index", methods=["GET"]), + Mount('/submount', app=sub_app, name='submount') + ]) + + client = test_client_factory( + app, base_url="https://www.example.org/", root_path="/sub_path" + ) + response = client.get("/") + assert response.json() == { + "index": "https://www.example.org/sub_path/", + "submount:subapp_index": "https://www.example.org/sub_path/submount/", + } + response = client.get("/submount/") + assert response.json() == { + "subapp_index": "https://www.example.org/sub_path/submount/" + } + async def stub_app(scope, receive, send): pass # pragma: no cover From 1ad58b724b25b7b602c899970cba17a959833afe Mon Sep 17 00:00:00 2001 From: Thomas De Smedt Date: Sat, 15 Jan 2022 13:07:06 +0100 Subject: [PATCH 2/3] fixed test --- tests/test_routing.py | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/tests/test_routing.py b/tests/test_routing.py index 23d1b134f..da8a5320d 100644 --- a/tests/test_routing.py +++ b/tests/test_routing.py @@ -520,23 +520,26 @@ def test_url_for_with_root_path(test_client_factory): def echo_urls_for(*keys): def echo_urls(request): - return JSONResponse( - { - k: request.url_for(k) - for k in keys - } - ) + def try_url_for(name): + try: + return request.url_for(name) + except NoMatchFound: + return 'NoMatchFound' + return JSONResponse({ k: try_url_for(k) for k in keys }) return echo_urls def test_url_with_sub_app(test_client_factory): sub_app = Starlette( - routes=[Route("/", echo_urls_for('subapp_index'), name="subapp_index", methods=["GET"]) - ]) + routes=[ + Route("/", echo_urls_for('subapp_index','submount:subapp_index','index'), name="subapp_index", methods=["GET"]) + ] + ) app = Starlette( routes=[ Route("/", echo_urls_for('index', 'submount:subapp_index'), name="index", methods=["GET"]), Mount('/submount', app=sub_app, name='submount') - ]) + ] + ) client = test_client_factory( app, base_url="https://www.example.org/", root_path="/sub_path" @@ -548,7 +551,9 @@ def test_url_with_sub_app(test_client_factory): } response = client.get("/submount/") assert response.json() == { - "subapp_index": "https://www.example.org/sub_path/submount/" + "submount:subapp_index": "https://www.example.org/sub_path/submount/", + "subapp_index": "NoMatchFound", # "https://www.example.org/sub_path/submount/", + "index": "https://www.example.org/sub_path/", } async def stub_app(scope, receive, send): From 2e3b00553a3528b07cefb7434fad1f3ec58ea4db Mon Sep 17 00:00:00 2001 From: Thomas De Smedt Date: Fri, 14 Jan 2022 20:06:35 +0100 Subject: [PATCH 3/3] fix: correct Request.url_for for subapp mounts --- starlette/requests.py | 26 +++++++++++++++++++++----- tests/test_routing.py | 2 +- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/starlette/requests.py b/starlette/requests.py index a33367e1d..f4b8692d1 100644 --- a/starlette/requests.py +++ b/starlette/requests.py @@ -95,14 +95,23 @@ def url(self) -> URL: return self._url @property - def base_url(self) -> URL: - if not hasattr(self, "_base_url"): + def app_root_base_url(self) -> URL: + if not hasattr(self, "_app_root_base_url"): base_url_scope = dict(self.scope) base_url_scope["path"] = "/" base_url_scope["query_string"] = b"" base_url_scope["root_path"] = base_url_scope.get( "app_root_path", base_url_scope.get("root_path", "") ) + self._app_root_base_url = URL(scope=base_url_scope) + return self._app_root_base_url + + @property + def base_url(self) -> URL: + if not hasattr(self, "_base_url"): + base_url_scope = dict(self.scope) + base_url_scope["path"] = "/" + base_url_scope["query_string"] = b"" self._base_url = URL(scope=base_url_scope) return self._base_url @@ -170,9 +179,16 @@ def state(self) -> State: return self._state def url_for(self, name: str, **path_params: typing.Any) -> str: - router: Router = self.scope["router"] - url_path = router.url_path_for(name, **path_params) - return url_path.make_absolute_url(base_url=self.base_url) + app_root_router = self.scope["router"] + app_router = getattr(self.get('app', {}), 'router', None) + if app_router and app_router != app_root_router: + try: + url_path = app_router.url_path_for(name, **path_params) + return url_path.make_absolute_url(base_url=self.base_url) + except Exception: + pass + url_path = app_root_router.url_path_for(name, **path_params) + return url_path.make_absolute_url(base_url=self.app_root_base_url) async def empty_receive() -> typing.NoReturn: diff --git a/tests/test_routing.py b/tests/test_routing.py index da8a5320d..d6fc5473b 100644 --- a/tests/test_routing.py +++ b/tests/test_routing.py @@ -552,7 +552,7 @@ def test_url_with_sub_app(test_client_factory): response = client.get("/submount/") assert response.json() == { "submount:subapp_index": "https://www.example.org/sub_path/submount/", - "subapp_index": "NoMatchFound", # "https://www.example.org/sub_path/submount/", + "subapp_index": "https://www.example.org/sub_path/submount/", "index": "https://www.example.org/sub_path/", }