From fd8c038eda21a812fc78a9bccf4b8d716fdedaab Mon Sep 17 00:00:00 2001 From: Sam Date: Thu, 5 Feb 2026 16:20:54 +0000 Subject: [PATCH 1/7] Add gzip compression middleware and IMAT image endpoints for outputs --- .../plotting_service/plotting_api.py | 82 ++++++++++++++++++- plotting-service/plotting_service/utils.py | 14 +++- 2 files changed, 92 insertions(+), 4 deletions(-) diff --git a/plotting-service/plotting_service/plotting_api.py b/plotting-service/plotting_service/plotting_api.py index 555136a..e5e14b6 100644 --- a/plotting-service/plotting_service/plotting_api.py +++ b/plotting-service/plotting_service/plotting_api.py @@ -7,10 +7,13 @@ from http import HTTPStatus from pathlib import Path -from fastapi import FastAPI, HTTPException +from fastapi import FastAPI, HTTPException, Query +from starlette.middleware.gzip import GZipMiddleware +from PIL import Image from h5grove.fastapi_utils import router, settings # type: ignore from starlette.middleware.cors import CORSMiddleware from starlette.requests import Request +from starlette.responses import JSONResponse, PlainTextResponse, Response from plotting_service.auth import get_experiments_for_user, get_user_from_token from plotting_service.exceptions import AuthError @@ -48,6 +51,7 @@ allow_methods=["*"], allow_headers=["*"], ) +app.add_middleware(GZipMiddleware, minimum_size=1000) CEPH_DIR = os.environ.get("CEPH_DIR", "/ceph") logger.info("Setting ceph directory to %s", CEPH_DIR) @@ -185,6 +189,82 @@ async def check_live_permissions(request: Request, call_next: typing.Callable[.. logger.warning(f"User {user.user_number} denied access to live experiment {current_rb_int}") raise HTTPException(HTTPStatus.FORBIDDEN, detail="Forbidden: You do not have access to the current live experiment") +@app.get("/imat/list-images", summary="List images in a directory") +async def list_imat_images( + path: str = Query(..., description="Path to the directory containing images, relative to CEPH_DIR") +) -> list[str]: + """Return a sorted list of TIFF images in the given directory.""" + from plotting_service.utils import safe_check_filepath + + dir_path = (Path(CEPH_DIR) / path).resolve() + # Security: Ensure path is within CEPH_DIR + safe_check_filepath(dir_path, CEPH_DIR) + + if not dir_path.exists() or not dir_path.is_dir(): + raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Directory not found") + + images = [] + for entry in dir_path.iterdir(): + if entry.is_file() and entry.suffix.lower() in IMAGE_SUFFIXES: + images.append(entry.name) + + return sorted(images) + + +@app.get("/imat/image", summary="Fetch a specific TIFF image as raw data") +async def get_imat_image( + path: str = Query(..., description="Path to the TIFF image file, relative to CEPH_DIR"), + downsample_factor: typing.Annotated[ + int, + Query( + ge=1, + le=64, + description="Integer factor to reduce each dimension by (1 keeps original resolution).", + ), + ] = 1, +) -> Response: + """Return the raw data of a specific TIFF image as binary.""" + from plotting_service.utils import safe_check_filepath + + image_path = (Path(CEPH_DIR) / path).resolve() + # Security: Ensure path is within CEPH_DIR + safe_check_filepath(image_path, CEPH_DIR) + + if not image_path.exists() or not image_path.is_file(): + raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Image not found") + + try: + with Image.open(image_path) as img: + original_width, original_height = img.size + + if downsample_factor > 1: + target_width = max(1, round(original_width / downsample_factor)) + target_height = max(1, round(original_height / downsample_factor)) + img = img.resize((target_width, target_height), Image.Resampling.NEAREST) + + sampled_width, sampled_height = img.size + # For 16-bit TIFFs, tobytes() returns raw 16-bit bytes + data_bytes = img.tobytes() + + headers = { + "X-Image-Width": str(sampled_width), + "X-Image-Height": str(sampled_height), + "X-Original-Width": str(original_width), + "X-Original-Height": str(original_height), + "X-Downsample-Factor": str(downsample_factor), + "Access-Control-Expose-Headers": "X-Image-Width, X-Image-Height, X-Original-Width, X-Original-Height, X-Downsample-Factor" + } + + return Response( + content=data_bytes, + media_type="application/octet-stream", + headers=headers + ) + + except Exception as exc: + logger.error(f"Failed to process image {image_path}: {exc}") + raise HTTPException(HTTPStatus.INTERNAL_SERVER_ERROR, "Unable to process image") from exc + app.include_router(router) app.include_router(HealthRouter) diff --git a/plotting-service/plotting_service/utils.py b/plotting-service/plotting_service/utils.py index e5ac558..3baf1c7 100644 --- a/plotting-service/plotting_service/utils.py +++ b/plotting-service/plotting_service/utils.py @@ -35,9 +35,17 @@ def safe_check_filepath(filepath: Path, base_path: str) -> None: :param base_path: base path to check against :return: """ - filepath.resolve(strict=True) - if not filepath.is_relative_to(base_path): - raise HTTPException(status_code=HTTPStatus.FORBIDDEN, detail="Invalid path being accessed.") + try: + filepath.resolve(strict=True) + if not filepath.is_relative_to(base_path): + raise HTTPException(status_code=HTTPStatus.FORBIDDEN, detail="Invalid path being accessed.") + except FileNotFoundError as err: + # pathlibs is_file and is_dir do not work on non existent paths + # If it's a file with an extension, check its parent + if "." in filepath.name: + safe_check_filepath(filepath.parent, base_path) + else: + raise err def _safe_find_file_in_dir(dir_path: Path, base_path: str, filename: str) -> Path | None: From 1a4fb522d77eb9c54ef08897d47606cb3e0b3828 Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 6 Feb 2026 16:32:59 +0000 Subject: [PATCH 2/7] Manual Refactor of endpoints to correct router post revase --- .../plotting_service/plotting_api.py | 80 +----------------- .../plotting_service/routers/imat.py | 84 ++++++++++++++++++- 2 files changed, 84 insertions(+), 80 deletions(-) diff --git a/plotting-service/plotting_service/plotting_api.py b/plotting-service/plotting_service/plotting_api.py index e5e14b6..f143e72 100644 --- a/plotting-service/plotting_service/plotting_api.py +++ b/plotting-service/plotting_service/plotting_api.py @@ -7,13 +7,11 @@ from http import HTTPStatus from pathlib import Path -from fastapi import FastAPI, HTTPException, Query +from fastapi import FastAPI, HTTPException from starlette.middleware.gzip import GZipMiddleware -from PIL import Image from h5grove.fastapi_utils import router, settings # type: ignore from starlette.middleware.cors import CORSMiddleware from starlette.requests import Request -from starlette.responses import JSONResponse, PlainTextResponse, Response from plotting_service.auth import get_experiments_for_user, get_user_from_token from plotting_service.exceptions import AuthError @@ -189,82 +187,6 @@ async def check_live_permissions(request: Request, call_next: typing.Callable[.. logger.warning(f"User {user.user_number} denied access to live experiment {current_rb_int}") raise HTTPException(HTTPStatus.FORBIDDEN, detail="Forbidden: You do not have access to the current live experiment") -@app.get("/imat/list-images", summary="List images in a directory") -async def list_imat_images( - path: str = Query(..., description="Path to the directory containing images, relative to CEPH_DIR") -) -> list[str]: - """Return a sorted list of TIFF images in the given directory.""" - from plotting_service.utils import safe_check_filepath - - dir_path = (Path(CEPH_DIR) / path).resolve() - # Security: Ensure path is within CEPH_DIR - safe_check_filepath(dir_path, CEPH_DIR) - - if not dir_path.exists() or not dir_path.is_dir(): - raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Directory not found") - - images = [] - for entry in dir_path.iterdir(): - if entry.is_file() and entry.suffix.lower() in IMAGE_SUFFIXES: - images.append(entry.name) - - return sorted(images) - - -@app.get("/imat/image", summary="Fetch a specific TIFF image as raw data") -async def get_imat_image( - path: str = Query(..., description="Path to the TIFF image file, relative to CEPH_DIR"), - downsample_factor: typing.Annotated[ - int, - Query( - ge=1, - le=64, - description="Integer factor to reduce each dimension by (1 keeps original resolution).", - ), - ] = 1, -) -> Response: - """Return the raw data of a specific TIFF image as binary.""" - from plotting_service.utils import safe_check_filepath - - image_path = (Path(CEPH_DIR) / path).resolve() - # Security: Ensure path is within CEPH_DIR - safe_check_filepath(image_path, CEPH_DIR) - - if not image_path.exists() or not image_path.is_file(): - raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Image not found") - - try: - with Image.open(image_path) as img: - original_width, original_height = img.size - - if downsample_factor > 1: - target_width = max(1, round(original_width / downsample_factor)) - target_height = max(1, round(original_height / downsample_factor)) - img = img.resize((target_width, target_height), Image.Resampling.NEAREST) - - sampled_width, sampled_height = img.size - # For 16-bit TIFFs, tobytes() returns raw 16-bit bytes - data_bytes = img.tobytes() - - headers = { - "X-Image-Width": str(sampled_width), - "X-Image-Height": str(sampled_height), - "X-Original-Width": str(original_width), - "X-Original-Height": str(original_height), - "X-Downsample-Factor": str(downsample_factor), - "Access-Control-Expose-Headers": "X-Image-Width, X-Image-Height, X-Original-Width, X-Original-Height, X-Downsample-Factor" - } - - return Response( - content=data_bytes, - media_type="application/octet-stream", - headers=headers - ) - - except Exception as exc: - logger.error(f"Failed to process image {image_path}: {exc}") - raise HTTPException(HTTPStatus.INTERNAL_SERVER_ERROR, "Unable to process image") from exc - app.include_router(router) app.include_router(HealthRouter) diff --git a/plotting-service/plotting_service/routers/imat.py b/plotting-service/plotting_service/routers/imat.py index 4210561..21bc4f7 100644 --- a/plotting-service/plotting_service/routers/imat.py +++ b/plotting-service/plotting_service/routers/imat.py @@ -7,16 +7,21 @@ from pathlib import Path from fastapi import APIRouter, HTTPException, Query -from starlette.responses import JSONResponse +from starlette.responses import JSONResponse, Response +from PIL import Image + from plotting_service.services.image_service import ( convert_image_to_rgb_array, find_latest_image_in_directory, + IMAGE_SUFFIXES ) + ImatRouter = APIRouter() IMAT_DIR: Path = Path(os.getenv("IMAT_DIR", "/imat")).resolve() +CEPH_DIR = os.environ.get("CEPH_DIR", "/ceph") stdout_handler = logging.StreamHandler(stream=sys.stdout) logging.basicConfig( @@ -87,3 +92,80 @@ async def get_latest_imat_image( "downsampleFactor": effective_downsample, } return JSONResponse(payload) + + +@ImatRouter.get("/imat/list-images", summary="List images in a directory") +async def list_imat_images( + path: str = Query(..., description="Path to the directory containing images, relative to CEPH_DIR") +) -> list[str]: + """Return a sorted list of TIFF images in the given directory.""" + from plotting_service.utils import safe_check_filepath + + dir_path = (Path(CEPH_DIR) / path).resolve() + # Security: Ensure path is within CEPH_DIR + safe_check_filepath(dir_path, CEPH_DIR) + + if not dir_path.exists() or not dir_path.is_dir(): + raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Directory not found") + + images = [] + for entry in dir_path.iterdir(): + if entry.is_file() and entry.suffix.lower() in IMAGE_SUFFIXES: + images.append(entry.name) + + return sorted(images) + + +@ImatRouter.get("/imat/image", summary="Fetch a specific TIFF image as raw data") +async def get_imat_image( + path: str = Query(..., description="Path to the TIFF image file, relative to CEPH_DIR"), + downsample_factor: typing.Annotated[ + int, + Query( + ge=1, + le=64, + description="Integer factor to reduce each dimension by (1 keeps original resolution).", + ), + ] = 1, +) -> Response: + """Return the raw data of a specific TIFF image as binary.""" + from plotting_service.utils import safe_check_filepath + + image_path = (Path(CEPH_DIR) / path).resolve() + # Security: Ensure path is within CEPH_DIR + safe_check_filepath(image_path, CEPH_DIR) + + if not image_path.exists() or not image_path.is_file(): + raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Image not found") + + try: + with Image.open(image_path) as img: + original_width, original_height = img.size + + if downsample_factor > 1: + target_width = max(1, round(original_width / downsample_factor)) + target_height = max(1, round(original_height / downsample_factor)) + img = img.resize((target_width, target_height), Image.Resampling.NEAREST) + + sampled_width, sampled_height = img.size + # For 16-bit TIFFs, tobytes() returns raw 16-bit bytes + data_bytes = img.tobytes() + + headers = { + "X-Image-Width": str(sampled_width), + "X-Image-Height": str(sampled_height), + "X-Original-Width": str(original_width), + "X-Original-Height": str(original_height), + "X-Downsample-Factor": str(downsample_factor), + "Access-Control-Expose-Headers": "X-Image-Width, X-Image-Height, X-Original-Width, X-Original-Height, X-Downsample-Factor" + } + + return Response( + content=data_bytes, + media_type="application/octet-stream", + headers=headers + ) + + except Exception as exc: + logger.error(f"Failed to process image {image_path}: {exc}") + raise HTTPException(HTTPStatus.INTERNAL_SERVER_ERROR, "Unable to process image") from exc From c7f1e98427bfb00e8149f66900d78fe6f779d698 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 9 Feb 2026 13:51:38 +0000 Subject: [PATCH 3/7] finalise generated code --- .../plotting_service/routers/imat.py | 10 +- plotting-service/plotting_service/utils.py | 14 +- plotting-service/test/test_plotting_api.py | 179 ++++++++++++++++++ plotting-service/test/test_utils.py | 2 +- 4 files changed, 191 insertions(+), 14 deletions(-) diff --git a/plotting-service/plotting_service/routers/imat.py b/plotting-service/plotting_service/routers/imat.py index 21bc4f7..707f6b3 100644 --- a/plotting-service/plotting_service/routers/imat.py +++ b/plotting-service/plotting_service/routers/imat.py @@ -103,7 +103,10 @@ async def list_imat_images( dir_path = (Path(CEPH_DIR) / path).resolve() # Security: Ensure path is within CEPH_DIR - safe_check_filepath(dir_path, CEPH_DIR) + try: + safe_check_filepath(dir_path, CEPH_DIR) + except FileNotFoundError: + raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Directory not found") if not dir_path.exists() or not dir_path.is_dir(): raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Directory not found") @@ -133,7 +136,10 @@ async def get_imat_image( image_path = (Path(CEPH_DIR) / path).resolve() # Security: Ensure path is within CEPH_DIR - safe_check_filepath(image_path, CEPH_DIR) + try: + safe_check_filepath(image_path, CEPH_DIR) + except FileNotFoundError: + raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Directory not found") if not image_path.exists() or not image_path.is_file(): raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Image not found") diff --git a/plotting-service/plotting_service/utils.py b/plotting-service/plotting_service/utils.py index 3baf1c7..e5ac558 100644 --- a/plotting-service/plotting_service/utils.py +++ b/plotting-service/plotting_service/utils.py @@ -35,17 +35,9 @@ def safe_check_filepath(filepath: Path, base_path: str) -> None: :param base_path: base path to check against :return: """ - try: - filepath.resolve(strict=True) - if not filepath.is_relative_to(base_path): - raise HTTPException(status_code=HTTPStatus.FORBIDDEN, detail="Invalid path being accessed.") - except FileNotFoundError as err: - # pathlibs is_file and is_dir do not work on non existent paths - # If it's a file with an extension, check its parent - if "." in filepath.name: - safe_check_filepath(filepath.parent, base_path) - else: - raise err + filepath.resolve(strict=True) + if not filepath.is_relative_to(base_path): + raise HTTPException(status_code=HTTPStatus.FORBIDDEN, detail="Invalid path being accessed.") def _safe_find_file_in_dir(dir_path: Path, base_path: str, filename: str) -> Path | None: diff --git a/plotting-service/test/test_plotting_api.py b/plotting-service/test/test_plotting_api.py index 6b918fd..b9fb074 100644 --- a/plotting-service/test/test_plotting_api.py +++ b/plotting-service/test/test_plotting_api.py @@ -204,3 +204,182 @@ def test_get_latest_imat_image_with_mock_rb_folder(tmp_path, monkeypatch): assert payload["shape"] == [2, 4, 3] assert payload["downsampleFactor"] == 2 # noqa: PLR2004 assert payload["data"] == expected_bytes + + +def test_list_imat_images(tmp_path, monkeypatch): + """Verify that /imat/list-images correctly filters and sorts image files.""" + monkeypatch.setattr(imat, "CEPH_DIR", str(tmp_path)) + + # Create test directory + data_dir = tmp_path / "test_data" + data_dir.mkdir() + (data_dir / "image2.tiff").touch() + (data_dir / "image1.tif").touch() + (data_dir / "not_an_image.txt").touch() + + client = TestClient(plotting_api.app) + response = client.get( + "/imat/list-images", + params={"path": "test_data"}, + headers={"Authorization": "Bearer foo"} + ) + + assert response.status_code == HTTPStatus.OK + assert response.json() == ["image1.tif", "image2.tiff"] + + +def test_list_imat_images_not_found(tmp_path, monkeypatch): + """Ensure 404 is returned when the requested directory does not exist.""" + monkeypatch.setattr(imat, "CEPH_DIR", str(tmp_path)) + + client = TestClient(plotting_api.app) + response = client.get( + "/imat/list-images", + params={"path": "non_existent"}, + headers={"Authorization": "Bearer foo"} + ) + + assert response.status_code == HTTPStatus.NOT_FOUND + + +def test_list_imat_images_forbidden(tmp_path, monkeypatch): + """Verify that path traversal attempts are blocked with 403 Forbidden.""" + monkeypatch.setattr(imat, "CEPH_DIR", str(tmp_path)) + + client = TestClient(plotting_api.app) + # safe_check_filepath should block this + response = client.get( + "/imat/list-images", + params={"path": "../.."}, + headers={"Authorization": "Bearer foo"} + ) + + assert response.status_code == HTTPStatus.FORBIDDEN + + +def test_get_imat_image(tmp_path, monkeypatch): + """Ensure /imat/image returns raw binary data and correct metadata headers.""" + monkeypatch.setattr(imat, "CEPH_DIR", str(tmp_path)) + + # Create a tiny 16-bit TIFF (4x4, all 1000) + image_path = tmp_path / "test.tif" + image = Image.new("I;16", (4, 4), color=1000) + image.save(image_path, format="TIFF") + image.close() + + client = TestClient(plotting_api.app) + response = client.get( + "/imat/image", + params={"path": "test.tif"}, + headers={"Authorization": "Bearer foo"} + ) + + assert response.status_code == HTTPStatus.OK + assert response.headers["X-Image-Width"] == "4" + assert response.headers["X-Image-Height"] == "4" + assert response.headers["X-Original-Width"] == "4" + assert response.headers["X-Original-Height"] == "4" + assert response.headers["X-Downsample-Factor"] == "1" + assert response.content == Image.open(image_path).tobytes() + assert response.headers["Content-Type"] == "application/octet-stream" + + +def test_get_imat_image_downsampled(tmp_path, monkeypatch): + """Verify that downsampling works and headers reflect the sampled dimensions.""" + monkeypatch.setattr(imat, "CEPH_DIR", str(tmp_path)) + + image_path = tmp_path / "test.tif" + image = Image.new("I;16", (8, 4), color=1000) + image.save(image_path, format="TIFF") + image.close() + + client = TestClient(plotting_api.app) + response = client.get( + "/imat/image", + params={"path": "test.tif", "downsample_factor": 2}, + headers={"Authorization": "Bearer foo"} + ) + + assert response.status_code == HTTPStatus.OK + assert response.headers["X-Image-Width"] == "4" + assert response.headers["X-Image-Height"] == "2" + assert response.headers["X-Original-Width"] == "8" + assert response.headers["X-Original-Height"] == "4" + + +def test_get_latest_imat_image_no_rb_folders(tmp_path, monkeypatch): + """Ensure 404 is returned if no RB folders are present in the IMAT directory.""" + monkeypatch.setattr(imat, "IMAT_DIR", tmp_path) + + client = TestClient(plotting_api.app) + response = client.get( + "/imat/latest-image", + headers={"Authorization": "Bearer foo"} + ) + + assert response.status_code == HTTPStatus.NOT_FOUND + assert "No RB folders" in response.json()["detail"] + + +def test_get_imat_image_not_found(tmp_path, monkeypatch): + """Ensure /imat/image returns 404 for a missing file.""" + monkeypatch.setattr(imat, "CEPH_DIR", str(tmp_path)) + + client = TestClient(plotting_api.app) + response = client.get( + "/imat/image", + params={"path": "not_there.tif"}, + headers={"Authorization": "Bearer foo"} + ) + + assert response.status_code == HTTPStatus.NOT_FOUND + + +def test_get_latest_imat_image_no_images_in_rb(tmp_path, monkeypatch): + """Ensure 404 is returned if RB folders exist but contain no valid image files.""" + monkeypatch.setattr(imat, "IMAT_DIR", tmp_path) + (tmp_path / "RB1234").mkdir() + + client = TestClient(plotting_api.app) + response = client.get( + "/imat/latest-image", + headers={"Authorization": "Bearer foo"} + ) + assert response.status_code == HTTPStatus.NOT_FOUND + assert "No images found" in response.json()["detail"] + + +def test_get_imat_image_internal_error(tmp_path, monkeypatch): + """Verify that an exception during image processing returns a 500 error.""" + monkeypatch.setattr(imat, "CEPH_DIR", str(tmp_path)) + (tmp_path / "corrupt.tif").touch() + + client = TestClient(plotting_api.app) + with mock.patch("PIL.Image.open", side_effect=Exception("Simulated failure")): + response = client.get( + "/imat/image", + params={"path": "corrupt.tif"}, + headers={"Authorization": "Bearer foo"} + ) + + assert response.status_code == HTTPStatus.INTERNAL_SERVER_ERROR + assert "Unable to process image" in response.json()["detail"] + + +def test_get_latest_imat_image_conversion_error(tmp_path, monkeypatch): + """Verify that an error during RB latest image conversion returns a 500 error.""" + monkeypatch.setattr(imat, "IMAT_DIR", tmp_path) + rb_dir = tmp_path / "RB1234" + rb_dir.mkdir() + (rb_dir / "test.tif").touch() + + client = TestClient(plotting_api.app) + with mock.patch("plotting_service.routers.imat.convert_image_to_rgb_array", side_effect=Exception("Conversion failed")): + response = client.get( + "/imat/latest-image", + params={"downsample_factor": 1}, + headers={"Authorization": "Bearer foo"} + ) + + assert response.status_code == HTTPStatus.INTERNAL_SERVER_ERROR + assert "Unable to convert IMAT image" in response.json()["detail"] diff --git a/plotting-service/test/test_utils.py b/plotting-service/test/test_utils.py index 4fd991a..9b2b652 100644 --- a/plotting-service/test/test_utils.py +++ b/plotting-service/test/test_utils.py @@ -31,7 +31,7 @@ def _setup_and_clean_temp_dir(): [ (Path(CEPH_DIR) / "good" / "path" / "here" / "file.txt", True), (Path(CEPH_DIR) / "bad" / "path" / "here" / "file.txt", False), - (Path(CEPH_DIR) / ".." / ".." / ".." / "file.txt", False), + (Path(CEPH_DIR) / ".." / ".." / "file.txt", False), ], ) def test_safe_check_filepath(filepath_to_check: Path, result: bool): From ed74086ccfd234c5bc183c84cf96fb28750da02a Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 9 Feb 2026 14:11:27 +0000 Subject: [PATCH 4/7] Undo changes to test_utils --- plotting-service/test/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plotting-service/test/test_utils.py b/plotting-service/test/test_utils.py index 9b2b652..4fd991a 100644 --- a/plotting-service/test/test_utils.py +++ b/plotting-service/test/test_utils.py @@ -31,7 +31,7 @@ def _setup_and_clean_temp_dir(): [ (Path(CEPH_DIR) / "good" / "path" / "here" / "file.txt", True), (Path(CEPH_DIR) / "bad" / "path" / "here" / "file.txt", False), - (Path(CEPH_DIR) / ".." / ".." / "file.txt", False), + (Path(CEPH_DIR) / ".." / ".." / ".." / "file.txt", False), ], ) def test_safe_check_filepath(filepath_to_check: Path, result: bool): From fd05bb1697449bcca6319020147849af4fb95516 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 9 Feb 2026 14:24:45 +0000 Subject: [PATCH 5/7] Ruff and Mypy fixes --- .../plotting_service/plotting_api.py | 2 +- .../plotting_service/routers/imat.py | 37 ++++++++++--------- plotting-service/test/test_plotting_api.py | 4 +- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/plotting-service/plotting_service/plotting_api.py b/plotting-service/plotting_service/plotting_api.py index f143e72..84fec70 100644 --- a/plotting-service/plotting_service/plotting_api.py +++ b/plotting-service/plotting_service/plotting_api.py @@ -8,9 +8,9 @@ from pathlib import Path from fastapi import FastAPI, HTTPException -from starlette.middleware.gzip import GZipMiddleware from h5grove.fastapi_utils import router, settings # type: ignore from starlette.middleware.cors import CORSMiddleware +from starlette.middleware.gzip import GZipMiddleware from starlette.requests import Request from plotting_service.auth import get_experiments_for_user, get_user_from_token diff --git a/plotting-service/plotting_service/routers/imat.py b/plotting-service/plotting_service/routers/imat.py index 707f6b3..4dbe8d3 100644 --- a/plotting-service/plotting_service/routers/imat.py +++ b/plotting-service/plotting_service/routers/imat.py @@ -7,17 +7,15 @@ from pathlib import Path from fastapi import APIRouter, HTTPException, Query -from starlette.responses import JSONResponse, Response from PIL import Image - +from starlette.responses import JSONResponse, Response from plotting_service.services.image_service import ( + IMAGE_SUFFIXES, convert_image_to_rgb_array, find_latest_image_in_directory, - IMAGE_SUFFIXES ) - ImatRouter = APIRouter() IMAT_DIR: Path = Path(os.getenv("IMAT_DIR", "/imat")).resolve() @@ -96,7 +94,9 @@ async def get_latest_imat_image( @ImatRouter.get("/imat/list-images", summary="List images in a directory") async def list_imat_images( - path: str = Query(..., description="Path to the directory containing images, relative to CEPH_DIR") + path: typing.Annotated[ + str, Query(..., description="Path to the directory containing images, relative to CEPH_DIR") + ], ) -> list[str]: """Return a sorted list of TIFF images in the given directory.""" from plotting_service.utils import safe_check_filepath @@ -105,23 +105,20 @@ async def list_imat_images( # Security: Ensure path is within CEPH_DIR try: safe_check_filepath(dir_path, CEPH_DIR) - except FileNotFoundError: - raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Directory not found") + except FileNotFoundError as err: + raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Directory not found") from err if not dir_path.exists() or not dir_path.is_dir(): raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Directory not found") - images = [] - for entry in dir_path.iterdir(): - if entry.is_file() and entry.suffix.lower() in IMAGE_SUFFIXES: - images.append(entry.name) + images = [entry.name for entry in dir_path.iterdir() if entry.is_file() and entry.suffix.lower() in IMAGE_SUFFIXES] return sorted(images) @ImatRouter.get("/imat/image", summary="Fetch a specific TIFF image as raw data") async def get_imat_image( - path: str = Query(..., description="Path to the TIFF image file, relative to CEPH_DIR"), + path: typing.Annotated[str, Query(..., description="Path to the TIFF image file, relative to CEPH_DIR")], downsample_factor: typing.Annotated[ int, Query( @@ -138,8 +135,8 @@ async def get_imat_image( # Security: Ensure path is within CEPH_DIR try: safe_check_filepath(image_path, CEPH_DIR) - except FileNotFoundError: - raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Directory not found") + except FileNotFoundError as err: + raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Directory not found") from err if not image_path.exists() or not image_path.is_file(): raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Image not found") @@ -151,11 +148,13 @@ async def get_imat_image( if downsample_factor > 1: target_width = max(1, round(original_width / downsample_factor)) target_height = max(1, round(original_height / downsample_factor)) - img = img.resize((target_width, target_height), Image.Resampling.NEAREST) + display_img = img.resize((target_width, target_height), Image.Resampling.NEAREST) + else: + display_img = img - sampled_width, sampled_height = img.size + sampled_width, sampled_height = display_img.size # For 16-bit TIFFs, tobytes() returns raw 16-bit bytes - data_bytes = img.tobytes() + data_bytes = display_img.tobytes() headers = { "X-Image-Width": str(sampled_width), @@ -163,7 +162,9 @@ async def get_imat_image( "X-Original-Width": str(original_width), "X-Original-Height": str(original_height), "X-Downsample-Factor": str(downsample_factor), - "Access-Control-Expose-Headers": "X-Image-Width, X-Image-Height, X-Original-Width, X-Original-Height, X-Downsample-Factor" + "Access-Control-Expose-Headers": ( + "X-Image-Width, X-Image-Height, X-Original-Width, X-Original-Height, X-Downsample-Factor" + ), } return Response( diff --git a/plotting-service/test/test_plotting_api.py b/plotting-service/test/test_plotting_api.py index b9fb074..c27ed7c 100644 --- a/plotting-service/test/test_plotting_api.py +++ b/plotting-service/test/test_plotting_api.py @@ -374,7 +374,9 @@ def test_get_latest_imat_image_conversion_error(tmp_path, monkeypatch): (rb_dir / "test.tif").touch() client = TestClient(plotting_api.app) - with mock.patch("plotting_service.routers.imat.convert_image_to_rgb_array", side_effect=Exception("Conversion failed")): + with mock.patch( + "plotting_service.routers.imat.convert_image_to_rgb_array", side_effect=Exception("Conversion failed") + ): response = client.get( "/imat/latest-image", params={"downsample_factor": 1}, From 329c60bdfafe73282b7d50565799580542079694 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 9 Feb 2026 14:34:47 +0000 Subject: [PATCH 6/7] Update ruff for local installs and fix new ruff issues --- plotting-service/plotting_service/routers/imat.py | 3 +-- plotting-service/pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/plotting-service/plotting_service/routers/imat.py b/plotting-service/plotting_service/routers/imat.py index 4dbe8d3..ff95a78 100644 --- a/plotting-service/plotting_service/routers/imat.py +++ b/plotting-service/plotting_service/routers/imat.py @@ -15,6 +15,7 @@ convert_image_to_rgb_array, find_latest_image_in_directory, ) +from plotting_service.utils import safe_check_filepath ImatRouter = APIRouter() @@ -99,7 +100,6 @@ async def list_imat_images( ], ) -> list[str]: """Return a sorted list of TIFF images in the given directory.""" - from plotting_service.utils import safe_check_filepath dir_path = (Path(CEPH_DIR) / path).resolve() # Security: Ensure path is within CEPH_DIR @@ -129,7 +129,6 @@ async def get_imat_image( ] = 1, ) -> Response: """Return the raw data of a specific TIFF image as binary.""" - from plotting_service.utils import safe_check_filepath image_path = (Path(CEPH_DIR) / path).resolve() # Security: Ensure path is within CEPH_DIR diff --git a/plotting-service/pyproject.toml b/plotting-service/pyproject.toml index 89428d3..4c27ee6 100644 --- a/plotting-service/pyproject.toml +++ b/plotting-service/pyproject.toml @@ -18,7 +18,7 @@ dependencies = [ [project.optional-dependencies] formatting = [ - "ruff==0.12.3", + "ruff==0.15.0", "mypy==1.17.0", "plotting-service[test]", "types-requests==2.32.4.20250611", From c1d1aab33218d179c8c10478a0311f1d0e746374 Mon Sep 17 00:00:00 2001 From: github-actions Date: Mon, 9 Feb 2026 14:35:20 +0000 Subject: [PATCH 7/7] Formatting and linting commit --- .../plotting_service/routers/imat.py | 22 ++++---- plotting-service/test/test_plotting_api.py | 54 ++++--------------- 2 files changed, 19 insertions(+), 57 deletions(-) diff --git a/plotting-service/plotting_service/routers/imat.py b/plotting-service/plotting_service/routers/imat.py index ff95a78..af0ba7d 100644 --- a/plotting-service/plotting_service/routers/imat.py +++ b/plotting-service/plotting_service/routers/imat.py @@ -119,14 +119,14 @@ async def list_imat_images( @ImatRouter.get("/imat/image", summary="Fetch a specific TIFF image as raw data") async def get_imat_image( path: typing.Annotated[str, Query(..., description="Path to the TIFF image file, relative to CEPH_DIR")], - downsample_factor: typing.Annotated[ - int, - Query( - ge=1, - le=64, - description="Integer factor to reduce each dimension by (1 keeps original resolution).", - ), - ] = 1, + downsample_factor: typing.Annotated[ + int, + Query( + ge=1, + le=64, + description="Integer factor to reduce each dimension by (1 keeps original resolution).", + ), + ] = 1, ) -> Response: """Return the raw data of a specific TIFF image as binary.""" @@ -166,11 +166,7 @@ async def get_imat_image( ), } - return Response( - content=data_bytes, - media_type="application/octet-stream", - headers=headers - ) + return Response(content=data_bytes, media_type="application/octet-stream", headers=headers) except Exception as exc: logger.error(f"Failed to process image {image_path}: {exc}") diff --git a/plotting-service/test/test_plotting_api.py b/plotting-service/test/test_plotting_api.py index c27ed7c..2e50477 100644 --- a/plotting-service/test/test_plotting_api.py +++ b/plotting-service/test/test_plotting_api.py @@ -218,11 +218,7 @@ def test_list_imat_images(tmp_path, monkeypatch): (data_dir / "not_an_image.txt").touch() client = TestClient(plotting_api.app) - response = client.get( - "/imat/list-images", - params={"path": "test_data"}, - headers={"Authorization": "Bearer foo"} - ) + response = client.get("/imat/list-images", params={"path": "test_data"}, headers={"Authorization": "Bearer foo"}) assert response.status_code == HTTPStatus.OK assert response.json() == ["image1.tif", "image2.tiff"] @@ -233,11 +229,7 @@ def test_list_imat_images_not_found(tmp_path, monkeypatch): monkeypatch.setattr(imat, "CEPH_DIR", str(tmp_path)) client = TestClient(plotting_api.app) - response = client.get( - "/imat/list-images", - params={"path": "non_existent"}, - headers={"Authorization": "Bearer foo"} - ) + response = client.get("/imat/list-images", params={"path": "non_existent"}, headers={"Authorization": "Bearer foo"}) assert response.status_code == HTTPStatus.NOT_FOUND @@ -248,11 +240,7 @@ def test_list_imat_images_forbidden(tmp_path, monkeypatch): client = TestClient(plotting_api.app) # safe_check_filepath should block this - response = client.get( - "/imat/list-images", - params={"path": "../.."}, - headers={"Authorization": "Bearer foo"} - ) + response = client.get("/imat/list-images", params={"path": "../.."}, headers={"Authorization": "Bearer foo"}) assert response.status_code == HTTPStatus.FORBIDDEN @@ -268,11 +256,7 @@ def test_get_imat_image(tmp_path, monkeypatch): image.close() client = TestClient(plotting_api.app) - response = client.get( - "/imat/image", - params={"path": "test.tif"}, - headers={"Authorization": "Bearer foo"} - ) + response = client.get("/imat/image", params={"path": "test.tif"}, headers={"Authorization": "Bearer foo"}) assert response.status_code == HTTPStatus.OK assert response.headers["X-Image-Width"] == "4" @@ -295,9 +279,7 @@ def test_get_imat_image_downsampled(tmp_path, monkeypatch): client = TestClient(plotting_api.app) response = client.get( - "/imat/image", - params={"path": "test.tif", "downsample_factor": 2}, - headers={"Authorization": "Bearer foo"} + "/imat/image", params={"path": "test.tif", "downsample_factor": 2}, headers={"Authorization": "Bearer foo"} ) assert response.status_code == HTTPStatus.OK @@ -312,10 +294,7 @@ def test_get_latest_imat_image_no_rb_folders(tmp_path, monkeypatch): monkeypatch.setattr(imat, "IMAT_DIR", tmp_path) client = TestClient(plotting_api.app) - response = client.get( - "/imat/latest-image", - headers={"Authorization": "Bearer foo"} - ) + response = client.get("/imat/latest-image", headers={"Authorization": "Bearer foo"}) assert response.status_code == HTTPStatus.NOT_FOUND assert "No RB folders" in response.json()["detail"] @@ -326,11 +305,7 @@ def test_get_imat_image_not_found(tmp_path, monkeypatch): monkeypatch.setattr(imat, "CEPH_DIR", str(tmp_path)) client = TestClient(plotting_api.app) - response = client.get( - "/imat/image", - params={"path": "not_there.tif"}, - headers={"Authorization": "Bearer foo"} - ) + response = client.get("/imat/image", params={"path": "not_there.tif"}, headers={"Authorization": "Bearer foo"}) assert response.status_code == HTTPStatus.NOT_FOUND @@ -341,10 +316,7 @@ def test_get_latest_imat_image_no_images_in_rb(tmp_path, monkeypatch): (tmp_path / "RB1234").mkdir() client = TestClient(plotting_api.app) - response = client.get( - "/imat/latest-image", - headers={"Authorization": "Bearer foo"} - ) + response = client.get("/imat/latest-image", headers={"Authorization": "Bearer foo"}) assert response.status_code == HTTPStatus.NOT_FOUND assert "No images found" in response.json()["detail"] @@ -356,11 +328,7 @@ def test_get_imat_image_internal_error(tmp_path, monkeypatch): client = TestClient(plotting_api.app) with mock.patch("PIL.Image.open", side_effect=Exception("Simulated failure")): - response = client.get( - "/imat/image", - params={"path": "corrupt.tif"}, - headers={"Authorization": "Bearer foo"} - ) + response = client.get("/imat/image", params={"path": "corrupt.tif"}, headers={"Authorization": "Bearer foo"}) assert response.status_code == HTTPStatus.INTERNAL_SERVER_ERROR assert "Unable to process image" in response.json()["detail"] @@ -378,9 +346,7 @@ def test_get_latest_imat_image_conversion_error(tmp_path, monkeypatch): "plotting_service.routers.imat.convert_image_to_rgb_array", side_effect=Exception("Conversion failed") ): response = client.get( - "/imat/latest-image", - params={"downsample_factor": 1}, - headers={"Authorization": "Bearer foo"} + "/imat/latest-image", params={"downsample_factor": 1}, headers={"Authorization": "Bearer foo"} ) assert response.status_code == HTTPStatus.INTERNAL_SERVER_ERROR