Merge branch 'check-static-files' into 'v3'

Check static files exist or throw a useful error

See merge request openflexure/openflexure-microscope-server!368

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",
@@ -27,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)(
@@ -38,19 +39,25 @@ 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.
     """
-    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 +75,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."
+        )

tests/test_serve_static_files.py

@@ -1,7 +1,13 @@
 """Test the code that mounts static files to the server, without creating a server."""
 
+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
 
 
@@ -17,6 +23,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>mock</mock>")
+        yield tmpdir
+
+
 @pytest.mark.parametrize(
     "filename, allow_cache",
     [
@@ -32,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")
 
@@ -60,3 +81,104 @@ 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(mock_static_dir, mocker):
+    """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()
+
+
+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