From 44e24f42a6788880ab0f9e63bcb06e6955f57656 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Thu, 14 Aug 2025 23:16:48 +0100 Subject: [PATCH 1/3] Check webapp is available or give useful error --- .../server/serve_static_files.py | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/src/openflexure_microscope_server/server/serve_static_files.py b/src/openflexure_microscope_server/server/serve_static_files.py index 581a1824..360f035c 100644 --- a/src/openflexure_microscope_server/server/serve_static_files.py +++ b/src/openflexure_microscope_server/server/serve_static_files.py @@ -8,6 +8,7 @@ from fastapi.staticfiles import StaticFiles from fastapi import FastAPI THIS_DIR = os.path.dirname(os.path.abspath(__file__)) +STATIC_PATH = os.path.normpath(os.path.join(THIS_DIR, "..", "static")) NO_CACHE_HEADERS = { "Cache-Control": "no-store, no-cache, must-revalidate, max-age=0", @@ -40,17 +41,17 @@ def add_static_files(app: FastAPI, scans_folder: Optional[str]) -> None: app: The FastAPI app to add to, in this case the OpenFlexure server """ - static_path = os.path.normpath(os.path.join(THIS_DIR, "..", "static")) + check_static_dir() @app.get("/", response_class=RedirectResponse) async def redirect_fastapi(): return "/index.html" # Mounting the webapp at / file by file to allow other endpoints to be created - for fname in os.listdir(static_path): - fpath = os.path.join(static_path, fname) + for fname in os.listdir(STATIC_PATH): + fpath = os.path.join(STATIC_PATH, fname) if os.path.isfile(fpath): - add_static_file(app, fname, static_path) + add_static_file(app, fname, STATIC_PATH) elif os.path.isdir(fpath): app.mount( f"/{fname}/", @@ -68,3 +69,22 @@ def add_static_files(app: FastAPI, scans_folder: Optional[str]) -> None: StaticFiles(directory=scans_folder), name="scans", ) + + +def check_static_dir(): + """Check that the static dir exists and contains expected files and dirs.""" + if not os.path.isdir(STATIC_PATH): + raise FileNotFoundError( + "The static directory does not exist. You will need to pull or compile the" + "web app." + ) + expected = [ + os.path.isdir(os.path.join(STATIC_PATH, "js")), + os.path.isdir(os.path.join(STATIC_PATH, "css")), + os.path.isfile(os.path.join(STATIC_PATH, "index.html")), + ] + if not all(expected): + raise FileNotFoundError( + "The static directory does not contain a valid web app. You will need to " + "pull or compile the web app." + ) From db41d673fbf749c9ff2c92ed4e850214da93048c Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Fri, 15 Aug 2025 09:21:31 +0100 Subject: [PATCH 2/3] Add tests for static directory checking --- tests/test_serve_static_files.py | 64 ++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/tests/test_serve_static_files.py b/tests/test_serve_static_files.py index c60cff95..0ed4f766 100644 --- a/tests/test_serve_static_files.py +++ b/tests/test_serve_static_files.py @@ -1,7 +1,12 @@ """Test the code that mounts static files to the server, without creating a server.""" +import os +import shutil +import tempfile + import pytest from starlette.responses import FileResponse + from openflexure_microscope_server.server import serve_static_files @@ -17,6 +22,20 @@ def mock_app(mocker): return app +@pytest.fixture +def mock_static_dir(): + """Return the path of a mock static directory. + + It contains enough files to be recognised as a webapp. + """ + with tempfile.TemporaryDirectory() as tmpdir: + os.makedirs(os.path.join(tmpdir, "js")) + os.makedirs(os.path.join(tmpdir, "css")) + with open(os.path.join(tmpdir, "index.html"), "w", encoding="utf-8") as f_obj: + f_obj.write("mock") + yield tmpdir + + @pytest.mark.parametrize( "filename, allow_cache", [ @@ -60,3 +79,48 @@ def test_add_static_file(filename, allow_cache, mock_app): # Check headers that refuse caching are present assert key in response.headers assert response.headers[key] == value + + +def test_add_static_with_no_static_dir(mock_app, mocker): + """Test that a FileNotFound error is raised if the static dir does not exist.""" + # Mock STATIC_PATH to something that doesn't exist + mocker.patch( + "openflexure_microscope_server.server.serve_static_files.STATIC_PATH", + "fakepath", + ) + # Should raise as the mock static dir does not exist + with pytest.raises(FileNotFoundError): + serve_static_files.add_static_files(mock_app, scans_folder=None) + assert mock_app.get.call_count == 0 + + +def test_check_static_dir(mocker, mock_static_dir): + """Test check_static_dir recognises a basic webapp structure.""" + mocker.patch( + "openflexure_microscope_server.server.serve_static_files.STATIC_PATH", + mock_static_dir, + ) + # First run shouldn't error as the mock dir has enough files. + serve_static_files.check_static_dir() + + js_path = os.path.join(mock_static_dir, "js") + index_path = os.path.join(mock_static_dir, "index.html") + + # Remove javascript dir and check error. + shutil.rmtree(js_path) + with pytest.raises(FileNotFoundError): + serve_static_files.check_static_dir() + # replace javascript dir with file, it should still error. + shutil.copyfile(index_path, js_path) + with pytest.raises(FileNotFoundError): + serve_static_files.check_static_dir() + + # Return it to a folder and check everything works again + os.remove(js_path) + os.makedirs(js_path) + serve_static_files.check_static_dir() + + # But it errors again if index.html is removed. + os.remove(index_path) + with pytest.raises(FileNotFoundError): + serve_static_files.check_static_dir() From 3289221c6b52a6c617453964d48a1a5070e9d4cd Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Fri, 15 Aug 2025 10:33:30 +0100 Subject: [PATCH 3/3] Add unit tests for all of serving static files. Update docs in src to clarify behaviour --- .../server/serve_static_files.py | 10 ++- tests/test_serve_static_files.py | 62 ++++++++++++++++++- 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/src/openflexure_microscope_server/server/serve_static_files.py b/src/openflexure_microscope_server/server/serve_static_files.py index 360f035c..5defc149 100644 --- a/src/openflexure_microscope_server/server/serve_static_files.py +++ b/src/openflexure_microscope_server/server/serve_static_files.py @@ -28,7 +28,7 @@ def add_static_file(app: FastAPI, fname: str, folder: str) -> None: folder: the containing folder of the file """ file_path = os.path.join(folder, fname) - # Cache font files, but not other static files. + # Cache font files, but not other static files in the root static directory. headers = {} if fname.endswith(".woff2") else NO_CACHE_HEADERS app.get(f"/{fname}", response_class=FileResponse, include_in_schema=False)( @@ -39,7 +39,13 @@ def add_static_file(app: FastAPI, fname: str, folder: str) -> None: def add_static_files(app: FastAPI, scans_folder: Optional[str]) -> None: """Add the static files responsible for the webapp app to the FastAPI app. - app: The FastAPI app to add to, in this case the OpenFlexure server + Note that any file in the root of the static dir will not be cached. However, the + files in mounted subdirectories are not sent with no-cache headers. + The Vue CSS and JS are hashed, so if updated their filename will update. The most + important file not to cache is "index.html". + + :param app: The FastAPI app to add to, in this case the OpenFlexure server + :param scans_folder: The directory for the scans. """ check_static_dir() diff --git a/tests/test_serve_static_files.py b/tests/test_serve_static_files.py index 0ed4f766..53413448 100644 --- a/tests/test_serve_static_files.py +++ b/tests/test_serve_static_files.py @@ -3,9 +3,10 @@ import os import shutil import tempfile +import asyncio import pytest -from starlette.responses import FileResponse +from starlette.responses import FileResponse, RedirectResponse from openflexure_microscope_server.server import serve_static_files @@ -51,6 +52,7 @@ def test_add_static_file(filename, allow_cache, mock_app): This should creates a wrapped function that returns a FileResponse, this should be have the path on disk and the no cache headers if not a woff2 font. """ + # Get the wrapper function from the mocked decorator wrapper = mock_app.get.return_value serve_static_files.add_static_file(mock_app, fname=filename, folder="bar") @@ -94,7 +96,7 @@ def test_add_static_with_no_static_dir(mock_app, mocker): assert mock_app.get.call_count == 0 -def test_check_static_dir(mocker, mock_static_dir): +def test_check_static_dir(mock_static_dir, mocker): """Test check_static_dir recognises a basic webapp structure.""" mocker.patch( "openflexure_microscope_server.server.serve_static_files.STATIC_PATH", @@ -124,3 +126,59 @@ def test_check_static_dir(mocker, mock_static_dir): os.remove(index_path) with pytest.raises(FileNotFoundError): serve_static_files.check_static_dir() + + +def test_add_static_files(mock_app, mock_static_dir, mocker): + """Check add_static_files mounts the internal webapp dirs, and sets endpoints for files.""" + mocker.patch( + "openflexure_microscope_server.server.serve_static_files.STATIC_PATH", + mock_static_dir, + ) + # Get the wrapper function from the mocked decorator + wrapper = mock_app.get.return_value + serve_static_files.add_static_files(mock_app, scans_folder=None) + + # Get should have been called twice to create a route for index + assert mock_app.get.call_count == 2 + # Once at root as a redirict + assert mock_app.get.call_args_list[0].args[0] == "/" + assert mock_app.get.call_args_list[0].kwargs["response_class"] == RedirectResponse + # Once at index.html as a file response + assert mock_app.get.call_args_list[1].args[0] == "/index.html" + assert mock_app.get.call_args_list[1].kwargs["response_class"] == FileResponse + + # The wrapper was also called twice to wrap two different functions + assert wrapper.call_count == 2 + first_wrapped = wrapper.call_args_list[0].args[0] + second_wrapped = wrapper.call_args_list[1].args[0] + + # The first wrapped function is the redirect to "index.html", it is async + assert asyncio.run(first_wrapped()) == "/index.html" + # The second is the file response for index.html, the path should be to the file on + # disk + assert second_wrapped().path == os.path.join(mock_static_dir, "index.html") + # It shouldn't cache + assert second_wrapped().headers["Pragma"] == "no-cache" + + # Also should have mounted both dirs + assert mock_app.mount.call_count == 2 + mounted_paths = [call.args[0] for call in mock_app.mount.call_args_list] + mounted_dirs = [call.args[1].directory for call in mock_app.mount.call_args_list] + assert "/css/" in mounted_paths + assert "/js/" in mounted_paths + assert os.path.join(mock_static_dir, "css") in mounted_dirs + assert os.path.join(mock_static_dir, "js") in mounted_dirs + + +def test_add_static_files_with_scan_dir(mock_app, mock_static_dir, mocker): + """Check the scan dir mounts if supplied.""" + mocker.patch( + "openflexure_microscope_server.server.serve_static_files.STATIC_PATH", + mock_static_dir, + ) + with tempfile.TemporaryDirectory() as scandir: + serve_static_files.add_static_files(mock_app, scans_folder=scandir) + # The scan dir should be the last to mount so can use call args. It should mount + # at /scans/ + assert mock_app.mount.call_args.args[0] == "/scans/" + assert mock_app.mount.call_args.args[1].directory == scandir