diff --git a/openadapt_capture/__init__.py b/openadapt_capture/__init__.py index e44acc3..5d1b48c 100644 --- a/openadapt_capture/__init__.py +++ b/openadapt_capture/__init__.py @@ -70,9 +70,17 @@ # Recorder requires pynput which needs a display server (X11/Wayland/macOS/Windows). # Make it optional so the package is importable in headless environments (CI, servers). +# The recorder must never take a screenshot at import time, but guard against a +# display/screenshot failure (mss ScreenShotError) as well as a missing dependency +# so a headless import degrades to ``Recorder = None`` instead of crashing. +try: + from mss.exception import ScreenShotError as _ScreenShotError +except ImportError: # pragma: no cover - mss is a hard dependency + _ScreenShotError = () # type: ignore[assignment,misc] + try: from openadapt_capture.recorder import Recorder -except ImportError: +except (ImportError, OSError, _ScreenShotError): Recorder = None # type: ignore[assignment,misc] # Performance statistics diff --git a/openadapt_capture/db/__init__.py b/openadapt_capture/db/__init__.py index e805ab0..0884a73 100644 --- a/openadapt_capture/db/__init__.py +++ b/openadapt_capture/db/__init__.py @@ -4,7 +4,7 @@ """ import sqlalchemy as sa -from sqlalchemy import create_engine +from sqlalchemy import create_engine, inspect, text from sqlalchemy.ext.declarative import declarative_base from sqlalchemy.orm import sessionmaker from sqlalchemy.schema import MetaData @@ -73,6 +73,46 @@ def get_session_maker(engine: sa.engine) -> sessionmaker: return sessionmaker(bind=engine) +def migrate_missing_columns(engine: sa.engine) -> None: + """Add columns present in the models but missing from an existing DB. + + Per-capture SQLite databases have no migration framework: each capture + gets a fresh schema via ``create_all``. When the models gain a new + column, older ``recording.db`` files predate it, so loading them would + fail with ``no such column``. SQLite supports cheap + ``ALTER TABLE ... ADD COLUMN``, so we reconcile missing columns here. + + Strictly additive: it never drops or alters existing columns, and adds + new columns without a DEFAULT so pre-existing rows read back as NULL + (letting callers fall back to legacy sources like the config JSON). + + Args: + engine: Engine bound to the per-capture SQLite database. + """ + # Import models to ensure the tables are registered with Base. + from openadapt_capture.db import models # noqa: F401 + + inspector = inspect(engine) + existing_tables = set(inspector.get_table_names()) + + with engine.begin() as conn: + for table in Base.metadata.sorted_tables: + if table.name not in existing_tables: + # create_all handles brand-new tables. + continue + existing_cols = {c["name"] for c in inspector.get_columns(table.name)} + for column in table.columns: + if column.name in existing_cols: + continue + col_type = column.type.compile(dialect=engine.dialect) + conn.execute( + text( + f'ALTER TABLE "{table.name}" ' + f'ADD COLUMN "{column.name}" {col_type}' + ) + ) + + def create_db(db_path: str, echo: bool = False) -> tuple: """Create a new database at the given path, returning (engine, Session). @@ -92,6 +132,8 @@ def create_db(db_path: str, echo: bool = False) -> tuple: from openadapt_capture.db import models # noqa: F401 Base.metadata.create_all(engine) + # Reconcile schemas of pre-existing DBs that predate newer columns. + migrate_missing_columns(engine) Session = get_session_maker(engine) return engine, Session @@ -111,5 +153,9 @@ def get_session_for_path(db_path: str, echo: bool = False): """ db_url = f"sqlite:///{db_path}" engine = get_engine(db_url, echo=echo) + # Older recording.db files may predate columns the models now expect; + # add any missing ones so loading them does not fail with 'no such + # column'. Safe/no-op when the schema is already current. + migrate_missing_columns(engine) Session = get_session_maker(engine) return Session() diff --git a/openadapt_capture/db/models.py b/openadapt_capture/db/models.py index 381b588..60c0768 100644 --- a/openadapt_capture/db/models.py +++ b/openadapt_capture/db/models.py @@ -39,6 +39,10 @@ class Recording(Base): timestamp = sa.Column(ForceFloat) monitor_width = sa.Column(sa.Integer) monitor_height = sa.Column(sa.Integer) + # Physical/logical display pixel ratio (e.g. 2.0 on Retina/HiDPI). + # Nullable so older recording.db files (which predate this column and + # get NULL from the additive migration) fall back to the config JSON. + pixel_ratio = sa.Column(sa.Float) double_click_interval_seconds = sa.Column(sa.Numeric(asdecimal=False)) double_click_distance_pixels = sa.Column(sa.Numeric(asdecimal=False)) platform = sa.Column(sa.String) diff --git a/openadapt_capture/recorder.py b/openadapt_capture/recorder.py index 3362ae7..284fc99 100644 --- a/openadapt_capture/recorder.py +++ b/openadapt_capture/recorder.py @@ -32,7 +32,7 @@ from pynput import keyboard, mouse from tqdm import tqdm -from openadapt_capture import plotting, utils, video, window +from openadapt_capture import platform, plotting, utils, video, window from openadapt_capture.config import config from openadapt_capture.db import create_db, crud, get_session_for_path from openadapt_capture.db.models import ActionEvent, Recording @@ -1016,6 +1016,7 @@ def create_recording( timestamp = utils.set_start_time() monitor_width, monitor_height = utils.get_monitor_dims() + pixel_ratio = platform.get_display_pixel_ratio() double_click_distance_pixels = utils.get_double_click_distance_pixels() double_click_interval_seconds = utils.get_double_click_interval_seconds() recording_data = { @@ -1023,6 +1024,7 @@ def create_recording( "timestamp": timestamp, "monitor_width": monitor_width, "monitor_height": monitor_height, + "pixel_ratio": pixel_ratio, "double_click_distance_pixels": double_click_distance_pixels, "double_click_interval_seconds": double_click_interval_seconds, "platform": sys.platform, diff --git a/tests/test_headless_import.py b/tests/test_headless_import.py index c7e4172..b265cca 100644 --- a/tests/test_headless_import.py +++ b/tests/test_headless_import.py @@ -15,6 +15,9 @@ from __future__ import annotations import ast +import subprocess +import sys +import textwrap from pathlib import Path PACKAGE_ROOT = Path(__file__).resolve().parent.parent / "openadapt_capture" @@ -56,3 +59,66 @@ def test_no_display_calls_at_module_scope(): "(CI, servers, containers). Move them inside the function that " "uses them:\n " + "\n ".join(problems) ) + + +# Runtime complement to the static check above: import the package fresh in a +# subprocess where the display is simulated as unavailable (mss.grab raises +# ScreenShotError and utils.take_screenshot raises), and assert the import and +# the high-level load API still succeed. This reproduces a genuinely headless +# host and fails if any import-time code path touches the screen. +_HEADLESS_IMPORT_SCRIPT = textwrap.dedent( + """ + import mss + import mss.exception + + class _HeadlessSct: + # monitors[0] reports a zero-size region, as headless hosts do. + monitors = [{"left": 0, "top": 0, "width": 0, "height": 0}] + + def grab(self, monitor): + raise mss.exception.ScreenShotError("headless: no display") + + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + # Any attempt to screenshot at import time now fails. + mss.mss = lambda *a, **k: _HeadlessSct() + + import openadapt_capture + from openadapt_capture import Capture, CaptureSession + + # take_screenshot must also raise, proving nothing depends on it at import. + from openadapt_capture import utils + + def _boom(*a, **k): + raise mss.exception.ScreenShotError("headless: no display") + + utils.take_screenshot = _boom + + assert CaptureSession is not None + assert Capture is not None + print("HEADLESS_IMPORT_OK") + """ +) + + +def test_package_imports_when_screenshot_fails(): + """`import openadapt_capture` must not crash on a headless host. + + Simulates a host with no usable display (mss.grab raises + ScreenShotError) and imports the package + high-level API fresh in a + subprocess. + """ + result = subprocess.run( + [sys.executable, "-c", _HEADLESS_IMPORT_SCRIPT], + capture_output=True, + text=True, + ) + assert result.returncode == 0, ( + "Importing openadapt_capture crashed on a simulated headless host.\n" + f"stdout:\n{result.stdout}\nstderr:\n{result.stderr}" + ) + assert "HEADLESS_IMPORT_OK" in result.stdout diff --git a/tests/test_highlevel.py b/tests/test_highlevel.py index b391377..27641f4 100644 --- a/tests/test_highlevel.py +++ b/tests/test_highlevel.py @@ -483,3 +483,116 @@ def test_config_override_restores_on_exception(self): raise ValueError("test") assert config.RECORD_VIDEO == original_video + + +class TestPixelRatio: + """Tests for pixel_ratio persistence on the SQLAlchemy Recording model.""" + + def test_pixel_ratio_round_trips_through_model(self, temp_capture_dir): + """A HiDPI pixel_ratio written to the model survives load.""" + import os + import sqlite3 + import sys + + capture_path = str(Path(temp_capture_dir) / "capture") + os.makedirs(capture_path, exist_ok=True) + db_path = os.path.join(capture_path, "recording.db") + engine, Session = create_db(db_path) + session = Session() + crud.insert_recording( + session, + { + "timestamp": time.time(), + "monitor_width": 3840, + "monitor_height": 2160, + "pixel_ratio": 2.0, + "double_click_interval_seconds": 0.5, + "double_click_distance_pixels": 5, + "platform": sys.platform, + "task_description": "HiDPI", + }, + ) + session.close() + + # The column is real: it is present in the on-disk schema. + con = sqlite3.connect(db_path) + cols = {row[1] for row in con.execute("PRAGMA table_info(recording)")} + con.close() + assert "pixel_ratio" in cols + + capture = Capture.load(capture_path) + assert capture.pixel_ratio == 2.0 + capture.close() + + def test_old_recording_without_column_loads(self, temp_capture_dir): + """A recording.db predating the pixel_ratio column still loads. + + The additive migration adds the column (NULL for existing rows) so + the query does not fail, and pixel_ratio falls back to the config + JSON, then to 1.0 when genuinely unknown. + """ + import os + import sqlite3 + import sys + + # config-JSON fallback preserved. + capture_path = str(Path(temp_capture_dir) / "old_with_config") + os.makedirs(capture_path, exist_ok=True) + db_path = os.path.join(capture_path, "recording.db") + engine, Session = create_db(db_path) + session = Session() + crud.insert_recording( + session, + { + "timestamp": time.time(), + "monitor_width": 2560, + "monitor_height": 1440, + "double_click_interval_seconds": 0.5, + "double_click_distance_pixels": 5, + "platform": sys.platform, + "task_description": "old", + "config": {"pixel_ratio": 1.5}, + }, + ) + session.close() + engine.dispose() + + # Simulate an old DB that predates the column by dropping it. + con = sqlite3.connect(db_path) + con.execute("ALTER TABLE recording DROP COLUMN pixel_ratio") + con.commit() + con.close() + + capture = Capture.load(capture_path) + assert capture.pixel_ratio == 1.5 # recovered from config JSON + capture.close() + + # No column and no config -> genuinely unknown -> 1.0. + capture_path2 = str(Path(temp_capture_dir) / "old_no_config") + os.makedirs(capture_path2, exist_ok=True) + db_path2 = os.path.join(capture_path2, "recording.db") + engine2, Session2 = create_db(db_path2) + session2 = Session2() + crud.insert_recording( + session2, + { + "timestamp": time.time(), + "monitor_width": 1920, + "monitor_height": 1080, + "double_click_interval_seconds": 0.5, + "double_click_distance_pixels": 5, + "platform": sys.platform, + "task_description": "old2", + }, + ) + session2.close() + engine2.dispose() + + con = sqlite3.connect(db_path2) + con.execute("ALTER TABLE recording DROP COLUMN pixel_ratio") + con.commit() + con.close() + + capture2 = Capture.load(capture_path2) + assert capture2.pixel_ratio == 1.0 + capture2.close()