diff --git a/tests/conftest.py b/tests/conftest.py index aab4b374c..917686191 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -4,7 +4,9 @@ import os import shutil +import socket import time +from contextlib import closing from pathlib import Path from typing import TYPE_CHECKING @@ -16,6 +18,13 @@ from tiatoolbox.data import _fetch_remote_sample from tiatoolbox.utils.env_detection import has_gpu, running_on_ci +# Reserve a free port for tileserver tests +_TILES_ENV = "TIATOOLBOX_TILESERVER_PORT" +if _TILES_ENV not in os.environ: + with closing(socket.socket(socket.AF_INET, socket.SOCK_STREAM)) as sock: + sock.bind(("127.0.0.1", 0)) + os.environ[_TILES_ENV] = str(sock.getsockname()[1]) + if TYPE_CHECKING: from collections.abc import Callable diff --git a/tests/test_app_bokeh.py b/tests/test_app_bokeh.py index ce97fb2fd..d5ae40393 100644 --- a/tests/test_app_bokeh.py +++ b/tests/test_app_bokeh.py @@ -9,6 +9,7 @@ import re import time from pathlib import Path +from types import SimpleNamespace from typing import TYPE_CHECKING import bokeh.models as bkmodels @@ -25,7 +26,7 @@ from scipy.ndimage import label from tiatoolbox.data import _fetch_remote_sample -from tiatoolbox.visualization.bokeh_app import main +from tiatoolbox.visualization.bokeh_app import app_hooks, main from tiatoolbox.visualization.tileserver import TileServer from tiatoolbox.visualization.ui_utils import get_level_by_extent @@ -41,6 +42,13 @@ GRIDLINES = 2 +class _DummySessionContext: + """Simple shim matching the subset of Bokeh's SessionContext we use.""" + + def __init__(self: _DummySessionContext, user: str) -> None: + self.request = SimpleNamespace(arguments={"user": user}) + + # helper functions and fixtures def get_tile(layer: str, x: float, y: float, z: float, *, show: bool) -> np.ndarray: """Get a tile from the server. @@ -77,7 +85,9 @@ def get_renderer_prop(prop: str) -> json: The property to get. """ - resp = main.UI["s"].get(f"http://{main.host2}:5000/tileserver/renderer/{prop}") + resp = main.UI["s"].get( + f"http://{main.host2}:{main.port}/tileserver/renderer/{prop}", + ) return resp.json() @@ -144,7 +154,7 @@ def run_app() -> None: layers={}, ) CORS(app, send_wildcard=True) - app.run(host="127.0.0.1", threaded=True) + app.run(host="127.0.0.1", port=int(main.port), threaded=True) @pytest.fixture(scope="module") @@ -376,13 +386,17 @@ def test_type_cmap_select(doc: Document) -> None: # remove the type cmap cmap_select.value = [] - resp = main.UI["s"].get(f"http://{main.host2}:5000/tileserver/secondary_cmap") + resp = main.UI["s"].get( + f"http://{main.host2}:{main.port}/tileserver/secondary_cmap" + ) assert resp.json()["score_prop"] == "None" # check callback works regardless of order cmap_select.value = ["0"] cmap_select.value = ["0", "prob"] - resp = main.UI["s"].get(f"http://{main.host2}:5000/tileserver/secondary_cmap") + resp = main.UI["s"].get( + f"http://{main.host2}:{main.port}/tileserver/secondary_cmap" + ) assert resp.json()["score_prop"] == "prob" @@ -753,16 +767,16 @@ def test_cmap_select(doc: Document) -> None: main.UI["cprop_input"].value = ["prob"] # set to jet cmap_select.value = "jet" - resp = main.UI["s"].get(f"http://{main.host2}:5000/tileserver/cmap") + resp = main.UI["s"].get(f"http://{main.host2}:{main.port}/tileserver/cmap") assert resp.json() == "jet" # set to dict cmap_select.value = "dict" - resp = main.UI["s"].get(f"http://{main.host2}:5000/tileserver/cmap") + resp = main.UI["s"].get(f"http://{main.host2}:{main.port}/tileserver/cmap") assert isinstance(resp.json(), dict) main.UI["cprop_input"].value = ["type"] # should now be the type mapping - resp = main.UI["s"].get(f"http://{main.host2}:5000/tileserver/cmap") + resp = main.UI["s"].get(f"http://{main.host2}:{main.port}/tileserver/cmap") for key in main.UI["vstate"].mapper: assert str(key) in resp.json() assert np.all( @@ -770,7 +784,7 @@ def test_cmap_select(doc: Document) -> None: ) # set the cmap to "coolwarm" cmap_select.value = "coolwarm" - resp = main.UI["s"].get(f"http://{main.host2}:5000/tileserver/cmap") + resp = main.UI["s"].get(f"http://{main.host2}:{main.port}/tileserver/cmap") # as cprop is type (categorical), it should have had no effect for key in main.UI["vstate"].mapper: assert str(key) in resp.json() @@ -779,7 +793,7 @@ def test_cmap_select(doc: Document) -> None: ) main.UI["cprop_input"].value = ["prob"] - resp = main.UI["s"].get(f"http://{main.host2}:5000/tileserver/cmap") + resp = main.UI["s"].get(f"http://{main.host2}:{main.port}/tileserver/cmap") # should be coolwarm as that is the last cmap we set, and prob is continuous assert resp.json() == "coolwarm" @@ -825,3 +839,51 @@ def test_clearing_doc(doc: Document) -> None: """Test that the doc can be cleared.""" doc.clear() assert len(doc.roots) == 0 + + +def test_app_hooks_session_destroyed(monkeypatch: pytest.MonkeyPatch) -> None: + """Hook should call reset endpoint and exit.""" + recorded: dict[str, object] = {} + + def fake_get(url: str, *, timeout: int) -> None: + """Fake requests.get to record parameters.""" + recorded["url"] = url + recorded["timeout"] = timeout + + monkeypatch.setattr(app_hooks, "PORT", "6150") + monkeypatch.setattr(app_hooks.requests, "get", fake_get) + exited = False + + def fake_exit() -> None: + """Fake sys.exit to record call.""" + nonlocal exited + exited = True + + monkeypatch.setattr(app_hooks, "sys", SimpleNamespace(exit=fake_exit)) + app_hooks.on_session_destroyed(_DummySessionContext("user-1")) + assert recorded["url"] == "http://127.0.0.1:6150/tileserver/reset/user-1" + assert recorded["timeout"] == 5 + assert exited + + +def test_app_hooks_session_destroyed_suppresses_timeout( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """ReadTimeout should be suppressed and exit still called.""" + + def fake_get(*_: object, **__: object) -> None: + """Fake requests.get to raise ReadTimeout.""" + raise app_hooks.requests.exceptions.ReadTimeout # type: ignore[attr-defined] + + monkeypatch.setattr(app_hooks, "PORT", "6160") + monkeypatch.setattr(app_hooks.requests, "get", fake_get) + exited = False + + def fake_exit() -> None: + """Fake sys.exit to record call.""" + nonlocal exited + exited = True + + monkeypatch.setattr(app_hooks, "sys", SimpleNamespace(exit=fake_exit)) + app_hooks.on_session_destroyed(_DummySessionContext("user-2")) + assert exited diff --git a/tests/test_json_config_bokeh.py b/tests/test_json_config_bokeh.py index 2c3a16e7b..0434a1c9d 100644 --- a/tests/test_json_config_bokeh.py +++ b/tests/test_json_config_bokeh.py @@ -2,6 +2,7 @@ from __future__ import annotations +import os import time from contextlib import suppress from threading import Thread @@ -14,6 +15,8 @@ from tiatoolbox.cli.visualize import run_bokeh, run_tileserver from tiatoolbox.data import _fetch_remote_sample +PORT = os.environ.get("TIATOOLBOX_TILESERVER_PORT", "5000") + if TYPE_CHECKING: from pathlib import Path @@ -65,7 +68,10 @@ def bk_session(data_path: dict[str, Path]) -> ClientSession: yield session session.close() with suppress(requests.exceptions.ConnectionError): - requests.post("http://localhost:5000/tileserver/shutdown", timeout=2) + requests.post( + f"http://localhost:{PORT}/tileserver/shutdown", + timeout=2, + ) def test_slides_available(bk_session: ClientSession) -> None: diff --git a/tests/test_server_bokeh.py b/tests/test_server_bokeh.py index 9544abc6d..4cc74de20 100644 --- a/tests/test_server_bokeh.py +++ b/tests/test_server_bokeh.py @@ -2,6 +2,7 @@ from __future__ import annotations +import os import time from contextlib import suppress from threading import Thread @@ -16,6 +17,8 @@ from tiatoolbox.cli.visualize import run_bokeh, run_tileserver from tiatoolbox.data import _fetch_remote_sample +PORT = os.environ.get("TIATOOLBOX_TILESERVER_PORT", "5000") + if TYPE_CHECKING: from pathlib import Path @@ -72,7 +75,10 @@ def bk_session(data_path: dict[str, Path]) -> ClientSession: yield session session.close() with suppress(requests.exceptions.ConnectionError): - requests.post("http://localhost:5000/tileserver/shutdown", timeout=2) + requests.post( + f"http://localhost:{PORT}/tileserver/shutdown", + timeout=2, + ) def test_slides_available(bk_session: ClientSession) -> None: diff --git a/tiatoolbox/annotation/storage.py b/tiatoolbox/annotation/storage.py index 012ea23d2..001403685 100644 --- a/tiatoolbox/annotation/storage.py +++ b/tiatoolbox/annotation/storage.py @@ -2861,6 +2861,33 @@ def _initialize_query_string_parameters( return query_string, query_parameters + @staticmethod + def _warn_if_query_not_using_index( + cur: sqlite3.Cursor, + query_string: str, + query_parameters: dict[str, object], + ) -> None: + """Log a warning when SQLite does not use an index.""" + query_plan = cur.execute( + "EXPLAIN QUERY PLAN " + query_string, + query_parameters, + ).fetchall() + uses_index = False + for plan in query_plan: + detail = str(plan[-1]).upper() + # macOS SQLite may report "USING COVERING INDEX". + if "AUTOMATIC" not in detail and ( + "USING INDEX" in detail or "USING COVERING INDEX" in detail + ): + uses_index = True + break + if not uses_index: + logger.warning( + "Query is not using an index. " + "Consider adding an index to improve performance.", + stacklevel=2, + ) + def _query( self: SQLiteStore, columns: str, @@ -2970,16 +2997,7 @@ def _query( # Warn if the query is not using an index if index_warning: - query_plan = cur.execute( - "EXPLAIN QUERY PLAN " + query_string, - query_parameters, - ).fetchone() - if "USING INDEX" not in query_plan[-1]: - logger.warning( - "Query is not using an index. " - "Consider adding an index to improve performance.", - stacklevel=2, - ) + self._warn_if_query_not_using_index(cur, query_string, query_parameters) # if area column exists, sort annotations by area if "area" in self.table_columns: query_string += "\nORDER BY area DESC" diff --git a/tiatoolbox/cli/visualize.py b/tiatoolbox/cli/visualize.py index 30627dcf2..336f2d9d3 100644 --- a/tiatoolbox/cli/visualize.py +++ b/tiatoolbox/cli/visualize.py @@ -26,7 +26,8 @@ def run_app() -> None: layers={}, ) CORS(app, send_wildcard=True) - app.run(host="127.0.0.1", threaded=True) + port = int(os.environ.get("TIATOOLBOX_TILESERVER_PORT", "5000")) + app.run(host="127.0.0.1", port=port, threaded=True) proc = Thread(target=run_app, daemon=True) proc.start() diff --git a/tiatoolbox/visualization/bokeh_app/app_hooks.py b/tiatoolbox/visualization/bokeh_app/app_hooks.py index 21b25e526..d38f45672 100644 --- a/tiatoolbox/visualization/bokeh_app/app_hooks.py +++ b/tiatoolbox/visualization/bokeh_app/app_hooks.py @@ -1,15 +1,21 @@ """Hooks to be executed upon specific events in bokeh app.""" +import os import sys from contextlib import suppress import requests from bokeh.application.application import SessionContext +PORT = os.environ.get("TIATOOLBOX_TILESERVER_PORT", "5000") + def on_session_destroyed(session_context: SessionContext) -> None: """Hook to be executed when a session is destroyed.""" user = session_context.request.arguments["user"] with suppress(requests.exceptions.ReadTimeout): - requests.get(f"http://127.0.0.1:5000/tileserver/reset/{user}", timeout=5) + requests.get( + f"http://127.0.0.1:{PORT}/tileserver/reset/{user}", + timeout=5, + ) sys.exit() diff --git a/tiatoolbox/visualization/bokeh_app/main.py b/tiatoolbox/visualization/bokeh_app/main.py index 5df7acb04..832d8661d 100644 --- a/tiatoolbox/visualization/bokeh_app/main.py +++ b/tiatoolbox/visualization/bokeh_app/main.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import os import sys import tempfile import urllib @@ -306,7 +307,7 @@ def get_mapper_for_prop(prop: str, mapper_type: str = "auto") -> str | dict[str, UI["vstate"].is_categorical = True return UI["vstate"].mapper # Find out the unique values of the chosen property - resp = UI["s"].get(f"http://{host2}:5000/tileserver/prop_values/{prop}/all") + resp = UI["s"].get(f"http://{host2}:{port}/tileserver/prop_values/{prop}/all") prop_vals = json.loads(resp.text) # If auto, guess what cmap should be if ( @@ -344,12 +345,12 @@ def update_renderer(prop: str, value: Any) -> None: # noqa: ANN401 # Send keys and values separately so types are preserved value = {"keys": list(value.keys()), "values": list(value.values())} UI["s"].put( - f"http://{host2}:5000/tileserver/cmap", + f"http://{host2}:{port}/tileserver/cmap", data={"cmap": json.dumps(value)}, ) return UI["s"].put( - f"http://{host2}:5000/tileserver/renderer/{prop}", + f"http://{host2}:{port}/tileserver/renderer/{prop}", data={"val": json.dumps(value)}, ) @@ -774,7 +775,7 @@ def cprop_input_cb(attr: str, old: str, new: list[str]) -> None: # noqa: ARG001 UI["vstate"].cprop = new[0] update_renderer("mapper", cmap) UI["s"].put( - f"http://{host2}:5000/tileserver/color_prop", + f"http://{host2}:{port}/tileserver/color_prop", data={"prop": json.dumps(new[0])}, ) UI["vstate"].update_state = 1 @@ -882,7 +883,7 @@ def slide_select_cb(attr: str, old: str, new: str) -> None: # noqa: ARG001 UI["vstate"].wsi = WSIReader.open(slide_path) initialise_slide() fname = make_safe_name(str(slide_path)) - UI["s"].put(f"http://{host2}:5000/tileserver/slide", data={"slide_path": fname}) + UI["s"].put(f"http://{host2}:{port}/tileserver/slide", data={"slide_path": fname}) change_tiles("slide") # Load the overlay and graph automatically if set in config @@ -974,7 +975,7 @@ def handle_graph_layer(attr: MenuItemClick) -> None: # skipcq: PY-R1000 def update_ui_on_new_annotations(ann_types: list[str]) -> None: """Update the UI when new annotations are added.""" UI["vstate"].types = ann_types - props = UI["s"].get(f"http://{host2}:5000/tileserver/prop_names/all") + props = UI["s"].get(f"http://{host2}:{port}/tileserver/prop_names/all") UI["vstate"].props = json.loads(props.text) # Update the color type by prop menu UI["type_cmap_select"].options = list(UI["vstate"].types) @@ -1011,7 +1012,7 @@ def layer_drop_cb(attr: MenuItemClick) -> None: # Otherwise it's a tile-based overlay of some form fname = make_safe_name(attr.item) resp = UI["s"].put( - f"http://{host2}:5000/tileserver/overlay", + f"http://{host2}:{port}/tileserver/overlay", data={"overlay_path": fname}, ) resp = json.loads(resp.text) @@ -1129,7 +1130,7 @@ def type_cmap_cb(attr: str, old: list[str], new: list[str]) -> None: # noqa: AR # Remove type-specific coloring UI["type_cmap_select"].options = [*UI["vstate"].types, "graph_overlay"] UI["s"].put( - f"http://{host2}:5000/tileserver/secondary_cmap", + f"http://{host2}:{port}/tileserver/secondary_cmap", data={ "type_id": json.dumps("None"), "prop": "None", @@ -1172,7 +1173,7 @@ def type_cmap_cb(attr: str, old: list[str], new: list[str]) -> None: # noqa: AR return cmap = get_mapper_for_prop(new[1]) # separate cmap select ? UI["s"].put( - f"http://{host2}:5000/tileserver/secondary_cmap", + f"http://{host2}:{port}/tileserver/secondary_cmap", data={ "type_id": json.dumps(UI["vstate"].orig_types.get(new[0], new[0])), "prop": new[1], @@ -1197,14 +1198,16 @@ def save_cb(attr: ButtonClick) -> None: # noqa: ARG001 ), ) UI["s"].post( - f"http://{host2}:5000/tileserver/commit", + f"http://{host2}:{port}/tileserver/commit", data={"save_path": save_path}, ) def tap_event_cb(event: DoubleTap) -> None: """Callback to handle double tap events to inspect annotations.""" - resp = UI["s"].get(f"http://{host2}:5000/tileserver/tap_query/{event.x}/{-event.y}") + resp = UI["s"].get( + f"http://{host2}:{port}/tileserver/tap_query/{event.x}/{-event.y}" + ) data_dict = json.loads(resp.text) popup_table.source.data = { @@ -1262,7 +1265,7 @@ def segment_on_box() -> None: fname = make_safe_name(tmp_save_dir / "hover_out" / "0.dat") resp = UI["s"].put( - f"http://{host2}:5000/tileserver/annotations", + f"http://{host2}:{port}/tileserver/annotations", data={"file_path": fname, "model_mpp": json.dumps(UI["vstate"].model_mpp)}, ) ann_types = json.loads(resp.text) @@ -1767,13 +1770,15 @@ def make_window(vstate: ViewerState) -> dict: # noqa: PLR0915 # Set up a session for communicating with tile server s = requests.Session() + s.trust_env = False # bypass system proxies for local tile server requests + s.proxies.update({"http": None, "https": None}) retries = Retry( total=5, backoff_factor=0.1, ) s.mount("http://", HTTPAdapter(max_retries=retries)) - resp = s.get(f"http://{host2}:5000/tileserver/session_id") + resp = s.get(f"http://{host2}:{port}/tileserver/session_id") user = resp.cookies.get("session_id") if curdoc().session_context: curdoc().session_context.request.arguments["user"] = user @@ -1943,7 +1948,7 @@ def make_window(vstate: ViewerState) -> dict: # noqa: PLR0915 # Set hosts and ports host = "127.0.0.1" host2 = "127.0.0.1" -port = "5000" +port = os.environ.get("TIATOOLBOX_TILESERVER_PORT", "5000") def update() -> None: @@ -2006,7 +2011,7 @@ def setup_config_ui_settings(config: dict) -> None: update_renderer(k, config["UI_settings"][k]) if "default_cprop" in config and config["default_cprop"] is not None: UI["s"].put( - f"http://{host2}:5000/tileserver/color_prop", + f"http://{host2}:{port}/tileserver/color_prop", data={"prop": json.dumps(config["default_cprop"])}, ) # Open up initial slide