From 61590a0e2484b76a68f3f36db71ef41b161412ac Mon Sep 17 00:00:00 2001 From: gabalafou Date: Tue, 29 Oct 2024 14:06:24 +0100 Subject: [PATCH 01/14] Create catch all/fallback route for UI app --- .../_internal/server/app.py | 45 ++++++++++--------- .../server/templates/conda-store-ui.html | 4 +- .../_internal/server/views/conda_store_ui.py | 3 +- 3 files changed, 28 insertions(+), 24 deletions(-) diff --git a/conda-store-server/conda_store_server/_internal/server/app.py b/conda-store-server/conda_store_server/_internal/server/app.py index 7e23361d8..006f936f1 100644 --- a/conda-store-server/conda_store_server/_internal/server/app.py +++ b/conda-store-server/conda_store_server/_internal/server/app.py @@ -293,15 +293,32 @@ async def exception_handler(request, exc): # docker registry api specification does not support a url_prefix app.include_router(views.router_registry) - if self.enable_ui: + if self.enable_metrics: app.include_router( - views.router_ui, - prefix=trim_slash(self.url_prefix) + "/admin", + views.router_metrics, + prefix=trim_slash(self.url_prefix), ) + if self.additional_routes: + for path, method, func in self.additional_routes: + getattr(app, method)(path, name=func.__name__)(func) + + if isinstance(self.conda_store.storage, storage.LocalStorage): + self.conda_store.storage.storage_url = ( + f"{trim_slash(self.url_prefix)}/storage" + ) + app.mount( + self.conda_store.storage.storage_url, + StaticFiles(directory=self.conda_store.storage.storage_path), + name="static-storage", + ) + + # This needs to come at the end because if the UI is enabled, + # it becomes the catch all route + if self.enable_ui: app.include_router( - views.router_conda_store_ui, - prefix=trim_slash(self.url_prefix), + views.router_ui, + prefix=trim_slash(self.url_prefix) + "/admin", ) # serving static files @@ -336,26 +353,12 @@ async def favicon(): ) ) - if self.enable_metrics: + # Put this at the very end app.include_router( - views.router_metrics, + views.router_conda_store_ui, prefix=trim_slash(self.url_prefix), ) - if self.additional_routes: - for path, method, func in self.additional_routes: - getattr(app, method)(path, name=func.__name__)(func) - - if isinstance(self.conda_store.storage, storage.LocalStorage): - self.conda_store.storage.storage_url = ( - f"{trim_slash(self.url_prefix)}/storage" - ) - app.mount( - self.conda_store.storage.storage_url, - StaticFiles(directory=self.conda_store.storage.storage_path), - name="static-storage", - ) - return app def _check_worker(self, delay=5): diff --git a/conda-store-server/conda_store_server/_internal/server/templates/conda-store-ui.html b/conda-store-server/conda_store_server/_internal/server/templates/conda-store-ui.html index 2a05a49db..3ce8ffce0 100644 --- a/conda-store-server/conda_store_server/_internal/server/templates/conda-store-ui.html +++ b/conda-store-server/conda_store_server/_internal/server/templates/conda-store-ui.html @@ -28,8 +28,8 @@ REACT_APP_URL_BASENAME: new URL("{{ url_for('get_conda_store_ui') }}").pathname }; - - + + diff --git a/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py b/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py index ece7d8a74..c7a32d1b4 100644 --- a/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py +++ b/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py @@ -9,10 +9,11 @@ router_conda_store_ui = APIRouter(tags=["conda-store-ui"]) - @router_conda_store_ui.get("/") +@router_conda_store_ui.get("/{full_path:path}") async def get_conda_store_ui( request: Request, + full_path: str, templates=Depends(dependencies.get_templates), ): context = { From ecff9ab253686e774f26490fe4927ccbf8f6278e Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 29 Oct 2024 13:15:44 +0000 Subject: [PATCH 02/14] [pre-commit.ci] Apply automatic pre-commit fixes --- .../conda_store_server/_internal/server/views/conda_store_ui.py | 1 + 1 file changed, 1 insertion(+) diff --git a/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py b/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py index c7a32d1b4..1daf54357 100644 --- a/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py +++ b/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py @@ -9,6 +9,7 @@ router_conda_store_ui = APIRouter(tags=["conda-store-ui"]) + @router_conda_store_ui.get("/") @router_conda_store_ui.get("/{full_path:path}") async def get_conda_store_ui( From 27ab92e77ee7ce8eb4acfa440203172240f03e5f Mon Sep 17 00:00:00 2001 From: gabalafou Date: Tue, 29 Oct 2024 14:47:15 +0100 Subject: [PATCH 03/14] add 404 status code to url_prefix/not_found route --- .../_internal/server/views/conda_store_ui.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py b/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py index 1daf54357..d48570440 100644 --- a/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py +++ b/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py @@ -20,4 +20,7 @@ async def get_conda_store_ui( context = { "request": request, } - return templates.TemplateResponse("conda-store-ui.html", context) + response = templates.TemplateResponse("conda-store-ui.html", context) + if full_path.endswith("not-found"): + response.status_code = 404 + return response \ No newline at end of file From ebf8b66b8ef7b101f6afcd324284c2cc56e1dcd2 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 29 Oct 2024 13:51:46 +0000 Subject: [PATCH 04/14] [pre-commit.ci] Apply automatic pre-commit fixes --- .../conda_store_server/_internal/server/views/conda_store_ui.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py b/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py index d48570440..695780fae 100644 --- a/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py +++ b/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py @@ -23,4 +23,4 @@ async def get_conda_store_ui( response = templates.TemplateResponse("conda-store-ui.html", context) if full_path.endswith("not-found"): response.status_code = 404 - return response \ No newline at end of file + return response From dd8fa8b5a0a818cf4f85d751438a0888b30ae36f Mon Sep 17 00:00:00 2001 From: gabalafou Date: Mon, 4 Nov 2024 16:58:41 +0100 Subject: [PATCH 05/14] add passing tests --- .../server/views/test_conda_store_ui.py | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 conda-store-server/tests/_internal/server/views/test_conda_store_ui.py diff --git a/conda-store-server/tests/_internal/server/views/test_conda_store_ui.py b/conda-store-server/tests/_internal/server/views/test_conda_store_ui.py new file mode 100644 index 000000000..cf4bba361 --- /dev/null +++ b/conda-store-server/tests/_internal/server/views/test_conda_store_ui.py @@ -0,0 +1,109 @@ +# Copyright (c) conda-store development team. All rights reserved. +# Use of this source code is governed by a BSD-style +# license that can be found in the LICENSE file. + +from fastapi.testclient import TestClient + + +def test_base_route(testclient): + """The server should return the client app""" + response = testclient.get("/") + assert response.is_success + assert "text/html" in response.headers["content-type"] + assert "condaStoreConfig" in response.text + + +def test_unknown_routes(testclient): + """Rather than return a 404, the server should return the client app + and let it handle unknown routes""" + response = testclient.get("/foo") + assert response.is_success + assert "text/html" in response.headers["content-type"] + assert "condaStoreConfig" in response.text + + response = testclient.get("/foo/bar") + assert response.is_success + assert "text/html" in response.headers["content-type"] + assert "condaStoreConfig" in response.text + + +def test_not_found_route(testclient): + """The /not-found route should also return the client app + but with a 404 status code""" + response = testclient.get("/not-found") + assert response.status_code == 404 + assert "text/html" in response.headers["content-type"] + assert "condaStoreConfig" in response.text + + +def test_route_outside_ui_app(testclient): + """The server should not return the client app for a server-side route""" + response = testclient.get("/favicon.ico") + assert response.is_success + assert response.headers["content-type"].startswith("image/") + + +def test_base_route_prefix(conda_store_server): + conda_store_server.url_prefix = "/conda-store" + testclient = TestClient(conda_store_server.init_fastapi_app()) + response = testclient.get("/conda-store") + assert response.is_success + assert "text/html" in response.headers["content-type"] + assert "condaStoreConfig" in response.text + + +def test_unknown_routes_prefix(conda_store_server): + conda_store_server.url_prefix = "/conda-store" + testclient = TestClient(conda_store_server.init_fastapi_app()) + + response = testclient.get("/conda-store/foo") + assert response.is_success + assert "text/html" in response.headers["content-type"] + assert "condaStoreConfig" in response.text + + response = testclient.get("/conda-store/foo/bar") + assert response.is_success + assert "text/html" in response.headers["content-type"] + assert "condaStoreConfig" in response.text + + +def test_not_found_route_prefix(conda_store_server): + conda_store_server.url_prefix = "/conda-store" + testclient = TestClient(conda_store_server.init_fastapi_app()) + + response = testclient.get("/conda-store/not-found") + assert response.status_code == 404 + assert "text/html" in response.headers["content-type"] + assert "condaStoreConfig" in response.text + + +def test_route_outside_ui_app_prefix(conda_store_server): + """The server should not return the client app for a known route + that is already mapped server-side""" + conda_store_server.url_prefix = "/conda-store" + testclient = TestClient(conda_store_server.init_fastapi_app()) + + response = testclient.get("/favicon.ico") + assert response.is_success + assert response.headers["content-type"].startswith("image/") + + # test nonsense route + response = testclient.get("/foo/bar") + assert response.status_code == 404 + assert "condaStoreConfig" not in response.text + + +def test_ui_disabled(conda_store_server): + conda_store_server.enable_ui = False + testclient = TestClient(conda_store_server.init_fastapi_app()) + + response = testclient.get("/") + assert "condaStoreConfig" not in response.text + + response = testclient.get("/not-found") + assert response.status_code == 404 + assert "condaStoreConfig" not in response.text + + response = testclient.get("/favicon.ico") + assert response.status_code == 404 + assert "condaStoreConfig" not in response.text From b40e428fe9457e9786f62462c6f539dbcbc6b406 Mon Sep 17 00:00:00 2001 From: gabalafou Date: Tue, 5 Nov 2024 23:25:22 +0100 Subject: [PATCH 06/14] clean tests and corresponding code --- .../_internal/server/app.py | 2 +- .../server/templates/conda-store-ui.html | 7 +- .../_internal/server/views/conda_store_ui.py | 20 ++- .../conda_store_server/server/dependencies.py | 3 + .../server/views/test_conda_store_ui.py | 155 ++++++++---------- 5 files changed, 95 insertions(+), 92 deletions(-) diff --git a/conda-store-server/conda_store_server/_internal/server/app.py b/conda-store-server/conda_store_server/_internal/server/app.py index 006f936f1..1836e4235 100644 --- a/conda-store-server/conda_store_server/_internal/server/app.py +++ b/conda-store-server/conda_store_server/_internal/server/app.py @@ -343,7 +343,7 @@ async def exception_handler(request, exc): def redirect_home(request: Request): return RedirectResponse(request.url_for("get_conda_store_ui")) - @app.get("/favicon.ico", include_in_schema=False) + @app.get(trim_slash(self.url_prefix) + "/favicon.ico", include_in_schema=False) async def favicon(): return FileResponse( os.path.join( diff --git a/conda-store-server/conda_store_server/_internal/server/templates/conda-store-ui.html b/conda-store-server/conda_store_server/_internal/server/templates/conda-store-ui.html index 3ce8ffce0..03545c27e 100644 --- a/conda-store-server/conda_store_server/_internal/server/templates/conda-store-ui.html +++ b/conda-store-server/conda_store_server/_internal/server/templates/conda-store-ui.html @@ -25,11 +25,12 @@ REACT_APP_API_URL: "{{ url_for('get_conda_store_ui') }}", REACT_APP_LOGIN_PAGE_URL: "{{ url_for('get_login_method') }}?next=", REACT_APP_LOGOUT_PAGE_URL: "{{ url_for('post_logout_method') }}?next=/", - REACT_APP_URL_BASENAME: new URL("{{ url_for('get_conda_store_ui') }}").pathname + REACT_APP_URL_BASENAME: "{{ url_prefix }}" }; - - + + + diff --git a/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py b/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py index 695780fae..54410d24d 100644 --- a/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py +++ b/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py @@ -3,24 +3,34 @@ # license that can be found in the LICENSE file. from fastapi import APIRouter, Depends, Request +from fastapi.responses import HTMLResponse from conda_store_server.server import dependencies -router_conda_store_ui = APIRouter(tags=["conda-store-ui"]) - +router_conda_store_ui = APIRouter( + tags=["conda-store-ui"], + # Provide the response class so that it can show up in the autogenerated API + # docs. The docs show the endpoints as returning HTML rather than JSON. + default_response_class=HTMLResponse +) +# Yes, "{path:path}"" makes "/" redundant. But if we don't define "/", then we +# always have to pass `path` to the `url_for` function like so: +# url_for("get_conda_store_ui", path="") @router_conda_store_ui.get("/") -@router_conda_store_ui.get("/{full_path:path}") +@router_conda_store_ui.get("{path:path}") async def get_conda_store_ui( request: Request, - full_path: str, + path: str, templates=Depends(dependencies.get_templates), + url_prefix=Depends(dependencies.get_url_prefix), ): context = { "request": request, + "url_prefix": url_prefix, } response = templates.TemplateResponse("conda-store-ui.html", context) - if full_path.endswith("not-found"): + if path.endswith("not-found"): response.status_code = 404 return response diff --git a/conda-store-server/conda_store_server/server/dependencies.py b/conda-store-server/conda_store_server/server/dependencies.py index 84c04fd54..292e17daa 100644 --- a/conda-store-server/conda_store_server/server/dependencies.py +++ b/conda-store-server/conda_store_server/server/dependencies.py @@ -23,3 +23,6 @@ async def get_entity(request: Request, auth=Depends(get_auth)): async def get_templates(request: Request): return request.state.templates + +async def get_url_prefix(request: Request, server=Depends(get_server)): + return server.url_prefix \ No newline at end of file diff --git a/conda-store-server/tests/_internal/server/views/test_conda_store_ui.py b/conda-store-server/tests/_internal/server/views/test_conda_store_ui.py index cf4bba361..45e16c4ba 100644 --- a/conda-store-server/tests/_internal/server/views/test_conda_store_ui.py +++ b/conda-store-server/tests/_internal/server/views/test_conda_store_ui.py @@ -2,108 +2,97 @@ # Use of this source code is governed by a BSD-style # license that can be found in the LICENSE file. +import pytest from fastapi.testclient import TestClient -def test_base_route(testclient): - """The server should return the client app""" - response = testclient.get("/") - assert response.is_success - assert "text/html" in response.headers["content-type"] - assert "condaStoreConfig" in response.text - - -def test_unknown_routes(testclient): - """Rather than return a 404, the server should return the client app - and let it handle unknown routes""" - response = testclient.get("/foo") - assert response.is_success - assert "text/html" in response.headers["content-type"] - assert "condaStoreConfig" in response.text - - response = testclient.get("/foo/bar") +def assert_client_app(response): + """A few checks that should all pass if the response is ok and contains + the client app""" assert response.is_success assert "text/html" in response.headers["content-type"] assert "condaStoreConfig" in response.text -def test_not_found_route(testclient): - """The /not-found route should also return the client app - but with a 404 status code""" - response = testclient.get("/not-found") +class TestUIRoutes: + url_prefix = "/" + + def full_route(self, route): + if self.url_prefix == "/": + return route + else: + return self.url_prefix + route + + @pytest.fixture + def testclient(self, conda_store_server): + conda_store_server.enable_ui = True + conda_store_server.url_prefix = self.url_prefix + return TestClient(conda_store_server.init_fastapi_app()) + + def test_base_route(self, testclient): + """The server should return the client app""" + response = testclient.get(self.url_prefix) + assert_client_app(response) + + def test_unknown_routes(self, testclient): + """Rather than return a 404, the server should return the client app + and let it handle unknown routes""" + response = testclient.get(self.full_route("/foo")) + assert_client_app(response) + + response = testclient.get(self.full_route("/foo/bar")) + assert_client_app(response) + + def test_not_found_route(self, testclient): + """The /not-found route should also return the client app + but with a 404 status code""" + response = testclient.get(self.full_route("/not-found")) + assert response.status_code == 404 + assert "text/html" in response.headers["content-type"] + assert "condaStoreConfig" in response.text + + def test_route_outside_ui_app(self, testclient): + """The server should not return the client app for a server-side + route""" + # Trailing slash needed on next line. Is this a problem? + response = testclient.get(self.full_route("/admin/")) + assert response.is_success + assert "condaStoreConfig" not in response.text + + response = testclient.get(self.full_route("/favicon.ico")) + assert response.is_success + assert response.headers["content-type"].startswith("image/") + + +def assert_not_found_not_client_app(response): assert response.status_code == 404 - assert "text/html" in response.headers["content-type"] - assert "condaStoreConfig" in response.text - - -def test_route_outside_ui_app(testclient): - """The server should not return the client app for a server-side route""" - response = testclient.get("/favicon.ico") - assert response.is_success - assert response.headers["content-type"].startswith("image/") - - -def test_base_route_prefix(conda_store_server): - conda_store_server.url_prefix = "/conda-store" - testclient = TestClient(conda_store_server.init_fastapi_app()) - response = testclient.get("/conda-store") - assert response.is_success - assert "text/html" in response.headers["content-type"] - assert "condaStoreConfig" in response.text - - -def test_unknown_routes_prefix(conda_store_server): - conda_store_server.url_prefix = "/conda-store" - testclient = TestClient(conda_store_server.init_fastapi_app()) - - response = testclient.get("/conda-store/foo") - assert response.is_success - assert "text/html" in response.headers["content-type"] - assert "condaStoreConfig" in response.text - - response = testclient.get("/conda-store/foo/bar") - assert response.is_success - assert "text/html" in response.headers["content-type"] - assert "condaStoreConfig" in response.text - - -def test_not_found_route_prefix(conda_store_server): - conda_store_server.url_prefix = "/conda-store" - testclient = TestClient(conda_store_server.init_fastapi_app()) - - response = testclient.get("/conda-store/not-found") - assert response.status_code == 404 - assert "text/html" in response.headers["content-type"] - assert "condaStoreConfig" in response.text - + assert "condaStoreConfig" not in response.text -def test_route_outside_ui_app_prefix(conda_store_server): - """The server should not return the client app for a known route - that is already mapped server-side""" - conda_store_server.url_prefix = "/conda-store" - testclient = TestClient(conda_store_server.init_fastapi_app()) +class TestUIRoutesCustomPrefix(TestUIRoutes): + url_prefix = "/conda-store" - response = testclient.get("/favicon.ico") - assert response.is_success - assert response.headers["content-type"].startswith("image/") - - # test nonsense route - response = testclient.get("/foo/bar") - assert response.status_code == 404 - assert "condaStoreConfig" not in response.text + def test_unknown_route_outside_prefix(self, testclient): + """The server should return a 404 for an unknown route outside + the url prefix and should not return the client app""" + response = testclient.get("/foo/bar") + assert_not_found_not_client_app(response) def test_ui_disabled(conda_store_server): + """When the UI is disabled, the server should return 404s for + all UI routes and should not return the client app""" conda_store_server.enable_ui = False + conda_store_server.url_prefix = "/" testclient = TestClient(conda_store_server.init_fastapi_app()) response = testclient.get("/") - assert "condaStoreConfig" not in response.text + assert_not_found_not_client_app(response) response = testclient.get("/not-found") - assert response.status_code == 404 - assert "condaStoreConfig" not in response.text + assert_not_found_not_client_app(response) + + response = testclient.get("/admin/") + assert_not_found_not_client_app(response) response = testclient.get("/favicon.ico") - assert response.status_code == 404 - assert "condaStoreConfig" not in response.text + assert_not_found_not_client_app(response) From 046df62dd9eb3686873dc4a647f1e5b993a3a693 Mon Sep 17 00:00:00 2001 From: gabalafou Date: Tue, 5 Nov 2024 23:55:55 +0100 Subject: [PATCH 07/14] new line end of file --- conda-store-server/conda_store_server/server/dependencies.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conda-store-server/conda_store_server/server/dependencies.py b/conda-store-server/conda_store_server/server/dependencies.py index 292e17daa..1d39861d6 100644 --- a/conda-store-server/conda_store_server/server/dependencies.py +++ b/conda-store-server/conda_store_server/server/dependencies.py @@ -25,4 +25,4 @@ async def get_templates(request: Request): return request.state.templates async def get_url_prefix(request: Request, server=Depends(get_server)): - return server.url_prefix \ No newline at end of file + return server.url_prefix From b336d06d510d36b92fd4204d6c694b22d7530d47 Mon Sep 17 00:00:00 2001 From: gabalafou Date: Wed, 6 Nov 2024 00:09:11 +0100 Subject: [PATCH 08/14] format --- .../conda_store_server/_internal/server/views/conda_store_ui.py | 1 - 1 file changed, 1 deletion(-) diff --git a/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py b/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py index 54410d24d..8d1dbb995 100644 --- a/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py +++ b/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py @@ -7,7 +7,6 @@ from conda_store_server.server import dependencies - router_conda_store_ui = APIRouter( tags=["conda-store-ui"], # Provide the response class so that it can show up in the autogenerated API From 45a61b0d91bdbbb7e121d6d33c171b4afaf861a3 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 5 Nov 2024 23:13:24 +0000 Subject: [PATCH 09/14] [pre-commit.ci] Apply automatic pre-commit fixes --- .../_internal/server/app.py | 4 +++- .../_internal/server/dependencies.py | 1 + .../server/views/test_conda_store_ui.py | 19 +++++++++++++------ 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/conda-store-server/conda_store_server/_internal/server/app.py b/conda-store-server/conda_store_server/_internal/server/app.py index 569a13512..146ce5e34 100644 --- a/conda-store-server/conda_store_server/_internal/server/app.py +++ b/conda-store-server/conda_store_server/_internal/server/app.py @@ -338,7 +338,9 @@ async def exception_handler(request, exc): def redirect_home(request: Request): return RedirectResponse(request.url_for("get_conda_store_ui")) - @app.get(trim_slash(self.url_prefix) + "/favicon.ico", include_in_schema=False) + @app.get( + trim_slash(self.url_prefix) + "/favicon.ico", include_in_schema=False + ) async def favicon(): return FileResponse( os.path.join( diff --git a/conda-store-server/conda_store_server/_internal/server/dependencies.py b/conda-store-server/conda_store_server/_internal/server/dependencies.py index 1d39861d6..7226760bd 100644 --- a/conda-store-server/conda_store_server/_internal/server/dependencies.py +++ b/conda-store-server/conda_store_server/_internal/server/dependencies.py @@ -24,5 +24,6 @@ async def get_entity(request: Request, auth=Depends(get_auth)): async def get_templates(request: Request): return request.state.templates + async def get_url_prefix(request: Request, server=Depends(get_server)): return server.url_prefix diff --git a/conda-store-server/tests/_internal/server/views/test_conda_store_ui.py b/conda-store-server/tests/_internal/server/views/test_conda_store_ui.py index 45e16c4ba..9e8e32d41 100644 --- a/conda-store-server/tests/_internal/server/views/test_conda_store_ui.py +++ b/conda-store-server/tests/_internal/server/views/test_conda_store_ui.py @@ -8,7 +8,8 @@ def assert_client_app(response): """A few checks that should all pass if the response is ok and contains - the client app""" + the client app + """ assert response.is_success assert "text/html" in response.headers["content-type"] assert "condaStoreConfig" in response.text @@ -36,7 +37,8 @@ def test_base_route(self, testclient): def test_unknown_routes(self, testclient): """Rather than return a 404, the server should return the client app - and let it handle unknown routes""" + and let it handle unknown routes + """ response = testclient.get(self.full_route("/foo")) assert_client_app(response) @@ -45,7 +47,8 @@ def test_unknown_routes(self, testclient): def test_not_found_route(self, testclient): """The /not-found route should also return the client app - but with a 404 status code""" + but with a 404 status code + """ response = testclient.get(self.full_route("/not-found")) assert response.status_code == 404 assert "text/html" in response.headers["content-type"] @@ -53,7 +56,8 @@ def test_not_found_route(self, testclient): def test_route_outside_ui_app(self, testclient): """The server should not return the client app for a server-side - route""" + route + """ # Trailing slash needed on next line. Is this a problem? response = testclient.get(self.full_route("/admin/")) assert response.is_success @@ -68,19 +72,22 @@ def assert_not_found_not_client_app(response): assert response.status_code == 404 assert "condaStoreConfig" not in response.text + class TestUIRoutesCustomPrefix(TestUIRoutes): url_prefix = "/conda-store" def test_unknown_route_outside_prefix(self, testclient): """The server should return a 404 for an unknown route outside - the url prefix and should not return the client app""" + the url prefix and should not return the client app + """ response = testclient.get("/foo/bar") assert_not_found_not_client_app(response) def test_ui_disabled(conda_store_server): """When the UI is disabled, the server should return 404s for - all UI routes and should not return the client app""" + all UI routes and should not return the client app + """ conda_store_server.enable_ui = False conda_store_server.url_prefix = "/" testclient = TestClient(conda_store_server.init_fastapi_app()) From 5fc2fdc989bc4cf6fb823b80f3b1dd45e98f2e43 Mon Sep 17 00:00:00 2001 From: gabalafou Date: Wed, 13 Nov 2024 15:12:46 +0100 Subject: [PATCH 10/14] Put the React app under /ui --- .../_internal/server/app.py | 19 +++++++++++-------- .../server/templates/conda-store-ui.html | 4 ++-- .../_internal/server/views/conda_store_ui.py | 6 ++++++ .../conda-store-ui/how-tos/configure-ui.md | 2 ++ 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/conda-store-server/conda_store_server/_internal/server/app.py b/conda-store-server/conda_store_server/_internal/server/app.py index 146ce5e34..08cececd3 100644 --- a/conda-store-server/conda_store_server/_internal/server/app.py +++ b/conda-store-server/conda_store_server/_internal/server/app.py @@ -104,7 +104,7 @@ class CondaStoreServer(Application): ) url_prefix = Unicode( - "/", + "/conda-store", help="the prefix URL (subdirectory) for the entire application; " "it MUST start with a forward slash - tip: " "use this to run conda-store within an existing website.", @@ -330,13 +330,16 @@ async def exception_handler(request, exc): name="static", ) - # convenience to redirect "/" to home page when using a prefix - # realistically this url will not be hit with a proxy + prefix - if self.url_prefix != "/": - - @app.get("/") - def redirect_home(request: Request): - return RedirectResponse(request.url_for("get_conda_store_ui")) + # Redirect both "/" and `url_prefix` to the conda-store-ui React app. + # Realistically the "/" will not be hit with a proxy + prefix. + @app.get( + # Yes if url_prefix is "/" then this decorator is redundant but FastAPI doesn't seem to be bothered. + "/" + ) + @app.get(self.url_prefix) + # This function name may be used by url_for() so be careful renaming it + def redirect_root_to_ui(request: Request): + return RedirectResponse(request.url_for("get_conda_store_ui")) @app.get( trim_slash(self.url_prefix) + "/favicon.ico", include_in_schema=False diff --git a/conda-store-server/conda_store_server/_internal/server/templates/conda-store-ui.html b/conda-store-server/conda_store_server/_internal/server/templates/conda-store-ui.html index 03545c27e..b651732c1 100644 --- a/conda-store-server/conda_store_server/_internal/server/templates/conda-store-ui.html +++ b/conda-store-server/conda_store_server/_internal/server/templates/conda-store-ui.html @@ -22,10 +22,10 @@ REACT_APP_AUTH_TOKEN: "", REACT_APP_STYLE_TYPE: "green-accent", REACT_APP_SHOW_LOGIN_ICON: "true", - REACT_APP_API_URL: "{{ url_for('get_conda_store_ui') }}", + REACT_APP_API_URL: "{{ url_for('redirect_root_to_ui') }}", REACT_APP_LOGIN_PAGE_URL: "{{ url_for('get_login_method') }}?next=", REACT_APP_LOGOUT_PAGE_URL: "{{ url_for('post_logout_method') }}?next=/", - REACT_APP_URL_BASENAME: "{{ url_prefix }}" + REACT_APP_URL_BASENAME: "{{ url_prefix.rstrip('/') }}{{ ui_prefix }}" }; diff --git a/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py b/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py index a82d8a72e..e2b31989b 100644 --- a/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py +++ b/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py @@ -8,6 +8,7 @@ from conda_store_server._internal.server import dependencies router_conda_store_ui = APIRouter( + prefix="/ui", tags=["conda-store-ui"], # Provide the response class so that it can show up in the autogenerated API # docs. The docs show the endpoints as returning HTML rather than JSON. @@ -26,9 +27,14 @@ async def get_conda_store_ui( templates=Depends(dependencies.get_templates), url_prefix=Depends(dependencies.get_url_prefix), ): + conda_store_ui_prefix = router_conda_store_ui.prefix + if (url_prefix != "/"): + conda_store_ui_prefix = url_prefix + conda_store_ui_prefix + context = { "request": request, "url_prefix": url_prefix, + "ui_prefix": router_conda_store_ui.prefix } response = templates.TemplateResponse("conda-store-ui.html", context) if path.endswith("not-found"): diff --git a/docusaurus-docs/conda-store-ui/how-tos/configure-ui.md b/docusaurus-docs/conda-store-ui/how-tos/configure-ui.md index 5d1d71670..71a1d271b 100644 --- a/docusaurus-docs/conda-store-ui/how-tos/configure-ui.md +++ b/docusaurus-docs/conda-store-ui/how-tos/configure-ui.md @@ -23,6 +23,7 @@ REACT_APP_AUTH_TOKEN= REACT_APP_STYLE_TYPE=green-accent REACT_APP_SHOW_AUTH_BUTTON=true REACT_APP_LOGOUT_PAGE_URL=http://localhost:8080/conda-store/logout?next=/ +REACT_APP_URL_BASENAME="/conda-store/ui" ``` ### At runtime, using `condaStoreConfig` @@ -40,6 +41,7 @@ In your HTML file, add the following **before** loading the react app : REACT_APP_API_URL: "http://localhost:8080/conda-store", REACT_APP_LOGIN_PAGE_URL: "http://localhost:8080/conda-store/login?next=", REACT_APP_LOGOUT_PAGE_URL: "http://localhost:8080/conda-store/logout?next=/", + REACT_APP_URL_BASENAME="/conda-store/ui", }; ``` From c9f578e42ab31b4b1de3dd63bb35f6dfd698f0ce Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 13 Nov 2024 14:12:59 +0000 Subject: [PATCH 11/14] [pre-commit.ci] Apply automatic pre-commit fixes --- .../_internal/server/views/conda_store_ui.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py b/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py index e2b31989b..4889fca99 100644 --- a/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py +++ b/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py @@ -28,13 +28,13 @@ async def get_conda_store_ui( url_prefix=Depends(dependencies.get_url_prefix), ): conda_store_ui_prefix = router_conda_store_ui.prefix - if (url_prefix != "/"): + if url_prefix != "/": conda_store_ui_prefix = url_prefix + conda_store_ui_prefix context = { "request": request, "url_prefix": url_prefix, - "ui_prefix": router_conda_store_ui.prefix + "ui_prefix": router_conda_store_ui.prefix, } response = templates.TemplateResponse("conda-store-ui.html", context) if path.endswith("not-found"): From ce8d3c2f78eed57d4a8d1153bbaf5208476dd5dd Mon Sep 17 00:00:00 2001 From: gabalafou Date: Wed, 13 Nov 2024 15:14:30 +0100 Subject: [PATCH 12/14] remove unused code --- .../_internal/server/views/conda_store_ui.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py b/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py index 4889fca99..bd60c14ce 100644 --- a/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py +++ b/conda-store-server/conda_store_server/_internal/server/views/conda_store_ui.py @@ -27,10 +27,6 @@ async def get_conda_store_ui( templates=Depends(dependencies.get_templates), url_prefix=Depends(dependencies.get_url_prefix), ): - conda_store_ui_prefix = router_conda_store_ui.prefix - if url_prefix != "/": - conda_store_ui_prefix = url_prefix + conda_store_ui_prefix - context = { "request": request, "url_prefix": url_prefix, From 3bae0cf405504e1859d8194a150089b434ad7a1f Mon Sep 17 00:00:00 2001 From: gabalafou Date: Wed, 13 Nov 2024 15:15:22 +0100 Subject: [PATCH 13/14] undo accidental change --- conda-store-server/conda_store_server/_internal/server/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conda-store-server/conda_store_server/_internal/server/app.py b/conda-store-server/conda_store_server/_internal/server/app.py index 08cececd3..ae003b3aa 100644 --- a/conda-store-server/conda_store_server/_internal/server/app.py +++ b/conda-store-server/conda_store_server/_internal/server/app.py @@ -104,7 +104,7 @@ class CondaStoreServer(Application): ) url_prefix = Unicode( - "/conda-store", + "/", help="the prefix URL (subdirectory) for the entire application; " "it MUST start with a forward slash - tip: " "use this to run conda-store within an existing website.", From 3745e2c2c99365791ea55350ed4af75543ecd650 Mon Sep 17 00:00:00 2001 From: gabalafou Date: Wed, 13 Nov 2024 16:24:13 +0100 Subject: [PATCH 14/14] fix tests --- .../_internal/server/views/test_conda_store_ui.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/conda-store-server/tests/_internal/server/views/test_conda_store_ui.py b/conda-store-server/tests/_internal/server/views/test_conda_store_ui.py index 9e8e32d41..cba77cf18 100644 --- a/conda-store-server/tests/_internal/server/views/test_conda_store_ui.py +++ b/conda-store-server/tests/_internal/server/views/test_conda_store_ui.py @@ -39,17 +39,17 @@ def test_unknown_routes(self, testclient): """Rather than return a 404, the server should return the client app and let it handle unknown routes """ - response = testclient.get(self.full_route("/foo")) + response = testclient.get(self.full_route("/ui/foo")) assert_client_app(response) - response = testclient.get(self.full_route("/foo/bar")) + response = testclient.get(self.full_route("/ui/foo/bar")) assert_client_app(response) def test_not_found_route(self, testclient): """The /not-found route should also return the client app but with a 404 status code """ - response = testclient.get(self.full_route("/not-found")) + response = testclient.get(self.full_route("/ui/not-found")) assert response.status_code == 404 assert "text/html" in response.headers["content-type"] assert "condaStoreConfig" in response.text @@ -58,7 +58,6 @@ def test_route_outside_ui_app(self, testclient): """The server should not return the client app for a server-side route """ - # Trailing slash needed on next line. Is this a problem? response = testclient.get(self.full_route("/admin/")) assert response.is_success assert "condaStoreConfig" not in response.text @@ -80,7 +79,7 @@ def test_unknown_route_outside_prefix(self, testclient): """The server should return a 404 for an unknown route outside the url prefix and should not return the client app """ - response = testclient.get("/foo/bar") + response = testclient.get("/ui/foo/bar") assert_not_found_not_client_app(response) @@ -95,7 +94,7 @@ def test_ui_disabled(conda_store_server): response = testclient.get("/") assert_not_found_not_client_app(response) - response = testclient.get("/not-found") + response = testclient.get("/ui/not-found") assert_not_found_not_client_app(response) response = testclient.get("/admin/")